feat(sdk): comprehensive DPoP nonce handling and verification - #939
Draft
dmihalcik-virtru wants to merge 69 commits into
Draft
feat(sdk): comprehensive DPoP nonce handling and verification#939dmihalcik-virtru wants to merge 69 commits into
dmihalcik-virtru wants to merge 69 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces DPoP-Nonce caching per RFC 9449 §8, implementing a DPoPNonceCache to store and reuse server-issued nonces by origin, and updating both the authentication interceptor and OIDC client to handle nonce-based retries on 401 challenges. It also adds a CLI command to check for DPoP support. The review feedback highlights several critical improvements: ensuring that 401 retries occur when a new or different nonce is received (rather than only when no nonce was cached), using optional chaining on error metadata to prevent runtime crashes, avoiding direct process.exit calls in the CLI handler, and adding defensive checks when extracting nonces from headers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
June 10, 2026 17:10
bce2ae8 to
cc190c6
Compare
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
dmihalcik-virtru
changed the title
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
June 11, 2026 17:56
bda4566 to
5e933b0
Compare
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
June 24, 2026 17:24
de2ca53 to
686bd9f
Compare
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
June 24, 2026 17:31
db36c75 to
8f43bb7
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
console.assert only prints to stderr and never sets a non-zero exit code, so `inspect` could report success even when a requested DPoP proof or cnf.jkt binding never took effect. Throw CLIError for those security-outcome checks. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
fetchKasBasePubKey threw NetworkError for a malformed platform config and the surrounding catch re-wrapped it under a [PublicKey] network banner, pointing operators at KAS connectivity for a config problem. Throw ConfigurationError and re-throw it unchanged; only genuine RPC failures become NetworkError. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…-3397) fetchKasPubKey dumped the raw base-key error via console.log(e) with no context. Use the existing errBrief helper and a descriptive prefix so the log is a one-liner and never dumps a Connect error object (which may carry DPoP nonce metadata), consistent with the rest of the auth path. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…ers (DSPX-3397) signJwt/verifyJwt cast header.alg to AsymmetricSigningAlgorithm unchecked, while dpop.ts validated with a local type guard — sibling signers enforcing the same invariant with different rigor. Promote the guard and its backing list to declarations.ts (next to the type), reuse it in dpop.ts, and replace both jwt.ts casts so an unsupported alg fails fast with a clear error. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- DER↔P1363 comments in dpop.ts/jwt.ts said RSA/EdDSA are passed through raw, but EdDSA is rejected upstream and never reaches those branches. - fetchWrappedKey JSDoc documented `requestBody`/`clientVersion` params that don't exist; align with the actual (url, signedRequestToken, auth, ...) signature. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
August 4, 2026 16:00
b3874dc to
2c91d78
Compare
Keycloak 25+ serves health on a separate management interface (port 9000) which inherits KC_HTTP_RELATIVE_PATH, so the old 8888/auth/health/live probe 404s against the 26.2 image and the container never reports healthy. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
from
August 4, 2026 17:19
5410c71 to
250d2ec
Compare
config-demo-idp.sh downloaded the Keycloak release zip and ran kcadm.sh on the runner JRE. Keycloak 26.2 kcadm is compiled for Java 17 (class file 61.0) but ubuntu-22.04 defaults to Java 11 (max 55.0), so every call died with UnsupportedClassVersionError and no clients or users were created: Error: LinkageError occurred while loading main class org.keycloak.client.admin.cli.KcAdmMain Run kcadm inside the keycloak container instead. It ships a JRE matching its own Keycloak version, which removes both the JRE-skew failure and a ~155MB download. Resolve the compose file from the script directory so the caller cwd does not matter, and target the container-internal port since the 65432 vite proxy is only reachable from the host. Also give the config-demo-idp.sh failure branch in wait-and-test.sh its own error string; it previously printed the same text as the provision step, making the failing stage ambiguous in CI logs. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-3397-web-sdk
branch
2 times, most recently
from
August 4, 2026 17:51
eb63dd5 to
9f45ab0
Compare
X-Test Failure Report |
The roundtrip already runs every CLI and browser flow with DPoP, but only proves plain proof-of-possession: KAS never challenges, so the server-issued nonce retry this PR adds is never walked. xtest can't fill the gap. Its nonce cases are gated on opentdf/tests' shared `dpop-challenge` input, which also turns on require_nonce for every SDK in the matrix -- including ones with no nonce support -- so it stays off. Without this, the feature ships with no CI coverage. Set server.auth.dpop.require_nonce so KAS answers the first proofed request with 401 + DPoP-Nonce. `enforce` stays off, so bearer tokens are still accepted and the non-DPoP paths in this job are unchanged. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
added a commit
to opentdf/tests
that referenced
this pull request
Aug 6, 2026
) ## What Bump the `start-up-with-containers` pin to `18b8070f` (opentdf/platform#3792) and pass `bootstrap-ref: main` on the `main` platform lane only. ## Why The action overlays `docker-compose.yaml` from `bootstrap-ref`, **independent of `platform-ref`**. That input defaults to the `pqc-enabled` tag, which is still pinned to `keycloak/keycloak:25.0` — a Keycloak that cannot issue DPoP-bound access tokens. So every lane ran KC 25 even though platform `main`'s own compose has been `ghcr.io/opentdf/keycloak-standard:26.4.0` with `KC_FEATURES: dpop` since #3792, and `service/cmd/keycloak_data.yaml` provisions `opentdf-dpop` with `dpop.bound.access.tokens: "true"`. The visible symptom: `test_dpop.py::test_dpop_happy_path_roundtrip` fails for any SDK that checks token binding. The token comes back `typ=Bearer` with no `cnf.jkt`, and the js CLI rejects it: ``` [CRITICAL] DPoP requested but the access token is not bound (missing cnf.jkt) ``` go and java pass only because they don't assert `cnf.jkt`. The other `test_dpop.py` cases already self-skip on `token_type != DPoP`; the SDK-level roundtrips gate on the platform well-known and SDK features, not on IdP capability, so they run and fail. This is what keeps opentdf/web-sdk#939 red. Released platform tags keep the 25.0 bootstrap — they predate the move to standard Keycloak token exchange (opentdf/platform#3754). ## Testing Dispatched against this branch with `platform-ref=main`, `js-ref=refs/pull/939/merge`, `focus-sdk=js` — https://github.com/opentdf/tests/actions/runs/31016132383 — all three lanes green (`go@main`, `java@main`, `js@pull-939`). Confirmed Keycloak 26.4 came up, and these ran rather than skipped: ``` PASSED test_dpop.py::test_dpop_happy_path_roundtrip[small-js@pull-939-js@pull-939-in_focus0] PASSED test_dpop.py::test_dpop_happy_path_roundtrip[small-js@pull-939-java@main-in_focus0] PASSED test_dpop.py::test_dpop_rejects_replayed_jti[small-js@pull-939-in_focus0] PASSED test_dpop.py::test_dpop_rejects_tampered_proof_htu[small-js@pull-939-in_focus0] PASSED test_dpop.py::test_dpop_bearer_scheme_warns_but_accepted_for_dpop_token[small-js@pull-939-in_focus0] ``` The nonce-dependent cases still skip, correctly — `dpop-challenge` is off by default, so `require_nonce` is unset. ## Notes - The new action's input set is a strict superset of the old pin's; no call-site changes beyond `bootstrap-ref`. - `start-additional-kas` is left at `6dd5f649` — it downloads no compose. - Overlaps #568 (DSPX-4190), which took the broader approach of moving every lane to 26.4 via a platform PR branch. This is the narrow slice needed now; #568's remaining checklist (re-point the `pqc-enabled` tag, align `vulnerability.yml`) still stands. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated platform startup workflows to use the Keycloak 26.4 revision. * Adjusted bootstrap selection so the main platform lane uses the main configuration while released tags retain the PQC-enabled configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
|
This was referenced Aug 10, 2026
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
2 participants
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.
Summary
Implements comprehensive DPoP (RFC 9449) support for the web-sdk as part of the Keycloak v26 upgrade and platform-wide DPoP feature.
Parent Jira: https://virtru.atlassian.net/browse/DSPX-3397
Test Scenario: xtest/scenarios/DSPX-3397.yaml
Green e2e Test Run: https://github.com/opentdf/tests/actions/runs/29624686479
Changes
DPoP-Nonce Support (RFC 9449 §8)
lib/src/auth/dpop-nonce.ts- Per-origin nonce cache managerlib/src/auth/interceptors.ts- Auto-retry on 401 with DPoP-Nonce challengelib/src/auth/oidc.ts- Token endpoint and userinfo nonce handlingVerification & Testing
athclaim on resource callscnf.jktvia existing proof generationxtest Integration
cli/src/cli.ts- Addedsupports dpopcommand for feature detectionopentdf supports dpop→ exit 0 if supportedImplementation Details
Per RFC 9449 §8, the SDK now:
DPoP-Nonceresponse header:Works across:
Related PRs
opentdf/tests#DSPX-3397-kc26-dpop- Integration tests & otdf-local KC26 bumpopentdf/platform#DSPX-3397-platform-service- Platform service DPoP validationopentdf/platform#DSPX-3397-platform-go-sdk- Go SDK DPoP clientopentdf/java-sdk#DSPX-3397-java-sdk- Java SDK DPoP clientTesting
Local builds and lints pass. Integration tests will activate once the tests-cell KC26 bump lands and this PR's CI exposes
supports dpop.🤖 Generated with Claude Code