feat(core): Adds comprehensive DPoP (RFC 9449) support#3582
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds server-side DPoP nonce enforcement with a rotating concurrent nonce manager, strict/loose HTU matching, nonce challenge headers in HTTP mux and Connect middleware, exported nonce error types, ChangesDPoP Nonce Enforcement and Keycloak 26.2 Upgrade
Sequence Diagram(s)sequenceDiagram
participant Client
participant MuxHandler
participant checkToken
participant validateDPoP
participant dpopNonceManager
Client->>MuxHandler: HTTP request + Authorization: DPoP <token> + DPoP: <proof>
MuxHandler->>checkToken: receiverInfo{u:[http://host/path, https://host/path], m:[POST]}
checkToken->>validateDPoP: token, dpopProof, receiverInfo
validateDPoP->>dpopNonceManager: validate(nonceClaim)
alt Missing or expired nonce
dpopNonceManager-->>validateDPoP: DPoPNonceError
validateDPoP-->>MuxHandler: DPoPNonceError
MuxHandler-->>Client: 401 + DPoP-Nonce: <fresh> + WWW-Authenticate: DPoP error="use_dpop_nonce"
else Malformed nonce type
dpopNonceManager-->>validateDPoP: DPoPNonceMalformedError
validateDPoP-->>MuxHandler: DPoPNonceMalformedError
MuxHandler-->>Client: 401 (no retry challenge)
else Valid nonce
dpopNonceManager-->>validateDPoP: nil
validateDPoP-->>checkToken: claims
checkToken-->>MuxHandler: claims
MuxHandler-->>Client: 200 + DPoP-Nonce: <rotated>
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces full RFC 9449 DPoP support to the OpenTDF platform service. By integrating DPoP proof validation and server-issued nonce management, the changes significantly enhance the security of token-based authentication for both HTTP and gRPC interfaces. The implementation includes robust error handling for nonce challenges and ensures compatibility with existing authentication flows. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Tokens held with proof of key, RFC standards, plain to see. Nonces rotate, threats subside, Security with nowhere to hide. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces server-issued DPoP nonce management per RFC 9449 §8, including configuration options, a nonce manager with rotation and validation, and interceptor integrations for both HTTP and Connect handlers. Feedback on these changes highlights two critical issues: a race condition in getCurrentNonce() that can cause concurrent double-rotation of nonces, and incorrect response header propagation in the Connect interceptor where metadata.AppendToOutgoingContext is used instead of setting headers on the response directly.
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.
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
dmihalcik-virtru
force-pushed
the
DSPX-3397-platform-service
branch
from
2f36d01 to
d304bd7
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
jentfoo
reviewed
Jun 10, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
dmihalcik-virtru
force-pushed
the
DSPX-3397-platform-service
branch
from
f8d30ac to
77a7d4d
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
dmihalcik-virtru
force-pushed
the
DSPX-3397-platform-service
branch
from
77a7d4d to
d0155a6
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
Error Summary
TDF3 Benchmark Results:
Error Summary:
|
Contributor
Two fixups for the DSPX-3397 platform-service work: - authn.go: structpb.NewStruct rejects []string when serializing the well-known configuration. Convert dpop_supported_alg_values to []any before registration. Without this, /.well-known/opentdf-configuration returns 500 with "proto: invalid type: []string". - docker-compose.yaml: KC26 dropped admin-fine-grained-authz from the default preview profile. The platform's `service provision keycloak` calls setManagementPermissionsEnabled which requires this feature. Enable it explicitly via KC_FEATURES so provisioning succeeds against the bumped Keycloak image. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…gation - getCurrentNonce: use double-checked locking to prevent concurrent double-rotation; inlines rotation under write lock instead of calling rotate() (which acquires its own lock and would deadlock) - ConnectUnaryServerInterceptor: replace metadata.AppendToOutgoingContext with a next-wrapper that sets DPoP-Nonce on res.Header() so the header is actually returned to the client Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- Rename well-known key to dpop_signing_alg_values_supported (RFC 9449 §5.1) - Add DPoPNonceMalformedError type for non-string nonce claims; malformed proofs fall through to hard rejection rather than issuing a nonce challenge - Downgrade DPoPNonceError log entries from Warn to Debug since nonce challenges are normal protocol handshakes, not failures - Set DPoP-Nonce on Connect error responses so clients retain nonce after downstream handler errors - Add DPoPConfig.Validate() to reject zero NonceExpiration when RequireNonce is true; call it from validateAuthNConfig - Remove tautological tests that asserted only on local variables; replace with real checkToken coverage for missing/valid/wrong/malformed nonce cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…empty nonce validation Replace sync.RWMutex with atomic.Pointer[nonceState] for lock-free reads on the hot path (getCurrentNonce, validateNonce), keeping sync.Mutex only to serialize writes. Also guards validateNonce against empty-string nonce matching the initial zero-value previousNonce. 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 under the "DPoP" Authorization scheme. The platform currently accepts either scheme as long as a valid DPoP proof is attached. This change emits a WARN log when a cnf-bound token arrives under "Bearer" scheme, surfacing non-compliant SDKs without breaking existing clients. A follow-up will promote this to a hard reject once all SDKs are compliant. 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>
…ptor The ConnectRPC interceptor was using req.Spec().Procedure (path only) as the expected htu value, but RFC 9449 requires htu to be the full request URI (scheme + host + path). Clients correctly send the full URL, causing htu validation to always fail for gRPC/ConnectRPC endpoints. Fix both the ConnectRPC unary interceptor and ipcReauthCheck to construct the full URL from the Host header, accepting both http and https schemes since TLS state is not available in the interceptor context. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
In loose mode (default, strict_htu: false) a path-only htu claim is accepted when its path matches the path of any acceptable URI, easing SDK skew during rollout. If the origin is present it must still match exactly (scheme-flexible via the http+https pair already in dpopInfo.u). In strict mode (strict_htu: true) the origin must be present and match; path-only htu claims are rejected outright. Config path: server.auth.dpop.strict_htu (bool, default false) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…interceptor ConnectRPC supports idempotent unary RPCs over HTTP GET (proto option idempotency_level = NO_SIDE_EFFECTS). The Java connect-go client (Buf's 'Hasan'/Get-requests feature) uses this whenever the server advertises an idempotent method. DPoP requires the proof JWT's htm claim to equal the actual HTTP method, but the Connect interceptor hardcoded POST in receiverInfo.m, so any GET request with htm:'GET' was rejected as 'incorrect htm claim'. Use connect.AnyRequest.HTTPMethod() (which is populated server-side from the underlying *http.Request) to bring the Connect path to parity with the MuxHandler path, which already reads r.Method. IPCUnaryServerInterceptor is unchanged: IPC traffic goes through memhttp which always uses POST. Tests: - New positive checkToken case for htm:GET. - New interceptor test that captures receiverInfo via _testCheckTokenFunc and asserts the method is propagated (parameterized over GET and POST). - New invalid-DPoP table row exercising the wrong-method failure direction (GET-DPoP against POST-only receiver). Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Log the htm claim at every site where DpopInfo.m is set on the server and where the htm claim is built on the client, plus the htm comparison in validateDPoP. Helps diagnose method mismatches between client and server (e.g. client hardcoding POST while server reads the transport method). Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Map errors from canAccess into categorized 403/500 responses with a sanitized cause-class tag (forbidden: pdp-denied, internal: auth-service-unavailable, internal: context-cancelled) so clients and tests can distinguish routine denials from infrastructure failures without parsing logs. Previously every category from canAccess collapsed to code=Internal with the opaque message "could not perform access". Split logging at the two GetDecision call sites and the rewrap call site into Info (terse, no sensitive payload — category, connect code, batch size) plus Debug (full error, policies, resource attribute FQNs, fulfillable obligation FQNs). The Info lines previously dropped err entirely at ErrorContext, so debug logging in CI couldn't surface what the authorization service actually said. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…-ref
- Pin StrictHTU:false in nonce test fixture for deterministic nonce tests
- Use full-origin htu in GET DPoP test to decouple from loose HTU matching
- Use ${{ inputs.platform-ref }} instead of hardcoded feat-kc26-dpop tag in test action
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Revert switch to platform-ref; prefer a stricter explicit pin for the downloaded scripts/compose. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Keep the default docker-compose on Keycloak 25 so downstream consumers that aren't DPoP-ready stay on it. The start-up action now overlays Keycloak 26.2 plus the admin-fine-grained-authz:v1 feature flag onto docker-compose only when dpop-challenge-enabled=true, so DPoP/nonce testing runs against 26.2 without forcing the bump on everyone. Stop overwriting docker-compose.yaml from the feature tag so the checked-out (KC 25) compose is the base. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Add a server-side replay cache that rejects reused DPoP proof jti values within the acceptance window (RFC 9449 §11.1). The check runs as the last step in validateDPoP so only otherwise-valid proofs populate the cache, and the cache TTL tracks DPoPSkew since older proofs are already rejected by the iat freshness check. Also addresses review nits: move the dpopNonceManager doc comment onto the correct type and use WarnContext in checkToken. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-3397-platform-service
branch
from
b28062c to
e3489bf
Compare
policy-bot-opentdf
Bot
dismissed
elizabethhealy’s stale review
Invalidated by push of e3489bf
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
elizabethhealy
approved these changes
Jun 22, 2026
This was referenced Jun 22, 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
3 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.
Implements comprehensive DPoP (Demonstrating Proof-of-Possession) support per RFC 9449 for the OpenTDF platform service.
Summary
This PR adds full RFC 9449 DPoP support to the platform authentication middleware, enabling:
Authorization: BearerandAuthorization: DPoPtoken schemessupports_dpop)Since we have an existing, partial implementation, some fallback protection is in place:
Authorization: Bearer [jwt]headers.cnfclaim.htuclaims with missing origin, e.g./kas.AccessService/Rewrapwill matchhtu, setserver.auth.dpop.strict_htu: trueImplementation Details
DPoP Proof Validation (RFC 9449 §4.3 + §7.1)
The middleware validates all required DPoP proof claims:
dpop+jwtcnf.jktclaimDPoP-Nonce Support (RFC 9449 §8)
Server-issued nonces prevent replay attacks:
server.auth.dpop.require_nonce(default: false),server.auth.dpop.nonce_expiration(default: 5m)DPoP-Nonceheader andWWW-Authenticate: DPoP error="use_dpop_nonce"gRPC Support
DPoP works seamlessly with gRPC/Connect:
POSTfor gRPC calls/kas.AccessService/Rewrap)Feature Detection
Registers
supports_dpop: truein the wellknown service for integration with xtest feature gates (pfs.skip_if_unsupported("dpop")).Testing
Unit Tests (
dpop_nonce_test.go)Comprehensive test coverage:
Integration Tests
Existing DPoP tests in
test/integration/oauth/oauth_test.gocontinue to pass and validate end-to-end flows with real Keycloak.End-to-End Tests
Validated end-to-end with the xtest DPoP scenario: opentdf/tests#529.
Related
xtest/scenarios/DSPX-3397.yamlAll PRs:
Summary by CodeRabbit
New Features
Improvements