Skip to content

Introduce dedicated delegation permission for token exchange delegation - #51520

Open
mabartos wants to merge 3 commits into
keycloak:mainfrom
mabartos:KC-51481
Open

Introduce dedicated delegation permission for token exchange delegation#51520
mabartos wants to merge 3 commits into
keycloak:mainfrom
mabartos:KC-51481

Conversation

@mabartos

@mabartos mabartos commented Aug 6, 2026

Copy link
Copy Markdown
Member
  • Closes Introduce dedicated delegation permission for token exchange delegation #51481
  • New delegate scope on USERS resource type and delegate-members on GROUPS (mirrors impersonate/impersonate-members pattern)
  • Delegation permissions managed exclusively through FGAP V2 (no admin roles)
  • canDelegate() only supported in FGAP V2, V1 throws UnsupportedOperationException - which is handled as warn log and ignored for client scopes
  • DelegationScopeType now calls canDelegate() instead of canImpersonate()

@mabartos
mabartos force-pushed the KC-51481 branch 2 times, most recently from 65316b5 to 14ef744 Compare August 6, 2026 15:36

@pedroigor pedroigor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need migration to add the new scope to existing realms. See #48375.

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

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.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testPostBadAssertionSignature

Keycloak CI - Adapter IT Strict Cookies

org.openqa.selenium.JavascriptException: 
TypeError: can't access property "outerHTML", document.documentElement is null
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1020-azure', java.version: '21.0.11'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
...

Report flaky test

@mabartos
mabartos requested a balanced review from Copilot August 7, 2026 10:32
@mabartos
mabartos marked this pull request as ready for review August 7, 2026 10:32
@mabartos
mabartos requested a review from a team as a code owner August 7, 2026 10:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Introduces FGAP V2-specific delegation permissions for token exchange, separating delegation from administrator impersonation.

Changes:

  • Adds delegate and delegate-members scopes with migration support.
  • Evaluates delegation through FGAP V2 and the feature flag.
  • Updates delegation tests and documentation.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/base/.../TokenExchangeDelegationTest.java Tests delegation permissions.
tests/base/.../ParameterizedScopesOAuthGrantTest.java Updates implicit-flow delegation test.
services/.../UserPermissionsV2.java Implements delegation evaluation.
services/.../UserPermissions.java Rejects delegation under FGAP V1.
services/.../UserPermissionEvaluator.java Adds delegation API methods.
services/.../IdentityProviderPermissions.java Uses shared client attribute constant.
services/.../ClientPermissions.java Uses shared client attribute constant.
services/.../DelegationScopeType.java Switches to delegation permission.
services/.../ClientScopeAuthorizationRequestParser.java Handles unsupported FGAP operations.
services/.../DefaultEvaluationContext.java Uses shared attribute constants.
server-spi-private/.../EvaluationContext.java Defines evaluation attribute constants.
server-spi-private/.../AdminPermissionsSchema.java Defines new FGAP scopes.
model/storage-private/.../DefaultMigrationManager.java Registers the migration.
model/storage-private/.../MigrateTo26_8_0.java Migrates existing FGAP schemas.
docs/guides/.../token-exchange.adoc Documents delegation configuration.
docs/documentation/.../changes-26_8_0.adoc Adds upgrade guidance.
authz/policy/common/.../ClientPolicyProvider.java Uses the shared client attribute.

Comment thread docs/guides/securing-apps/token-exchange.adoc
Copilot AI review requested due to automatic review settings August 7, 2026 10:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (2)

services/src/main/java/org/keycloak/protocol/oidc/scope/DelegationScopeType.java:72

  • Client policies cannot authorize this path because this overload supplies no requester, so evaluation from the actor's UserModelIdentity has no kc.client.id; ClientPolicyProvider only grants when that attribute is present. Pass the intended client into delegation evaluation while preserving the actor identity, or remove the documented Client-policy support.
        if (!evaluator.users().canDelegate(currentUser)) {

services/src/main/java/org/keycloak/services/resources/admin/fgap/UserPermissionEvaluator.java:133

  • The earlier canImpersonate() Javadoc still links to ImpersonationConstants even though this change removes that import, leaving an unresolved Javadoc reference. Update that link to AdminRoles#IMPERSONATION as well.
     * Returns {@code true} if the caller has the {@link AdminRoles#IMPERSONATION} role.

@rmartinc rmartinc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @mabartos! LGTM, just two comments for your consideration, in case you think that they are useful.

Copilot AI review requested due to automatic review settings August 10, 2026 11:56
@mabartos
mabartos requested a review from rmartinc August 10, 2026 11:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docs/guides/securing-apps/token-exchange.adoc:371

  • A Client policy cannot authorize this flow: canDelegate evaluates with a UserModelIdentity and no kc.client.id context attribute, while ClientPolicyProvider only matches that attribute. Following this guidance silently drops the scope, so recommend a User policy for both administrators and service-account users unless the evaluator is changed to provide the actor client ID.
NOTE: To restrict delegation to specific users, select individual users when creating the permission. To restrict which administrators or service accounts can delegate, use a more specific policy (for example, a *User* or *Client* policy).

@mabartos

Copy link
Copy Markdown
Member Author

@rmartinc Updated based on the discussion. Should be ready for another review.

External links failure is not related to this PR.

@rmartinc rmartinc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One last comment. You can forget about it if you think that it's too pretentious.

Copilot AI review requested due to automatic review settings August 10, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

services/src/main/java/org/keycloak/services/resources/admin/fgap/UserPermissionsV2.java:144

  • The new service-account branch that injects CLIENT_ID_ATTRIBUTE is untested; all delegation tests use a regular administrator with a User policy. Add coverage with a service-account actor and a Client policy, including a nonmatching client, to verify this authorization path.
        DefaultEvaluationContext context = Optional.ofNullable(root.admin())
                .map(UserModel::getServiceAccountClientLink)
                .map(root.realm::getClientById)
                .map(client -> new DefaultEvaluationContext(root.identity(), Map.of(CLIENT_ID_ATTRIBUTE, List.of(client.getClientId())), session))
                .orElse(null);

Copilot AI review requested due to automatic review settings August 11, 2026 06:41
@mabartos
mabartos force-pushed the KC-51481 branch 2 times, most recently from f87f80a to 4f3af66 Compare August 11, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

services/src/main/java/org/keycloak/services/resources/admin/fgap/UserPermissionsV2.java:144

  • The new service-account-specific evaluation path is not covered: all added delegation tests use User policies, so a regression in Client-policy matching would go unnoticed. Add a delegation test using a service-account actor and a Client policy, including a non-matching client case.
        DefaultEvaluationContext context = Optional.ofNullable(root.admin())
                .map(UserModel::getServiceAccountClientLink)
                .map(root.realm::getClientById)
                .map(client -> new DefaultEvaluationContext(root.identity(), Map.of(CLIENT_ID_ATTRIBUTE, List.of(client.getClientId())), session))
                .orElse(null);

Copilot AI review requested due to automatic review settings August 11, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

services/src/main/java/org/keycloak/services/resources/admin/fgap/UserPermissionsV2.java:144

  • The new service-account/Client-policy authorization branch is not exercised: all added delegation tests use UserPolicyRepresentation, even though this path and the documentation promise Client policies for service accounts. Add an integration test with a service-account actor and a Client policy, including a non-matching client case, to prevent delegation authorization regressions in this security-sensitive path.
        DefaultEvaluationContext context = Optional.ofNullable(root.admin())
                .map(UserModel::getServiceAccountClientLink)
                .map(root.realm::getClientById)
                .map(client -> new DefaultEvaluationContext(root.identity(), Map.of(CLIENT_ID_ATTRIBUTE, List.of(client.getClientId())), session))
                .orElse(null);

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

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.keycloak.testsuite.webauthn.registration.passwordless.PwdLessAuthAttachmentRegTest#authenticatorAttachmentPlatform

Keycloak CI - WebAuthn IT

org.openqa.selenium.WebDriverException: 
unknown error: unhandled inspector error: {"code":-32000,"message":"Node with given id does not belong to the document"}
  (Session info: chrome=150.0.7871.128)
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1020-azure', java.version: '25.0.3'
...

Report flaky test

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

Closes keycloak#51481

Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Signed-off-by: Martin Bartoš <mabartos@redhat.com>
Copilot AI review requested due to automatic review settings August 11, 2026 08:01

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

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.keycloak.testsuite.webauthn.registration.passwordless.PwdLessAuthAttachmentRegTest#authenticatorAttachmentPlatform

Keycloak CI - WebAuthn IT

org.openqa.selenium.WebDriverException: 
unknown error: unhandled inspector error: {"code":-32000,"message":"Node with given id does not belong to the document"}
  (Session info: chrome=150.0.7871.128)
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1020-azure', java.version: '25.0.3'
...

Report flaky test

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (1)

services/src/main/java/org/keycloak/services/resources/admin/fgap/UserPermissionsV2.java:143

  • This service-account/Client-policy evaluation path is not exercised by the updated delegation tests; they only use a regular user actor with UserPolicyRepresentation. Add a delegation test using a service-account actor and an FGAP Client policy so the new CLIENT_ID_ATTRIBUTE behavior is verified.
        DefaultEvaluationContext context = Optional.ofNullable(root.admin())
                .map(UserModel::getServiceAccountClientLink)
                .map(root.realm::getClientById)
                .map(client -> new DefaultEvaluationContext(root.identity(), Map.of(CLIENT_ID_ATTRIBUTE, List.of(client.getClientId())), session))

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

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.keycloak.testsuite.webauthn.registration.passwordless.PwdLessOtherSettingsTest#excludeCredentials

Keycloak CI - WebAuthn IT

org.openqa.selenium.NoSuchElementException: 
no such element: Unable to locate element: {"method":"link text","selector":"Register"}
  (Session info: chrome=150.0.7871.128)
For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
Build info: version: '4.28.1', revision: '73f5ad48a2'
...

Report flaky test

@mabartos

Copy link
Copy Markdown
Member Author

@rmartinc Should be ready for the review.

@rmartinc rmartinc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @mabartos! LGTM!
@pedroigor Do you want to take a look to this? You know much more than us.... 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce dedicated delegation permission for token exchange delegation

4 participants