diff --git a/models/auth/auth_token.go b/models/auth/auth_token.go index 0eaf700c2e2..757ed97a6ef 100644 --- a/models/auth/auth_token.go +++ b/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 { diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 54ab556d986..0fd6bf4967d 100644 --- a/routers/web/auth/auth.go +++ b/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, diff --git a/services/auth/auth_token.go b/services/auth/auth_token.go index 809dff5b261..a68f0f26b62 100644 --- a/services/auth/auth_token.go +++ b/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 } diff --git a/services/auth/auth_token_test.go b/services/auth/auth_token_test.go index ab24381d0a9..86e04be5e0a 100644 --- a/services/auth/auth_token_test.go +++ b/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) +}