PartialEvaluator ignores view-* and manage-* roles

Closes #38284

Signed-off-by: vramik <vramik@redhat.com>
This commit is contained in:
vramik
2025-03-20 11:04:50 +01:00
committed by Pedro Igor
parent 8f7c1871a7
commit a72d15b857
4 changed files with 36 additions and 8 deletions

View File

@@ -157,10 +157,9 @@ public class BruteForceUsersResource {
Boolean briefRepresentation, Stream<UserModel> userModels) {
boolean briefRepresentationB = briefRepresentation != null && briefRepresentation;
usersEvaluator.grantIfNoPermission(session.getAttribute(UserModel.GROUPS) != null);
if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(realm)) {
userModels = userModels.filter(usersEvaluator::canView);
usersEvaluator.grantIfNoPermission(session.getAttribute(UserModel.GROUPS) != null);
}
return userModels.map(user -> {

View File

@@ -59,8 +59,8 @@ public class PartialEvaluator {
KeycloakContext context = session.getContext();
UserModel adminUser = context.getUser();
if (!hasAnyQueryAdminRole(session, adminUser, realm)) {
// only run partial evaluation if the admin user has any query-* role
if (skipPartialEvaluation(session, adminUser, realm, resourceType)) {
// only run partial evaluation if the admin user does not have view-* or manage-* role for specified resourceType or has any query-* role
return List.of();
}
@@ -173,7 +173,7 @@ public class PartialEvaluator {
.toList();
}
private boolean hasAnyQueryAdminRole(KeycloakSession session, UserModel user, RealmModel realm) {
private boolean skipPartialEvaluation(KeycloakSession session, UserModel user, RealmModel realm, ResourceType resourceType) {
if (user == null) {
return false;
}
@@ -192,7 +192,17 @@ public class PartialEvaluator {
return false;
}
for (String adminRole : AdminRoles.ALL_QUERY_ROLES) {
if (resourceType.equals(AdminPermissionsSchema.USERS) || resourceType.equals(AdminPermissionsSchema.GROUPS)) {
return user.hasRole(client.getRole(AdminRoles.VIEW_USERS)) || user.hasRole(client.getRole(AdminRoles.MANAGE_USERS)) || !hasAnyQueryAdminRole(client, user);
} else if (resourceType.equals(AdminPermissionsSchema.CLIENTS)) {
return user.hasRole(client.getRole(AdminRoles.VIEW_CLIENTS)) || user.hasRole(client.getRole(AdminRoles.MANAGE_CLIENTS)) || !hasAnyQueryAdminRole(client, user);
} else {
return false;
}
}
private boolean hasAnyQueryAdminRole(ClientModel client, UserModel user) {
for (String adminRole : List.of(AdminRoles.QUERY_CLIENTS, AdminRoles.QUERY_GROUPS, AdminRoles.QUERY_USERS)) {
RoleModel role = client.getRole(adminRole);
if (user.hasRole(role)) {

View File

@@ -478,10 +478,9 @@ public class UsersResource {
private Stream<UserRepresentation> toRepresentation(RealmModel realm, UserPermissionEvaluator usersEvaluator, Boolean briefRepresentation, Stream<UserModel> userModels) {
boolean briefRepresentationB = briefRepresentation != null && briefRepresentation;
usersEvaluator.grantIfNoPermission(session.getAttribute(UserModel.GROUPS) != null);
if (!AdminPermissionsSchema.SCHEMA.isAdminPermissionsEnabled(realm)) {
userModels = userModels.filter(usersEvaluator::canView);
usersEvaluator.grantIfNoPermission(session.getAttribute(UserModel.GROUPS) != null);
}
return userModels

View File

@@ -17,6 +17,9 @@
package org.keycloak.tests.admin.authz.fgap;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -28,6 +31,7 @@ import java.util.List;
import java.util.Set;
import jakarta.ws.rs.core.Response;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -35,6 +39,8 @@ import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.RolePoliciesResource;
import org.keycloak.admin.client.resource.ScopePermissionsResource;
import org.keycloak.authorization.AdminPermissionsSchema;
import org.keycloak.models.AdminRoles;
import org.keycloak.models.Constants;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.idm.GroupRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
@@ -222,4 +228,18 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest {
assertFalse(search.isEmpty());
assertTrue(search.stream().map(UserRepresentation::getUsername).noneMatch("user-0"::equals));
}
@Test
public void testListingUsersWithRolesOnly() {
List<UserRepresentation> search = realm.admin().users().search("myadmin");
assertThat(search, Matchers.hasSize(1));
String userId = search.get(0).getId();
String clientUuid = realm.admin().clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId();
RoleRepresentation viewUsers = realm.admin().clients().get(clientUuid).roles().get(AdminRoles.VIEW_USERS).toRepresentation();
realm.admin().users().get(userId).roles().clientLevel(clientUuid).add(List.of(viewUsers));
realm.cleanup().add(r -> r.users().get(userId).roles().clientLevel(clientUuid).remove(List.of(viewUsers)));
assertThat(realmAdminClient.realm(realm.getName()).users().list(), not(empty()));
}
}