Skip to content

Commit bdbf01e

Browse files
reactimacstockton
andauthored
fix(oauth): prevent authorization-code replay race at /oauth/token (#2612)
# fix(oauth): prevent authorization-code replay race at `/oauth/token` ## What Fixes an authorization-code replay race (TOCTOU) in the `authorization_code` grant at `POST /oauth/token`. ## Why `handleAuthorizationCodeGrant` read the code (`FindOAuthServerAuthorizationByCode`) and validated expiry/client/redirect_uri/PKCE **with no transaction or row lock**, opening a transaction only later to issue tokens and `Destroy` the code. Two requests with the same code can both clear validation before either commits; `tx.Destroy`'s `DELETE` matching 0 rows still commits under READ COMMITTED — so **both mint a full token set for one single-use code**, violating RFC 6749 §4.1.2 / PKCE. The authorize/consent path was hardened in #2512; token-exchange never got the equivalent locked read-modify-write. ## Change - Reuse `FindOAuthServerAuthorizationByIDForUpdate` (#2512's `FOR UPDATE SKIP LOCKED` helper) to lock the row at the top of the issuing transaction. Concurrent redemptions serialize — the loser skips the locked row and gets `invalid_grant`. No new model code. - Propagate `*apierrors.OAuthError` out of the transaction so `invalid_grant` reaches the client instead of a 500. ## Impact No schema/API changes. Behavior differs only under concurrent redemption of the same code. ## Testing `go build ./...` and `go vet` pass. Reproduction: N concurrent exchanges of one fresh code behind a start barrier — before, ≥2 responses carried `access_token`; after, exactly one succeeds and the rest return `400 invalid_grant` (`success <= 1`). Co-authored-by: Chris Stockton <180184+cstockton@users.noreply.github.com>
1 parent 8748000 commit bdbf01e

2 files changed

Lines changed: 75 additions & 0 deletions

File tree

internal/api/oauthserver/handlers.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,13 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon
394394
grantParams.Scopes = &scopes
395395

396396
err = db.Transaction(func(tx *storage.Connection) error {
397+
if _, terr := models.FindOAuthServerAuthorizationByIDForUpdate(tx, authorization.AuthorizationID); terr != nil {
398+
if models.IsNotFoundError(terr) {
399+
return apierrors.NewOAuthError("invalid_grant", "Invalid authorization code")
400+
}
401+
return apierrors.NewInternalServerError("Error locking authorization code").WithInternalError(terr)
402+
}
403+
397404
authMethod := models.OAuthProviderAuthorizationCode
398405

399406
// Create audit log entry for OAuth token exchange
@@ -424,6 +431,9 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon
424431
if httpErr, ok := err.(*apierrors.HTTPError); ok {
425432
return httpErr
426433
}
434+
if oauthErr, ok := err.(*apierrors.OAuthError); ok {
435+
return oauthErr
436+
}
427437
return apierrors.NewInternalServerError("Error exchanging authorization code").WithInternalError(err)
428438
}
429439

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package oauthserver
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"sync"
7+
"sync/atomic"
8+
"time"
9+
10+
"github.com/gofrs/uuid"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
"github.com/supabase/auth/internal/api/shared"
14+
"github.com/supabase/auth/internal/models"
15+
)
16+
17+
func (ts *OAuthClientTestSuite) mintApprovedCode(clientID, userID uuid.UUID) string {
18+
auth := models.NewOAuthServerAuthorization(models.NewOAuthServerAuthorizationParams{
19+
ClientID: clientID,
20+
RedirectURI: "https://example.com/callback",
21+
Scope: "profile",
22+
TTL: time.Hour,
23+
})
24+
require.NoError(ts.T(), models.CreateOAuthServerAuthorization(ts.DB, auth))
25+
require.NoError(ts.T(), auth.SetUser(ts.DB, userID))
26+
require.NoError(ts.T(), auth.Approve(ts.DB))
27+
return *auth.AuthorizationCode
28+
}
29+
30+
func (ts *OAuthClientTestSuite) TestAuthCodeReplayRace() {
31+
client, _ := ts.createTestOAuthClient()
32+
user := ts.createTestUser("replay-race@example.com")
33+
code := ts.mintApprovedCode(client.ID, user.ID)
34+
35+
const n = 10
36+
start := make(chan struct{})
37+
var wg sync.WaitGroup
38+
var success int32
39+
40+
for i := 0; i < n; i++ {
41+
wg.Add(1)
42+
go func() {
43+
defer wg.Done()
44+
45+
req := httptest.NewRequest(http.MethodPost, "/oauth/token", nil)
46+
ctx := shared.WithOAuthServerClient(req.Context(), client)
47+
req = req.WithContext(ctx)
48+
w := httptest.NewRecorder()
49+
params := &OAuthTokenParams{GrantType: GrantTypeAuthorizationCode, Code: code}
50+
51+
<-start
52+
if err := ts.Server.handleAuthorizationCodeGrant(ctx, w, req, params); err == nil {
53+
atomic.AddInt32(&success, 1)
54+
}
55+
}()
56+
}
57+
58+
close(start)
59+
wg.Wait()
60+
61+
assert.Equal(ts.T(), int32(1), success, "authorization code must be single-use: expected exactly one successful redemption, got %d", success)
62+
63+
_, err := models.FindOAuthServerAuthorizationByCode(ts.DB, code)
64+
assert.True(ts.T(), models.IsNotFoundError(err), "authorization code should be consumed after redemption")
65+
}

0 commit comments

Comments
 (0)