Skip to content

upstreamtoken: ID token can go permanently stale because refresh is keyed on access-token expiry #6237

Open
#6773
Open
upstreamtoken: ID token can go permanently stale because refresh is keyed on access-token expiry#6237
#6773
Assignees
tgrunnagle
Labels
bugSomething isn't working

Description

@tgrunnagle
tgrunnagle
opened on Aug 7, 2026

Summary

upstreamtoken.Service refreshes an upstream provider's tokens based on access-token expiry only, and carries the original, possibly expired ID token forward when the refresh response omits id_token. A consumer that treats the ID token's own exp as meaningful will therefore see a permanently stale subject token for the rest of the session, with no path to recover.

Detail

Two behaviours combine.

1. Refresh is triggered by access-token expiry only.

pkg/auth/upstreamtoken/service.go gates refresh on tokens.IsExpired(time.Now()), which reads tokens.ExpiresAt — the access token's lifetime:

if !tokens.ExpiresAt.IsZero() && tokens.IsExpired(time.Now()) {
    return s.refreshOrFail(ctx, sessionID, providerName, tokens)
}

and the same test in the batch path. The ID token's own exp is never consulted.

2. An omitted id_token on refresh falls back to the original.

idToken := refreshed.IDToken
if idToken == "" {
    idToken = expired.IDToken
}

The comment correctly notes OIDC Core 1.0 §12.2 permits — but does not require — a new id_token on refresh, and frames the fallback as defense-in-depth so the caller never sees an empty subject token. That is reasonable in isolation.

Why the combination is a problem

ID tokens are typically much shorter-lived than refresh tokens. Against a provider that omits id_token on refresh, the sequence is:

  1. Login: access token (1h) + ID token (1h) stored.
  2. t+1h: access token expires, refresh succeeds, response has no id_token.
  3. The expired ID token is carried forward. Access token is fresh; session continues.
  4. Every subsequent read returns a fresh access token and an ID token that is now hours or days past exp.

A consumer that validates exp on the ID token — the correct thing to do for a token it is about to derive identity or claims from — must reject it. Because the access token keeps refreshing successfully, the provider never enters a failed state, so the auth chain regards the leg as healthy and does not re-prompt. The user cannot self-heal by reconnecting.

A related shape: when RefreshToken is empty (for example, an operator-narrowed scope set that drops offline_access), refreshOrFail fails and the provider lands in a failed state, so the credential is absent rather than stale. That case is at least visible.

Impact

Any downstream consumer that validates ID-token freshness. Concretely, this was found while adding a consumer in the Stacklok enterprise distribution that validates the platform IdP's ID token (exp/iss/aud/iat/nbf) before using its claims for an authorization decision. The stale-token state makes that consumer fail closed indefinitely for an otherwise healthy session.

We are not asking for a behaviour change to suit that consumer specifically — the underlying issue is that UpstreamCredential.IDToken has no freshness contract, so every consumer has to guess.

Suggested directions

Roughly in order of preference:

  1. Give IDToken a freshness contract. Refresh when either token is near expiry, by parsing the ID token's exp at store time and tracking it alongside ExpiresAt. This makes the field mean what consumers assume.
  2. Make staleness visible instead of silent. Return the stale ID token with an explicit signal (a IDTokenExpiresAt field, or a distinguishable sentinel) so a consumer can decide, and so a re-auth can be triggered rather than inferred.
  3. Drop the fallback and return an empty IDToken once the original has expired. Simplest, but it moves the failure to consumers that currently rely on the carry-forward — hence third.

Happy to send a PR for whichever direction maintainers prefer.

References

  • OIDC Core 1.0 §12.2 (Successful Refresh Response) — id_token is optional on refresh.
  • OIDC Core 1.0 §3.1.3.7 (ID Token Validation) — step 9 requires the current time be before exp.

Activity

  1. github-actions
    added
    needs-triageIssue needs initial triage by a maintainer
    on Aug 7, 2026
  2. tgrunnagle
    self-assigned this
    on Aug 10, 2026
  3. tgrunnagle
    added
    bugSomething isn't working
    and removed
    needs-triageIssue needs initial triage by a maintainer
    on Aug 10, 2026

  4. tgrunnagle commented on Oct 9, 2026

    @tgrunnagle
    tgrunnagle
    on Oct 9, 2026
    CollaboratorAuthor

    A second shape of this bug: it also hits providers that DO rotate the ID token

    The summary above covers the case where the provider omits id_token on refresh. There is a second case with the same root cause (refresh is keyed only on access-token expiry). It affects providers that rotate the ID token correctly, as long as their ID token expires before their access token.

    Microsoft Entra ID is the concrete example. Per Microsoft's token-lifetime documentation, ID tokens have a fixed lifetime of about 60 minutes, while access tokens get a randomised lifetime of 60 to 90 minutes. Entra v2 does return a new id_token on a refresh_token grant when openid is in scope, and ToolHive handles that part correctly at v0.51.4:

    The gap is in when refresh runs. GetValidTokens (service.go#L71) and GetAllUpstreamCredentials (service.go#L113-L122) refresh only when ExpiresAt, which is the access token's expiry, has passed. With Entra, each cycle goes like this:

    1. t=0: login stores an access token (expires somewhere in t+60..90m) and an ID token (expires at t+60m).
    2. t+60m to t+60..90m: the ID token has expired, but the access token has not, so nothing refreshes. Identity.UpstreamIDTokens carries an expired ID token for up to about 30 minutes.
    3. Once the access token expires, refresh runs, Entra returns a fresh ID token, and the consumer recovers until the next cycle.

    Any consumer that needs a currently valid ID token fails closed throughout step 2 of every cycle. One example is a consumer that evaluates the ID token's claims as an authorization input and treats exp as its staleness signal. This is a recurring outage window, not a permanent one. It affects any provider whose ID-token lifetime is shorter than its access-token lifetime, which includes some Okta configurations as well as Entra.

    Proposal: an opt-in ID-token staleness trigger

    This is direction 1 from the issue body, made opt-in:

    • Add an opt-in setting to InProcessService, either a constructor option or a per-provider config flag. The name is open; something like RefreshOnExpiredIDToken.
    • When the setting is enabled, the stored row has a non-empty IDToken whose exp has passed (minus a small skew), and a refresh token exists, refresh exactly as for an expired access token. This applies in both GetValidTokens and GetAllUpstreamCredentials through refreshOrFail, so the existing singleflight/CAS path is reused. Parsing exp once at store time and keeping it on UpstreamTokens (for example IDTokenExpiresAt) avoids re-parsing the JWT on every read.
    • Why opt-in: the Identity.UpstreamIDTokens doc comment (identity.go#L157-L176) and the comment at service.go#L137-L145 deliberately treat ID-token claims as login-time facts that claim-readers should not expire. The in-tree consumers keep that contract. An opt-in trigger serves consumers that need a fresh token without changing it for anyone else, and adds no extra refresh traffic by default.
    • When the provider omits id_token on refresh: keep today's carry-forward. The consumer still sees an expired ID token and fails closed, so this proposal does not mask the omitted-id_token case in the issue body. Directions 2 and 3 remain the way to make that case visible. With the trigger enabled, the refresh-on-every-read loop this could cause should be bounded. For example, skip the ID-token trigger after a refresh that returned no id_token until the access token itself next expires.
    • When there is no refresh token: do not trigger. Return the stored row unchanged, as today.

    Acceptance criteria

    • Unit test (pkg/auth/upstreamtoken): with the option enabled, a stored row with a valid access token and an expired ID token triggers exactly one refresh through the refresher, and the returned UpstreamCredential.IDToken is the rotated one. Cover both GetValidTokens and GetAllUpstreamCredentials.
    • Unit test: with the option enabled and no refresh token, no refresh is attempted and the stored (expired) ID token is returned.
    • Unit test: with the option enabled and a refresh response that omits id_token, the old ID token is carried forward (consumer sees it stale), and a subsequent read before access-token expiry does not refresh again.
    • Unit test: with the option disabled (the default), a row with a valid access token and an expired ID token triggers no refresh, so existing behaviour is unchanged.
    • The Identity.UpstreamIDTokens doc comment states that, with the option enabled, the ID token is refreshed when expired, and keeps the omitted-id_token caveat.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions