From aecf43d755c9a461bc98d274dbe992de358e4b3a Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Tue, 23 Jul 2024 16:03:13 +0100 Subject: [PATCH 1/7] add validation to prevent changeParentGroup creating circular group hierarchies Signed-off-by: TomFitzpatrick --- .../writer/impl/group/impl/GroupWriterImpl.kt | 2 + .../impl/validation/EntityValidationUtil.kt | 22 ++++++++++ .../writer/impl/group/GroupWriterImplTest.kt | 32 ++++++++++++++ .../validation/EntityValidationUtilTest.kt | 42 +++++++++++++++++++ 4 files changed, 98 insertions(+) diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt index 476aae71624..880a17db455 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt @@ -60,6 +60,8 @@ class GroupWriterImpl( throw ConcurrentEntityModificationException("Group '${group.id}' has been modified since version '$requestVersion'.") } + validator.validateNewParentGroupNotADescendant(group.id, newParentGroup.id) + group.parentGroup = newParentGroup val updateTimestamp = Instant.now() diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt index 6dcbfa85ba9..f4612437b2e 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt @@ -74,6 +74,28 @@ class EntityValidationUtil(private val entityManager: EntityManager) { } } + fun validateNewParentGroupNotADescendant(groupToModify: String, newParentGroupId: String) { + val query = "SELECT g.id, g.parent_group FROM rbac_group g" + val groupMap = entityManager.createNativeQuery(query) + .resultList + .associate { row -> + val (groupId, parentGroupId) = row as Array<*> + groupId.toString() to parentGroupId?.toString() + } + + var currentGroup: String? = newParentGroupId + while (currentGroup != null && currentGroup != groupToModify) { + currentGroup = groupMap[currentGroup] + } + + if (currentGroup == groupToModify) { + throw IllegalEntityStateException( + "Cannot set Group '$newParentGroupId' as the parent of " + + "Group '$groupToModify' because it would create a cycle in the group hierarchy." + ) + } + } + fun validateAndGetRoleAssociatedWithGroup(group: Group, roleId: String): RoleGroupAssociation { val value = group.roleGroupAssociations.singleOrNull { it.role.id == roleId } if (value == null) { diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt index b3a907a3e37..f9945e254cd 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt @@ -205,6 +205,38 @@ class GroupWriterImplTest { assertEquals("Group 'groupId' has been modified since version '1'.", e.message) } + @Test + fun `changing parent group fails if it creates a circular dependency`() { + val changeGroupParentIdRequest = ChangeGroupParentIdRequest().apply { + groupId = "group1" + newParentGroupId = "group2" + } + val group1 = Group("group1", Instant.now(), "Group 1", null) + val group2 = Group("group2", Instant.now(), "Group 2", group1) + + val query = "SELECT g.id, g.parent_group FROM rbac_group g" + val resultList = listOf( + arrayOf("group1", null), + arrayOf("group2", "group1"), + arrayOf("group3", "group2") + ) + + whenever(entityManager.find(Group::class.java, "group1")).thenReturn(group1) + whenever(entityManager.find(Group::class.java, "group2")).thenReturn(group2) + whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) + whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) + + val e = assertThrows { + groupWriter.changeParentGroup(changeGroupParentIdRequest, requestUserId) + } + + assertEquals( + "Cannot set Group 'group2' as the parent of Group 'group1' because it would " + + "create a cycle in the group hierarchy.", + e.message + ) + } + @Test fun `add role to group fails when group does not exist`() { val addRoleToGroupRequest = AddRoleToGroupRequest().apply { diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt index ec65f1400fb..b9d15d944f6 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt @@ -203,6 +203,48 @@ class EntityValidationUtilTest { } } + @Test + fun `validateNewParentGroupNotADescendant throws exception when new parent is a descendant`() { + val groupToModify = "group1" + val newParentGroupId = "group2" + val query = "SELECT g.id, g.parent_group FROM rbac_group g" + val resultList = listOf( + arrayOf("group1", null), + arrayOf("group2", "group1"), + arrayOf("group3", "group2") + ) + + whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) + whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) + + assertThrows( + "Cannot set Group '$newParentGroupId' as the parent of Group '$groupToModify' because it " + + "would create a cycle in the group hierarchy." + ) { + validator.validateNewParentGroupNotADescendant(groupToModify, newParentGroupId) + } + } + + @Test + fun `validateNewParentGroupNotADescendant throws exception when new parent is the same group`() { + val groupToModify = "group1" + val newParentGroupId = "group1" + val query = "SELECT g.id, g.parent_group FROM rbac_group g" + val resultList = listOf( + arrayOf("group1", null) + ) + + whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) + whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) + + assertThrows( + "Cannot set Group '$newParentGroupId' as the parent of Group '$groupToModify' because it " + + "would create a cycle in the group hierarchy." + ) { + validator.validateNewParentGroupNotADescendant(groupToModify, newParentGroupId) + } + } + @Test fun `validateAndGetRoleAssociatedWithGroup returns association when role is associated with group`() { val roleId = "role1" From 4867f19b90c852178cfc32b3a2b4c73c18750a69 Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Tue, 23 Jul 2024 17:06:27 +0100 Subject: [PATCH 2/7] fix test Signed-off-by: TomFitzpatrick --- .../permissions/storage/writer/impl/group/GroupWriterImplTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt index f9945e254cd..f9c57879a60 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt @@ -159,6 +159,7 @@ class GroupWriterImplTest { whenever(entityManager.find(Group::class.java, "groupId")).thenReturn(group) whenever(entityManager.find(Group::class.java, "parentId")).thenReturn(parentGroup) + whenever(entityManager.createNativeQuery(any())).thenReturn(mock()) // Add this line groupWriter.changeParentGroup(changeGroupParentIdRequest, requestUserId) From 8fa7ccc1dfe59876da9ac93569b06ff95bb617ca Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Wed, 24 Jul 2024 16:59:44 +0100 Subject: [PATCH 3/7] remove unnecessary comment Signed-off-by: TomFitzpatrick --- .../storage/writer/impl/group/GroupWriterImplTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt index f9c57879a60..991552100fb 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt @@ -159,7 +159,7 @@ class GroupWriterImplTest { whenever(entityManager.find(Group::class.java, "groupId")).thenReturn(group) whenever(entityManager.find(Group::class.java, "parentId")).thenReturn(parentGroup) - whenever(entityManager.createNativeQuery(any())).thenReturn(mock()) // Add this line + whenever(entityManager.createNativeQuery(any())).thenReturn(mock()) groupWriter.changeParentGroup(changeGroupParentIdRequest, requestUserId) From 3aab05f797ddbc5c8458a3826d02b0ed33403a05 Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Wed, 24 Jul 2024 17:24:01 +0100 Subject: [PATCH 4/7] fix detekt Signed-off-by: TomFitzpatrick --- .../storage/writer/impl/validation/EntityValidationUtil.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt index f4612437b2e..d7edf98b43c 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt @@ -13,6 +13,7 @@ import net.corda.permissions.model.RoleUserAssociation import net.corda.permissions.model.User import javax.persistence.EntityManager +@Suppress("TooManyFunctions") class EntityValidationUtil(private val entityManager: EntityManager) { fun requireEntityExists(type: Class, id: Any): T { From 311be9d9f1ea644d3c8edf39443f60bdcf774491 Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Wed, 24 Jul 2024 18:00:53 +0100 Subject: [PATCH 5/7] address comments Signed-off-by: TomFitzpatrick --- .../impl/validation/EntityValidationUtil.kt | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt index d7edf98b43c..1ea9f52399b 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt @@ -76,24 +76,22 @@ class EntityValidationUtil(private val entityManager: EntityManager) { } fun validateNewParentGroupNotADescendant(groupToModify: String, newParentGroupId: String) { - val query = "SELECT g.id, g.parent_group FROM rbac_group g" - val groupMap = entityManager.createNativeQuery(query) + val groupMap = entityManager.createQuery("FROM Group", Group::class.java) .resultList - .associate { row -> - val (groupId, parentGroupId) = row as Array<*> - groupId.toString() to parentGroupId?.toString() + .associate { group -> + group.id to group.parentGroup?.id } var currentGroup: String? = newParentGroupId - while (currentGroup != null && currentGroup != groupToModify) { + while (currentGroup != null) { currentGroup = groupMap[currentGroup] - } - if (currentGroup == groupToModify) { - throw IllegalEntityStateException( - "Cannot set Group '$newParentGroupId' as the parent of " + - "Group '$groupToModify' because it would create a cycle in the group hierarchy." - ) + if (currentGroup == groupToModify) { + throw IllegalEntityStateException( + "Cannot set Group '$newParentGroupId' as the parent of " + + "Group '$groupToModify' because it would create a cycle in the group hierarchy." + ) + } } } From 0e58dc4aea4eeed81b465eb00d1dabefb9e422c0 Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Wed, 24 Jul 2024 18:41:47 +0100 Subject: [PATCH 6/7] move validation to rest worker Signed-off-by: TomFitzpatrick --- .../impl/PermissionGroupManagerImpl.kt | 18 ++++++++ .../impl/PermissionGroupManagerImplTest.kt | 24 +++++++++++ .../writer/impl/group/impl/GroupWriterImpl.kt | 2 - .../impl/validation/EntityValidationUtil.kt | 21 ---------- .../writer/impl/group/GroupWriterImplTest.kt | 33 --------------- .../validation/EntityValidationUtilTest.kt | 42 ------------------- 6 files changed, 42 insertions(+), 98 deletions(-) diff --git a/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt b/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt index 632079406d3..c902dc30f6d 100644 --- a/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt +++ b/libs/permissions/permission-manager-impl/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( rpcSender, @@ -140,4 +141,21 @@ 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/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt b/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt index 4a9139a0414..e349eb01cb7 100644 --- a/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt +++ b/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt @@ -130,6 +130,30 @@ 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>() + whenever(future.getOrThrow(defaultTimeout)).thenReturn(PermissionManagementResponse(avroGroup)) + whenever(permissionManagementCache.getGroup(groupId)).thenReturn(avroGroup) + whenever(permissionManagementCache.getGroup(childGroupId)).thenReturn(childGroup) + + val requestCaptor = argumentCaptor() + whenever(rpcSender.sendRequest(requestCaptor.capture())).thenReturn(future) + + val changeGroupParentIdDto = ChangeGroupParentIdDto("requestedBy", groupId, childGroupId) + + val exception = assertThrows { + 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() diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt index 880a17db455..476aae71624 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/group/impl/GroupWriterImpl.kt @@ -60,8 +60,6 @@ class GroupWriterImpl( throw ConcurrentEntityModificationException("Group '${group.id}' has been modified since version '$requestVersion'.") } - validator.validateNewParentGroupNotADescendant(group.id, newParentGroup.id) - group.parentGroup = newParentGroup val updateTimestamp = Instant.now() diff --git a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt index 1ea9f52399b..6dcbfa85ba9 100644 --- a/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt +++ b/libs/permissions/permission-storage-writer-impl/src/main/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtil.kt @@ -13,7 +13,6 @@ import net.corda.permissions.model.RoleUserAssociation import net.corda.permissions.model.User import javax.persistence.EntityManager -@Suppress("TooManyFunctions") class EntityValidationUtil(private val entityManager: EntityManager) { fun requireEntityExists(type: Class, id: Any): T { @@ -75,26 +74,6 @@ class EntityValidationUtil(private val entityManager: EntityManager) { } } - fun validateNewParentGroupNotADescendant(groupToModify: String, newParentGroupId: String) { - val groupMap = entityManager.createQuery("FROM Group", Group::class.java) - .resultList - .associate { group -> - group.id to group.parentGroup?.id - } - - var currentGroup: String? = newParentGroupId - while (currentGroup != null) { - currentGroup = groupMap[currentGroup] - - if (currentGroup == groupToModify) { - throw IllegalEntityStateException( - "Cannot set Group '$newParentGroupId' as the parent of " + - "Group '$groupToModify' because it would create a cycle in the group hierarchy." - ) - } - } - } - fun validateAndGetRoleAssociatedWithGroup(group: Group, roleId: String): RoleGroupAssociation { val value = group.roleGroupAssociations.singleOrNull { it.role.id == roleId } if (value == null) { diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt index 991552100fb..b3a907a3e37 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/group/GroupWriterImplTest.kt @@ -159,7 +159,6 @@ class GroupWriterImplTest { whenever(entityManager.find(Group::class.java, "groupId")).thenReturn(group) whenever(entityManager.find(Group::class.java, "parentId")).thenReturn(parentGroup) - whenever(entityManager.createNativeQuery(any())).thenReturn(mock()) groupWriter.changeParentGroup(changeGroupParentIdRequest, requestUserId) @@ -206,38 +205,6 @@ class GroupWriterImplTest { assertEquals("Group 'groupId' has been modified since version '1'.", e.message) } - @Test - fun `changing parent group fails if it creates a circular dependency`() { - val changeGroupParentIdRequest = ChangeGroupParentIdRequest().apply { - groupId = "group1" - newParentGroupId = "group2" - } - val group1 = Group("group1", Instant.now(), "Group 1", null) - val group2 = Group("group2", Instant.now(), "Group 2", group1) - - val query = "SELECT g.id, g.parent_group FROM rbac_group g" - val resultList = listOf( - arrayOf("group1", null), - arrayOf("group2", "group1"), - arrayOf("group3", "group2") - ) - - whenever(entityManager.find(Group::class.java, "group1")).thenReturn(group1) - whenever(entityManager.find(Group::class.java, "group2")).thenReturn(group2) - whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) - whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) - - val e = assertThrows { - groupWriter.changeParentGroup(changeGroupParentIdRequest, requestUserId) - } - - assertEquals( - "Cannot set Group 'group2' as the parent of Group 'group1' because it would " + - "create a cycle in the group hierarchy.", - e.message - ) - } - @Test fun `add role to group fails when group does not exist`() { val addRoleToGroupRequest = AddRoleToGroupRequest().apply { diff --git a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt index b9d15d944f6..ec65f1400fb 100644 --- a/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt +++ b/libs/permissions/permission-storage-writer-impl/src/test/kotlin/net/corda/libs/permissions/storage/writer/impl/validation/EntityValidationUtilTest.kt @@ -203,48 +203,6 @@ class EntityValidationUtilTest { } } - @Test - fun `validateNewParentGroupNotADescendant throws exception when new parent is a descendant`() { - val groupToModify = "group1" - val newParentGroupId = "group2" - val query = "SELECT g.id, g.parent_group FROM rbac_group g" - val resultList = listOf( - arrayOf("group1", null), - arrayOf("group2", "group1"), - arrayOf("group3", "group2") - ) - - whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) - whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) - - assertThrows( - "Cannot set Group '$newParentGroupId' as the parent of Group '$groupToModify' because it " + - "would create a cycle in the group hierarchy." - ) { - validator.validateNewParentGroupNotADescendant(groupToModify, newParentGroupId) - } - } - - @Test - fun `validateNewParentGroupNotADescendant throws exception when new parent is the same group`() { - val groupToModify = "group1" - val newParentGroupId = "group1" - val query = "SELECT g.id, g.parent_group FROM rbac_group g" - val resultList = listOf( - arrayOf("group1", null) - ) - - whenever(entityManager.createNativeQuery(query)).thenReturn(mock()) - whenever(entityManager.createNativeQuery(query).resultList).thenReturn(resultList) - - assertThrows( - "Cannot set Group '$newParentGroupId' as the parent of Group '$groupToModify' because it " + - "would create a cycle in the group hierarchy." - ) { - validator.validateNewParentGroupNotADescendant(groupToModify, newParentGroupId) - } - } - @Test fun `validateAndGetRoleAssociatedWithGroup returns association when role is associated with group`() { val roleId = "role1" From d44ec15d1625ae544aa6ce52c6254b08afdf4498 Mon Sep 17 00:00:00 2001 From: TomFitzpatrick Date: Wed, 24 Jul 2024 19:11:02 +0100 Subject: [PATCH 7/7] fix detekt Signed-off-by: TomFitzpatrick --- .../manager/impl/PermissionGroupManagerImpl.kt | 8 +++++--- .../manager/impl/PermissionGroupManagerImplTest.kt | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt b/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt index c902dc30f6d..db70719f0e5 100644 --- a/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt +++ b/libs/permissions/permission-manager-impl/src/main/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImpl.kt @@ -150,10 +150,12 @@ class PermissionGroupManagerImpl( "Permission management cache is null." } var currentGroup: String? = newParentGroupId - while(currentGroup != null) { + 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.") + 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/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt b/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt index e349eb01cb7..2f49a6cf706 100644 --- a/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt +++ b/libs/permissions/permission-manager-impl/src/test/kotlin/net/corda/libs/permissions/manager/impl/PermissionGroupManagerImplTest.kt @@ -151,7 +151,10 @@ class PermissionGroupManagerImplTest { 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) + assertEquals( + "Cannot set Group '$childGroupId' as the parent of Group '$groupId' because it would create a cycle in the group hierarchy.", + exception.message + ) } @Test