Comments, fixes

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 {

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}
+}

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
 }

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"
 )

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),
 	}