From f49ec89ad393bd699c4c11b15a8c6aabf04e6015 Mon Sep 17 00:00:00 2001 From: Terje Holene Date: Thu, 30 Mar 2023 18:27:26 +0200 Subject: [PATCH 1/4] Add GET endpoint to trigger copy of instance --- .../Controllers/InstancesController.cs | 274 +++++++++++----- src/Altinn.App.Core/Helpers/SelfLinkHelper.cs | 17 + .../InstancesController_CopyInstanceTests.cs | 307 ++++++++++++++++++ .../Utils/PrincipalUtil.cs | 39 +-- .../Helpers/SelfLinkHelperTests.cs | 42 +++ 5 files changed, 563 insertions(+), 116 deletions(-) create mode 100644 test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs create mode 100644 test/Altinn.App.Core.Tests/Helpers/SelfLinkHelperTests.cs diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index b7f46b9ef..19fbd7eaf 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -2,6 +2,7 @@ using System.Net; using System.Text; + using Altinn.App.Api.Helpers.RequestHandling; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Mappers; @@ -24,10 +25,12 @@ using Altinn.Platform.Profile.Models; using Altinn.Platform.Register.Models; using Altinn.Platform.Storage.Interface.Models; + using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; + using Newtonsoft.Json; namespace Altinn.App.Api.Controllers @@ -174,8 +177,6 @@ public async Task> Post( return BadRequest("The path parameter 'app' cannot be empty"); } - ApplicationMetadata application = await _appMetadata.GetApplicationMetadata(); - MultipartRequestReader parsedRequest = new MultipartRequestReader(Request); await parsedRequest.Read(); @@ -186,26 +187,17 @@ public async Task> Post( Instance? instanceTemplate = await ExtractInstanceTemplate(parsedRequest); - if (!instanceOwnerPartyId.HasValue && instanceTemplate == null) + if (instanceOwnerPartyId is null && instanceTemplate is null) { return BadRequest("Cannot create an instance without an instanceOwner.partyId. Either provide instanceOwner party Id as a query parameter or an instanceTemplate object in the body."); } - if (instanceOwnerPartyId.HasValue && instanceTemplate?.InstanceOwner?.PartyId != null) + if (instanceOwnerPartyId is not null && instanceTemplate?.InstanceOwner?.PartyId is not null) { return BadRequest("You cannot provide an instanceOwnerPartyId as a query param as well as an instance template in the body. Choose one or the other."); } - RequestPartValidator requestValidator = new RequestPartValidator(application); - - string multipartError = requestValidator.ValidateParts(parsedRequest.Parts); - - if (!string.IsNullOrEmpty(multipartError)) - { - return BadRequest($"Error when comparing content to application metadata: {multipartError}"); - } - - if (instanceTemplate != null) + if (instanceTemplate is not null) { InstanceOwner lookup = instanceTemplate.InstanceOwner; @@ -216,13 +208,22 @@ public async Task> Post( } else { - // create minimum instance template instanceTemplate = new Instance { InstanceOwner = new InstanceOwner { PartyId = instanceOwnerPartyId.Value.ToString() } }; } + ApplicationMetadata application = await _appMetadata.GetApplicationMetadata(); + + RequestPartValidator requestValidator = new RequestPartValidator(application); + string multipartError = requestValidator.ValidateParts(parsedRequest.Parts); + + if (!string.IsNullOrEmpty(multipartError)) + { + return BadRequest($"Error when comparing content to application metadata: {multipartError}"); + } + Party party; try { @@ -261,24 +262,17 @@ public async Task> Post( return StatusCode((int)HttpStatusCode.Forbidden, validationResult); } + instanceTemplate.Org = application.Org; + ConditionallySetReadStatus(instanceTemplate); + Instance instance; - ProcessStateChange processResult; instanceTemplate.Process = null; ProcessChangeContext processChangeContext = new ProcessChangeContext(instanceTemplate, User); try { - // start process and goto next task + // start process processChangeContext.DontUpdateProcessAndDispatchEvents = true; processChangeContext = await _processEngine.StartProcess(processChangeContext); - processResult = processChangeContext.ProcessStateChange; - - string? userOrgClaim = User.GetOrg(); - - if (userOrgClaim == null || !org.Equals(userOrgClaim, StringComparison.InvariantCultureIgnoreCase)) - { - instanceTemplate.Status ??= new InstanceStatus(); - instanceTemplate.Status.ReadStatus = ReadStatus.Read; - } // create the instance instance = await _instanceClient.CreateInstance(org, app, instanceTemplate); @@ -399,6 +393,9 @@ public async Task> PostSimplified( DueBefore = instansiationInstance.DueBefore }; + instanceTemplate.Org = application.Org; + ConditionallySetReadStatus(instanceTemplate); + // Run custom app logic to validate instantiation InstantiationValidationResult? validationResult = await _instantiationValidator.Validate(instanceTemplate); if (validationResult != null && !validationResult.Valid) @@ -407,25 +404,15 @@ public async Task> PostSimplified( } Instance instance; - ProcessStateChange processResult; try { - // start process and goto next task instanceTemplate.Process = null; + // start process ProcessChangeContext processChangeContext = new ProcessChangeContext(instanceTemplate, User); processChangeContext.Prefill = instansiationInstance.Prefill; processChangeContext.DontUpdateProcessAndDispatchEvents = true; processChangeContext = await _processEngine.StartProcess(processChangeContext); - processResult = processChangeContext.ProcessStateChange; - - string? userOrgClaim = User.GetOrg(); - - if (userOrgClaim == null || !org.Equals(userOrgClaim, StringComparison.InvariantCultureIgnoreCase)) - { - instanceTemplate.Status ??= new InstanceStatus(); - instanceTemplate.Status.ReadStatus = ReadStatus.Read; - } Instance? source = null; @@ -443,7 +430,7 @@ public async Task> PostSimplified( return StatusCode(500, $"Retrieving source instance failed with status code {exception.Response.StatusCode}"); } - if (source.Process.Ended == null) + if (!source.Status.IsArchived) { return BadRequest("It is not possible to copy an instance that isn't archived."); } @@ -475,66 +462,100 @@ public async Task> PostSimplified( return Created(url, instance); } - private async Task CopyDataFromSourceInstance(ApplicationMetadata application, Instance targetInstance, Instance sourceInstance) + /// + /// 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. + /// + /// Unique identifier of the organisation responsible for the app + /// Application identifier which is unique within an organisation + /// Unique id of the party that is the owner of the instance + /// Unique id to identify the instance + /// A representing the result of the asynchronous operation. + /// + /// The endpoint will return a redirect to the new instance if the copy operation was successful. + /// + [Authorize] + [HttpGet("{instanceOwnerPartyId:int}/{instanceGuid:guid}/copy")] + [Produces("application/json")] + [ProducesResponseType(typeof(Instance), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task CopyInstance( + [FromRoute] string org, + [FromRoute] string app, + [FromRoute] int instanceOwnerPartyId, + [FromRoute] Guid instanceGuid) { - string org = application.Org; - string app = application.AppIdentifier.App; - int instanceOwnerPartyId = int.Parse(targetInstance.InstanceOwner.PartyId); - - string[] sourceSplit = sourceInstance.Id.Split("/"); - Guid sourceInstanceGuid = Guid.Parse(sourceSplit[1]); + // 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) + { + return Forbid(); + } - List dts = application.DataTypes - .Where(dt => dt.AppLogic?.ClassRef != null) - .Where(dt => dt.TaskId != null && dt.TaskId.Equals(targetInstance.Process.CurrentTask.ElementId)) - .ToList(); - List excludedDataTypes = application.CopyInstanceSettings.ExcludedDataTypes; + ApplicationMetadata application = await _appMetadata.GetApplicationMetadata(); - foreach (DataElement de in sourceInstance.Data) + if (application.CopyInstanceSettings?.Enabled is null or false) { - if (excludedDataTypes != null && excludedDataTypes.Contains(de.DataType)) - { - continue; - } + return BadRequest("Creating instance based on a copy from an archived instance is not enabled for this app."); + } - if (dts.Any(dts => dts.Id.Equals(de.DataType))) - { - DataType dt = dts.First(dt => dt.Id.Equals(de.DataType)); + EnforcementResult readAccess = await AuthorizeAction(org, app, instanceOwnerPartyId, instanceGuid, "read"); - Type type; - try - { - type = _appModel.GetModelType(dt.AppLogic.ClassRef); - } - catch (Exception altinnAppException) - { - throw new ServiceException(HttpStatusCode.InternalServerError, $"App.GetAppModelType failed: {altinnAppException.Message}", altinnAppException); - } + if (!readAccess.Authorized) + { + return Forbidden(readAccess); + } - object data = await _dataClient.GetFormData(sourceInstanceGuid, type, org, app, instanceOwnerPartyId, Guid.Parse(de.Id)); + Instance sourceInstance = await _instanceClient.GetInstance(app, org, instanceOwnerPartyId, instanceGuid); - if (application.CopyInstanceSettings.ExcludedDataFields != null) - { - DataHelper.ResetDataFields(application.CopyInstanceSettings.ExcludedDataFields, data); - } + if (sourceInstance?.Status?.IsArchived is null or false) + { + return BadRequest("The instance being copied must be archived."); + } - await _prefillService.PrefillDataModel(instanceOwnerPartyId.ToString(), dt.Id, data); + EnforcementResult instantiateAccess = await AuthorizeAction(org, app, instanceOwnerPartyId, null, "instantiate"); - await _instantiationProcessor.DataCreation(targetInstance, data, null); + if (!instantiateAccess.Authorized) + { + return Forbidden(instantiateAccess); + } - await _dataClient.InsertFormData( - data, - Guid.Parse(targetInstance.Id.Split("/")[1]), - type, - org, - app, - instanceOwnerPartyId, - dt.Id); + // Multiple properties like Org and AppId will be set by Storage + Instance targetInstance = new() + { + InstanceOwner = sourceInstance.InstanceOwner, + VisibleAfter = sourceInstance.VisibleAfter, + Status = new() { ReadStatus = ReadStatus.Read } + }; - await UpdatePresentationTextsOnInstance(application.PresentationFields, targetInstance, dt.Id, data); - await UpdateDataValuesOnInstance(application.DataFields, targetInstance, dt.Id, data); - } + InstantiationValidationResult? validationResult = await _instantiationValidator.Validate(targetInstance); + if (validationResult != null && !validationResult.Valid) + { + return StatusCode((int)HttpStatusCode.Forbidden, validationResult); } + + ProcessChangeContext processChangeContext = new(targetInstance, User) + { + DontUpdateProcessAndDispatchEvents = true + }; + processChangeContext = await _processEngine.StartProcess(processChangeContext); + + targetInstance = await _instanceClient.CreateInstance(org, app, targetInstance); + + await CopyDataFromSourceInstance(application, targetInstance, sourceInstance); + + targetInstance = await _instanceClient.GetInstance(targetInstance); + + processChangeContext.Instance = targetInstance; + processChangeContext.DontUpdateProcessAndDispatchEvents = false; + await _processEngine.StartTask(processChangeContext); + + await RegisterEvent("app.instance.created", targetInstance); + + string url = SelfLinkHelper.BuildFrontendSelfLink(targetInstance, Request); + + return Redirect(url); } /// @@ -702,6 +723,82 @@ public async Task>> GetActiveInstances([FromRo return Ok(SimpleInstanceMapper.MapInstanceListToSimpleInstanceList(activeInstances, userAndOrgLookup)); } + private void ConditionallySetReadStatus(Instance instance) + { + string? orgClaimValue = User.GetOrg(); + + if (orgClaimValue == instance.Org) + { + // Default value for ReadStatus is "not read" + return; + } + + instance.Status ??= new InstanceStatus(); + instance.Status.ReadStatus = ReadStatus.Read; + } + + private async Task CopyDataFromSourceInstance(ApplicationMetadata application, Instance targetInstance, Instance sourceInstance) + { + string org = application.Org; + string app = application.AppIdentifier.App; + int instanceOwnerPartyId = int.Parse(targetInstance.InstanceOwner.PartyId); + + string[] sourceSplit = sourceInstance.Id.Split("/"); + Guid sourceInstanceGuid = Guid.Parse(sourceSplit[1]); + + List dts = application.DataTypes + .Where(dt => dt.AppLogic?.ClassRef != null) + .Where(dt => dt.TaskId != null && dt.TaskId.Equals(targetInstance.Process.CurrentTask.ElementId)) + .ToList(); + List excludedDataTypes = application.CopyInstanceSettings.ExcludedDataTypes; + + foreach (DataElement de in sourceInstance.Data) + { + if (excludedDataTypes != null && excludedDataTypes.Contains(de.DataType)) + { + continue; + } + + if (dts.Any(dts => dts.Id.Equals(de.DataType))) + { + DataType dt = dts.First(dt => dt.Id.Equals(de.DataType)); + + Type type; + try + { + type = _appModel.GetModelType(dt.AppLogic.ClassRef); + } + catch (Exception altinnAppException) + { + throw new ServiceException(HttpStatusCode.InternalServerError, $"App.GetAppModelType failed: {altinnAppException.Message}", altinnAppException); + } + + object data = await _dataClient.GetFormData(sourceInstanceGuid, type, org, app, instanceOwnerPartyId, Guid.Parse(de.Id)); + + if (application.CopyInstanceSettings.ExcludedDataFields != null) + { + DataHelper.ResetDataFields(application.CopyInstanceSettings.ExcludedDataFields, data); + } + + await _prefillService.PrefillDataModel(instanceOwnerPartyId.ToString(), dt.Id, data); + + await _instantiationProcessor.DataCreation(targetInstance, data, null); + + await _dataClient.InsertFormData( + data, + Guid.Parse(targetInstance.Id.Split("/")[1]), + type, + org, + app, + instanceOwnerPartyId, + dt.Id); + + await UpdatePresentationTextsOnInstance(application.PresentationFields, targetInstance, dt.Id, data); + await UpdateDataValuesOnInstance(application.DataFields, targetInstance, dt.Id, data); + } + } + } + private ActionResult ExceptionResponse(Exception exception, string message) { _logger.LogError(exception, message); @@ -733,7 +830,8 @@ private async Task AuthorizeAction(string org, string app, in if (response?.Response == null) { - _logger.LogInformation($"// Instances Controller // Authorization of action {action} failed with request: {JsonConvert.SerializeObject(request)}."); + string serializedRequest = JsonConvert.SerializeObject(request); + _logger.LogInformation("// Instances Controller // Authorization of action {action} failed with request: {serializedRequest}.", action, serializedRequest); return enforcementResult; } @@ -751,7 +849,7 @@ private async Task LookupParty(InstanceOwner instanceOwner) } catch (Exception e) when (e is not ServiceException) { - _logger.LogWarning($"Failed to lookup party by partyId: {instanceOwner.PartyId}. The exception was: {e.Message}"); + _logger.LogWarning(e, "Failed to lookup party by partyId: {partyId}", instanceOwner.PartyId); throw new ServiceException(HttpStatusCode.BadRequest, $"Failed to lookup party by partyId: {instanceOwner.PartyId}. The exception was: {e.Message}", e); } } @@ -778,7 +876,7 @@ private async Task LookupParty(InstanceOwner instanceOwner) } catch (Exception e) { - _logger.LogWarning($"Failed to lookup party by {lookupNumber}: {personOrOrganisationNumber}. The exception was: {e}"); + _logger.LogWarning(e, "Failed to lookup party by {lookupNumber}: {personOrOrganisationNumber}", lookupNumber, personOrOrganisationNumber); throw new ServiceException(HttpStatusCode.BadRequest, $"Failed to lookup party by {lookupNumber}: {personOrOrganisationNumber}. The exception was: {e.Message}", e); } } diff --git a/src/Altinn.App.Core/Helpers/SelfLinkHelper.cs b/src/Altinn.App.Core/Helpers/SelfLinkHelper.cs index dfc3c570f..82565cebf 100644 --- a/src/Altinn.App.Core/Helpers/SelfLinkHelper.cs +++ b/src/Altinn.App.Core/Helpers/SelfLinkHelper.cs @@ -1,5 +1,6 @@ using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Http; +using System.Text; namespace Altinn.App.Core.Helpers { @@ -72,5 +73,21 @@ public static void SetDataAppSelfLinks(int instanceOwnerPartyId, Guid instanceGu dataElement.SelfLinks.Apps = $"{selfLink}/data/{dataElement.Id}"; } + + /// + /// Build a url that can be opened in a browser + /// + /// The instance metadata document. + /// The original http request. + /// + public static string BuildFrontendSelfLink(Instance instance, HttpRequest request) + { + StringBuilder urlBuilder = new($"https://{request.Host.ToUriComponent()}/"); + urlBuilder.Append(instance.AppId); + urlBuilder.Append("/#/instance/"); + urlBuilder.Append(instance.Id); + + return urlBuilder.ToString(); + } } } diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs new file mode 100644 index 000000000..7e2fe6316 --- /dev/null +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -0,0 +1,307 @@ +using Altinn.App.Api.Controllers; +using Altinn.App.Api.Tests.Utils; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; +using Altinn.App.Core.Interface; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.AppModel; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Validation; + +using Altinn.Authorization.ABAC.Xacml.JsonProfile; +using Altinn.Common.PEP.Interfaces; +using Altinn.Platform.Storage.Interface.Models; + +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +using Moq; +using Xunit; + +namespace Altinn.App.Api.Tests.Controllers; + +public class InstancesController_CopyInstanceTests +{ + private readonly Mock> _logger = new(); + private readonly Mock _registrer = new(); + private readonly Mock _instanceClient = new(); + private readonly Mock _data = new(); + private readonly Mock _appMetadata = new(); + private readonly Mock _appModel = new(); + private readonly Mock _instantiationProcessor = new(); + private readonly Mock _instantiationValidator = new(); + private readonly Mock _pdp = new(); + private readonly Mock _eventsService = new(); + private readonly IOptions _appSettings = Options.Create(new()); + private readonly Mock _prefill = new(); + private readonly Mock _profile = new(); + private readonly Mock _processEngine = new(); + private readonly Mock _httpContextMock = new(); + + private readonly InstancesController SUT; + + public InstancesController_CopyInstanceTests() + { + ControllerContext controllerContext = new ControllerContext() + { + HttpContext = _httpContextMock.Object + }; + + SUT = new InstancesController( + _logger.Object, + _registrer.Object, + _instanceClient.Object, + _data.Object, + _appMetadata.Object, + _appModel.Object, + _instantiationProcessor.Object, + _instantiationValidator.Object, + _pdp.Object, + _eventsService.Object, + _appSettings, + _prefill.Object, + _profile.Object, + _processEngine.Object) + { + ControllerContext = controllerContext + }; + } + + private void VerifyNoOtherCalls() + { + _registrer.VerifyNoOtherCalls(); + _instanceClient.VerifyNoOtherCalls(); + _data.VerifyNoOtherCalls(); + _appMetadata.VerifyNoOtherCalls(); + _appModel.VerifyNoOtherCalls(); + _instantiationProcessor.VerifyNoOtherCalls(); + _instantiationValidator.VerifyNoOtherCalls(); + _pdp.VerifyNoOtherCalls(); + _eventsService.VerifyNoOtherCalls(); + _prefill.VerifyNoOtherCalls(); + _profile.VerifyNoOtherCalls(); + _processEngine.VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_CopyInstanceNotDefined_ReturnsBadRequest() + { + // Arrange + ApplicationMetadata application = new("ttd/copy-instance") { }; + _appMetadata.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(application); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid()); + + // Assert + Assert.IsType(actual); + BadRequestObjectResult badRequest = (BadRequestObjectResult)actual; + Assert.Contains("copy from an archived instance is not enabled for this app", badRequest!.Value!.ToString()); + + _appMetadata.VerifyAll(); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_CopyInstanceNotEnabled_ReturnsBadRequest() + { + // Arrange + const string Org = "ttd"; + const string AppName = "copy-instance"; + _appMetadata.Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", false)); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid()); + + // Assert + Assert.IsType(actual); + BadRequestObjectResult badRequest = (BadRequestObjectResult)actual; + Assert.Contains("copy from an archived instance is not enabled for this app", badRequest!.Value!.ToString()); + + _appMetadata.VerifyAll(); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_AsAppOwner_ReturnsForbidResult() + { + // Arrange + _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetOrgPrincipal("ttd")); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid()); + + // Assert + Assert.IsType(actual); + + _appMetadata.VerifyAll(); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_AsUnauthorized_ReturnsForbidden() + { + // Arrange + const string Org = "ttd"; + const string AppName = "copy-instance"; + _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetUserPrincipal(1337)); + _appMetadata.Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true)); + _pdp.Setup>(p => p.GetDecisionForRequest(It.IsAny())) + .ReturnsAsync(CreateXacmlResponse("Deny")); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid()); + + // Assert + Assert.IsType(actual); + StatusCodeResult statusCodeResult = (StatusCodeResult)actual; + Assert.Equal(403, statusCodeResult.StatusCode); + + _appMetadata.VerifyAll(); + _pdp.VerifyAll(); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_InstanceNotArchived_ReturnsBadRequest() + { + // Arrange + const string Org = "ttd"; + const string AppName = "copy-instance"; + int instanceOwnerPartyId = 343234; + Guid instanceGuid = Guid.NewGuid(); + Instance instance = new() + { + Id = $"{instanceOwnerPartyId}/{instanceGuid}", + Status = new InstanceStatus() { IsArchived = false } + }; + + _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetUserPrincipal(1337)); + _appMetadata.Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true)); + _pdp.Setup>(p => p.GetDecisionForRequest(It.IsAny())) + .ReturnsAsync(CreateXacmlResponse("Permit")); + _instanceClient.Setup(i => i.GetInstance(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(instance); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid); + + // Assert + Assert.IsType(actual); + BadRequestObjectResult badRequest = (BadRequestObjectResult)actual; + Assert.Contains("instance being copied must be archived", badRequest!.Value!.ToString()); + + _appMetadata.VerifyAll(); + _pdp.VerifyAll(); + _instanceClient.VerifyAll(); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_InstantiationValidationFails_ReturnsForbidden() + { + // Arrange + const string Org = "ttd"; + const string AppName = "copy-instance"; + int instanceOwnerPartyId = 343234; + Guid instanceGuid = Guid.NewGuid(); + Instance instance = new() + { + Id = $"{instanceOwnerPartyId}/{instanceGuid}", + Status = new InstanceStatus() { IsArchived = true } + }; + InstantiationValidationResult? instantiationValidationResult = new() { Valid = false }; + + _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetUserPrincipal(1337)); + _appMetadata.Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true)); + _pdp.Setup>(p => p.GetDecisionForRequest(It.IsAny())) + .ReturnsAsync(CreateXacmlResponse("Permit")); + _instanceClient.Setup(i => i.GetInstance(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(instance); + _instantiationValidator.Setup(v => v.Validate(It.IsAny())).ReturnsAsync(instantiationValidationResult); + + // Act + ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid); + + // Assert + Assert.IsType(actual); + ObjectResult objectResult = (ObjectResult)actual; + Assert.Equal(403, objectResult.StatusCode); + + _appMetadata.VerifyAll(); + _pdp.VerifyAll(); + _instanceClient.VerifyAll(); + _instantiationValidator.VerifyAll(); + + VerifyNoOtherCalls(); + } + + [Fact] + public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() + { + // Arrange + const string Org = "ttd"; + const string AppName = "copy-instance"; + const int InstanceOwnerPartyId = 343234; + Guid instanceGuid = Guid.NewGuid(); + Instance instance = new() + { + Id = $"{InstanceOwnerPartyId}/{instanceGuid}", + AppId = $"{Org}/{AppName}", + InstanceOwner = new InstanceOwner() { PartyId = InstanceOwnerPartyId.ToString() }, + Status = new InstanceStatus() { IsArchived = true }, + Data = new List() + }; + InstantiationValidationResult? instantiationValidationResult = new() { Valid = true }; + + _httpContextMock.Setup(hc => hc.User).Returns(PrincipalUtil.GetUserPrincipal(1337)); + _httpContextMock.Setup(hc => hc.Request).Returns(Mock.Of()); + _appMetadata.Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true)); + _pdp.Setup>(p => p.GetDecisionForRequest(It.IsAny())) + .ReturnsAsync(CreateXacmlResponse("Permit")); + _instanceClient.Setup(i => i.GetInstance(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(instance); + _instanceClient.Setup(i => i.CreateInstance(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(instance); + _instanceClient.Setup(i => i.GetInstance(It.IsAny())).ReturnsAsync(instance); + _instantiationValidator.Setup(v => v.Validate(It.IsAny())).ReturnsAsync(instantiationValidationResult); + _processEngine.Setup(p => p.StartProcess(It.IsAny())) + .ReturnsAsync((ProcessChangeContext pcc) => { return pcc; }); + _processEngine.Setup(p => p.StartTask(It.IsAny())); + + // Act + ActionResult actual = await SUT.CopyInstance(Org, AppName, InstanceOwnerPartyId, instanceGuid); + + // Assert + Assert.IsType(actual); + RedirectResult objectResult = (RedirectResult)actual; + + _appMetadata.VerifyAll(); + _pdp.VerifyAll(); + _instanceClient.VerifyAll(); + _processEngine.VerifyAll(); + _instantiationValidator.VerifyAll(); + + VerifyNoOtherCalls(); + } + + private static ApplicationMetadata CreateApplicationMetadata(string appId, bool enableCopyInstance) + { + return new(appId) + { + CopyInstanceSettings = new CopyInstanceSettings { Enabled = enableCopyInstance }, + DataTypes = new List() + }; + } + + private static XacmlJsonResponse CreateXacmlResponse(string decision) + { + return new XacmlJsonResponse() { Response = new() { new XacmlJsonResult() { Decision = decision } } }; + } +} diff --git a/test/Altinn.App.Api.Tests/Utils/PrincipalUtil.cs b/test/Altinn.App.Api.Tests/Utils/PrincipalUtil.cs index dd7154512..81c5aff61 100644 --- a/test/Altinn.App.Api.Tests/Utils/PrincipalUtil.cs +++ b/test/Altinn.App.Api.Tests/Utils/PrincipalUtil.cs @@ -1,11 +1,7 @@ -using Altinn.App.Api.Tests.Mocks; +using System.Security.Claims; + +using Altinn.App.Api.Tests.Mocks; using AltinnCore.Authentication.Constants; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Security.Claims; -using System.Text; -using System.Threading.Tasks; namespace Altinn.App.Api.Tests.Utils { @@ -35,7 +31,7 @@ public static ClaimsPrincipal GetUserPrincipal(int userId, int authenticationLev return principal; } - public static string GetOrgToken(string org, int authenticationLevel = 3) + public static ClaimsPrincipal GetOrgPrincipal(string org, int authenticationLevel = 3) { List claims = new List(); string issuer = "www.altinn.no"; @@ -45,10 +41,14 @@ public static string GetOrgToken(string org, int authenticationLevel = 3) ClaimsIdentity identity = new ClaimsIdentity("mock"); identity.AddClaims(claims); - ClaimsPrincipal principal = new ClaimsPrincipal(identity); - string token = JwtTokenMock.GenerateToken(principal, new TimeSpan(1, 1, 1)); - return token; + return new ClaimsPrincipal(identity); + } + + public static string GetOrgToken(string org, int authenticationLevel = 3) + { + ClaimsPrincipal principal = GetOrgPrincipal(org, authenticationLevel); + return JwtTokenMock.GenerateToken(principal, new TimeSpan(1, 1, 1)); } public static string GetSelfIdentifiedUserToken( @@ -70,22 +70,5 @@ public static string GetSelfIdentifiedUserToken( return token; } - - public static string GetOrgToken(string org, string orgNo, int authenticationLevel = 4) - { - List claims = new List(); - string issuer = "www.altinn.no"; - claims.Add(new Claim(AltinnCoreClaimTypes.Org, org, ClaimValueTypes.String, issuer)); - claims.Add(new Claim(AltinnCoreClaimTypes.OrgNumber, orgNo, ClaimValueTypes.String, issuer)); - claims.Add(new Claim(AltinnCoreClaimTypes.AuthenticateMethod, "Mock", ClaimValueTypes.String, issuer)); - claims.Add(new Claim(AltinnCoreClaimTypes.AuthenticationLevel, authenticationLevel.ToString(), ClaimValueTypes.Integer32, issuer)); - - ClaimsIdentity identity = new ClaimsIdentity("mock"); - identity.AddClaims(claims); - ClaimsPrincipal principal = new ClaimsPrincipal(identity); - string token = JwtTokenMock.GenerateToken(principal, new TimeSpan(1, 1, 1)); - - return token; - } } } diff --git a/test/Altinn.App.Core.Tests/Helpers/SelfLinkHelperTests.cs b/test/Altinn.App.Core.Tests/Helpers/SelfLinkHelperTests.cs new file mode 100644 index 000000000..5a086eb4a --- /dev/null +++ b/test/Altinn.App.Core.Tests/Helpers/SelfLinkHelperTests.cs @@ -0,0 +1,42 @@ +#nullable enable + +using Altinn.App.Core.Helpers; +using Altinn.Platform.Storage.Interface.Models; + +using Microsoft.AspNetCore.Http; + +using Moq; +using Xunit; + +namespace Altinn.App.Core.Tests.Helpers +{ + public class SelfLinkHelperTests + { + [Theory] + [InlineData("ttd.apps.altinn.no", "copy-me", "1337")] + [InlineData("skd.apps.altinn.no", "copy-this", "7474")] + public void BuildFrontendSelfLink(string host, string appId, string partyId) + { + // Arrange + Guid instanceGuid = Guid.NewGuid(); + Instance instance = new() + { + Id = $"{partyId}/{instanceGuid}", + AppId = appId + }; + + Mock requestMock = new(); + requestMock.Setup(r => r.Host).Returns(new HostString(host)); + + // Act + string url = SelfLinkHelper.BuildFrontendSelfLink(instance, requestMock.Object); + + // Assert + Assert.Contains(host, url); + Assert.Contains("/#/instance/", url); + Assert.Contains(partyId, url); + Assert.Contains(instanceGuid.ToString(), url); + Assert.Contains(appId, url); + } + } +} From 18574939908658fdd7b821bf4825fad16a11849f Mon Sep 17 00:00:00 2001 From: Terje Holene Date: Mon, 3 Apr 2023 16:16:36 +0200 Subject: [PATCH 2/4] Improved coverage a little bit. --- .../InstancesController_CopyInstanceTests.cs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs index 7e2fe6316..1f0ffe68c 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -73,14 +73,10 @@ private void VerifyNoOtherCalls() { _registrer.VerifyNoOtherCalls(); _instanceClient.VerifyNoOtherCalls(); - _data.VerifyNoOtherCalls(); _appMetadata.VerifyNoOtherCalls(); - _appModel.VerifyNoOtherCalls(); - _instantiationProcessor.VerifyNoOtherCalls(); _instantiationValidator.VerifyNoOtherCalls(); _pdp.VerifyNoOtherCalls(); _eventsService.VerifyNoOtherCalls(); - _prefill.VerifyNoOtherCalls(); _profile.VerifyNoOtherCalls(); _processEngine.VerifyNoOtherCalls(); } @@ -256,7 +252,11 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() AppId = $"{Org}/{AppName}", InstanceOwner = new InstanceOwner() { PartyId = InstanceOwnerPartyId.ToString() }, Status = new InstanceStatus() { IsArchived = true }, - Data = new List() + Process = new ProcessState() { CurrentTask = new ProcessElementInfo() { ElementId = "First" } }, + Data = new List + { + new DataElement { Id = Guid.NewGuid().ToString(), DataType = "data_type_1" } + } }; InstantiationValidationResult? instantiationValidationResult = new() { Valid = true }; @@ -296,7 +296,18 @@ private static ApplicationMetadata CreateApplicationMetadata(string appId, bool return new(appId) { CopyInstanceSettings = new CopyInstanceSettings { Enabled = enableCopyInstance }, - DataTypes = new List() + DataTypes = new List + { + new DataType + { + Id = "data_type_1", + AppLogic = new ApplicationLogic + { + ClassRef = "App.Models.Skjema", + }, + TaskId = "First" + } + } }; } From 0fc45c90341ed93dca9a37a21191dffd88a8cbff Mon Sep 17 00:00:00 2001 From: Terje Holene Date: Wed, 12 Apr 2023 11:02:02 +0200 Subject: [PATCH 3/4] Assert redirect result url --- .../Controllers/InstancesController_CopyInstanceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs index 1f0ffe68c..e6d9f3265 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -281,6 +281,7 @@ public async Task CopyInstance_EverythingIsFine_ReturnsRedirect() // Assert Assert.IsType(actual); RedirectResult objectResult = (RedirectResult)actual; + Assert.Contains($"/#/instance/{InstanceOwnerPartyId}/", objectResult.Url); _appMetadata.VerifyAll(); _pdp.VerifyAll(); From 2e55428cf4d7d0b2a22fe3f8ca84b454b0ba979a Mon Sep 17 00:00:00 2001 From: Terje Holene Date: Tue, 25 Apr 2023 16:46:50 +0200 Subject: [PATCH 4/4] Add legacy to path and obsolete attribute --- src/Altinn.App.Api/Controllers/InstancesController.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index 19fbd7eaf..d7e7a1b17 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -474,9 +474,10 @@ public async Task> PostSimplified( /// /// 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] - [HttpGet("{instanceOwnerPartyId:int}/{instanceGuid:guid}/copy")] - [Produces("application/json")] + [HttpGet("/{org}/{app}/legacy/instances/{instanceOwnerPartyId:int}/{instanceGuid:guid}/copy")] [ProducesResponseType(typeof(Instance), StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task CopyInstance(