mirror of
https://github.com/coder/coder.git
synced 2026-06-03 13:08:25 +00:00
Kyle Carberry
53e52aef78
## Problem When multiple concurrent callers (e.g., parallel workspace builds) read the same single-use OAuth2 refresh token from the database and race to exchange it with the provider, the first caller succeeds but subsequent callers get `bad_refresh_token`. The losing caller then **clears the valid new token** from the database, permanently breaking the auth link until the user manually re-authenticates. This is reliably reproducible when launching multiple workspaces simultaneously with GitHub App external auth and user-to-server token expiration enabled. ## Solution Two layers of protection: ### 1. Singleflight deduplication (`Config.RefreshToken` + `ObtainOIDCAccessToken`) Concurrent callers for the same user/provider share a single refresh call via `golang.org/x/sync/singleflight`, keyed by `userID`. The singleflight callback re-reads the link from the database to pick up any token already refreshed by a prior in-flight call, avoiding redundant IDP round-trips entirely. ### 2. Optimistic locking on `UpdateExternalAuthLinkRefreshToken` The SQL `WHERE` clause now includes `AND oauth_refresh_token = @old_oauth_refresh_token`, so if two replicas (HA) race past singleflight, the loser's destructive UPDATE is a harmless no-op rather than overwriting the winner's valid token. ## Changes | File | Change | |------|--------| | `coderd/externalauth/externalauth.go` | Added `singleflight.Group` to `Config`; split `RefreshToken` into public wrapper + `refreshTokenInner`; pass `OldOauthRefreshToken` to DB update | | `coderd/provisionerdserver/provisionerdserver.go` | Wrapped OIDC refresh in `ObtainOIDCAccessToken` with package-level singleflight | | `coderd/database/queries/externalauth.sql` | Added optimistic lock (`WHERE ... AND oauth_refresh_token = @old_oauth_refresh_token`) | | `coderd/database/queries.sql.go` | Regenerated | | `coderd/database/querier.go` | Regenerated | | `coderd/database/dbauthz/dbauthz_test.go` | Updated test params for new field | | `coderd/externalauth/externalauth_test.go` | Added `ConcurrentRefreshDedup` test; updated existing tests for singleflight DB re-read | ## Testing - **New test `ConcurrentRefreshDedup`**: 5 goroutines call `RefreshToken` concurrently, asserts IDP refresh called exactly once, all callers get same token. - All existing `TestRefreshToken/*` subtests updated and passing. - `TestObtainOIDCAccessToken` passing. - `dbauthz` tests passing.