From c38b6d585e317221c1bd774bdd59a81a3a633a4e Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Wed, 28 Mar 2018 05:15:37 -0400 Subject: [PATCH] KEYCLOAK-528 (#5103) --- .../java/org/keycloak/models/AdminRoles.java | 1 + .../admin/permissions/MgmtPermissions.java | 4 + .../admin/permissions/RolePermissions.java | 16 +- .../admin/IllegalAdminUpgradeTest.java | 170 ++++++++++++++++++ 4 files changed, 190 insertions(+), 1 deletion(-) diff --git a/server-spi-private/src/main/java/org/keycloak/models/AdminRoles.java b/server-spi-private/src/main/java/org/keycloak/models/AdminRoles.java index c2304e8572..6178dc81b4 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/AdminRoles.java +++ b/server-spi-private/src/main/java/org/keycloak/models/AdminRoles.java @@ -65,5 +65,6 @@ public class AdminRoles { ALL_ROLES.add(ADMIN); ALL_ROLES.add(CREATE_REALM); ALL_ROLES.add(CREATE_CLIENT); + ALL_ROLES.add(REALM_ADMIN); } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java index 6fa044f255..8994718403 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java @@ -196,6 +196,10 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage return admin; } + public RealmModel adminsRealm() { + return adminsRealm; + } + @Override public RolePermissions roles() { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissions.java index 361cb0c25a..464f512b42 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RolePermissions.java @@ -230,7 +230,20 @@ class RolePermissions implements RolePermissionEvaluator, RolePermissionManageme } else { return true; } - } else { + } else if (role.getName().equals(AdminRoles.REALM_ADMIN)) { + // check to see if we have masterRealm.admin role. Otherwise abort + if (root.adminsRealm() == null || !root.adminsRealm().getName().equals(Config.getAdminRealm())) { + return adminConflictMessage(role); + } + + RealmModel masterRealm = root.adminsRealm(); + RoleModel adminRole = masterRealm.getRole(AdminRoles.ADMIN); + if (root.admin().hasRole(adminRole)) { + return true; + } else { + return adminConflictMessage(role); + } + } else { return adminConflictMessage(role); } @@ -239,6 +252,7 @@ class RolePermissions implements RolePermissionEvaluator, RolePermissionManageme if (role.getContainer() instanceof RealmModel) { RealmModel realm = (RealmModel)role.getContainer(); // If realm role is master admin role then abort + // if realm name is master realm, than we know this is a admin role in master realm. if (realm.getName().equals(Config.getAdminRealm())) { return adminConflictMessage(role); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IllegalAdminUpgradeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IllegalAdminUpgradeTest.java index 3318a6db01..e155cb5076 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IllegalAdminUpgradeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IllegalAdminUpgradeTest.java @@ -21,6 +21,7 @@ import org.jboss.shrinkwrap.api.spec.WebArchive; import org.junit.Assert; import org.junit.Test; import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.models.AdminRoles; @@ -115,6 +116,12 @@ public class IllegalAdminUpgradeTest extends AbstractKeycloakTest { session.userCredentialManager().updateCredential(realm, user, UserCredentialModel.password("password")); } + //@Test + public void testConsole() throws Exception { + testingClient.server().run(IllegalAdminUpgradeTest::setupUsers); + Thread.sleep(10000000); + } + @Test public void testRestEvaluation() throws Exception { testingClient.server().run(IllegalAdminUpgradeTest::setupUsers); @@ -141,6 +148,7 @@ public class IllegalAdminUpgradeTest extends AbstractKeycloakTest { RoleRepresentation realmQueryUsers = adminClient.realm(TEST).clients().get(realmAdminClient.getId()).roles().get(AdminRoles.QUERY_USERS).toRepresentation(); RoleRepresentation realmQueryClients = adminClient.realm(TEST).clients().get(realmAdminClient.getId()).roles().get(AdminRoles.QUERY_CLIENTS).toRepresentation(); RoleRepresentation realmQueryGroups = adminClient.realm(TEST).clients().get(realmAdminClient.getId()).roles().get(AdminRoles.QUERY_GROUPS).toRepresentation(); + RoleRepresentation realmAdmin = adminClient.realm(TEST).clients().get(realmAdminClient.getId()).roles().get(AdminRoles.REALM_ADMIN).toRepresentation(); ClientRepresentation masterClient = adminClient.realm("master").clients().findByClientId(TEST + "-realm").get(0); RoleRepresentation masterManageAuthorization = adminClient.realm("master").clients().get(masterClient.getId()).roles().get(AdminRoles.MANAGE_AUTHORIZATION).toRepresentation(); @@ -186,6 +194,168 @@ public class IllegalAdminUpgradeTest extends AbstractKeycloakTest { } + roles.clear(); + roles.add(realmAdmin); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmManageClients); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmViewClients); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmManageEvents); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmViewEvents); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmManageIdentityProviders); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmViewIdentityProviders); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmManageRealm); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmViewRealm); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmImpersonate); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmManageUsers); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).remove(roles); + + roles.clear(); + roles.add(realmViewUsers); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).remove(roles); + + roles.clear(); + roles.add(realmQueryUsers); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).remove(roles); + + roles.clear(); + roles.add(realmQueryGroups); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).remove(roles); + + roles.clear(); + roles.add(realmQueryClients); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).remove(roles); + + realmClient.close(); + } + // test master user with manage_users can't assign realm's admin roles + { + ClientRepresentation client = realmAdminClient; + Keycloak realmClient = AdminClientUtil.createAdminClient(suiteContext.isAdapterCompatTesting(), + "master", "userAdmin", "password", Constants.ADMIN_CLI_CLIENT_ID, null); + roles.clear(); + roles.add(realmManageAuthorization); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmViewAuthorization); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + + roles.clear(); + roles.add(realmAdmin); + try { + realmClient.realm(TEST).users().get(realmUser.getId()).roles().clientLevel(client.getId()).add(roles); + Assert.fail("should fail with forbidden exception"); + } catch (ClientErrorException e) { + Assert.assertEquals(e.getResponse().getStatus(), 403); + + } + roles.clear(); roles.add(realmManageClients); try {