[ODS-3401] Cycles in claims taxonomy (security metadata for resource claims) causes requests to hang

Application/EdFi.Security.DataAccess/Repositories/SecurityRepository.cs

@@ -36,7 +36,6 @@ public virtual AuthorizationStrategy GetAuthorizationStrategyByName(string autho
 
         public virtual IList<ClaimSetResourceClaimAction> GetClaimsForClaimSet(string claimSetName)
         {
-
             return _securityTableGateway.GetClaimSetResourceClaimActions()
                 .Where(c => c.ClaimSet.ClaimSetName.Equals(claimSetName, StringComparison.OrdinalIgnoreCase))
                 .ToList();
@@ -54,10 +53,9 @@ public virtual IList<string> GetResourceClaimLineage(string resourceClaimUri)
                 .ToList();
         }
 
-        private IEnumerable<ResourceClaim> GetResourceClaimLineageForResourceClaim(string resourceClaimUri)
+        private IEnumerable<ResourceClaim> GetResourceClaimLineageForResourceClaim(string resourceClaimUri, List<ResourceClaim> resourceClaimLineage = null)
         {
-            var resourceClaimLineage = new List<ResourceClaim>();
-
+            resourceClaimLineage ??= new List<ResourceClaim>();
             ResourceClaim resourceClaim;
 
             try
@@ -72,14 +70,21 @@ private IEnumerable<ResourceClaim> GetResourceClaimLineageForResourceClaim(strin
                 throw new InvalidOperationException($"Multiple resource claims with a claim name of '{resourceClaimUri}' were found in the Ed-Fi API's security configuration. Authorization cannot be performed.", ex);
             }
 
-            if (resourceClaim != null)
+            if (resourceClaim == null)
+            {
+                return resourceClaimLineage;
+            }
+
+            if (resourceClaimLineage.Any(rc => rc.ClaimName == resourceClaim.ClaimName))
             {
-                resourceClaimLineage.Add(resourceClaim);
+                throw new InvalidOperationException($"A cycle was detected in the resource claim hierarchy of the security metadata: '{string.Join("' -> '", resourceClaimLineage.SkipWhile(rc => rc.ClaimName != resourceClaim.ClaimName).Select(rc => rc.ClaimName))}' -> '{resourceClaimUri}'");
+            }
+
+            resourceClaimLineage.Add(resourceClaim);
 
-                if (resourceClaim.ParentResourceClaim != null)
-                {
-                    resourceClaimLineage.AddRange(GetResourceClaimLineageForResourceClaim(resourceClaim.ParentResourceClaim.ClaimName));
-                }
+            if (resourceClaim.ParentResourceClaim != null)
+            {
+                GetResourceClaimLineageForResourceClaim(resourceClaim.ParentResourceClaim.ClaimName, resourceClaimLineage);
             }
 
             return resourceClaimLineage;

tests/EdFi.Security.DataAccess.UnitTests/Repositories/SecurityRepositoryTests.cs

@@ -3,6 +3,7 @@
 // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
 // See the LICENSE and NOTICES files in the project root for more information.
 
+using System;
 using System.Linq;
 
 namespace EdFi.Security.DataAccess.UnitTests.Repositories;
@@ -165,6 +166,26 @@ public void GetResourceClaimLineage_ShouldReturnEmptyListForInvalidResourceClaim
         // Assert
         lineage.ShouldBeEmpty();
     }
+
+    [Test]
+    public void GetResourceClaimLineage_ThrowsException_WhenResourceClaimLineageContainsCycle()
+    {
+        // Arrange
+        var resourceClaimA = new ResourceClaim { ClaimName = "uri-A", ResourceClaimId = 1, ParentResourceClaim = null, ParentResourceClaimId = null};
+        var resourceClaimB = new ResourceClaim { ClaimName = "uri-B", ResourceClaimId = 2, ParentResourceClaim = resourceClaimA, ParentResourceClaimId = 1};
+        var resourceClaimC = new ResourceClaim { ClaimName = "uri-C", ResourceClaimId = 3, ParentResourceClaim = resourceClaimB, ParentResourceClaimId = 2};
+        var resourceClaimD = new ResourceClaim { ClaimName = "uri-D", ResourceClaimId = 4, ParentResourceClaim = resourceClaimC, ParentResourceClaimId = 3};
+
+        resourceClaimA.ParentResourceClaimId = resourceClaimC.ResourceClaimId;
+        resourceClaimA.ParentResourceClaim = resourceClaimC;
+
+        A.CallTo(() => _securityTableGateway.GetResourceClaims())
+            .Returns(new List<ResourceClaim> { resourceClaimA, resourceClaimB, resourceClaimC, resourceClaimD });
+
+        // Act & Assert
+        Should.Throw<InvalidOperationException>(() => _securityRepository.GetResourceClaimLineage("uri-D"))
+            .Message.ShouldBe("A cycle was detected in the resource claim hierarchy of the security metadata: 'uri-C' -> 'uri-B' -> 'uri-A' -> 'uri-C'");
+    }
 
     [Test]
     public void GetResourceClaimLineageMetadata_ShouldReturnMetadataForValidResourceClaimAndAction()