[OID4VCI] User-editable did attribute used as credential subject with realm-local uniqueness only by tdiesler · Pull Request #51221 · keycloak/keycloak

tdiesler · 2026-07-28T07:38:09Z

Signed-off-by: Thomas Diesler tdiesler@proton.me
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

Remove DID-specific support from OID4VCI, treating the subject identifier as a plain user attribute with realm-local uniqueness only.

Deleted (bulk of the change — 497 lines removed):

DIDUtils.java — Entire utility class for DID encoding/decoding (did:key for P-256/ES256, multibase/multicodec/base58btc). Gone.
DuplicateDidValidator.java — Custom validator that enforced DID-format uniqueness on user attributes. Gone.
OID4VCUserDidAttributeTest.java — Tests for the DID user attribute behavior. Gone.
ValidatorFactory SPI entry — Removed the DuplicateDidValidator service registration.

Modified:

OID4VCConstants.java — Removed the comment "Would in most cases be the subject's DID" from the CLAIM_NAME_SUBJECT_ID constant.
DeclarativeUserProfileProviderFactory.java — Removed the automatic registration of a did user attribute with the DuplicateDidValidator. The factory no longer injects DID-specific profile configuration.
OID4VCLoginProtocolFactory.java — Changed the subject ID mapper from did to subjectId as the mapped user attribute.
OID4VCSubjectIdMapper.java — Same rename: reads subjectId attribute instead of did.
UserModel.java — Removed the ATTR_DID constant.
Test files — Updated tests to use subjectId instead of did, removed DID-specific test scenarios.

In essence:

The credential subject identifier is no longer assumed to be a DID. It's now a generic subjectId user attribute with no special format validation — just realm-local
uniqueness via standard mechanisms.

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Removes DID-specific handling from OID4VCI issuance by eliminating DID utilities/validators and switching subject identifier mapping away from a DID-focused user attribute.

Changes:

Deleted DID-related utility/validator code and corresponding test coverage.
Removed automatic “did” user-profile attribute injection and validator SPI registration.
Changed OID4VC subject-id mapping defaults in factories/tests (now mapped to username in the reviewed hunks).

Reviewed changes

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

Show a summary per file

File	Description
tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCUserDidAttributeTest.java	Removes DID-specific integration tests.
tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java	Stops seeding users with `did` and remaps subject-id mapper input to username.
tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerEndpointTest.java	Removes test-time injection of a DID attribute + validators into user profile config.
tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCINaturalPersonTest.java	Updates expected subject identifier value from DID to username.
services/src/main/resources/META-INF/services/org.keycloak.validate.ValidatorFactory	Removes `DuplicateDidValidator` service registration.
services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java	Removes VC-enabled DID attribute injection and associated validation logic.
services/src/main/java/org/keycloak/protocol/oid4vc/userprofile/DuplicateDidValidator.java	Deletes the DID-uniqueness validator implementation.
services/src/main/java/org/keycloak/protocol/oid4vc/issuance/mappers/OID4VCSubjectIdMapper.java	Removes DID from selectable options and changes default to username.
services/src/main/java/org/keycloak/protocol/oid4vc/OID4VCLoginProtocolFactory.java	Changes built-in subject-id mapper default from DID to username.
server-spi/src/main/java/org/keycloak/models/UserModel.java	Removes the `DID` attribute constant from the public SPI.
core/src/main/java/org/keycloak/util/DIDUtils.java	Deletes DID encoding/decoding utilities.
core/src/main/java/org/keycloak/OID4VCConstants.java	Removes DID-specific wording from subject-id constant documentation.

keycloak-github-bot · 2026-07-28T14:15:51Z

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.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

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

org.openqa.selenium.TimeoutException: 
Expected condition failed: waiting for value to contain (ignoring case) "sign in to". Current value: "AUTH_RESPONSE" (tried for 5 second(s) with 500 milliseconds interval)
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.11'
Driver info: org.jboss.arquillian.drone.webdriver.htmlunit.DroneHtmlUnitDriver_ByGraphene
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

services/src/main/java/org/keycloak/protocol/oid4vc/issuance/mappers/OID4VCSubjectIdMapper.java:102

The class-level Javadoc still describes a default user-editable did attribute and DID-based authorization flow, but this configuration now defaults to username and no longer creates that profile attribute. Please update the mapper documentation so it no longer advertises the removed DID-specific behavior.

        userAttributeConfig.setOptions(List.of(UserModel.USERNAME, UserModel.EMAIL, UserModel.ID));
        userAttributeConfig.setDefaultValue(UserModel.USERNAME);

tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java:734

Removing these did values leaves the JWT credential scope without a subject identifier: test-credential-mappers.json still configures its oid4vc-subject-id-mapper with userAttribute: "did". Consequently the JWT issuance tests now exercise credentials with no mapped id/sub; update that mapper fixture to username together with this data change.

            realm.users(createUser("John Doe", Map.of(), List.of(), Collections.emptyMap()));
            realm.users(createUser("Alice Wonderland", Map.of(), List.of(), Map.of()));

mposolda

@tdiesler Thanks! It would be nice to include this PR, but test failures in CI are likely caused by the changes in this PR. Can you please fix the remaining failing test?

tdiesler · 2026-07-29T09:05:54Z

Can you please fix the remaining failing test?

It must be some funky side-effect from removing the did attribute. It is this code in the test case that works in main, but fails in the PR

            // Now remove lastName — credential request should fail (mandatory via auth details)
            userState.userRep.setLastName(null);
            userState.user.update(userState.userRep);

            UserRepresentation userRep = testRealm.admin().users().search(TEST_USER).get(0);
            var lastName = userRep.getLastName();
            assertNull(lastName, "User last name should be null, not: " + lastName);

So it seems that the user update does no longer work.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

services/src/main/java/org/keycloak/services/resources/admin/UserResource.java:224

This also excludes locale, but unlike the other root attributes, locale has no top-level field in UserRepresentation. When an update omits attributes, the profile normalizes the missing editable locale to an empty value and profile.update(false) removes the user's existing locale; preserve locale in this merge while filtering the top-level root fields.

                    if (!UserProfileUtil.isRootAttribute(entry.getKey())) {
                        attributes.putIfAbsent(entry.getKey(), entry.getValue());
                    }

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

services/src/main/java/org/keycloak/services/resources/admin/UserResource.java:223

When attributes and top-level profile fields are omitted in a partial update, getRawAttributes() does not supply those fields. Skipping the user's current root attributes therefore normalizes editable email/name/locale fields to empty, so credential-only updates can clear unrelated profile data or fail required-field validation; the changed hashed-credential test now fetches the full representation and masks this established partial-update path.

                // account the existing attributes associated with the user. Root attributes (username, email,
                // firstName, lastName, locale) are already handled by getRawAttributes() via top-level fields.
                for (Map.Entry<String, List<String>> entry : user.getAttributes().entrySet()) {
                    if (!UserProfileUtil.isRootAttribute(entry.getKey())) {
                        attributes.putIfAbsent(entry.getKey(), entry.getValue());

mposolda

@tdiesler I think it should be possible to fix the OID4VCI test failures without a need to change core user-profile logic in classes like AbstractUserRepresentation or UserResource . As you can see, there are other test failures now and changes in those classes might be backwards incompatible... I suppose the root cause of OID4VCI test failures after removal of did is somewhere else (scoped within OID4VCI)...?

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java:298

This changes the established user-representation contract from null to {} when no attributes remain. Existing tests explicitly require null after attributes are removed (for example tests/base/src/test/java/org/keycloak/tests/admin/user/UserAttributesTest.java:132 and DeclarativeUserTest.java:127-137), so this causes unrelated test failures and an Admin REST response compatibility change.

        rep.setAttributes(attributesRep);

tdiesler · 2026-07-29T13:16:17Z

I think it should be possible to fix the OID4VCI test failures without a need to change core user-profile logic in classes like AbstractUserRepresentation or UserResource . As you can see, there are other test failures now and changes in those classes might be backwards incompatible... I suppose the root cause of OID4VCI test failures after removal of did is somewhere else (scoped within OID4VCI)...?

It seems to be related to null vs. empty map of user attributes. This workaround in OID4VCAuthorizationCodeFlowTestBase fixes it (for that test case)

        // Ensure attributes is non-null so that subsequent updates (e.g. setLastName(null))
        // skip the putIfAbsent merge in UserResource.updateUser and actually clear the field.
        if (userRep.getAttributes() == null) {
            userRep.setAttributes(Collections.emptyMap());
        }

Copilot

Pull request overview

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

Copilot

Pull request overview

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

… realm-local uniqueness only Closes keycloak#50524 Signed-off-by: Thomas Diesler <tdiesler@proton.me> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Remove DID-specific support from OID4VCI, treating the subject identifier as a plain user attribute with realm-local uniqueness only. Deleted (bulk of the change — 497 lines removed): - DIDUtils.java — Entire utility class for DID encoding/decoding (did:key for P-256/ES256, multibase/multicodec/base58btc). Gone. - DuplicateDidValidator.java — Custom validator that enforced DID-format uniqueness on user attributes. Gone. - OID4VCUserDidAttributeTest.java — Tests for the DID user attribute behavior. Gone. - ValidatorFactory SPI entry — Removed the DuplicateDidValidator service registration. Modified: - OID4VCConstants.java — Removed the comment "Would in most cases be the subject's DID" from the CLAIM_NAME_SUBJECT_ID constant. - DeclarativeUserProfileProviderFactory.java — Removed the automatic registration of a did user attribute with the DuplicateDidValidator. The factory no longer injects DID-specific profile configuration. - OID4VCLoginProtocolFactory.java — Changed the subject ID mapper from did to subjectId as the mapped user attribute. - OID4VCSubjectIdMapper.java — Same rename: reads subjectId attribute instead of did. - UserModel.java — Removed the ATTR_DID constant. - Test files — Updated tests to use subjectId instead of did, removed DID-specific test scenarios. In essence: The credential subject identifier is no longer assumed to be a DID. It's now a generic subjectId user attribute with no special format validation — just realm-local uniqueness via standard mechanisms.

keycloak-github-bot · 2026-07-29T20:06:16Z

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.WebAuthnPropertyTest#timeout

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

Unreported flaky test detected, please review

mposolda

@tdiesler Cool, Thanks!

tdiesler requested a review from a team as a code owner July 28, 2026 07:38

Copilot AI review requested due to automatic review settings July 28, 2026 07:38

keycloak-github-bot Bot added the team/core-protocols label Jul 28, 2026

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/protocol/oid4vc/OID4VCLoginProtocolFactory.java

Comment thread services/src/main/java/org/keycloak/protocol/oid4vc/issuance/mappers/OID4VCSubjectIdMapper.java

Comment thread server-spi/src/main/java/org/keycloak/models/UserModel.java

Copilot started reviewing on behalf of tdiesler July 28, 2026 09:17 View session

keycloak-github-bot Bot added the flaky-test label Jul 28, 2026

keycloak-github-bot Bot reviewed Jul 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 05:18

tdiesler force-pushed the ghi50524-didsupport branch from ebca0f6 to 264b0f2 Compare July 29, 2026 05:18

tdiesler changed the title ~~OID4VCI: User-editable did attribute used as credential subject with realm-local uniqueness only~~ [OID4VCI] User-editable did attribute used as credential subject with realm-local uniqueness only Jul 29, 2026

Copilot started reviewing on behalf of tdiesler July 29, 2026 05:18 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

mposolda reviewed Jul 29, 2026

View reviewed changes

mposolda self-assigned this Jul 29, 2026

Copilot AI review requested due to automatic review settings July 29, 2026 09:28

tdiesler force-pushed the ghi50524-didsupport branch from 264b0f2 to 9bc5fc2 Compare July 29, 2026 09:28

tdiesler requested a review from a team as a code owner July 29, 2026 09:28

Copilot started reviewing on behalf of tdiesler July 29, 2026 09:29 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 10:37

tdiesler force-pushed the ghi50524-didsupport branch from 9bc5fc2 to 5498528 Compare July 29, 2026 10:37

Copilot started reviewing on behalf of tdiesler July 29, 2026 10:37 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

tdiesler force-pushed the ghi50524-didsupport branch from 5498528 to 5fe6cd0 Compare July 29, 2026 10:51

mposolda reviewed Jul 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 12:40

tdiesler force-pushed the ghi50524-didsupport branch from 5fe6cd0 to 928ebcf Compare July 29, 2026 12:40

Copilot started reviewing on behalf of tdiesler July 29, 2026 12:40 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

tdiesler force-pushed the ghi50524-didsupport branch from 928ebcf to 72d84eb Compare July 29, 2026 13:11

Copilot AI review requested due to automatic review settings July 29, 2026 13:11

Copilot started reviewing on behalf of tdiesler July 29, 2026 13:12 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 16:54

tdiesler force-pushed the ghi50524-didsupport branch from 72d84eb to 373cd7c Compare July 29, 2026 16:54

Copilot started reviewing on behalf of tdiesler July 29, 2026 16:56 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

tdiesler force-pushed the ghi50524-didsupport branch from 373cd7c to 9532c13 Compare July 29, 2026 19:04

keycloak-github-bot Bot mentioned this pull request Jul 29, 2026

Flaky test: org.keycloak.testsuite.webauthn.registration.WebAuthnOtherSettingsTest#apiInvalidStateErrorMessage #51175

Open

keycloak-github-bot Bot reviewed Jul 29, 2026

View reviewed changes

mposolda approved these changes Jul 30, 2026

View reviewed changes

mposolda merged commit 0ac222f into keycloak:main Jul 30, 2026
91 checks passed

tdiesler deleted the ghi50524-didsupport branch July 30, 2026 13:47

Uh oh!

Conversation

tdiesler commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keycloak-github-bot Bot commented Jul 28, 2026

Unreported flaky test detected

org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

tdiesler commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tdiesler commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot commented Jul 29, 2026

Unreported flaky test detected

org.keycloak.testsuite.webauthn.WebAuthnPropertyTest#timeout

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tdiesler commented Jul 29, 2026 •

edited

Loading