Skip to content

feat(sdk): comprehensive DPoP nonce handling and verification#939

Draft
dmihalcik-virtru wants to merge 35 commits into
mainfrom
DSPX-3397-web-sdk
Draft

feat(sdk): comprehensive DPoP nonce handling and verification#939
dmihalcik-virtru wants to merge 35 commits into
mainfrom
DSPX-3397-web-sdk

Conversation

@dmihalcik-virtru

Copy link
Copy Markdown
Member

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

Changes

DPoP-Nonce Support (RFC 9449 §8)

  • New: lib/src/auth/dpop-nonce.ts - Per-origin nonce cache manager
  • Updated: lib/src/auth/interceptors.ts - Auto-retry on 401 with DPoP-Nonce challenge
  • Updated: lib/src/auth/oidc.ts - Token endpoint and userinfo nonce handling

Verification & Testing

  • ✅ Existing DPoP implementation already includes ath claim on resource calls
  • ✅ JWK thumbprint flows through to cnf.jkt via existing proof generation
  • ✅ All KAS-facing clients (rewrap, policy, kasregistry) use interceptors with DPoP support

xtest Integration

  • Updated: cli/src/cli.ts - Added supports dpop command for feature detection
    • Usage: opentdf supports dpop → exit 0 if supported
    • Activates integration tests when this PR's CI runs

Implementation Details

Per RFC 9449 §8, the SDK now:

  1. Caches server-issued nonces by origin
  2. Includes cached nonce in DPoP proofs when available
  3. On 401 with DPoP-Nonce response header:
    • Caches the server nonce
    • Regenerates proof with nonce claim
    • Retries request once
  4. Updates nonce cache from successful responses

Works across:

  • Token endpoint requests (client credentials, token exchange, refresh)
  • Resource requests (KAS rewrap, policy queries, kasregistry)
  • gRPC/Connect-RPC interceptors
  • Legacy fetch-based paths

Related PRs

  • opentdf/tests#DSPX-3397-kc26-dpop - Integration tests & otdf-local KC26 bump
  • opentdf/platform#DSPX-3397-platform-service - Platform service DPoP validation
  • opentdf/platform#DSPX-3397-platform-go-sdk - Go SDK DPoP client
  • opentdf/java-sdk#DSPX-3397-java-sdk - Java SDK DPoP client

Testing

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

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4872e03-168e-4070-8ac0-38b63c47014c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch DSPX-3397-web-sdk

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread lib/src/auth/interceptors.ts Outdated
Comment on lines +129 to +131
if (serverNonce && !cachedNonce) {
// Server sent a nonce and we didn't have one cached
// Cache it and retry once

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This condition prevents retrying the request if a cachedNonce already exists. If the cached nonce has expired or been invalidated, the server will reject it with a 401 and return a new DPoP-Nonce. Because of !cachedNonce, the client will fail to retry with the new nonce and throw a 401 error. Changing the condition to serverNonce !== cachedNonce correctly allows retrying when the server provides a new/different nonce, while still preventing infinite loops if the same nonce is repeatedly returned.

Suggested change
if (serverNonce && !cachedNonce) {
// Server sent a nonce and we didn't have one cached
// Cache it and retry once
if (serverNonce && serverNonce !== cachedNonce) {
// Server sent a new nonce (or we didn't have one cached)
// Cache it and retry once

Comment thread lib/src/auth/interceptors.ts Outdated
Comment on lines +126 to +127
const metadata = err.metadata as { get?: (key: string) => string | null };
const serverNonce = metadata.get?.('dpop-nonce');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If err.metadata is null or undefined, attempting to access metadata.get will throw a TypeError (e.g., Cannot read properties of undefined (reading 'get')), which will crash the error handler and mask the original authentication error. Using optional chaining on metadata prevents this potential runtime crash.

Suggested change
const metadata = err.metadata as { get?: (key: string) => string | null };
const serverNonce = metadata.get?.('dpop-nonce');
const metadata = err.metadata as { get?: (key: string) => string | null } | undefined;
const serverNonce = metadata?.get?.('dpop-nonce');

Comment thread cli/src/cli.ts Outdated
Comment on lines +523 to +531
async (argv) => {
const feature = argv.feature as string;
if (feature === 'dpop') {
// DPoP is supported - exit 0
process.exit(0);
}
// Unknown feature - exit 1
process.exit(1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Calling process.exit directly inside a CLI command handler is discouraged. It prevents graceful cleanup/shutdown, makes unit testing the command extremely difficult (as it terminates the test runner), and is redundant here. Since choices: ['dpop'] is defined on the positional argument, yargs automatically validates the input and will fail with a non-zero exit code if any other feature is requested. If 'dpop' is passed, the handler can simply return successfully to exit with code 0 naturally.

        async () => {
          // If we reached here, yargs validation passed and the feature is supported.
        }

Comment thread lib/src/auth/oidc.ts Outdated
Comment on lines +210 to +212
if (this.config.dpopEnabled && response.status === 401) {
const responseNonce = DPoPNonceCache.extractNonce(response.headers);
if (responseNonce) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the server returns a 401 with the exact same nonce that we just sent, retrying the request will inevitably fail again with another 401. To avoid redundant and wasteful HTTP requests, we should verify that the returned responseNonce is different from the cachedNonce we used in the initial request before attempting a retry.

Suggested change
if (this.config.dpopEnabled && response.status === 401) {
const responseNonce = DPoPNonceCache.extractNonce(response.headers);
if (responseNonce) {
if (this.config.dpopEnabled && response.status === 401) {
const responseNonce = DPoPNonceCache.extractNonce(response.headers);
const cachedNonce = globalNonceCache.get(origin);
if (responseNonce && responseNonce !== cachedNonce) {

Comment thread lib/src/auth/dpop-nonce.ts Outdated
Comment on lines +34 to +37
static extractNonce(headers: Headers): string | undefined {
// Headers.get() is case-insensitive per spec
return headers.get('dpop-nonce') || undefined;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To enforce defensive programming and prevent potential runtime crashes, we should guard against cases where headers is null, undefined, or does not implement the standard Headers interface (which can easily happen in test environments with mocked responses or custom fetch implementations). Checking if headers?.get is a function before calling it makes this utility much more robust.

  static extractNonce(headers?: Headers): string | undefined {
    return typeof headers?.get === 'function' ? headers.get('dpop-nonce') || undefined : undefined;
  }

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@dmihalcik-virtru dmihalcik-virtru changed the title feat(web-sdk): comprehensive DPoP nonce handling and verification (DSPX-3397) feat(sdk): comprehensive DPoP nonce handling and verification Jun 10, 2026
@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

@github-actions

Copy link
Copy Markdown

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

dmihalcik-virtru and others added 10 commits June 24, 2026 13:31
…PX-3397)

Implements RFC 9449 DPoP-Nonce support across SDK and CLI:

- Add DPoP-Nonce cache manager (dpop-nonce.ts) with per-origin nonce storage
- Update authTokenDPoPInterceptor with automatic nonce retry on 401 challenges
- Extend OIDC token endpoint and userinfo flows to handle nonce caching/refresh
- Add 'supports dpop' CLI command for xtest integration testing detection
- Refresh cached nonces from successful response headers per RFC 9449 §8

All DPoP proofs now include cached nonces when available and automatically
retry with server-provided nonces on 401 use_dpop_nonce errors.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…3397)

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…DSPX-3397)

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…nsive handling (DSPX-3397)

- interceptors.ts: use serverNonce !== cachedNonce to allow retry on nonce rotation, not just first nonce
- interceptors.ts: optional-chain err.metadata to prevent TypeError crash on absent metadata
- dpop-nonce.ts: guard extractNonce against null/non-standard headers (test-env robustness)
- oidc.ts: hoist cachedNonce outside dpopEnabled block; skip retry when server returns same nonce
- cli.ts: remove redundant process.exit(0) from supports command handler; yargs choices handles validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…ogic (DSPX-3397)

- dpop-nonce.ts: add clearAll() to DPoPNonceCache for test teardown
- server.ts: add /protocol/openid-connect/token endpoint that issues a
  DPoP-Nonce challenge (fixed nonce 'dpop-test-nonce-abc') when the
  incoming DPoP proof has no nonce, accepts on retry with correct nonce
- tests/web/auth/dpop-nonce.test.ts: WTR unit tests covering doPost()
  nonce retry (via mock fetch) and authTokenDPoPInterceptor nonce retry
  (via mock next), including no-retry-on-same-nonce regression cases
- tests/mocha/dpop-nonce.spec.ts: Mocha integration tests hitting the
  real server — verifies transparent retry, nonce cache population, and
  pre-cached nonce path

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru and others added 21 commits June 24, 2026 13:31
…n roundtrip tests (DSPX-3397)

Upgrade the roundtrip CI Keycloak to 26.2, enable admin-fine-grained-authz:v1, drop the
keycloakdb dependency (KC 26.2 uses embedded H2 in dev mode), require dpop.bound.access.tokens
on existing clients (opentdf-sdk, testclient), and add --dpop to the CLI encrypt/decrypt
invocations so both playwright and CLI roundtrip tests exercise the full nonce challenge/retry
path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…ompatibility (DSPX-3397)

x-virtrupubkey was missing from Access-Control-Allow-Headers, causing Chrome
to block the CORS preflight for DPoP-enabled requests. DPoP-Nonce was missing
from Access-Control-Expose-Headers, preventing browser JS from reading the
nonce challenge on 401 responses. Node.js fetch ignores CORS so mocha tests
passed; Chrome Headless tests failed with TypeError: Failed to fetch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dev-local.sh starts the web-app dev server pointed at a local otdf-local
instance (PLATFORM_URL/KC_URL/KC_CLIENT_ID overrideable via env).
rebuild-local-lib.sh builds lib/ from source and installs it into web-app,
replacing the published @opentdf/sdk with the local build.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Adapts .github/workflows/roundtrip/config-demo-idp.sh for local otdf-local
use: creates the browsertest public KC client with dpop.bound.access.tokens
enforced and an audience mapper pointing at PLATFORM_URL, then execs
dev-local.sh to start the Vite dev server. No kcadm install required —
uses the KC admin REST API directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Matches the user creation step from config-demo-idp.sh that was omitted
in the initial port. user1 / testuser123 is the expected login for the
browser demo app.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Browser requests to Keycloak must go through the Vite /auth proxy
(localhost:65432/auth → localhost:8888) rather than hitting port 8888
directly. Replaced the hard-coded KC_URL in VITE_TDF_CFG with APP_URL
so the oidc.host always matches the app origin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Both OIDC and KAS endpoints now use APP_URL (localhost:65432) so all
browser requests go through Vite's reverse proxy rather than hitting
ports 8888 or 8080 directly. Mirrors how the roundtrip scripts pass
--kasEndpoint http://localhost:65432/kas.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Per RFC 9449 §7.1, DPoP-bound access tokens (cnf.jkt claim present)
MUST be presented to resource servers under the "DPoP" Authorization
scheme, not "Bearer". Three lib paths (oidc.ts withCreds and info,
interceptors.ts authTokenDPoPInterceptor) and the web-app sample's
OidcClient.withCreds were sending Bearer alongside the DPoP proof
header — silently accepted by lenient enforcers, but rejected by
spec-compliant ones once enforcement is enabled.

Non-DPoP paths continue to send Bearer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Intercepts the OIDC token POST and the KAS Rewrap POST, asserts the
DPoP header is present, and surfaces the captured Authorization scheme
(DPoP vs Bearer) for visual inspection. Serves as a regression guard
for the RFC 9449 §7.1 scheme requirement.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- doPost: trigger nonce retry on any non-OK response carrying a fresh
  DPoP-Nonce header. The previous status==401 gate never fired against
  spec-compliant authorization servers (Keycloak 26.2 included), which
  return HTTP 400 with error=use_dpop_nonce per RFC 9449 §8.
- info: fix htm claim mismatch — the proof was generated with 'POST' but
  the userinfo request is GET, violating RFC 9449 §4.2.
- info: add the missing single-shot nonce retry mirroring doPost, so the
  first userinfo request against a nonce-enforcing resource server
  succeeds instead of bubbling up as a spurious token-renewal cycle.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- loadDPoPKeyPairFromPem: narrow the curve-detection catches so that
  only crypto.subtle.importKey failures are swallowed. SDK-layer
  errors from buildKeyPairFromCryptoKey now propagate with full context
  instead of producing the misleading 'expected PKCS8 PEM' message.
- loadDPoPKeyPairFromPem: wrap atob() in try/catch so malformed PEM
  content surfaces as a descriptive CLIError instead of a raw
  DOMException.
- derToPem: replace btoa(String.fromCharCode(...bytes)) with
  Buffer.from(bytes).toString('base64') — matches the safer pattern
  already used in lib/src/auth/dpop.ts and avoids spread-on-large-array
  call-stack risk.
- requireImportPrivateKey: guarded accessor that fails loudly with a
  CLIError if the optional WebCryptoService.importPrivateKey method is
  missing, replacing the silent ! non-null assertions.
- resolveDPoPFromArgs: new exported helper that encapsulates the
  --dpop / --dpopKey three-way argv parsing (bare --dpop defaults to
  ES256). Lets cli.ts dedupe and unit tests exercise the helper
  without triggering yargs side effects.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…SPX-3397)

- interceptors.ts: replace the duck-typed metadata cast with a real
  'instanceof ConnectError && err.code === Code.Unauthenticated' check.
  ConnectError.metadata is typed as Headers so .get() is non-optional,
  and instanceof catches actual production errors rather than a
  hand-rolled shape.
- dpop-nonce.test.ts: update the rejection stubs to throw real
  ConnectError instances (matches what Connect actually surfaces to
  interceptors in production; the prior plain objects only worked
  because of the duck-typing that we just removed).
- cli.ts: replace the stale eslint-disable + _resolveDPoPKeyPair
  alias with a clean import. Replace the copy-pasted three-line DPoP
  resolution block in the encrypt and decrypt handlers with the new
  resolveDPoPFromArgs helper.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… (DSPX-3397)

Adds the missing coverage flagged during PR review:

- loadDPoPKeyPairFromPem: round-trip tests for P-256, P-384, P-521,
  and RSA-2048 PEMs (generated in-memory via WebCrypto + derToPem),
  plus error paths for missing file, invalid base64, and valid base64
  whose bytes are not a recognized key type.
- resolveDPoPKeyPair: keyPath-only and keyPath-overrides-alg branches
  (previously the keyPath path was completely uncovered).
- resolveDPoPFromArgs: full argv matrix — no flags, bare --dpop
  defaults to ES256, explicit algorithm, --dpopKey alone, and the
  CLIError propagation for an unknown algorithm.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Keycloak clients with dpop_bound_access_tokens=true reject the
client_credentials POST /token unless it carries a DPoP proof header
(RFC 9449 §5). The SDK was minting the proof only for KAS calls — the
initial token exchange went out bare and Keycloak responded with
400 invalid_request 'DPoP proof is missing'.

Two wiring gaps caused this:

1. AccessToken.refreshTokenClaimsWithClientPubkeyIfNeeded set
   this.signingKey but left config.dpopEnabled false, so doPost
   skipped the DPoP branch even after the key was bound. Now it also
   flips dpopEnabled and clears the cached token (a pre-binding token
   would lack cnf.jkt and immediately fail downstream KAS calls).

2. CLI processAuth performed a warm-up oidcAuth.get() before
   OpenTDF constructed, so the first token fetch pre-dated key
   binding regardless of (1). processAuth now accepts the resolved
   dpopKeyPair and binds it via updateClientPublicKey before the
   warm-up call; encrypt/decrypt handlers resolve DPoP first.

Existing nonce-retry path in doPost (RFC 9449 §8) is unchanged and
exercised by the new regression test. Proof is minted without ath
on the token endpoint; ath remains scoped to resource requests.

Regression test in dpop-nonce.spec.ts drives the full provider path
(clientSecretAuthProvider -> updateClientPublicKey -> get) and
reproduces the bug's exact 400 error when the fix is reverted.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…DSPX-3397)

e693605 enabled DPoP unconditionally inside
AccessToken.refreshTokenClaimsWithClientPubkeyIfNeeded. But
TDF3Client.createSessionKeys always calls updateClientPublicKey —
even for non-DPoP flows — because the same key is reused for TDF
body signing. The unconditional flip therefore turned DPoP on for
every CLI invocation, including 'opentdf:secret' (non-DPoP client).
Keycloak then issued a DPoP-bound token (cnf.jkt set), but the
SDK's Connect-RPC interceptors still presented it as plain Bearer
to the platform, producing 401 on /key-access-servers and breaking
test_legacy.py::test_decrypt_* in xtest.

Plumb dpopEnabled and signingKey through clientSecretAuthProvider,
refreshAuthProvider, externalAuthProvider and their provider
classes into the AccessToken constructor (the AccessToken type
already accepted these fields). The CLI now passes them at
construction time so DPoP is on iff --dpop was requested.

refreshTokenClaimsWithClientPubkeyIfNeeded no longer flips
dpopEnabled. It still drops the cached token when DPoP is on
(rotating the key invalidates cnf.jkt) but leaves cached non-DPoP
Bearer tokens alone — they're key-independent.

dpop-nonce.spec.ts: updated the DPoP-path test to use the new
config-time wiring, and added a non-DPoP test asserting that
updateClientPublicKey (the call TDF3Client.createSessionKeys makes
unconditionally) does NOT flip dpopEnabled. Verified it fails on
e693605 and passes here.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… verifier (DSPX-3397)

Keycloak rejected our DPoP proofs with TokenSignatureInvalidException
(Invalid token signature) because cryptoService.sign re-encodes
ECDSA signatures in DER, while RFC 7518 §3.4 (the JWS spec DPoP
inherits via RFC 9449) mandates raw R||S concatenation. crypto.subtle
already returns that raw form; the DER re-encode in signing.ts:189-192
is the bug.

Scope this change to DPoP only:
- Export derToIeeeP1363 from lib/tdf3/src/crypto/core/signing.ts.
- In lib/src/auth/dpop.ts, convert DER → raw at the JWS call site for
  ES* algs before base64url-encoding. RSA/EdDSA pass through unchanged.
- Do NOT touch cryptoService.sign/verify or jwt.ts::signJwt — those
  back TDF assertion signing, and flipping their format would break
  reading existing ES256-signed assertions from older SDK versions.
  Tracked separately as DSPX-3634.

Harden the mock test server so this regression is caught locally:
- lib/tests/server.ts token endpoint: replace jose.decodeJwt with a
  full RFC 9449 verifier — decodeProtectedHeader (typ/alg/jwk no-priv
  checks), importJWK + jwtVerify (catches DER), htm/htu/iat/jti claim
  validation. Fix existing wrong 401 → 400 on use_dpop_nonce per §8.
- Add a resource-server DPoP block to the rewrap handler: ath +
  cnf.jkt binding (Map<accessToken, jkt> tracked across token mint
  and rewrap). 401 + WWW-Authenticate: DPoP for nonce challenge.

New regression tests in lib/tests/mocha/dpop-proof.spec.ts:
- ES256/ES384/ES512 round-trip proofs minted by dpopFn through
  jose.jwtVerify (the verifier inside real Keycloak). The pre-fix
  dpop.js produces DER and fails all three with
  JWSSignatureVerificationFailed — verified by adversarial revert.
- Negative: flipped signature byte → rejected.
- Negative: forged proof with swapped jwk header → rejected.

Verification: lib 346 mocha pass (+9 new); cli 25 pass; assertion and
crypto-service unit tests unaffected (Part A scoped only to DPoP).

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
The authProviderInterceptor handed withCreds only the URL pathname. A
DPoP-enabled AuthProvider computes the proof's htu claim and the nonce
cache origin via new URL(req.url), which throws "Invalid URL" on a bare
path. This surfaced as a CRITICAL [GetAttributeValuesByFqns] [unknown]
Invalid URL during encrypt, and as the masked v2 request error in the
KAS-list fallback during decrypt.

Pass the absolute req.url instead. Non-DPoP providers ignore the URL
(they only add a Bearer header), so legacy AuthProviders are unaffected.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…om htu (DSPX-3397)

The legacy resource-server fetch helpers (fetchKeyAccessServers,
fetchWrappedKey) signed once via AuthProvider.withCreds and fetched once,
with no DPoP-Nonce challenge handling. On the first request to a platform
origin there is no cached nonce, so the server replies 401 +
use_dpop_nonce + DPoP-Nonce and the helper gives up — the
"unable to fetch kas list ... status: 401" seen during decrypt.

Add fetchWithCredsAndNonceRetry: on a non-ok response carrying a fresh
DPoP-Nonce, cache it by origin and retry once so withCreds can mint a
proof bound to it. Non-DPoP providers/servers never emit DPoP-Nonce, so
they keep the single-request path (legacy users unaffected).

Also fix AccessToken.withCreds to strip query/fragment from the proof's
htu claim per RFC 9449 §4.2; the kas-list URL carries
?pagination.offset=0, which an RFC-conformant verifier rejects.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Comment thread lib/tests/server.ts Fixed
… path (DSPX-3397)

The authProviderInterceptor (used by the CLI's --auth opentdf-dpop) lacked
DPoP-Nonce challenge retry on the Connect-RPC path, so a server nonce
challenge on ListKeyAccessServers failed instead of retrying. This reached
xtest because the mock server never challenged that endpoint.

- interceptors.ts: add nonce-challenge retry to authProviderInterceptor,
  mirroring authTokenDPoPInterceptor and the legacy fetch path.
- tests/server.ts: add shared enforceRsDpop() gate (gated on
  Authorization: DPoP) and wire it into ListKeyAccessServers,
  GetAttributeValuesByFqns, ListAttributes; refactor the rewrap DPoP
  block to use it and return Connect-correct {code,message} 401s.
- add node + browser regression tests driving PlatformClient through a
  DPoP provider so the RPC nonce-retry path is exercised end to end.
Comment thread lib/tests/server.ts
*/
async function enforceRsDpop(req: IncomingMessage, res: ServerResponse): Promise<boolean> {
const authHeader = (req.headers['authorization'] as string | undefined) ?? '';
const dpopMatch = /^DPoP\s+(.+)$/.exec(authHeader);
…SPX-3397)

The KAS rewrap request token was always signed with RS256, so an EC dpop
key (e.g. --dpop ES256) made WebCrypto throw 'Unable to use this key to
sign', surfacing as 'unable to unwrap key from kas'. Derive the JWS alg
from the dpop private key's algorithm instead.

Adds a regression test decrypting with EC dpop keys.
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants