diff --git a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java index d05f33b6f8..6582ad134a 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java @@ -135,7 +135,7 @@ public class AuthenticationManagementResource { @NoCache @Produces(MediaType.APPLICATION_JSON) public List> getClientAuthenticatorProviders() { - auth.requireAnyAdminRole(); + auth.realm().requireViewRealm(); List factories = session.getKeycloakSessionFactory().getProviderFactories(ClientAuthenticator.class); return buildProviderMetadata(factories); @@ -900,7 +900,7 @@ public class AuthenticationManagementResource { @Produces(MediaType.APPLICATION_JSON) @NoCache public List getRequiredActions() { - auth.requireAnyAdminRole(); + auth.realm().requireViewRealm(); List list = new LinkedList<>(); for (RequiredActionProviderModel model : realm.getRequiredActionProviders()) { @@ -1095,7 +1095,7 @@ public class AuthenticationManagementResource { @Produces(MediaType.APPLICATION_JSON) @NoCache public Map> getPerClientConfigDescription() { - auth.requireAnyAdminRole(); + auth.realm().requireViewRealm(); List factories = session.getKeycloakSessionFactory().getProviderFactories(ClientAuthenticator.class); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java index bc8474bcbc..a87dbeee7a 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java @@ -107,7 +107,7 @@ public class ClientsResource { ClientRepresentation representation = ModelToRepresentation.toRepresentation(clientModel, session); rep.add(representation); representation.setAccess(auth.clients().getAccess(clientModel)); - } else if (!viewableOnly) { + } else if (!viewableOnly && auth.clients().canView(clientModel)) { ClientRepresentation client = new ClientRepresentation(); client.setId(clientModel.getId()); client.setClientId(clientModel.getClientId()); @@ -122,7 +122,7 @@ public class ClientsResource { ClientRepresentation representation = ModelToRepresentation.toRepresentation(clientModel, session); representation.setAccess(auth.clients().getAccess(clientModel)); rep.add(representation); - } else if (!viewableOnly && auth.clients().canList()){ + } else if (!viewableOnly && auth.clients().canView(clientModel)){ ClientRepresentation client = new ClientRepresentation(); client.setId(clientModel.getId()); client.setClientId(clientModel.getClientId()); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissions.java index f6ab47e8c6..ae8ac33de6 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissions.java @@ -224,7 +224,11 @@ class ClientPermissions implements ClientPermissionEvaluator, ClientPermissionM @Override public boolean canList() { - return root.hasAnyAdminRole(); + return canView() || root.hasOneAdminRole(AdminRoles.QUERY_CLIENTS); + } + + public boolean canList(ClientModel clientModel) { + return canView(clientModel) || root.hasOneAdminRole(AdminRoles.QUERY_CLIENTS); } @Override @@ -236,7 +240,7 @@ class ClientPermissions implements ClientPermissionEvaluator, ClientPermissionM @Override public boolean canListClientScopes() { - return root.hasAnyAdminRole(); + return canView() || root.hasOneAdminRole(AdminRoles.QUERY_CLIENTS); } @Override diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java index dabc02e8f6..d51dee2b46 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java @@ -141,7 +141,7 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag @Override public boolean canList() { - return root.hasOneAdminRole(AdminRoles.VIEW_USERS, AdminRoles.MANAGE_USERS, AdminRoles.QUERY_GROUPS); + return canView() || root.hasOneAdminRole(AdminRoles.VIEW_USERS, AdminRoles.MANAGE_USERS, AdminRoles.QUERY_GROUPS); } @Override diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RealmPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RealmPermissions.java index ce04d6d7c1..477fe3a11c 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/RealmPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/RealmPermissions.java @@ -77,7 +77,7 @@ class RealmPermissions implements RealmPermissionEvaluator { @Override public boolean canListRealms() { - return root.hasAnyAdminRole(); + return canViewRealm() || root.hasOneAdminRole(AdminRoles.QUERY_REALMS); } @Override 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 5ef81005b4..3280d27642 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 @@ -324,7 +324,13 @@ class RolePermissions implements RolePermissionEvaluator, RolePermissionManageme @Override public boolean canList(RoleContainerModel container) { - return root.hasAnyAdminRole(); + if (canView(container)) { + return true; + } else if (container instanceof RealmModel) { + return root.realm().canListRealms(); + } else { + return root.clients().canList((ClientModel)container); + } } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java index 736c0c2aa8..e061d1906b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.admin; +import org.hamcrest.Matchers; import org.jboss.resteasy.plugins.providers.multipart.MultipartFormDataOutput; import org.junit.AfterClass; import org.junit.Before; @@ -36,6 +37,7 @@ import org.keycloak.representations.idm.ClientInitialAccessCreatePresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.ConfigPropertyRepresentation; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; @@ -293,7 +295,14 @@ public class PermissionsTest extends AbstractKeycloakTest { realm.toRepresentation(); } }, Resource.REALM, false, true); - assertGettersEmpty(clients.get(AdminRoles.VIEW_USERS).realm(REALM_NAME).toRepresentation()); + assertGettersEmpty(clients.get(AdminRoles.QUERY_REALMS).realm(REALM_NAME).toRepresentation()); + + // this should throw forbidden as "query-users" role isn't enough + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.QUERY_USERS).realm(REALM_NAME).toRepresentation(); + } + }, clients.get(AdminRoles.QUERY_USERS), false); invoke(new Invocation() { public void invoke(RealmResource realm) { @@ -480,8 +489,18 @@ public class PermissionsTest extends AbstractKeycloakTest { realm.clients().findAll(); } }, Resource.CLIENT, false, true); - List l = clients.get(AdminRoles.VIEW_USERS).realm(REALM_NAME).clients().findAll(); - assertGettersEmpty(l.get(0)); + List l = clients.get(AdminRoles.QUERY_CLIENTS).realm(REALM_NAME).clients().findAll(); + Assert.assertThat(l, Matchers.empty()); + + l = clients.get(AdminRoles.VIEW_CLIENTS).realm(REALM_NAME).clients().findAll(); + Assert.assertThat(l, Matchers.not(Matchers.empty())); + + // this should throw forbidden as "query-users" role isn't enough + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.QUERY_USERS).realm(REALM_NAME).clients().findAll(); + } + }, clients.get(AdminRoles.QUERY_USERS), false); invoke(new Invocation() { public void invoke(RealmResource realm) { @@ -808,6 +827,13 @@ public class PermissionsTest extends AbstractKeycloakTest { invoke((RealmResource realm) -> { realm.clientScopes().get(scope.getId()).getScopeMappings().clientLevel(realmAccessClient.getId()).remove(Collections.emptyList()); }, Resource.CLIENT, true); + + // this should throw forbidden as "query-users" role isn't enough + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.QUERY_USERS).realm(REALM_NAME).clientScopes().findAll(); + } + }, clients.get(AdminRoles.QUERY_USERS), false); } @Test @@ -945,6 +971,13 @@ public class PermissionsTest extends AbstractKeycloakTest { realm.roles().list(); } }, Resource.REALM, false, true); + + // this should throw forbidden as "create-client" role isn't enough + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.CREATE_CLIENT).realm(REALM_NAME).roles().list(); + } + }, clients.get(AdminRoles.CREATE_CLIENT), false); invoke(new Invocation() { public void invoke(RealmResource realm) { realm.roles().get("sample-role").toRepresentation(); @@ -1155,6 +1188,13 @@ public class PermissionsTest extends AbstractKeycloakTest { realm.flows().updateAuthenticatorConfig("nosuch", new AuthenticatorConfigRepresentation()); } }, Resource.REALM, true); + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.VIEW_REALM).realm(REALM_NAME).flows().getPerClientConfigDescription(); + clients.get(AdminRoles.VIEW_REALM).realm(REALM_NAME).flows().getClientAuthenticatorProviders(); + clients.get(AdminRoles.VIEW_REALM).realm(REALM_NAME).flows().getRequiredActions(); + } + }, adminClient, true); // Re-create realm adminClient.realm(REALM_NAME).remove(); @@ -1239,6 +1279,12 @@ public class PermissionsTest extends AbstractKeycloakTest { GroupRepresentation group = adminClient.realms().realm(REALM_NAME).getGroupByPath("mygroup"); ClientRepresentation realmAccessClient = adminClient.realms().realm(REALM_NAME).clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0); + // this should throw forbidden as "create-client" role isn't enough + invoke(new Invocation() { + public void invoke(RealmResource realm) { + clients.get(AdminRoles.CREATE_CLIENT).realm(REALM_NAME).groups().groups(); + } + }, clients.get(AdminRoles.CREATE_CLIENT), false); invoke(new Invocation() { public void invoke(RealmResource realm) {