stacklok / toolhive / 22912366653

64%

Build Type

push

github

Committed by web-flow

Commit Message

fix: refresh upstream tokens transparently instead of forcing re-auth (#4036)

* Refresh upstream tokens transparently instead of forcing re-auth

The upstreamswap middleware returned 401 when upstream access tokens
expired, forcing users through full re-authentication even though
valid refresh tokens existed in storage. This happened because:

1. Redis/memory storage TTL was set to access token expiry, deleting
   the entry (and refresh token) when the access token expired
2. Storage returned nil on ErrExpired, discarding the refresh token
3. The middleware had no refresh path — only 401

Fix all three layers:

- Add DefaultRefreshTokenTTL (30 days) to storage entry TTL so
  refresh tokens survive past access token expiry
- Return token data alongside ErrExpired from storage so callers
  can use the refresh token
- Add UpstreamTokenRefresher interface and implementation that wraps
  the upstream OAuth2Provider and storage
- Plumb the refresher through Server → EmbeddedAuthServer → Runner →
  MiddlewareRunner
- Update upstreamswap middleware to attempt refresh before returning
  401, only requiring re-auth when the refresh token itself fails

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add unit tests for token refresh paths

Add comprehensive tests for RefreshAndStore (6 cases) and middleware
refresh paths (4 cases: successful refresh, failed refresh, no refresh
token, defense-in-depth expired-without-error).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update pkg/authserver/refresher.go

Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>

* Update pkg/auth/upstreamswap/middleware.go

Co-authored-by: Jakub Hrozek <jakub.hrozek@posteo.se>

* Address PR review comments

- Fix step numbering: renumber step 6 → 5 after step 5 removal
- Update redis integration test: assert returned token data is non-nil
  on ErrExpired, consistent with the unit test contract
- Fix test closures: pass subtest t to ... (continued)

Run Details

162 of 186 new or added lines in 9 files covered. (87.1%)

5 existing lines in 3 files now uncovered.

47561 of 74407 relevant lines covered (63.92%)

72.36 hits per line