[OID4VP] Introduce SD-JWT User Attribute / Session mappers - #51632
[OID4VP] Introduce SD-JWT User Attribute / Session mappers#51632dominikschlosser wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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.
4010455 to
3d1f694
Compare
There was a problem hiding this comment.
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, whilesplitClaimPathtreats 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;
3d1f694 to
7f63fcc
Compare
There was a problem hiding this comment.
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
updateBrokeredUserdeliberately ignores them viasetIfPresent. In particular, mapping a blank claim tousernamesets 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.splitClaimPathdoes not reject empty components:.emailis parsed as the field.email, andaddress..localityasaddressfollowed by.locality. These malformed paths can therefore select matching JSON keys instead of returningnullas this parser promises; validate leading and consecutive unescaped separators before constructing steps.
for (String segment : JsonUtils.splitClaimPath(path)) {
Unreported flaky test detectedIf 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#loginLDAPUserAuthenticationSimpleEncryptionStartTLSKeycloak CI - Java Distribution IT (windows-latest - temurin - 21) |
Signed-off-by: Dominik Schlosser <dominik.schlosser@gmail.com>
7f63fcc to
a57da3f
Compare
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.