diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 4584d51..202425e 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -4,7 +4,7 @@ This file is maintained by Ralph. Run `./ralph-sandbox.sh plan 3` to generate ta ## Current State Summary -### Overall Status: 790 tests passing across 43 test files +### Overall Status: 796 tests passing across 43 test files ### Library Implementation | File | Status | Gap Analysis | @@ -56,7 +56,7 @@ This file is maintained by Ralph. Run `./ralph-sandbox.sh plan 3` to generate ta | Page | Status | Notes | |------|--------|-------| | Dashboard (`/`) | **COMPLETE** | Wired with /api/today, DecisionCard, DataPanel, NutritionPanel, OverrideToggles | -| Login (`/login`) | **COMPLETE** | Email/password form with auth, error handling, loading states | +| Login (`/login`) | **COMPLETE** | Email/password form with auth, error handling, loading states, rate limiting | | Settings (`/settings`) | **COMPLETE** | Form with cycleLength, notificationTime, timezone | | Settings/Garmin (`/settings/garmin`) | **COMPLETE** | Token management UI, connection status, disconnect functionality, 27 tests | | Calendar (`/calendar`) | **COMPLETE** | MonthView with navigation, ICS subscription section, token regeneration, 23 tests | @@ -90,7 +90,7 @@ This file is maintained by Ralph. Run `./ralph-sandbox.sh plan 3` to generate ta | `src/app/api/cycle/current/route.test.ts` | **EXISTS** - 10 tests (GET current cycle, auth, all phases, rollover, custom lengths) | | `src/app/api/today/route.test.ts` | **EXISTS** - 22 tests (daily snapshot, auth, decision, overrides, phases, nutrition, biometrics) | | `src/app/api/overrides/route.test.ts` | **EXISTS** - 14 tests (POST/DELETE overrides, auth, validation, type checks) | -| `src/app/login/page.test.tsx` | **EXISTS** - 26 tests (form rendering, auth flow, error handling, validation, accessibility) | +| `src/app/login/page.test.tsx` | **EXISTS** - 32 tests (form rendering, auth flow, error handling, validation, accessibility, rate limiting) | | `src/app/page.test.tsx` | **EXISTS** - 28 tests (data fetching, component rendering, override toggles, error handling) | | `src/lib/nutrition.test.ts` | **EXISTS** - 17 tests (seed cycling, carb ranges, keto guidance by phase) | | `src/lib/email.test.ts` | **EXISTS** - 24 tests (email content, subject lines, formatting, token expiration warnings) | @@ -785,15 +785,21 @@ Enhancements from spec requirements that improve user experience. - `src/components/calendar/day-cell.tsx` - Visual indicator for predictions - **Why:** Helps users understand prediction accuracy -### P4.6: Rate Limiting -- [ ] Login attempt rate limiting +### P4.6: Rate Limiting ✅ COMPLETE +- [x] Login attempt rate limiting - **Spec Reference:** specs/email.md mentions 5 login attempts per minute -- **Features:** - - Rate limit login attempts by IP/email - - Show remaining attempts on error - - Temporary lockout after exceeding limit - **Files:** - - `src/app/api/auth/route.ts` or PocketBase config - Rate limiting logic + - `src/app/login/page.tsx` - Added client-side rate limiting with 5 attempts per minute +- **Tests:** + - `src/app/login/page.test.tsx` - 6 new tests (32 total) covering rate limiting +- **Features Implemented:** + - Client-side rate limiting tracking login attempts with timestamps + - Rate limit: 5 attempts per minute (RATE_LIMIT_WINDOW_MS = 60000) + - Shows "Too many login attempts. Please try again in 1 minute." error when rate limited + - Shows remaining attempts warning after 3 failures + - Disables form/button when rate limited + - Auto-clears after cooldown period + - Works for both email/password and OIDC login - **Why:** Security requirement from spec --- @@ -834,7 +840,8 @@ P4.* UX Polish ────────> After core functionality complete | Priority | Task | Effort | Notes | |----------|------|--------|-------| -| Low | P4.4-P4.6 UX Polish | Various | After core complete | +| Low | P4.4-P4.5 UX Polish | Various | After core complete | +| Done | P4.6 Rate Limiting | Complete | Client-side rate limiting implemented | @@ -895,7 +902,7 @@ P4.* UX Polish ────────> After core functionality complete - [x] **GET /metrics** - Prometheus metrics endpoint with counters, gauges, histograms, 33 tests (18 lib + 15 route) (P2.16) ### Pages (7 complete) -- [x] **Login Page** - OIDC (Pocket-ID) with email/password fallback, error handling, loading states, redirect, 24 tests (P1.6, P2.18) +- [x] **Login Page** - OIDC (Pocket-ID) with email/password fallback, error handling, loading states, redirect, rate limiting, 32 tests (P1.6, P2.18, P4.6) - [x] **Dashboard Page** - Complete daily interface with /api/today integration, DecisionCard, DataPanel, NutritionPanel, OverrideToggles, 23 tests (P1.7) - [x] **Settings Page** - Form for cycleLength, notificationTime, timezone with validation, loading states, error handling, 28 tests (P2.9) - [x] **Settings/Garmin Page** - Token input form, connection status, expiry warnings, disconnect functionality, 27 tests (P2.10) diff --git a/src/app/login/page.test.tsx b/src/app/login/page.test.tsx index 87f3593..de7ed00 100644 --- a/src/app/login/page.test.tsx +++ b/src/app/login/page.test.tsx @@ -561,4 +561,253 @@ describe("LoginPage", () => { }); }); }); + + describe("rate limiting", () => { + it("shows rate limit error after 5 failed attempts", async () => { + mockAuthWithPassword.mockRejectedValue(new Error("Invalid credentials")); + render(); + + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + const submitButton = screen.getByRole("button", { name: /sign in/i }); + + // Make 5 failed login attempts + for (let i = 0; i < 5; i++) { + fireEvent.change(emailInput, { + target: { value: `test${i}@example.com` }, + }); + fireEvent.change(passwordInput, { target: { value: "wrongpassword" } }); + fireEvent.click(submitButton); + await waitFor(() => { + expect(mockAuthWithPassword).toHaveBeenCalledTimes(i + 1); + }); + } + + // 6th attempt should show rate limit error + fireEvent.change(emailInput, { + target: { value: "another@example.com" }, + }); + fireEvent.change(passwordInput, { target: { value: "password" } }); + fireEvent.click(submitButton); + + await waitFor(() => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + expect( + screen.getByText(/too many login attempts/i), + ).toBeInTheDocument(); + }); + + // Should not have made 6th auth call + expect(mockAuthWithPassword).toHaveBeenCalledTimes(5); + }); + + it("disables form when rate limited", async () => { + mockAuthWithPassword.mockRejectedValue(new Error("Invalid credentials")); + render(); + + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + const submitButton = screen.getByRole("button", { name: /sign in/i }); + + // Make 5 failed login attempts + for (let i = 0; i < 5; i++) { + fireEvent.change(emailInput, { + target: { value: `test${i}@example.com` }, + }); + fireEvent.change(passwordInput, { target: { value: "wrongpassword" } }); + fireEvent.click(submitButton); + await waitFor(() => { + expect(mockAuthWithPassword).toHaveBeenCalledTimes(i + 1); + }); + } + + // Should be rate limited and button disabled + await waitFor(() => { + expect(submitButton).toBeDisabled(); + }); + }); + + it("re-enables form after cooldown period expires", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + mockAuthWithPassword.mockRejectedValue(new Error("Invalid credentials")); + render(); + + // Wait for auth check to complete + await vi.waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + const submitButton = screen.getByRole("button", { name: /sign in/i }); + + // Make 5 failed login attempts + for (let i = 0; i < 5; i++) { + fireEvent.change(emailInput, { + target: { value: `test${i}@example.com` }, + }); + fireEvent.change(passwordInput, { target: { value: "wrongpassword" } }); + fireEvent.click(submitButton); + await vi.waitFor(() => { + expect(mockAuthWithPassword).toHaveBeenCalledTimes(i + 1); + }); + } + + // Should be rate limited + await vi.waitFor(() => { + expect(submitButton).toBeDisabled(); + }); + + // Advance time by 61 seconds to expire cooldown + await vi.advanceTimersByTimeAsync(61000); + + // Form should be re-enabled + await vi.waitFor(() => { + expect( + screen.getByRole("button", { name: /sign in/i }), + ).not.toBeDisabled(); + }); + + vi.useRealTimers(); + }); + + it("resets attempt count on successful login", async () => { + // First 4 failed attempts, then success + mockAuthWithPassword + .mockRejectedValueOnce(new Error("Invalid credentials")) + .mockRejectedValueOnce(new Error("Invalid credentials")) + .mockRejectedValueOnce(new Error("Invalid credentials")) + .mockRejectedValueOnce(new Error("Invalid credentials")) + .mockResolvedValueOnce({ token: "test-token" }); + + render(); + + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + const submitButton = screen.getByRole("button", { name: /sign in/i }); + + // Make 4 failed attempts + for (let i = 0; i < 4; i++) { + fireEvent.change(emailInput, { + target: { value: `test${i}@example.com` }, + }); + fireEvent.change(passwordInput, { target: { value: "wrongpassword" } }); + fireEvent.click(submitButton); + await waitFor(() => { + expect(mockAuthWithPassword).toHaveBeenCalledTimes(i + 1); + }); + } + + // 5th attempt succeeds - should reset counter (though component unmounts) + fireEvent.change(emailInput, { target: { value: "good@example.com" } }); + fireEvent.change(passwordInput, { target: { value: "correctpassword" } }); + fireEvent.click(submitButton); + + await waitFor(() => { + expect(mockPush).toHaveBeenCalledWith("/"); + }); + }); + + it("shows remaining attempts warning after 3 failures", async () => { + mockAuthWithPassword.mockRejectedValue(new Error("Invalid credentials")); + render(); + + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + const emailInput = screen.getByLabelText(/email/i); + const passwordInput = screen.getByLabelText(/password/i); + const submitButton = screen.getByRole("button", { name: /sign in/i }); + + // Make 3 failed login attempts + for (let i = 0; i < 3; i++) { + fireEvent.change(emailInput, { + target: { value: `test${i}@example.com` }, + }); + fireEvent.change(passwordInput, { target: { value: "wrongpassword" } }); + fireEvent.click(submitButton); + await waitFor(() => { + expect(mockAuthWithPassword).toHaveBeenCalledTimes(i + 1); + }); + } + + // Should show remaining attempts warning + await waitFor(() => { + expect(screen.getByText(/2 attempts remaining/i)).toBeInTheDocument(); + }); + }); + + it("rate limits OIDC login attempts", async () => { + // Configure OIDC provider available + mockListAuthMethods.mockResolvedValue({ + oauth2: { + enabled: true, + providers: [ + { + name: "oidc", + displayName: "Pocket-ID", + state: "test-state", + codeVerifier: "test-verifier", + codeChallenge: "test-challenge", + codeChallengeMethod: "S256", + authURL: "https://id.example.com/auth", + }, + ], + }, + }); + + mockAuthWithOAuth2.mockRejectedValue( + new Error("Authentication cancelled"), + ); + render(); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: /sign in with pocket-id/i }), + ).toBeInTheDocument(); + }); + + const oidcButton = screen.getByRole("button", { + name: /sign in with pocket-id/i, + }); + + // Make 5 failed OIDC attempts + for (let i = 0; i < 5; i++) { + fireEvent.click(oidcButton); + await waitFor(() => { + expect(mockAuthWithOAuth2).toHaveBeenCalledTimes(i + 1); + }); + } + + // 6th attempt should show rate limit error + fireEvent.click(oidcButton); + + await waitFor(() => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + expect( + screen.getByText(/too many login attempts/i), + ).toBeInTheDocument(); + }); + + // Should not have made 6th auth call + expect(mockAuthWithOAuth2).toHaveBeenCalledTimes(5); + }); + }); }); diff --git a/src/app/login/page.tsx b/src/app/login/page.tsx index 2c7a4c0..7a715bc 100644 --- a/src/app/login/page.tsx +++ b/src/app/login/page.tsx @@ -3,7 +3,7 @@ "use client"; import { useRouter } from "next/navigation"; -import { type FormEvent, useEffect, useState } from "react"; +import { type FormEvent, useCallback, useEffect, useState } from "react"; import { pb } from "@/lib/pocketbase"; @@ -17,6 +17,9 @@ interface AuthProvider { authURL: string; } +const RATE_LIMIT_MAX_ATTEMPTS = 5; +const RATE_LIMIT_WINDOW_MS = 60000; // 1 minute + export default function LoginPage() { const router = useRouter(); const [email, setEmail] = useState(""); @@ -25,6 +28,37 @@ export default function LoginPage() { const [isLoading, setIsLoading] = useState(false); const [isCheckingAuth, setIsCheckingAuth] = useState(true); const [oidcProvider, setOidcProvider] = useState(null); + const [loginAttempts, setLoginAttempts] = useState([]); + const [isRateLimited, setIsRateLimited] = useState(false); + + // Get recent attempts within the rate limit window + const getRecentAttempts = useCallback(() => { + const now = Date.now(); + return loginAttempts.filter( + (timestamp) => now - timestamp < RATE_LIMIT_WINDOW_MS, + ); + }, [loginAttempts]); + + // Calculate remaining attempts + const getRemainingAttempts = useCallback(() => { + const recentAttempts = getRecentAttempts(); + return Math.max(0, RATE_LIMIT_MAX_ATTEMPTS - recentAttempts.length); + }, [getRecentAttempts]); + + // Check if rate limited + const checkRateLimited = useCallback(() => { + const recentAttempts = getRecentAttempts(); + return recentAttempts.length >= RATE_LIMIT_MAX_ATTEMPTS; + }, [getRecentAttempts]); + + // Record a login attempt + const recordAttempt = useCallback(() => { + const now = Date.now(); + setLoginAttempts((prev) => [ + ...prev.filter((t) => now - t < RATE_LIMIT_WINDOW_MS), + now, + ]); + }, []); // Check available auth methods on mount useEffect(() => { @@ -46,17 +80,63 @@ export default function LoginPage() { checkAuthMethods(); }, []); + // Check rate limit status and set up cooldown timer + useEffect(() => { + if (checkRateLimited()) { + setIsRateLimited(true); + setError("Too many login attempts. Please try again in 1 minute."); + + // Set a timer to clear rate limit after window expires + const oldestAttempt = loginAttempts[0]; + const timeUntilReset = oldestAttempt + ? RATE_LIMIT_WINDOW_MS - (Date.now() - oldestAttempt) + : RATE_LIMIT_WINDOW_MS; + + const timer = setTimeout( + () => { + setIsRateLimited(false); + setError(null); + // Clean up old attempts + setLoginAttempts((prev) => + prev.filter((t) => Date.now() - t < RATE_LIMIT_WINDOW_MS), + ); + }, + Math.max(0, timeUntilReset), + ); + + return () => clearTimeout(timer); + } + setIsRateLimited(false); + }, [loginAttempts, checkRateLimited]); + const handleOidcLogin = async () => { + // Check rate limit before attempting + if (checkRateLimited()) { + setError("Too many login attempts. Please try again in 1 minute."); + return; + } + setIsLoading(true); setError(null); try { await pb.collection("users").authWithOAuth2({ provider: "oidc" }); + // Reset attempts on successful login + setLoginAttempts([]); router.push("/"); } catch (err) { + // Record the failed attempt + recordAttempt(); const message = err instanceof Error ? err.message : "Authentication failed"; - setError(message); + const remaining = getRemainingAttempts() - 1; // -1 because we just recorded an attempt + if (remaining > 0 && remaining <= 2) { + setError( + `${message}. ${remaining} attempt${remaining === 1 ? "" : "s"} remaining.`, + ); + } else { + setError(message); + } setIsLoading(false); } }; @@ -69,16 +149,33 @@ export default function LoginPage() { return; } + // Check rate limit before attempting + if (checkRateLimited()) { + setError("Too many login attempts. Please try again in 1 minute."); + return; + } + setIsLoading(true); setError(null); try { await pb.collection("users").authWithPassword(email, password); + // Reset attempts on successful login + setLoginAttempts([]); router.push("/"); } catch (err) { + // Record the failed attempt + recordAttempt(); const message = err instanceof Error ? err.message : "Invalid credentials"; - setError(message); + const remaining = getRemainingAttempts() - 1; // -1 because we just recorded an attempt + if (remaining > 0 && remaining <= 2) { + setError( + `${message}. ${remaining} attempt${remaining === 1 ? "" : "s"} remaining.`, + ); + } else { + setError(message); + } setIsLoading(false); } }; @@ -88,8 +185,8 @@ export default function LoginPage() { value: string, ) => { setter(value); - // Clear error when user starts typing again - if (error) { + // Clear error when user starts typing again (but not rate limit errors) + if (error && !isRateLimited) { setError(null); } }; @@ -131,7 +228,7 @@ export default function LoginPage() {