Skip to content

fix(core): infer JWT algorithms for JWKS keys without alg#3434

Merged
strantalis merged 4 commits intomainfrom
fix/dspx-3172-jwks-alg-inference
May 6, 2026
Merged

fix(core): infer JWT algorithms for JWKS keys without alg#3434
strantalis merged 4 commits intomainfrom
fix/dspx-3172-jwks-alg-inference

Conversation

@strantalis
Copy link
Copy Markdown
Member

@strantalis strantalis commented May 5, 2026

Summary

  • Fix JWT verification for IdPs whose JWKS keys omit alg, such as Microsoft Entra.
  • Pass jws.WithInferAlgorithmFromKey(true) to jwt.WithKeySet while preserving issuer, audience, skew, kid, and signature validation.
  • Add regression coverage for a JWKS key with kid but no alg.

Jira

  • DSPX-3172

Testing

  • ~/go/bin/gofumpt -w service/internal/auth/token_verifier.go service/internal/auth/token_verifier_test.go
  • cd service && go test ./internal/auth -run 'TestTokenVerifier|TestNewTokenVerifier' -v
  • cd service && go test ./internal/auth
  • cd service && golangci-lint run --new ./internal/auth

Known Existing Failures

  • cd service && golangci-lint run ./internal/auth fails on pre-existing unused //nolint directives in authn.go.
  • make test fails in lib/fixtures on TestTokenManager_InitialLogin and TestTokenManager_CustomTokenBuffer, unrelated to this auth verifier change.

Summary by CodeRabbit

  • Bug Fixes

    • JWT verification now infers the signing algorithm from the verification key, allowing token authentication to succeed when JWKS keys omit an explicit algorithm, improving robustness and interoperability.
  • Tests

    • Added coverage for tokens validated against JWKS entries that do not include an alg field.

Signed-off-by: strantalis <strantalis@virtru.com>
@strantalis strantalis requested a review from a team as a code owner May 5, 2026 20:10
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 updates the authentication token verification logic to handle JWKS keys that do not specify an algorithm. By enabling automatic algorithm inference, the system becomes more robust when interacting with Identity Providers that omit the 'alg' field in their key sets, ensuring consistent JWT validation without compromising existing security configurations.

Highlights

  • JWT Verification Improvement: Enabled algorithm inference for JWKS keys that omit the 'alg' field by passing 'jws.WithInferAlgorithmFromKey(true)' to the JWT parser.
  • Compatibility: Improved support for Identity Providers like Microsoft Entra that do not explicitly include the algorithm in their JWKS keys.
  • Regression Testing: Added a new test case to verify that tokens can be successfully validated even when the associated JWKS key lacks an algorithm definition.
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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.


When keys arrive without a sign, / To tell us how to draw the line, / We infer the way to make it true, / And let the auth pass safely through.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

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

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 updates the token verification logic to support JWKS keys that do not explicitly include an 'alg' field by enabling algorithm inference via jws.WithInferAlgorithmFromKey(true). Additionally, the test suite was updated with a new test case to verify that tokens can be successfully validated even when the JWKS key is missing the algorithm specification. I have no feedback to provide as there were no review comments to evaluate.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

JWT verification now instructs the parser to infer the signing algorithm from the JWKS verification key (jws.WithInferAlgorithmFromKey(true)); tests add a fixture and a subtest to ensure verification succeeds when JWKS keys omit the alg field.

Changes

JWT Algorithm Inference for Token Verification

Layer / File(s) Summary
Dependencies & Imports
service/internal/auth/token_verifier.go
Adds github.com/lestrrat-go/jwx/v2/jws import to enable algorithm inference configuration.
JWT Verification Core Logic
service/internal/auth/token_verifier.go
VerifyAccessToken now calls jwt.WithKeySet(v.cachedKeySet, jws.WithInferAlgorithmFromKey(true)) to allow algorithm inference from the verification key.
Test Fixtures
service/internal/auth/token_verifier_test.go
Adds fixture constructor newTokenVerifierFixtureWithoutPublicKeyAlgorithm (and adjusts related builders) to publish JWKS entries that omit the alg attribute.
Test Coverage
service/internal/auth/token_verifier_test.go
Adds subtest "valid token with JWKS key missing alg" that signs an RS256 token and verifies it succeeds against a JWKS lacking alg.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • opentdf/platform#3428: Modifies TokenVerifier.VerifyAccessToken and adds tests for JWKS entries missing alg, directly related to these changes.

Suggested labels

size/m

Suggested reviewers

  • dmihalcik-virtru
  • jentfoo
  • pflynn-virtru

Poem

🐇 I sniff the keys beneath the sunlit log,
No alg stamped on their quiet face —
I let the key itself sing out the song,
And hop along, verification in grace. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: enabling JWT algorithm inference for JWKS keys missing the alg field, which directly addresses the code changes in token_verifier.go.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dspx-3172-jwks-alg-inference

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 181.429716ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 98.083768ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 430.912089ms
Throughput 232.07 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.171580858s
Average Latency 430.22634ms
Throughput 115.82 requests/second

Signed-off-by: strantalis <strantalis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Signed-off-by: strantalis <strantalis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

X-Test Failure Report

opentdfplatformIF3KUG.dockerbuild
govulncheck-failure-8

@strantalis strantalis changed the title fix(auth): infer JWT algorithms for JWKS keys without alg fix(core): infer JWT algorithms for JWKS keys without alg May 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 197.857189ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 99.553327ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 401.434459ms
Throughput 249.11 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.013929506s
Average Latency 437.958756ms
Throughput 113.60 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 205.451121ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 106.835614ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 415.295692ms
Throughput 240.79 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.973443212s
Average Latency 438.027161ms
Throughput 113.70 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

jrschumacher
jrschumacher previously approved these changes May 5, 2026
Signed-off-by: strantalis <strantalis@virtru.com>
@strantalis
Copy link
Copy Markdown
Member Author

@CodeRabbit please review this pr again

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Tip

For best results, initiate chat on the files or code changes.

@strantalis Sure! I'll kick off a fresh review of PR #3434 right away.

[full_review]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 180.437862ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 97.493172ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 394.241487ms
Throughput 253.65 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.232173892s
Average Latency 421.391137ms
Throughput 118.39 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • tests-bdd

See the workflow run for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

@strantalis strantalis added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit 83285e7 May 6, 2026
40 checks passed
@strantalis strantalis deleted the fix/dspx-3172-jwks-alg-inference branch May 6, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants