feat(core): pass access token verifier down to registered services#3428
Conversation
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
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 refactors the authentication system by centralizing token verification logic into a reusable component. By passing this verifier down to registered services, the platform now enables services to perform their own token validation independently of the main authentication interceptor, enhancing flexibility for various authentication scenarios. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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 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 counter productive. 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 checked with care and grace, Moving to a better place. Services now can verify, With the verifier standing by. Footnotes
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Auth as Authentication
participant TV as TokenVerifier
participant OIDC as OIDC Discovery
participant Cache as JWKS Cache
Client->>Auth: checkToken(rawJWT)
Auth->>TV: VerifyAccessToken(ctx, rawJWT)
Note over TV: (using cached JWKS)
TV->>Cache: Retrieve cached key (kid)
Cache-->>TV: JWK public key
TV->>TV: Parse & validate JWT signature
TV->>TV: Verify issuer, audience, clock skew
TV-->>Auth: Parsed token or error
Auth-->>Client: Authorization result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 54 seconds.Comment |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated TokenVerifier component to handle access token validation, decoupling it from the core Authentication logic. It adds an AccessTokenVerifier interface and a concrete implementation that manages OIDC discovery and JWKS caching. These changes are integrated into the service registry, allowing services to access the verifier during initialization. Feedback was provided to ensure the VerifyAccessToken method uses context-aware logging for better traceability.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/internal/auth/token_verifier.go`:
- Around line 74-79: The AccessTokenVerifier accessor currently returns a
non-nil AccessTokenVerifier interface even when a.tokenVerifier is nil, which
lets callers' != nil checks pass and leads to panics; update
Authentication.AccessTokenVerifier to explicitly return nil when a == nil or
a.tokenVerifier == nil (i.e., check a.tokenVerifier before wrapping in the
interface), and add a nil-receiver guard at the start of
(*TokenVerifier).VerifyAccessToken to return an appropriate error when the
receiver is nil; reference the Authentication.AccessTokenVerifier method, the
tokenVerifier field, and the VerifyAccessToken method when making these changes.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e5ea6c9-5811-4c61-9793-0c06b2c9cf46
📒 Files selected for processing (5)
service/internal/auth/authn.goservice/internal/auth/token_verifier.goservice/internal/auth/token_verifier_test.goservice/pkg/server/services.goservice/pkg/serviceregistry/serviceregistry.go
|
@coderabbitai should this be named a token verifier or a token validator? cc @dillonthompson |
|
Tip For best results, initiate chat on the files or code changes.
In practice, many systems combine both steps into what is colloquially called "JWT verification," encompassing both validation and verification for comprehensive security checks. Given what this component actually does — it performs OIDC discovery, uses a cached JWKS to check the cryptographic signature, and validates claims (issuer, audience, clock skew) — it really does both. However, the naming question comes down to emphasis:
Recommendation: keep That said, if downstream consumers of |
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
🤖 I have created a release *beep* *boop* --- ## [0.15.0](opentdf/platform@service/v0.14.0...service/v0.15.0) (2026-05-06) ### Features * **core:** pass access token verifier down to registered services ([opentdf#3428](opentdf#3428)) ([b8abf17](opentdf@b8abf17)) * **policy:** add sort support to listkaskeys ([opentdf#3344](opentdf#3344)) ([de1fe92](opentdf@de1fe92)) * **policy:** support inline obligation triggers on attribute value create ([opentdf#3432](opentdf#3432)) ([876f512](opentdf@876f512)) ### Bug Fixes * **core:** infer JWT algorithms for JWKS keys without alg ([opentdf#3434](opentdf#3434)) ([83285e7](opentdf@83285e7)) * **deps:** bump github.com/Azure/go-ntlmssp from 0.0.0-20221128193559-754e69321358 to 0.1.1 in /service ([opentdf#3388](opentdf#3388)) ([ef79989](opentdf@ef79989)) * **deps:** bump github.com/jackc/pgx/v5 from 5.9.0 to 5.9.2 in /service ([opentdf#3371](opentdf#3371)) ([ab0974b](opentdf@ab0974b)) * **deps:** bump github.com/opentdf/platform/lib/identifier from 0.3.0 to 0.4.0 in /service ([opentdf#3366](opentdf#3366)) ([4650e9b](opentdf@4650e9b)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.25.0 to 0.26.0 in /service ([opentdf#3381](opentdf#3381)) ([ebc65f6](opentdf@ebc65f6)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.26.0 to 0.27.0 in /service ([opentdf#3392](opentdf#3392)) ([0c36cfa](opentdf@0c36cfa)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.27.0 to 0.28.0 in /service ([opentdf#3416](opentdf#3416)) ([bc137f6](opentdf@bc137f6)) * **deps:** bump github.com/opentdf/platform/sdk from 0.16.0 to 0.17.0 in /service ([opentdf#3395](opentdf#3395)) ([0382742](opentdf@0382742)) * **deps:** bump github.com/opentdf/platform/sdk from 0.17.0 to 0.19.0 in /service ([opentdf#3423](opentdf#3423)) ([969ac33](opentdf@969ac33)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Proposed Changes
AccessTokenVerifierChecklist
Summary by CodeRabbit