From ef9155860082e9d9f33040a2c70e7bc76dd0f902 Mon Sep 17 00:00:00 2001 From: mstar Date: Tue, 1 Apr 2025 17:24:06 +0200 Subject: [PATCH] Comments, fixes --- auth-new/auth.go | 1 + auth-new/errors.go | 25 ++++++++++++--- auth-new/fakeUser.go | 2 ++ auth-new/helpers.go | 2 +- auth-new/totp.go | 36 +++++++++++++++++----- go.mod | 2 +- storage-new/dbgen/user_auth_methods.gen.go | 20 +++++++++--- storage-new/models/UserAuthentication.go | 4 ++- 8 files changed, 73 insertions(+), 19 deletions(-) diff --git a/auth-new/auth.go b/auth-new/auth.go index eb23074..0e7079f 100644 --- a/auth-new/auth.go +++ b/auth-new/auth.go @@ -30,6 +30,7 @@ const ( LoginStartPasskey // Login starts with a passkey ) +// Create a new authenticator func New(webauthnConfig *webauthn.Config) (*Authenticator, error) { webauthn, err := webauthn.New(webauthnConfig) if err != nil { diff --git a/auth-new/errors.go b/auth-new/errors.go index ffad91a..a68efb2 100644 --- a/auth-new/errors.go +++ b/auth-new/errors.go @@ -1,20 +1,31 @@ package auth -import "errors" +import ( + "errors" +) var ( // The provided authentication method is not known to the server ErrUnknownAuthMethod = errors.New("unknown authentication method") // The user hasn't setup the provided authentication method ErrUnsupportedAuthMethod = errors.New("authentication method not supported for this user") - ErrInvalidCombination = errors.New("invalid account and token combination") - ErrProcessTimeout = errors.New("authentication process timed out") + // The given combination of token and account is invalid + // Explicitly doesn't mention which part is valid to improve security + ErrInvalidCombination = errors.New("invalid account and token combination") + // The current authentication attempt has expired and needs to be restarted + ErrProcessTimeout = errors.New("authentication process timed out") // A user may not login, for whatever reason - ErrCantLogin = errors.New("user can't login") + ErrCantLogin = errors.New("user can't login") + // Failed to decrypt the relevant data ErrDecryptionFailure = errors.New("failed to decrypt content") - ErrTotpRecentlyUsed = errors.New("totp token was used too recently") + // The given totp token was recently (90 seconds) used for that username + // For security reasons, this case will be caught and blocked + ErrTotpRecentlyUsed = errors.New("totp token was used too recently") ) +// Helper error type to combine two errors into one +// For when two different errors need to be passed together +// since fmt.Errorf doesn't really allow that as far as I know type CombinedError struct { Err1, Err2 error } @@ -26,3 +37,7 @@ func (c *CombinedError) Is(e error) bool { func (c *CombinedError) Error() string { return c.Err1.Error() + " + " + c.Err2.Error() } + +func (c *CombinedError) Unwrap() []error { + return []error{c.Err1, c.Err2} +} diff --git a/auth-new/fakeUser.go b/auth-new/fakeUser.go index 92d156b..3743279 100644 --- a/auth-new/fakeUser.go +++ b/auth-new/fakeUser.go @@ -10,6 +10,8 @@ import ( "git.mstar.dev/mstar/linstrom/storage-new/models" ) +// A fake user struct for implementing the webauthn.User interface +// on top of the storage system type fakeUser struct { actualUser *models.User } diff --git a/auth-new/helpers.go b/auth-new/helpers.go index 7600626..5bf4153 100644 --- a/auth-new/helpers.go +++ b/auth-new/helpers.go @@ -10,7 +10,7 @@ import ( "git.mstar.dev/mstar/goutils/sliceutils" "golang.org/x/crypto/argon2" - "git.mstar.dev/mstar/linstrom/storage" + "git.mstar.dev/mstar/linstrom/storage-new" "git.mstar.dev/mstar/linstrom/storage-new/dbgen" "git.mstar.dev/mstar/linstrom/storage-new/models" ) diff --git a/auth-new/totp.go b/auth-new/totp.go index a8a7eac..14b90ee 100644 --- a/auth-new/totp.go +++ b/auth-new/totp.go @@ -3,6 +3,7 @@ package auth // Some helpful comments from: https://waters.me/internet/google-authenticator-implementation-note-key-length-token-reuse/ import ( + "strings" "time" "git.mstar.dev/mstar/goutils/other" @@ -21,6 +22,7 @@ const totpTokenNoLongerRecentlyUsed = time.Second * 90 func (a *Authenticator) PerformTotpLogin( username string, + sessionId uint64, totpToken string, ) (LoginNextState, string, error) { // First check if that token has been seen recently for that user @@ -31,17 +33,26 @@ func (a *Authenticator) PerformTotpLogin( delete(a.recentlyUsedTotpTokens, totpToken+"+"+username) } } + // Then ensure user is allowed to log in if ok, err := a.canUsernameLogin(username); !ok { return 0, "", other.Error("auth", "user may not login", err) } - acc, err := dbgen.User.Where(dbgen.User.Username.Eq(username)).First() + // DO NOT fetch the account directly. Go via session id + // otherwise you could in theory log in with only the totp token + // which obviously is a huge risk with it being at most 1'000'000 unique values. + // And even with rate limiting (performed by one of the upper layers) + // this wouldn't take too long + loginSession, err := dbgen.LoginProcessToken.Where(dbgen.LoginProcessToken.ID.Eq(sessionId)). + First() if err != nil { - return LoginNextFailure, "", other.Error("auth", "failed to find account", err) + return 0, "", other.Error("auth", "no login session with this id", err) } + acc := loginSession.User + dbSecrets := sliceutils.Filter(acc.AuthMethods, func(t models.UserAuthMethod) bool { + return t.AuthMethod == models.AuthMethodGAuth + }) encryptedSecrets := sliceutils.Map( - sliceutils.Filter(acc.AuthMethods, func(t models.UserAuthMethod) bool { - return t.AuthMethod == models.AuthMethodGAuth - }), + dbSecrets, func(t models.UserAuthMethod) string { return string(t.Token) }, @@ -59,9 +70,11 @@ func (a *Authenticator) PerformTotpLogin( secrets = append(secrets, string(decrypted)) } found := false - for _, secret := range secrets { + foundIndex := -1 + for i, secret := range secrets { if totp.Validate(totpToken, secret) { found = true + foundIndex = i break } } @@ -72,11 +85,20 @@ func (a *Authenticator) PerformTotpLogin( ErrInvalidCombination, ) } + // If not verified yet, mark as verified + if strings.HasSuffix(dbSecrets[foundIndex].Name, totpUnverifiedSuffix) { + dbgen.UserAuthMethod. + Where(dbgen.UserAuthMethod.ID. + Eq(dbSecrets[foundIndex].ID)). + Update(dbgen.UserAuthMethod.Name, strings.TrimSuffix(dbSecrets[foundIndex].Name, totpUnverifiedSuffix)) + } + // store this token and username combination as recently used a.recentlyUsedTotpTokens[totpToken+"+"+username] = time.Now() + // Generate access token and return since totp would be the end station for 2fa token := models.AccessToken{ - User: *acc, + User: acc, UserId: acc.ID, ExpiresAt: time.Now().Add(time.Hour * 24 * 365), } diff --git a/go.mod b/go.mod index 3796f90..b2d11d1 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( gorm.io/gen v0.3.26 gorm.io/gorm v1.25.12 gorm.io/plugin/dbresolver v1.5.3 + github.com/pquerna/otp v1.4.0 ) require ( @@ -69,7 +70,6 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect - github.com/pquerna/otp v1.4.0 // indirect github.com/prometheus/client_golang v1.14.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.37.0 // indirect diff --git a/storage-new/dbgen/user_auth_methods.gen.go b/storage-new/dbgen/user_auth_methods.gen.go index 01ed1d1..d5ea2ae 100644 --- a/storage-new/dbgen/user_auth_methods.gen.go +++ b/storage-new/dbgen/user_auth_methods.gen.go @@ -26,7 +26,10 @@ func newUserAuthMethod(db *gorm.DB, opts ...gen.DOOption) userAuthMethod { tableName := _userAuthMethod.userAuthMethodDo.TableName() _userAuthMethod.ALL = field.NewAsterisk(tableName) - _userAuthMethod.ID = field.NewUint64(tableName, "id") + _userAuthMethod.ID = field.NewUint(tableName, "id") + _userAuthMethod.CreatedAt = field.NewTime(tableName, "created_at") + _userAuthMethod.UpdatedAt = field.NewTime(tableName, "updated_at") + _userAuthMethod.DeletedAt = field.NewField(tableName, "deleted_at") _userAuthMethod.UserId = field.NewString(tableName, "user_id") _userAuthMethod.AuthMethod = field.NewField(tableName, "auth_method") _userAuthMethod.Token = field.NewBytes(tableName, "token") @@ -181,7 +184,10 @@ type userAuthMethod struct { userAuthMethodDo ALL field.Asterisk - ID field.Uint64 + ID field.Uint + CreatedAt field.Time + UpdatedAt field.Time + DeletedAt field.Field UserId field.String AuthMethod field.Field Token field.Bytes @@ -203,7 +209,10 @@ func (u userAuthMethod) As(alias string) *userAuthMethod { func (u *userAuthMethod) updateTableName(table string) *userAuthMethod { u.ALL = field.NewAsterisk(table) - u.ID = field.NewUint64(table, "id") + u.ID = field.NewUint(table, "id") + u.CreatedAt = field.NewTime(table, "created_at") + u.UpdatedAt = field.NewTime(table, "updated_at") + u.DeletedAt = field.NewField(table, "deleted_at") u.UserId = field.NewString(table, "user_id") u.AuthMethod = field.NewField(table, "auth_method") u.Token = field.NewBytes(table, "token") @@ -224,8 +233,11 @@ func (u *userAuthMethod) GetFieldByName(fieldName string) (field.OrderExpr, bool } func (u *userAuthMethod) fillFieldMap() { - u.fieldMap = make(map[string]field.Expr, 6) + u.fieldMap = make(map[string]field.Expr, 9) u.fieldMap["id"] = u.ID + u.fieldMap["created_at"] = u.CreatedAt + u.fieldMap["updated_at"] = u.UpdatedAt + u.fieldMap["deleted_at"] = u.DeletedAt u.fieldMap["user_id"] = u.UserId u.fieldMap["auth_method"] = u.AuthMethod u.fieldMap["token"] = u.Token diff --git a/storage-new/models/UserAuthentication.go b/storage-new/models/UserAuthentication.go index f0ca758..a6e7803 100644 --- a/storage-new/models/UserAuthentication.go +++ b/storage-new/models/UserAuthentication.go @@ -1,5 +1,7 @@ package models +import "gorm.io/gorm" + // One authentication method linked to one account. // Contains the method and whatever the token may be // For a password, this would be a hash of that password, @@ -8,7 +10,7 @@ package models // // Password hashes may only exist at most once per user, the rest 0-m type UserAuthMethod struct { - ID uint64 `gorm:"primarykey"` + gorm.Model User User UserId string AuthMethod AuthenticationMethodType `gorm:"type:auth_method_type"`