Просмотр исходного кода

fix: grace period 30s для refresh token при ротации

Race condition: при быстром F5 первый /auth/refresh
успевает удалить старый токен, но browser отбрасывает
Set-Cookie с новым из-за навигации. Второй F5 шлёт
старый (уже удалённый) токен → 401 → деавторизация.

Решение:
- Вместо DELETE при ротации — UPDATE SET replaced_at=now()
- GetValid теперь принимает токены с replaced_at, если
  он был установлен не более 30 секунд назад
- Новая миграция 000012: колонка replaced_at
- Secure-флаг куки определяется по протоколу, а не APP_ENV
  (добавлен X-Forwarded-Proto в Caddyfile)
neyrogovnarik 1 месяц назад
Родитель
Сommit
7df386d818

+ 7 - 6
backend/internal/models/refresh_token.go

@@ -3,10 +3,11 @@ package models
 import "time"
 
 type RefreshToken struct {
-	ID        string
-	UserID    string
-	TokenHash string
-	ExpiresAt time.Time
-	CreatedAt time.Time
-	RevokedAt *time.Time
+	ID          string
+	UserID      string
+	TokenHash   string
+	ExpiresAt   time.Time
+	CreatedAt   time.Time
+	RevokedAt   *time.Time
+	ReplacedAt  *time.Time
 }

+ 31 - 29
backend/internal/repository/refresh_tokens.go

@@ -1,4 +1,3 @@
-// Package repository
 package repository
 
 import (
@@ -43,14 +42,17 @@ func (r *RefreshTokenRepo) Create(ctx context.Context, userID, plainToken string
 func (r *RefreshTokenRepo) GetValid(ctx context.Context, plainToken string) (*models.RefreshToken, error) {
 	tokenHash := hashToken(plainToken)
 	row := r.pool.QueryRow(ctx,
-		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at
+		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at, replaced_at
 		 FROM refresh_tokens
-		 WHERE token_hash = $1 AND expires_at > now() AND revoked_at IS NULL`,
+		 WHERE token_hash = $1
+		   AND expires_at > now()
+		   AND revoked_at IS NULL
+		   AND (replaced_at IS NULL OR replaced_at > now() - interval '30 seconds')`,
 		tokenHash,
 	)
 
 	var rt models.RefreshToken
-	err := row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt)
+	err := row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt, &rt.ReplacedAt)
 	if err != nil {
 		if err == pgx.ErrNoRows {
 			return nil, nil
@@ -62,7 +64,6 @@ func (r *RefreshTokenRepo) GetValid(ctx context.Context, plainToken string) (*mo
 
 // GetValidAndLock atomically reads a valid token and locks the row FOR UPDATE.
 // Returns the token and a cleanup func that releases the lock on error.
-// Used to prevent race conditions on concurrent refresh token rotation.
 func (r *RefreshTokenRepo) GetValidAndLock(ctx context.Context, plainToken string) (*models.RefreshToken, func(), error) {
 	tx, err := r.pool.Begin(ctx)
 	if err != nil {
@@ -71,15 +72,18 @@ func (r *RefreshTokenRepo) GetValidAndLock(ctx context.Context, plainToken strin
 
 	tokenHash := hashToken(plainToken)
 	row := tx.QueryRow(ctx,
-		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at
+		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at, replaced_at
 		 FROM refresh_tokens
-		 WHERE token_hash = $1 AND expires_at > now() AND revoked_at IS NULL
+		 WHERE token_hash = $1
+		   AND expires_at > now()
+		   AND revoked_at IS NULL
+		   AND (replaced_at IS NULL OR replaced_at > now() - interval '30 seconds')
 		 FOR UPDATE`,
 		tokenHash,
 	)
 
 	var rt models.RefreshToken
-	err = row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt)
+	err = row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt, &rt.ReplacedAt)
 	if err != nil {
 		tx.Rollback(ctx)
 		if err == pgx.ErrNoRows {
@@ -94,8 +98,9 @@ func (r *RefreshTokenRepo) GetValidAndLock(ctx context.Context, plainToken strin
 	return &rt, cleanup, nil
 }
 
-// RotateToken atomically deletes the old refresh token and creates a new one.
-// Must be called after GetValidAndLock within the same request lifecycle.
+// RotateToken atomically replaces the old refresh token and creates a new one
+// within a transaction. Uses soft-replace (sets replaced_at) to allow a grace
+// period where the old token is still accepted.
 func (r *RefreshTokenRepo) RotateToken(ctx context.Context, oldPlainToken, newPlainToken, userID string, expiresAt time.Time) error {
 	tx, err := r.pool.Begin(ctx)
 	if err != nil {
@@ -104,12 +109,15 @@ func (r *RefreshTokenRepo) RotateToken(ctx context.Context, oldPlainToken, newPl
 	defer tx.Rollback(ctx)
 
 	oldHash := hashToken(oldPlainToken)
-	_, err = tx.Exec(ctx,
-		`DELETE FROM refresh_tokens WHERE token_hash = $1`,
+	tag, err := tx.Exec(ctx,
+		`UPDATE refresh_tokens SET replaced_at = now() WHERE token_hash = $1 AND replaced_at IS NULL`,
 		oldHash,
 	)
 	if err != nil {
-		return fmt.Errorf("delete old token: %w", err)
+		return fmt.Errorf("replace old token: %w", err)
+	}
+	if tag.RowsAffected() == 0 {
+		return fmt.Errorf("old token not found or already replaced")
 	}
 
 	newHash := hashToken(newPlainToken)
@@ -127,14 +135,14 @@ func (r *RefreshTokenRepo) RotateToken(ctx context.Context, oldPlainToken, newPl
 func (r *RefreshTokenRepo) GetRevoked(ctx context.Context, plainToken string) (*models.RefreshToken, error) {
 	tokenHash := hashToken(plainToken)
 	row := r.pool.QueryRow(ctx,
-		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at
+		`SELECT id, user_id, token_hash, expires_at, created_at, revoked_at, replaced_at
 		 FROM refresh_tokens
 		 WHERE token_hash = $1 AND revoked_at IS NOT NULL`,
 		tokenHash,
 	)
 
 	var rt models.RefreshToken
-	err := row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt)
+	err := row.Scan(&rt.ID, &rt.UserID, &rt.TokenHash, &rt.ExpiresAt, &rt.CreatedAt, &rt.RevokedAt, &rt.ReplacedAt)
 	if err != nil {
 		if err == pgx.ErrNoRows {
 			return nil, nil
@@ -160,30 +168,24 @@ func (r *RefreshTokenRepo) RevokeAllForUser(ctx context.Context, userID string)
 	return err
 }
 
-func (r *RefreshTokenRepo) Delete(ctx context.Context, tokenHash string) error {
-	_, err := r.pool.Exec(ctx,
-		`DELETE FROM refresh_tokens WHERE token_hash = $1`,
-		tokenHash,
-	)
-	return err
-}
-
-// DeleteIfExists atomically deletes a token and returns true if a row was removed.
-// Returns false if the token was already deleted by another request.
-func (r *RefreshTokenRepo) DeleteIfExists(ctx context.Context, tokenHash string) (bool, error) {
+// ReplaceIfExists soft-replaces a token (sets replaced_at) and returns true if a row was updated.
+// Returns false if the token was already replaced by another request.
+func (r *RefreshTokenRepo) ReplaceIfExists(ctx context.Context, tokenHash string) (bool, error) {
 	tag, err := r.pool.Exec(ctx,
-		`DELETE FROM refresh_tokens WHERE token_hash = $1`,
+		`UPDATE refresh_tokens SET replaced_at = now() WHERE token_hash = $1 AND replaced_at IS NULL`,
 		tokenHash,
 	)
 	if err != nil {
-		return false, fmt.Errorf("delete if exists: %w", err)
+		return false, fmt.Errorf("replace if exists: %w", err)
 	}
 	return tag.RowsAffected() > 0, nil
 }
 
 func (r *RefreshTokenRepo) CleanupExpired(ctx context.Context) error {
 	_, err := r.pool.Exec(ctx,
-		`DELETE FROM refresh_tokens WHERE expires_at < now() - interval '1 day'`,
+		`DELETE FROM refresh_tokens
+		 WHERE expires_at < now() - interval '1 day'
+		    OR (replaced_at IS NOT NULL AND replaced_at < now() - interval '1 hour')`,
 	)
 	return err
 }

+ 5 - 5
backend/internal/services/auth.go

@@ -48,7 +48,7 @@ type RefreshTokenRepo interface {
 	Revoke(ctx context.Context, tokenHash string) error
 	RevokeAllForUser(ctx context.Context, userID string) error
 	Delete(ctx context.Context, tokenHash string) error
-	DeleteIfExists(ctx context.Context, tokenHash string) (bool, error)
+	ReplaceIfExists(ctx context.Context, tokenHash string) (bool, error)
 	CleanupExpired(ctx context.Context) error
 }
 
@@ -233,12 +233,12 @@ func (s *AuthService) RefreshSession(ctx context.Context, plainRefreshToken stri
 		return nil, "", ErrUserBanned
 	}
 
-	deleted, err := s.refreshTokenRepo.DeleteIfExists(ctx, storedToken.TokenHash)
+	replaced, err := s.refreshTokenRepo.ReplaceIfExists(ctx, storedToken.TokenHash)
 	if err != nil {
-		logger.ErrorContext(ctx, "failed to delete old refresh token", log.WithError(err))
-		return nil, "", fmt.Errorf("delete old refresh token: %w", err)
+		logger.ErrorContext(ctx, "failed to replace old refresh token", log.WithError(err))
+		return nil, "", fmt.Errorf("replace old refresh token: %w", err)
 	}
-	if !deleted {
+	if !replaced {
 		logger.WarnContext(ctx, "concurrent token rotation detected", slog.String("user_id", user.ID))
 		return nil, "", ErrTokenReused
 	}

+ 1 - 1
backend/internal/services/auth_test.go

@@ -80,7 +80,7 @@ func (m *mockRefreshTokenRepo) Delete(ctx context.Context, tokenHash string) err
 	return nil
 }
 
-func (m *mockRefreshTokenRepo) DeleteIfExists(ctx context.Context, tokenHash string) (bool, error) {
+func (m *mockRefreshTokenRepo) ReplaceIfExists(ctx context.Context, tokenHash string) (bool, error) {
 	return true, nil
 }
 

+ 2 - 0
backend/migrations/000012_add_refresh_tokens_replaced_at.down.sql

@@ -0,0 +1,2 @@
+DROP INDEX IF EXISTS idx_refresh_tokens_replaced_at;
+ALTER TABLE refresh_tokens DROP COLUMN IF EXISTS replaced_at;

+ 2 - 0
backend/migrations/000012_add_refresh_tokens_replaced_at.up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE refresh_tokens ADD COLUMN replaced_at TIMESTAMPTZ;
+CREATE INDEX idx_refresh_tokens_replaced_at ON refresh_tokens(replaced_at);

+ 16 - 0
obsidian_data/Photoplaces_data/atomic-cookie-secure-via-protocol.md

@@ -0,0 +1,16 @@
+## Secure-флаг куки: определение через протокол запроса, а не APP_ENV
+
+**Контекст:** На тестовом сервере (192.168.88.128) Caddy работает через HTTP (порт 80), но `APP_ENV=production`. Cookie refresh_token устанавливалась с `Secure: true`, из-за чего браузер отклонял её на HTTP-соединении → рефреш токен терялся при перезагрузке страницы → деавторизация.
+
+**Суть:** `Secure`-флаг теперь вычисляется динамически:
+- `r.TLS != nil` (прямое HTTPS-соединение)
+- `X-Forwarded-Proto: https` (за прокси — Caddy)
+
+**Что изменилось:**
+- `helpers.go`: добавлена `isSecure(r *http.Request) bool`; `setRefreshTokenCookie` принимает `*http.Request` вместо `bool`
+- `auth.go`, `setup.go`: убран мёртвый `isProd`, вызовы передают `r`
+- `Caddyfile`: добавлен `header_up X-Forwarded-Proto {scheme}` в прокси на backend
+- `main.go`: `NewAuthHandler` и `NewSetupHandler` больше не принимают `appEnv`
+
+**Связанные заметки:** [[atomic-api-contract-refresh-cookie]], [[atomic-global-appenv-package-var]]
+**Источник:** Баг на тестовом сервере — деавторизация после нескольких F5