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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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()})
|
||||
|
||||
Reference in New Issue
Block a user