Skip to content

Fix missing resource parameter on cached token restore#4292

Merged
amirejaz merged 2 commits intostacklok:mainfrom
rodrigo1208:fix-rfc8707-resource-parameter-cached-token-restore
Mar 20, 2026
Merged

Fix missing resource parameter on cached token restore#4292
amirejaz merged 2 commits intostacklok:mainfrom
rodrigo1208:fix-rfc8707-resource-parameter-cached-token-restore

Conversation

@rodrigo1208
Copy link
Copy Markdown
Contributor

When restoring from cached tokens, token refreshes were using the plain oauth2.Config.TokenSource, which drops the RFC 8707 resource parameter. Use NewResourceTokenSource (exported) on the cached path, matching the existing fresh OAuth flow behaviour.

Summary

When a session is restored from cached tokens (i.e. after a process restart), token refreshes were issued without the RFC 8707 resource parameter. This caused the authorization server to return access
tokens with a missing or incorrect aud claim, breaking authentication on any endpoint that strictly validates it. The fresh OAuth flow was already correct — NewResourceTokenSource was introduced in #3713
to handle this — but the cached restore path bypassed it entirely, always falling back to the plain oauth2.Config.TokenSource.

  • Export newResourceTokenSource → NewResourceTokenSource so it can be used outside the oauth package
  • Apply the same resource-aware token source on the cached restore path in tryRestoreFromCachedTokens, mirroring the pattern already used in flow.go

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Manual testing (describe below)

Stop + start the server with a cached session configured with a resource parameter. Confirmed the restored token source now sends resource on refresh and the server returns tokens with the correct aud
claim.

Special notes for reviewers

The root cause was subtle: CreateTokenSourceFromCached wraps oauth2.Config.TokenSource, which has no knowledge of RFC 8707. The fix mirrors exactly what flow.go:processToken already does — the only missing
piece was exposing NewResourceTokenSource for cross-package use.

When restoring from cached tokens, token refreshes were using the
plain oauth2.Config.TokenSource, which drops the RFC 8707 resource
parameter. Use NewResourceTokenSource (exported) on the cached path,
matching the existing fresh OAuth flow behaviour.
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 20, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 20, 2026

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.77%. Comparing base (4d4fbe2) to head (6892308).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/auth/remote/persisting_token_source.go 42.85% 2 Missing and 2 partials ⚠️
pkg/auth/remote/handler.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4292      +/-   ##
==========================================
- Coverage   68.95%   68.77%   -0.19%     
==========================================
  Files         473      473              
  Lines       47854    47917      +63     
==========================================
- Hits        33000    32957      -43     
- Misses      12266    12298      +32     
- Partials     2588     2662      +74     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rodrigo1208 rodrigo1208 requested a review from rdimitrov as a code owner March 20, 2026 18:23
@github-actions github-actions bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Mar 20, 2026
@amirejaz
Copy link
Copy Markdown
Contributor

Approved, thanks for addressing the feedback.

@amirejaz amirejaz merged commit 4bb1016 into stacklok:main Mar 20, 2026
39 checks passed
Sanskarzz pushed a commit to Sanskarzz/toolhive that referenced this pull request Mar 23, 2026
* Fix missing resource parameter on cached token restore

When restoring from cached tokens, token refreshes were using the
plain oauth2.Config.TokenSource, which drops the RFC 8707 resource
parameter. Use NewResourceTokenSource (exported) on the cached path,
matching the existing fresh OAuth flow behaviour.

* Move resource parameter logic into CreateTokenSourceFromCached
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants