Skip to content

[OID4VP] Introduce SD-JWT User Attribute / Session mappers - #51632

Open
dominikschlosser wants to merge 1 commit into
keycloak:mainfrom
dominikschlosser:vp-mappers
Open

[OID4VP] Introduce SD-JWT User Attribute / Session mappers#51632
dominikschlosser wants to merge 1 commit into
keycloak:mainfrom
dominikschlosser:vp-mappers

Conversation

@dominikschlosser

Copy link
Copy Markdown
Contributor

Closes #51427

@rmartinc @vaceksimon @mabartos Could you please review?

Note that we do not support to select array indices other than 0 (or the whole array), because we a) do not know if the whole array or just the element will be disclosed (depends on how the data in the requested credential is modeled) and b) in case of selectively disclosing array elements, the other elements are not present at all... so adding two mappers for the same array but different indices would never work (at least not without synchronization between the mappers which is messy). Furthermore i really can't think of a good reason to select for example the 3rd index of an array... you generally don't even know how much elements an array has or how they are ordered.

Copilot AI balanced review requested due to automatic review settings August 11, 2026 22:27
@dominikschlosser
dominikschlosser requested a review from a team as a code owner August 11, 2026 22:27

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

Adds SD-JWT credential claim mapping for OID4VP broker logins.

Changes:

  • Adds user attribute and session-note mappers with nested/array claim paths.
  • Defers brokered-context creation until browser login completion.
  • Adds unit and integration coverage for mapping behavior.

Reviewed changes

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

Show a summary per file
File Description
OID4VPX509HashDirectPostTest.java Tests completed-login mapping and resynchronization.
OID4VPVerifierTestBase.java Configures verifier mappers and test helpers.
OID4VPSdJwtUserSessionAttributeMapperTest.java Tests session-note mapping.
OID4VPSdJwtUserAttributeMapperTest.java Tests user attribute mapping.
ClaimPathTest.java Tests claim-path parsing and selection.
IdentityProviderMapper Registers both new mapper providers.
OID4VPIdentityProviderEndpoint.java Defers context construction and exposes verified claims.
OID4VPIdentityProvider.java Defines claim-related context keys.
OID4VPSdJwtUserSessionAttributeMapper.java Maps claims to session notes.
OID4VPSdJwtUserAttributeMapper.java Maps claims to user properties and attributes.
ClaimPath.java Implements nested and array claim selection.
AbstractOID4VPClaimMapper.java Provides shared claim conversion and mapper behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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 12 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

services/src/main/java/org/keycloak/broker/oid4vp/mappers/ClaimPath.java:77

  • The trailing-dot guard rejects an escaped terminal dot such as org\., despite the documented literal-dot syntax, while splitClaimPath treats leading or consecutive unescaped separators (.a, a..b) as field text instead of malformed components. Validate separators by backslash parity so only unescaped empty path components are rejected.
        // The splitting below would silently drop a trailing separator, so a path ending in a
        // dot is always malformed, even an escaped one.
        if (path == null || path.endsWith(".")) {
            return null;

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 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (3)

services/src/main/java/org/keycloak/broker/oid4vp/mappers/ClaimPath.java:76

  • This rejects an escaped terminal dot as well as a separator, so a JSON claim whose literal name ends in . cannot be addressed despite the documented \. escaping syntax. Detect only an unescaped terminal separator (accounting for odd/even preceding backslashes) and keep escaped terminal dots as field content.
        // The splitting below would silently drop a trailing separator, so a path ending in a
        // dot is always malformed, even an escaped one.
        if (path == null || path.endsWith(".")) {

services/src/main/java/org/keycloak/broker/oid4vp/mappers/OID4VPSdJwtUserAttributeMapper.java:105

  • Blank scalar claims are applied to the broker context here, while updateBrokeredUser deliberately ignores them via setIfPresent. In particular, mapping a blank claim to username sets a non-null empty model username, preventing the broker's normal username fallback and causing first-login user creation to fail; use the same non-blank guard on this path.
            case USERNAME -> context.setModelUsername(values.get(0));
            case EMAIL -> context.setEmail(values.get(0));
            case FIRST_NAME -> context.setFirstName(values.get(0));
            case LAST_NAME -> context.setLastName(values.get(0));

services/src/main/java/org/keycloak/broker/oid4vp/mappers/ClaimPath.java:80

  • JsonUtils.splitClaimPath does not reject empty components: .email is parsed as the field .email, and address..locality as address followed by .locality. These malformed paths can therefore select matching JSON keys instead of returning null as this parser promises; validate leading and consecutive unescaped separators before constructing steps.
        for (String segment : JsonUtils.splitClaimPath(path)) {

@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.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLS

Keycloak CI - Java Distribution IT (windows-latest - temurin - 21)

org.openqa.selenium.TimeoutException: 
java.net.SocketTimeoutException: Read timed out
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Windows Server 2025', os.arch: 'amd64', os.version: '10.0', java.version: '21.0.12'
Driver info: driver.version: HtmlUnitDriver
...

Report flaky test

Signed-off-by: Dominik Schlosser <dominik.schlosser@gmail.com>
Copilot AI review requested due to automatic review settings August 12, 2026 23:28

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

[OID4VP] Implement identity provider mapper for SD-JWT

3 participants