Skip to content

Commit 1770117

Browse files
Gustedearl-warren
authored andcommitted
fix: extend forgejo_auth_token table
- Add a `purpose` column, this allows the `forgejo_auth_token` table to be used by other parts of Forgejo, while still enjoying the no-compromise architecture. - Remove the 'roll your own crypto' time limited code functions and migrate them to the `forgejo_auth_token` table. This migration ensures generated codes can only be used for their purpose and ensure they are invalidated after their usage by deleting it from the database, this also should help making auditing of the security code easier, as we're no longer trying to stuff a lot of data into a HMAC construction. -Helper functions are rewritten to ensure a safe-by-design approach to these tokens. - Add the `forgejo_auth_token` to dbconsistency doctor and add it to the `deleteUser` function. - TODO: Add cron job to delete expired authorization tokens. - Unit and integration tests added. (cherry picked from commit 1ce33aa) v9: Removed migration - XORM can handle this case automatically without migration. Add `DEFAULT 'long_term_authorization'`.
1 parent 1379914 commit 1770117

File tree

16 files changed

+425
-254
lines changed

16 files changed

+425
-254
lines changed

models/auth/auth_token.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,31 @@ import (
1515
"code.gitea.io/gitea/modules/util"
1616
)
1717

18+
type AuthorizationPurpose string
19+
20+
var (
21+
// Used to store long term authorization tokens.
22+
LongTermAuthorization AuthorizationPurpose = "long_term_authorization"
23+
24+
// Used to activate a user account.
25+
UserActivation AuthorizationPurpose = "user_activation"
26+
27+
// Used to reset the password.
28+
PasswordReset AuthorizationPurpose = "password_reset"
29+
)
30+
31+
// Used to activate the specified email address for a user.
32+
func EmailActivation(email string) AuthorizationPurpose {
33+
return AuthorizationPurpose("email_activation:" + email)
34+
}
35+
1836
// AuthorizationToken represents a authorization token to a user.
1937
type AuthorizationToken struct {
2038
ID int64 `xorm:"pk autoincr"`
2139
UID int64 `xorm:"INDEX"`
2240
LookupKey string `xorm:"INDEX UNIQUE"`
2341
HashedValidator string
42+
Purpose AuthorizationPurpose `xorm:"NOT NULL DEFAULT 'long_term_authorization'"`
2443
Expiry timeutil.TimeStamp
2544
}
2645

@@ -41,7 +60,7 @@ func (authToken *AuthorizationToken) IsExpired() bool {
4160
// GenerateAuthToken generates a new authentication token for the given user.
4261
// It returns the lookup key and validator values that should be passed to the
4362
// user via a long-term cookie.
44-
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp) (lookupKey, validator string, err error) {
63+
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp, purpose AuthorizationPurpose) (lookupKey, validator string, err error) {
4564
// Request 64 random bytes. The first 32 bytes will be used for the lookupKey
4665
// and the other 32 bytes will be used for the validator.
4766
rBytes, err := util.CryptoRandomBytes(64)
@@ -56,14 +75,15 @@ func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeSt
5675
Expiry: expiry,
5776
LookupKey: lookupKey,
5877
HashedValidator: HashValidator(rBytes[32:]),
78+
Purpose: purpose,
5979
})
6080
return lookupKey, validator, err
6181
}
6282

6383
// FindAuthToken will find a authorization token via the lookup key.
64-
func FindAuthToken(ctx context.Context, lookupKey string) (*AuthorizationToken, error) {
84+
func FindAuthToken(ctx context.Context, lookupKey string, purpose AuthorizationPurpose) (*AuthorizationToken, error) {
6585
var authToken AuthorizationToken
66-
has, err := db.GetEngine(ctx).Where("lookup_key = ?", lookupKey).Get(&authToken)
86+
has, err := db.GetEngine(ctx).Where("lookup_key = ? AND purpose = ?", lookupKey, purpose).Get(&authToken)
6787
if err != nil {
6888
return nil, err
6989
} else if !has {

models/user/email_address.go

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ import (
1010
"net/mail"
1111
"regexp"
1212
"strings"
13-
"time"
1413

1514
"code.gitea.io/gitea/models/db"
16-
"code.gitea.io/gitea/modules/base"
1715
"code.gitea.io/gitea/modules/log"
1816
"code.gitea.io/gitea/modules/optional"
1917
"code.gitea.io/gitea/modules/setting"
@@ -307,23 +305,6 @@ func updateActivation(ctx context.Context, email *EmailAddress, activate bool) e
307305
return UpdateUserCols(ctx, user, "rands")
308306
}
309307

310-
// VerifyActiveEmailCode verifies active email code when active account
311-
func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddress {
312-
if user := GetVerifyUser(ctx, code); user != nil {
313-
// time limit code
314-
prefix := code[:base.TimeLimitCodeLength]
315-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, email, user.LowerName, user.Passwd, user.Rands)
316-
317-
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
318-
emailAddress := &EmailAddress{UID: user.ID, Email: email}
319-
if has, _ := db.GetEngine(ctx).Get(emailAddress); has {
320-
return emailAddress
321-
}
322-
}
323-
}
324-
return nil
325-
}
326-
327308
// SearchEmailOrderBy is used to sort the results from SearchEmails()
328309
type SearchEmailOrderBy string
329310

models/user/user.go

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ package user
77

88
import (
99
"context"
10+
"crypto/subtle"
1011
"encoding/hex"
12+
"errors"
1113
"fmt"
1214
"net/mail"
1315
"net/url"
@@ -318,15 +320,14 @@ func (u *User) OrganisationLink() string {
318320
return setting.AppSubURL + "/org/" + url.PathEscape(u.Name)
319321
}
320322

321-
// GenerateEmailActivateCode generates an activate code based on user information and given e-mail.
322-
func (u *User) GenerateEmailActivateCode(email string) string {
323-
code := base.CreateTimeLimitCode(
324-
fmt.Sprintf("%d%s%s%s%s", u.ID, email, u.LowerName, u.Passwd, u.Rands),
325-
setting.Service.ActiveCodeLives, time.Now(), nil)
326-
327-
// Add tail hex username
328-
code += hex.EncodeToString([]byte(u.LowerName))
329-
return code
323+
// GenerateEmailAuthorizationCode generates an activation code based for the user for the specified purpose.
324+
// The standard expiry is ActiveCodeLives minutes.
325+
func (u *User) GenerateEmailAuthorizationCode(ctx context.Context, purpose auth.AuthorizationPurpose) (string, error) {
326+
lookup, validator, err := auth.GenerateAuthToken(ctx, u.ID, timeutil.TimeStampNow().Add(int64(setting.Service.ActiveCodeLives)*60), purpose)
327+
if err != nil {
328+
return "", err
329+
}
330+
return lookup + ":" + validator, nil
330331
}
331332

332333
// GetUserFollowers returns range of user's followers.
@@ -836,35 +837,50 @@ func countUsers(ctx context.Context, opts *CountUserFilter) int64 {
836837
return count
837838
}
838839

839-
// GetVerifyUser get user by verify code
840-
func GetVerifyUser(ctx context.Context, code string) (user *User) {
841-
if len(code) <= base.TimeLimitCodeLength {
842-
return nil
840+
// VerifyUserActiveCode verifies that the code is valid for the given purpose for this user.
841+
// If delete is specified, the token will be deleted.
842+
func VerifyUserAuthorizationToken(ctx context.Context, code string, purpose auth.AuthorizationPurpose, delete bool) (*User, error) {
843+
lookupKey, validator, found := strings.Cut(code, ":")
844+
if !found {
845+
return nil, nil
843846
}
844847

845-
// use tail hex username query user
846-
hexStr := code[base.TimeLimitCodeLength:]
847-
if b, err := hex.DecodeString(hexStr); err == nil {
848-
if user, err = GetUserByName(ctx, string(b)); user != nil {
849-
return user
848+
authToken, err := auth.FindAuthToken(ctx, lookupKey, purpose)
849+
if err != nil {
850+
if errors.Is(err, util.ErrNotExist) {
851+
return nil, nil
850852
}
851-
log.Error("user.getVerifyUser: %v", err)
853+
return nil, err
852854
}
853855

854-
return nil
855-
}
856+
if authToken.IsExpired() {
857+
return nil, auth.DeleteAuthToken(ctx, authToken)
858+
}
859+
860+
rawValidator, err := hex.DecodeString(validator)
861+
if err != nil {
862+
return nil, err
863+
}
864+
865+
if subtle.ConstantTimeCompare([]byte(authToken.HashedValidator), []byte(auth.HashValidator(rawValidator))) == 0 {
866+
return nil, errors.New("validator doesn't match")
867+
}
856868

857-
// VerifyUserActiveCode verifies active code when active account
858-
func VerifyUserActiveCode(ctx context.Context, code string) (user *User) {
859-
if user = GetVerifyUser(ctx, code); user != nil {
860-
// time limit code
861-
prefix := code[:base.TimeLimitCodeLength]
862-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, user.Email, user.LowerName, user.Passwd, user.Rands)
863-
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
864-
return user
869+
u, err := GetUserByID(ctx, authToken.UID)
870+
if err != nil {
871+
if IsErrUserNotExist(err) {
872+
return nil, nil
865873
}
874+
return nil, err
866875
}
867-
return nil
876+
877+
if delete {
878+
if err := auth.DeleteAuthToken(ctx, authToken); err != nil {
879+
return nil, err
880+
}
881+
}
882+
883+
return u, nil
868884
}
869885

870886
// ValidateUser check if user is valid to insert / update into database

models/user/user_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package user_test
77
import (
88
"context"
99
"crypto/rand"
10+
"encoding/hex"
1011
"fmt"
1112
"strings"
1213
"testing"
@@ -21,7 +22,9 @@ import (
2122
"code.gitea.io/gitea/modules/optional"
2223
"code.gitea.io/gitea/modules/setting"
2324
"code.gitea.io/gitea/modules/structs"
25+
"code.gitea.io/gitea/modules/test"
2426
"code.gitea.io/gitea/modules/timeutil"
27+
"code.gitea.io/gitea/modules/util"
2528
"code.gitea.io/gitea/tests"
2629

2730
"github.com/stretchr/testify/assert"
@@ -699,3 +702,66 @@ func TestDisabledUserFeatures(t *testing.T) {
699702
assert.True(t, user_model.IsFeatureDisabledWithLoginType(user, f))
700703
}
701704
}
705+
706+
func TestGenerateEmailAuthorizationCode(t *testing.T) {
707+
defer test.MockVariableValue(&setting.Service.ActiveCodeLives, 2)()
708+
require.NoError(t, unittest.PrepareTestDatabase())
709+
710+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
711+
712+
code, err := user.GenerateEmailAuthorizationCode(db.DefaultContext, auth.UserActivation)
713+
require.NoError(t, err)
714+
715+
lookupKey, validator, ok := strings.Cut(code, ":")
716+
assert.True(t, ok)
717+
718+
rawValidator, err := hex.DecodeString(validator)
719+
require.NoError(t, err)
720+
721+
authToken, err := auth.FindAuthToken(db.DefaultContext, lookupKey, auth.UserActivation)
722+
require.NoError(t, err)
723+
assert.False(t, authToken.IsExpired())
724+
assert.EqualValues(t, authToken.HashedValidator, auth.HashValidator(rawValidator))
725+
726+
authToken.Expiry = authToken.Expiry.Add(-int64(setting.Service.ActiveCodeLives) * 60)
727+
assert.True(t, authToken.IsExpired())
728+
}
729+
730+
func TestVerifyUserAuthorizationToken(t *testing.T) {
731+
defer test.MockVariableValue(&setting.Service.ActiveCodeLives, 2)()
732+
require.NoError(t, unittest.PrepareTestDatabase())
733+
734+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
735+
736+
code, err := user.GenerateEmailAuthorizationCode(db.DefaultContext, auth.UserActivation)
737+
require.NoError(t, err)
738+
739+
lookupKey, _, ok := strings.Cut(code, ":")
740+
assert.True(t, ok)
741+
742+
t.Run("Wrong purpose", func(t *testing.T) {
743+
u, err := user_model.VerifyUserAuthorizationToken(db.DefaultContext, code, auth.PasswordReset, false)
744+
require.NoError(t, err)
745+
assert.Nil(t, u)
746+
})
747+
748+
t.Run("No delete", func(t *testing.T) {
749+
u, err := user_model.VerifyUserAuthorizationToken(db.DefaultContext, code, auth.UserActivation, false)
750+
require.NoError(t, err)
751+
assert.EqualValues(t, user.ID, u.ID)
752+
753+
authToken, err := auth.FindAuthToken(db.DefaultContext, lookupKey, auth.UserActivation)
754+
require.NoError(t, err)
755+
assert.NotNil(t, authToken)
756+
})
757+
758+
t.Run("Delete", func(t *testing.T) {
759+
u, err := user_model.VerifyUserAuthorizationToken(db.DefaultContext, code, auth.UserActivation, true)
760+
require.NoError(t, err)
761+
assert.EqualValues(t, user.ID, u.ID)
762+
763+
authToken, err := auth.FindAuthToken(db.DefaultContext, lookupKey, auth.UserActivation)
764+
require.ErrorIs(t, err, util.ErrNotExist)
765+
assert.Nil(t, authToken)
766+
})
767+
}

modules/base/tool.go

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,20 @@
44
package base
55

66
import (
7-
"crypto/hmac"
8-
"crypto/sha1"
97
"crypto/sha256"
10-
"crypto/subtle"
118
"encoding/base64"
129
"encoding/hex"
1310
"errors"
1411
"fmt"
15-
"hash"
1612
"os"
1713
"path/filepath"
1814
"runtime"
1915
"strconv"
2016
"strings"
21-
"time"
2217
"unicode/utf8"
2318

2419
"code.gitea.io/gitea/modules/git"
2520
"code.gitea.io/gitea/modules/log"
26-
"code.gitea.io/gitea/modules/setting"
2721

2822
"github.com/dustin/go-humanize"
2923
)
@@ -54,66 +48,6 @@ func BasicAuthDecode(encoded string) (string, string, error) {
5448
return "", "", errors.New("invalid basic authentication")
5549
}
5650

57-
// VerifyTimeLimitCode verify time limit code
58-
func VerifyTimeLimitCode(now time.Time, data string, minutes int, code string) bool {
59-
if len(code) <= 18 {
60-
return false
61-
}
62-
63-
startTimeStr := code[:12]
64-
aliveTimeStr := code[12:18]
65-
aliveTime, _ := strconv.Atoi(aliveTimeStr) // no need to check err, if anything wrong, the following code check will fail soon
66-
67-
// check code
68-
retCode := CreateTimeLimitCode(data, aliveTime, startTimeStr, nil)
69-
if subtle.ConstantTimeCompare([]byte(retCode), []byte(code)) != 1 {
70-
retCode = CreateTimeLimitCode(data, aliveTime, startTimeStr, sha1.New()) // TODO: this is only for the support of legacy codes, remove this in/after 1.23
71-
if subtle.ConstantTimeCompare([]byte(retCode), []byte(code)) != 1 {
72-
return false
73-
}
74-
}
75-
76-
// check time is expired or not: startTime <= now && now < startTime + minutes
77-
startTime, _ := time.ParseInLocation("200601021504", startTimeStr, time.Local)
78-
return (startTime.Before(now) || startTime.Equal(now)) && now.Before(startTime.Add(time.Minute*time.Duration(minutes)))
79-
}
80-
81-
// TimeLimitCodeLength default value for time limit code
82-
const TimeLimitCodeLength = 12 + 6 + 40
83-
84-
// CreateTimeLimitCode create a time-limited code.
85-
// Format: 12 length date time string + 6 minutes string (not used) + 40 hash string, some other code depends on this fixed length
86-
// If h is nil, then use the default hmac hash.
87-
func CreateTimeLimitCode[T time.Time | string](data string, minutes int, startTimeGeneric T, h hash.Hash) string {
88-
const format = "200601021504"
89-
90-
var start time.Time
91-
var startTimeAny any = startTimeGeneric
92-
if t, ok := startTimeAny.(time.Time); ok {
93-
start = t
94-
} else {
95-
var err error
96-
start, err = time.ParseInLocation(format, startTimeAny.(string), time.Local)
97-
if err != nil {
98-
return "" // return an invalid code because the "parse" failed
99-
}
100-
}
101-
startStr := start.Format(format)
102-
end := start.Add(time.Minute * time.Duration(minutes))
103-
104-
if h == nil {
105-
h = hmac.New(sha1.New, setting.GetGeneralTokenSigningSecret())
106-
}
107-
_, _ = fmt.Fprintf(h, "%s%s%s%s%d", data, hex.EncodeToString(setting.GetGeneralTokenSigningSecret()), startStr, end.Format(format), minutes)
108-
encoded := hex.EncodeToString(h.Sum(nil))
109-
110-
code := fmt.Sprintf("%s%06d%s", startStr, minutes, encoded)
111-
if len(code) != TimeLimitCodeLength {
112-
panic("there is a hard requirement for the length of time-limited code") // it shouldn't happen
113-
}
114-
return code
115-
}
116-
11751
// FileSize calculates the file size and generate user-friendly string.
11852
func FileSize(s int64) string {
11953
return humanize.IBytes(uint64(s))

0 commit comments

Comments
 (0)