From e4dc88de13bceaf56face350dd1e37d62d026128 Mon Sep 17 00:00:00 2001 From: vramik Date: Tue, 30 Sep 2025 16:33:32 +0200 Subject: [PATCH] [FGAP] Make additional rest endpoints respect permissions Closes #40058 Signed-off-by: vramik --- .../keycloak/models/jpa/JpaUserProvider.java | 22 +++- .../jpa/entities/UserRoleMappingEntity.java | 1 - .../admin/ui/rest/SessionsResource.java | 15 ++- .../admin/RoleContainerResource.java | 2 + .../realm/UserConfigBuilder.java | 10 ++ .../fgap/UserResourceTypeFilteringTest.java | 122 ++++++++++++++++++ 6 files changed, 161 insertions(+), 11 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java index 3f900113628..08a8c9d984c 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java @@ -45,6 +45,7 @@ import org.keycloak.models.jpa.entities.UserConsentClientScopeEntity; import org.keycloak.models.jpa.entities.UserConsentEntity; import org.keycloak.models.jpa.entities.UserEntity; import org.keycloak.models.jpa.entities.UserGroupMembershipEntity; +import org.keycloak.models.jpa.entities.UserRoleMappingEntity; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; @@ -498,9 +499,7 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore, JpaUs @Override public Stream getRoleMembersStream(RealmModel realm, RoleModel role) { - TypedQuery query = em.createNamedQuery("usersInRole", UserEntity.class); - query.setParameter("roleId", role.getId()); - return closing(query.getResultStream().map(entity -> new UserAdapter(session, realm, em, entity))); + return getRoleMembersStream(realm, role, -1, -1); } @Override @@ -776,8 +775,21 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore, JpaUs @Override public Stream getRoleMembersStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) { - TypedQuery query = em.createNamedQuery("usersInRole", UserEntity.class); - query.setParameter("roleId", role.getId()); + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(UserEntity.class); + Root userRoleMapping = cq.from(UserRoleMappingEntity.class); + Root user = cq.from(UserEntity.class); + + List predicates = new ArrayList<>(); + predicates.add(cb.equal(userRoleMapping.get("roleId"), role.getId())); + predicates.add(cb.equal(userRoleMapping.get("user"), user)); + predicates.addAll(AdminPermissionsSchema.SCHEMA.applyAuthorizationFilters(session, AdminPermissionsSchema.USERS, this, realm, cb, cq, user)); + + cq.select(user) + .where(predicates.toArray(Predicate[]::new)) + .orderBy(cb.asc(user.get("username"))); + + TypedQuery query = em.createQuery(cq); final UserProvider users = session.users(); return closing(paginateQuery(query, firstResult, maxResults).getResultStream()) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java index 70500505665..ab2f800b336 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserRoleMappingEntity.java @@ -34,7 +34,6 @@ import java.io.Serializable; * @version $Revision: 1 $ */ @NamedQueries({ - @NamedQuery(name="usersInRole", query="select u from UserRoleMappingEntity m, UserEntity u where m.roleId=:roleId and u=m.user order by u.username"), @NamedQuery(name="userHasRole", query="select m from UserRoleMappingEntity m where m.user = :user and m.roleId = :roleId"), @NamedQuery(name="userRoleMappings", query="select m from UserRoleMappingEntity m where m.user = :user"), @NamedQuery(name="userRoleMappingIds", query="select m.roleId from UserRoleMappingEntity m where m.user = :user"), diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/SessionsResource.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/SessionsResource.java index c16dc75702b..2c389956436 100644 --- a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/SessionsResource.java +++ b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/SessionsResource.java @@ -59,8 +59,9 @@ public class SessionsResource { )} ) public Stream realmSessions(@QueryParam("type") @DefaultValue("ALL") final SessionType type, - @QueryParam("search") @DefaultValue("") final String search, @QueryParam("first") - @DefaultValue("0") int first, @QueryParam("max") @DefaultValue("10") int max) { + @QueryParam("search") @DefaultValue("") final String search, + @QueryParam("first") @DefaultValue("0") int first, + @QueryParam("max") @DefaultValue("10") int max) { auth.realm().requireViewRealm(); Stream sessionIdStream = Stream.builder().build(); @@ -115,8 +116,9 @@ public class SessionsResource { ) public Stream clientSessions(@QueryParam("clientId") final String clientId, @QueryParam("type") @DefaultValue("ALL") final SessionType type, - @QueryParam("search") @DefaultValue("") final String search, @QueryParam("first") - @DefaultValue("0") int first, @QueryParam("max") @DefaultValue("10") int max) { + @QueryParam("search") @DefaultValue("") final String search, + @QueryParam("first") @DefaultValue("0") int first, + @QueryParam("max") @DefaultValue("10") int max) { ClientModel clientModel = realm.getClientById(clientId); auth.clients().requireView(clientModel); @@ -134,7 +136,10 @@ public class SessionsResource { return applySearch(search, result).distinct().skip(first).limit(max); } - private static Stream applySearch(String search, Stream result) { + private Stream applySearch(String search, Stream result) { + result = result.filter(sessionRep -> + auth.users().canView(session.users().getUserById(realm, sessionRep.getUserId())) + ); if (!StringUtil.isBlank(search)) { String searchTrimmed = search.trim(); result = result.filter(s -> s.getUsername().contains(searchTrimmed) || s.getIpAddress().contains(searchTrimmed) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java index 130eddfb8f7..cc19aaea78a 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java @@ -569,6 +569,8 @@ public class RoleContainerResource extends RoleResource { @Parameter(description = "maximum number of results to return. Ignored if negative or {@code null}.") @QueryParam("max") Integer maxResults) { auth.roles().requireView(roleContainer); + auth.users().requireQuery(); + firstResult = firstResult != null ? firstResult : 0; maxResults = maxResults != null ? maxResults : Constants.DEFAULT_MAX_RESULTS; diff --git a/test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java b/test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java index 50b6b9e3f7f..860f6c30b33 100644 --- a/test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java +++ b/test-framework/core/src/main/java/org/keycloak/testframework/realm/UserConfigBuilder.java @@ -55,6 +55,16 @@ public class UserConfigBuilder { return this; } + public UserConfigBuilder firstName(String firstName) { + rep.setFirstName(firstName); + return this; + } + + public UserConfigBuilder lastName(String lastName) { + rep.setLastName(lastName); + return this; + } + public UserConfigBuilder emailVerified(boolean verified) { rep.setEmailVerified(verified); return this; diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java index 7e24b1dde0a..986b4c182b3 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java @@ -17,6 +17,7 @@ package org.keycloak.tests.admin.authz.fgap; +import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; @@ -34,13 +35,22 @@ import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; +import jakarta.ws.rs.client.Client; +import jakarta.ws.rs.client.WebTarget; +import jakarta.ws.rs.core.GenericType; +import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; 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.BearerAuthFilter; import org.keycloak.admin.client.resource.RolePoliciesResource; +import org.keycloak.admin.ui.rest.model.SessionRepresentation; import org.keycloak.authorization.fgap.AdminPermissionsSchema; import org.keycloak.models.AdminRoles; import org.keycloak.models.Constants; @@ -53,8 +63,12 @@ import org.keycloak.representations.idm.authorization.Logic; import org.keycloak.representations.idm.authorization.RolePolicyRepresentation; import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; import org.keycloak.testframework.annotations.InjectAdminClient; +import org.keycloak.testframework.annotations.InjectClient; +import org.keycloak.testframework.annotations.InjectKeycloakUrls; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.realm.ManagedClient; import org.keycloak.testframework.realm.UserConfigBuilder; +import org.keycloak.testframework.server.KeycloakUrls; import org.keycloak.testframework.util.ApiUtil; import org.keycloak.testframework.realm.RoleConfigBuilder; @@ -64,6 +78,12 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest { @InjectAdminClient(mode = InjectAdminClient.Mode.MANAGED_REALM, client = "myclient", user = "myadmin") Keycloak realmAdminClient; + @InjectKeycloakUrls + KeycloakUrls keycloakUrls; + + @InjectClient(ref = "test_client") + ManagedClient testClient; + private final String usersType = AdminPermissionsSchema.USERS.getType(); @BeforeEach @@ -402,4 +422,106 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest { assertFalse(search.isEmpty()); assertEquals(1, search.size()); } + + @Test + public void testSessionEndpointRespectsUserViewPermission() { + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + String clientUuid = realm.admin().clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId(); + RoleRepresentation viewRealmRole = realm.admin().clients().get(clientUuid).roles().get(AdminRoles.VIEW_REALM).toRepresentation(); + + // create users + for (int i = 0; i < 4; i++) { + String userId = ApiUtil.handleCreatedResponse(realm.admin().users().create(UserConfigBuilder.create() + .username("user" + i) + .password("password") + .firstName("user") + .lastName(Integer.toString(i)) + .email("user" + i + "@test") + .build())); + // assign view-realm role to user to be able to access the server info endpoint (to create session) + realm.admin().users().get(userId).roles().clientLevel(clientUuid).add(List.of(viewRealmRole)); + } + + // grant permission to view user1 and user2 to myadmin + UserPolicyRepresentation policy = createUserPolicy(realm, client, "Myadmin user policy", myadmin.getId()); + Set allowedUsers = Set.of("user1", "user2"); + createPermission(client, allowedUsers, usersType, Set.of(VIEW), policy); + + // assign view-realm role to myadmin so that the user can access the sessions endpoint + realm.admin().users().get(myadmin.getId()).roles().clientLevel(clientUuid).add(List.of(viewRealmRole)); + realm.cleanup().add(r -> r.users().get(myadmin.getId()).roles().clientLevel(clientUuid).remove(List.of(viewRealmRole))); + + // Create sessions for user1, user2 and user3 + Client httpClient = Keycloak.getClientProvider().newRestEasyClient(null, null, true);; + List keycloakInstances = List.of(); + try { + keycloakInstances = Stream.of("user1", "user2", "user3") + .map(username -> KeycloakBuilder.builder() + .serverUrl(keycloakUrls.getBaseUrl().toString()) + .realm(realm.getName()) + .grantType(OAuth2Constants.PASSWORD) + .clientId(Constants.ADMIN_CLI_CLIENT_ID) + .username(username) + .password("password") + .build()) + .peek(kc -> kc.serverInfo().getInfo()) // get server info to create the session + .toList(); + + WebTarget target = httpClient.target(keycloakUrls.getBaseUrl().toString()) + .path("admin") + .path("realms") + .path(realm.getName()) + .path("ui-ext") + .path("sessions") + .register(new BearerAuthFilter(realmAdminClient.tokenManager())); + + Response response = target.request(MediaType.APPLICATION_JSON).get(); + + assertThat(response.getStatus(), is(Response.Status.OK.getStatusCode())); + List sessions = response.readEntity(new GenericType>() {}).stream().map(SessionRepresentation::getUsername).toList(); + assertThat(sessions, hasSize(allowedUsers.size())); + assertThat(sessions, hasItems(allowedUsers.toArray(new String[0]))); + } finally { + //close http client + httpClient.close(); + //close keycloak instances + keycloakInstances.forEach(Keycloak::close); + } + } + + @Test + public void testRoleMemberFilteringByViewPermission() { + // Create client role + RoleRepresentation role = new RoleRepresentation(); + role.setName("test_role"); + realm.admin().clients().get(testClient.getId()).roles().create(role); + role = realm.admin().clients().get(testClient.getId()).roles().get(role.getName()).toRepresentation(); + realm.cleanup().add(r -> r.roles().deleteRole("test_role")); + + // assign role to users + for (String username : List.of("user_x", "user_y", "user_z")) { + String userId = ApiUtil.handleCreatedResponse(realm.admin().users().create(UserConfigBuilder.create() + .username(username) + .password("password") + .firstName("user") + .lastName(username) + .email(username + "@test") + .build())); + realm.admin().users().get(userId).roles().clientLevel(testClient.getId()).add(List.of(role)); + realm.cleanup().add(r -> r.users().delete(userId)); + } + + // Grant myadmin permission to view user_x and user_y, and to view the test client + UserPolicyRepresentation policy = createUserPolicy(realm, client, "Myadmin user policy", realm.admin().users().search("myadmin").get(0).getId()); + Set allowedUsers = Set.of("user_x", "user_y"); + createPermission(client, allowedUsers, AdminPermissionsSchema.USERS.getType(), Set.of(AdminPermissionsSchema.VIEW), policy); + createPermission(client, Set.of(testClient.getId()), AdminPermissionsSchema.CLIENTS.getType(), Set.of(AdminPermissionsSchema.VIEW), policy); + + // Query role members as myadmin + List roleMembers = realmAdminClient.realm(realm.getName()).clients().get(testClient.getId()).roles().get(role.getName()).getUserMembers().stream().map(UserRepresentation::getUsername).toList(); + + // Assert only permitted users are returned as role members + assertThat(roleMembers, hasSize(allowedUsers.size())); + assertThat(roleMembers, hasItems(allowedUsers.toArray(new String[0]))); + } }