diff --git a/services/src/main/java/org/keycloak/services/resources/admin/fgap/FineGrainedAdminPermissionEvaluator.java b/services/src/main/java/org/keycloak/services/resources/admin/fgap/FineGrainedAdminPermissionEvaluator.java index 49ce3196a4d..02186ebc1c1 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/fgap/FineGrainedAdminPermissionEvaluator.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/fgap/FineGrainedAdminPermissionEvaluator.java @@ -18,7 +18,6 @@ package org.keycloak.services.resources.admin.fgap; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; @@ -90,6 +89,9 @@ class FineGrainedAdminPermissionEvaluator { if (!root.isAdminSameRealm()) { return false; } + if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(root.realm)) { + return false; + } ResourceServer server = root.realmResourceServer(); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java index 403550fa555..d871caa939c 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java @@ -39,12 +39,16 @@ import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import org.junit.jupiter.api.Test; +import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.KeycloakBuilder; import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.authorization.fgap.AdminPermissionsSchema; +import org.keycloak.models.AdminRoles; +import org.keycloak.models.Constants; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.GroupRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.authorization.GroupPolicyRepresentation; @@ -52,10 +56,12 @@ import org.keycloak.representations.idm.authorization.Logic; import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; import org.keycloak.testframework.annotations.InjectAdminClient; +import org.keycloak.testframework.annotations.InjectKeycloakUrls; import org.keycloak.testframework.annotations.InjectUser; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.realm.ManagedUser; import org.keycloak.testframework.realm.UserConfigBuilder; +import org.keycloak.testframework.server.KeycloakUrls; import org.keycloak.testframework.util.ApiUtil; @KeycloakIntegrationTest @@ -67,6 +73,9 @@ public class UserResourceTypeEvaluationTest extends AbstractPermissionTest { @InjectAdminClient(mode = InjectAdminClient.Mode.MANAGED_REALM, client = "myclient", user = "myadmin") Keycloak realmAdminClient; + @InjectKeycloakUrls + KeycloakUrls keycloakUrls; + private final String usersType = AdminPermissionsSchema.USERS.getType(); private final String newUserUsername = "new_user"; @@ -166,9 +175,9 @@ public class UserResourceTypeEvaluationTest extends AbstractPermissionTest { assertEquals("email@test.com", realmAdminClient.realm(realm.getName()).users().get(newUserId).toRepresentation().getEmail()); // remove the user permission - getScopePermissionsResource(client).findAll(null, null, null, null, null).forEach(permission -> { - getScopePermissionsResource(client).findById(permission.getId()).remove(); - }); + getScopePermissionsResource(client).findAll(null, null, null, null, null).forEach(permission -> + getScopePermissionsResource(client).findById(permission.getId()).remove() + ); // updating the user should be denied try { @@ -495,4 +504,40 @@ public class UserResourceTypeEvaluationTest extends AbstractPermissionTest { UserRepresentation representation = realmAdminClient.realm(realm.getName()).users().get(myadmin.getId()).toRepresentation(); assertThat(representation, notNullValue()); } + + @Test + public void testViewUserWithAdminRoleAfterDisablingFgap() { + // setup permission to allow view all users by myadmin + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + UserPolicyRepresentation allowMyAdminPermission = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId()); + createAllPermission(client, usersType, allowMyAdminPermission, Set.of(VIEW)); + + // get userAlice user by myadmin + UserRepresentation representation = realmAdminClient.realm(realm.getName()).users().get(userAlice.getId()).toRepresentation(); + assertThat(representation, notNullValue()); + + // disable FGAP for realm + RealmRepresentation realmRep = realm.admin().toRepresentation(); + realmRep.setAdminPermissionsEnabled(Boolean.FALSE); + realm.admin().update(realmRep); + + //assign view-users role to myadmin + String realmManagementClientId = realm.admin().clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId(); + RoleRepresentation viewUsersRole = realm.admin().clients().get(realmManagementClientId).roles().get(AdminRoles.VIEW_USERS).toRepresentation(); + realm.admin().users().get(myadmin.getId()).roles().clientLevel(realmManagementClientId).add(List.of(viewUsersRole)); + + // get userAlice user by myadmin again - it threw NPE before the fix + // need to use separate Keycloak instance so that new role assignment is picked up + try (Keycloak keycloak = KeycloakBuilder.builder() + .serverUrl(keycloakUrls.getBaseUrl().toString()) + .realm(realm.getName()) + .grantType(OAuth2Constants.PASSWORD) + .clientId(Constants.ADMIN_CLI_CLIENT_ID) + .username(myadmin.getUsername()) + .password("password") + .build()) { + representation = keycloak.realm(realm.getName()).users().get(userAlice.getId()).toRepresentation(); + assertThat(representation, notNullValue()); + } + } }