CORE-20803: Add validation to prevent changeParentGroup creating circ…

…ular group hierarchies (#6304)
corda · Jul 25, 2024 · 583dfe3 · 583dfe3
1 parent edfe1ee
commit 583dfe3
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 0 deletions.
diff --git a/...mpl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt b/...mpl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt
@@ -55,6 +55,7 @@ class PermissionGroupManagerImpl(
         }
         val cachedGroup = permissionManagementCache.getGroup(changeGroupParentIdDto.groupId)!!
         val groupEntityVersion = cachedGroup.version
+        validateNewGroupIsNotChildOfGroup(changeGroupParentIdDto.groupId, changeGroupParentIdDto.newParentGroupId)
 
         val result = sendPermissionWriteRequest<Group>(
             rpcSender,
@@ -140,4 +141,23 @@ class PermissionGroupManagerImpl(
         )
         return result.convertToResponseDto()
     }
+
+    private fun validateNewGroupIsNotChildOfGroup(groupToModifyId: String, newParentGroupId: String?) {
+        if (groupToModifyId == newParentGroupId) {
+            throw IllegalArgumentException("Group '$groupToModifyId' cannot be its own parent.")
+        }
+        val permissionManagementCache = checkNotNull(permissionManagementCacheRef.get()) {
+            "Permission management cache is null."
+        }
+        var currentGroup: String? = newParentGroupId
+        while (currentGroup != null) {
+            if (currentGroup == groupToModifyId) {
+                throw IllegalArgumentException(
+                    "Cannot set Group '$newParentGroupId' as the parent of " +
+                        "Group '$groupToModifyId' because it would create a cycle in the group hierarchy."
+                )
+            }
+            currentGroup = permissionManagementCache.getGroup(currentGroup)?.parentGroupId
+        }
+    }
 }
diff --git a/...src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt b/...src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt
@@ -130,6 +130,33 @@ class PermissionGroupManagerImplTest {
         assertTrue(result.roleAssociations.isEmpty())
     }
 
+    @Test
+    fun `change parent group throws exception if new parent group is a child of the group`() {
+        val groupId = UUID.randomUUID().toString()
+        val childGroupId = UUID.randomUUID().toString()
+        val avroGroup = Group(groupId, 0, ChangeDetails(Instant.now()), "groupName", null, emptyList(), emptyList())
+        val childGroup = Group(childGroupId, 0, ChangeDetails(Instant.now()), "childGroupName", groupId, emptyList(), emptyList())
+
+        val future = mock<CompletableFuture<PermissionManagementResponse>>()
+        whenever(future.getOrThrow(defaultTimeout)).thenReturn(PermissionManagementResponse(avroGroup))
+        whenever(permissionManagementCache.getGroup(groupId)).thenReturn(avroGroup)
+        whenever(permissionManagementCache.getGroup(childGroupId)).thenReturn(childGroup)
+
+        val requestCaptor = argumentCaptor<PermissionManagementRequest>()
+        whenever(rpcSender.sendRequest(requestCaptor.capture())).thenReturn(future)
+
+        val changeGroupParentIdDto = ChangeGroupParentIdDto("requestedBy", groupId, childGroupId)
+
+        val exception = assertThrows<IllegalArgumentException> {
+            manager.changeParentGroup(changeGroupParentIdDto)
+        }
+
+        assertEquals(
+            "Cannot set Group '$childGroupId' as the parent of Group '$groupId' because it would create a cycle in the group hierarchy.",
+            exception.message
+        )
+    }
+
     @Test
     fun `add role to group sends rpc request and converts result to response dto`() {
         val groupId = UUID.randomUUID().toString()