feat(sdk): add comprehensive DPoP (RFC 9449) support (DSPX-3397)#374
feat(sdk): add comprehensive DPoP (RFC 9449) support (DSPX-3397)#374dmihalcik-virtru wants to merge 43 commits into
Conversation
📝 WalkthroughWalkthroughThe CLI adds feature-support reporting, verbose diagnostics, and explicit credential validation. The SDK adds configurable DPoP keys and algorithms, nonce-aware token handling, bearer fallback, interceptor retries, and expanded tests for authentication flows. ChangesCLI behavior and build setup
SDK DPoP authentication
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SDKBuilder
participant TokenSource
participant AuthInterceptor
participant ResourceServer
SDKBuilder->>TokenSource: configure DPoP credentials
AuthInterceptor->>TokenSource: request authorization headers
TokenSource-->>AuthInterceptor: Authorization and optional DPoP
AuthInterceptor->>ResourceServer: send authenticated request
ResourceServer-->>AuthInterceptor: 401 use_dpop_nonce with nonce
AuthInterceptor->>TokenSource: cache nonce and refresh headers
AuthInterceptor->>ResourceServer: retry request once
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 support for custom DPoP keys in the SDKBuilder and implements DPoP nonce caching in TokenSource and AuthInterceptor to handle server-issued nonces. It also adds a new "supports" subcommand to the CLI to check for feature support (e.g., "dpop"). Regarding the feedback, the "supports" subcommand should avoid calling System.exit() directly, as this abruptly terminates the JVM and hinders testing; instead, it should implement Callable and return the exit code.
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.
5242c69 to
00414f1
Compare
X-Test Failure Report |
X-Test Results✅ java@pull-374-main |
dmihalcik-virtru
changed the title
dmihalcik-virtru
changed the title
dmihalcik-virtru
changed the title
f8baeab to
244f4ba
Compare
X-Test Failure Report |
X-Test Failure Report |
95ce0d0 to
b8d21d1
Compare
b8d21d1 to
28510cb
Compare
- Add server-issued nonce caching infrastructure in TokenSource with per-origin storage - Add dpopKey() method to SDKBuilder for caller-supplied RSA keys (defaults to auto-generated ephemeral key) - Update AuthInterceptor to cache DPoP-Nonce from successful responses - Add 'supports dpop' CLI command for xtest feature detection - Extend TokenSource.getAuthHeaders() to accept optional nonce parameter for proof generation Implementation uses Nimbus OAuth2 SDK's DefaultDPoPProofFactory for RFC 9449 compliant DPoP proof generation with htm/htu/iat/jti claims (plus ath for resource endpoints). Current implementation uses RSA-2048/RS256 for DPoP keys. The SDK already had DPoP proof generation via Nimbus OAuth2 SDK; this PR adds nonce support infrastructure and makes the DPoP key configurable. Note: Full 401 retry logic with nonce challenges requires Connect RPC interceptor changes and is deferred to future work. Nonce caching infrastructure is in place for when retry logic is added. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- AuthInterceptor.kt: fix resp.code→resp.status and resp.message.request() compilation errors; use ThreadLocal<URL> to thread request URL into responseFunction for nonce caching; change private→internal so SDKBuilder.java can access dpopRetryInterceptor(); add dpopRetryInterceptor() OkHttp interceptor that caches DPoP-Nonce and retries 401 once - TokenSource.java: wrap nonce String as new Nonce(nonce) to match DefaultDPoPProofFactory.createDPoPJWT signature; generalize RSAKey to JWK+JWSAlgorithm to support EC keys for ES256/ES384/ES512 - SDKBuilder.java: update to JWK+JWSAlgorithm, separate SRT key from DPoP key (EC DPoP key auto-generates RSA for SRT), wire dpopRetryInterceptor into OkHttpClient for KAS and all platform services; add dpopAlgorithm() builder method - Add TokenSourceTest and DPoPRetryInterceptorTest (6 new tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Add DPoP configuration flags to the tdf cmdline tool (shared by encrypt and decrypt via buildSDK()): - --dpop[=<alg>]: enable DPoP; optional algorithm (RS256, RS384, RS512, ES256, ES384, ES512); defaults to RS256; generates ephemeral key - --dpop-key <path>: use PEM-encoded private key from file; algorithm inferred from key type (EC or RSA); combinable with --dpop=<alg> Both flags work for encrypt and decrypt subcommands. Help text contains "dpop" so the grep probe matches: encrypt --help | grep -i dpop. 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>
…ithm
Add a retry test asserting that when the token refreshed on a DPoP nonce
challenge comes back as a plain Bearer token, the retried request sends
Authorization: Bearer with no DPoP header (guards AuthInterceptor's
removeHeader("DPoP")). Add a TokenSource test that an explicit RS512
override reaches the proof factory instead of defaulting to RS256.
Clarify coverage-boundary and pre-network-failure comments in
SDKBuilderTest and CommandTest.
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
f454394 to
c05ceef
Compare
The SDK already builds a DPoP-capable key by default without any CLI input (SDKBuilder auto-generates a key when none is configured), and capability detection is already exposed via `tdf supports dpop` / `tdf supports dpop_nonce_challenge`. Mirrors the go-sdk direction in opentdf/platform#3581, which dropped otdfctl's --dpop/--dpop-key flags in favor of always-on DPoP plus capability detection. Removes CliDpopOptions (and its test) and the --dpop/--dpop-key options from Command.java.
c05ceef to
6e13d27
Compare
Drops orphaned imports that SonarCloud (java:S1128) reported: - SDKBuilder: com.nimbusds.jose.jwk.Curve, com.nimbusds.jose.jwk.gen.ECKeyGenerator - TokenSource: com.nimbusds.jose.jwk.RSAKey
f975143 to
1b2ea46
Compare
`tdf supports` with no argument now prints the supported feature names,
one per line. A new --json flag emits a JSON object mapping feature name
to a boolean (e.g. {"dpop":true}); with no argument it lists all
supported features as true. Exit codes are unchanged: 0 when a queried
feature is supported, 1 otherwise.
1b2ea46 to
6ae85b2
Compare
## Problem The java-sdk removed the user-facing `--dpop` / `--dpop-key` CLI flags in opentdf/java-sdk#374. DPoP is now always-on by default, and capability is exposed through the `supports` subcommand instead. The Java wrapper's `supports dpop` case still detects DPoP by grepping `help encrypt` for the `--dpop` flag: ```bash dpop) set -o pipefail java -jar "$SCRIPT_DIR"/cmdline.jar help encrypt | grep -iE -- '--dpop' exit $? ;; ``` With the flags gone this grep finds nothing, exits 1, and xtest skips java DPoP tests: ``` SKIPPED [2] tdfs.py:619: java@... sdk doesn't yet support [dpop] ``` The sibling `dpop_nonce_challenge` case already delegates to the `supports` subcommand and is unaffected. ## Fix Detect `dpop` the same way `dpop_nonce_challenge` already does — via the `supports` subcommand, whose exit code is the capability contract (0 = supported, 1 = not): ```bash dpop) java -jar "$SCRIPT_DIR"/cmdline.jar supports dpop exit $? ;; ``` `set -o pipefail` is dropped since there's no longer a pipe. ## Verification With a current java `cmdline.jar` installed: `./cli.sh supports dpop; echo $?` prints `0`, and `dpop_nonce_challenge` still returns `0`. Only `xtest/sdk/java/cli.sh` changes; other SDK wrappers are untouched. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved detection of DPoP support in the Java CLI feature probe. * Prevents incorrect results caused by relying on help output and flag matching. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
There was a problem hiding this comment.
Pull request overview
Adds end-to-end DPoP (RFC 9449) support to the Java SDK (proof minting, nonce caching, and single-retry nonce challenges) alongside CLI changes to make DPoP always-on for authenticated usage and to provide stable feature detection via a supports subcommand.
Changes:
- Implement DPoP proof generation and Bearer fallback in
TokenSource, including per-origin nonce caching and token-endpoint retry onuse_dpop_nonce. - Add OkHttp-level DPoP nonce-challenge retry and Connect unary-path nonce caching in
AuthInterceptor, and disable Connect-GET for authenticated clients to preserve thehtmbinding. - Update CLI behavior: introduce
tdf supports(plain / JSON), add--verboseplumbing, adjust logging defaults, and expand test coverage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/src/test/kotlin/io/opentdf/platform/sdk/AuthInterceptorConnectPathTest.kt | New Kotlin tests covering Connect unary ThreadLocal URL handoff + nonce caching behavior. |
| sdk/src/test/java/io/opentdf/platform/sdk/TokenSourceTest.java | New unit tests covering nonce caching, htu normalization, token endpoint retry, key/alg validation, and Bearer fallback. |
| sdk/src/test/java/io/opentdf/platform/sdk/SDKBuilderTest.java | Adds tests for EC DPoP key behavior (separate RSA SRT signer) and missing platform_issuer failure when DPoP is explicitly requested. |
| sdk/src/test/java/io/opentdf/platform/sdk/DPoPRetryInterceptorTest.java | New tests for OkHttp 401 nonce-challenge retry, single-retry guarantee, concurrency, and Bearer downgrade behavior. |
| sdk/src/main/kotlin/io/opentdf/platform/sdk/AuthInterceptor.kt | Adds Connect unary response nonce caching + OkHttp DPoP nonce retry interceptor and debug logging summaries. |
| sdk/src/main/java/io/opentdf/platform/sdk/TokenSource.java | Introduces DPoP/Bearer scheme snapshotting, JWK-based DPoP key support, nonce cache, token endpoint retry, and improved error shaping. |
| sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java | Adds DPoP key/algorithm configuration, resolves EC-vs-RSA DPoP/SRT key material, disables Connect-GET for authenticated requests, wires in retry interceptor. |
| sdk/src/main/java/io/opentdf/platform/sdk/DpopKeyValidation.java | New centralized validation for DPoP JWK + algorithm compatibility (RSA/EC) and EC curve→alg inference. |
| sdk/pom.xml | Ensures Kotlin test sources are included in test compilation. |
| cmdline/src/test/java/io/opentdf/platform/CommandTest.java | New CLI tests for supports behavior, JSON output, and ensuring supports does not require credentials. |
| cmdline/src/main/resources/log4j2.xml | Lowers default CLI logging verbosity (root from trace → info). |
| cmdline/src/main/java/io/opentdf/platform/TDF.java | Adds execution exception handler and verbose stack-trace behavior for the CLI entrypoint. |
| cmdline/src/main/java/io/opentdf/platform/Command.java | Adds supports subcommand, makes credentials options non-required (enforced in buildSDK()), adds --verbose, and refactors some parsing. |
| cmdline/pom.xml | Adds runtime BouncyCastle dependency and JUnit/AssertJ test dependencies. |
| AGENTS.md | Documents local JAVA_HOME setup guidance for the agent environment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SDKBuilder: extract DPoP/SRT key resolution out of buildServices into a resolveDpopMaterial() helper + DpopMaterial holder, cutting the method's cognitive complexity below the threshold (S3776). No behavior change. - Tests: drop a redundant 'public' modifier (S5786) and two unthrowable 'throws Exception' declarations (S1130), document the intentionally empty catch (S108), and hoist URL/URI construction out of five assertThatThrownBy lambdas so each has a single throwing call (S5778).
- TokenSource: cache the DPoP-Nonce from every settled token response, not just the use_dpop_nonce retry path, so a nonce the AS rotates on a successful (200) response is honored on the next request (RFC 9449 §8.2). Collapses the duplicate retry-path caching into one call. - cmdline supports --json: emit the canonical feature name for recognized features (echo raw input only for unknown ones) so output is stable for automation. - cmdline/pom.xml: drop the bouncycastle bcpkix dependency this PR added; it was orphaned when the DPoP key CLI flags (its only caller) were removed. - Tests: cover nonce caching on a successful response and canonical-name JSON output. (Also carries the TokenSourceTest SonarCloud test-hygiene edits.)
6ae85b2 to
f16063b
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cmdline/src/main/java/io/opentdf/platform/Command.java (1)
384-385: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
Files.readStringfor reading text files.Since the SDK targets Java 11, you can use
Files.readString(Path)instead ofnew String(Files.readAllBytes(Path)). The latter relies on the platform's default charset, whileFiles.readStringexplicitly defaults toUTF-8and is more idiomatic.♻️ Proposed refactor
- String fileJson = new String(Files.readAllBytes(Paths.get(assertionVerificationInput))); + String fileJson = Files.readString(Paths.get(assertionVerificationInput));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmdline/src/main/java/io/opentdf/platform/Command.java` around lines 384 - 385, Update the file-reading logic that supplies fileJson to gson.fromJson in Command so it uses Files.readString(Path) instead of constructing a String from Files.readAllBytes. Preserve UTF-8 text decoding and the existing Config.AssertionVerificationKeys deserialization behavior.sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java (1)
351-357: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate default-algorithm logic between
resolveDpopMaterial()andresolveDpopAlgorithm().The auto-generated-key branch reimplements a subset of
resolveDpopAlgorithm's default logic inline instead of delegating to it. Functionally equivalent today (generated key is always RSA), but the two defaulting paths can silently diverge if the default policy changes later.♻️ Proposed fix to reuse the single source of truth
private DpopMaterial resolveDpopMaterial() { if (this.dpopKey == null) { // Auto-generate a single RSA-2048 key used for both DPoP and SRT. RSAKey generated = generateSrtRsaKey("Error generating DPoP key"); - JWSAlgorithm alg = this.dpopAlg != null ? this.dpopAlg : JWSAlgorithm.RS256; - return new DpopMaterial(generated, alg, generated); + return new DpopMaterial(generated, resolveDpopAlgorithm(generated), generated); }Also applies to: 366-371
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java` around lines 351 - 357, Update resolveDpopMaterial() to obtain the algorithm by delegating to resolveDpopAlgorithm() instead of duplicating the dpopAlg-null fallback inline, including the corresponding logic in the alternate branch around the referenced lines. Preserve the generated RSA key handling while ensuring all DPoP algorithm defaulting uses resolveDpopAlgorithm() as the single source of truth.sdk/src/main/kotlin/io/opentdf/platform/sdk/AuthInterceptor.kt (1)
31-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDebug-log arguments are evaluated on every call, not only when DEBUG is enabled.
SLF4J's
logger.debug("...{}", arg)defers message formatting, but Kotlin/Java still evaluates the arguments eagerly before the call.dpopSummary(...)parses the DPoP JWT (SignedJWT.parse) andauthScheme(...)runs on every stream/unary request, every unary/okhttp response, and every retry — regardless of whether DEBUG is actually enabled. In production (INFO/WARN), this is unconditional JWT-parsing overhead on the hot authenticated-request path for no benefit.⚡ Proposed fix (guard with isDebugEnabled)
- logger.debug("DPoP path=stream url={} method=POST authScheme={} {}", - request.url, authScheme(authHeaders.authHeader), dpopSummary(authHeaders.dpopHeader)) + if (logger.isDebugEnabled) { + logger.debug("DPoP path=stream url={} method=POST authScheme={} {}", + request.url, authScheme(authHeaders.authHeader), dpopSummary(authHeaders.dpopHeader)) + }Apply the same
if (logger.isDebugEnabled) { ... }guard at the other three call sites (unary request, okhttp path, okhttp retry).Also applies to: 60-63, 114-117, 133-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/src/main/kotlin/io/opentdf/platform/sdk/AuthInterceptor.kt` around lines 31 - 32, Guard the debug logging call sites in the interceptor—including the shown stream path and the unary request, okhttp response, and okhttp retry paths— with logger.isDebugEnabled before evaluating authScheme(...) and dpopSummary(...). Keep the existing log messages and behavior unchanged when DEBUG is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/src/main/java/io/opentdf/platform/sdk/TokenSource.java`:
- Around line 213-324: Reduce cognitive complexity in getToken() by extracting
request construction/sending into sendTokenRequest(String cachedNonce), the
use_dpop_nonce retry flow into retryWithNonce(String dpopNonce), and settled
success-response field assignment into applyTokenResponse(TokenResponse, URL
tokenEndpointUrl). Keep getToken() as a short orchestration method while
preserving nonce caching, retry behavior, error handling, and token field
updates.
- Around line 241-271: Update the nonce-retry handling in the token request flow
around TokenRequest and tokenResponse so that a DPoP-Nonce returned on the
retried response is cached before the final error is thrown. Preserve the
existing single retry behavior, and ensure the latest nonce from either response
is stored for the next getToken() call.
---
Nitpick comments:
In `@cmdline/src/main/java/io/opentdf/platform/Command.java`:
- Around line 384-385: Update the file-reading logic that supplies fileJson to
gson.fromJson in Command so it uses Files.readString(Path) instead of
constructing a String from Files.readAllBytes. Preserve UTF-8 text decoding and
the existing Config.AssertionVerificationKeys deserialization behavior.
In `@sdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.java`:
- Around line 351-357: Update resolveDpopMaterial() to obtain the algorithm by
delegating to resolveDpopAlgorithm() instead of duplicating the dpopAlg-null
fallback inline, including the corresponding logic in the alternate branch
around the referenced lines. Preserve the generated RSA key handling while
ensuring all DPoP algorithm defaulting uses resolveDpopAlgorithm() as the single
source of truth.
In `@sdk/src/main/kotlin/io/opentdf/platform/sdk/AuthInterceptor.kt`:
- Around line 31-32: Guard the debug logging call sites in the
interceptor—including the shown stream path and the unary request, okhttp
response, and okhttp retry paths— with logger.isDebugEnabled before evaluating
authScheme(...) and dpopSummary(...). Keep the existing log messages and
behavior unchanged when DEBUG is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 975d89e9-d720-42b9-87a1-bd004126e193
📒 Files selected for processing (15)
AGENTS.mdcmdline/pom.xmlcmdline/src/main/java/io/opentdf/platform/Command.javacmdline/src/main/java/io/opentdf/platform/TDF.javacmdline/src/main/resources/log4j2.xmlcmdline/src/test/java/io/opentdf/platform/CommandTest.javasdk/pom.xmlsdk/src/main/java/io/opentdf/platform/sdk/DpopKeyValidation.javasdk/src/main/java/io/opentdf/platform/sdk/SDKBuilder.javasdk/src/main/java/io/opentdf/platform/sdk/TokenSource.javasdk/src/main/kotlin/io/opentdf/platform/sdk/AuthInterceptor.ktsdk/src/test/java/io/opentdf/platform/sdk/DPoPRetryInterceptorTest.javasdk/src/test/java/io/opentdf/platform/sdk/SDKBuilderTest.javasdk/src/test/java/io/opentdf/platform/sdk/TokenSourceTest.javasdk/src/test/kotlin/io/opentdf/platform/sdk/AuthInterceptorConnectPathTest.kt
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import javax.annotation.Nullable; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'javax\.annotation\.Nullable' --type=java -g '!**/generated-sources/**' sdk/src
rg -n 'jsr305|findbugs' sdk/pom.xmlRepository: opentdf/java-sdk
Length of output: 439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'jsr305|findbugs|javax\.annotation|annotation-api' -g 'pom.xml' -g '!**/target/**' .
printf '\n--- sdk/pom.xml ---\n'
cat -n sdk/pom.xml | sed -n '1,260p'
printf '\n--- root pom.xml ---\n'
cat -n pom.xml | sed -n '1,260p'Repository: opentdf/java-sdk
Length of output: 25277
🌐 Web query:
org.apache.tomcat annotations-api javax.annotation.Nullable
💡 Result:
The class Nullable is provided by the Common Annotations API, but its package name depends on the version being used [1][2]. In modern versions of Apache Tomcat (specifically Tomcat 10 and later), the package has migrated from javax.annotation to jakarta.annotation due to the transition from Java EE to Jakarta EE [1][3][4][2]. Therefore, if you are using Tomcat 10, 11, or newer, you should use jakarta.annotation.Nullable [1][3]. For older versions of Tomcat (such as Tomcat 9 and earlier), common nullability annotations like Nullable often originated from the JSR-305 library (often found as com.google.code.findbugs:jsr305) [5][6][7]. While early versions of the Common Annotations API included a javax.annotation package, it was primarily focused on annotations for generated code and resources (like @Generated or @Resource) rather than nullability checks [8][9]. In summary: - For Tomcat 10/11+: Use jakarta.annotation.Nullable [1][3]. - For older environments or legacy dependencies: Nullable is typically found in javax.annotation provided by the JSR-305 library [5][10][6]. If you are seeing errors related to javax.annotation.Nullable in a project migrating to Jakarta EE, you should update your imports to use the jakarta.annotation namespace [1][4].
Citations:
- 1: https://tomcat.apache.org/tomcat-11.0-doc/annotationapi/jakarta/annotation/Nullable.html
- 2: https://tomcat.apache.org/tomcat-10.1-doc/annotationapi/jakarta/annotation/package-summary.html
- 3: https://tomcat.apache.org/tomcat-10.1-doc/annotationapi/jakarta/annotation/Nullable.html
- 4: https://apache.googlesource.com/tomcat/+/9c3816d890999c390d3fc3b74273c044dd777d87/java/jakarta/annotation/Nullable.java
- 5: https://www.javadoc.io/doc/com.google.code.findbugs/jsr305/latest/javax/annotation/Nullable.html
- 6: https://berrueta.net/software/2025/06/07/nullability-annotations.html
- 7: https://github.com/amaembo/jsr-305/blob/master/ri/src/main/java/javax/annotation/Nullable.java
- 8: https://tomcat.apache.org/tomcat-9.0-doc/annotationapi/javax/annotation/package-summary.html
- 9: http://www.cs.umd.edu/%7Epugh/JSR-305.pdf
- 10: https://chromium.googlesource.com/external/jsr-305/+/4e1f9dc708a57e175e76833b0d1b27212b74d0c5/ri/src/main/java/javax/annotation/Nullable.java
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
pom = Path('sdk/pom.xml').read_text()
for pat in [
r'<groupId>org\.apache\.tomcat</groupId>\s*<artifactId>annotations-api</artifactId>',
r'<groupId>javax\.annotation</groupId>\s*<artifactId>javax\.annotation-api</artifactId>',
r'<groupId>com\.google\.code\.findbugs</groupId>\s*<artifactId>jsr305</artifactId>',
]:
print(pat, bool(re.search(pat, pom, re.S)))
PYRepository: opentdf/java-sdk
Length of output: 420
🌐 Web query:
Maven org.apache.tomcat annotations-api Nullable javax.annotation.Nullable
💡 Result:
The dependency org.apache.tomcat:annotations-api (now provided via org.apache.tomcat:tomcat-annotations-api) [1][2] does not contain javax.annotation.Nullable. Historically, the javax.annotation.Nullable annotation was part of the JSR 305 project (commonly accessed via the com.google.code.findbugs:jsr305 artifact), which has been abandoned and is not part of the official Java or Jakarta EE specifications [3][4]. Modern versions of the Apache Tomcat annotations API (starting with Jakarta Annotations 2.1) now provide jakarta.annotation.Nullable within the jakarta.annotation package [5][6][7][8]. If you require nullable annotations, it is recommended to migrate from javax.annotation to the Jakarta-based annotations or use a widely supported library like the Checker Framework [3][9]. Key points: - Maven artifact: Use org.apache.tomcat:tomcat-annotations-api [1]. - Package shift: javax.annotation has been superseded by jakarta.annotation for modern Java environments [10][9]. - Status of @Nullable: javax.annotation.Nullable was never an official Java standard and is considered abandonware [3][4]. The jakarta.annotation.Nullable annotation is the current standard equivalent [3].
Citations:
- 1: https://mvnrepository.com/artifact/org.apache.tomcat/tomcat-annotations-api
- 2: https://mvnrepository.com/artifact/org.apache.tomcat/annotations-api
- 3: https://stackoverflow.com/questions/76630457/what-nullable-to-use-in-java-as-of-2023-jdk21
- 4: https://berrueta.net/software/2025/06/07/nullability-annotations.html
- 5: https://tomcat.apache.org/tomcat-11.0-doc/annotationapi/jakarta/annotation/Nullable.html
- 6: https://tomcat.apache.org/tomcat-11.0-doc/annotationapi/jakarta/annotation/package-summary.html
- 7: https://tomcat.apache.org/tomcat-10.1-doc/annotationapi/jakarta/annotation/package-summary.html
- 8: https://tomcat.apache.org/tomcat-10.1-doc/annotationapi/overview-tree.html
- 9: https://stackoverflow.com/questions/77427878/module-reads-package-javax-annotation-from-both-jsr305-and-java-annotation
- 10: Upgrade Jakarta Annotations API to 2.1.1 spring-projects/spring-boot#32523
🌐 Web query:
"org.apache.tomcat:annotations-api" "javax.annotation.Nullable"
💡 Result:
The annotation @Nullable is provided by the Common Annotations API, but its package namespace has changed in newer versions of Apache Tomcat. In modern versions of Apache Tomcat (Tomcat 10.1 and later, which use Jakarta EE 9+), the Nullable annotation is located in the jakarta.annotation package, not the javax.annotation package [1][2]. Specifically, it is identified as jakarta.annotation.Nullable [1][3]. In older versions of Apache Tomcat (Tomcat 9.0 and earlier, which use Java EE 8 and below), the Common Annotations API used the javax.annotation package [4]. However, the Nullable annotation was not part of the standard javax.annotation package found in those older Tomcat releases [4]. If you are working with a project that requires javax.annotation.Nullable, you are likely looking for a different library, such as JSR-305 (com.google.code.findbugs:jsr305) or the JetBrains annotations library (org.jetbrains:annotations), as these common libraries provide a Nullable annotation within the javax.annotation namespace. Tomcat's annotations-api artifact is specifically intended to provide the Jakarta/Java EE Common Annotations API [5].
Citations:
- 1: https://tomcat.apache.org/tomcat-10.1-doc/annotationapi/jakarta/annotation/Nullable.html
- 2: https://tomcat.apache.org/tomcat-11.0-doc/annotationapi/jakarta/annotation/Nullable.html
- 3: https://apache.googlesource.com/tomcat/+/9c3816d890999c390d3fc3b74273c044dd777d87/java/jakarta/annotation/Nullable.java
- 4: https://tomcat.apache.org/tomcat-9.0-doc/annotationapi/javax/annotation/package-summary.html
- 5: https://tomcat.apache.org/tomcat-11.0-doc/annotationapi/index.html
Add a dependency that provides javax.annotation.Nullable. The only annotation-related dependency in sdk/pom.xml is org.apache.tomcat:annotations-api, and it does not supply this package; add com.google.code.findbugs:jsr305 or switch the import to jakarta.annotation.Nullable so the SDK still compiles.
| private synchronized TokenSnapshot getToken() { | ||
| try { | ||
| // If the token is expired or initially null, get a new token | ||
| if (token == null || isTokenExpired()) { | ||
|
|
||
| logger.trace("The current access token is expired or empty, getting a new one"); | ||
|
|
||
| // Make the token request | ||
| TokenRequest tokenRequest = new TokenRequest(this.tokenEndpointURI, | ||
| clientAuth, authzGrant, null); | ||
| DPoPProofFactory dpopFactory = new DefaultDPoPProofFactory(dpopJwk, dpopAlg); | ||
|
|
||
| // Proactively use any cached nonce for the token endpoint origin (RFC 9449 §8.2) | ||
| URL tokenEndpointUrl = tokenEndpointURI.toURL(); | ||
| String cachedNonce = nonceCache.get(getOrigin(tokenEndpointUrl)); | ||
|
|
||
| TokenRequest tokenRequest = new TokenRequest(this.tokenEndpointURI, clientAuth, authzGrant, null); | ||
| HTTPRequest httpRequest = tokenRequest.toHTTPRequest(); | ||
| if (sslSocketFactory != null) { | ||
| httpRequest.setSSLSocketFactory(sslSocketFactory); | ||
| } | ||
|
|
||
| DPoPProofFactory dpopFactory = new DefaultDPoPProofFactory(rsaKey, JWSAlgorithm.RS256); | ||
|
|
||
| SignedJWT proof = dpopFactory.createDPoPJWT(httpRequest.getMethod().name(), httpRequest.getURI()); | ||
|
|
||
| URI tokenHtu = htuOf(httpRequest.getURI()); | ||
| SignedJWT proof = (cachedNonce != null) | ||
| ? dpopFactory.createDPoPJWT(httpRequest.getMethod().name(), tokenHtu, new Nonce(cachedNonce)) | ||
| : dpopFactory.createDPoPJWT(httpRequest.getMethod().name(), tokenHtu); | ||
| httpRequest.setDPoP(proof); | ||
| TokenResponse tokenResponse; | ||
|
|
||
| HTTPResponse httpResponse = httpRequest.send(); | ||
|
|
||
| tokenResponse = TokenResponse.parse(httpResponse); | ||
| TokenResponse tokenResponse = TokenResponse.parse(httpResponse); | ||
|
|
||
| // RFC 9449 §8.2: if AS requires a nonce, cache it and retry once | ||
| if (!tokenResponse.indicatesSuccess()) { | ||
| ErrorObject error = tokenResponse.toErrorResponse().getErrorObject(); | ||
| throw new SDKException("failure to get token. description = [" + error.getDescription() + "] error code = [" + error.getCode() + "] error uri = [" + error.getURI() + "]"); | ||
| if ("use_dpop_nonce".equals(error.getCode())) { | ||
| String dpopNonce = httpResponse.getHeaderValue("DPoP-Nonce"); | ||
| if (dpopNonce != null) { | ||
| cacheNonce(tokenEndpointUrl, dpopNonce); | ||
| TokenRequest retryRequest = new TokenRequest(tokenEndpointURI, clientAuth, authzGrant, null); | ||
| HTTPRequest retryHttpRequest = retryRequest.toHTTPRequest(); | ||
| if (sslSocketFactory != null) { | ||
| retryHttpRequest.setSSLSocketFactory(sslSocketFactory); | ||
| } | ||
| SignedJWT retryProof = dpopFactory.createDPoPJWT( | ||
| retryHttpRequest.getMethod().name(), | ||
| htuOf(retryHttpRequest.getURI()), | ||
| new Nonce(dpopNonce)); | ||
| retryHttpRequest.setDPoP(retryProof); | ||
| httpResponse = retryHttpRequest.send(); | ||
| tokenResponse = TokenResponse.parse(httpResponse); | ||
| } else { | ||
| logger.warn("token endpoint {} returned use_dpop_nonce but did not supply a DPoP-Nonce response header", | ||
| tokenEndpointURI); | ||
| } | ||
| } | ||
| if (!tokenResponse.indicatesSuccess()) { | ||
| ErrorObject finalError = tokenResponse.toErrorResponse().getErrorObject(); | ||
| throw new SDKException("failure to get token. description = [" + finalError.getDescription() | ||
| + "] error code = [" + finalError.getCode() | ||
| + "] error uri = [" + finalError.getURI() + "]"); | ||
| } | ||
| } | ||
|
|
||
| // RFC 9449 §8.2: the AS may supply/rotate a DPoP-Nonce on any response, | ||
| // including a successful one. Cache the nonce from the final settled response | ||
| // so the next request uses the freshest value (cacheNonce ignores null/empty). | ||
| cacheNonce(tokenEndpointUrl, httpResponse.getHeaderValue("DPoP-Nonce")); | ||
|
|
||
| var tokens = tokenResponse.toSuccessResponse().getTokens(); | ||
| if (tokens.getDPoPAccessToken() != null) { | ||
| boolean asAssertsDpop = tokens.getDPoPAccessToken() != null; | ||
| if (asAssertsDpop) { | ||
| logger.trace("retrieved a new DPoP access token"); | ||
| } else if (tokens.getAccessToken() != null) { | ||
| logger.trace("retrieved a new access token"); | ||
| } else { | ||
| logger.trace("got an access token of unknown type"); | ||
| logger.warn("token endpoint {} returned a success response with an unknown access token type", | ||
| tokenEndpointURI); | ||
| } | ||
|
|
||
| this.token = tokens.getAccessToken(); | ||
| if (this.token == null) { | ||
| throw new SDKException("token endpoint " + tokenEndpointURI | ||
| + " returned a success response with no access token"); | ||
| } | ||
| this.tokenScheme = asAssertsDpop ? TokenScheme.DPOP : TokenScheme.BEARER; | ||
| if (!asAssertsDpop) { | ||
| logger.warn("token endpoint {} returned a non-DPoP-bound access token (token_type=Bearer) despite" | ||
| + " DPoP proof — falling back to Bearer scheme. Check the IdP DPoP configuration.", | ||
| tokenEndpointURI); | ||
| } | ||
|
|
||
| if (token.getLifetime() != 0) { | ||
| // Need some type of leeway but not sure whats best | ||
| this.tokenExpiryTime = Instant.now().plusSeconds(token.getLifetime() / 3); | ||
| } | ||
|
|
||
| } else { | ||
| // If the token is still valid or not initially null, return the cached token | ||
| return this.token; | ||
| return new TokenSnapshot(this.token, this.tokenScheme); | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
| throw new SDKException("failed to get token", e); | ||
| } catch (SDKException e) { | ||
| // Already shaped for the caller — don't double-wrap. | ||
| throw e; | ||
| } catch (MalformedURLException e) { | ||
| throw new SDKException("invalid token endpoint URL: " + tokenEndpointURI, e); | ||
| } catch (IOException e) { | ||
| throw new SDKException("network error contacting token endpoint " + tokenEndpointURI, e); | ||
| } catch (JOSEException e) { | ||
| throw new SDKException("DPoP proof generation failed for token endpoint " + tokenEndpointURI, e); | ||
| } catch (com.nimbusds.oauth2.sdk.ParseException e) { | ||
| throw new SDKException("malformed token response from " + tokenEndpointURI, e); | ||
| } catch (RuntimeException e) { | ||
| throw new SDKException("unexpected error fetching token from " + tokenEndpointURI, e); | ||
| } | ||
| return this.token; | ||
| return new TokenSnapshot(this.token, this.tokenScheme); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
getToken() trips SonarCloud's Cognitive Complexity gate (43 vs 15 allowed).
Static analysis flags this as a failure (LOC 92→64, complexity 16→14, cognitive complexity 43→15, nesting 6→2), which can block the quality gate. The retry/nonce logic, request building, and response-to-field application are all inlined into one method. Consider extracting:
sendTokenRequest(String cachedNonce)— build+send the initial request.retryWithNonce(String dpopNonce)— theuse_dpop_nonceretry path.applyTokenResponse(TokenResponse, URL tokenEndpointUrl)— assigntoken/tokenScheme/tokenExpiryTimefrom a settled success response.
This keeps getToken() as a short orchestrator and each helper independently testable.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 236-236: SMTP server identity must be enforced
Context: httpRequest.send()
Note: [CWE-297] Improper Validation of Certificate with Host Mismatch.
(smtp-insecure-connection)
[warning] 257-257: SMTP server identity must be enforced
Context: retryHttpRequest.send()
Note: [CWE-297] Improper Validation of Certificate with Host Mismatch.
(smtp-insecure-connection)
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 213-213: A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 92 to 64, Complexity from 16 to 14, Nesting Level from 6 to 2, Number of Variables from 23 to 6.
[failure] 213-213: Refactor this method to reduce its Cognitive Complexity from 43 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/src/main/java/io/opentdf/platform/sdk/TokenSource.java` around lines 213
- 324, Reduce cognitive complexity in getToken() by extracting request
construction/sending into sendTokenRequest(String cachedNonce), the
use_dpop_nonce retry flow into retryWithNonce(String dpopNonce), and settled
success-response field assignment into applyTokenResponse(TokenResponse, URL
tokenEndpointUrl). Keep getToken() as a short orchestration method while
preserving nonce caching, retry behavior, error handling, and token field
updates.
Source: Linters/SAST tools
| // RFC 9449 §8.2: if AS requires a nonce, cache it and retry once | ||
| if (!tokenResponse.indicatesSuccess()) { | ||
| ErrorObject error = tokenResponse.toErrorResponse().getErrorObject(); | ||
| throw new SDKException("failure to get token. description = [" + error.getDescription() + "] error code = [" + error.getCode() + "] error uri = [" + error.getURI() + "]"); | ||
| if ("use_dpop_nonce".equals(error.getCode())) { | ||
| String dpopNonce = httpResponse.getHeaderValue("DPoP-Nonce"); | ||
| if (dpopNonce != null) { | ||
| cacheNonce(tokenEndpointUrl, dpopNonce); | ||
| TokenRequest retryRequest = new TokenRequest(tokenEndpointURI, clientAuth, authzGrant, null); | ||
| HTTPRequest retryHttpRequest = retryRequest.toHTTPRequest(); | ||
| if (sslSocketFactory != null) { | ||
| retryHttpRequest.setSSLSocketFactory(sslSocketFactory); | ||
| } | ||
| SignedJWT retryProof = dpopFactory.createDPoPJWT( | ||
| retryHttpRequest.getMethod().name(), | ||
| htuOf(retryHttpRequest.getURI()), | ||
| new Nonce(dpopNonce)); | ||
| retryHttpRequest.setDPoP(retryProof); | ||
| httpResponse = retryHttpRequest.send(); | ||
| tokenResponse = TokenResponse.parse(httpResponse); | ||
| } else { | ||
| logger.warn("token endpoint {} returned use_dpop_nonce but did not supply a DPoP-Nonce response header", | ||
| tokenEndpointURI); | ||
| } | ||
| } | ||
| if (!tokenResponse.indicatesSuccess()) { | ||
| ErrorObject finalError = tokenResponse.toErrorResponse().getErrorObject(); | ||
| throw new SDKException("failure to get token. description = [" + finalError.getDescription() | ||
| + "] error code = [" + finalError.getCode() | ||
| + "] error uri = [" + finalError.getURI() + "]"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Nonce rotated on a failed retry response is never cached, risking a repeated-failure loop.
If the AS returns use_dpop_nonce again on the retried request (a second rotation), the code throws at Line 267-269 before the unconditional cacheNonce at Line 276 ever runs — only the nonce from the first 401 gets cached (Line 247). The next call to getToken() will retry with that stale, already-rejected nonce and fail again, since nothing else refreshes the token-endpoint's cached nonce.
🛡️ Proposed fix
retryHttpRequest.setDPoP(retryProof);
httpResponse = retryHttpRequest.send();
tokenResponse = TokenResponse.parse(httpResponse);
+ // Cache the nonce from the retry response too, even if it also failed,
+ // so the next getToken() call doesn't retry with a stale nonce.
+ cacheNonce(tokenEndpointUrl, httpResponse.getHeaderValue("DPoP-Nonce"));
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // RFC 9449 §8.2: if AS requires a nonce, cache it and retry once | |
| if (!tokenResponse.indicatesSuccess()) { | |
| ErrorObject error = tokenResponse.toErrorResponse().getErrorObject(); | |
| throw new SDKException("failure to get token. description = [" + error.getDescription() + "] error code = [" + error.getCode() + "] error uri = [" + error.getURI() + "]"); | |
| if ("use_dpop_nonce".equals(error.getCode())) { | |
| String dpopNonce = httpResponse.getHeaderValue("DPoP-Nonce"); | |
| if (dpopNonce != null) { | |
| cacheNonce(tokenEndpointUrl, dpopNonce); | |
| TokenRequest retryRequest = new TokenRequest(tokenEndpointURI, clientAuth, authzGrant, null); | |
| HTTPRequest retryHttpRequest = retryRequest.toHTTPRequest(); | |
| if (sslSocketFactory != null) { | |
| retryHttpRequest.setSSLSocketFactory(sslSocketFactory); | |
| } | |
| SignedJWT retryProof = dpopFactory.createDPoPJWT( | |
| retryHttpRequest.getMethod().name(), | |
| htuOf(retryHttpRequest.getURI()), | |
| new Nonce(dpopNonce)); | |
| retryHttpRequest.setDPoP(retryProof); | |
| httpResponse = retryHttpRequest.send(); | |
| tokenResponse = TokenResponse.parse(httpResponse); | |
| } else { | |
| logger.warn("token endpoint {} returned use_dpop_nonce but did not supply a DPoP-Nonce response header", | |
| tokenEndpointURI); | |
| } | |
| } | |
| if (!tokenResponse.indicatesSuccess()) { | |
| ErrorObject finalError = tokenResponse.toErrorResponse().getErrorObject(); | |
| throw new SDKException("failure to get token. description = [" + finalError.getDescription() | |
| + "] error code = [" + finalError.getCode() | |
| + "] error uri = [" + finalError.getURI() + "]"); | |
| } | |
| } | |
| // RFC 9449 §8.2: if AS requires a nonce, cache it and retry once | |
| if (!tokenResponse.indicatesSuccess()) { | |
| ErrorObject error = tokenResponse.toErrorResponse().getErrorObject(); | |
| if ("use_dpop_nonce".equals(error.getCode())) { | |
| String dpopNonce = httpResponse.getHeaderValue("DPoP-Nonce"); | |
| if (dpopNonce != null) { | |
| cacheNonce(tokenEndpointUrl, dpopNonce); | |
| TokenRequest retryRequest = new TokenRequest(tokenEndpointURI, clientAuth, authzGrant, null); | |
| HTTPRequest retryHttpRequest = retryRequest.toHTTPRequest(); | |
| if (sslSocketFactory != null) { | |
| retryHttpRequest.setSSLSocketFactory(sslSocketFactory); | |
| } | |
| SignedJWT retryProof = dpopFactory.createDPoPJWT( | |
| retryHttpRequest.getMethod().name(), | |
| htuOf(retryHttpRequest.getURI()), | |
| new Nonce(dpopNonce)); | |
| retryHttpRequest.setDPoP(retryProof); | |
| httpResponse = retryHttpRequest.send(); | |
| tokenResponse = TokenResponse.parse(httpResponse); | |
| cacheNonce(tokenEndpointUrl, httpResponse.getHeaderValue("DPoP-Nonce")); | |
| } else { | |
| logger.warn("token endpoint {} returned use_dpop_nonce but did not supply a DPoP-Nonce response header", | |
| tokenEndpointURI); | |
| } | |
| } | |
| if (!tokenResponse.indicatesSuccess()) { | |
| ErrorObject finalError = tokenResponse.toErrorResponse().getErrorObject(); | |
| throw new SDKException("failure to get token. description = [" + finalError.getDescription() | |
| "] error code = [" + finalError.getCode() | |
| "] error uri = [" + finalError.getURI() + "]"); | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 257-257: SMTP server identity must be enforced
Context: retryHttpRequest.send()
Note: [CWE-297] Improper Validation of Certificate with Host Mismatch.
(smtp-insecure-connection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/src/main/java/io/opentdf/platform/sdk/TokenSource.java` around lines 241
- 271, Update the nonce-retry handling in the token request flow around
TokenRequest and tokenResponse so that a DPoP-Nonce returned on the retried
response is cached before the final error is thrown. Preserve the existing
single retry behavior, and ensure the latest nonce from either response is
stored for the next getToken() call.
mkleene
left a comment
mkleene
left a comment
There was a problem hiding this comment.
Looks great! Really thorough tests and worked around some tricky issues.
| } | ||
|
|
||
| private SDK buildSDK() { | ||
| // The picocli @Option annotations on platformEndpoint/clientId/clientSecret are |
| * @param dpopKey JWK (RSA or EC) to use for DPoP proofs | ||
| * @return this builder instance for method chaining | ||
| */ | ||
| public SDKBuilder dpopKey(JWK dpopKey) { |
There was a problem hiding this comment.
Not sure if I love imposing a compile dependency on com.nimbusds.jose on consumers.
The only other thing that I can think of is taking in a PEM or a java.security.Key as well as the algorithm.
Probably not a real issue since I think that this library is pretty standard and well supported.
| * @param dpopAlg JWS algorithm (e.g. RS256, ES256) | ||
| * @return this builder instance for method chaining | ||
| */ | ||
| public SDKBuilder dpopAlgorithm(JWSAlgorithm dpopAlg) { |
There was a problem hiding this comment.
maybe have one setter for both of these since you need both?
| // The connect-kotlin Interceptor API exposes no per-call context to thread the | ||
| // request URL into responseFunction. ThreadLocal is the workaround, relying on | ||
| // connect-kotlin's contract that requestFunction and responseFunction for a single | ||
| // unary call run synchronously on the same thread. If that assumption ever breaks, | ||
| // nonces could be cached against the wrong origin. The okhttp-level | ||
| // dpopRetryInterceptor below avoids the issue by reading the URL straight from | ||
| // chain.request(). |
There was a problem hiding this comment.
It looks like we might be able to use the fact that we should be creating the auth interceptor for each request/response to pair these and not rely on ThreadLocals. Not a big deal since this seems to work but we could wire in per-response creation here
Summary
Adds comprehensive DPoP (RFC 9449) support to the Java SDK as part of the Keycloak v26 upgrade and DPoP implementation effort.
Related Jira: https://virtru.atlassian.net/browse/DSPX-3397
Test Scenario: xtest/scenarios/DSPX-3397.yaml (in tests repo)
Changes
Core DPoP proof generation
DefaultDPoPProofFactory, with claimsjti,htm,htu,iat(plusathfor resource endpoints).htuclaim strips query and fragment per RFC 9449 §4.2.DPoPvsBearer) modeled as an enum; token + scheme read atomically to avoid a refresh-time data race.Server-issued nonce handling (full retry — implemented)
use_dpop_nonce, retries the token request once with the AS-provided nonce.DPoP-Noncefrom aWWW-Authenticatechallenge; drops the stale proof header on the retry.Key configuration & validation
dpopKey(...)accepts a caller-supplied RSA or EC key; auto-generates an ephemeral key if none provided. An EC DPoP key triggers a separate RSA-2048 SRT key.htmclaim.Bearer fallback
Bearerscheme when the AS returns a non-DPoP-bound token, with a warning; stale DPoP proof is stripped so a Bearer request never carries a DPoP header.platform_issuer.CLI (cmdline)
DPoP is always-on for any authenticated client, so the CLI needs no DPoP flags:
encrypt,decrypt, and metadata commands are sender-constrained without any opt-in.--dpop/--dpop-keyflags are exposed (an earlier revision added them; they were removed together withCliDpopOptionssince they served no purpose once DPoP is unconditional). This mirrors the go-sdk direction in feat(sdk): add DPoP client support with HTTP RoundTripper (DSPX-3397) platform#3581, which likewise dropped otdfctl's--dpopparameters.supportssubcommand (no help-text scraping required):tdf supports <feature>exits0if supported,1otherwise.tdf supportswith no argument lists all supported feature names, one per line.--jsonemits a JSON object mapping feature name → boolean, e.g.{"dpop":true}or, with no argument,{"dpop":true,"dpop_nonce_challenge":true}.Feature Detection
Testing
New coverage:
TokenSourceTest,DPoPRetryInterceptorTest,AuthInterceptorConnectPathTest(Kotlin), andCommandTest— covering nonce caching/origin isolation, both retry paths, EC/RSA key handling, and thesupportssubcommand (bare exit codes, no-arg listing, and--jsonoutput).Related PRs
cli.shto detectdpopviasupports dpopinstead of grepping the removed--dpopflag.Checklist
--dpop/--dpop-key+CliDpopOptions)supportsfeature detection: bare exit code, no-arg listing,--jsonSummary by CodeRabbit
New Features
supportscommand to list supported features or return machine-readable JSON results.-v/--verbose.Documentation