You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
tkyjovsk
changed the title
Add a IdP config option to control IdP admin role escalation
Add an IdP config option to control IdP admin role escalation
Sep 10, 2026
If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.
org.opentest4j.AssertionFailedError: cache entry should have expired ==> expected: <null> but was: <org.keycloak.loginfailures.jpa.UserLoginFailureAdapter@c27c372>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertNull.failNotNull(AssertNull.java:50)
at org.junit.jupiter.api.AssertNull.assertNull(AssertNull.java:35)
...
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical and moderate findings affect default security behavior, composite and group mapper enforcement, mapper consistency, and null-configuration handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a per-IdP allowAdminRoleMapping option to control administrative role escalation through identity-provider mappers.
Changes:
Adds model, API, UI, and translation support.
Enforces mapper restrictions during administration and broker login.
Validates IdP and mapper updates. Critical (1 vote): group mappers can bypass equivalent admin-role validation. Moderate (1 vote): realm-admin-created mappers can be persisted while runtime enforcement rejects them. Moderate (1 vote): null mapper configuration can cause a NullPointerException.
AdminRoles.isAdminRole only recognizes a role whose own name is in ALL_ROLES; it does not recognize a custom composite containing, for example, realm-management.manage-users. With the flag disabled, a non-realm-admin can therefore create or update a mapper for that composite and receive the admin child transitively. Use the composite-aware AdminRoles.isAdminRoleOrComposite predicate here.
RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName);
if (role == null || !AdminRoles.isAdminRole(role)) {
return;
This early return lets a realm administrator create an admin-role mapper while the setting is disabled, but isAdminRoleGrantAllowed later rejects that mapper based only on the IdP flag. The API can therefore report a successful create/update for a mapper that never grants its configured role during broker login; either require the setting for realm-admin-created mappers too or persist an explicit trusted exemption that the runtime check honors.
if (auth.hasOneAdminRole(AdminRoles.MANAGE_REALM)) {
return;
IdentityProviderMapperRepresentation permits a null config and RepresentationToModel.removeEmptyString preserves it as null. Because every add/update mapper request now reaches this method, .get can throw a NullPointerException and return 500 for such a request; guard the null config before reading roleName.
This direct-role check misses composite roles that contain an admin role. Because user role checks resolve composites, a mapper for a custom composite containing a realm-management role can still grant admin privileges while this setting is disabled; use the composite-aware AdminRoles.isAdminRoleOrComposite predicate here and in the admin endpoint validation.
if (role == null || !AdminRoles.isAdminRole(role)) {
This validation only inspects config.role. A user with manage-identity-providers can instead add a hardcoded or claim-to-group mapper that joins users to a group carrying a built-in admin role; DefaultPermissions.isAdminUser resolves group-inherited admin roles, so this still escalates while the setting is disabled. Extend the protection to admin-bearing target groups (including inherited/composite roles) and enforce it when the group mapper is applied.
IdentityProviderMapperModel model = RepresentationToModel.toModel(mapper);
validateMapperAdminRoleMapping(model);
When a realm administrator creates an IdP without this key, this branch is skipped and isAllowAdminRoleMapping() defaults to true, so the new IdP is enabled by default through the REST API despite the new help text saying it is disabled by default. Track whether the key was supplied and persist false for omitted values while retaining explicit values.
if (!auth.hasOneAdminRole(AdminRoles.MANAGE_REALM)) {
if (identityProvider.getConfig().containsKey(IdentityProviderModel.ALLOW_ADMIN_ROLE_MAPPING)
&& identityProvider.isAllowAdminRoleMapping()) {
throw ErrorResponse.error("Only users with '" + AdminRoles.MANAGE_REALM
+ "' role can enable the '" + IdentityProviderModel.ALLOW_ADMIN_ROLE_MAPPING + "' setting.",
Response.Status.FORBIDDEN);
}
identityProvider.setAllowAdminRoleMapping(false);
}
Files reviewed: 11/11 changed files
Comments generated: 1
Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
This validator only inspects the role property, so group mappers are always accepted. A manager can map users into a group (or child group) carrying an admin role and bypass the new switch; validate ConfigConstants.GROUP with AdminRoles.groupHasAdminRoles and enforce the same rule in the OIDC/SAML group mapper runtime paths.
A custom composite role containing realm-management.realm-admin is not itself recognized by isAdminRole, so it can be submitted while this switch is disabled and still confer its admin subroles. Use the composite-aware helper already provided by AdminRoles.
RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName);
if (role == null || !AdminRoles.isAdminRole(role)) {
return;
The runtime guard also treats a composite containing admin roles as non-admin, so pre-existing/imported mappers can still grant such a composite while the switch is disabled. Use the composite-aware check here as well.
The mapper fetched using the path ID is discarded, while validation uses the path IdP and persistence uses the body-controlled ID and alias. This permits updating a mapper belonging to a disabled IdP through an enabled IdP's URL; verify ownership and make the path ID/alias authoritative before validation.
IdentityProviderMapperModel model = session.identityProviders().getMapperById(id);
if (model == null) throw new NotFoundException("Model not found");
model = RepresentationToModel.toModel(rep);
validateMapperAdminRoleMapping(model);
Only an administrator with the `manage-realm` permission can enable the setting.
After upgrading, if you rely on identity provider mappers to assign administrative roles or to add brokered users to groups that carry administrative roles, an administrator with the `manage-realm` permission must enable *Allow granting admin roles via mappers* on each affected identity provider.
Otherwise, those mappings stop taking effect and the affected users no longer receive the administrative roles on their next login.
The new broker-login test only exercises direct OIDC hardcoded-role mapping, so this security-sensitive group gate is not executed. Add a login test with an existing hardcoded admin-group mapper proving that disabled blocks the join and enabled permits it.
No added broker-login test enters this SAML runtime guard; the new test uses the separate OIDC hardcoded mapper. Add SAML broker coverage for an admin-role mapper with the setting disabled and enabled so this CVE fix cannot regress only on SAML providers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #50444.