[OID4VCI] Replace realm options Trusted Key IDs and Trusted Keys (JSON) with trust-material IdPs configured on the client#50129
Open
IngridPuppet wants to merge 4 commits into
Conversation
…ust-material IdPs configured on the client Closes keycloak#48414 Signed-off-by: Ingrid Kamga <Ingrid.Kamga@adorsys.com>
Signed-off-by: Ingrid Kamga <Ingrid.Kamga@adorsys.com>
Contributor
Author
|
Hello @Awambeng @forkimenjeckayang - Thank you for your review comments in adorsys#300. They should now be addressed in this corresponding upstream PR. Please leave any further comment here directly. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns OID4VCI key-attestation trust configuration with the existing trust-material IdP approach by removing realm-level “Trusted Key IDs / Trusted Keys (JSON)” and instead resolving trusted attestation keys from trust-material identity providers configured per-client.
Changes:
- Replace realm-attribute trusted attestation key resolution with a client attribute (
oid4vci.attester_trust_idps) that points to trust-material IdPs. - Update Admin UI to remove the realm-level fields and add a client-level OID4VCI “Attester Trust Identity Providers” selector (only shown when OID4VCI is enabled).
- Refactor/extend tests and docs to match the new configuration model.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCProofTestUtils.java | Test utilities updated; adds JWKS conversion helper. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCIssuerTestBase.java | Configures the new client attribute for trusted attester IdPs in test clients. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCINaturalPersonTest.java | Switches test setup from realm attributes to trust-material IdP configuration. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java | Updates wallet test helper to accept an explicit attestation key instead of using a callback. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/issuance/signing/OID4VCKeyAttestationTest.java | Reworks attestation validation tests to configure trust-material IdP JWKS. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/abca/OIDCAttestationBasedClientAuthenticationTest.java | Removes dependency on removed ABCA constant; uses test base constant. |
| tests/base/src/test/java/org/keycloak/tests/oid4vc/abca/HAIPIssuerConformanceTest.java | Same as above for HAIP conformance tests. |
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/TrustedAttestationKeysLoader.java | Removes realm-level trusted-key loader implementation. |
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/TrustedAttestationKeyResolver.java | New resolver that loads trusted keys via client-configured trust-material IdPs. |
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidatorFactory.java | Switches JWT proof validator to use the new resolver. |
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationProofValidatorFactory.java | Switches attestation proof validator to use the new resolver. |
| services/src/main/java/org/keycloak/broker/trust/DefaultTrustIdentityProvider.java | Avoids streaming null when a specific key lookup returns null. |
| services/src/main/java/org/keycloak/authentication/authenticators/client/AttestationBasedClientAuthenticator.java | Removes default trust IdP alias constant. |
| server-spi-private/src/main/java/org/keycloak/constants/OID4VCIConstants.java | Replaces realm attribute constants with the new client attribute constant. |
| js/libs/keycloak-admin-client/src/defs/identityProviderRepresentation.ts | Adds TRUST_MATERIAL identity provider type for UI filtering. |
| js/apps/admin-ui/test/utils/form.ts | Makes switch helpers idempotent to reduce flaky UI interactions. |
| js/apps/admin-ui/test/utils/AdminClient.ts | Adds getClient helper used by new OID4VCI UI tests. |
| js/apps/admin-ui/test/realm-settings/oid4vci-attributes.spec.ts | Removes realm settings UI test for legacy trusted keys/IDs fields. |
| js/apps/admin-ui/test/identity-providers/main.ts | Updates Default Trust IdP test helper; now accepts alias and asserts creation notification. |
| js/apps/admin-ui/test/identity-providers/default-trust.spec.ts | Adjusts Default Trust IdP test to new helper signature. |
| js/apps/admin-ui/test/clients/advanced.ts | Adds helpers for interacting with the new OID4VCI attester-trust-idps selector. |
| js/apps/admin-ui/test/clients/advanced.spec.ts | Adds UI coverage for visibility/persistence of the new client attribute. |
| js/apps/admin-ui/src/realm-settings/TokensTab.tsx | Removes realm-level “Attestation Trust” configuration fields. |
| js/apps/admin-ui/src/components/identity-provider/IdentityProviderSelect.tsx | Refactors prop types and continues to fetch filtered IdPs for select. |
| js/apps/admin-ui/src/components/dynamic/MultivaluedListComponent.tsx | Adds configurable stringify separator (used for comma-separated attributes). |
| js/apps/admin-ui/src/clients/AdvancedTab.tsx | Ensures reset covers both OID4VCI enablement and trust-idps attribute. |
| js/apps/admin-ui/src/clients/advanced/OpenIdVerifiableCredentials.tsx | Adds trust-material IdP multi-select shown only when OID4VCI is enabled. |
| js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties | Removes legacy realm field labels; adds new client field labels/help. |
| docs/documentation/server_admin/topics/oid4vci/proofs.adoc | Updates docs from realm attributes to client attribute referencing trust-material IdPs. |
Comment on lines
+39
to
+42
| ClientModel client = session.getContext().getClient(); | ||
| if (client == null) { | ||
| throw new IllegalStateException("Cannot load trust-material IdP aliases because client is null"); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Dear Copilot, this is definitely an illegal state. Under no circumstances the client is expected to be null at this point, and the user shouldn't be blamed for it. Very very unexpected.
Signed-off-by: Ingrid Kamga <Ingrid.Kamga@adorsys.com>
Signed-off-by: Ingrid Kamga <xingridkamga@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #48414