Add GET endpoint to trigger copy of instance #224

SandGrainOne · 2023-03-30T16:31:33Z

Description

Adding a new GET endpoint in InstanceController. The new endpoint is intended to be an easy to access function to create a new instance based on an existing instance. Data is copied from one to the other based on CopyInstanceSettings.

Related Issue(s)

Implement logic in messagebox for copying archived instances altinn-platform#355

Verification

Your code builds clean without any errors or warnings
Manual testing done (required)
Relevant automated test added (if you find this hard, leave it and we'll help out)
All tests run green

Documentation

User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

src/Altinn.App.Api/Controllers/InstancesController.cs

+                    catch (Exception altinnAppException)
+                    {
+                        throw new ServiceException(HttpStatusCode.InternalServerError, $"App.GetAppModelType failed: {altinnAppException.Message}", altinnAppException);
+                    }


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

ivarne · 2023-03-30T16:50:11Z

Doesn’t mutating apis usually try to avoid using the GET method?

tjololo · 2023-04-14T12:11:35Z

Doesn’t mutating apis usually try to avoid using the GET method?

To explain the GET suggestion it is due to technical challenges in Altinn2. We need to have a discussion in the team as to how we should solve this.
I think it's a bad idea to have technical dept in an legacy system result in technical dept in a new system.

ivarne · 2023-04-14T12:45:00Z

There are two standard ways this could work without xsrf issues.

"altinn2" does an authenticated backchannel call to the app to an instantiation endpoint that accepts an existing instance to copy from and then redirects the user to the correct url for the newly created instance.
"altinn2" redirects the user to a get endpoint that does no mutation, but then asks the user to trigger a Post with the action.

I get that the first might be limited by altinn2 capabilities, but the manual confirmation step is just as simple on the altinn 2 side. It should probably be implemented in app-frontend-react, but could just be a simple form a user might click submit on.
Just redirecting a user to a GET endpoint that does the mutation was much worse before "same-orgin" cookies, but still don't feel like a safe thing to do.

tjololo

Not happy with mutating GET endpoint, but as the old system has limitations we would need to accept technical debt somewhere. By accepting the dept here we would not introduce dept in both frontend and backend

src/Altinn.App.Api/Controllers/InstancesController.cs

ivarne · 2023-04-21T07:58:23Z

See also the suggestion in Altinn/app-frontend-react#286

The workflow should be that triggering copy in SBL should send a request to app frontend. That again builds the request to app backend and renders the newly created instance.

Personally I think frontend should ask the user for a confirmation that they want to copy the instance (unless altinn2 is able to provide a signed assertion), so that there isn't any risk that third party websites can trigger copies with user cookies without user involvement.

test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+        _instantiationValidator.Setup(v => v.Validate(It.IsAny<Instance>())).ReturnsAsync(instantiationValidationResult);
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid);


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+        _appMetadata.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(application);
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid());


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+            .ReturnsAsync(CreateXacmlResponse("Deny"));
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid());


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+            .ReturnsAsync(instance);
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid);


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+        _processEngine.Setup(p => p.StartTask(It.IsAny<ProcessChangeContext>()));
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance(Org, AppName, InstanceOwnerPartyId, instanceGuid);


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+            .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", false));
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid());


test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs

+        _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetOrgPrincipal("ttd"));
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", 343234, Guid.NewGuid());
