Instance authorization (#531)

* Clean up code for getting organization id and remove prefix where necessary

* We do not need to check IsRecipient/IsSender explicitly because the PDP verifies whether the caller is allowed to receive/send on behalf of recipient/sender

* Re-write all authorization code to use pre-defined "simple" methods to hide authorization logic/complexity. Make the "raw" function private.

* Remove now unused dependencies

* Add hotfix to main as a fix is not in priority

* Bug: Purge needs CORS support to be used from Arbeidsflate

* As we no longer use OnBehalfOf directly in the logic, we no longer need it anywhere aside from GetCorrespondences

* Had to change logic in GetCorrespondenceOverview/Details because someone may have access both as a sender and recipient, but calling the API as a recipient, hence fetched should still be set.

* Suggestions from CodeRabbit

* Fix tests with authorization override

* Also need override for legacy authentication

* Smart coderabbit

* Return org number without prefix

Altinn.Correspondence.sln

@@ -17,6 +17,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.Correspondence.Tests
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.Correspondence.Integrations", "src\Altinn.Correspondence.Integrations\Altinn.Correspondence.Integrations.csproj", "{5C88BAAD-CF82-4D7A-8FB6-879605EF4E03}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.Correspondence.Common", "src\Altinn.Correspondence.Common\Altinn.Correspondence.Common.csproj", "{AC3CF570-F9E1-4616-A56B-FF94C89CBC0B}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -47,6 +49,10 @@ Global
 		{5C88BAAD-CF82-4D7A-8FB6-879605EF4E03}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{5C88BAAD-CF82-4D7A-8FB6-879605EF4E03}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{5C88BAAD-CF82-4D7A-8FB6-879605EF4E03}.Release|Any CPU.Build.0 = Release|Any CPU
+		{AC3CF570-F9E1-4616-A56B-FF94C89CBC0B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{AC3CF570-F9E1-4616-A56B-FF94C89CBC0B}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{AC3CF570-F9E1-4616-A56B-FF94C89CBC0B}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{AC3CF570-F9E1-4616-A56B-FF94C89CBC0B}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

Test/Altinn.Correspondence.Tests/Helpers/AuthorizationOverride.cs

@@ -0,0 +1,90 @@
+using Altinn.Correspondence.Core.Models.Entities;
+using Altinn.Correspondence.Core.Models.Enums;
+using Altinn.Correspondence.Core.Repositories;
+using Microsoft.Extensions.DependencyInjection;
+using Moq;
+using System.Security.Claims;
+
+namespace Altinn.Correspondence.Tests.Helpers;
+
+public static class AuthorizationOverride
+{
+    public static IServiceCollection OverrideAuthorization(this IServiceCollection services)
+    {
+        var altinnAuthorizationService = new Mock<IAltinnAuthorizationService>();
+        altinnAuthorizationService
+            .Setup(x => x.CheckAccessAsRecipient(
+                It.IsAny<ClaimsPrincipal>(),
+                It.IsAny<CorrespondenceEntity>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((ClaimsPrincipal? user, CorrespondenceEntity corr, CancellationToken token) => {
+                return Task.FromResult(NotRecipient(user));
+            });
+
+        altinnAuthorizationService
+            .Setup(x => x.CheckAccessAsSender(
+                It.IsAny<ClaimsPrincipal>(),
+                It.IsAny<CorrespondenceEntity>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((ClaimsPrincipal? user, CorrespondenceEntity corr, CancellationToken token) => {
+                return Task.FromResult(NotSender(user));
+            });
+
+        altinnAuthorizationService
+            .Setup(x => x.CheckAccessAsSender(
+                It.IsAny<ClaimsPrincipal>(),
+                It.IsAny<string>(),
+                It.IsAny<string>(),
+                It.IsAny<string>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((ClaimsPrincipal? user, string resourceId, string sender, string? instance, CancellationToken token) => {
+                return Task.FromResult(NotSender(user));
+            });
+
+        altinnAuthorizationService
+            .Setup(x => x.CheckAccessAsAny(
+                It.IsAny<ClaimsPrincipal>(),
+                It.IsAny<string>(),
+                It.IsAny<string>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((ClaimsPrincipal? user, string resource, string party, CancellationToken token) => {
+                return Task.FromResult(!NotRecipient(user) || !NotSender(user));
+            });
+
+        altinnAuthorizationService
+            .Setup(x => x.CheckMigrationAccess(
+                It.IsAny<string>(),
+                It.IsAny<List<ResourceAccessLevel>>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((string resourceId, IEnumerable<ResourceAccessLevel> levels, CancellationToken token) =>
+            {
+                return Task.FromResult(true);
+            });
+
+        altinnAuthorizationService
+            .Setup(x => x.CheckUserAccessAndGetMinimumAuthLevel(
+                It.IsAny<ClaimsPrincipal>(),
+                It.IsAny<string>(),
+                It.IsAny<string>(),
+                It.IsAny<List<ResourceAccessLevel>>(),
+                It.IsAny<string>(),
+                It.IsAny<CancellationToken>()))
+            .Returns((ClaimsPrincipal? user, string ssn, string resourceId, List<ResourceAccessLevel> rights, string recipientOrgNo, CancellationToken token) =>
+            {
+                return Task.FromResult<int?>(3);
+            });
+
+        return services.AddScoped(_ => altinnAuthorizationService.Object);
+    }
+
+    private static bool NotSender(ClaimsPrincipal? user)
+    {
+        return !user?.Claims.Any(c =>
+            c.Type == "notSender") ?? true;
+    }
+    private static bool NotRecipient(ClaimsPrincipal? user)
+    {
+        return !user?.Claims.Any(c =>
+            c.Type == "notRecipient") ?? true;
+    }
+}

Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs

@@ -1,7 +1,6 @@
 using Altinn.Correspondence.Core.Repositories;
 using Altinn.Correspondence.Core.Services;
 using Altinn.Correspondence.Integrations.Altinn.AccessManagement;
-using Altinn.Correspondence.Integrations.Altinn.Authorization;
 using Altinn.Correspondence.Integrations.Altinn.Events;
 using Altinn.Correspondence.Integrations.Altinn.Notifications;
 using Altinn.Correspondence.Integrations.Altinn.Register;
@@ -25,6 +24,7 @@ public class CustomWebApplicationFactory : WebApplicationFactory<Program>
 {
     internal Mock<IBackgroundJobClient>? HangfireBackgroundJobClient;
 
+
     protected override void ConfigureWebHost(
         IWebHostBuilder builder)
     {
@@ -46,14 +46,16 @@ protected override void ConfigureWebHost(
             services.AddScoped<IEventBus, ConsoleLogEventBus>();
             services.AddScoped<IAltinnNotificationService, AltinnDevNotificationService>();
             services.AddScoped<IDialogportenService, DialogportenDevService>();
-            services.AddScoped<IAltinnAuthorizationService, AltinnAuthorizationDevService>();
+            services.OverrideAuthorization();
             services.AddScoped<IAltinnRegisterService, AltinnRegisterDevService>();
             services.AddScoped<IAltinnAccessManagementService, AltinnAccessManagementDevService>();
             var resourceRightsService = new Mock<IResourceRightsService>();
             resourceRightsService.Setup(x => x.GetServiceOwnerOfResource(It.IsAny<string>(), It.IsAny<CancellationToken>())).ReturnsAsync("");
             services.AddScoped(_ => resourceRightsService.Object);
         });
     }
+
+
     public HttpClient CreateClientWithAddedClaims(params (string type, string value)[] claims)
     {
         var defaultClaims = new List<Claim>

Test/Altinn.Correspondence.Tests/Helpers/MockPolicyEvaluator.cs

@@ -1,5 +1,5 @@
 using Altinn.Common.PEP.Authorization;
-using Altinn.Correspondence.Application.Configuration;
+using Altinn.Correspondence.Common.Constants;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Authorization.Infrastructure;

Test/Altinn.Correspondence.Tests/TestingController/Attachment/AttachmentDeletionTests.cs

@@ -100,13 +100,13 @@ public async Task DeleteAttachment_AsRecipient_ReturnsForbidden()
             Assert.Equal(HttpStatusCode.Forbidden, deleteResponse.StatusCode);
         }
         [Fact]
-        public async Task DeleteAttachment_AsWrongSender_ReturnsBadRequest()
+        public async Task DeleteAttachment_AsWrongSender_ReturnsUnauthorized()
         {
             // Arrange
             var attachmentId = await AttachmentHelper.GetPublishedAttachment(_senderClient, _responseSerializerOptions);
             var deleteResponse = await _wrongSenderClient.DeleteAsync($"correspondence/api/v1/attachment/{attachmentId}");
             // Assert failure before correspondence is created
-            Assert.Equal(HttpStatusCode.BadRequest, deleteResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, deleteResponse.StatusCode);
         }
     }
 }

Test/Altinn.Correspondence.Tests/TestingController/Attachment/AttachmentDownloadTests.cs

@@ -37,7 +37,7 @@ public async Task DownloadAttachment_AsWrongSender_ReturnsBadRequest()
             var data = await downloadResponse.Content.ReadFromJsonAsync<ProblemDetails>();
 
             // Assert
-            Assert.Equal(HttpStatusCode.BadRequest, downloadResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, downloadResponse.StatusCode);
             Assert.NotNull(data?.Title);
         }
     }

Test/Altinn.Correspondence.Tests/TestingController/Attachment/AttachmentInitializationTests.cs

@@ -31,11 +31,11 @@ public async Task InitializeAttachment_AsRecipient_ReturnsForbidden()
         }
 
         [Fact]
-        public async Task InitializeAttachment_As_Different_Sender_As_Token_ReturnsBadRequest()
+        public async Task InitializeAttachment_As_Different_Sender_As_Token_ReturnsUnauthorized()
         {
             var attachment = new AttachmentBuilder().CreateAttachment().Build();
             var initializeAttachmentResponse = await _wrongSenderClient.PostAsJsonAsync("correspondence/api/v1/attachment", attachment);
-            Assert.Equal(HttpStatusCode.BadRequest, initializeAttachmentResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, initializeAttachmentResponse.StatusCode);
         }
 
         [Fact]

Test/Altinn.Correspondence.Tests/TestingController/Attachment/AttachmentRetrievalTests.cs

@@ -1,11 +1,6 @@
 using Altinn.Correspondence.Tests.Helpers;
 using Altinn.Correspondence.Tests.TestingController.Attachment.Base;
-using System;
-using System.Collections.Generic;
-using System.Linq;
 using System.Net;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace Altinn.Correspondence.Tests.TestingController.Attachment
 {
@@ -31,11 +26,11 @@ public async Task GetAttachmentOverview_AsRecipient_ReturnsForbidden()
         }
 
         [Fact]
-        public async Task GetAttachmentOverview_As_Different_sender_ReturnsBadRequest()
+        public async Task GetAttachmentOverview_As_Different_sender_ReturnsUnauthorized()
         {
             var attachmentId = await AttachmentHelper.GetInitializedAttachment(_senderClient, _responseSerializerOptions);
             var getAttachmentOverviewResponse = await _wrongSenderClient.GetAsync($"correspondence/api/v1/attachment/{attachmentId}");
-            Assert.Equal(HttpStatusCode.BadRequest, getAttachmentOverviewResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, getAttachmentOverviewResponse.StatusCode);
         }
 
         [Fact]
@@ -54,11 +49,11 @@ public async Task GetAttachmentDetails_AsRecipient_ReturnsForbidden()
         }
 
         [Fact]
-        public async Task GetAttachmentDetails_As_Different_sender_ReturnsBadRequest()
+        public async Task GetAttachmentDetails_As_Different_sender_ReturnsUnauthorized()
         {
             var attachmentId = await AttachmentHelper.GetInitializedAttachment(_senderClient, _responseSerializerOptions);
             var getAttachmentOverviewResponse = await _wrongSenderClient.GetAsync($"correspondence/api/v1/attachment/{attachmentId}/details");
-            Assert.Equal(HttpStatusCode.BadRequest, getAttachmentOverviewResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, getAttachmentOverviewResponse.StatusCode);
         }
     }
 }

Test/Altinn.Correspondence.Tests/TestingController/Attachment/AttachmentUploadTests.cs

@@ -39,7 +39,7 @@ public async Task UploadAttachmentData_WrongSender_Fails()
         {
             var attachmentId = await AttachmentHelper.GetInitializedAttachment(_senderClient, _responseSerializerOptions);
             var uploadResponse = await AttachmentHelper.UploadAttachment(attachmentId, _wrongSenderClient);
-            Assert.Equal(HttpStatusCode.BadRequest, uploadResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, uploadResponse.StatusCode);
         }
 
         [Fact]

Test/Altinn.Correspondence.Tests/TestingController/Attachment/Base/AttachmentTestBase.cs

@@ -1,4 +1,4 @@
-using Altinn.Correspondence.Application.Configuration;
+using Altinn.Correspondence.Common.Constants;
 using System.Text.Json;
 
 namespace Altinn.Correspondence.Tests.TestingController.Attachment.Base
@@ -18,8 +18,7 @@ public AttachmentTestBase(CustomWebApplicationFactory factory)
             _recipientClient = factory.CreateClientWithAddedClaims(("scope", AuthorizationConstants.RecipientScope));
             _wrongSenderClient = factory.CreateClientWithAddedClaims(
                 ("scope", AuthorizationConstants.SenderScope), 
-                ("client_orgnr", "123456789"), 
-                ("consumer", "{\"authority\":\"iso6523-actorid-upis\",\"ID\":\"0192:123456789\"}")
+                ("notSender", "true")
             );
             _responseSerializerOptions = new JsonSerializerOptions()
             {

Test/Altinn.Correspondence.Tests/TestingController/Correspondence/Base/CorrespondenceTestBase.cs

@@ -1,4 +1,4 @@
-using Altinn.Correspondence.Application.Configuration;
+using Altinn.Correspondence.Common.Constants;
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -18,8 +18,14 @@ public class CorrespondenceTestBase : IClassFixture<CustomWebApplicationFactory>
         public CorrespondenceTestBase(CustomWebApplicationFactory factory)
         {
             _factory = factory;
-            _senderClient = _factory.CreateClientWithAddedClaims(("scope", AuthorizationConstants.SenderScope));
-            _recipientClient = _factory.CreateClientWithAddedClaims(("scope", AuthorizationConstants.RecipientScope));
+            _senderClient = _factory.CreateClientWithAddedClaims(
+                ("notRecipient", "true"),
+                ("scope", AuthorizationConstants.SenderScope)
+            );
+            _recipientClient = _factory.CreateClientWithAddedClaims(
+                ("notSender", "true"),
+                ("scope", AuthorizationConstants.RecipientScope)
+            );
             _responseSerializerOptions = new JsonSerializerOptions(new JsonSerializerOptions()
             {
                 PropertyNameCaseInsensitive = true

Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceAttachmentTests.cs

@@ -190,7 +190,7 @@ public async Task DownloadCorrespondenceAttachment_AsRecipient_Succeeds()
         }
 
         [Fact]
-        public async Task DownloadCorrespondenceAttachment_WhenNotARecipient_Returns404()
+        public async Task DownloadCorrespondenceAttachment_WhenNotARecipient_Returns401()
         {
             // Arrange
             var attachmentId = await AttachmentHelper.GetPublishedAttachment(_senderClient, _responseSerializerOptions);
@@ -203,10 +203,10 @@ public async Task DownloadCorrespondenceAttachment_WhenNotARecipient_Returns404(
             // Act
             var initializeCorrespondenceResponse = await _senderClient.PostAsJsonAsync("correspondence/api/v1/correspondence", payload, _responseSerializerOptions);
             var response = await initializeCorrespondenceResponse.Content.ReadFromJsonAsync<InitializeCorrespondencesResponseExt>(_responseSerializerOptions);
-            var downloadResponse = await _recipientClient.GetAsync($"correspondence/api/v1/correspondence/{response?.Correspondences.FirstOrDefault().CorrespondenceId}/attachment/{attachmentId}/download");
+            var downloadResponse = await _senderClient.GetAsync($"correspondence/api/v1/correspondence/{response?.Correspondences.FirstOrDefault().CorrespondenceId}/attachment/{attachmentId}/download");
 
             // Assert
-            Assert.Equal(HttpStatusCode.NotFound, downloadResponse.StatusCode);
+            Assert.Equal(HttpStatusCode.Unauthorized, downloadResponse.StatusCode);
         }
 
         [Fact]

Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceNotificationTests.cs

@@ -1,6 +1,6 @@
 using Altinn.Correspondence.API.Models;
 using Altinn.Correspondence.API.Models.Enums;
-using Altinn.Correspondence.Application.Configuration;
+using Altinn.Correspondence.Common.Constants;
 using Altinn.Correspondence.Core.Models.Notifications;
 using Altinn.Correspondence.Core.Repositories;
 using Altinn.Correspondence.Tests.Factories;