Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,27 @@
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";

//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";
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));
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 AdminPermissionsSchema SCHEMA = new AdminPermissionsSchema();

private AdminPermissionsSchema() {
super(Map.of(USERS_RESOURCE_TYPE, USERS));
super(Map.of(USERS_RESOURCE_TYPE, USERS, CLIENTS_RESOURCE_TYPE, CLIENTS));
}

public Resource getOrCreateResource(KeycloakSession session, ResourceServer resourceServer, String policyType, String resourceType, String id) {
Expand All @@ -85,6 +90,8 @@ public Resource getOrCreateResource(KeycloakSession session, ResourceServer reso

if (USERS.getType().equals(resourceType)) {
name = resolveUser(session, id);
} else if (CLIENTS.getType().equals(resourceType)) {
name = resolveClient(session, id);
}

if (name == null) {
Expand Down Expand Up @@ -165,6 +172,17 @@ private String resolveUser(KeycloakSession session, String id) {
return user == null ? null : user.getId();
}

private String resolveClient(KeycloakSession session, String id) {
RealmModel realm = session.getContext().getRealm();
ClientModel client = session.clients().getClientById(realm, id);

if (client == null) {
client = session.clients().getClientByClientId(realm, id);
}

return client == null ? null : client.getId();
}

private StoreFactory getStoreFactory(KeycloakSession session) {
AuthorizationProvider authzProvider = session.getProvider(AuthorizationProvider.class);
return authzProvider.getStoreFactory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,25 @@ public static void registerListener(ProviderEventManager manager) {
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();
RealmModel realm;
if (role.getContainer() instanceof ClientModel) {
realm = ((ClientModel)role.getContainer()).getRealm();
if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The provider event listener that disables permissions on role, client, or group removal is now gated only by the legacy ADMIN_FINE_GRAINED_AUTHZ feature flag, so when only the new ADMIN_FINE_GRAINED_AUTHZ_V2 flag is enabled the cleanup never runs, leaving stale permissions and roles enabled in V2 deployments. [logic error]

Severity Level: Critical 🚨
- ❌ V2-only fine-grained admin realms leave orphan admin permissions.
- ❌ Deleting groups/clients/roles under v2 never disables their permissions.
- ⚠️ Authorization schema (`AdminPermissionsSchema`) accumulates stale resources.
- ⚠️ ManagementPermissions behavior diverges between v1 and v2 configurations.
Suggested change
if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {
// Run cleanup when either fine-grained admin permissions v1 or v2 is enabled
if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)
|| Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ_V2)) {
Steps of Reproduction ✅
1. Start a Keycloak server configured to enable only the v2 fine-grained admin permissions
feature, for example using the test server config in
`tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/KeycloakAdminPermissionsServerConfig.java:24-29`,
where `configure()` returns `config.features(Feature.ADMIN_FINE_GRAINED_AUTHZ_V2)` and
does NOT enable `ADMIN_FINE_GRAINED_AUTHZ` v1.

2. On server startup, the session factory (e.g.
`quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/QuarkusKeycloakSessionFactory.java:88`
and `services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java:122`)
calls `AdminPermissions.registerListener(this)`, registering the listener implemented in
`services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java:73-99`,
which currently guards all cleanup logic with `if
(Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ))`.

3. In a realm where admin permissions are enabled under v2 (created as in
`RealmManager.importRealm()` at
`services/src/main/java/org/keycloak/services/managers/RealmManager.java:566-573`, which
calls `AdminPermissionsSchema.SCHEMA.init(session, realm)` when
`ADMIN_FINE_GRAINED_AUTHZ_V2` is enabled and `rep.isAdminPermissionsEnabled()` is true),
use the admin REST API as in
`tests/base/src/test/java/org/keycloak/tests/admin/ManagementPermissionsTest.java` to: (a)
create a group/client/role, then (b) turn on management permissions via
`setPermissions(new ManagementPermissionRepresentation(true))`, which internally uses
`AdminPermissions.management(...)` (see `AdminPermissions.management()` at
`services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java:66-70`
and `MgmtPermissionsV2` at
`services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissionsV2.java:31-48`)
to create the necessary authorization resources and mark them enabled.

4. Still in the same v2-only configuration, delete the group/client/role you just enabled
permissions for (e.g. using the same admin client patterns as in
`ManagementPermissionsTest` for `GroupResource`, `ClientResource`, or `RoleResource`);
this triggers a `GroupModel.GroupRemovedEvent`, `ClientModel.ClientRemovedEvent`, or
`RoleContainerModel.RoleRemovedEvent` that is dispatched through the
`ProviderEventManager` to the registered listener at `AdminPermissions.java:73-99`, but
because only `ADMIN_FINE_GRAINED_AUTHZ_V2` is enabled, the condition
`Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)` evaluates to false,
so none of the cleanup calls `management(...).roles().setPermissionsEnabled(...)`,
`management(...).clients().setPermissionsEnabled(...)`, or
`management(...).groups().setPermissionsEnabled(...)` are executed—leaving the
corresponding admin-permission authorization resources created by
`MgmtPermissions`/`MgmtPermissionsV2` enabled and orphaned in the realm.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java
**Line:** 77:77
**Comment:**
	*Logic Error: The provider event listener that disables permissions on role, client, or group removal is now gated only by the legacy ADMIN_FINE_GRAINED_AUTHZ feature flag, so when only the new ADMIN_FINE_GRAINED_AUTHZ_V2 flag is enabled the cleanup never runs, leaving stale permissions and roles enabled in V2 deployments.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

if (event instanceof RoleContainerModel.RoleRemovedEvent) {
RoleContainerModel.RoleRemovedEvent cast = (RoleContainerModel.RoleRemovedEvent) event;
RoleModel role = cast.getRole();
RealmModel realm;
if (role.getContainer() instanceof ClientModel) {
realm = ((ClientModel) role.getContainer()).getRealm();

} else {
realm = (RealmModel)role.getContainer();
} else {
realm = (RealmModel) role.getContainer();
}
management(cast.getKeycloakSession(), realm).roles().setPermissionsEnabled(role, false);
} else if (event instanceof ClientModel.ClientRemovedEvent) {
ClientModel.ClientRemovedEvent cast = (ClientModel.ClientRemovedEvent) event;
management(cast.getKeycloakSession(), cast.getClient().getRealm()).clients().setPermissionsEnabled(cast.getClient(), false);
} else if (event instanceof GroupModel.GroupRemovedEvent) {
GroupModel.GroupRemovedEvent cast = (GroupModel.GroupRemovedEvent) event;
management(cast.getKeycloakSession(), cast.getRealm()).groups().setPermissionsEnabled(cast.getGroup(), false);
}
management(cast.getKeycloakSession(), realm).roles().setPermissionsEnabled(role, false);
} else if (event instanceof ClientModel.ClientRemovedEvent) {
ClientModel.ClientRemovedEvent cast = (ClientModel.ClientRemovedEvent)event;
management(cast.getKeycloakSession(), cast.getClient().getRealm()).clients().setPermissionsEnabled(cast.getClient(), false);
} else if (event instanceof GroupModel.GroupRemovedEvent) {
GroupModel.GroupRemovedEvent cast = (GroupModel.GroupRemovedEvent)event;
management(cast.getKeycloakSession(), cast.getRealm()).groups().setPermissionsEnabled(cast.getGroup(), false);
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.keycloak.services.resources.admin.permissions;

import org.keycloak.models.AdminRoles;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;

Expand All @@ -31,54 +32,161 @@ public interface ClientPermissionEvaluator {

void setPermissionsEnabled(ClientModel client, boolean enable);

/**
* Throws ForbiddenException if {@link #canListClientScopes()} returns {@code false}.
*/
void requireListClientScopes();

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManage();

/**
* Throws ForbiddenException if {@link #canManage()} returns {@code false}.
*/
void requireManage();

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManageClientScopes();

/**
* Throws ForbiddenException if {@link #canManageClientScopes()} returns {@code false}.
*/
void requireManageClientScopes();

/**
* Returns {@code true} if the caller has at least one of the {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} or {@link org.keycloak.models.AdminRoles#VIEW_CLIENTS} roles.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW}.
*/
boolean canView();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
* <p/>
* Or if the caller has at least one of the {@link AdminRoles#QUERY_CLIENTS} or {@link AdminRoles#QUERY_USERS} roles.
*/
boolean canList();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
*/
boolean canViewClientScopes();

/**
* Throws ForbiddenException if {@link #canList()} returns {@code false}.
*/
void requireList();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
* <p/>
* Or if the caller has {@link AdminRoles#QUERY_CLIENTS} role.
*/
boolean canListClientScopes();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
*/
void requireView();

/**
* Returns {@code true} if {@link #canViewClientScopes()} returns {@code true}.
*/
void requireViewClientScopes();

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* Or if the caller has a permission to {@link AdminPermissionManagement#MANAGE_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE} all clients.
*/
boolean canManage(ClientModel client);

/**
* Returns {@code true} if {@link #canManage(ClientModel)} returns {@code true}.
* <p/>
* Or if the caller has a permission to {@link ClientPermissionManagement#CONFIGURE_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#CONFIGURE} all clients.
*/
boolean canConfigure(ClientModel client);

/**
* Throws ForbiddenException if {@link #canConfigure(ClientModel)} returns {@code false}.
*/
void requireConfigure(ClientModel client);

/**
* Throws ForbiddenException if {@link #canManage(ClientModel)} returns {@code false}.
*/
void requireManage(ClientModel client);

/**
* Returns {@code true} if {@link #canView()} or {@link #canConfigure(ClientModel)} returns {@code true}.
* <p/>
* Or if the caller has a permission to {@link AdminPermissionManagement#VIEW_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW} all clients.
*/
boolean canView(ClientModel client);

/**
* Throws ForbiddenException if {@link #canView(ClientModel)} returns {@code false}.
*/
void requireView(ClientModel client);

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManage(ClientScopeModel clientScope);

/**
* Throws ForbiddenException if {@link #canManage(ClientScopeModel)} returns {@code false}.
*/
void requireManage(ClientScopeModel clientScope);

/**
* Returns {@code true} if the caller has at least one of the {@link org.keycloak.models.AdminRoles#VIEW_CLIENTS} or {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} roles.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW} or {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canView(ClientScopeModel clientScope);

/**
* Throws ForbiddenException if {@link #canView(ClientScopeModel)} returns {@code false}.
*/
void requireView(ClientScopeModel clientScope);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES} for all clients.
*/
boolean canMapRoles(ClientModel client);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_COMPOSITE_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES_COMPOSITE} for all clients.
*/
boolean canMapCompositeRoles(ClientModel client);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_CLIENT_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES_CLIENT_SCOPE} for all clients.
*/
boolean canMapClientScopeRoles(ClientModel client);

Map<String, Boolean> getAccess(ClientModel client);
Expand Down
Loading