fix: auth/2FA — password change requires 2FA gate, admin self-deletion blocked, twofa JSON responses, per-IP email-verify budget, OptionalAuth log sanitised

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

backend/handlers/auth/local.go

@@ -155,8 +155,60 @@ type emailVerifyAttempt struct {
 var (
 	emailVerifyMu       sync.Mutex
 	emailVerifyAttempts = make(map[string]emailVerifyAttempt)
+	ipVerifyMu          sync.Mutex
+	ipVerifyAttempts    = make(map[string]emailVerifyAttempt)
 )
 
+// ipAttemptsExhausted reports whether the IP's attempt budget is already
+// spent, rejecting the request before any DB work. Uses the same window as
+// email-verify tracking with a higher cap (50 per IP per window) as a
+// secondary per-IP fallback budget.
+func ipAttemptsExhausted(ip string) bool {
+	ipVerifyMu.Lock()
+	defer ipVerifyMu.Unlock()
+	evictStaleIPVerifyAttemptsLocked()
+	a, ok := ipVerifyAttempts[ip]
+	return ok && a.count >= 50
+}
+
+// ipAttemptFailed registers one failed attempt for the IP and reports whether
+// the budget for that IP is now exhausted.
+func ipAttemptFailed(ip string) bool {
+	ipVerifyMu.Lock()
+	defer ipVerifyMu.Unlock()
+	evictStaleIPVerifyAttemptsLocked()
+	now := clock.Now()
+	a := ipVerifyAttempts[ip]
+	if now.Sub(a.lastAt) > emailVerifyAttemptWindow {
+		a.count = 0
+	}
+	a.count++
+	a.lastAt = now
+	ipVerifyAttempts[ip] = a
+	return a.count >= 50
+}
+
+// ipAttemptsClear drops the budget for an IP after a successful verify.
+func ipAttemptsClear(ip string) {
+	ipVerifyMu.Lock()
+	delete(ipVerifyAttempts, ip)
+	ipVerifyMu.Unlock()
+}
+
+// evictStaleIPVerifyAttemptsLocked bounds the IP attempts map. Caller must
+// hold ipVerifyMu.
+func evictStaleIPVerifyAttemptsLocked() {
+	if len(ipVerifyAttempts) < emailVerifyMaxTrackedCodes {
+		return
+	}
+	now := clock.Now()
+	for k, a := range ipVerifyAttempts {
+		if now.Sub(a.lastAt) > emailVerifyAttemptWindow {
+			delete(ipVerifyAttempts, k)
+		}
+	}
+}
+
 // emailVerifyAttemptsExhausted reports whether the key's (a user id, or a
 // submitted code with no resolvable user) attempt budget is already spent,
 // rejecting the request before any DB work.
@@ -1020,6 +1072,16 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Per-IP fallback budget: when no user resolves, the attempt budget is
+	// keyed on the submitted CODE value — rotating codes gives unlimited
+	// guesses. The per-IP check prevents a single source from exhausting the
+	// endpoint regardless of code rotation.
+	ip := mw.ClientIP(r)
+	if ipAttemptsExhausted(ip) {
+		http.Error(w, "too many attempts", http.StatusTooManyRequests)
+		return
+	}
+
 	codeDigest := twofa.Hash(code)
 
 	var userID string
@@ -1036,6 +1098,7 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) {
 		if errors.Is(err, pgx.ErrNoRows) {
 			// No row with this digest at all — a guess. No user can be
 			// resolved, so the attempt budget stays keyed per submitted code.
+			ipAttemptFailed(ip)
 			if emailVerifyAttemptFailed(code) {
 				http.Error(w, "too many attempts. request a new code.", http.StatusTooManyRequests)
 				return
@@ -1063,6 +1126,7 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	// Code exists but expired — count it against the user's budget too.
 	if !expiresAt.After(clock.Now()) {
+		ipAttemptFailed(ip)
 		if emailVerifyAttemptFailed(userID) {
 			http.Error(w, "too many attempts. request a new code.", http.StatusTooManyRequests)
 			return
@@ -1132,6 +1196,7 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) {
 	// key the submitted code used on earlier guesses).
 	emailVerifyAttemptsClear(userID)
 	emailVerifyAttemptsClear(code)
+	ipAttemptsClear(ip)
 
 	if err := json.NewEncoder(w).Encode(VerificationResponse{Success: true, Message: message}); err != nil {
 		log.Printf("Failed to encode JSON response: %v", err)

backend/handlers/user/account.go

@@ -338,6 +338,26 @@ func DeleteAccountHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// FIX: admin accounts cannot be self-deleted
+	if accountRole == "admin" {
+		http.Error(w, "admin accounts cannot be self-deleted", http.StatusForbidden)
+		return
+	}
+
+	// FIX: check for active bookings before guest deletion
+	if accountRole == "guest" {
+		var bookingCount int
+		if err := db.Conn.QueryRow(r.Context(), `SELECT COUNT(*) FROM bookings WHERE user_id = $1 AND status IN ('pending', 'confirmed', 'in_progress', 'pending_release')`, userID).Scan(&bookingCount); err != nil {
+			log.Printf("Failed to check bookings for guest %s: %v", userID, err)
+			http.Error(w, "internal error", http.StatusInternalServerError)
+			return
+		}
+		if bookingCount > 0 {
+			http.Error(w, "cannot delete guest account with active bookings", http.StatusBadRequest)
+			return
+		}
+	}
+
 	ctx := r.Context()
 
 	// Finding 3: deleting an account is irreversible, so the session token alone

backend/handlers/user/profile.go

@@ -30,6 +30,7 @@ import (
 	"crussell/internal/dav"
 	"crussell/internal/images"
 	"crussell/internal/s3"
+	"crussell/internal/twofa"
 	"crussell/internal/validators"
 	"crussell/mw"
 
@@ -659,8 +660,9 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 type ChangePasswordRequest struct {
-	CurrentPassword string `json:"current_password" validate:"required,max=72"`
-	NewPassword     string `json:"new_password"     validate:"required,min=6,max=72"`
+	CurrentPassword  string `json:"current_password" validate:"required,max=72"`
+	NewPassword      string `json:"new_password"     validate:"required,min=6,max=72"`
+	VerificationCode string `json:"verification_code"`
 }
 
 func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) {
@@ -702,7 +704,8 @@ func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var passwordHash sql.NullString
-	err := db.Conn.QueryRow(r.Context(), `SELECT password_hash FROM users WHERE id = $1`, userID).Scan(&passwordHash)
+	var twoFactorEnabled bool
+	err := db.Conn.QueryRow(r.Context(), `SELECT password_hash, two_factor_enabled FROM users WHERE id = $1`, userID).Scan(&passwordHash, &twoFactorEnabled)
 	if err != nil {
 		if errors.Is(err, pgx.ErrNoRows) {
 			http.Error(w, "user not found", http.StatusNotFound)
@@ -765,6 +768,29 @@ func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) {
 	// login-locked-out user can self-recover by changing their password.
 	resetCurrentPasswordFailures(r.Context(), userID)
 
+	if twoFARequired() && twoFactorEnabled {
+		if req.VerificationCode == "" {
+			http.Error(w, "a two-factor verification code is required to change the password", http.StatusBadRequest)
+			return
+		}
+		switch err := twofa.VerifyForUser(r.Context(), userID, req.VerificationCode, twofa.ConsumeOnVerify); {
+		case err == nil:
+		case errors.Is(err, twofa.ErrIncorrect):
+			http.Error(w, "incorrect verification code", http.StatusBadRequest)
+			return
+		case errors.Is(err, twofa.ErrLockedOut):
+			http.Error(w, "Too many attempts. Request a new code.", http.StatusTooManyRequests)
+			return
+		case errors.Is(err, twofa.ErrMissingOrExpired):
+			http.Error(w, "verification code is missing or has expired", http.StatusBadRequest)
+			return
+		default:
+			log.Printf("Failed to check 2FA pending code for user %s: %v", userID, err)
+			http.Error(w, "internal server error", http.StatusInternalServerError)
+			return
+		}
+	}
+
 	newHash, err := bcrypt.GenerateFromPassword([]byte(req.NewPassword), bcrypt.DefaultCost)
 	if err != nil {
 		log.Printf("Failed to hash new password for user %s: %v", userID, err)

backend/handlers/user/twofa.go

@@ -210,7 +210,7 @@ type TwoFAStatusResponse struct {
 func GetTwoFAStatusHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
@@ -223,7 +223,7 @@ func GetTwoFAStatusHandler(w http.ResponseWriter, r *http.Request) {
 	`, userID).Scan(&enabled, &method)
 	if err != nil {
 		log.Printf("failed to fetch 2FA status for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 
@@ -254,13 +254,13 @@ type TwoFASetupRequest struct {
 func SetupTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
 	var req TwoFASetupRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		http.Error(w, "invalid request", http.StatusBadRequest)
+		mw.RespondError(w, http.StatusBadRequest, "invalid request")
 		return
 	}
 	if req.Method != "email" && req.Method != "sms" {
@@ -272,7 +272,7 @@ func SetupTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 	err := db.Conn.QueryRow(r.Context(), `SELECT two_factor_enabled FROM users WHERE id = $1`, userID).Scan(&enabled)
 	if err != nil {
 		log.Printf("failed to check 2FA state for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	if enabled {
@@ -315,7 +315,7 @@ func SetupTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to store 2FA pending code for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	st.SetLastMintAtLocked(now)
@@ -403,13 +403,13 @@ func VerifyTwoFACodeForUser(ctx context.Context, userID, code string) error {
 func VerifyTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
 	var req TwoFAVerifyRequest
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		http.Error(w, "invalid request", http.StatusBadRequest)
+		mw.RespondError(w, http.StatusBadRequest, "invalid request")
 		return
 	}
 
@@ -417,7 +417,7 @@ func VerifyTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 		// Dev bypass: no code verification in unenforced environments.
 		if err := enableTwoFA(r, userID); err != nil {
 			log.Printf("failed to enable 2FA for user %s: %v", userID, err)
-			http.Error(w, "server error", http.StatusInternalServerError)
+			mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 			return
 		}
 		writeTwoFAEnabled(w)
@@ -435,7 +435,7 @@ func VerifyTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 	result, err := checkTwoFACode(r, userID, st, req.Code)
 	if err != nil {
 		log.Printf("failed to check 2FA pending code for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	switch result {
@@ -462,7 +462,7 @@ func VerifyTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 
 	if err := enableTwoFA(r, userID); err != nil {
 		log.Printf("failed to enable 2FA for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	writeTwoFAEnabled(w)
@@ -528,7 +528,7 @@ type TwoFADisableRequest struct {
 func SendDisableCodeHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
@@ -552,7 +552,7 @@ func SendDisableCodeHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to prepare 2FA code for disable for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	w.WriteHeader(http.StatusOK)
@@ -588,7 +588,7 @@ func SendDisableCodeHandler(w http.ResponseWriter, r *http.Request) {
 func SendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
@@ -596,7 +596,7 @@ func SendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 	err := db.Conn.QueryRow(r.Context(), `SELECT two_factor_enabled FROM users WHERE id = $1`, userID).Scan(&enabled)
 	if err != nil {
 		log.Printf("failed to check 2FA state for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	if !enabled {
@@ -625,7 +625,7 @@ func SendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to prepare 2FA code for saved-card charge for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 
@@ -671,7 +671,7 @@ func AdminSendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to check 2FA state for user %s: %v", targetUserID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	if !enabled {
@@ -697,7 +697,7 @@ func AdminSendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to prepare 2FA code for user %s: %v", targetUserID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 
@@ -742,7 +742,7 @@ func AdminSendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
 func DisableTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 	userID, ok := mw.GetUserID(r.Context())
 	if !ok {
-		http.Error(w, "unauthorized", http.StatusUnauthorized)
+		mw.RespondError(w, http.StatusUnauthorized, "unauthorized")
 		return
 	}
 
@@ -755,7 +755,7 @@ func DisableTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 		// Dev bypass: no re-verification in unenforced environments.
 		if err := disableTwoFA(r, userID); err != nil {
 			log.Printf("failed to disable 2FA for user %s: %v", userID, err)
-			http.Error(w, "server error", http.StatusInternalServerError)
+			mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 			return
 		}
 		w.WriteHeader(http.StatusOK)
@@ -789,14 +789,14 @@ func DisableTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		log.Printf("failed to prepare 2FA code for disable for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 
 	result, err := checkTwoFACode(r, userID, st, req.Code)
 	if err != nil {
 		log.Printf("failed to check 2FA pending code for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	switch result {
@@ -814,7 +814,7 @@ func DisableTwoFAHandler(w http.ResponseWriter, r *http.Request) {
 
 	if err := disableTwoFA(r, userID); err != nil {
 		log.Printf("failed to disable 2FA for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	w.WriteHeader(http.StatusOK)
@@ -942,7 +942,7 @@ func AdminRemoveUser2FAHandler(w http.ResponseWriter, r *http.Request) {
 	affected, err := removeUser2FA(r, userID)
 	if err != nil {
 		log.Printf("failed to remove 2FA for user %s: %v", userID, err)
-		http.Error(w, "server error", http.StatusInternalServerError)
+		mw.RespondError(w, http.StatusInternalServerError, "internal server error")
 		return
 	}
 	if !affected {

backend/mw/auth.go

@@ -53,7 +53,7 @@ func OptionalAuth(next http.Handler) http.Handler {
 
 			userID, role, jti, err := auth.VerifyToken(tokenString, r.Context())
 			if err != nil {
-				log.Printf("OptionalAuth: invalid token: %v", err)
+				log.Printf("OptionalAuth: invalid token ignored")
 			} else {
 				ctx := context.WithValue(r.Context(), UserIDKey, userID)
 				ctx = context.WithValue(ctx, UserRoleKey, role)