diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index 3b4fd17f8..7a9c6e7b3 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -11,6 +11,7 @@ using Altinn.App.Core.Constants; using Altinn.App.Core.Extensions; using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Auth; using Altinn.App.Core.Helpers; using Altinn.App.Core.Helpers.Serialization; using Altinn.App.Core.Internal.App; @@ -70,7 +71,7 @@ public class InstancesController : ControllerBase private readonly ModelSerializationService _serializationService; private readonly InternalPatchService _patchService; private readonly InstanceDataUnitOfWorkInitializer _instanceDataUnitOfWorkInitializer; - + private readonly IAuthenticationContext _authenticationContext; private const long RequestSizeLimit = 2000 * 1024 * 1024; /// @@ -93,7 +94,8 @@ public InstancesController( IHostEnvironment env, ModelSerializationService serializationService, InternalPatchService patchService, - IServiceProvider serviceProvider + IServiceProvider serviceProvider, + IAuthenticationContext authenticationContext ) { _logger = logger; @@ -114,6 +116,7 @@ IServiceProvider serviceProvider _serializationService = serializationService; _patchService = patchService; _instanceDataUnitOfWorkInitializer = serviceProvider.GetRequiredService(); + _authenticationContext = authenticationContext; } /// @@ -597,7 +600,8 @@ await _processEngine.HandleEventsAndUpdateStorage( /// /// This method handles the copy endpoint for when a user wants to create a copy of an existing instance. - /// The endpoint will primarily be accessed directly by a user clicking the copy button for an archived instance. + /// The endpoint will primarily be accessed directly by a user clicking the copy button for an archived instance + /// from the message box in the Altinn 2 portal/Altinn 3 arbeidsflate. /// /// Unique identifier of the organisation responsible for the app /// Application identifier which is unique within an organisation @@ -607,9 +611,13 @@ await _processEngine.HandleEventsAndUpdateStorage( /// /// The endpoint will return a redirect to the new instance if the copy operation was successful. /// - [Obsolete("This endpoint will be removed in a future release of the app template packages.")] [ApiExplorerSettings(IgnoreApi = true)] [Authorize] + // The URL contains "legacy", but it is not really legacy. + // Originally it was thought of as tech debt to do mutation like this in a GET endpoint, + // but after further consideration, it was decided to keep it as is. + // A related topic is the fact that Altinn tokens are "global" and not scoped to a specific app. + // Since it now would be a breaking change to rename or remove, it still has "legacy" as part of the name. [HttpGet("/{org}/{app}/legacy/instances/{instanceOwnerPartyId:int}/{instanceGuid:guid}/copy")] [ProducesResponseType(typeof(Instance), StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] @@ -621,9 +629,9 @@ [FromRoute] Guid instanceGuid ) { // This endpoint should be used exclusively by end users. Ideally from a browser as a request after clicking - // a button in the message box, but for now we simply just exclude app owner(s). - string? orgClaim = User.GetOrg(); - if (orgClaim is not null) + // a button in the message box. + var auth = _authenticationContext.Current; + if (auth is not Authenticated.User and not Authenticated.SelfIdentifiedUser) { return Forbid(); } diff --git a/src/Altinn.App.Core/Features/Auth/Authenticated.cs b/src/Altinn.App.Core/Features/Auth/Authenticated.cs index 1cf4cc4ac..6eeffaec4 100644 --- a/src/Altinn.App.Core/Features/Auth/Authenticated.cs +++ b/src/Altinn.App.Core/Features/Auth/Authenticated.cs @@ -336,6 +336,11 @@ public sealed class SelfIdentifiedUser : Authenticated /// public string AuthenticationMethod { get; } + /// + /// True if the user was authenticated through the Altinn portal + /// + public bool InAltinnPortal { get; } + private Details? _extra; private readonly Func> _getUserProfile; private readonly ApplicationMetadata _appMetadata; @@ -355,6 +360,7 @@ ref ParseContext context AuthenticationMethod = authenticationMethod; // Since they are self-identified, they are always 0 AuthenticationLevel = 0; + InAltinnPortal = context.IsInAltinnPortal; _getUserProfile = context.GetUserProfile; _appMetadata = context.AppMetadata; } diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesControllerFixture.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesControllerFixture.cs index d411b0619..f30410632 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesControllerFixture.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesControllerFixture.cs @@ -2,6 +2,7 @@ using Altinn.App.Api.Helpers.Patch; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Auth; using Altinn.App.Core.Helpers.Serialization; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; @@ -55,7 +56,7 @@ public void VerifyNoOtherCalls( public void Dispose() => (ServiceProvider as IDisposable)?.Dispose(); - internal static InstancesControllerFixture Create() + internal static InstancesControllerFixture Create(Authenticated? auth = null) { var services = new ServiceCollection(); services.AddLogging(logging => logging.AddProvider(NullLoggerProvider.Instance)); @@ -86,6 +87,11 @@ internal static InstancesControllerFixture Create() services.AddTransient(); services.AddTransient(); + Mock authenticationContextMock = new(); + services.AddSingleton(authenticationContextMock.Object); + if (auth is not null) + authenticationContextMock.Setup(m => m.Current).Returns(auth); + services.AddTransient(sp => { var controller = ActivatorUtilities.CreateInstance(sp); diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs index ccd723cfc..f3c43ea15 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -24,7 +24,8 @@ public class InstancesController_CopyInstanceTests public async Task CopyInstance_CopyInstanceNotDefined_ReturnsBadRequest() { // Arrange - using var fixture = InstancesControllerFixture.Create(); + var auth = TestAuthentication.GetUserAuthentication(userPartyId: 343234); + using var fixture = InstancesControllerFixture.Create(auth); ApplicationMetadata application = new("ttd/copy-instance") { }; fixture.Mock().Setup(a => a.GetApplicationMetadata()).ReturnsAsync(application); @@ -45,7 +46,8 @@ public async Task CopyInstance_CopyInstanceNotDefined_ReturnsBadRequest() public async Task CopyInstance_CopyInstanceNotEnabled_ReturnsBadRequest() { // Arrange - using var fixture = InstancesControllerFixture.Create(); + var auth = TestAuthentication.GetUserAuthentication(userPartyId: 343234); + using var fixture = InstancesControllerFixture.Create(auth); const string Org = "ttd"; const string AppName = "copy-instance"; fixture @@ -70,7 +72,8 @@ public async Task CopyInstance_CopyInstanceNotEnabled_ReturnsBadRequest() public async Task CopyInstance_AsAppOwner_ReturnsForbidResult() { // Arrange - using var fixture = InstancesControllerFixture.Create(); + var auth = TestAuthentication.GetServiceOwnerAuthentication(org: "ttd"); + using var fixture = InstancesControllerFixture.Create(auth); fixture .Mock() .Setup(httpContext => httpContext.User) @@ -91,13 +94,14 @@ public async Task CopyInstance_AsAppOwner_ReturnsForbidResult() public async Task CopyInstance_AsUnauthorized_ReturnsForbidden() { // Arrange - using var fixture = InstancesControllerFixture.Create(); - const string Org = "ttd"; - const string AppName = "copy-instance"; + var auth = TestAuthentication.GetUserAuthentication(userPartyId: 65434312); + using var fixture = InstancesControllerFixture.Create(auth); fixture .Mock() .Setup(httpContext => httpContext.User) - .Returns(TestAuthentication.GetUserPrincipal(1337)); + .Returns(TestAuthentication.GetUserPrincipal(65434312)); + const string Org = "ttd"; + const string AppName = "copy-instance"; fixture .Mock() .Setup(a => a.GetApplicationMetadata()) @@ -125,7 +129,6 @@ public async Task CopyInstance_AsUnauthorized_ReturnsForbidden() public async Task CopyInstance_InstanceNotArchived_ReturnsBadRequest() { // Arrange - using var fixture = InstancesControllerFixture.Create(); const string Org = "ttd"; const string AppName = "copy-instance"; int instanceOwnerPartyId = 343234; @@ -135,11 +138,13 @@ public async Task CopyInstance_InstanceNotArchived_ReturnsBadRequest() Id = $"{instanceOwnerPartyId}/{instanceGuid}", Status = new InstanceStatus() { IsArchived = false }, }; - + var auth = TestAuthentication.GetUserAuthentication(userPartyId: instanceOwnerPartyId); + using var fixture = InstancesControllerFixture.Create(auth); fixture .Mock() .Setup(httpContext => httpContext.User) - .Returns(TestAuthentication.GetUserPrincipal(1337, instanceOwnerPartyId)); + .Returns(TestAuthentication.GetUserPrincipal(partyId: instanceOwnerPartyId)); + fixture .Mock() .Setup(a => a.GetApplicationMetadata()) @@ -172,11 +177,12 @@ public async Task CopyInstance_InstanceNotArchived_ReturnsBadRequest() public async Task CopyInstance_InstanceDoesNotExists_ReturnsBadRequest() { // Arrange - using var fixture = InstancesControllerFixture.Create(); const string Org = "ttd"; const string AppName = "copy-instance"; int instanceOwnerPartyId = 343234; Guid instanceGuid = Guid.NewGuid(); + var auth = TestAuthentication.GetUserAuthentication(userPartyId: instanceOwnerPartyId); + using var fixture = InstancesControllerFixture.Create(auth); // Storage returns Forbidden if the given instance id is wrong. PlatformHttpException platformHttpException = await PlatformHttpException.CreateAsync( @@ -186,7 +192,7 @@ public async Task CopyInstance_InstanceDoesNotExists_ReturnsBadRequest() fixture .Mock() .Setup(httpContext => httpContext.User) - .Returns(TestAuthentication.GetUserPrincipal(1337, instanceOwnerPartyId)); + .Returns(TestAuthentication.GetUserPrincipal(partyId: instanceOwnerPartyId)); fixture .Mock() .Setup(a => a.GetApplicationMetadata()) @@ -219,11 +225,12 @@ public async Task CopyInstance_InstanceDoesNotExists_ReturnsBadRequest() public async Task CopyInstance_PlatformReturnsError_ThrowsException() { // Arrange - using var fixture = InstancesControllerFixture.Create(); const string Org = "ttd"; const string AppName = "copy-instance"; int instanceOwnerPartyId = 343234; Guid instanceGuid = Guid.NewGuid(); + var auth = TestAuthentication.GetUserAuthentication(userPartyId: instanceOwnerPartyId); + using var fixture = InstancesControllerFixture.Create(auth); // Simulate a BadGateway respons from Platform PlatformHttpException platformHttpException = await PlatformHttpException.CreateAsync( @@ -233,7 +240,7 @@ public async Task CopyInstance_PlatformReturnsError_ThrowsException() fixture .Mock() .Setup(httpContext => httpContext.User) - .Returns(TestAuthentication.GetUserPrincipal(1337, instanceOwnerPartyId)); + .Returns(TestAuthentication.GetUserPrincipal(partyId: instanceOwnerPartyId)); fixture .Mock() .Setup(a => a.GetApplicationMetadata()) @@ -247,22 +254,13 @@ public async Task CopyInstance_PlatformReturnsError_ThrowsException() .Setup(i => i.GetInstance(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ThrowsAsync(platformHttpException); - PlatformHttpException? actual = null; - // Act - try - { - var controller = fixture.ServiceProvider.GetRequiredService(); - await controller.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid); - } - catch (PlatformHttpException phe) - { - actual = phe; - } + var controller = fixture.ServiceProvider.GetRequiredService(); + var actual = await Assert.ThrowsAsync( + async () => await controller.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid) + ); // Assert - Assert.NotNull(actual); - fixture.Mock().VerifyAll(); fixture.Mock().VerifyAll(); fixture.Mock().VerifyAll(); @@ -273,7 +271,6 @@ public async Task CopyInstance_PlatformReturnsError_ThrowsException() public async Task CopyInstance_InstantiationValidationFails_ReturnsForbidden() { // Arrange - using var fixture = InstancesControllerFixture.Create(); const string Org = "ttd"; const string AppName = "copy-instance"; int instanceOwnerPartyId = 343234; @@ -284,11 +281,13 @@ public async Task CopyInstance_InstantiationValidationFails_ReturnsForbidden() Status = new InstanceStatus() { IsArchived = true }, }; InstantiationValidationResult? instantiationValidationResult = new() { Valid = false }; + var auth = TestAuthentication.GetUserAuthentication(userPartyId: instanceOwnerPartyId); + using var fixture = InstancesControllerFixture.Create(auth); fixture .Mock() .Setup(httpContext => httpContext.User) - .Returns(TestAuthentication.GetUserPrincipal(1337, instanceOwnerPartyId)); + .Returns(TestAuthentication.GetUserPrincipal(partyId: instanceOwnerPartyId)); fixture .Mock() .Setup(a => a.GetApplicationMetadata()) @@ -327,18 +326,17 @@ public async Task CopyInstance_InstantiationValidationFails_ReturnsForbidden() public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() { // Arrange - using var fixture = InstancesControllerFixture.Create(); const string Org = "ttd"; const string AppName = "copy-instance"; - const int InstanceOwnerPartyId = 343234; + const int instanceOwnerPartyId = 343234; Guid instanceGuid = Guid.NewGuid(); Guid dataGuid = Guid.NewGuid(); const string dataTypeId = "data_type_1"; Instance instance = new() { - Id = $"{InstanceOwnerPartyId}/{instanceGuid}", + Id = $"{instanceOwnerPartyId}/{instanceGuid}", AppId = $"{Org}/{AppName}", - InstanceOwner = new InstanceOwner() { PartyId = InstanceOwnerPartyId.ToString() }, + InstanceOwner = new InstanceOwner() { PartyId = instanceOwnerPartyId.ToString() }, Status = new InstanceStatus() { IsArchived = true }, Process = new ProcessState() { CurrentTask = new ProcessElementInfo() { ElementId = "First" } }, Data = new List @@ -347,11 +345,13 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() }, }; InstantiationValidationResult? instantiationValidationResult = new() { Valid = true }; + var auth = TestAuthentication.GetUserAuthentication(userPartyId: instanceOwnerPartyId); + using var fixture = InstancesControllerFixture.Create(auth); fixture .Mock() - .Setup(hc => hc.User) - .Returns(TestAuthentication.GetUserPrincipal(1337, InstanceOwnerPartyId)); + .Setup(httpContext => httpContext.User) + .Returns(TestAuthentication.GetUserPrincipal(partyId: instanceOwnerPartyId)); fixture.Mock().Setup(hc => hc.Request).Returns(Mock.Of()); fixture .Mock() @@ -392,7 +392,7 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() ); fixture .Mock() - .Setup(p => p.GetFormData(instanceGuid, It.IsAny()!, Org, AppName, InstanceOwnerPartyId, dataGuid)) + .Setup(p => p.GetFormData(instanceGuid, It.IsAny()!, Org, AppName, instanceOwnerPartyId, dataGuid)) .ReturnsAsync(new { test = "test" }); fixture .Mock() @@ -403,7 +403,7 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() It.IsAny()!, Org, AppName, - InstanceOwnerPartyId, + instanceOwnerPartyId, dataTypeId ) ) @@ -411,12 +411,12 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() // Act var controller = fixture.ServiceProvider.GetRequiredService(); - ActionResult actual = await controller.CopyInstance(Org, AppName, InstanceOwnerPartyId, instanceGuid); + ActionResult actual = await controller.CopyInstance(Org, AppName, instanceOwnerPartyId, instanceGuid); // Assert Assert.IsType(actual); RedirectResult objectResult = (RedirectResult)actual; - Assert.Contains($"/#/instance/{InstanceOwnerPartyId}/", objectResult.Url); + Assert.Contains($"/#/instance/{instanceOwnerPartyId}/", objectResult.Url); fixture.Mock().VerifyAll(); fixture.Mock().VerifyAll(); diff --git a/test/Altinn.App.Core.Tests/Features/Auth/AuthenticatedTests.Can_Parse_Real_Tokens_type=SelfIdentifiedUser_ffv+.verified.txt b/test/Altinn.App.Core.Tests/Features/Auth/AuthenticatedTests.Can_Parse_Real_Tokens_type=SelfIdentifiedUser_ffv+.verified.txt index 63efd1a90..b4d94539a 100644 --- a/test/Altinn.App.Core.Tests/Features/Auth/AuthenticatedTests.Can_Parse_Real_Tokens_type=SelfIdentifiedUser_ffv+.verified.txt +++ b/test/Altinn.App.Core.Tests/Features/Auth/AuthenticatedTests.Can_Parse_Real_Tokens_type=SelfIdentifiedUser_ffv+.verified.txt @@ -6,6 +6,7 @@ UserId: 1428813, PartyId: 53328660, AuthenticationMethod: SelfIdentified, + InAltinnPortal: true, TokenIssuer: Altinn, TokenIsExchanged: false, Scopes: altinn:portal/enduser,