fix(auth): prevent competing remember-me token rotations (#39686) (#39688)

Partially backport #39686

models/auth/auth_token.go

@@ -44,9 +44,10 @@ func GetAuthTokenByID(ctx context.Context, id string) (*AuthToken, error) {
 	return at, nil
 }
 
-func UpdateAuthTokenByID(ctx context.Context, t *AuthToken) error {
-	_, err := db.GetEngine(ctx).ID(t.ID).Cols("token_hash", "expires_unix").Update(t)
-	return err
+func UpdateAuthTokenByID(ctx context.Context, t *AuthToken, oldHash string) (bool, error) {
+	affected, err := db.GetEngine(ctx).ID(t.ID).Where("token_hash = ?", oldHash).
+		Cols("token_hash", "expires_unix").Update(t)
+	return affected == 1, err
 }
 
 func DeleteAuthTokenByID(ctx context.Context, id string) error {

routers/web/auth/auth.go

@@ -77,9 +77,9 @@ func prepareCommonAuthPageData(ctx *context.Context, opt CommonAuthOptions) {
 
 // autoSignIn reads cookie and try to auto-login.
 func autoSignIn(ctx *context.Context) (bool, error) {
-	isSucceed := false
+	keepRememberCookie := false
 	defer func() {
-		if !isSucceed {
+		if !keepRememberCookie {
 			ctx.DeleteSiteCookie(setting.CookieRememberName)
 		}
 	}()
@@ -112,14 +112,20 @@ func autoSignIn(ctx *context.Context) (bool, error) {
 		return false, fmt.Errorf("HasTwoFactorOrWebAuthn: %w", err)
 	}
 
-	isSucceed = true
-
 	nt, token, err := auth_service.RegenerateAuthToken(ctx, t)
+	if errors.Is(err, auth_service.ErrAuthTokenRotationConflict) {
+		keepRememberCookie = true // A late response must not clear the winner's cookie.
+		return false, nil
+	}
+	if errors.Is(err, auth_service.ErrAuthTokenExpired) {
+		return false, nil
+	}
 	if err != nil {
 		return false, err
 	}
 
 	ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day)
+	keepRememberCookie = true
 
 	if err := regenerateSession(ctx, map[string]any{
 		session.KeyUID:                  u.ID,

services/auth/auth_token.go

@@ -24,9 +24,10 @@ import (
 // If a device uses the token to login into the instance, a fresh token gets generated which has the same id but a new hash.
 
 var (
-	ErrAuthTokenInvalidFormat = util.NewInvalidArgumentErrorf("auth token has an invalid format")
-	ErrAuthTokenExpired       = util.NewInvalidArgumentErrorf("auth token has expired")
-	ErrAuthTokenInvalidHash   = util.NewInvalidArgumentErrorf("auth token is invalid")
+	ErrAuthTokenInvalidFormat    = util.NewInvalidArgumentErrorf("auth token has an invalid format")
+	ErrAuthTokenExpired          = util.NewInvalidArgumentErrorf("auth token has expired")
+	ErrAuthTokenInvalidHash      = util.NewInvalidArgumentErrorf("auth token is invalid")
+	ErrAuthTokenRotationConflict = util.NewInvalidArgumentErrorf("auth token was rotated by another request")
 )
 
 func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, error) {
@@ -76,9 +77,23 @@ func RegenerateAuthToken(ctx context.Context, t *auth_model.AuthToken) (*auth_mo
 		ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour),
 	}
 
-	if err := auth_model.UpdateAuthTokenByID(ctx, newToken); err != nil {
+	updated, err := auth_model.UpdateAuthTokenByID(ctx, newToken, t.TokenHash)
+	if err != nil {
 		return nil, "", err
 	}
+	if !updated {
+		current, err := auth_model.GetAuthTokenByID(ctx, t.ID)
+		if errors.Is(err, util.ErrNotExist) {
+			return nil, "", ErrAuthTokenExpired
+		}
+		if err != nil {
+			return nil, "", err
+		}
+		if current.ExpiresUnix < timeutil.TimeStampNow() {
+			return nil, "", ErrAuthTokenExpired
+		}
+		return nil, "", ErrAuthTokenRotationConflict
+	}
 
 	return newToken, token, nil
 }

services/auth/auth_token_test.go

@@ -13,6 +13,7 @@ import (
 	"gitea.dev/modules/util"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCheckAuthToken(t *testing.T) {
@@ -105,5 +106,25 @@ func TestRegenerateAuthToken(t *testing.T) {
 	assert.NotEqual(t, token, token2)
 	assert.NotEqual(t, at.ExpiresUnix, at2.ExpiresUnix)
 
+	// Only one request may rotate the same validated cookie.
+	competing, competingToken, err := RegenerateAuthToken(t.Context(), at)
+	require.ErrorIs(t, err, ErrAuthTokenRotationConflict)
+	require.Nil(t, competing)
+	require.Empty(t, competingToken)
+	current, err := CheckAuthToken(t.Context(), at2.ID+":"+token2)
+	require.NoError(t, err)
+	require.Equal(t, at2.TokenHash, current.TokenHash)
+
 	assert.NoError(t, auth_model.DeleteAuthTokenByID(t.Context(), at.ID))
 }
+
+func TestRegenerateRevokedAuthToken(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	at, _, err := CreateAuthTokenForUserID(t.Context(), 2)
+	require.NoError(t, err)
+	require.NoError(t, auth_model.DeleteAuthTokenByID(t.Context(), at.ID))
+	rotated, token, err := RegenerateAuthToken(t.Context(), at)
+	require.ErrorIs(t, err, ErrAuthTokenExpired)
+	require.Nil(t, rotated)
+	require.Empty(t, token)
+}