From 2d75832cebe4bdd105701de29eb11c55cfe197d0 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Wed, 16 Sep 2026 08:39:48 +0000 Subject: [PATCH] feat(mfa): second-factor sign-in with TOTP and recovery codes --- server/internal/api/handlers.go | 2 + server/internal/auth/local.go | 46 ++++++++-- server/internal/auth/mfa_login.go | 121 +++++++++++++++++++++++++ server/internal/auth/mfa_login_test.go | 27 ++++++ server/internal/auth/session.go | 20 ++++ server/internal/services/mfa.go | 45 +++++++++ 6 files changed, 254 insertions(+), 7 deletions(-) create mode 100644 server/internal/auth/mfa_login.go create mode 100644 server/internal/auth/mfa_login_test.go diff --git a/server/internal/api/handlers.go b/server/internal/api/handlers.go index 408f70c..c5ffcb4 100644 --- a/server/internal/api/handlers.go +++ b/server/internal/api/handlers.go @@ -43,6 +43,8 @@ func RegisterRoutes(r *gin.Engine) { r.GET("/auth/bootstrap-status", auth.HandleBootstrapStatus) r.POST("/auth/bootstrap", auth.HandleBootstrap) r.POST("/auth/login", auth.HandleLocalLogin) + r.POST("/auth/mfa/totp", auth.HandleMFATOTP) + r.POST("/auth/mfa/recovery", auth.HandleMFARecovery) r.POST("/auth/logout", auth.HandleLogout) r.GET("/auth/me", auth.HandleMe) r.GET("/auth/oidc/:providerId/start", auth.HandleSSOStart) diff --git a/server/internal/auth/local.go b/server/internal/auth/local.go index b20f795..e784f13 100644 --- a/server/internal/auth/local.go +++ b/server/internal/auth/local.go @@ -102,16 +102,48 @@ func HandleLocalLogin(c *gin.Context) { c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid credentials"}) return } - sessionID, err := SaveSession(c.Request.Context(), &Session{ - UserID: u.UserID, InstanceID: u.InstanceID, Role: u.Role, Email: u.Email, - }) + requireMFA := services.RequireMFAForInstance(instanceID) + hasMFA, err := services.HasMFA(u.InstanceID, u.UserID) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "could not read MFA state"}) return } - _ = services.TouchLastLogin(u.UserID) - SetSessionCookie(c, sessionID) - c.JSON(http.StatusOK, gin.H{"ok": true}) + + switch loginDecision(hasMFA, requireMFA) { + case "session": + if err := mintSession(c, u, []string{"pwd"}); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + return + } + c.JSON(http.StatusOK, gin.H{"ok": true}) + + case "verify": + methods, err := services.MFAMethods(u.InstanceID, u.UserID) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "could not read MFA state"}) + return + } + id, err := CreateTicket(c.Request.Context(), &Ticket{ + UserID: u.UserID, InstanceID: u.InstanceID, Email: u.Email, Methods: methods, + }) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + return + } + SetPendingCookie(c, id) + c.JSON(http.StatusOK, gin.H{"mfa_required": true, "methods": methods}) + + case "enrol": + id, err := CreateTicket(c.Request.Context(), &Ticket{ + UserID: u.UserID, InstanceID: u.InstanceID, Email: u.Email, EnrolOnly: true, + }) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + return + } + SetPendingCookie(c, id) + c.JSON(http.StatusOK, gin.H{"enrol_required": true}) + } } // HandleListPublicProviders is unauthenticated: it is what the login page reads diff --git a/server/internal/auth/mfa_login.go b/server/internal/auth/mfa_login.go new file mode 100644 index 0000000..88bc5e5 --- /dev/null +++ b/server/internal/auth/mfa_login.go @@ -0,0 +1,121 @@ +package auth + +import ( + "errors" + "net/http" + "time" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/services" + "github.com/gin-gonic/gin" +) + +// loginDecision is the branch a verified password takes. +func loginDecision(hasMFA, requireMFA bool) string { + switch { + case hasMFA: + return "verify" + case requireMFA: + return "enrol" + default: + return "session" + } +} + +// mintSession is the single place a session is created from a user, so every +// path records amr and step-up freshness the same way. +func mintSession(c *gin.Context, u *models.User, amr []string) error { + now := time.Now() + sessionID, err := SaveSession(c.Request.Context(), &Session{ + UserID: u.UserID, InstanceID: u.InstanceID, Role: u.Role, Email: u.Email, + AMR: amr, StepUpAt: &now, + }) + if err != nil { + return err + } + _ = services.TouchLastLogin(u.UserID) + ClearPendingCookie(c) + SetSessionCookie(c, sessionID) + return nil +} + +// HandleMFATOTP completes a sign-in with a TOTP code. +// +// @Summary Complete sign-in with a TOTP code +// @Tags auth +// @Accept json +// @Produce json +// @Param body body object{code=string} true "Six-digit code" +// @Success 200 {object} object{ok=bool} +// @Failure 401 {object} object{error=string,code=string} +// @Router /auth/mfa/totp [post] +func HandleMFATOTP(c *gin.Context) { + handleMFAVerify(c, services.FactorTOTP) +} + +// HandleMFARecovery completes a sign-in with a recovery code. +// +// @Summary Complete sign-in with a recovery code +// @Tags auth +// @Accept json +// @Produce json +// @Param body body object{code=string} true "Recovery code" +// @Success 200 {object} object{ok=bool} +// @Failure 401 {object} object{error=string,code=string} +// @Router /auth/mfa/recovery [post] +func HandleMFARecovery(c *gin.Context) { + handleMFAVerify(c, services.FactorRecovery) +} + +func handleMFAVerify(c *gin.Context, factor string) { + t, ticketID, ok := ticketFromRequest(c, scopeVerify) + if !ok { + return + } + var body struct { + Code string `json:"code"` + } + if err := c.ShouldBindJSON(&body); err != nil || body.Code == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "code required"}) + return + } + + var err error + switch factor { + case services.FactorTOTP: + err = services.VerifyTOTPCode(t.InstanceID, t.UserID, body.Code) + case services.FactorRecovery: + err = services.UseRecoveryCode(t.InstanceID, t.UserID, body.Code) + } + if err != nil { + left, ferr := FailTicket(c.Request.Context(), ticketID) + services.LogEvent(t.InstanceID, "mfa.failed", t.Email, "", "", "factor="+factor) + if ferr != nil || left == 0 { + abortTicketExpired(c) + return + } + code := "invalid_code" + if errors.Is(err, services.ErrCodeReplayed) { + code = "invalid_code" + } + c.JSON(http.StatusUnauthorized, gin.H{ + "error": "that code is not valid", "code": code, "attempts_left": left, + }) + return + } + + u, err := services.GetUserInInstance(t.InstanceID, t.UserID) + if err != nil { + c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid credentials"}) + return + } + _ = DeleteTicket(c.Request.Context(), ticketID) + if err := mintSession(c, u, []string{"pwd", factor}); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "session save failed"}) + return + } + if factor == services.FactorRecovery { + services.LogEvent(t.InstanceID, "mfa.recovery_used", u.Email, "", "", "") + } + c.JSON(http.StatusOK, gin.H{"ok": true}) +} diff --git a/server/internal/auth/mfa_login_test.go b/server/internal/auth/mfa_login_test.go new file mode 100644 index 0000000..b544689 --- /dev/null +++ b/server/internal/auth/mfa_login_test.go @@ -0,0 +1,27 @@ +package auth + +import "testing" + +// The whole point of the feature is in this table: a user with MFA is never +// handed a session by a password alone, and require_mfa turns "no factor" into +// forced enrolment rather than a free pass. +func TestLoginDecision(t *testing.T) { + cases := []struct { + name string + hasMFA bool + requireMFA bool + want string + }{ + {"no mfa, not required", false, false, "session"}, + {"no mfa, required", false, true, "enrol"}, + {"has mfa, not required", true, false, "verify"}, + {"has mfa, required", true, true, "verify"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := loginDecision(tc.hasMFA, tc.requireMFA); got != tc.want { + t.Fatalf("loginDecision(%v,%v) = %q, want %q", tc.hasMFA, tc.requireMFA, got, tc.want) + } + }) + } +} diff --git a/server/internal/auth/session.go b/server/internal/auth/session.go index 6464be2..009a270 100644 --- a/server/internal/auth/session.go +++ b/server/internal/auth/session.go @@ -31,6 +31,14 @@ type Session struct { TokenName string `json:"-"` Scopes []string `json:"-"` TokenScope map[string]string `json:"-"` + + // AMR records how this session authenticated: pwd, otp, webauthn, + // recovery, oidc. Step-up reads it to exempt OIDC sessions, whose IdP owns + // authentication policy. + AMR []string `json:"amr,omitempty"` + + // StepUpAt is the last successful re-authentication. Sign-in counts as one. + StepUpAt *time.Time `json:"step_up_at,omitempty"` } var rdb *redis.Client @@ -103,3 +111,15 @@ func GetSession(ctx context.Context, id string) (*Session, error) { func DeleteSession(ctx context.Context, id string) error { return rdb.Del(ctx, sessionPrefix+id).Err() } + +// TouchStepUp records a fresh re-authentication without disturbing the +// session's remaining lifetime. +func TouchStepUp(ctx context.Context, id string, sess *Session) error { + now := time.Now() + sess.StepUpAt = &now + data, err := json.Marshal(sess) + if err != nil { + return err + } + return rdb.Set(ctx, sessionPrefix+id, data, redis.KeepTTL).Err() +} diff --git a/server/internal/services/mfa.go b/server/internal/services/mfa.go index ee778cd..add9ed6 100644 --- a/server/internal/services/mfa.go +++ b/server/internal/services/mfa.go @@ -290,6 +290,51 @@ func ClearMFA(instanceID, userID string) error { return err } +// UseRecoveryCode consumes one unused code, marking it used by index so a +// concurrent second attempt with the same code finds it spent. +func UseRecoveryCode(instanceID, userID, input string) error { + m, err := GetUserMFA(instanceID, userID) + if err != nil { + return err + } + if m == nil || len(m.RecoveryCodes) == 0 { + return ErrNoMFA + } + idx, ok := ConsumeRecoveryCode(m.RecoveryCodes, input, time.Now()) + if !ok { + return ErrBadCode + } + ctx, cancel := mfaCtx() + defer cancel() + now := time.Now() + res, err := db.Col("user_mfa").UpdateOne(ctx, + bson.M{ + "instance_id": instanceID, "user_id": userID, + "recovery_codes." + strconv.Itoa(idx) + ".used_at": bson.M{"$exists": false}, + }, + bson.M{"$set": bson.M{ + "recovery_codes." + strconv.Itoa(idx) + ".used_at": now, + "updated_at": now, + }}) + if err != nil { + return err + } + if res.MatchedCount == 0 { + return ErrBadCode + } + return nil +} + +// RequireMFAForInstance reads the policy, defaulting to off on any error: a +// database blip must not lock an entire instance out of its own control plane. +func RequireMFAForInstance(instanceID string) bool { + s, err := GetSettings(instanceID) + if err != nil { + return false + } + return models.RequireMFA(s) +} + // burnTOTPCode makes a step single-use for 90 seconds - longer than the +-1 // step window it could still validate in. Keyed on the time step the code // was accepted against, never on the code itself: a raw code sitting in a