Fix missing service owner case (#516)

* Wrote test case for our bug

* Re-factored tests some. Got rid of the second test resource as it served no purpose.

* Re-purposed the "resource not configured" error as it is no longer necessary to register a resource directly with broker (it is done implicitly if resource was not previously used).

* Ensured error message "covered" our bases

* Fixed test as it created two file transfers, and the one it asserted on was not the one that was actually under test

* A timing issue occurred when several tests ran concurrently because they encountered the create-if-not-exists logic for resource entities simultaneously.

* Removed wrong comment

* Fixed naming

src/Altinn.Broker.Application/ConfigureResource/ConfigureResourceHandler.cs

@@ -28,7 +28,7 @@ public async Task<OneOf<Task, Error>> Process(ConfigureResourceRequest request,
         var resource = await _resourceRepository.GetResource(request.ResourceId, cancellationToken);
         if (resource is null)
         {
-            return Errors.ResourceNotConfigured;
+            return Errors.InvalidResourceDefinition;
         }
         if (resource.ServiceOwnerId != request.Token.Consumer)
         {

src/Altinn.Broker.Application/DownloadFile/DownloadFileHandler.cs

@@ -55,7 +55,7 @@ public async Task<OneOf<DownloadFileResponse, Error>> Process(DownloadFileReques
         var resource = await _resourceRepository.GetResource(fileTransfer.ResourceId, cancellationToken);
         if (resource is null)
         {
-            return Errors.ResourceNotConfigured;
+            return Errors.InvalidResourceDefinition;
         };
         var serviceOwner = await _serviceOwnerRepository.GetServiceOwner(resource.ServiceOwnerId);
         if (serviceOwner is null)

src/Altinn.Broker.Application/Errors.cs

@@ -12,7 +12,7 @@ public static class Errors
     public static Error ServiceOwnerNotReadyInfrastructure = new Error(4, "Service owner infrastructure is not ready.", HttpStatusCode.UnprocessableEntity);
     public static Error NoFileUploaded = new Error(5, "No file uploaded yet", HttpStatusCode.BadRequest);
     public static Error FileTransferAlreadyUploaded = new Error(6, "A file transfer has already been, or attempted to be, created. Create a new file transfer resource to try again.", HttpStatusCode.Conflict);
-    public static Error ResourceNotConfigured = new Error(7, "Resource needs to be configured to use the broker API", HttpStatusCode.Forbidden);
+    public static Error InvalidResourceDefinition = new Error(7, "The resource needs to be registered as an Altinn 3 resource and it has to be associated with a service owner", HttpStatusCode.Forbidden);
     public static Error NoAccessToResource = new Error(8, "You must use a bearer token that represents a system user with access to the resource in the Resource Rights Registry", HttpStatusCode.Unauthorized);
     public static Error FileTransferNotAvailable = new Error(9, "The requested file transfer's file is not ready for download. See file transfer status.", HttpStatusCode.Forbidden);
     public static Error UploadFailed = new Error(10, "Error occurred while uploading file See /details for more information.", HttpStatusCode.InternalServerError);

src/Altinn.Broker.Application/GetFileTransfers/GetFileTransfersHandler.cs

@@ -36,7 +36,7 @@ public async Task<OneOf<List<Guid>, Error>> Process(GetFileTransfersRequest requ
         var service = await _resourceRepository.GetResource(request.ResourceId, cancellationToken);
         if (service is null)
         {
-            return Errors.ResourceNotConfigured;
+            return Errors.InvalidResourceDefinition;
         };
         var callingActor = await _actorRepository.GetActorAsync(request.Token.Consumer, cancellationToken);
         if (callingActor is null)

src/Altinn.Broker.Application/InitializeFileTransfer/InitializeFileTransferHandler.cs

@@ -58,7 +58,7 @@ public async Task<OneOf<Guid, Error>> Process(InitializeFileTransferRequest requ
         var resource = await _resourceRepository.GetResource(request.ResourceId, cancellationToken);
         if (resource is null)
         {
-            return Errors.ResourceNotConfigured;
+            return Errors.InvalidResourceDefinition;
         };
         var serviceOwner = await _serviceOwnerRepository.GetServiceOwner(resource.ServiceOwnerId);
         if (serviceOwner?.StorageProvider is null)

src/Altinn.Broker.Application/UploadFile/UploadFileHandler.cs

@@ -64,7 +64,7 @@ public async Task<OneOf<Guid, Error>> Process(UploadFileRequest request, Cancell
         var resource = await _resourceRepository.GetResource(fileTransfer.ResourceId, cancellationToken);
         if (resource is null)
         {
-            return Errors.ResourceNotConfigured;
+            return Errors.InvalidResourceDefinition;
         };
         var serviceOwner = await _serviceOwnerRepository.GetServiceOwner(resource.ServiceOwnerId);
         if (serviceOwner?.StorageProvider is null)

src/Altinn.Broker.Persistence/Repositories/ResourceRepository.cs

@@ -40,7 +40,7 @@ public ResourceRepository(DatabaseConnectionProvider connectionProvider, IAltinn
         if (resource is null)
         {
             resource = await _altinnResourceRepository.GetResource(resourceId, cancellationToken);
-            if (resource is null)
+            if (resource is null || string.IsNullOrWhiteSpace(resource.ServiceOwnerId))
             {
                 return null;
             }

tests/Altinn.Broker.Tests/Altinn.Broker.Tests.csproj

@@ -32,12 +32,10 @@
 
   <ItemGroup>
     <None Remove="Data\altinn-broker-test-resource-1.json" />
-    <None Remove="Data\altinn-broker-test-resource-2.json" />
   </ItemGroup>
 
   <ItemGroup>
     <Content Include="Data\altinn-broker-test-resource-1.json" />
-    <Content Include="Data\altinn-broker-test-resource-2.json" />
   </ItemGroup>
 
 </Project>

tests/Altinn.Broker.Tests/Data/R__Prepare_Test_Data.sql

@@ -3,3 +3,19 @@ VALUES ('0192:991825827', 'Digitaliseringsdirektoratet Avd Oslo');
 
 INSERT INTO broker.storage_provider (storage_provider_id_pk, service_owner_id_fk, created, storage_provider_type, resource_name)
 VALUES (DEFAULT, '0192:991825827', NOW(), 'Azurite', 'dummy-value');
+
+INSERT INTO broker.altinn_resource (
+    resource_id_pk,
+    created,
+    max_file_transfer_size,
+    file_transfer_time_to_live,
+    organization_number,
+    service_owner_id_fk
+) VALUES (
+    'altinn-broker-test-resource-1',
+    NOW(),
+    null,
+    null,
+    '991825827',
+    '0192:991825827'
+);

tests/Altinn.Broker.Tests/Factories/FileTransferInitializeExtTestFactory.cs

@@ -1,11 +1,12 @@
 using Altinn.Broker.Models;
+using Altinn.Broker.Tests.Helpers;
 
 namespace Altinn.Broker.Tests.Factories;
 internal static class FileTransferInitializeExtTestFactory
 {
     internal static FileTransferInitalizeExt BasicFileTransfer() => new FileTransferInitalizeExt()
     {
-        ResourceId = "altinn-broker-test-resource-1",
+        ResourceId = TestConstants.RESOURCE_FOR_TEST,
         Checksum = null,
         FileName = "input.txt",
         PropertyList = [],
@@ -15,7 +16,7 @@ internal static class FileTransferInitializeExtTestFactory
     };
     internal static FileTransferInitalizeExt BasicFileTransfer_MultipleRecipients() => new FileTransferInitalizeExt()
     {
-        ResourceId = "altinn-broker-test-resource-2",
+        ResourceId = TestConstants.RESOURCE_FOR_TEST,
         Checksum = null,
         FileName = "input.txt",
         PropertyList = [],

tests/Altinn.Broker.Tests/FileTransferControllerTests.cs

@@ -358,18 +358,24 @@ public async Task Search_SearchFileTransferWith_RecipientStatus_NotFound()
     public async Task UploadFileTransfer_ChecksumCorrect_Succeeds()
     {
         // Arrange
-        var fileTransferId = await InitializeAndAssertBasicFileTransfer();
         var fileContent = "This is the contents of the uploaded file";
         var fileContentBytes = Encoding.UTF8.GetBytes(fileContent);
         var checksum = CalculateChecksum(fileContentBytes);
-        var fileTransfer = FileTransferInitializeExtTestFactory.BasicFileTransfer();
+        var fileTransfer = FileTransferInitializeExtTestFactory.BasicFileTransfer();       
         fileTransfer.Checksum = checksum;
 
         // Act
         var initializeFileTransferResponse = await _senderClient.PostAsJsonAsync("broker/api/v1/filetransfer", fileTransfer);
-        Assert.True(initializeFileTransferResponse.IsSuccessStatusCode, await initializeFileTransferResponse.Content.ReadAsStringAsync());
+        var fileTransferId = await initializeFileTransferResponse.Content.ReadAsStringAsync();
+        Assert.True(initializeFileTransferResponse.IsSuccessStatusCode, fileTransferId);
         var uploadResponse = await UploadTextFileTransfer(fileTransferId, fileContent);
+
+        // Assert
         Assert.True(uploadResponse.IsSuccessStatusCode, await uploadResponse.Content.ReadAsStringAsync());
+        var fileTransferDetails = await _senderClient.GetFromJsonAsync<FileTransferOverviewExt>($"broker/api/v1/filetransfer/{fileTransferId}", _responseSerializerOptions);
+        Assert.NotNull(fileTransferDetails);
+        Assert.NotNull(fileTransferDetails.Checksum);
+        Assert.Equal(checksum, fileTransferDetails.Checksum);
     }
 
     [Fact]
@@ -443,7 +449,7 @@ public async Task UploadFileTransfer_ChecksumSetWhenInitialized_SameChecksumSetA
     }
 
     [Fact]
-    public async Task SendFileTransfer_UsingUnregisteredUser_Fails()
+    public async Task SendFileTransfer_UserWithoutAccess_Fails()
     {
         // Arrange
         var file = FileTransferInitializeExtTestFactory.BasicFileTransfer();
@@ -459,6 +465,23 @@ public async Task SendFileTransfer_UsingUnregisteredUser_Fails()
         Assert.Equal(Errors.NoAccessToResource.Message, parsedError.Detail);
     }
 
+    [Fact]
+    public async Task SendFileTransfer_ResourceWithBlankServiceOwner_Fails()
+    {
+        // Arrange
+        var file = FileTransferInitializeExtTestFactory.BasicFileTransfer();
+        file.ResourceId = TestConstants.RESOURCE_WITH_NO_SERVICE_OWNER;
+
+        // Act
+        var initializeFileTransferResponse = await _senderClient.PostAsJsonAsync("broker/api/v1/filetransfer", file);
+
+        // Assert
+        Assert.False(initializeFileTransferResponse.IsSuccessStatusCode);
+        var parsedError = await initializeFileTransferResponse.Content.ReadFromJsonAsync<ProblemDetails>();
+        Assert.NotNull(parsedError);
+        Assert.Equal(Errors.InvalidResourceDefinition.Message, parsedError.Detail);
+    }
+
     private async Task<HttpResponseMessage> UploadTextFileTransfer(string fileTransferId, string fileContent)
     {
         var fileContents = Encoding.UTF8.GetBytes(fileContent);

tests/Altinn.Broker.Tests/Helpers/CustomWebApplicationFactory.cs

@@ -85,19 +85,24 @@ protected override void ConfigureWebHost(
             HangfireRecurringJobClient = new Mock<IRecurringJobManager>();
             services.AddSingleton(HangfireRecurringJobClient.Object);
 
-
-            var resourceRegistryRepository = new Mock<IResourceRepository>();
-            string capturedId = "";
-            resourceRegistryRepository.Setup(x => x.GetResource(It.IsAny<string>(), It.IsAny<CancellationToken>()))
-                .Callback((string id, CancellationToken _) => capturedId = id)
+            var altinnResourceRepository = new Mock<IAltinnResourceRepository>();
+            altinnResourceRepository.Setup(x => x.GetResource(It.Is(TestConstants.RESOURCE_FOR_TEST, StringComparer.Ordinal), It.IsAny<CancellationToken>()))
                 .ReturnsAsync(() => new ResourceEntity
                 {
-                    Id = capturedId,
+                    Id = TestConstants.RESOURCE_FOR_TEST,
                     Created = DateTime.UtcNow,
                     ServiceOwnerId = $"0192:991825827",
                     OrganizationNumber = "991825827",
                 });
-            services.AddSingleton(resourceRegistryRepository.Object);
+            altinnResourceRepository.Setup(x => x.GetResource(It.Is(TestConstants.RESOURCE_WITH_NO_SERVICE_OWNER, StringComparer.Ordinal), It.IsAny<CancellationToken>()))
+                .ReturnsAsync(() => new ResourceEntity
+                {
+                    Id = TestConstants.RESOURCE_WITH_NO_SERVICE_OWNER,
+                    Created = DateTime.UtcNow,
+                    ServiceOwnerId = "",
+                    OrganizationNumber = "",
+                });
+            services.AddSingleton(altinnResourceRepository.Object);
 
             var authorizationService = new Mock<IAuthorizationService>();
             authorizationService.Setup(x => x.CheckUserAccess(It.IsAny<string>(), It.IsAny<List<ResourceAccessLevel>>(), It.IsAny<bool>(), It.IsAny<CancellationToken>())).ReturnsAsync(true);

tests/Altinn.Broker.Tests/Helpers/TestConstants.cs

@@ -7,6 +7,11 @@ internal class TestConstants
 
     internal const string DUMMY_LEGACY_TOKEN = "eyJraWQiOiJiZFhMRVduRGpMSGpwRThPZnl5TUp4UlJLbVo3MUxCOHUxeUREbVBpdVQwIiwiYWxnIjoiUlMyNTYifQ.eyJzY29wZSI6ImFsdGlubjpicm9rZXIubGVnYWN5IiwiaXNzIjoiaHR0cHM6Ly90ZXN0Lm1hc2tpbnBvcnRlbi5uby8iLCJjbGllbnRfYW1yIjoidmlya3NvbWhldHNzZXJ0aWZpa2F0IiwidG9rZW5fdHlwZSI6IkJlYXJlciIsImV4cCI6MTcwODU5Nzg3OCwiaWF0IjoxNzA4NTk0Mjc4LCJjbGllbnRfaWQiOiI2NzFjNTE2Zi03YWIwLTRhMTUtOTU1Yi1hODJkMTg2Y2VjYzAiLCJqdGkiOiJSTU90Mmk5NkxWZ3d4Z29NaWtXTnFNeDI0TndNZlBHc082Tl9YQjBQZzZ3IiwiY29uc3VtZXIiOnsiYXV0aG9yaXR5IjoiaXNvNjUyMy1hY3RvcmlkLXVwaXMiLCJJRCI6IjAxOTI6OTkxODI1ODI3In19.G268zp-aLUvmR1aTkkaMsZ9j6FT9FmvqKfTFOSP277F8X4BX5kLkm5v7G1MTgDybG0CUXxNGsyhMMlsGQscOZIsOe6QW05aoBFa1vWGOCsTLBaRbBm-LEU41dEPYqKzsDCh61p-zvINdNswuc5CG5vOwkKZi_PBbYUCEF6wIwe3eJ8ttNmunmEjBvOQcSIRllo-unIbzm4nsSQADnXRDAgeJ_jdl8k2s2N_Ose7qIE-usoVlKY53Ayax-V3ws8L22YxKHEbYnhx3oswfKg-ux2PrNFFFWfarUlpVnj1CFqY11ZlxXOS7sDRcwgc1gSnpTZWgysxAU0mGCoV03KwkYOMkVJp4UXkxL6WZ25RqTVb2YsIVq7g6m5BbAPJmZW-_OnpP6KZYQ8fCILYo6EIdn9TEot5Ffm8RzjlbXNseMS10oPCmQswe18TnzKaqFk2U6hOVhhakCvKxSsN0yDj9tsZitP_MOZPZ9ybVmK_jYrYNViJ02PLqnF5n3DMqcXDT";
 
-    internal const string RESOURCE_WITH_NO_ACCESS = "altinn-broker-test-resource-1-failed-access";
     internal const string DUMMY_SERVICE_OWNER_TOKEN = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjM4QUE3QTc5MjUzNDNCQjE0NjFCRUUwMURCNUQwOTRBM0VCOTgwMjUiLCJ0eXAiOiJKV1QiLCJ4NWMiOiIzOEFBN0E3OTI1MzQzQkIxNDYxQkVFMDFEQjVEMDk0QTNFQjk4MDI1In0.eyJzY29wZSI6ImFsdGlubjpicm9rZXIuYWRtaW4gYWx0aW5uOnJlc291cmNlcmVnaXN0cnkvcmVzb3VyY2Uud3JpdGUgYWx0aW5uOmF1dGhvcml6YXRpb246cGRwIiwidG9rZW5fdHlwZSI6IkJlYXJlciIsImV4cCI6MTcwOTEzMDc2OCwiaWF0IjoxNzA5MTI4OTY4LCJjbGllbnRfaWQiOiI3MjJhOTYyZS1hNDVlLTQ3NGMtYjgwNi05M2NhOTQ1YWYyZDIiLCJqdGkiOiJqSGF4a0VJa3FmUjRaNDFEc05TZ2FjaXJvbW9oN0J0a0lHdWlHR2RKWGNxIiwiY29uc3VtZXIiOnsiYXV0aG9yaXR5IjoiaXNvNjUyMy1hY3RvcmlkLXVwaXMiLCJJRCI6IjAxOTI6OTkxODI1ODI3In0sInVybjphbHRpbm46b3JnTnVtYmVyIjoiOTkxODI1ODI3IiwidXJuOmFsdGlubjphdXRoZW50aWNhdGVtZXRob2QiOiJtYXNraW5wb3J0ZW4iLCJ1cm46YWx0aW5uOmF1dGhsZXZlbCI6MywiaXNzIjoiaHR0cHM6Ly9wbGF0Zm9ybS50dDAyLmFsdGlubi5uby9hdXRoZW50aWNhdGlvbi9hcGkvdjEvb3BlbmlkLyIsImFjdHVhbF9pc3MiOiJhbHRpbm4tdGVzdC10b29scyIsIm5iZiI6MTcwOTEyODk2OCwidXJuOmFsdGlubjpvcmciOiJkaWdkaXIifQ.sDV1h6Vfu154z2JBEenJFtoT - 6vJol3-4GHGlGnGYr2PHZnZNNyW-t9aE0yS-KOHpUXylfSV8l8JQE_3CPwYfIfHX4Sx_C6ku29VaQ4rLVr-2UFSKdk1qrUwUsMfeS9GKYMEtvtPzF3u9kYBnqKtkO2_cNpG1jJzdrGKdgwy36APqmeUZo1c0oUcj2fShvS_vQwxizJSGwGxP4GZ8VVGGIfX2CGkVyIT4o_dECImutfMKq0RTQWxL1PKTpnaIy8gG7sbnMMqE0w2WwgJzJpqmq9wA1Ccw48MfUQRoFURjMR2lxJnVxJaoMGxoPowQQMdKlL7OSqxbh8H2e1CjJSTMw";
+
+    internal const string RESOURCE_FOR_TEST = "altinn-broker-test-resource-1";
+
+    internal const string RESOURCE_WITH_NO_ACCESS = "altinn-broker-test-resource-failed-access";
+
+    internal const string RESOURCE_WITH_NO_SERVICE_OWNER = "altinn-broker-test-resource-with-blank-serviceowner";
 }

tests/Altinn.Broker.Tests/ResourceControllerTests.cs

@@ -28,13 +28,12 @@ public ResourceControllerTests(CustomWebApplicationFactory factory)
             PropertyNameCaseInsensitive = true
         });
         _responseSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter());
-
     }
 
     [Fact]
     public async Task Update_Resource_Max_Upload_Size()
     {
-        var response = await _serviceOwnerClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _serviceOwnerClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             MaxFileTransferSize = 99999
         });
@@ -43,7 +42,7 @@ public async Task Update_Resource_Max_Upload_Size()
     [Fact]
     public async Task Update_Resource_Max_Upload_Size_Over_Global_Should_Fail()
     {
-        var response = await _serviceOwnerClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _serviceOwnerClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             MaxFileTransferSize = 999999999999999
         });
@@ -53,7 +52,7 @@ public async Task Update_Resource_Max_Upload_Size_Over_Global_Should_Fail()
     [Fact]
     public async Task Update_Resource_Should_Fail_For_Sender()
     {
-        var response = await _senderClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _senderClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             MaxFileTransferSize = 1000000,
             FileTransferTimeToLive = "P30D"
@@ -64,7 +63,7 @@ public async Task Update_Resource_Should_Fail_For_Sender()
     [Fact]
     public async Task Update_Resource_Should_Fail_For_Receiver()
     {
-        var response = await _recipientClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _recipientClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             MaxFileTransferSize = 1000000,
             FileTransferTimeToLive = "P30D"
@@ -75,7 +74,7 @@ public async Task Update_Resource_Should_Fail_For_Receiver()
     [Fact]
     public async Task Update_Resource_file_transfer_time_to_live()
     {
-        var response = await _serviceOwnerClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _serviceOwnerClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             FileTransferTimeToLive = "P30D"
         });
@@ -84,7 +83,7 @@ public async Task Update_Resource_file_transfer_time_to_live()
     [Fact]
     public async Task Update_Resource_file_transfer_time_to_live_Over_Limit_Should_Fail()
     {
-        var response = await _serviceOwnerClient.PutAsJsonAsync<ResourceExt>($"broker/api/v1/resource/altinn-broker-test-resource-1", new ResourceExt
+        var response = await _serviceOwnerClient.PutAsJsonAsync($"broker/api/v1/resource/{TestConstants.RESOURCE_FOR_TEST}", new ResourceExt
         {
             FileTransferTimeToLive = "P366D"
         });