System Register -Bug fixes (#944)

* Fixed existing system register test that was not working and added null check for allowredirecturl

* Helper method to validate for duplicate rights and unit tests for the helper method

* Added more validation for the rights and unit tests for the same

* Fixed failing tests

src/Authentication/Controllers/SystemRegisterController.cs

@@ -6,11 +6,11 @@
 using Altinn.Authentication.Core.Problems;
 using Altinn.Authorization.ProblemDetails;
 using Altinn.Platform.Authentication.Core.Constants;
+using Altinn.Platform.Authentication.Core.Errors;
 using Altinn.Platform.Authentication.Core.Models;
 using Altinn.Platform.Authentication.Core.SystemRegister.Models;
 using Altinn.Platform.Authentication.Helpers;
 using Altinn.Platform.Authentication.Services.Interfaces;
-using Altinn.ResourceRegistry.Core.Errors;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 
@@ -104,13 +104,29 @@ public async Task<ActionResult<RegisteredSystem>> GetRegisteredSystemInfo(string
     [HttpPut("vendor/{systemId}")]
     public async Task<ActionResult<SystemRegisterUpdateResult>> UpdateWholeRegisteredSystem([FromBody] RegisteredSystem updateSystem, string systemId, CancellationToken cancellationToken = default)
     {
+        ValidationErrorBuilder errors = default;
         if (!AuthenticationHelper.HasWriteAccess(AuthenticationHelper.GetOrgNumber(updateSystem.Vendor.ID), User))
         {
             return Forbid();
         }
 
+        if (AuthenticationHelper.HasDuplicateRights(updateSystem.Rights))
+        {
+            errors.Add(ValidationErrors.SystemRegister_ResourceId_Duplicates, [
+                "/registersystemrequest/rights/resource"
+            ]);
+        }
+
         List<MaskinPortenClientInfo> maskinPortenClients = await _systemRegisterService.GetMaskinportenClients(updateSystem.ClientId, cancellationToken);
         RegisteredSystem systemInfo = await _systemRegisterService.GetRegisteredSystemInfo(systemId);
+
+        if (AuthenticationHelper.DoesResourceAlreadyExists(updateSystem.Rights, systemInfo.Rights))
+        {
+            errors.Add(ValidationErrors.SystemRegister_ResourceId_AlreadyExists, [
+                "/registersystemrequest/rights/resource"
+            ]);
+        }
+
         foreach (string clientId in updateSystem.ClientId)
         {
             bool clientExistsForAnotherSystem = maskinPortenClients.FindAll(x => x.ClientId == clientId && x.SystemInternalId != systemInfo.InternalId).Count > 0;
@@ -201,6 +217,13 @@ public async Task<ActionResult<Guid>> CreateRegisteredSystem([FromBody] Register
                 ]);
             }
 
+            if (AuthenticationHelper.HasDuplicateRights(registerNewSystem.Rights))
+            {
+                errors.Add(ValidationErrors.SystemRegister_ResourceId_Duplicates, [
+                    "/registersystemrequest/rights/resource"
+                ]);
+            }
+
             if (!AuthenticationHelper.IsValidRedirectUrl(registerNewSystem.AllowedRedirectUrls))
             {
                 errors.Add(ValidationErrors.SystemRegister_InValid_RedirectUrlFormat, [
@@ -245,14 +268,41 @@ public async Task<ActionResult<Guid>> CreateRegisteredSystem([FromBody] Register
     /// <returns>true if changed</returns>
     [HttpPut("vendor/{systemId}/rights")]
     [Authorize(Policy = AuthzConstants.POLICY_SCOPE_SYSTEMREGISTER_WRITE)]
-    public async Task<ActionResult<SystemRegisterUpdateResult>> UpdateRightsOnRegisteredSystem([FromBody] List<Right> rights, string systemId)
+    public async Task<ActionResult<SystemRegisterUpdateResult>> UpdateRightsOnRegisteredSystem([FromBody] List<Right> rights, string systemId, CancellationToken cancellationToken = default)
     {
+        ValidationErrorBuilder errors = default;
         RegisteredSystem registerSystemResponse = await _systemRegisterService.GetRegisteredSystemInfo(systemId);
         if (!AuthenticationHelper.HasWriteAccess(registerSystemResponse.SystemVendorOrgNumber, User))
         {
             return Forbid();
         }
 
+        if (AuthenticationHelper.HasDuplicateRights(rights))
+        {
+            errors.Add(ValidationErrors.SystemRegister_ResourceId_Duplicates, [
+                "/registersystemrequest/rights/resource"
+            ]);
+        }
+
+        if (AuthenticationHelper.DoesResourceAlreadyExists(rights, registerSystemResponse.Rights))
+        {
+            errors.Add(ValidationErrors.SystemRegister_ResourceId_AlreadyExists, [
+                "/registersystemrequest/rights/resource"
+            ]);
+        }
+
+        if (!await _systemRegisterService.DoesResourceIdExists(rights, cancellationToken))
+        {
+            errors.Add(ValidationErrors.SystemRegister_ResourceId_DoesNotExist, [
+                "/registersystemrequest/rights/resource"
+            ]);
+        }
+
+        if (errors.TryToActionResult(out var errorResult))
+        {
+            return errorResult;
+        }
+
         bool success = await _systemRegisterService.UpdateRightsForRegisteredSystem(rights, systemId);
         if (!success)
         {

src/Authentication/Helpers/AuthenticationHelper.cs

@@ -383,5 +383,49 @@ public static int GetUserId(HttpContext context)
 
             return 0;
         }
+
+        /// <summary>
+        /// Check for duplicates in the rights
+        /// </summary>
+        /// <param name="rights">the resources that the system gives rights to</param>
+        /// <returns>true if duplicate rights found</returns>
+        public static bool HasDuplicateRights(List<Right> rights)
+        {
+            var uniqueRights = new HashSet<string>();
+
+            foreach (var right in rights)
+            {
+                var rightKey = $"{right.Action}:{string.Join(",", right.Resource.Select(r => $"{r.Id}:{r.Value}"))}";
+
+                if (!uniqueRights.Add(rightKey))
+                {
+                    return true; // Duplicate found
+                }
+            }
+
+            return false; // No duplicates
+        }
+
+        /// <summary>
+        /// Check for duplicates in the rights
+        /// </summary>
+        /// <param name="rights">the resources that the system gives rights to</param>
+        /// <returns>true if duplicate rights found</returns>
+        public static bool DoesResourceAlreadyExists(List<Right> rights, List<Right> existingRights)
+        {
+            var existingRightsSet = new HashSet<string>(existingRights.Select(r => $"{r.Action}:{string.Join(",", r.Resource.Select(res => $"{res.Id}:{res.Value}"))}"));
+
+            foreach (var right in rights)
+            {
+                var rightKey = $"{right.Action}:{string.Join(",", right.Resource.Select(r => $"{r.Id}:{r.Value}"))}";
+
+                if (existingRightsSet.Contains(rightKey))
+                {
+                    return true; // Duplicate found
+                }
+            }
+
+            return false; // No duplicates
+        }
     }
 }

src/Core/Errors/ValidationErrors.cs

@@ -2,10 +2,10 @@
 
 using Altinn.Authorization.ProblemDetails;
 
-namespace Altinn.ResourceRegistry.Core.Errors;
+namespace Altinn.Platform.Authentication.Core.Errors;
 
 /// <summary>
-/// Validation errors for the Resource Registry.
+/// Validation errors for the Authentication.
 /// </summary>
 public static class ValidationErrors
 {
@@ -47,4 +47,16 @@ private static readonly ValidationErrorDescriptorFactory _factory
     /// </summary>
     public static ValidationErrorDescriptor SystemRegister_InValid_RedirectUrlFormat { get; }
         = _factory.Create(5, "One or more of the redirect urls format is not valid. The valid format is https://xxx.xx");
+
+    /// <summary>
+    /// Gets a validation error descriptor for duplicate resource(rights) id
+    /// </summary>
+    public static ValidationErrorDescriptor SystemRegister_ResourceId_Duplicates { get; }
+        = _factory.Create(6, "One or more duplicate rights found");
+
+    /// <summary>
+    /// Gets a validation error descriptor for already existing resource(rights) id
+    /// </summary>
+    public static ValidationErrorDescriptor SystemRegister_ResourceId_AlreadyExists { get; }
+        = _factory.Create(7, "One or all the resources in rights to be updated is already found in the system");
 }

src/Persistance/RepositoryImplementations/SystemRegisterRepository.cs

@@ -364,7 +364,7 @@ private static ValueTask<RegisteredSystem> ConvertFromReaderToSystemRegister(Npg
             ClientId = clientIds,
             Rights = rights,
             IsVisible = reader.GetFieldValue<bool>("is_visible"),
-            AllowedRedirectUrls = reader.GetFieldValue<List<string>>("allowedredirecturls").ConvertAll<Uri>(delegate (string u) { return new Uri(u); })
+            AllowedRedirectUrls = reader.IsDBNull("allowedredirecturls") ? null : reader.GetFieldValue<List<string>>("allowedredirecturls")?.ConvertAll<Uri>(delegate (string u) { return new Uri(u); })
         });
     }
 

test/Altinn.Platform.Authentication.Tests/Altinn.Platform.Authentication.Tests.csproj

@@ -130,6 +130,12 @@
     <None Update="Data\Roles\user_1337\party_501337\roles.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
+    <None Update="Data\SystemRegister\Json\SystemRegisterWithoutResource.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
+    <None Update="Data\SystemRegister\Json\SystemRegisterDuplicateResource.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
     <None Update="Data\SystemRegister\Json\SystemRegister2RightsSubResource.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
@@ -172,6 +178,15 @@
     <None Update="Data\SystemRegister\Json\SystemRegister01.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
+    <None Update="Data\SystemRegister\Json\UpdateRightResourceIdExists.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
+    <None Update="Data\SystemRegister\Json\UpdateRightResourceIdNotExist.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
+    <None Update="Data\SystemRegister\Json\UpdateRightDuplicate.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
     <None Update="Data\SystemRegister\Json\UpdateRightInvalidRequest.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>