Skip to content

Commit 255c19d

Browse files
committed
[RMZ-413] Fix setUserRole when defaultRole is admin
1 parent 10ae3b0 commit 255c19d

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
@@ -677,7 +679,7 @@ class CsmRbacTests {
677679
assertTrue(rbacTest.check(rbacSecurity, customPermission, definition))
678680
}
679681

680-
// Utilitary methods for rbac creation
682+
// Utility methods for rbac creation
681683
@Test
682684
fun `can add resource id and resource security in a second step`() {
683685
val definition = getCommonRolesDefinition()
@@ -689,6 +691,105 @@ class CsmRbacTests {
689691
assertTrue(rbacTest.check(rbacSecurity, PERMISSION_READ, definition))
690692
}
691693

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

0 commit comments

Comments
 (0)