From c6a53c3172b6da826e5f7f2f59d744308e9567bf Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 22 Nov 2023 17:26:21 +0100 Subject: [PATCH] [SECURITY] Rework long-term authentication - This is a 'front-port' of the already existing patch on v1.21 and v1.20, but applied on top of what Gitea has done to rework the LTA mechanism. Forgejo will stick with the reworked mechanism by the Forgejo Security team for the time being. The removal of legacy code (AES-GCM) has been left out. - The current architecture is inherently insecure, because you can construct the 'secret' cookie value with values that are available in the database. Thus provides zero protection when a database is dumped/leaked. - This patch implements a new architecture that's inspired from: [Paragonie Initiative](https://paragonie.com/blog/2015/04/secure-authentication-php-with-long-term-persistence#secure-remember-me-cookies). - Integration testing is added to ensure the new mechanism works. - Removes a setting, because it's not used anymore. (cherry picked from commit e3d6622a63da9c33eed1e3d102cf28a92ff653d6) (cherry picked from commit fef1a6dac5e25579e42d40209c4cfc06879948b9) (cherry picked from commit b0c5165145fa52f2f7bbec1f50b308bdf1d20ef3) (cherry picked from commit 7ad51b9f8d0647eecacd258f6ee26155da3872e1) (cherry picked from commit 64f053f3834e764112cde26bb0d16c5e88d6b2af) (cherry picked from commit f5e78e4c204ce50b800645d614218b6b6096eecb) Conflicts: services/auth/auth_token_test.go https://codeberg.org/forgejo/forgejo/pulls/2069 (cherry picked from commit f69fc23d4bbadf388c7857040ee0774b824e418e) (cherry picked from commit d955ab3ab02cbb7f1245a8cddec426d64d3ac500) (cherry picked from commit 9220088f902a25c4690bcabf5a40a8d02e784182) (cherry picked from commit c73ac636962c41c71814c273510146f0533264ab) (cherry picked from commit 747a176048ea93085b406429db0e25bb21912eda) Conflicts: models/user/user.go routers/web/user/setting/account.go https://codeberg.org/forgejo/forgejo/pulls/2295 --- .deadcode-out | 1 - models/auth/auth_token.go | 96 ++++++++++----- models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v1_20/v3.go | 26 ++++ models/user/user.go | 5 + modules/context/context_cookie.go | 14 +++ routers/install/install.go | 17 +-- routers/web/auth/auth.go | 59 +++++----- routers/web/auth/oauth.go | 3 +- routers/web/user/setting/account.go | 9 ++ services/auth/auth.go | 4 - services/auth/auth_token.go | 123 ------------------- services/auth/auth_token_test.go | 107 ----------------- tests/integration/auth_token_test.go | 163 ++++++++++++++++++++++++++ tests/integration/integration_test.go | 8 ++ 15 files changed, 328 insertions(+), 309 deletions(-) create mode 100644 models/forgejo_migrations/v1_20/v3.go delete mode 100644 services/auth/auth_token.go delete mode 100644 services/auth/auth_token_test.go create mode 100644 tests/integration/auth_token_test.go diff --git a/.deadcode-out b/.deadcode-out index 6b4114dd27..d973769997 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -31,7 +31,6 @@ package "code.gitea.io/gitea/models/asymkey" func HasDeployKey package "code.gitea.io/gitea/models/auth" - func DeleteAuthTokenByID func GetSourceByName func GetWebAuthnCredentialByID func WebAuthnCredentials diff --git a/models/auth/auth_token.go b/models/auth/auth_token.go index 65f1b169eb..2c3ca90734 100644 --- a/models/auth/auth_token.go +++ b/models/auth/auth_token.go @@ -1,60 +1,96 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. +// Copyright 2023 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package auth import ( "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "time" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" - - "xorm.io/builder" ) -var ErrAuthTokenNotExist = util.NewNotExistErrorf("auth token does not exist") +// AuthorizationToken represents a authorization token to a user. +type AuthorizationToken struct { + ID int64 `xorm:"pk autoincr"` + UID int64 `xorm:"INDEX"` + LookupKey string `xorm:"INDEX UNIQUE"` + HashedValidator string + Expiry timeutil.TimeStamp +} -type AuthToken struct { //nolint:revive - ID string `xorm:"pk"` - TokenHash string - UserID int64 `xorm:"INDEX"` - ExpiresUnix timeutil.TimeStamp `xorm:"INDEX"` +// TableName provides the real table name. +func (AuthorizationToken) TableName() string { + return "forgejo_auth_token" } func init() { - db.RegisterModel(new(AuthToken)) + db.RegisterModel(new(AuthorizationToken)) } -func InsertAuthToken(ctx context.Context, t *AuthToken) error { - _, err := db.GetEngine(ctx).Insert(t) - return err +// IsExpired returns if the authorization token is expired. +func (authToken *AuthorizationToken) IsExpired() bool { + return authToken.Expiry.AsLocalTime().Before(time.Now()) } -func GetAuthTokenByID(ctx context.Context, id string) (*AuthToken, error) { - at := &AuthToken{} +// GenerateAuthToken generates a new authentication token for the given user. +// It returns the lookup key and validator values that should be passed to the +// user via a long-term cookie. +func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp) (lookupKey, validator string, err error) { + // Request 64 random bytes. The first 32 bytes will be used for the lookupKey + // and the other 32 bytes will be used for the validator. + rBytes, err := util.CryptoRandomBytes(64) + if err != nil { + return "", "", err + } + hexEncoded := hex.EncodeToString(rBytes) + validator, lookupKey = hexEncoded[64:], hexEncoded[:64] - has, err := db.GetEngine(ctx).ID(id).Get(at) + _, err = db.GetEngine(ctx).Insert(&AuthorizationToken{ + UID: userID, + Expiry: expiry, + LookupKey: lookupKey, + HashedValidator: HashValidator(rBytes[32:]), + }) + return lookupKey, validator, err +} + +// FindAuthToken will find a authorization token via the lookup key. +func FindAuthToken(ctx context.Context, lookupKey string) (*AuthorizationToken, error) { + var authToken AuthorizationToken + has, err := db.GetEngine(ctx).Where("lookup_key = ?", lookupKey).Get(&authToken) if err != nil { return nil, err + } else if !has { + return nil, fmt.Errorf("lookup key %q: %w", lookupKey, util.ErrNotExist) } - if !has { - return nil, ErrAuthTokenNotExist + return &authToken, nil +} + +// DeleteAuthToken will delete the authorization token. +func DeleteAuthToken(ctx context.Context, authToken *AuthorizationToken) error { + _, err := db.DeleteByBean(ctx, authToken) + return err +} + +// DeleteAuthTokenByUser will delete all authorization tokens for the user. +func DeleteAuthTokenByUser(ctx context.Context, userID int64) error { + if userID == 0 { + return nil } - 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) + _, err := db.DeleteByBean(ctx, &AuthorizationToken{UID: userID}) return err } -func DeleteAuthTokenByID(ctx context.Context, id string) error { - _, err := db.GetEngine(ctx).ID(id).Delete(&AuthToken{}) - return err -} - -func DeleteExpiredAuthTokens(ctx context.Context) error { - _, err := db.GetEngine(ctx).Where(builder.Lt{"expires_unix": timeutil.TimeStampNow()}).Delete(&AuthToken{}) - return err +// HashValidator will return a hexified hashed version of the validator. +func HashValidator(validator []byte) string { + h := sha256.New() + h.Write(validator) + return hex.EncodeToString(h.Sum(nil)) } diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 2becf1b713..58f158bd17 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -41,6 +41,8 @@ var migrations = []*Migration{ NewMigration("Add Forgejo Blocked Users table", forgejo_v1_20.AddForgejoBlockedUser), // v1 -> v2 NewMigration("create the forgejo_sem_ver table", forgejo_v1_20.CreateSemVerTable), + // v2 -> v3 + NewMigration("create the forgejo_auth_token table", forgejo_v1_20.CreateAuthorizationTokenTable), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v1_20/v3.go b/models/forgejo_migrations/v1_20/v3.go new file mode 100644 index 0000000000..caa4f1aa99 --- /dev/null +++ b/models/forgejo_migrations/v1_20/v3.go @@ -0,0 +1,26 @@ +// Copyright 2023 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package forgejo_v1_20 //nolint:revive + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +type AuthorizationToken struct { + ID int64 `xorm:"pk autoincr"` + UID int64 `xorm:"INDEX"` + LookupKey string `xorm:"INDEX UNIQUE"` + HashedValidator string + Expiry timeutil.TimeStamp +} + +func (AuthorizationToken) TableName() string { + return "forgejo_auth_token" +} + +func CreateAuthorizationTokenTable(x *xorm.Engine) error { + return x.Sync(new(AuthorizationToken)) +} diff --git a/models/user/user.go b/models/user/user.go index e5245dfbb0..b6b2ee2b04 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -366,6 +366,11 @@ func (u *User) NewGitSig() *git.Signature { // SetPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO // change passwd, salt and passwd_hash_algo fields func (u *User) SetPassword(passwd string) (err error) { + // Invalidate all authentication tokens for this user. + if err := auth.DeleteAuthTokenByUser(db.DefaultContext, u.ID); err != nil { + return err + } + if u.Salt, err = GetUserSalt(); err != nil { return err } diff --git a/modules/context/context_cookie.go b/modules/context/context_cookie.go index b6f8dadb56..39e3218d1b 100644 --- a/modules/context/context_cookie.go +++ b/modules/context/context_cookie.go @@ -7,7 +7,10 @@ import ( "net/http" "strings" + auth_model "code.gitea.io/gitea/models/auth" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web/middleware" ) @@ -40,3 +43,14 @@ func (ctx *Context) DeleteSiteCookie(name string) { func (ctx *Context) GetSiteCookie(name string) string { return middleware.GetSiteCookie(ctx.Req, name) } + +// SetLTACookie will generate a LTA token and add it as an cookie. +func (ctx *Context) SetLTACookie(u *user_model.User) error { + days := 86400 * setting.LogInRememberDays + lookup, validator, err := auth_model.GenerateAuthToken(ctx, u.ID, timeutil.TimeStampNow().Add(int64(days))) + if err != nil { + return err + } + ctx.SetSiteCookie(setting.CookieRememberName, lookup+":"+validator, days) + return nil +} diff --git a/routers/install/install.go b/routers/install/install.go index 5c0290d2cc..1dbfafcd14 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -27,14 +27,12 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" - "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/user" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/routers/common" - auth_service "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/forms" "gitea.com/go-chi/session" @@ -549,20 +547,13 @@ func SubmitInstall(ctx *context.Context) { u, _ = user_model.GetUserByName(ctx, u.Name) } - nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID) - if err != nil { - ctx.ServerError("CreateAuthTokenForUserID", err) - return - } - - ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day) - - // Auto-login for admin - if err = ctx.Session.Set("uid", u.ID); err != nil { + if err := ctx.SetLTACookie(u); err != nil { ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form) return } - if err = ctx.Session.Set("uname", u.Name); err != nil { + + // Auto-login for admin + if err = ctx.Session.Set("uid", u.ID); err != nil { ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form) return } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 3de1f3373d..0e2fd92a28 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -5,6 +5,8 @@ package auth import ( + "crypto/subtle" + "encoding/hex" "errors" "fmt" "net/http" @@ -54,23 +56,39 @@ func autoSignIn(ctx *context.Context) (bool, error) { } }() - if err := auth.DeleteExpiredAuthTokens(ctx); err != nil { - log.Error("Failed to delete expired auth tokens: %v", err) + authCookie := ctx.GetSiteCookie(setting.CookieRememberName) + if len(authCookie) == 0 { + return false, nil } - t, err := auth_service.CheckAuthToken(ctx, ctx.GetSiteCookie(setting.CookieRememberName)) + lookupKey, validator, found := strings.Cut(authCookie, ":") + if !found { + return false, nil + } + + authToken, err := auth.FindAuthToken(ctx, lookupKey) if err != nil { - switch err { - case auth_service.ErrAuthTokenInvalidFormat, auth_service.ErrAuthTokenExpired: + if errors.Is(err, util.ErrNotExist) { return false, nil } return false, err } - if t == nil { + + if authToken.IsExpired() { + err = auth.DeleteAuthToken(ctx, authToken) + return false, err + } + + rawValidator, err := hex.DecodeString(validator) + if err != nil { + return false, err + } + + if subtle.ConstantTimeCompare([]byte(authToken.HashedValidator), []byte(auth.HashValidator(rawValidator))) == 0 { return false, nil } - u, err := user_model.GetUserByID(ctx, t.UserID) + u, err := user_model.GetUserByID(ctx, authToken.UID) if err != nil { if !user_model.IsErrUserNotExist(err) { return false, fmt.Errorf("GetUserByID: %w", err) @@ -80,17 +98,9 @@ func autoSignIn(ctx *context.Context) (bool, error) { isSucceed = true - nt, token, err := auth_service.RegenerateAuthToken(ctx, t) - if err != nil { - return false, err - } - - ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day) - if err := updateSession(ctx, nil, map[string]any{ // Set session IDs - "uid": u.ID, - "uname": u.Name, + "uid": u.ID, }); err != nil { return false, fmt.Errorf("unable to updateSession: %w", err) } @@ -128,10 +138,6 @@ func CheckAutoLogin(ctx *context.Context) bool { // Check auto-login isSucceed, err := autoSignIn(ctx) if err != nil { - if errors.Is(err, auth_service.ErrAuthTokenInvalidHash) { - ctx.Flash.Error(ctx.Tr("auth.remember_me.compromised"), true) - return false - } ctx.ServerError("autoSignIn", err) return true } @@ -306,13 +312,10 @@ func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) { func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRedirect bool) string { if remember { - nt, token, err := auth_service.CreateAuthTokenForUserID(ctx, u.ID) - if err != nil { - ctx.ServerError("CreateAuthTokenForUserID", err) + if err := ctx.SetLTACookie(u); err != nil { + ctx.ServerError("GenerateAuthToken", err) return setting.AppSubURL + "/" } - - ctx.SetSiteCookie(setting.CookieRememberName, nt.ID+":"+token, setting.LogInRememberDays*timeutil.Day) } if err := updateSession(ctx, []string{ @@ -325,8 +328,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe "twofaRemember", "linkAccount", }, map[string]any{ - "uid": u.ID, - "uname": u.Name, + "uid": u.ID, }); err != nil { ctx.ServerError("RegenerateSession", err) return setting.AppSubURL + "/" @@ -745,8 +747,7 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) { log.Trace("User activated: %s", user.Name) if err := updateSession(ctx, nil, map[string]any{ - "uid": user.ID, - "uname": user.Name, + "uid": user.ID, }); err != nil { log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err) ctx.ServerError("ActivateUserEmail", err) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 07140b6674..dc106a1cc0 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1124,8 +1124,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page. if !needs2FA { if err := updateSession(ctx, nil, map[string]any{ - "uid": u.ID, - "uname": u.Name, + "uid": u.ID, }); err != nil { ctx.ServerError("updateSession", err) return diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index c7f194a3b5..371718ba23 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -79,6 +79,15 @@ func AccountPost(ctx *context.Context) { return } } else { + // Re-generate LTA cookie. + if len(ctx.GetSiteCookie(setting.CookieRememberName)) != 0 { + if err := ctx.SetLTACookie(ctx.Doer); err != nil { + ctx.ServerError("SetLTACookie", err) + return + } + } + + log.Trace("User password updated: %s", ctx.Doer.Name) ctx.Flash.Success(ctx.Tr("settings.change_password_success")) } } diff --git a/services/auth/auth.go b/services/auth/auth.go index 6746dc2a54..6e22d8298e 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -78,10 +78,6 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore if err != nil { log.Error(fmt.Sprintf("Error setting session: %v", err)) } - err = sess.Set("uname", user.Name) - if err != nil { - log.Error(fmt.Sprintf("Error setting session: %v", err)) - } // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. diff --git a/services/auth/auth_token.go b/services/auth/auth_token.go deleted file mode 100644 index 6b59238c98..0000000000 --- a/services/auth/auth_token.go +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package auth - -import ( - "context" - "crypto/sha256" - "crypto/subtle" - "encoding/hex" - "errors" - "strings" - "time" - - auth_model "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" -) - -// Based on https://paragonie.com/blog/2015/04/secure-authentication-php-with-long-term-persistence#secure-remember-me-cookies - -// The auth token consists of two parts: ID and token hash -// Every device login creates a new auth token with an individual id and hash. -// 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") -) - -func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, error) { - if len(value) == 0 { - return nil, nil - } - - parts := strings.SplitN(value, ":", 2) - if len(parts) != 2 { - return nil, ErrAuthTokenInvalidFormat - } - - t, err := auth_model.GetAuthTokenByID(ctx, parts[0]) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - return nil, ErrAuthTokenExpired - } - return nil, err - } - - if t.ExpiresUnix < timeutil.TimeStampNow() { - return nil, ErrAuthTokenExpired - } - - hashedToken := sha256.Sum256([]byte(parts[1])) - - if subtle.ConstantTimeCompare([]byte(t.TokenHash), []byte(hex.EncodeToString(hashedToken[:]))) == 0 { - // If an attacker steals a token and uses the token to create a new session the hash gets updated. - // When the victim uses the old token the hashes don't match anymore and the victim should be notified about the compromised token. - return nil, ErrAuthTokenInvalidHash - } - - return t, nil -} - -func RegenerateAuthToken(ctx context.Context, t *auth_model.AuthToken) (*auth_model.AuthToken, string, error) { - token, hash, err := generateTokenAndHash() - if err != nil { - return nil, "", err - } - - newToken := &auth_model.AuthToken{ - ID: t.ID, - TokenHash: hash, - UserID: t.UserID, - ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour), - } - - if err := auth_model.UpdateAuthTokenByID(ctx, newToken); err != nil { - return nil, "", err - } - - return newToken, token, nil -} - -func CreateAuthTokenForUserID(ctx context.Context, userID int64) (*auth_model.AuthToken, string, error) { - t := &auth_model.AuthToken{ - UserID: userID, - ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour), - } - - var err error - t.ID, err = util.CryptoRandomString(10) - if err != nil { - return nil, "", err - } - - token, hash, err := generateTokenAndHash() - if err != nil { - return nil, "", err - } - - t.TokenHash = hash - - if err := auth_model.InsertAuthToken(ctx, t); err != nil { - return nil, "", err - } - - return t, token, nil -} - -func generateTokenAndHash() (string, string, error) { - buf, err := util.CryptoRandomBytes(32) - if err != nil { - return "", "", err - } - - token := hex.EncodeToString(buf) - - hashedToken := sha256.Sum256([]byte(token)) - - return token, hex.EncodeToString(hashedToken[:]), nil -} diff --git a/services/auth/auth_token_test.go b/services/auth/auth_token_test.go deleted file mode 100644 index 23c8d17e59..0000000000 --- a/services/auth/auth_token_test.go +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package auth - -import ( - "testing" - "time" - - auth_model "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/timeutil" - - "github.com/stretchr/testify/assert" -) - -func TestCheckAuthToken(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - t.Run("Empty", func(t *testing.T) { - token, err := CheckAuthToken(db.DefaultContext, "") - assert.NoError(t, err) - assert.Nil(t, token) - }) - - t.Run("InvalidFormat", func(t *testing.T) { - token, err := CheckAuthToken(db.DefaultContext, "dummy") - assert.ErrorIs(t, err, ErrAuthTokenInvalidFormat) - assert.Nil(t, token) - }) - - t.Run("NotFound", func(t *testing.T) { - token, err := CheckAuthToken(db.DefaultContext, "notexists:dummy") - assert.ErrorIs(t, err, ErrAuthTokenExpired) - assert.Nil(t, token) - }) - - t.Run("Expired", func(t *testing.T) { - timeutil.MockSet(time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)) - - at, token, err := CreateAuthTokenForUserID(db.DefaultContext, 2) - assert.NoError(t, err) - assert.NotNil(t, at) - assert.NotEmpty(t, token) - - timeutil.MockUnset() - - at2, err := CheckAuthToken(db.DefaultContext, at.ID+":"+token) - assert.ErrorIs(t, err, ErrAuthTokenExpired) - assert.Nil(t, at2) - - assert.NoError(t, auth_model.DeleteAuthTokenByID(db.DefaultContext, at.ID)) - }) - - t.Run("InvalidHash", func(t *testing.T) { - at, token, err := CreateAuthTokenForUserID(db.DefaultContext, 2) - assert.NoError(t, err) - assert.NotNil(t, at) - assert.NotEmpty(t, token) - - at2, err := CheckAuthToken(db.DefaultContext, at.ID+":"+token+"dummy") - assert.ErrorIs(t, err, ErrAuthTokenInvalidHash) - assert.Nil(t, at2) - - assert.NoError(t, auth_model.DeleteAuthTokenByID(db.DefaultContext, at.ID)) - }) - - t.Run("Valid", func(t *testing.T) { - at, token, err := CreateAuthTokenForUserID(db.DefaultContext, 2) - assert.NoError(t, err) - assert.NotNil(t, at) - assert.NotEmpty(t, token) - - at2, err := CheckAuthToken(db.DefaultContext, at.ID+":"+token) - assert.NoError(t, err) - assert.NotNil(t, at2) - - assert.NoError(t, auth_model.DeleteAuthTokenByID(db.DefaultContext, at.ID)) - }) -} - -func TestRegenerateAuthToken(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - timeutil.MockSet(time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)) - defer timeutil.MockUnset() - - at, token, err := CreateAuthTokenForUserID(db.DefaultContext, 2) - assert.NoError(t, err) - assert.NotNil(t, at) - assert.NotEmpty(t, token) - - timeutil.MockSet(time.Date(2023, 1, 1, 0, 0, 1, 0, time.UTC)) - - at2, token2, err := RegenerateAuthToken(db.DefaultContext, at) - assert.NoError(t, err) - assert.NotNil(t, at2) - assert.NotEmpty(t, token2) - - assert.Equal(t, at.ID, at2.ID) - assert.Equal(t, at.UserID, at2.UserID) - assert.NotEqual(t, token, token2) - assert.NotEqual(t, at.ExpiresUnix, at2.ExpiresUnix) - - assert.NoError(t, auth_model.DeleteAuthTokenByID(db.DefaultContext, at.ID)) -} diff --git a/tests/integration/auth_token_test.go b/tests/integration/auth_token_test.go new file mode 100644 index 0000000000..24c66ee261 --- /dev/null +++ b/tests/integration/auth_token_test.go @@ -0,0 +1,163 @@ +// Copyright 2023 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "encoding/hex" + "net/http" + "net/url" + "strings" + "testing" + + "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +// GetSessionForLTACookie returns a new session with only the LTA cookie being set. +func GetSessionForLTACookie(t *testing.T, ltaCookie *http.Cookie) *TestSession { + t.Helper() + + ch := http.Header{} + ch.Add("Cookie", ltaCookie.String()) + cr := http.Request{Header: ch} + + session := emptyTestSession(t) + baseURL, err := url.Parse(setting.AppURL) + assert.NoError(t, err) + session.jar.SetCookies(baseURL, cr.Cookies()) + + return session +} + +// GetLTACookieValue returns the value of the LTA cookie. +func GetLTACookieValue(t *testing.T, sess *TestSession) string { + t.Helper() + + rememberCookie := sess.GetCookie(setting.CookieRememberName) + assert.NotNil(t, rememberCookie) + + cookieValue, err := url.QueryUnescape(rememberCookie.Value) + assert.NoError(t, err) + + return cookieValue +} + +// TestSessionCookie checks if the session cookie provides authentication. +func TestSessionCookie(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + sess := loginUser(t, "user1") + assert.NotNil(t, sess.GetCookie(setting.SessionConfig.CookieName)) + + req := NewRequest(t, "GET", "/user/settings") + sess.MakeRequest(t, req, http.StatusOK) +} + +// TestLTACookie checks if the LTA cookie that's returned is valid, exists in the database +// and provides authentication of no session cookie is present. +func TestLTACookie(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + sess := emptyTestSession(t) + + req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{ + "_csrf": GetCSRF(t, sess, "/user/login"), + "user_name": user.Name, + "password": userPassword, + "remember": "true", + }) + sess.MakeRequest(t, req, http.StatusSeeOther) + + // Checks if the database entry exist for the user. + ltaCookieValue := GetLTACookieValue(t, sess) + lookupKey, validator, found := strings.Cut(ltaCookieValue, ":") + assert.True(t, found) + rawValidator, err := hex.DecodeString(validator) + assert.NoError(t, err) + unittest.AssertExistsAndLoadBean(t, &auth.AuthorizationToken{LookupKey: lookupKey, HashedValidator: auth.HashValidator(rawValidator), UID: user.ID}) + + // Check if the LTA cookie it provides authentication. + // If LTA cookie provides authentication /user/login shouldn't return status 200. + session := GetSessionForLTACookie(t, sess.GetCookie(setting.CookieRememberName)) + req = NewRequest(t, "GET", "/user/login") + session.MakeRequest(t, req, http.StatusSeeOther) +} + +// TestLTAPasswordChange checks that LTA doesn't provide authentication when a +// password change has happened and that the new LTA does provide authentication. +func TestLTAPasswordChange(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + sess := loginUserWithPasswordRemember(t, user.Name, userPassword, true) + oldRememberCookie := sess.GetCookie(setting.CookieRememberName) + assert.NotNil(t, oldRememberCookie) + + // Make a simple password change. + req := NewRequestWithValues(t, "POST", "/user/settings/account", map[string]string{ + "_csrf": GetCSRF(t, sess, "/user/settings/account"), + "old_password": userPassword, + "password": "password2", + "retype": "password2", + }) + sess.MakeRequest(t, req, http.StatusSeeOther) + rememberCookie := sess.GetCookie(setting.CookieRememberName) + assert.NotNil(t, rememberCookie) + + // Check if the password really changed. + assert.NotEqualValues(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}).Passwd, user.Passwd) + + // /user/settings/account should provide with a new LTA cookie, so check for that. + // If LTA cookie provides authentication /user/login shouldn't return status 200. + session := GetSessionForLTACookie(t, rememberCookie) + req = NewRequest(t, "GET", "/user/login") + session.MakeRequest(t, req, http.StatusSeeOther) + + // Check if the old LTA token is invalidated. + session = GetSessionForLTACookie(t, oldRememberCookie) + req = NewRequest(t, "GET", "/user/login") + session.MakeRequest(t, req, http.StatusOK) +} + +// TestLTAExpiry tests that the LTA expiry works. +func TestLTAExpiry(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + sess := loginUserWithPasswordRemember(t, user.Name, userPassword, true) + + ltaCookieValie := GetLTACookieValue(t, sess) + lookupKey, _, found := strings.Cut(ltaCookieValie, ":") + assert.True(t, found) + + // Ensure it's not expired. + lta := unittest.AssertExistsAndLoadBean(t, &auth.AuthorizationToken{UID: user.ID, LookupKey: lookupKey}) + assert.False(t, lta.IsExpired()) + + // Manually stub LTA's expiry. + _, err := db.GetEngine(db.DefaultContext).ID(lta.ID).Table("forgejo_auth_token").Cols("expiry").Update(&auth.AuthorizationToken{Expiry: timeutil.TimeStampNow()}) + assert.NoError(t, err) + + // Ensure it's expired. + lta = unittest.AssertExistsAndLoadBean(t, &auth.AuthorizationToken{UID: user.ID, LookupKey: lookupKey}) + assert.True(t, lta.IsExpired()) + + // Should return 200 OK, because LTA doesn't provide authorization anymore. + session := GetSessionForLTACookie(t, sess.GetCookie(setting.CookieRememberName)) + req := NewRequest(t, "GET", "/user/login") + session.MakeRequest(t, req, http.StatusOK) + + // Ensure it's deleted. + unittest.AssertNotExistsBean(t, &auth.AuthorizationToken{UID: user.ID, LookupKey: lookupKey}) +} diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 616111787e..b505c9d857 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -17,6 +17,7 @@ import ( "net/url" "os" "path/filepath" + "strconv" "strings" "sync/atomic" "testing" @@ -302,6 +303,12 @@ func loginUser(t testing.TB, userName string) *TestSession { func loginUserWithPassword(t testing.TB, userName, password string) *TestSession { t.Helper() + + return loginUserWithPasswordRemember(t, userName, password, false) +} + +func loginUserWithPasswordRemember(t testing.TB, userName, password string, rememberMe bool) *TestSession { + t.Helper() req := NewRequest(t, "GET", "/user/login") resp := MakeRequest(t, req, http.StatusOK) @@ -310,6 +317,7 @@ func loginUserWithPassword(t testing.TB, userName, password string) *TestSession "_csrf": doc.GetCSRF(), "user_name": userName, "password": password, + "remember": strconv.FormatBool(rememberMe), }) resp = MakeRequest(t, req, http.StatusSeeOther)