Skip to content

Commit 922b005

Browse files
committed
[RMZ-413] Fix setUserRole when defaultRole is admin
1 parent f608c36 commit 922b005

File tree

3 files changed

+108
-4
lines changed

3 files changed

+108
-4
lines changed

build.gradle.kts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@ tasks.jar {
144144
}
145145
}
146146

147+
tasks.test { useJUnitPlatform() }
148+
147149
// Dependencies version
148150

149151
// Required versions
@@ -237,7 +239,7 @@ dependencies {
237239
testImplementation("org.jetbrains.kotlin:kotlin-test")
238240

239241
// Use the Kotlin JUnit integration.
240-
testImplementation("org.jetbrains.kotlin:kotlin-test-junit")
242+
testImplementation("org.jetbrains.kotlin:kotlin-test-junit5")
241243

242244
annotationProcessor("org.springframework.boot:spring-boot-configuration-processor")
243245
}

src/main/kotlin/com/cosmotech/api/rbac/CsmRbac.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,10 @@ open class CsmRbac(
114114
): RbacSecurity {
115115
logger.info("RBAC ${rbacSecurity.id} - Setting user $userId roles")
116116
this.verifyRoleOrThrow(rbacSecurity, role, rolesDefinition)
117-
val currentRole = this.getUserRole(rbacSecurity, userId)
117+
val currentACLRole =
118+
rbacSecurity.accessControlList.firstOrNull { it.id.lowercase() == userId.lowercase() }?.role
118119
val adminRole = this.getAdminRole(rolesDefinition)
119-
if (currentRole == adminRole &&
120+
if (currentACLRole == adminRole &&
120121
role != adminRole &&
121122
this.getAdminCount(rbacSecurity, rolesDefinition) == 1) {
122123
throw CsmAccessForbiddenException(

src/test/kotlin/com/cosmotech/api/rbac/CsmRbacTests.kt

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import kotlin.test.Test
1919
import kotlin.test.assertEquals
2020
import kotlin.test.assertFalse
2121
import kotlin.test.assertTrue
22+
import org.junit.jupiter.api.DynamicTest
23+
import org.junit.jupiter.api.TestFactory
2224
import org.junit.jupiter.api.assertDoesNotThrow
2325
import org.junit.jupiter.api.assertThrows
2426
import org.slf4j.Logger
@@ -683,7 +685,7 @@ class CsmRbacTests {
683685
assertTrue(rbacTest.check(rbacSecurity, customPermission, definition))
684686
}
685687

686-
// Utilitary methods for rbac creation
688+
// Utility methods for rbac creation
687689
@Test
688690
fun `can add resource id and resource security in a second step`() {
689691
val definition = getCommonRolesDefinition()
@@ -695,6 +697,105 @@ class CsmRbacTests {
695697
assertTrue(rbacTest.check(rbacSecurity, PERMISSION_READ, definition))
696698
}
697699

700+
@TestFactory
701+
fun `add ACL entry when default role is admin with only one admin defined`() =
702+
listOf(ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).map { role ->
703+
DynamicTest.dynamicTest(
704+
"add ACL entry $role when default role is admin (with only one admin defined)") {
705+
val rbacDefinition =
706+
RbacSecurity(
707+
id = "rbacOnlyOneAdminWithAdminDefaultRole",
708+
default = ROLE_ADMIN,
709+
accessControlList =
710+
mutableListOf(
711+
RbacAccessControl(id = "[email protected]", role = ROLE_ADMIN)))
712+
val newUserId = "[email protected]"
713+
rbac.setUserRole(rbacDefinition, newUserId, role, getCommonRolesDefinition())
714+
assertTrue(rbacDefinition.accessControlList.size == 2)
715+
assertTrue(
716+
rbacDefinition.accessControlList.contains(
717+
RbacAccessControl(id = newUserId, role = role)))
718+
}
719+
}
720+
721+
@TestFactory
722+
fun `update ACL entry with only one admin defined`() =
723+
mapOf(ROLE_VIEWER to true, ROLE_USER to true, ROLE_EDITOR to true, ROLE_ADMIN to false).map {
724+
(role, shouldThrows) ->
725+
DynamicTest.dynamicTest("update ACL entry $role with only one admin defined") {
726+
val userId = "[email protected]"
727+
val rbacDefinition =
728+
RbacSecurity(
729+
id = "rbacOnlyOneAdmin",
730+
default = ROLE_NONE,
731+
accessControlList =
732+
mutableListOf(RbacAccessControl(id = userId, role = ROLE_ADMIN)))
733+
if (shouldThrows) {
734+
val assertThrows =
735+
assertThrows<CsmAccessForbiddenException> {
736+
rbac.setUserRole(rbacDefinition, userId, role, getCommonRolesDefinition())
737+
}
738+
assertEquals(
739+
"RBAC ${rbacDefinition.id} - It is forbidden to unset the last administrator",
740+
assertThrows.message)
741+
} else {
742+
assertDoesNotThrow {
743+
rbac.setUserRole(rbacDefinition, userId, role, getCommonRolesDefinition())
744+
assertTrue(rbacDefinition.accessControlList.size == 1)
745+
assertTrue(
746+
rbacDefinition.accessControlList.contains(
747+
RbacAccessControl(id = userId, role = role)))
748+
}
749+
}
750+
}
751+
}
752+
753+
@TestFactory
754+
fun `remove ACL entry when default role is admin with only one admin defined`() =
755+
listOf(ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).map { role ->
756+
DynamicTest.dynamicTest(
757+
"remove ACL entry $role when default role is admin (with only one admin defined)") {
758+
val userId = "[email protected]"
759+
val rbacDefinition =
760+
RbacSecurity(
761+
id = "rbacOnlyOneAdminWithAdminDefaultRole",
762+
default = ROLE_ADMIN,
763+
accessControlList =
764+
mutableListOf(RbacAccessControl(id = userId, role = ROLE_ADMIN)))
765+
val assertThrows =
766+
assertThrows<CsmAccessForbiddenException> {
767+
rbac.removeUser(rbacDefinition, userId, getCommonRolesDefinition())
768+
}
769+
assertEquals(
770+
"RBAC ${rbacDefinition.id} - It is forbidden to remove the last administrator",
771+
assertThrows.message)
772+
}
773+
}
774+
775+
@TestFactory
776+
fun `remove ACL entry when default role is admin with an admin and another user defined`() =
777+
listOf(ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).map { role ->
778+
DynamicTest.dynamicTest(
779+
"remove ACL entry $role when default role is admin (with only one admin defined)") {
780+
val userId = "[email protected]"
781+
val rbacDefinition =
782+
RbacSecurity(
783+
id = "rbacOnlyOneAdminWithAdminDefaultRole",
784+
default = ROLE_ADMIN,
785+
accessControlList =
786+
mutableListOf(
787+
RbacAccessControl(id = USER_ADMIN, role = ROLE_ADMIN),
788+
RbacAccessControl(id = userId, role = role)))
789+
assertDoesNotThrow {
790+
rbac.removeUser(rbacDefinition, userId, getCommonRolesDefinition())
791+
assertTrue(rbacDefinition.accessControlList.size == 1)
792+
assertTrue(
793+
rbacDefinition.accessControlList.contains(
794+
RbacAccessControl(id = USER_ADMIN, role = ROLE_ADMIN)))
795+
}
796+
}
797+
}
798+
698799
@Test
699800
fun `can add resource id and resource security in one call`() {
700801
val definition = getCommonRolesDefinition()

0 commit comments

Comments
 (0)