From 8e8c9700edc1f9c93b7edcc8971c9444d9612634 Mon Sep 17 00:00:00 2001 From: vramik Date: Mon, 26 May 2025 12:45:16 +0200 Subject: [PATCH 1/2] Allow mapping Admin roles by server administrator only Closes #39956 # Conflicts: # services/src/main/java/org/keycloak/services/resources/admin/fgap/RolePermissionsV2.java # tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/RoleResourceTypeEvaluationTest.java Signed-off-by: vramik --- .../admin/permissions/RolePermissionsV2.java | 39 +++++++++++++++---- .../fgap/RoleResourceTypeEvaluationTest.java | 32 +++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissionsV2.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissionsV2.java index ee5e0fb02c30..d8a025edd998 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissionsV2.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissionsV2.java @@ -19,6 +19,7 @@ import static org.keycloak.authorization.AdminPermissionsSchema.ROLES_RESOURCE_TYPE; +import org.keycloak.Config; import org.keycloak.authorization.AdminPermissionsSchema; import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.model.Policy; @@ -49,12 +50,28 @@ public class RolePermissionsV2 extends RolePermissions { super(session, realm, authz, root); } + private boolean hasMasterAdminRole() { + RealmModel masterRealm = root.adminsRealm().getName().equals(Config.getAdminRealm()) ? + root.adminsRealm(): + session.realms().getRealmByName(Config.getAdminRealm()); + + RoleModel adminRole = masterRealm.getRole(AdminRoles.ADMIN); + return root.admin().hasRole(adminRole); + } + @Override public boolean canMapClientScope(RoleModel role) { - if (root.clients().canManageClientsDefault()) return true; - if (role.getContainer() instanceof ClientModel clientModel) { - if (root.clients().canMapClientScopeRoles(clientModel)) return true; + if (root.hasOneAdminRole(AdminRoles.MANAGE_CLIENTS)) { + return true; + } + if (root.clients().canMapClientScopeRoles(clientModel)) { + return true; + } + } else { + if (root.hasOneAdminRole(AdminRoles.MANAGE_REALM)) { + return true; + } } return hasPermission(role, MAP_ROLE_CLIENT_SCOPE_SCOPE); @@ -62,24 +79,32 @@ public boolean canMapClientScope(RoleModel role) { @Override public boolean canMapComposite(RoleModel role) { - if (canManageDefault(role)) return checkAdminRoles(role); + if (AdminRoles.ALL_ROLES.contains(role.getName()) && !hasMasterAdminRole()) { + return false; + } if (role.getContainer() instanceof ClientModel clientModel) { if (root.clients().canMapCompositeRoles(clientModel)) return true; } - return hasPermission(role, MAP_ROLE_COMPOSITE_SCOPE) && checkAdminRoles(role); + return hasPermission(role, MAP_ROLE_COMPOSITE_SCOPE); } @Override public boolean canMapRole(RoleModel role) { - if (root.hasOneAdminRole(AdminRoles.MANAGE_USERS)) return checkAdminRoles(role); + if (AdminRoles.ALL_ROLES.contains(role.getName()) && !hasMasterAdminRole()) { + return false; + } + + if (root.hasOneAdminRole(AdminRoles.MANAGE_USERS)) { + return true; + } if (role.getContainer() instanceof ClientModel clientModel) { if (root.clients().canMapRoles(clientModel)) return true; } - return hasPermission(role, MAP_ROLE_SCOPE) && checkAdminRoles(role); + return hasPermission(role, MAP_ROLE_SCOPE); } @Override diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/RoleResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/RoleResourceTypeEvaluationTest.java index 50a92809440c..f444277178ec 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/RoleResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/RoleResourceTypeEvaluationTest.java @@ -25,6 +25,7 @@ import org.keycloak.admin.client.resource.ClientScopeResource; import org.keycloak.admin.client.resource.ScopePermissionsResource; import org.keycloak.authorization.AdminPermissionsSchema; +import org.keycloak.models.AdminRoles; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.RoleRepresentation; @@ -171,4 +172,35 @@ public void testMapRoleOnlySpecificRole() { assertThat(ex, instanceOf(ForbiddenException.class)); } } + + @Test + public void testMappingAdminRoles() { + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + ClientRepresentation realmManagement = realm.admin().clients().findByClientId("realm-management").get(0); + RoleRepresentation createClientRole = realm.admin().clients().get(realmManagement.getId()).roles().get(AdminRoles.CREATE_CLIENT).toRepresentation(); + + // create permission to map roles from all clients and to all users + UserPolicyRepresentation onlyMyAdminUserPolicy = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId()); + createAllPermission(client, AdminPermissionsSchema.CLIENTS_RESOURCE_TYPE, onlyMyAdminUserPolicy, Set.of(MAP_ROLES)); + createAllPermission(client, AdminPermissionsSchema.USERS_RESOURCE_TYPE, onlyMyAdminUserPolicy, Set.of(MAP_ROLES)); + + // create a role + RoleRepresentation role = new RoleRepresentation(); + role.setName("myRole"); + ClientRepresentation myclient = realm.admin().clients().findByClientId("myclient").get(0); + realm.admin().clients().get(myclient.getId()).roles().create(role); + role = realm.admin().clients().get(myclient.getId()).roles().get("myRole").toRepresentation(); + + // should pass + realmAdminClient.realm(realm.getName()).users().get(myadmin.getId()).roles().clientLevel(myclient.getId()).add(List.of(role)); + + // should fail as it is admin role and myadmin does not have master realm admin role assigned + try { + realmAdminClient.realm(realm.getName()).users().get(myadmin.getId()).roles().clientLevel(realmManagement.getId()) + .add(List.of(createClientRole)); + fail("Expected exception wasn't thrown."); + } catch (Exception ex) { + assertThat(ex, instanceOf(ForbiddenException.class)); + } + } } From 11dc04ccb5daa4a209dd4f0b83cbb436dd7b5866 Mon Sep 17 00:00:00 2001 From: vramik Date: Thu, 5 Jun 2025 10:59:54 +0200 Subject: [PATCH 2/2] Add a note to release notes about admin roles mapping Fixes #39956 # Conflicts: # docs/documentation/release_notes/topics/26_3_0.adoc Signed-off-by: vramik --- docs/documentation/release_notes/index.adoc | 3 +++ docs/documentation/release_notes/topics/26_2_6.adoc | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 docs/documentation/release_notes/topics/26_2_6.adoc diff --git a/docs/documentation/release_notes/index.adoc b/docs/documentation/release_notes/index.adoc index 43d812a0885d..9e5fb3633166 100644 --- a/docs/documentation/release_notes/index.adoc +++ b/docs/documentation/release_notes/index.adoc @@ -13,6 +13,9 @@ include::topics/templates/document-attributes.adoc[] :release_header_latest_link: {releasenotes_link_latest} include::topics/templates/release-header.adoc[] +== {project_name_full} 26.2.6 +include::topics/26_2_6.adoc[leveloffset=2] + == {project_name_full} 26.2.0 include::topics/26_2_0.adoc[leveloffset=2] diff --git a/docs/documentation/release_notes/topics/26_2_6.adoc b/docs/documentation/release_notes/topics/26_2_6.adoc new file mode 100644 index 000000000000..834cebda688b --- /dev/null +++ b/docs/documentation/release_notes/topics/26_2_6.adoc @@ -0,0 +1,3 @@ += Restrict admin role mappings to server administrators + +To enhance security, only users with the `admin` role in the `master` realm (server admins) can assign admin roles. This ensures that critical permissions cannot be delegated by realm-level administrators.