Retry ID-token validation after OIDC refresh

[siddiqueirshad]

Summary

When an upstream refresh exchange succeeds but ID-token verification then fails (for example a JWKS fetch timeout), ToolHive currently discards the new tokens and retries the token request. With a rotating one-time refresh token the first exchange already consumed the grant, so the retry returns invalid_grant and the session is killed even though the IdP had issued valid tokens.

This change retries ID-token verification in place after a successful exchange, without sending the refresh token again. If every attempt is still a transient key-fetch failure, the unvalidated ID token is dropped and the new access and refresh tokens are returned so the rotated grant is preserved. The storage layer already carries forward the previous ID token when the new one is empty. Permanent failures such as a bad signature or subject mismatch still fail closed.

Fixes #6194

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (go test ./pkg/authserver/upstream/ ./pkg/authserver/)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Covered by new regression tests:

• JWKS returns 500 once, then succeeds: refresh succeeds and the token endpoint is called once
• JWKS stays unavailable: refresh still returns the new access and refresh tokens with the ID token cleared
• ID token signed with a different key: validation still fails and the token endpoint is not retried

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Does this introduce a user-facing change?

Yes. Users whose upstream IdP rotates refresh tokens no longer lose their session when a JWKS timeout or similar transient failure happens after a successful token refresh.

Special notes for reviewers

• The retry is only for transient key-fetch / network failures (fetching keys, timeouts, deadline exceeded). Signature and subject failures are not retried and are not bypassed.
• The fallback that drops an unvalidated ID token after exhausted retries is deliberate: returning an error at that point is what caused the caller to replay the consumed refresh token in Upstream refresh replays a consumed rotating refresh token when ID-token validation fails after a successful exchange #6194. Access and refresh tokens still came from the trusted token endpoint; the new ID token is not used for identity.

Made with Cursor

[jhrozek]

Reviewed the fix end to end: RefreshTokens -> validateRefreshedIDToken -> refresher.go's carry-forward of the previous IDToken when the new one is empty. The design holds up:

• Permanent failures (bad signature, subject mismatch) still fail closed and are never retried.
• Transient JWKS/network failures are retried in place (no re-exchange, so the already-consumed rotating refresh token is never replayed).
• When verification still can't fetch keys after 3 attempts, the unvalidated ID token is dropped rather than surfaced — no unvalidated claims ever reach the caller, and the previous validated ID token is retained via the existing carry-forward in refresher.go.

Verified isTransientIDTokenValidationError's string matching ("fetching keys" / "get keys failed") against the vendored go-oidc v3.21.0 source — it correctly matches today; a future go-oidc upgrade changing that wording would just fail closed (safe) rather than silently mis-detect.

Ran the full unit suite (task test) and task lint — all green except one pre-existing gci pair in unrelated files this PR doesn't touch.

One trivial nit (non-blocking): mustNewOIDCProvider(t *testing.T, ctx context.Context, issuer string) in the new test code puts ctx second; revive's context-as-argument flags it. Worth a quick reorder to (t, issuer, ctx) or (ctx, t, issuer) in a follow-up.

Nice, well-tested fix — LGTM.

[codecov]

Codecov Report

❌ Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.69%. Comparing base (ab27a23) to head (b7f3a96).
⚠️ Report is 80 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/upstream/oidc.go	97.22%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6559   +/-   ##
=======================================
  Coverage   78.69%   78.69%           
=======================================
  Files         777      777           
  Lines       77092    77121   +29     
=======================================
+ Hits        60665    60688   +23     
- Misses      16422    16428    +6     
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 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.