Repository navigation
Restrict defaultRoles and full scope via client registration policies - #54887
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Migration-only manifests leave compatibility metadata unchanged, preventing required recreate-upgrade enforcement, while missing baseline classes cause the test-phase compatibility verifier to fail.
2 open issues
What changed in this PR
Ports the CVE-2026-109081 fix to main, restricting Dynamic Client Registration from changing realm default roles or enabling full scope unless an administrator removes the corresponding policies.
Changes:
- Adds default-role protection and enables the existing full-scope restriction for authenticated registration.
- Installs policies in existing realms and declares the migration incompatible with rolling upgrades.
- Adds regression tests and documents the restrictions and administrator opt-out.
| File | Description |
|---|---|
tests/base/src/test/java/org/keycloak/tests/client/ClientRegistrationTest.java |
Tests policy enforcement and removal. |
tests/base/src/test/java/org/keycloak/tests/client/AbstractClientRegistrationTest.java |
Adds a realm-management user and authentication helper. |
services/src/test/java/org/keycloak/services/clientregistration/policy/impl/DefaultRolesClientRegistrationPolicyTest.java |
Tests rejection of nonempty and null-element default roles. |
services/src/main/resources/META-INF/services/org.keycloak.services.clientregistration.policy.ClientRegistrationPolicyFactory |
Registers the new policy factory. |
services/src/main/java/org/keycloak/services/clientregistration/policy/impl/DefaultRolesClientRegistrationPolicyFactory.java |
Defines the default-role policy provider. |
services/src/main/java/org/keycloak/services/clientregistration/policy/impl/DefaultRolesClientRegistrationPolicy.java |
Rejects registration requests that change default roles. |
services/src/main/java/org/keycloak/services/clientregistration/policy/DefaultClientRegistrationPolicies.java |
Enables the restrictions by default. |
services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java |
Skips null role names during updates. |
services/src/main/java/org/keycloak/protocol/oauth2/cimd/provider/AbstractPersistentClientIdMetadataDocumentProvider.java |
Removes default-role creation from metadata registration. |
model/storage-private/src/main/resources/META-INF/rolling-upgrades-unsupported-changes.json |
Marks the policy migration as rolling-upgrade incompatible. |
model/storage-private/src/main/resources/META-INF/rolling-upgrades-supported-changes.json |
Records the supported migration baseline. |
model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultMigrationManager.java |
Registers the new migrator. |
model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo26_8_2.java |
Installs missing policies in existing realms. |
docs/guides/securing-apps/client-registration.adoc |
Describes policy defaults and removal. |
docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc |
Documents behavior changes and cluster upgrade requirements. |
Review effort: Balanced
Replace hardcoded manage-realm/RAT checks with Default Roles Disabled and authenticated Full Scope Disabled policies so admins can opt back in. Closes CVE-2026-109081 Closes keycloak#54702 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Main requires both supported and unsupported JSON inventories for the db-compatibility verifier; mark the new policy migrator as unsupported. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Include Migration helper classes and OidcIssuerMigration in the supported baseline, and hash unsupported migration-only manifests so recreate upgrades are required. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
277a95a to
bb235de
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The security-sensitive upgrade path requires runtime validation that existing realms receive the policies and packaged Quarkus discovers the migration-only manifest to reject rolling upgrades.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
Drop SPI helper classes the Maven plugin does not discover so the supported/unsupported inventories match the module classpath. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The security backport changes persisted realm policies and rolling-upgrade compatibility, requiring runtime validation of affected-release upgrades and mixed-version cluster rejection that is unavailable in this review.
No open issues 🎉
Review effort: Balanced
There was a problem hiding this comment.
I am not sure why this PR needs to do any updates of the file rolling-upgrades-suported-changes.json considering that the original PR does not have anything like that. Also not sure about introducing the file rolling-upgrades-unsupported-changes.json .
Looks like the files were removed from main on purpose and they are not needed anymore?
@ahus1 Could you please confirm?
These inventories are only used on release branches that support rolling updates between patch releases, so drop them from the main backport along with the related compatibility hashing change. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
|
Removed |
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive DCR authorization and existing-realm migration still require integration validation, including upgrades from 26.8.2 and clustered upgrade compatibility, which could not be exercised in this review environment.
No open issues 🎉
Review effort: Balanced
Summary
Backports the CVE-2026-109081 fix from 26.8.2 onto
mainso Dynamic Client Registration cannot inject realm default-role composites or enable full scope unless an admin removes the new client registration policies.MigrateTo26_8_2now combines the existing Admin API audience/scope migration onmainwith the client registration policy installation from the backport. The upgrade note is documented under 27.0.0.Closes #54702