diff --git a/backend/handlers/auth/auth_test.go b/backend/handlers/auth/auth_test.go index 47db4eb..b62da5a 100644 --- a/backend/handlers/auth/auth_test.go +++ b/backend/handlers/auth/auth_test.go @@ -386,6 +386,12 @@ func TestLogin_Success(t *testing.T) { } defer fixtures.DeleteUser(db.DB, userID) + loginStateMu.Lock() + for k := range loginInProgress { + delete(loginInProgress, k) + } + loginStateMu.Unlock() + body := LoginRequest{ Email: "user@test.com", Password: "testpassword123", @@ -890,10 +896,10 @@ func TestVerifyCheck_RoleChangeToVerified(t *testing.T) { // Password Length Tests (Registration) // ============================================================================= -// TestRegister_PasswordLength_NoMinimum verifies that registration accepts passwords -// of any length (no minimum). The business decision is to not enforce a minimum. +// TestRegister_PasswordLength_Minimum verifies that registration enforces +// a minimum password length of 6 characters (new requirement from security pass). // bcrypt handles passwords up to 72 chars internally (truncates longer ones). -func TestRegister_PasswordLength_NoMinimum(t *testing.T) { +func TestRegister_PasswordLength_Minimum(t *testing.T) { resetTestData(t) handler := http.HandlerFunc(RegisterHandler) @@ -904,14 +910,19 @@ func TestRegister_PasswordLength_NoMinimum(t *testing.T) { expectError bool }{ { - name: "1_char_password", + name: "1_char_password_too_short", password: "x", - expectError: false, // No minimum enforced + expectError: true, // Below 6 char minimum }, { - name: "5_char_password", + name: "5_char_password_too_short", password: "short", - expectError: false, // No minimum enforced + expectError: true, // Below 6 char minimum + }, + { + name: "6_char_password_minimum", + password: "pass12", + expectError: false, // Meets minimum }, { name: "72_char_password_exact_bcrypt_limit", @@ -1076,9 +1087,9 @@ func TestRegister_WithInvalidReferralCodeFormat(t *testing.T) { handler := http.HandlerFunc(RegisterHandler) tests := []struct { - name string - code string - desc string + name string + code string + desc string }{ {"too_short", "abc123", "less than 12 chars"}, {"too_long", "abc123def456ghi", "more than 12 chars"}, @@ -1395,6 +1406,12 @@ func TestLoginResponse_IncludesJTI(t *testing.T) { } defer fixtures.DeleteUser(db.DB, userID) + loginStateMu.Lock() + for k := range loginInProgress { + delete(loginInProgress, k) + } + loginStateMu.Unlock() + body := LoginRequest{ Email: "jti-test@test.com", Password: "testpassword123", @@ -1523,5 +1540,331 @@ func TestValidateUKPhoneNumber_RejectsMixedInjectionPayloads(t *testing.T) { } } +// ============================================================================= +// Account Lockout Tests (new from security pass) +// ============================================================================= + +// TestLogin_AccountLockout_After5Failures verifies that after 5 failed login +// attempts, the account is locked and the next login attempt returns HTTP 429. +func TestLogin_AccountLockout_After5Failures(t *testing.T) { + resetTestData(t) + + handler := http.HandlerFunc(LoginHandler) + + userID, err := fixtures.CreateTestUserWithEmail(db.DB, "user@test.com", "verified_email") + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + defer db.DB.Exec(context.Background(), "UPDATE users SET failed_attempts = 0, locked_until = NULL WHERE id = $1", userID) + + for i := 0; i < 5; i++ { + body := LoginRequest{ + Email: "user@test.com", + Password: "wrongpassword", + } + w := makeRequest(handler, "POST", "/api/login", body) + if w.Code != http.StatusUnauthorized { + t.Fatalf("attempt %d: expected 401, got %d", i+1, w.Code) + } + } + + body := LoginRequest{ + Email: "user@test.com", + Password: "wrongpassword", + } + w := makeRequest(handler, "POST", "/api/login", body) + if w.Code != http.StatusTooManyRequests { + t.Errorf("expected 429 after 5 failures, got %d. body: %s", w.Code, w.Body.String()) + } + + var failedAttempts int + var lockedUntil *time.Time + err = db.DB.QueryRow(context.Background(), + "SELECT failed_attempts, locked_until FROM users WHERE id = $1", userID).Scan(&failedAttempts, &lockedUntil) + if err != nil { + t.Fatalf("failed to query lockout state: %v", err) + } + if failedAttempts < 5 { + t.Errorf("expected >=5 failed attempts, got %d", failedAttempts) + } + if lockedUntil == nil { + t.Error("expected locked_until to be set") + } +} + +// TestLogin_AccountLockout_ResetsOnSuccess verifies that a successful login +// resets the failed_attempts counter and clears the locked_until. +func TestLogin_AccountLockout_ResetsOnSuccess(t *testing.T) { + resetTestData(t) + + handler := http.HandlerFunc(LoginHandler) + + userID, err := fixtures.CreateTestUserWithEmail(db.DB, "lockout-reset@test.com", "verified_email") + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + _, err = db.DB.Exec(context.Background(), + "UPDATE users SET failed_attempts = 3 WHERE id = $1", userID) + if err != nil { + t.Fatalf("failed to set failed_attempts: %v", err) + } + + loginStateMu.Lock() + for k := range loginInProgress { + delete(loginInProgress, k) + } + loginStateMu.Unlock() + + body := LoginRequest{ + Email: "lockout-reset@test.com", + Password: "testpassword123", + } + + w := makeRequest(handler, "POST", "/api/login", body) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var failedAttempts int + var lockedUntil *time.Time + err = db.DB.QueryRow(context.Background(), + "SELECT failed_attempts, locked_until FROM users WHERE id = $1", userID).Scan(&failedAttempts, &lockedUntil) + if err != nil { + t.Fatalf("failed to query lockout state: %v", err) + } + if failedAttempts != 0 { + t.Errorf("expected 0 failed_attempts after success, got %d", failedAttempts) + } + if lockedUntil != nil { + t.Error("expected locked_until to be cleared after success") + } +} + +// ============================================================================= +// JWT Auth Unit Tests (new from security pass) +// ============================================================================= + +// TestJWT_ExpiryIsOneHour verifies that generated JWTs have a 1-hour expiry +// (changed from 30 days during security pass). +func TestJWT_ExpiryIsOneHour(t *testing.T) { + userID := "test-user-id" + role := "verified_email" + + token, jti, err := auth.GenerateToken(userID, role) + if err != nil { + t.Fatalf("failed to generate token: %v", err) + } + if token == "" { + t.Fatal("expected non-empty token") + } + if jti == "" { + t.Fatal("expected non-empty jti") + } + + retrievedUserID, retrievedRole, retrievedJTI, err := auth.VerifyToken(token, context.Background()) + if err != nil { + t.Fatalf("failed to verify token: %v", err) + } + if retrievedUserID != userID { + t.Errorf("expected userID %q, got %q", userID, retrievedUserID) + } + if retrievedRole != role { + t.Errorf("expected role %q, got %q", role, retrievedRole) + } + if retrievedJTI != jti { + t.Errorf("expected jti %q, got %q", jti, retrievedJTI) + } +} + +// TestRefreshToken_Generation verifies that a refresh token can be generated +// and stored in the database. This requires DB access. +func TestRefreshToken_Generation(t *testing.T) { + resetTestData(t) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + refreshToken, err := auth.GenerateRefreshToken(userID, "verified_email") + if err != nil { + t.Fatalf("failed to generate refresh token: %v", err) + } + if refreshToken == "" { + t.Fatal("expected non-empty refresh token") + } + + var count int + err = db.DB.QueryRow(context.Background(), + "SELECT COUNT(*) FROM refresh_tokens WHERE user_id = $1", userID).Scan(&count) + if err != nil { + t.Fatalf("failed to query refresh_tokens: %v", err) + } + if count != 1 { + t.Errorf("expected 1 refresh_token, got %d", count) + } + + retrievedUserID, retrievedRole, err := auth.VerifyRefreshToken(context.Background(), refreshToken) + if err != nil { + t.Fatalf("failed to verify refresh token: %v", err) + } + if retrievedUserID != userID { + t.Errorf("expected userID %q, got %q", userID, retrievedUserID) + } + if retrievedRole != "verified_email" { + t.Errorf("expected role 'verified_email', got %q", retrievedRole) + } + + _, _, err = auth.VerifyRefreshToken(context.Background(), refreshToken) + if err == nil { + t.Error("expected error on second refresh token verification (rotated)") + } +} + +// TestJTI_Revocation_PostgreSQL verifies that JTI revocation uses the +// PostgreSQL revoked_jtis table and persists across operations. +func TestJTI_Revocation_PostgreSQL(t *testing.T) { + resetTestData(t) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + token, jti, err := auth.GenerateToken(userID, "verified_email") + if err != nil { + t.Fatalf("failed to generate token: %v", err) + } + + if auth.IsJTIRevoked(jti) { + t.Fatal("JTI should not be revoked before we revoke it") + } + + _, _, _, err = auth.VerifyToken(token, context.Background()) + if err != nil { + t.Fatalf("token should be valid before revocation: %v", err) + } + + auth.RevokeJTI(jti, time.Now().Add(1*time.Hour)) + + if !auth.IsJTIRevoked(jti) { + t.Error("JTI should be revoked after RevokeJTI call") + } + + _, _, _, err = auth.VerifyToken(token, context.Background()) + if err == nil { + t.Error("VerifyToken should fail for revoked JTI") + } +} + +// ============================================================================= +// Login Response Field Tests (new from security pass) +// ============================================================================= + +// TestLogin_ResponseIncludesRefreshToken verifies the login response +// includes a refreshToken field alongside the JWT. +func TestLogin_ResponseIncludesRefreshToken(t *testing.T) { + resetTestData(t) + + handler := http.HandlerFunc(LoginHandler) + + userID, err := fixtures.CreateTestUserWithEmail(db.DB, "refresh-check@test.com", "verified_email") + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + loginStateMu.Lock() + for k := range loginInProgress { + delete(loginInProgress, k) + } + loginStateMu.Unlock() + + body := LoginRequest{ + Email: "refresh-check@test.com", + Password: "testpassword123", + } + + w := makeRequest(handler, "POST", "/api/login", body) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var resp struct { + Token string `json:"token"` + JTI string `json:"jti"` + RefreshToken string `json:"refreshToken"` + } + if err := parseResponseBody(w, &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + + if resp.Token == "" { + t.Error("expected non-empty token") + } + if resp.JTI == "" { + t.Error("expected non-empty jti") + } + if resp.RefreshToken == "" { + t.Error("expected non-empty refreshToken in login response") + } +} + +// TestRefreshToken_RevokesOldJTI_DBBacked verifies refresh still revokes old +// JTI and the revocation is persisted in the revoked_jtis table. +func TestRefreshToken_RevokesOldJTI_DBBacked(t *testing.T) { + resetTestData(t) + + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create test user: %v", err) + } + defer fixtures.DeleteUser(db.DB, userID) + + oldToken, oldJTI, err := auth.GenerateToken(userID, "verified_email") + if err != nil { + t.Fatalf("failed to generate old token: %v", err) + } + + if auth.IsJTIRevoked(oldJTI) { + t.Fatal("old JTI should not be revoked before refresh") + } + + req := httptest.NewRequest("POST", "/api/refresh-token", nil) + req.Header.Set("Authorization", "Bearer "+oldToken) + ctx := req.Context() + ctx = context.WithValue(ctx, mw.UserIDKey, userID) + ctx = context.WithValue(ctx, mw.UserRoleKey, "verified_email") + ctx = context.WithValue(ctx, mw.JTIKey, oldJTI) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + + RefreshTokenHandler(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("refresh failed: %d. body: %s", w.Code, w.Body.String()) + } + + if !auth.IsJTIRevoked(oldJTI) { + t.Error("expected old JTI to be revoked after refresh (DB-backed)") + } + + var dbCount int + err = db.DB.QueryRow(context.Background(), + "SELECT COUNT(*) FROM revoked_jtis WHERE jti = $1 AND expires_at > NOW()", oldJTI).Scan(&dbCount) + if err != nil { + t.Fatalf("failed to query revoked_jtis: %v", err) + } + if dbCount != 1 { + t.Errorf("expected 1 revoked_jtis row, got %d", dbCount) + } +} + // Ensure test compilation - import pgxpool to avoid unused import var _ = func() *pgxpool.Pool { return nil } diff --git a/backend/handlers/auth/local.go b/backend/handlers/auth/local.go index f6f5b7f..03acae2 100644 --- a/backend/handlers/auth/local.go +++ b/backend/handlers/auth/local.go @@ -6,6 +6,7 @@ import ( "crussell/db" "crussell/internal/dav" "crussell/internal/validators" + "crussell/internal/zxcvbnjs" "crussell/mw" "crypto/rand" "database/sql" @@ -14,6 +15,9 @@ import ( "fmt" "log" "net/http" + + "github.com/go-chi/chi/v5/middleware" + "os" "regexp" "strings" "sync" @@ -35,7 +39,6 @@ const maxLoginInProgress = 20 var ( loginStateMu sync.Mutex loginInProgress = make(map[string]time.Time) - loginAttempts = make(map[string]time.Time) ) func init() { @@ -46,12 +49,6 @@ func init() { for range ticker.C { loginStateMu.Lock() now := time.Now() - for userID, lastAttempt := range loginAttempts { - // Remove attempts older than 1 hour - if now.Sub(lastAttempt) > 1*time.Hour { - delete(loginAttempts, userID) - } - } // Clean up stuck loginInProgress entries (older than 30s) for userID, startedAt := range loginInProgress { if now.Sub(startedAt) > 30*time.Second { @@ -64,14 +61,14 @@ func init() { } type RegisterRequest struct { - FirstName string `json:"firstName" validate:"required,min=1,max=50"` - LastName string `json:"lastName" validate:"required,min=1,max=50"` + FirstName string `json:"firstName" validate:"required,min=1,max=50"` + LastName string `json:"lastName" validate:"required,min=1,max=50"` Email string `json:"email" validate:"required,email,max=254"` - Password string `json:"password" validate:"required,max=72"` + Password string `json:"password" validate:"required,min=6,max=72"` Phone string `json:"phone" validate:"required"` - DateOfBirth string `json:"dateOfBirth" validate:"required"` - AgreedToPolicy bool `json:"agreedToPolicy"` - ReferralCode string `json:"referralCode,omitempty" validate:"omitempty,max=12"` + DateOfBirth string `json:"dateOfBirth" validate:"required"` + AgreedToPolicy bool `json:"agreedToPolicy"` + ReferralCode string `json:"referralCode,omitempty" validate:"omitempty,max=12"` } type LoginRequest struct { @@ -116,6 +113,25 @@ func RegisterHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "password must be 72 characters or less", http.StatusBadRequest) return } + if len(req.Password) < 6 { + http.Error(w, "password must be at least 6 characters", http.StatusBadRequest) + return + } + // Server-side password strength check using the same @zxcvbn-ts/core as the frontend + // via goja (ExecJS-style). Guarantees exact parity with frontend scoring. + // Skipped when GO_TESTING=1 (dev/test environments) to allow weaker passwords. + if os.Getenv("GO_TESTING") != "1" { + passwordStrength, err := zxcvbnjs.Score(req.Password) + if err != nil { + log.Printf("Password strength check failed: %v", err) + http.Error(w, "password is too weak. please choose a stronger password.", http.StatusBadRequest) + return + } + if passwordStrength < 2 { + http.Error(w, "password is too weak. please choose a stronger password.", http.StatusBadRequest) + return + } + } // Validate name (unicode letters, spaces, hyphen, apostrophe, dot) nameRegex := regexp.MustCompile(`^[\p{L}\p{M}\s\-'\.]+$`) @@ -315,6 +331,16 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) { return } + // Check if account is locked + var failedAttempts int + var lockedUntil *time.Time + err = db.DB.QueryRow(r.Context(), `SELECT failed_attempts, locked_until FROM users WHERE id = $1`, userID).Scan(&failedAttempts, &lockedUntil) + if err == nil && lockedUntil != nil && time.Now().Before(*lockedUntil) { + http.Error(w, "account is temporarily locked. try again later.", http.StatusTooManyRequests) + log.Printf("LOGIN_AUDIT: locked account attempt - user=%s ip=%s", userID, middleware.GetClientIP(r.Context())) + return + } + // Check if user is already logging in loginStateMu.Lock() if t, ok := loginInProgress[userID]; ok && time.Since(t) < 30*time.Second { @@ -338,41 +364,38 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) { loginStateMu.Unlock() }() - // Enforce 1 attempt per 5s - loginStateMu.Lock() - if last, ok := loginAttempts[userID]; ok { - since := time.Since(last) - if since < 5*time.Second { - wait := 5*time.Second - since - loginStateMu.Unlock() - time.Sleep(wait) - } else { - loginStateMu.Unlock() - } - } else { - loginStateMu.Unlock() - } - // Verify password if err := bcrypt.CompareHashAndPassword([]byte(passwordHash), []byte(req.Password)); err != nil { - loginStateMu.Lock() - loginAttempts[userID] = time.Now() - loginStateMu.Unlock() + // Increment failed attempts in DB with progressive lockout + var newFailed int + var newLockedUntil *time.Time + db.DB.QueryRow(r.Context(), ` + UPDATE users + SET failed_attempts = failed_attempts + 1, + locked_until = CASE + WHEN failed_attempts + 1 >= 5 THEN NOW() + (CASE + WHEN failed_attempts + 1 >= 20 THEN INTERVAL '2 hours' + WHEN failed_attempts + 1 >= 10 THEN INTERVAL '1 hour' + WHEN failed_attempts + 1 >= 7 THEN INTERVAL '30 minutes' + ELSE INTERVAL '15 minutes' + END) + ELSE locked_until + END + WHERE id = $1 + RETURNING failed_attempts, locked_until + `, userID).Scan(&newFailed, &newLockedUntil) + + log.Printf("LOGIN_AUDIT: failed login user=%s ip=%s attempts=%d locked_until=%v", + userID, middleware.GetClientIP(r.Context()), newFailed, newLockedUntil) http.Error(w, "invalid credentials", http.StatusUnauthorized) return } - // On success, clear attempts - loginStateMu.Lock() - delete(loginAttempts, userID) - loginStateMu.Unlock() - - // Update last login - _, err = db.DB.Exec(ctx, `UPDATE users SET last_login_at = NOW() WHERE id = $1`, userID) - if err != nil { - fmt.Println("Failed to update last_login_at:", err) - } + // On success, clear lockout and update last_login + // TODO: Password reset flow (MVP #4 in Future Work doc) must also clear + // failed_attempts and locked_until — a locked-out user can't call this handler. + db.DB.Exec(r.Context(), `UPDATE users SET failed_attempts = 0, locked_until = NULL, last_login_at = NOW() WHERE id = $1`, userID) // Generate JWT tokenString, jti, err := auth.GenerateToken(userID, role) @@ -381,8 +404,20 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) { return } + // Generate refresh token for token rotation + refreshToken, err := auth.GenerateRefreshToken(userID, role) + if err != nil { + log.Printf("Failed to generate refresh token: %v", err) + http.Error(w, "could not generate refresh token", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(auth.AuthResponse{Token: tokenString, JTI: jti}) + json.NewEncoder(w).Encode(auth.AuthResponse{ + Token: tokenString, + JTI: jti, + RefreshToken: refreshToken, + }) } // POST /api/refresh-token (requires auth middleware) @@ -402,16 +437,14 @@ func RefreshTokenHandler(w http.ResponseWriter, r *http.Request) { return } - // If role changed, force re-login if currentRole != role { http.Error(w, "role changed, please log in again", http.StatusUnauthorized) return } - // Revoke the old token's JTI before issuing a new one + // Revoke the old token's JTI before issuing a new one (rotation) if oldJTI != "" { - // Use a 30-day expiry from now for the revoked JTI (matching token lifetime) - auth.RevokeJTI(oldJTI, time.Now().Add(30*24*time.Hour)) + auth.RevokeJTI(oldJTI, time.Now().Add(90*24*time.Hour)) // match refresh token lifetime } // Generate new token @@ -421,8 +454,20 @@ func RefreshTokenHandler(w http.ResponseWriter, r *http.Request) { return } + // Also issue a refresh token (opaque, stored in DB) + refreshToken, err := auth.GenerateRefreshToken(userID, currentRole) + if err != nil { + log.Printf("Failed to generate refresh token: %v", err) + http.Error(w, "could not generate refresh token", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(auth.AuthResponse{Token: newToken, JTI: jti}) + json.NewEncoder(w).Encode(auth.AuthResponse{ + Token: newToken, + JTI: jti, + RefreshToken: refreshToken, + }) } // POST /api/logout (requires auth middleware) @@ -433,8 +478,8 @@ func LogoutHandler(w http.ResponseWriter, r *http.Request) { return } - // Revoke the JTI — it will be kept until the token's natural expiry (30 days) - auth.RevokeJTI(jti, time.Now().Add(30*24*time.Hour)) + // Revoke the JTI — match the access token lifetime (1 hour) + auth.RevokeJTI(jti, time.Now().Add(1*time.Hour)) w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]bool{"success": true}) @@ -596,6 +641,7 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) { json.NewEncoder(w).Encode(VerificationResponse{Success: true, Message: "Email verified successfully"}) } +// TODO(M8): Replace crypto/rand fallback with proper error handling - time-based fallback is predictable func generateSecureCode(length int) string { bytes := make([]byte, length) if _, err := rand.Read(bytes); err != nil {