[OID4VCI-FAPI2] Pass fapi2-security-profile-final-dpop-negative-tests#50125
Open
tdiesler wants to merge 1 commit into
Open
[OID4VCI-FAPI2] Pass fapi2-security-profile-final-dpop-negative-tests#50125tdiesler wants to merge 1 commit into
tdiesler wants to merge 1 commit into
Conversation
closes keycloak#48070 Signed-off-by: Thomas Diesler <tdiesler@proton.me>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Keycloak’s DPoP validation and OID4VC test utilities to satisfy the FAPI2 “final DPoP negative tests” expectations (issue #48070), tightening proof/JWK validation and aligning HTTP URI matching behavior.
Changes:
- Extend OID4VC wallet test helper API to allow sending
dpop_jkton authorization requests. - Normalize DPoP
htucomparison by stripping URI fragments (in addition to query) to match expected DPoP URL processing. - Harden DPoP proof header/JWK validation by rejecting missing
algand rejecting private-key material (d) in the embedded JWK.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCBasicWallet.java | Adds a fluent helper to set dpop_jkt on authorization requests in OID4VC tests. |
| services/src/main/java/org/keycloak/services/util/DPoPUtil.java | Tightens DPoP validation (URI normalization, alg presence check, reject JWK private-key parameter d). |
| } | ||
| if (key.getPrivateKey() != null) { | ||
| throw new VerificationException("Private key is present in DPoP header"); | ||
| // [TODO >>>] JWKSUtils.getKeyWrapper never seems to extract the private key. Should that be changed? |
Comment on lines
107
to
+109
| private static URI normalize(URI uri) { | ||
| return UriBuilder.fromUri(uri).replaceQuery("").build(); | ||
| URI normalized = UriBuilder.fromUri(uri).fragment(null).replaceQuery("").build(); | ||
| return normalized; |
Comment on lines
+188
to
193
| if (header.getAlgorithm() == null) { | ||
| throw new VerificationException("Invalid or missing alg in DPoP header: " + header.getAlgorithm()); | ||
| } | ||
|
|
||
| String algorithm = header.getAlgorithm().name(); | ||
| if (!getDPoPSupportedAlgorithms(session).contains(algorithm)) { |
Comment on lines
+211
to
+212
| if (key.getPrivateKey() != null || jwk.getOtherClaims().containsKey("d")) { | ||
| throw new VerificationException("Private key parameter 'd' must not be present in DPoP JWK"); |
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.forms.MultipleTabsLoginTest#expiredAuthenticationAction_expiredCodeExpiredExecutionKeycloak CI - Forms IT (chrome) org.keycloak.testsuite.federation.kerberos.KerberosLdapTest#writableEditModeTestKeycloak CI - Java Distribution IT (windows-latest - temurin - 21) org.keycloak.testsuite.federation.kerberos.KerberosLdapTest#usernamePasswordLoginTestKeycloak CI - Java Distribution IT (windows-latest - temurin - 21) |
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 #48070