Skip to content

Commit fc708ef

Browse files
Leopold-CramerLeopold-Cramer
committed
feat: add PR feedbacks + fix broken tests + add new edge case tests for keycloak groups
1 parent 39d2985 commit fc708ef

File tree

19 files changed

+411
-332
lines changed

19 files changed

+411
-332
lines changed

‎common/src/main/kotlin/com/cosmotech/common/rbac/CsmRbac.kt‎

Lines changed: 81 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,20 @@ open class CsmRbac(
3030
val accessControls = mutableListOf<String>()
3131
objectSecurity.accessControlList.forEach {
3232
if (accessControls.contains(it.id)) {
33-
throw IllegalArgumentException("User ${it.id} is referenced multiple times in the security")
33+
throw IllegalArgumentException(
34+
"Entity ${it.id} is referenced multiple times in the security")
3435
}
3536
accessControls.add(it.id)
3637
}
3738

3839
// Make sure we have at least one admin
3940
if (!objectSecurity.accessControlList.any { it.role == ROLE_ADMIN }) {
40-
val currentUserId = getCurrentAccountIdentifier(csmPlatformProperties)
41-
val currentUserACL = objectSecurity.accessControlList.find { it.id == currentUserId }
42-
if (currentUserACL != null) {
43-
currentUserACL.role = ROLE_ADMIN
41+
val currentEntityId = getCurrentAccountIdentifier(csmPlatformProperties)
42+
val currentEntityACL = objectSecurity.accessControlList.find { it.id == currentEntityId }
43+
if (currentEntityACL != null) {
44+
currentEntityACL.role = ROLE_ADMIN
4445
} else {
45-
objectSecurity.accessControlList.add(RbacAccessControl(currentUserId, ROLE_ADMIN))
46+
objectSecurity.accessControlList.add(RbacAccessControl(currentEntityId, ROLE_ADMIN))
4647
}
4748
}
4849

@@ -56,27 +57,27 @@ open class CsmRbac(
5657
) {
5758
if (!this.check(rbacSecurity, permission, rolesDefinition))
5859
throw CsmAccessForbiddenException(
59-
"RBAC ${rbacSecurity.id} - User does not have permission $permission")
60+
"RBAC ${rbacSecurity.id} - Entity does not have permission $permission")
6061
}
6162

6263
fun check(
6364
rbacSecurity: RbacSecurity,
6465
permission: String,
6566
rolesDefinition: RolesDefinition = getCommonRolesDefinition()
6667
): Boolean {
67-
logger.info("RBAC ${rbacSecurity.id} - Verifying permission $permission for user")
68+
logger.info("RBAC ${rbacSecurity.id} - Verifying permission $permission for entity")
6869
if (!this.csmPlatformProperties.rbac.enabled) {
6970
logger.debug("RBAC ${rbacSecurity.id} - RBAC check not enabled")
7071
return true
7172
}
72-
var userIsAdminOrHasPermission = this.isAdmin(rbacSecurity, rolesDefinition)
73-
if (!userIsAdminOrHasPermission) {
74-
val user = getCurrentAccountIdentifier(this.csmPlatformProperties)
73+
var entityIsAdminOrHasPermission = this.isAdmin(rbacSecurity, rolesDefinition)
74+
if (!entityIsAdminOrHasPermission) {
75+
val entity = getCurrentAccountIdentifier(this.csmPlatformProperties)
7576
val groups = getCurrentAccountGroups(this.csmPlatformProperties)
76-
userIsAdminOrHasPermission =
77-
this.verifyRbac(rbacSecurity, permission, rolesDefinition, user, groups)
77+
entityIsAdminOrHasPermission =
78+
this.verifyRbac(rbacSecurity, permission, rolesDefinition, entity, groups)
7879
}
79-
return userIsAdminOrHasPermission
80+
return entityIsAdminOrHasPermission
8081
}
8182

8283
fun setDefault(
@@ -92,33 +93,35 @@ open class CsmRbac(
9293
return rbacSecurity
9394
}
9495

95-
fun addUserRole(
96+
fun addEntityRole(
9697
parentRbacSecurity: RbacSecurity,
9798
rbacSecurity: RbacSecurity,
98-
userId: String,
99+
entityId: String,
99100
role: String,
100101
rolesDefinition: RolesDefinition = getCommonRolesDefinition()
101102
): RbacSecurity {
102103

103104
if (!isAdmin(rbacSecurity, rolesDefinition)) {
104-
this.checkUserExists(
105+
this.checkEntityExists(
105106
parentRbacSecurity,
106-
userId,
107-
"User $userId not found in parent ${parentRbacSecurity.id} component")
107+
entityId,
108+
"Entity $entityId not found in parent ${parentRbacSecurity.id} component")
108109
}
109-
return setUserRole(rbacSecurity, userId, role, rolesDefinition)
110+
return setEntityRole(rbacSecurity, entityId, role, rolesDefinition)
110111
}
111112

112-
fun setUserRole(
113+
fun setEntityRole(
113114
rbacSecurity: RbacSecurity,
114-
userId: String,
115+
entityId: String,
115116
role: String,
116117
rolesDefinition: RolesDefinition = getCommonRolesDefinition()
117118
): RbacSecurity {
118-
logger.info("RBAC ${rbacSecurity.id} - Setting user $userId roles")
119+
logger.info("RBAC ${rbacSecurity.id} - Setting entity $entityId roles")
119120
this.verifyRoleOrThrow(rbacSecurity, role, rolesDefinition)
120121
val currentACLRole =
121-
rbacSecurity.accessControlList.firstOrNull { it.id.lowercase() == userId.lowercase() }?.role
122+
rbacSecurity.accessControlList
123+
.firstOrNull { it.id.lowercase() == entityId.lowercase() }
124+
?.role
122125
val adminRole = this.getAdminRole(rolesDefinition)
123126
if (currentACLRole == adminRole &&
124127
role != adminRole &&
@@ -127,48 +130,48 @@ open class CsmRbac(
127130
"RBAC ${rbacSecurity.id} - It is forbidden to unset the last administrator")
128131
}
129132
val accessList = rbacSecurity.accessControlList
130-
val userAccess = accessList.find { it.id == userId }
131-
if (userAccess == null) {
132-
accessList.add(RbacAccessControl(userId, role))
133+
val entityAccess = accessList.find { it.id == entityId }
134+
if (entityAccess == null) {
135+
accessList.add(RbacAccessControl(entityId, role))
133136
} else {
134-
userAccess.role = role
137+
entityAccess.role = role
135138
}
136139
return rbacSecurity
137140
}
138141

139-
fun getUsers(rbacSecurity: RbacSecurity): List<String> {
142+
fun getEntities(rbacSecurity: RbacSecurity): List<String> {
140143
return (rbacSecurity.accessControlList.map { it.id })
141144
}
142145

143-
fun getAccessControl(rbacSecurity: RbacSecurity, userId: String): RbacAccessControl {
144-
return rbacSecurity.accessControlList.find { it.id == userId }
146+
fun getAccessControl(rbacSecurity: RbacSecurity, entityId: String): RbacAccessControl {
147+
return rbacSecurity.accessControlList.find { it.id == entityId }
145148
?: throw CsmResourceNotFoundException(
146-
"User $userId not found in ${rbacSecurity.id} component")
149+
"Entity $entityId not found in ${rbacSecurity.id} component")
147150
}
148151

149-
fun checkUserExists(
152+
fun checkEntityExists(
150153
rbacSecurity: RbacSecurity,
151-
userId: String,
152-
exceptionUserNotFoundMessage: String
154+
entityId: String,
155+
exceptionEntityNotFoundMessage: String
153156
): RbacAccessControl {
154-
return rbacSecurity.accessControlList.find { it.id == userId }
155-
?: throw CsmResourceNotFoundException(exceptionUserNotFoundMessage)
157+
return rbacSecurity.accessControlList.find { it.id == entityId }
158+
?: throw CsmResourceNotFoundException(exceptionEntityNotFoundMessage)
156159
}
157160

158-
fun removeUser(
161+
fun removeEntity(
159162
rbacSecurity: RbacSecurity,
160-
userId: String,
163+
entityId: String,
161164
rolesDefinition: RolesDefinition = getCommonRolesDefinition()
162165
): RbacSecurity {
163-
logger.info("RBAC ${rbacSecurity.id} - Removing user $userId from security")
164-
checkUserExists(rbacSecurity, userId, "User $userId not found")
165-
val role = this.getUserRole(rbacSecurity, userId)
166+
logger.info("RBAC ${rbacSecurity.id} - Removing entity $entityId from security")
167+
checkEntityExists(rbacSecurity, entityId, "Entity $entityId not found")
168+
val role = this.getEntityRole(rbacSecurity, entityId)
166169
if (role == (this.getAdminRole(rolesDefinition)) &&
167170
this.getAdminCount(rbacSecurity, rolesDefinition) == 1) {
168171
throw CsmAccessForbiddenException(
169172
"RBAC ${rbacSecurity.id} - It is forbidden to remove the last administrator")
170173
}
171-
rbacSecurity.accessControlList.removeIf { it.id == userId }
174+
rbacSecurity.accessControlList.removeIf { it.id == entityId }
172175
return rbacSecurity
173176
}
174177

@@ -184,47 +187,46 @@ open class CsmRbac(
184187

185188
internal fun verifyAdminRole(
186189
rbacSecurity: RbacSecurity,
187-
user: String?,
188-
groups: List<String>?,
190+
user: String,
191+
groups: List<String>,
189192
rolesDefinition: RolesDefinition
190193
): Boolean {
191194
logger.debug("RBAC ${rbacSecurity.id} - Verifying if $user has default admin rbac role")
192-
var isAdmin = false
193-
groups?.forEach {
194-
if (this.getUserRole(rbacSecurity, it) == this.getAdminRole(rolesDefinition)) {
195-
isAdmin = true
196-
}
197-
}
198-
if (user != null) {
199-
if (this.getUserRole(rbacSecurity, user) == this.getAdminRole(rolesDefinition)) {
200-
isAdmin = true
201-
}
202-
}
203-
logger.debug("RBAC ${rbacSecurity.id} - $user have default admin rbac role: $isAdmin")
195+
val isAdmin =
196+
if (rbacSecurity.accessControlList.any() { it.id == user }) {
197+
this.getEntityRole(rbacSecurity, user) == this.getAdminRole(rolesDefinition)
198+
} else {
199+
groups.any {
200+
this.getEntityRole(rbacSecurity, it) == this.getAdminRole(rolesDefinition)
201+
} ||
202+
this.getEntityRole(rbacSecurity, rbacSecurity.default) ==
203+
this.getAdminRole(rolesDefinition)
204+
}
205+
logger.debug("RBAC ${rbacSecurity.id} - $user has default admin rbac role: $isAdmin")
204206
return isAdmin
205207
}
206208

207-
internal fun verifyUser(
209+
internal fun verifyEntity(
208210
rbacSecurity: RbacSecurity,
209211
permission: String,
210212
rolesDefinition: RolesDefinition,
211213
user: String,
212214
groups: List<String>
213215
): Boolean {
214-
logger.debug("RBAC ${rbacSecurity.id} - Verifying $user has permission in ACL: $permission")
215-
var isAuthorized = false
216-
groups.forEach {
217-
if (this.verifyPermissionFromRole(
218-
permission, getUserRole(rbacSecurity, it), rolesDefinition)) {
219-
isAuthorized = true
220-
}
221-
}
222-
if (user.isNotEmpty()) {
223-
if (this.verifyPermissionFromRole(
224-
permission, getUserRole(rbacSecurity, user), rolesDefinition))
225-
isAuthorized = true
226-
}
227-
logger.debug("RBAC ${rbacSecurity.id} - $user has permission $permission in ACL: $isAuthorized")
216+
logger.debug(
217+
"RBAC ${rbacSecurity.id} - Verifying $user or one of $groups has permission in ACL: $permission")
218+
val isAuthorized =
219+
if (rbacSecurity.accessControlList.any() { it.id == user }) {
220+
verifyPermissionFromRole(permission, getEntityRole(rbacSecurity, user), rolesDefinition)
221+
} else {
222+
groups.any {
223+
verifyPermissionFromRole(permission, getEntityRole(rbacSecurity, it), rolesDefinition)
224+
} ||
225+
verifyPermissionFromRole(
226+
permission, getEntityRole(rbacSecurity, rbacSecurity.default), rolesDefinition)
227+
}
228+
logger.debug(
229+
"RBAC ${rbacSecurity.id} - $user or one of $groups has permission $permission in ACL: $isAuthorized")
228230
return isAuthorized
229231
}
230232

@@ -249,7 +251,7 @@ open class CsmRbac(
249251
groups: List<String>
250252
): Boolean {
251253
return (this.verifyDefault(rbacSecurity, permission, rolesDefinition) ||
252-
this.verifyUser(rbacSecurity, permission, rolesDefinition, user, groups))
254+
this.verifyEntity(rbacSecurity, permission, rolesDefinition, user, groups))
253255
}
254256

255257
internal fun verifyPermissionFromRole(
@@ -268,9 +270,9 @@ open class CsmRbac(
268270
return rolesDefinition[role] ?: listOf()
269271
}
270272

271-
internal fun getUserRole(rbacSecurity: RbacSecurity, user: String): String {
273+
internal fun getEntityRole(rbacSecurity: RbacSecurity, entity: String): String {
272274
return rbacSecurity.accessControlList
273-
.firstOrNull { it.id.lowercase() == user.lowercase() }
275+
.firstOrNull { it.id.lowercase() == entity.lowercase() }
274276
?.role ?: rbacSecurity.default
275277
}
276278

@@ -290,8 +292,8 @@ open class CsmRbac(
290292
throw CsmClientException("RBAC ${rbacSecurity.id} - Role $role does not exist")
291293
}
292294

293-
internal fun verifyPermission(permission: String, userPermissions: List<String>): Boolean {
294-
return userPermissions.contains(permission)
295+
internal fun verifyPermission(permission: String, entityPermissions: List<String>): Boolean {
296+
return entityPermissions.contains(permission)
295297
}
296298

297299
internal fun verifyPermissionFromRoles(
@@ -303,9 +305,9 @@ open class CsmRbac(
303305
}
304306

305307
internal fun isAdminToken(rbacSecurity: RbacSecurity): Boolean {
306-
logger.debug("RBAC ${rbacSecurity.id} - Verifying if user has platform admin role in token")
308+
logger.debug("RBAC ${rbacSecurity.id} - Verifying if entity has platform admin role in token")
307309
val isAdmin = csmAdmin.verifyCurrentRolesAdmin()
308-
logger.debug("RBAC ${rbacSecurity.id} - user has platform admin role in token: $isAdmin")
310+
logger.debug("RBAC ${rbacSecurity.id} - entity has platform admin role in token: $isAdmin")
309311
return isAdmin
310312
}
311313

0 commit comments

Comments
 (0)