From 283e8f6b46e37776124766758250941b72e2b986 Mon Sep 17 00:00:00 2001 From: liangxu Date: Fri, 3 Jul 2026 20:55:19 +0800 Subject: [PATCH] fix: prevent forced re-login from concurrent refresh token rotation Rotated refresh tokens now stay valid for a 60s grace window so concurrent refreshes (e.g. multiple browser tabs) each obtain a valid new pair instead of being logged out. Admin-web additionally serializes refreshes across tabs via the Web Locks API and reuses tokens already rotated by another tab. Co-Authored-By: Claude Fable 5 --- apps/admin-web/src/lib/api.ts | 99 ++++++++++++------- server/internal/shared/auth/session_store.go | 46 +++++++-- .../shared/auth/session_store_test.go | 56 +++++++++++ 3 files changed, 158 insertions(+), 43 deletions(-) diff --git a/apps/admin-web/src/lib/api.ts b/apps/admin-web/src/lib/api.ts index e871601..bd13d2b 100644 --- a/apps/admin-web/src/lib/api.ts +++ b/apps/admin-web/src/lib/api.ts @@ -179,6 +179,7 @@ import { markStoredMembershipBlocked, readStoredSession, setStoredTokens, + type SessionSnapshot, } from './session' const rawBaseURL = import.meta.env.VITE_API_BASE_URL ?? '' @@ -265,6 +266,64 @@ export function shouldRefreshAccessToken( return !expiresAt || expiresAt * 1000 - Date.now() <= minRemainingMs } +const authRefreshLockName = 'geo.admin-web.auth-refresh' + +async function withCrossTabRefreshLock(task: () => Promise): Promise { + const locks = typeof navigator !== 'undefined' ? navigator.locks : undefined + if (!locks?.request) { + return task() + } + return (await locks.request(authRefreshLockName, task)) as T +} + +function storedTokensFromSnapshot(snapshot: SessionSnapshot): AuthTokens | null { + if (!snapshot.accessToken || !snapshot.refreshToken) { + return null + } + return { + accessToken: snapshot.accessToken, + refreshToken: snapshot.refreshToken, + expiresAt: snapshot.accessTokenExpiresAt ?? undefined, + } +} + +async function performStoredAuthRefresh(initialRefreshToken: string): Promise { + // Another tab may have rotated the token while we waited for the lock: + // reuse its result instead of racing the one-time-use refresh token. + const current = readStoredSession() + const currentTokens = storedTokensFromSnapshot(current) + if ( + currentTokens && + currentTokens.refreshToken !== initialRefreshToken && + !shouldRefreshAccessToken(current.accessTokenExpiresAt) + ) { + return currentTokens + } + + const requestedRefreshToken = currentTokens?.refreshToken ?? initialRefreshToken + + try { + const data = await publicClient.post( + '/api/auth/refresh', + { refresh_token: requestedRefreshToken }, + ) + const tokens = toAuthTokens(data) + setStoredTokens(tokens) + return tokens + } catch (error) { + if (isAuthSessionExpiredError(error)) { + const snapshot = readStoredSession() + const fallbackTokens = storedTokensFromSnapshot(snapshot) + if (fallbackTokens && fallbackTokens.refreshToken !== requestedRefreshToken) { + return fallbackTokens + } + + expireStoredSession() + } + throw markAuthErrorHandled(error) + } +} + export async function refreshStoredAuthSession( refreshToken = getStoredRefreshToken(), ): Promise { @@ -273,42 +332,12 @@ export async function refreshStoredAuthSession( throw authRequiredError() } - const requestedRefreshToken = refreshToken - if (!storedRefreshPromise) { - storedRefreshPromise = publicClient - .post('/api/auth/refresh', { - refresh_token: requestedRefreshToken, - }) - .then((data) => { - const tokens = toAuthTokens(data) - setStoredTokens(tokens) - return tokens - }) - .catch((error) => { - if (isAuthSessionExpiredError(error)) { - const current = readStoredSession() - if ( - current.accessToken && - current.refreshToken && - current.refreshToken !== requestedRefreshToken - ) { - const tokens = { - accessToken: current.accessToken, - refreshToken: current.refreshToken, - expiresAt: current.accessTokenExpiresAt ?? undefined, - } - setStoredTokens(tokens) - return tokens - } - - expireStoredSession() - } - throw markAuthErrorHandled(error) - }) - .finally(() => { - storedRefreshPromise = null - }) + storedRefreshPromise = withCrossTabRefreshLock(() => + performStoredAuthRefresh(refreshToken), + ).finally(() => { + storedRefreshPromise = null + }) } return storedRefreshPromise diff --git a/server/internal/shared/auth/session_store.go b/server/internal/shared/auth/session_store.go index 46ec45e..332b8db 100644 --- a/server/internal/shared/auth/session_store.go +++ b/server/internal/shared/auth/session_store.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strconv" + "strings" "sync" "time" @@ -21,12 +22,16 @@ local current = redis.call("GET", KEYS[1]) if not current then return 0 end -if current ~= ARGV[1] then - return -1 +if current == ARGV[1] then + redis.call("PSETEX", KEYS[1], ARGV[4], "grace:" .. ARGV[1]) + redis.call("PSETEX", KEYS[2], ARGV[3], ARGV[2]) + return 1 end -redis.call("DEL", KEYS[1]) -redis.call("PSETEX", KEYS[2], ARGV[3], ARGV[2]) -return 1 +if current == "grace:" .. ARGV[1] then + redis.call("PSETEX", KEYS[2], ARGV[3], ARGV[2]) + return 1 +end +return -1 `) var setMaxSessionVersionScript = redis.NewScript(` @@ -42,6 +47,13 @@ return next_number const sessionVersionFallbackTTL = 30 * 24 * time.Hour +// refreshRotationGraceTTL keeps a rotated refresh token usable for a short +// window so concurrent refreshes (e.g. multiple browser tabs racing with the +// same token) each obtain a valid new pair instead of being logged out. +const refreshRotationGraceTTL = 60 * time.Second + +const rotationGracePrefix = "grace:" + type SessionStore struct { rdb *redis.Client fallback *memorySessionStore @@ -71,10 +83,20 @@ func (s *SessionStore) GetRefresh(ctx context.Context, jti string) (string, erro if s.rdb != nil { value, err := s.rdb.Get(ctx, key).Result() if err == nil { + if strings.HasPrefix(value, rotationGracePrefix) { + return "", ErrRefreshSessionNotFound + } return value, nil } } - return s.fallback.get(key) + value, err := s.fallback.get(key) + if err != nil { + return "", err + } + if strings.HasPrefix(value, rotationGracePrefix) { + return "", ErrRefreshSessionNotFound + } + return value, nil } func (s *SessionStore) DeleteRefresh(ctx context.Context, jti string) error { @@ -104,6 +126,7 @@ func (s *SessionStore) RotateRefresh(ctx context.Context, oldJTI, oldHash, newJT oldHash, newHash, ttl.Milliseconds(), + refreshRotationGraceTTL.Milliseconds(), ).Int() if err != nil { return s.fallback.rotate(oldKey, oldHash, newKey, newHash, ttl) @@ -298,10 +321,17 @@ func (s *memorySessionStore) rotate(oldKey, oldHash, newKey, newHash string, ttl delete(s.items, oldKey) return ErrRefreshSessionNotFound } - if item.value != oldHash { + switch item.value { + case oldHash: + s.items[oldKey] = memorySessionItem{ + value: rotationGracePrefix + oldHash, + expiresAt: time.Now().Add(refreshRotationGraceTTL), + } + case rotationGracePrefix + oldHash: + // Already rotated within the grace window; keep the marker as-is. + default: return ErrRefreshTokenMismatch } - delete(s.items, oldKey) if ttl > 0 { s.items[newKey] = memorySessionItem{ value: newHash, diff --git a/server/internal/shared/auth/session_store_test.go b/server/internal/shared/auth/session_store_test.go index f2d5fb3..f64e19e 100644 --- a/server/internal/shared/auth/session_store_test.go +++ b/server/internal/shared/auth/session_store_test.go @@ -43,6 +43,62 @@ func TestSessionStoreRotateRefreshSuccess(t *testing.T) { assert.Equal(t, "new-hash", value) } +func TestSessionStoreRotateRefreshAllowsConcurrentReuseWithinGrace(t *testing.T) { + mr := miniredis.RunT(t) + client := goredis.NewClient(&goredis.Options{Addr: mr.Addr()}) + t.Cleanup(func() { _ = client.Close() }) + + store := NewSessionStore(client) + ctx := context.Background() + + require.NoError(t, store.SaveRefresh(ctx, "old-jti", "old-hash", time.Minute)) + + // Tab A rotates first, tab B retries with the same old token within the + // grace window: both must obtain valid new sessions. + require.NoError(t, store.RotateRefresh(ctx, "old-jti", "old-hash", "new-jti-a", "new-hash-a", 2*time.Minute)) + require.NoError(t, store.RotateRefresh(ctx, "old-jti", "old-hash", "new-jti-b", "new-hash-b", 2*time.Minute)) + + valueA, err := store.GetRefresh(ctx, "new-jti-a") + require.NoError(t, err) + assert.Equal(t, "new-hash-a", valueA) + + valueB, err := store.GetRefresh(ctx, "new-jti-b") + require.NoError(t, err) + assert.Equal(t, "new-hash-b", valueB) + + // A different (wrong) old hash must still be rejected during grace. + err = store.RotateRefresh(ctx, "old-jti", "wrong-hash", "new-jti-c", "new-hash-c", 2*time.Minute) + require.ErrorIs(t, err, ErrRefreshTokenMismatch) + + // The rotated old token stays hidden from session lookups during grace. + _, err = store.GetRefresh(ctx, "old-jti") + require.ErrorIs(t, err, ErrRefreshSessionNotFound) +} + +func TestMemorySessionStoreRotateAllowsConcurrentReuseWithinGrace(t *testing.T) { + store := NewSessionStore(nil) + ctx := context.Background() + + require.NoError(t, store.SaveRefresh(ctx, "old-jti", "old-hash", time.Minute)) + + require.NoError(t, store.RotateRefresh(ctx, "old-jti", "old-hash", "new-jti-a", "new-hash-a", 2*time.Minute)) + require.NoError(t, store.RotateRefresh(ctx, "old-jti", "old-hash", "new-jti-b", "new-hash-b", 2*time.Minute)) + + _, err := store.GetRefresh(ctx, "old-jti") + assert.Error(t, err) + + valueA, err := store.GetRefresh(ctx, "new-jti-a") + require.NoError(t, err) + assert.Equal(t, "new-hash-a", valueA) + + valueB, err := store.GetRefresh(ctx, "new-jti-b") + require.NoError(t, err) + assert.Equal(t, "new-hash-b", valueB) + + err = store.RotateRefresh(ctx, "old-jti", "wrong-hash", "new-jti-c", "new-hash-c", 2*time.Minute) + require.ErrorIs(t, err, ErrRefreshTokenMismatch) +} + func TestSessionStoreRotateRefreshRejectsMismatch(t *testing.T) { mr := miniredis.RunT(t) client := goredis.NewClient(&goredis.Options{Addr: mr.Addr()})