opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(coderd/externalauth): detect concurrent refresh race to prevent cache poisoning (#24228)

Browse Source 
<!--

If you have used AI to produce some or all of this PR, please ensure you
have read our [AI Contribution
guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING)
before submitting.

-->

Fixes https://github.com/coder/coder/issues/17069

Builds on #24332 and #24334 which addressed token persistence and rate
limit handling.

## Problem

When multiple concurrent requests race to refresh an expiring external
auth token, providers with single-use refresh tokens (e.g., GitHub Apps)
reject all but the first refresh attempt with `bad_refresh_token`. The
losing request caches this transient error in the
`oauth_refresh_failure_reason` database column and clears the refresh
token, blocking all subsequent refresh attempts until the user manually
re-authenticates.

This is common for users with multiple terminals, IDE connections, or
workspaces open, all of which poll the external auth endpoint and
trigger concurrent refreshes when the token nears expiry. Database
analysis showed 5 of 7 affected users failed within 5-10 seconds of
token expiry, matching the Go oauth2 library's `expiryDelta` window.

## Fix

Before caching a `bad_refresh_token` failure, re-read the external auth
link from the database. If the refresh token has changed (indicating a
concurrent caller already refreshed successfully), return the winner's
updated link instead of writing a failure. An empty-string guard ensures
a token cleared by another loser isn't mistaken for a winner's
successful refresh.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Garrett Delfosse <garrett@coder.com>
This commit is contained in:
Jason Barnett
2026-05-04 14:02:07 -04:00
committed by GitHub
co-authored by Claude Opus 4.6 Garrett Delfosse
parent 1ecdad689b
commit da6e708bd2
2 changed files with 72 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/externalauth/externalauth.go
+18
View File
@@ -200,6 +200,24 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, externalAu
//
// The error message is saved for debugging purposes.
if isFailedRefresh(existingToken, err) {
// Before caching the failure, re-read the external auth link
// from the database. A concurrent request may have already
// refreshed the token successfully, consuming the single-use
// refresh token (e.g., GitHub App tokens). In that case our
// "bad_refresh_token" error is a false positive from losing
// the race, and we should use the winner's updated token
// instead of poisoning the database with a cached failure.
currentLink, readErr := db.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{
ProviderID: externalAuthLink.ProviderID,
UserID: externalAuthLink.UserID,
})
if readErr == nil && currentLink.OAuthRefreshToken != externalAuthLink.OAuthRefreshToken {
// Another caller won the refresh race and stored a new
// refresh token. Return their updated link instead of
// caching a failure.
return currentLink, nil
}
reason := err.Error()
if len(reason) > failureReasonLimit {
// Limit the length of the error message to prevent