From aa0ca52d9a79f462f88b4f866eb5888b0c412c2e Mon Sep 17 00:00:00 2001 From: Awambeng Rodrick Date: Thu, 9 Jul 2026 10:58:06 +0100 Subject: [PATCH 1/2] OID4VCI: Fix hardcoded proof type key in getAttestationRequirements() - Thread proofTypeKey through validateAttestationPayload() to getAttestationRequirements() - Replace the hardcoded .get(JWT) lookup with .get(proofTypeKey) - Remove the unused ProofType.JWT import - Add unit tests verifying attestation requirements are read for the correct proof type Closes #50521 Signed-off-by: Awambeng Rodrick --- .../keybinding/AttestationValidatorUtil.java | 29 +-- .../keybinding/JwtProofValidator.java | 2 +- .../AttestationValidatorUtilTest.java | 166 ++++++++++++++++++ 3 files changed, 173 insertions(+), 24 deletions(-) create mode 100644 services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java diff --git a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java index 466b34c08b47..0648443ec7cf 100644 --- a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java +++ b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java @@ -1,20 +1,3 @@ -/* - * Copyright 2025 Red Hat, Inc. and/or its affiliates - * and other contributors as indicated by the @author tags. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - package org.keycloak.protocol.oid4vc.issuance.keybinding; import java.io.ByteArrayInputStream; @@ -73,7 +56,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import org.jboss.logging.Logger; -import static org.keycloak.protocol.oid4vc.model.ProofType.JWT; import static org.keycloak.services.clientpolicy.executor.FapiConstant.ALLOWED_ALGORITHMS; /** @@ -162,7 +144,7 @@ public static KeyAttestationJwtBody validateAttestationJwt( throw new VCIssuerException(ErrorType.INVALID_PROOF, "Could not verify signature of attestation JWT"); } - validateAttestationPayload(keycloakSession, vcIssuanceContext, attestationBody, requireExpForJwtProof); + validateAttestationPayload(keycloakSession, vcIssuanceContext, attestationBody, requireExpForJwtProof, proofTypeKeyForSigningAlgPolicy); if (attestationBody.getAttestedKeys() == null) { throw new VCIssuerException(ErrorType.INVALID_PROOF, "Missing required attested_keys claim in attestation"); @@ -175,7 +157,8 @@ private static void validateAttestationPayload( KeycloakSession keycloakSession, VCIssuanceContext vcIssuanceContext, KeyAttestationJwtBody attestationBody, - boolean requireExpForJwtProof) throws VCIssuerException, VerificationException { + boolean requireExpForJwtProof, + String proofTypeKey) throws VCIssuerException, VerificationException { if (attestationBody.getIat() == null) { throw new VCIssuerException(ErrorType.INVALID_PROOF, "Missing 'iat' claim in attestation"); @@ -191,7 +174,7 @@ private static void validateAttestationPayload( } // Get resistance level requirements from configuration - KeyAttestationsRequired attestationRequirements = getAttestationRequirements(vcIssuanceContext); + KeyAttestationsRequired attestationRequirements = getAttestationRequirements(vcIssuanceContext, proofTypeKey); validateResistanceLevel(attestationBody, attestationRequirements); KeycloakContext keycloakContext = keycloakSession.getContext(); @@ -227,7 +210,7 @@ private static void validateAttestationPayload( } } - public static KeyAttestationsRequired getAttestationRequirements(VCIssuanceContext vcIssuanceContext) { + public static KeyAttestationsRequired getAttestationRequirements(VCIssuanceContext vcIssuanceContext, String proofTypeKey) { if (vcIssuanceContext.getCredentialConfig() == null || vcIssuanceContext.getCredentialConfig().getProofTypesSupported() == null || vcIssuanceContext.getCredentialConfig().getProofTypesSupported().getSupportedProofTypes() == null) { @@ -237,7 +220,7 @@ public static KeyAttestationsRequired getAttestationRequirements(VCIssuanceConte SupportedProofTypeData proofTypeData = vcIssuanceContext.getCredentialConfig() .getProofTypesSupported() .getSupportedProofTypes() - .get(JWT); + .get(proofTypeKey); return proofTypeData != null ? proofTypeData.getKeyAttestationsRequired() : null; } diff --git a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidator.java b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidator.java index 65e78306d754..3a4d50d3e7c3 100644 --- a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidator.java +++ b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidator.java @@ -314,7 +314,7 @@ private void validateJwsHeader(VCIssuanceContext vcIssuanceContext, JWSHeader jw private KeyAttestationInfo resolveHeaderAttestation(VCIssuanceContext vcIssuanceContext, Map headerClaims) throws JWSInputException, VerificationException { if (!headerClaims.containsKey(KEY_ATTESTATION_CLAIM)) { - KeyAttestationsRequired attestationRequirements = AttestationValidatorUtil.getAttestationRequirements(vcIssuanceContext); + KeyAttestationsRequired attestationRequirements = AttestationValidatorUtil.getAttestationRequirements(vcIssuanceContext, ProofType.JWT); if (attestationRequirements != null) { throw new VCIssuerException(ErrorType.INVALID_PROOF, "key_attestation JWT header claim is required by the credential configuration but was not provided"); diff --git a/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java b/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java new file mode 100644 index 000000000000..3f45d136e8af --- /dev/null +++ b/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java @@ -0,0 +1,166 @@ +package org.keycloak.protocol.oid4vc.issuance.keybinding; + +import java.util.List; +import java.util.Map; + +import org.keycloak.protocol.oid4vc.issuance.VCIssuanceContext; +import org.keycloak.protocol.oid4vc.model.KeyAttestationsRequired; +import org.keycloak.protocol.oid4vc.model.ProofType; +import org.keycloak.protocol.oid4vc.model.ProofTypesSupported; +import org.keycloak.protocol.oid4vc.model.SupportedCredentialConfiguration; +import org.keycloak.protocol.oid4vc.model.SupportedProofTypeData; +import org.keycloak.utils.AbstractUtilSessionTest; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class AttestationValidatorUtilTest extends AbstractUtilSessionTest { + + @Test + public void testGetAttestationRequirements_ReadsFromJwtKeyWhenRequested() { + KeyAttestationsRequired jwtAttestation = new KeyAttestationsRequired() + .setKeyStorage(List.of("hardware")) + .setUserAuthentication(List.of("pin")); + + VCIssuanceContext ctx = createContextWithAttestation( + jwtAttestation, // under "jwt" key + new KeyAttestationsRequired() + .setKeyStorage(List.of("tpm")) + .setUserAuthentication(List.of("fingerprint")) // different under "attestation" + ); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + + Assertions.assertNotNull(result); + assertEquals(List.of("hardware"), result.getKeyStorage(), + "Should read key_storage from the jwt proof type entry"); + assertEquals(List.of("pin"), result.getUserAuthentication(), + "Should read user_authentication from the jwt proof type entry"); + } + + @Test + public void testGetAttestationRequirements_ReadsFromAttestationKeyWhenRequested() { + KeyAttestationsRequired attestationAttestation = new KeyAttestationsRequired() + .setKeyStorage(List.of("tpm")) + .setUserAuthentication(List.of("fingerprint")); + + VCIssuanceContext ctx = createContextWithAttestation( + new KeyAttestationsRequired() + .setKeyStorage(List.of("hardware")) + .setUserAuthentication(List.of("pin")), // different under "jwt" + attestationAttestation // under "attestation" key + ); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.ATTESTATION); + + Assertions.assertNotNull(result); + assertEquals(List.of("tpm"), result.getKeyStorage(), + "Should read key_storage from the attestation proof type entry"); + assertEquals(List.of("fingerprint"), result.getUserAuthentication(), + "Should read user_authentication from the attestation proof type entry"); + } + + @Test + public void testGetAttestationRequirements_ReturnsNullWhenProofTypeKeyNotFound() { + VCIssuanceContext ctx = createContextWithAttestation( + new KeyAttestationsRequired().setKeyStorage(List.of("hardware")), + null + ); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, "unknown_proof_type"); + + assertNull(result, "Should return null when proof type key does not exist"); + } + + @Test + public void testGetAttestationRequirements_ReturnsNullWhenConfigIsNull() { + VCIssuanceContext ctx = new VCIssuanceContext(); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + + assertNull(result, "Should return null when credentialConfig is null"); + } + + @Test + public void testGetAttestationRequirements_ReturnsNullWhenProofTypesSupportedIsNull() { + VCIssuanceContext ctx = new VCIssuanceContext(); + ctx.setCredentialConfig(new SupportedCredentialConfiguration()); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + + assertNull(result, "Should return null when proofTypesSupported is null"); + } + + @Test + public void testGetAttestationRequirements_ReturnsNullWhenSupportedProofTypesIsNull() { + VCIssuanceContext ctx = new VCIssuanceContext(); + SupportedCredentialConfiguration config = new SupportedCredentialConfiguration(); + config.setProofTypesSupported(new ProofTypesSupported()); + ctx.setCredentialConfig(config); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + + assertNull(result, "Should return null when supportedProofTypes map is empty/null"); + } + + @Test + public void testGetAttestationRequirements_JwtAndAttestationProduceDifferentResults() { + // Prove that the two proof types return different attestation requirements + KeyAttestationsRequired jwtReq = new KeyAttestationsRequired() + .setKeyStorage(List.of("hardware")); + KeyAttestationsRequired attestationReq = new KeyAttestationsRequired() + .setKeyStorage(List.of("tpm", "secure_enclave")); + + VCIssuanceContext ctx = createContextWithAttestation(jwtReq, attestationReq); + + KeyAttestationsRequired jwtResult = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + KeyAttestationsRequired attestationResult = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.ATTESTATION); + + Assertions.assertNotNull(jwtResult); + assertEquals(List.of("hardware"), jwtResult.getKeyStorage()); + Assertions.assertNotNull(attestationResult); + assertEquals(List.of("tpm", "secure_enclave"), attestationResult.getKeyStorage()); + } + + @Test + public void testGetAttestationRequirements_ReturnsNullWhenProofTypeEntryHasNoAttestationRequired() { + // A proof type entry exists but has null key_attestations_required + VCIssuanceContext ctx = new VCIssuanceContext(); + SupportedCredentialConfiguration config = new SupportedCredentialConfiguration(); + ProofTypesSupported proofTypesSupported = new ProofTypesSupported(); + // Put an entry with no keyAttestationsRequired + proofTypesSupported.getSupportedProofTypes().put(ProofType.JWT, + new SupportedProofTypeData(List.of("ES256"), null)); + config.setProofTypesSupported(proofTypesSupported); + ctx.setCredentialConfig(config); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); + + assertNull(result, "Should return null when the proof type entry has null key_attestations_required"); + } + + private static VCIssuanceContext createContextWithAttestation( + KeyAttestationsRequired jwtAttestation, + KeyAttestationsRequired attestationAttestation) { + VCIssuanceContext ctx = new VCIssuanceContext(); + SupportedCredentialConfiguration config = new SupportedCredentialConfiguration(); + ProofTypesSupported proofTypesSupported = new ProofTypesSupported(); + Map proofTypes = proofTypesSupported.getSupportedProofTypes(); + + if (jwtAttestation != null) { + proofTypes.put(ProofType.JWT, + new SupportedProofTypeData(List.of("ES256"), jwtAttestation)); + } + if (attestationAttestation != null) { + proofTypes.put(ProofType.ATTESTATION, + new SupportedProofTypeData(List.of("ES256"), attestationAttestation)); + } + + config.setProofTypesSupported(proofTypesSupported); + ctx.setCredentialConfig(config); + return ctx; + } +} From f33c1c68f9c0c8092ec87f710168e5c21802f6e3 Mon Sep 17 00:00:00 2001 From: Awambeng Rodrick Date: Thu, 9 Jul 2026 11:32:56 +0100 Subject: [PATCH 2/2] Address review comments from Copilot Signed-off-by: Awambeng Rodrick --- .../keybinding/AttestationValidatorUtil.java | 12 +++++-- .../AttestationValidatorUtilTest.java | 32 +++++++++++++------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java index 0648443ec7cf..faf95a09bd67 100644 --- a/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java +++ b/services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java @@ -47,6 +47,7 @@ import org.keycloak.protocol.oid4vc.model.ErrorType; import org.keycloak.protocol.oid4vc.model.KeyAttestationJwtBody; import org.keycloak.protocol.oid4vc.model.KeyAttestationsRequired; +import org.keycloak.protocol.oid4vc.model.ProofType; import org.keycloak.protocol.oid4vc.model.ProofTypesSupported; import org.keycloak.protocol.oid4vc.model.SupportedCredentialConfiguration; import org.keycloak.protocol.oid4vc.model.SupportedProofTypeData; @@ -80,8 +81,10 @@ public class AttestationValidatorUtil { * {@code jwt} proof type (embedded {@code key_attestation} header). * @param proofTypeKeyForSigningAlgPolicy {@link org.keycloak.protocol.oid4vc.model.ProofType} value * ({@code jwt} or {@code attestation}) to resolve - * {@code proof_signing_alg_values_supported}; if {@code null}, only - * FAPI {@code ALLOWED_ALGORITHMS} is enforced. + * {@code proof_signing_alg_values_supported} and + * {@code key_attestations_required}; if {@code null}, falls back to + * {@code ProofType.JWT} for attestation requirements and FAPI + * {@code ALLOWED_ALGORITHMS} for signing algorithms. */ public static KeyAttestationJwtBody validateAttestationJwt( String attestationJwt, @@ -217,10 +220,13 @@ public static KeyAttestationsRequired getAttestationRequirements(VCIssuanceConte return null; } + // Fall back to "jwt" when proofTypeKey is null (backward compatibility). + String effectiveKey = proofTypeKey != null ? proofTypeKey : ProofType.JWT; + SupportedProofTypeData proofTypeData = vcIssuanceContext.getCredentialConfig() .getProofTypesSupported() .getSupportedProofTypes() - .get(proofTypeKey); + .get(effectiveKey); return proofTypeData != null ? proofTypeData.getKeyAttestationsRequired() : null; } diff --git a/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java b/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java index 3f45d136e8af..2481f0bf3e69 100644 --- a/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java +++ b/services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilTest.java @@ -26,10 +26,10 @@ public void testGetAttestationRequirements_ReadsFromJwtKeyWhenRequested() { .setUserAuthentication(List.of("pin")); VCIssuanceContext ctx = createContextWithAttestation( - jwtAttestation, // under "jwt" key + jwtAttestation, new KeyAttestationsRequired() .setKeyStorage(List.of("tpm")) - .setUserAuthentication(List.of("fingerprint")) // different under "attestation" + .setUserAuthentication(List.of("fingerprint")) ); KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); @@ -50,8 +50,8 @@ public void testGetAttestationRequirements_ReadsFromAttestationKeyWhenRequested( VCIssuanceContext ctx = createContextWithAttestation( new KeyAttestationsRequired() .setKeyStorage(List.of("hardware")) - .setUserAuthentication(List.of("pin")), // different under "jwt" - attestationAttestation // under "attestation" key + .setUserAuthentication(List.of("pin")), + attestationAttestation ); KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.ATTESTATION); @@ -95,7 +95,8 @@ public void testGetAttestationRequirements_ReturnsNullWhenProofTypesSupportedIsN } @Test - public void testGetAttestationRequirements_ReturnsNullWhenSupportedProofTypesIsNull() { + public void testGetAttestationRequirements_ReturnsNullWhenSupportedProofTypesIsEmpty() { + // supportedProofTypes is always an initialized HashMap (never null), so this covers the empty-map case. VCIssuanceContext ctx = new VCIssuanceContext(); SupportedCredentialConfiguration config = new SupportedCredentialConfiguration(); config.setProofTypesSupported(new ProofTypesSupported()); @@ -103,12 +104,27 @@ public void testGetAttestationRequirements_ReturnsNullWhenSupportedProofTypesIsN KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, ProofType.JWT); - assertNull(result, "Should return null when supportedProofTypes map is empty/null"); + assertNull(result, "Should return null when supportedProofTypes map is empty"); + } + + @Test + public void testGetAttestationRequirements_ReturnsJwtFallbackWhenProofTypeKeyIsNull() { + // null proofTypeKey falls back to "jwt" for backward compatibility. + KeyAttestationsRequired jwtReq = new KeyAttestationsRequired() + .setKeyStorage(List.of("hardware")) + .setUserAuthentication(List.of("pin")); + + VCIssuanceContext ctx = createContextWithAttestation(jwtReq, null); + + KeyAttestationsRequired result = AttestationValidatorUtil.getAttestationRequirements(ctx, null); + + Assertions.assertNotNull(result, "Should return attestation requirements using JWT fallback when proofTypeKey is null"); + assertEquals(List.of("hardware"), result.getKeyStorage()); + assertEquals(List.of("pin"), result.getUserAuthentication()); } @Test public void testGetAttestationRequirements_JwtAndAttestationProduceDifferentResults() { - // Prove that the two proof types return different attestation requirements KeyAttestationsRequired jwtReq = new KeyAttestationsRequired() .setKeyStorage(List.of("hardware")); KeyAttestationsRequired attestationReq = new KeyAttestationsRequired() @@ -127,11 +143,9 @@ public void testGetAttestationRequirements_JwtAndAttestationProduceDifferentResu @Test public void testGetAttestationRequirements_ReturnsNullWhenProofTypeEntryHasNoAttestationRequired() { - // A proof type entry exists but has null key_attestations_required VCIssuanceContext ctx = new VCIssuanceContext(); SupportedCredentialConfiguration config = new SupportedCredentialConfiguration(); ProofTypesSupported proofTypesSupported = new ProofTypesSupported(); - // Put an entry with no keyAttestationsRequired proofTypesSupported.getSupportedProofTypes().put(ProofType.JWT, new SupportedProofTypeData(List.of("ES256"), null)); config.setProofTypesSupported(proofTypesSupported);