fix: refresh upstream tokens transparently instead of forcing re-auth

[aron-muon]

Summary

Users are forced to fully re-authenticate with upstream OAuth providers every time the upstream access token expires (controlled by accessTokenLifespan), even though valid refresh tokens exist in storage.

The root cause is that upstream access tokens and refresh tokens are stored together in a single storage entry, with the entry's TTL/expiry set to the access token's expiry. When the access token expires, the entry is deleted (Redis) or marked expired (memory) — losing the refresh token, which is typically valid for 30-90 days or longer depending on the provider. The upstreamswap middleware has no refresh path and returns 401, forcing full re-authentication.

This affects both Redis and in-memory storage backends — Redis deletes the key via TTL, and memory storage's cleanup goroutine removes the expired entry.

Type of change

• Bug fix

Root cause

The storage model bundles access + refresh tokens in a single entry (UpstreamTokens struct). The entry TTL is derived from the access token's ExpiresAt, so when the access token expires:

1. Storage deletes or expires the entry — losing the still-valid refresh token
2. GetUpstreamTokens returns nil, ErrExpired — discarding the token data
3. Middleware returns 401 — no refresh path exists

Ideally, access and refresh tokens would be stored separately with independent TTLs matching their actual lifetimes. This fix extends the bundled entry's TTL as a pragmatic solution; a future refactor could separate them for cleaner lifecycle management.

Changes

Storage layer (pkg/authserver/storage/)

• Extended upstream token entry TTL by DefaultRefreshTokenTTL (30 days) in both Redis and memory storage, so refresh tokens survive past access token expiry
• Changed GetUpstreamTokens to return token data alongside ErrExpired (instead of nil) so callers can use the refresh token
• Memory storage now checks the token's own ExpiresAt (access token expiry) rather than the entry's expiresAt (storage TTL) for the expired check

Token refresher (pkg/authserver/refresher.go)

• New UpstreamTokenRefresher interface in storage/types.go
• Implementation wraps upstream.OAuth2Provider.RefreshTokens() + UpstreamTokenStorage.StoreUpstreamTokens()
• Preserves binding fields (ProviderID, UserID, UpstreamSubject, ClientID) across refresh
• Handles refresh token rotation (keeps old refresh token if provider doesn't issue a new one)

Plumbing

• Exposed refresher through Server → EmbeddedAuthServer → Runner → MiddlewareRunner using the same lazy accessor pattern as GetUpstreamTokenStorage

Middleware (pkg/auth/upstreamswap/)

• Middleware now attempts transparent refresh before returning 401
• Extracted getOrRefreshUpstreamTokens helper to keep cyclomatic complexity under lint threshold
• Only requires re-auth when the refresh token itself is invalid/revoked

Production validation

Deployed to a production cluster with Redis (AWS Valkey) storage (we use a sentinel emulator which basically just returns the Valkey URL in each case. One of the little clever ways we could use Valkey). All four upstream providers successfully refreshed tokens transparently — no user re-authentication required:

Provider	Token Endpoint	Access Token Lifetime	Refresh Token Rotated
Atlassian	cf.mcp.atlassian.com/v1/token	1 hour	Yes
Asana	app.asana.com/-/oauth_token	1 hour	Yes
Slack (GovSlack)	slack-gov.com/api/oauth.v2.access	12 hours	Yes
Google	oauth2.googleapis.com/token	1 hour	Yes

Redis TTLs confirmed updated to ~30 days (previously ~1 hour). GitHub has not yet expired its 8-hour access token but uses the same code path.

Test plan

• Updated storage tests to verify tokens returned alongside ErrExpired
• Updated cleanup tests for extended TTL
• Updated middleware tests with refresher parameter
• All existing unit tests pass
• Build clean, golangci-lint clean
• Deployed to production — verified transparent refresh for Atlassian, Asana, Slack, and Google

Does this introduce a user-facing change?

Yes — upstream OAuth sessions now persist beyond the access token lifetime. Users will no longer be forced to re-authenticate as long as their upstream refresh token is valid (typically 30 days to indefinite depending on the provider).

Large PR Justification

• Multiple related changes that would break if separated

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 81.81818% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.65%. Comparing base (85c5f3e) to head (2f1c6dd).

Files with missing lines	Patch %	Lines
pkg/auth/upstreamswap/middleware.go	87.23%	4 Missing and 2 partials ⚠️
pkg/authserver/server_impl.go	0.00%	6 Missing ⚠️
pkg/runner/runner.go	0.00%	5 Missing ⚠️
pkg/authserver/runner/embeddedauthserver.go	0.00%	2 Missing ⚠️
pkg/authserver/storage/redis.go	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4036      +/-   ##
==========================================
+ Coverage   68.61%   68.65%   +0.03%     
==========================================
  Files         445      446       +1     
  Lines       45374    45462      +88     
==========================================
+ Hits        31135    31210      +75     
- Misses      11841    11844       +3     
- Partials     2398     2408      +10     

☔ 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.

[jhrozek]

hey @aron-muon thanks a lot for the patches this was actually something we had on our roadmap so the feature makes complete sense 🙏🏻

I'll make sure to review the PR as soon as possible.

[jhrozek]

Nice work on transparent token refresh! A few minor comments below.

[jhrozek]

Not a blocker for this PR, but worth flagging: when multiple concurrent requests hit getOrRefreshUpstreamTokens with the same expired session, each one will independently call RefreshAndStore. With providers that rotate refresh tokens (issuing a single-use replacement), all but the first caller will use a stale refresh token and fail.

A singleflight.Group keyed on sessionID would collapse concurrent refreshes into one. I've actually been working on something similar in a parallel branch, so I'll address this in a follow-up rather than piling onto this PR.

[aron-muon]

Nice call, added a singleflight.Group keyed on sessionID to deduplicate concurrent refreshes. Scoped per middleware instance to avoid test interference and also added TestSingleFlightRefresh_ConcurrentRequests to verify only one RefreshAndStore fires with 10 concurrent expired-session requests

[jhrozek]

Second pass: a few more findings from an adversarial review.

[jhrozek]

This is great work. I submitted a bunch of nits but take them really as nits, none of htem are really blocking, if there's one I'd love to have fixed before merging, it's the tokens.IsExpired, feel free to just dismiss the rest.

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

Large PR justification has been provided. Thank you!

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

[aron-muon]

This is great work. I submitted a bunch of nits but take them really as nits, none of htem are really blocking, if there's one I'd love to have fixed before merging, it's the tokens.IsExpired, feel free to just dismiss the rest.

Thanks for the review! I included a fix for each nit and also added a fix for the multiple concurrent requests issue.