ai-code-review-evaluations · akshayutture-augment · Feb 4, 2025
diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/BruteForceUsersResource.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/BruteForceUsersResource.java
@@ -144,16 +144,12 @@ public final Stream<BruteUser> searchUser(@QueryParam("search") String search,
     private Stream<BruteUser> searchForUser(Map<String, String> attributes, RealmModel realm, UserPermissionEvaluator usersEvaluator, Boolean briefRepresentation, Integer firstResult, Integer maxResults, Boolean includeServiceAccounts) {
         attributes.put(UserModel.INCLUDE_SERVICE_ACCOUNT, includeServiceAccounts.toString());
 
-        if (!auth.users().canView()) {
-            Set<String> groupModels = auth.groups().getGroupsWithViewPermission();
-
-            if (!groupModels.isEmpty()) {
-                session.setAttribute(UserModel.GROUPS, groupModels);
-            }
+        Set<String> groupIds = auth.groups().getGroupIdsWithViewPermission();
+        if (!groupIds.isEmpty()) {
+            session.setAttribute(UserModel.GROUPS, groupIds);
         }
 
-        Stream<UserModel> userModels = session.users().searchForUserStream(realm, attributes, firstResult, maxResults);
-        return toRepresentation(realm, usersEvaluator, briefRepresentation, userModels);
+        return toRepresentation(realm, usersEvaluator, briefRepresentation, session.users().searchForUserStream(realm, attributes, firstResult, maxResults));
     }
 
     private Stream<BruteUser> toRepresentation(RealmModel realm, UserPermissionEvaluator usersEvaluator,

diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java b/server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java
@@ -33,6 +33,7 @@
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientProvider;
 import org.keycloak.models.Constants;
+import org.keycloak.models.GroupModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.ModelIllegalStateException;
@@ -50,27 +51,42 @@
 import org.keycloak.representations.idm.authorization.ScopeRepresentation;
 
 public class AdminPermissionsSchema extends AuthorizationSchema {
-
-    public static final String USERS_RESOURCE_TYPE = "Users";
+
     public static final String CLIENTS_RESOURCE_TYPE = "Clients";
+    public static final String GROUPS_RESOURCE_TYPE = "Groups";
+    public static final String USERS_RESOURCE_TYPE = "Users";
 
-    //scopes
+    // common scopes
     public static final String MANAGE = "manage";
     public static final String VIEW = "view";
-    public static final String IMPERSONATE = "impersonate";
-    public static final String MAP_ROLES = "map-roles";
-    public static final String MANAGE_GROUP_MEMBERSHIP = "manage-group-membership";
+
+    // client specific scopes
     public static final String CONFIGURE = "configure";
     public static final String MAP_ROLES_CLIENT_SCOPE = "map-roles-client-scope";
     public static final String MAP_ROLES_COMPOSITE = "map-roles-composite";
 
-    public static final ResourceType USERS = new ResourceType(USERS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, IMPERSONATE, MAP_ROLES, MANAGE_GROUP_MEMBERSHIP));
+    // group specific scopes
+    public static final String MANAGE_MEMBERSHIP = "manage-membership";
+    public static final String MANAGE_MEMBERS = "manage-members";
+    public static final String VIEW_MEMBERS = "view-members";
+
+    // user specific scopes
+    public static final String IMPERSONATE = "impersonate";
+    public static final String MAP_ROLES = "map-roles";
+    public static final String MANAGE_GROUP_MEMBERSHIP = "manage-group-membership";
+
     public static final ResourceType CLIENTS = new ResourceType(CLIENTS_RESOURCE_TYPE, Set.of(CONFIGURE, MANAGE, MAP_ROLES, MAP_ROLES_CLIENT_SCOPE, MAP_ROLES_COMPOSITE, VIEW));
+    public static final ResourceType GROUPS = new ResourceType(GROUPS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, MANAGE_MEMBERSHIP, MANAGE_MEMBERS, VIEW_MEMBERS));
+    public static final ResourceType USERS = new ResourceType(USERS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, IMPERSONATE, MAP_ROLES, MANAGE_GROUP_MEMBERSHIP));
 
     public static final AdminPermissionsSchema SCHEMA = new AdminPermissionsSchema();
 
     private AdminPermissionsSchema() {
-        super(Map.of(USERS_RESOURCE_TYPE, USERS, CLIENTS_RESOURCE_TYPE, CLIENTS));
+        super(Map.of(
+            CLIENTS_RESOURCE_TYPE, CLIENTS,
+            GROUPS_RESOURCE_TYPE, GROUPS,
+            USERS_RESOURCE_TYPE, USERS
+        ));
     }
 
     public Resource getOrCreateResource(KeycloakSession session, ResourceServer resourceServer, String policyType, String resourceType, String id) {
@@ -86,12 +102,13 @@ public Resource getOrCreateResource(KeycloakSession session, ResourceServer reso
             return resource;
         }
 
-        String name = null;
+        String name;
 
-        if (USERS.getType().equals(resourceType)) {
-            name = resolveUser(session, id);
-        } else if (CLIENTS.getType().equals(resourceType)) {
-            name = resolveClient(session, id);
+        switch (resourceType) {
+            case CLIENTS_RESOURCE_TYPE -> name = resolveClient(session, id);
+            case GROUPS_RESOURCE_TYPE -> name = resolveGroup(session, id);
+            case USERS_RESOURCE_TYPE -> name = resolveUser(session, id);
+            default -> throw new IllegalStateException("Resource type [" + resourceType + "] not found.");
         }
 
         if (name == null) {
@@ -161,6 +178,13 @@ public void throwExceptionIfAdminPermissionClient(KeycloakSession session, Strin
         }
     }
 
+    private String resolveGroup(KeycloakSession session, String id) {
+        RealmModel realm = session.getContext().getRealm();
+        GroupModel group = session.groups().getGroupById(realm, id);
+
+        return group == null ? null : group.getId();
+    }
+
     private String resolveUser(KeycloakSession session, String id) {
         RealmModel realm = session.getContext().getRealm();
         UserModel user = session.users().getUserById(realm, id);

diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -173,10 +173,6 @@ public static GroupRepresentation toRepresentation(GroupModel group, boolean ful
         return rep;
     }
 
-    public static Stream<GroupModel> searchGroupModelsByAttributes(KeycloakSession session, RealmModel realm, Map<String,String> attributes, Integer first, Integer max) {
-        return session.groups().searchGroupsByAttributes(realm, attributes, first, max);
-    }
-
     @Deprecated
     public static Stream<GroupRepresentation> toGroupHierarchy(KeycloakSession session, RealmModel realm, boolean full) {
         return session.groups().getTopLevelGroupsStream(realm, null, null)

diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java
@@ -174,10 +174,9 @@ public Stream<GroupRepresentation> getSubGroups(
             @Parameter(description = "The maximum number of results that are to be returned. Defaults to 10") @QueryParam("max") @DefaultValue("10") Integer max,
             @Parameter(description = "Boolean which defines whether brief groups representations are returned or not (default: false)") @QueryParam("briefRepresentation") @DefaultValue("false") Boolean briefRepresentation) {
         this.auth.groups().requireView(group);
-        boolean canViewGlobal = auth.groups().canView();
         return paginatedStream(
             group.getSubGroupsStream(search, exact, -1, -1)
-            .filter(g -> canViewGlobal || auth.groups().canView(g)), first, max)
+            .filter(auth.groups()::canView), first, max)
             .map(g -> GroupUtils.populateSubGroupCount(g, GroupUtils.toRepresentation(auth.groups(), g, !briefRepresentation)));
     }
 
@@ -204,7 +203,7 @@ public Response addChild(GroupRepresentation rep) {
 
         try {
             Response.ResponseBuilder builder = Response.status(204);
-            GroupModel child = null;
+            GroupModel child;
             if (rep.getId() != null) {
                 child = realm.getGroupById(rep.getId());
                 if (child == null) {

diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java
@@ -47,7 +47,6 @@
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.RealmModel;
-import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.organization.utils.Organizations;
 import org.keycloak.representations.idm.GroupRepresentation;
 import org.keycloak.services.ErrorResponse;
@@ -100,7 +99,7 @@ public Stream<GroupRepresentation> getGroups(@QueryParam("search") String search
         Stream<GroupModel> stream;
         if (Objects.nonNull(searchQuery)) {
             Map<String, String> attributes = SearchQueryUtils.getFields(searchQuery);
-            stream = ModelToRepresentation.searchGroupModelsByAttributes(session, realm, attributes, firstResult, maxResults);
+            stream = session.groups().searchGroupsByAttributes(realm, attributes, firstResult, maxResults);
         } else if (Objects.nonNull(search)) {
             stream = session.groups().searchForGroupByNameStream(realm, search.trim(), exact, firstResult, maxResults);
         } else {
@@ -110,9 +109,8 @@ public Stream<GroupRepresentation> getGroups(@QueryParam("search") String search
         if (populateHierarchy) {
             return GroupUtils.populateGroupHierarchyFromSubGroups(session, realm, stream, !briefRepresentation, groupsEvaluator);
         }
-        boolean canViewGlobal = groupsEvaluator.canView();
-        return stream
-            .filter(g -> canViewGlobal || groupsEvaluator.canView(g))
+
+        return stream.filter(groupsEvaluator::canView)
             .map(g -> GroupUtils.populateSubGroupCount(g, GroupUtils.toRepresentation(groupsEvaluator, g, !briefRepresentation)));
     }
 

diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
@@ -71,7 +71,6 @@
 import java.util.Optional;
 import java.util.Properties;
 import java.util.Set;
-import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -396,7 +395,7 @@ public Integer getUsersCount(
             } else if (userPermissionEvaluator.canView()) {
                 return session.users().getUsersCount(realm, search.trim());
             } else {
-                return session.users().getUsersCount(realm, search.trim(), auth.groups().getGroupsWithViewPermission());
+                return session.users().getUsersCount(realm, search.trim(), auth.groups().getGroupIdsWithViewPermission());
             }
         } else if (last != null || first != null || email != null || username != null || emailVerified != null || enabled != null || !searchAttributes.isEmpty()) {
             Map<String, String> parameters = new HashMap<>();
@@ -423,12 +422,12 @@ public Integer getUsersCount(
             if (userPermissionEvaluator.canView()) {
                 return session.users().getUsersCount(realm, parameters);
             } else {
-                return session.users().getUsersCount(realm, parameters, auth.groups().getGroupsWithViewPermission());
+                return session.users().getUsersCount(realm, parameters, auth.groups().getGroupIdsWithViewPermission());
             }
         } else if (userPermissionEvaluator.canView()) {
             return session.users().getUsersCount(realm);
         } else {
-            return session.users().getUsersCount(realm, auth.groups().getGroupsWithViewPermission());
+            return session.users().getUsersCount(realm, auth.groups().getGroupIdsWithViewPermission());
         }
     }
 
@@ -446,16 +445,12 @@ public UserProfileResource userProfile() {
     private Stream<UserRepresentation> searchForUser(Map<String, String> attributes, RealmModel realm, UserPermissionEvaluator usersEvaluator, Boolean briefRepresentation, Integer firstResult, Integer maxResults, Boolean includeServiceAccounts) {
         attributes.put(UserModel.INCLUDE_SERVICE_ACCOUNT, includeServiceAccounts.toString());
 
-        if (!auth.users().canView()) {
-            Set<String> groupModels = auth.groups().getGroupsWithViewPermission();
-
-            if (!groupModels.isEmpty()) {
-                session.setAttribute(UserModel.GROUPS, groupModels);
-            }
+        Set<String> groupIds = auth.groups().getGroupIdsWithViewPermission();
+        if (!groupIds.isEmpty()) {
+            session.setAttribute(UserModel.GROUPS, groupIds);
         }
 
-        Stream<UserModel> userModels = session.users().searchForUserStream(realm, attributes, firstResult, maxResults).filter(usersEvaluator::canView);
-        return toRepresentation(realm, usersEvaluator, briefRepresentation, userModels);
+        return toRepresentation(realm, usersEvaluator, briefRepresentation, session.users().searchForUserStream(realm, attributes, firstResult, maxResults));
     }
 
     private Stream<UserRepresentation> toRepresentation(RealmModel realm, UserPermissionEvaluator usersEvaluator, Boolean briefRepresentation, Stream<UserModel> userModels) {

diff --git a/...ces/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java b/...ces/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java
@@ -71,10 +71,10 @@ public static AdminPermissionManagement management(KeycloakSession session, Real
     }
 
     public static void registerListener(ProviderEventManager manager) {
-        manager.register(new ProviderEventListener() {
-            @Override
-            public void onEvent(ProviderEvent event) {
-                if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {
+        if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {
+            manager.register(new ProviderEventListener() {
+                @Override
+                public void onEvent(ProviderEvent event) {
                     if (event instanceof RoleContainerModel.RoleRemovedEvent) {
                         RoleContainerModel.RoleRemovedEvent cast = (RoleContainerModel.RoleRemovedEvent) event;
                         RoleModel role = cast.getRole();
@@ -94,8 +94,8 @@ public void onEvent(ProviderEvent event) {
                         management(cast.getKeycloakSession(), cast.getRealm()).groups().setPermissionsEnabled(cast.getGroup(), false);
                     }
                 }
-            }
-        });
+            });
+        }
     }
 
 

diff --git a/...main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java b/...main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java
@@ -16,6 +16,8 @@
  */
 package org.keycloak.services.resources.admin.permissions;
 
+import org.keycloak.authorization.AdminPermissionsSchema;
+import org.keycloak.models.AdminRoles;
 import org.keycloak.models.GroupModel;
 
 import java.util.Map;
@@ -26,41 +28,121 @@
  * @version $Revision: 1 $
  */
 public interface GroupPermissionEvaluator {
+
+    /**
+     * Returns {@code true} if the caller has at least one of {@link AdminRoles#QUERY_GROUPS},
+     * {@link AdminRoles#MANAGE_USERS} or {@link AdminRoles#VIEW_USERS} roles.
+     * <p/>
+     * For V2 only: Also if it has a permission to {@link AdminPermissionsSchema#VIEW} or
+     * {@link AdminPermissionsSchema#MANAGE} groups.
+     */
     boolean canList();
 
+    /**
+     * Throws ForbiddenException if {@link #canList()} returns {@code false}.
+     */
     void requireList();
 
+    /**
+     * Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#MANAGE} the group.
+     */
     boolean canManage(GroupModel group);
 
+    /**
+     * Throws ForbiddenException if {@link #canManage(GroupModel)} returns {@code false}.
+     */
     void requireManage(GroupModel group);
 
+    /**
+     * Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or
+     * {@link AdminRoles#VIEW_USERS} roles.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
+     * {@link AdminPermissionsSchema#MANAGE} the group.
+     */
     boolean canView(GroupModel group);
 
+    /**
+     * Throws ForbiddenException if {@link #canView(GroupModel)} returns {@code false}.
+     */
     void requireView(GroupModel group);
 
+    /**
+     * Returns {@code true} if the caller has {@link AdminRoles#MANAGE_USERS} role.
+     * <p/>
+     * For V2 only: Also if it has permission to {@link AdminPermissionsSchema#VIEW} or
+     * {@link AdminPermissionsSchema#MANAGE} groups.
+     */
     boolean canManage();
 
+    /**
+     * Throws ForbiddenException if {@link #canManage()} returns {@code false}.
+     */
     void requireManage();
 
+    /**
+     * Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} or 
+     * {@link AdminRoles#VIEW_USERS} roles.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#VIEW} or
+     * {@link AdminPermissionsSchema#MANAGE} groups.
+     */
     boolean canView();
 
+    /**
+     * Throws ForbiddenException if {@link #canView()} returns {@code false}.
+     */
     void requireView();
 
-    boolean getGroupsWithViewPermission(GroupModel group);
-
+    /**
+     * Throws ForbiddenException if {@link #canViewMembers(GroupModel)} returns {@code false}.
+     */
     void requireViewMembers(GroupModel group);
 
+    /**
+     * Returns {@code true} if {@link UserPermissionEvaluator#canManage()} evaluates to {@code true}.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#MANAGE_MEMBERS} of the group.
+     */
     boolean canManageMembers(GroupModel group);
 
+    /**
+     * Returns {@code true} if the caller has one of {@link AdminRoles#MANAGE_USERS} role.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#MANAGE} the group or
+     * {@link AdminPermissionsSchema#MANAGE_MEMBERSHIP} of the group.
+     */
     boolean canManageMembership(GroupModel group);
-
+
+    /**
+     * Returns {@code true} if {@link UserPermissionEvaluator#canView()} evaluates to {@code true}.
+     * <p/>
+     * Or if it has a permission to {@link AdminPermissionsSchema#VIEW_MEMBERS} or
+     * {@link AdminPermissionsSchema#MANAGE_MEMBERS} of the group.
+     */
     boolean canViewMembers(GroupModel group);
-
+
+    /**
+     * Throws ForbiddenException if {@link #canManageMembership(GroupModel)} returns {@code false}.
+     */
     void requireManageMembership(GroupModel group);
 
+    /**
+     * Throws ForbiddenException if {@link #canManageMembership(GroupModel)} returns {@code false}.
+     */
     void requireManageMembers(GroupModel group);
 
+    /**
+     * Returns Map with information what access the caller for the provided group has.
+     */
     Map<String, Boolean> getAccess(GroupModel group);
 
-    Set<String> getGroupsWithViewPermission();
+    /**
+     * If {@link UserPermissionEvaluator#canView()} evaluates to {@code true}, returns empty set.
+     * 
+     * @return Stream of IDs of groups with view permission.
+     */
+    Set<String> getGroupIdsWithViewPermission();
 }