diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 5e8805b..25db5da 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: 707 tests passing across 40 test files +### Overall Status: 717 tests passing across 40 test files ### Library Implementation | File | Status | Gap Analysis | @@ -28,7 +28,7 @@ This file is maintained by Ralph. Run `./ralph-sandbox.sh plan 3` to generate ta | Health Check Endpoint | specs/observability.md | P2.15 | **COMPLETE** | | Prometheus Metrics | specs/observability.md | P2.16 | **COMPLETE** | | Structured Logging (pino) | specs/observability.md | P2.17 | **COMPLETE** | -| OIDC Authentication | specs/authentication.md | P2.18 | Medium | +| OIDC Authentication | specs/authentication.md | P2.18 | **COMPLETE** | | Token Expiration Warnings | specs/email.md | P3.9 | **COMPLETE** | ### API Routes (17 total) @@ -529,27 +529,28 @@ Full feature set for production use. - **Why:** Required for log aggregators and production debugging (per specs/observability.md) - **Next Step:** Integrate logger into API routes (can be done incrementally) -### P2.18: OIDC Authentication -- [ ] Replace email/password login with OIDC (Pocket-ID) -- **Current State:** Using email/password form, no OIDC code exists +### P2.18: OIDC Authentication ✅ COMPLETE +- [x] Replace email/password login with OIDC (Pocket-ID) - **Files:** - - `src/app/login/page.tsx` - Replace form with "Sign In with Pocket-ID" button - - `src/lib/pocketbase.ts` - Add OIDC redirect and callback handling + - `src/app/login/page.tsx` - OIDC button with email/password fallback - **Tests:** - - Update `src/app/login/page.test.tsx` - Tests for OIDC redirect flow + - `src/app/login/page.test.tsx` - 24 tests (10 new OIDC tests) +- **Features Implemented:** + - Auto-detection of OIDC provider via `listAuthMethods()` API + - "Sign In with Pocket-ID" button when OIDC provider is configured + - Email/password form fallback when OIDC is not available + - PocketBase `authWithOAuth2()` popup-based OAuth2 flow + - Loading states during authentication + - Error handling with user-friendly messages - **Flow:** - 1. User clicks "Sign In with Pocket-ID" - 2. Redirect to Pocket-ID authorization endpoint - 3. User authenticates (MFA if configured) - 4. Callback with authorization code - 5. PocketBase exchanges code for tokens - 6. Redirect to dashboard -- **Environment Variables:** - - `POCKETBASE_OIDC_CLIENT_ID` - - `POCKETBASE_OIDC_CLIENT_SECRET` - - `POCKETBASE_OIDC_ISSUER_URL` + 1. Page checks for available auth methods on mount + 2. If OIDC provider configured, shows "Sign In with Pocket-ID" button + 3. User clicks button, PocketBase handles OAuth2 popup flow + 4. On success, user redirected to dashboard + 5. Falls back to email/password when OIDC not available +- **Environment Variables (configured in PocketBase Admin):** + - Client ID, Client Secret, Issuer URL configured in PocketBase - **Why:** Required per specs/authentication.md for secure identity management -- **Note:** Current email/password implementation works but OIDC is the production requirement --- @@ -797,7 +798,6 @@ P4.* UX Polish ────────> After core functionality complete | Priority | Task | Effort | Notes | |----------|------|--------|-------| -| Medium | P2.18 OIDC Auth | Large | Production auth requirement | | Low | P3.7 Error Handling | Small | Polish | | Low | P3.8 Loading States | Small | Polish | | Low | P4.* UX Polish | Various | After core complete | @@ -810,7 +810,6 @@ P4.* UX Polish ────────> After core functionality complete | P0.2 | P0.1 | P0.4, P1.1-P1.5, P2.2-P2.3, P2.7-P2.8 | | P0.3 | - | P1.4, P1.5 | | P0.4 | P0.1, P0.2 | P1.7, P2.9, P2.10, P2.13 | -| P2.18 | P1.6 | - | | P3.9 | P2.4 | - | --- @@ -860,7 +859,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** - Email/password form with PocketBase auth, error handling, loading states, redirect, 14 tests (P1.6) +- [x] **Login Page** - OIDC (Pocket-ID) with email/password fallback, error handling, loading states, redirect, 24 tests (P1.6, P2.18) - [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) @@ -909,7 +908,7 @@ P4.* UX Polish ────────> After core functionality complete 10. **Token Warnings:** Per spec, warnings are sent at exactly 14 days and 7 days before expiry (P3.9 COMPLETE) 11. **Health Check Priority:** P2.15 (GET /api/health) should be implemented early - it's required for deployment monitoring and load balancer health probes 12. **Structured Logging:** P2.17 (pino logger) is COMPLETE - new code should use `import { logger } from "@/lib/logger"` for all logging -13. **OIDC vs Email/Password:** Current email/password login (P1.6) works for development. P2.18 upgrades to OIDC for production security per specs/authentication.md +13. **OIDC Authentication:** P2.18 COMPLETE - Login page auto-detects OIDC via `listAuthMethods()` and shows "Sign In with Pocket-ID" button when configured. Falls back to email/password when OIDC not available. Configure OIDC provider in PocketBase Admin under Settings → Auth providers → OpenID Connect 14. **E2E Tests:** Authorized skip per specs/testing.md - unit and integration tests are sufficient for MVP 15. **Dark Mode:** Partial Tailwind support exists via dark: classes but may need prefers-color-scheme configuration in tailwind.config.js (see P4.3) 16. **Component Tests:** P3.11 COMPLETE - All 5 dashboard and calendar components now have comprehensive unit tests (82 tests total) diff --git a/src/app/login/page.test.tsx b/src/app/login/page.test.tsx index b333d15..b5df759 100644 --- a/src/app/login/page.test.tsx +++ b/src/app/login/page.test.tsx @@ -13,10 +13,14 @@ vi.mock("next/navigation", () => ({ // Mock PocketBase const mockAuthWithPassword = vi.fn(); +const mockAuthWithOAuth2 = vi.fn(); +const mockListAuthMethods = vi.fn(); vi.mock("@/lib/pocketbase", () => ({ pb: { collection: () => ({ authWithPassword: mockAuthWithPassword, + authWithOAuth2: mockAuthWithOAuth2, + listAuthMethods: mockListAuthMethods, }), }, })); @@ -26,22 +30,33 @@ import LoginPage from "./page"; describe("LoginPage", () => { beforeEach(() => { vi.clearAllMocks(); + // Default: no OIDC configured, show email/password form + mockListAuthMethods.mockResolvedValue({ + oauth2: { + enabled: false, + providers: [], + }, + }); }); describe("rendering", () => { - it("renders the login form with email and password inputs", () => { + it("renders the login form with email and password inputs", async () => { render(); - expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); - expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + }); }); - it("renders a sign in button", () => { + it("renders a sign in button", async () => { render(); - expect( - screen.getByRole("button", { name: /sign in/i }), - ).toBeInTheDocument(); + await waitFor(() => { + expect( + screen.getByRole("button", { name: /sign in/i }), + ).toBeInTheDocument(); + }); }); it("renders the PhaseFlow branding", () => { @@ -50,18 +65,22 @@ describe("LoginPage", () => { expect(screen.getByText(/phaseflow/i)).toBeInTheDocument(); }); - it("has email input with type email", () => { + it("has email input with type email", async () => { render(); - const emailInput = screen.getByLabelText(/email/i); - expect(emailInput).toHaveAttribute("type", "email"); + await waitFor(() => { + const emailInput = screen.getByLabelText(/email/i); + expect(emailInput).toHaveAttribute("type", "email"); + }); }); - it("has password input with type password", () => { + it("has password input with type password", async () => { render(); - const passwordInput = screen.getByLabelText(/password/i); - expect(passwordInput).toHaveAttribute("type", "password"); + await waitFor(() => { + const passwordInput = screen.getByLabelText(/password/i); + expect(passwordInput).toHaveAttribute("type", "password"); + }); }); }); @@ -70,6 +89,11 @@ describe("LoginPage", () => { mockAuthWithPassword.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 }); @@ -90,6 +114,11 @@ describe("LoginPage", () => { mockAuthWithPassword.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 }); @@ -113,6 +142,11 @@ describe("LoginPage", () => { 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 }); @@ -141,6 +175,11 @@ describe("LoginPage", () => { 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 }); @@ -166,6 +205,11 @@ describe("LoginPage", () => { ); 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 }); @@ -186,6 +230,11 @@ describe("LoginPage", () => { ); 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 }); @@ -210,6 +259,11 @@ describe("LoginPage", () => { ); 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 }); @@ -235,6 +289,11 @@ describe("LoginPage", () => { it("does not submit with empty email", async () => { render(); + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + }); + const passwordInput = screen.getByLabelText(/password/i); const submitButton = screen.getByRole("button", { name: /sign in/i }); @@ -248,6 +307,11 @@ describe("LoginPage", () => { it("does not submit with empty password", async () => { render(); + // Wait for auth check to complete + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + const emailInput = screen.getByLabelText(/email/i); const submitButton = screen.getByRole("button", { name: /sign in/i }); @@ -258,4 +322,223 @@ describe("LoginPage", () => { expect(mockAuthWithPassword).not.toHaveBeenCalled(); }); }); + + describe("OIDC authentication", () => { + beforeEach(() => { + // 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", + }, + ], + }, + }); + }); + + it("shows OIDC button when provider is configured", async () => { + render(); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: /sign in with pocket-id/i }), + ).toBeInTheDocument(); + }); + }); + + it("hides email/password form when OIDC is available", async () => { + render(); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: /sign in with pocket-id/i }), + ).toBeInTheDocument(); + }); + + // Email/password form should not be visible + expect(screen.queryByLabelText(/email/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/password/i)).not.toBeInTheDocument(); + }); + + it("calls authWithOAuth2 when OIDC button is clicked", async () => { + mockAuthWithOAuth2.mockResolvedValueOnce({ token: "test-token" }); + 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, + }); + fireEvent.click(oidcButton); + + await waitFor(() => { + expect(mockAuthWithOAuth2).toHaveBeenCalledWith({ provider: "oidc" }); + }); + }); + + it("redirects to dashboard on successful OIDC login", async () => { + mockAuthWithOAuth2.mockResolvedValueOnce({ + token: "test-token", + record: { id: "user-123" }, + }); + 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, + }); + fireEvent.click(oidcButton); + + await waitFor(() => { + expect(mockPush).toHaveBeenCalledWith("/"); + }); + }); + + it("shows loading state during OIDC authentication", async () => { + let resolveAuth: (value: unknown) => void = () => {}; + const authPromise = new Promise((resolve) => { + resolveAuth = resolve; + }); + mockAuthWithOAuth2.mockReturnValue(authPromise); + + 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, + }); + fireEvent.click(oidcButton); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: /signing in/i }), + ).toBeInTheDocument(); + expect(screen.getByRole("button")).toBeDisabled(); + }); + + resolveAuth({ token: "test-token" }); + }); + + it("shows error message on OIDC failure", async () => { + mockAuthWithOAuth2.mockRejectedValueOnce( + 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, + }); + fireEvent.click(oidcButton); + + await waitFor(() => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + expect( + screen.getByText(/authentication cancelled/i), + ).toBeInTheDocument(); + }); + }); + + it("re-enables OIDC button after error", async () => { + mockAuthWithOAuth2.mockRejectedValueOnce( + 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, + }); + fireEvent.click(oidcButton); + + await waitFor(() => { + expect(screen.getByRole("alert")).toBeInTheDocument(); + }); + + // Button should be re-enabled + expect( + screen.getByRole("button", { name: /sign in with pocket-id/i }), + ).not.toBeDisabled(); + }); + }); + + describe("fallback to email/password", () => { + it("shows email/password form when OIDC is not configured", async () => { + mockListAuthMethods.mockResolvedValue({ + oauth2: { + enabled: false, + providers: [], + }, + }); + + render(); + + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + }); + }); + + it("shows email/password form when listAuthMethods fails", async () => { + mockListAuthMethods.mockRejectedValue(new Error("Network error")); + + render(); + + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + }); + }); + + it("does not show OIDC button when no providers configured", async () => { + mockListAuthMethods.mockResolvedValue({ + oauth2: { + enabled: false, + providers: [], + }, + }); + + render(); + + await waitFor(() => { + expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); + }); + + expect( + screen.queryByRole("button", { name: /sign in with pocket-id/i }), + ).not.toBeInTheDocument(); + }); + }); }); diff --git a/src/app/login/page.tsx b/src/app/login/page.tsx index 05e8e54..79bd616 100644 --- a/src/app/login/page.tsx +++ b/src/app/login/page.tsx @@ -1,18 +1,65 @@ // ABOUTME: Login page for user authentication. -// ABOUTME: Provides email/password login form using PocketBase auth. +// ABOUTME: Provides OIDC (Pocket-ID) login with email/password fallback. "use client"; import { useRouter } from "next/navigation"; -import { type FormEvent, useState } from "react"; +import { type FormEvent, useEffect, useState } from "react"; import { pb } from "@/lib/pocketbase"; +interface AuthProvider { + name: string; + displayName: string; + state: string; + codeVerifier: string; + codeChallenge: string; + codeChallengeMethod: string; + authURL: string; +} + export default function LoginPage() { const router = useRouter(); const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(false); + const [isCheckingAuth, setIsCheckingAuth] = useState(true); + const [oidcProvider, setOidcProvider] = useState(null); + + // Check available auth methods on mount + useEffect(() => { + const checkAuthMethods = async () => { + try { + const authMethods = await pb.collection("users").listAuthMethods(); + const oidc = authMethods.oauth2?.providers?.find( + (p: AuthProvider) => p.name === "oidc", + ); + if (oidc) { + setOidcProvider(oidc); + } + } catch { + // If listAuthMethods fails, fall back to email/password + } finally { + setIsCheckingAuth(false); + } + }; + checkAuthMethods(); + }, []); + + const handleOidcLogin = async () => { + setIsLoading(true); + setError(null); + + try { + await pb.collection("users").authWithOAuth2({ provider: "oidc" }); + router.push("/"); + } catch (err) { + const message = + err instanceof Error ? err.message : "Authentication failed"; + setError(message); + setIsLoading(false); + } + }; const handleSubmit = async (e: FormEvent) => { e.preventDefault(); @@ -47,65 +94,92 @@ export default function LoginPage() { } }; + // Show loading state while checking auth methods + if (isCheckingAuth) { + return ( +
+
+

PhaseFlow

+
Loading...
+
+
+ ); + } + return (

PhaseFlow

-
- {error && ( -
- {error} -
- )} - -
- - handleInputChange(setEmail, e.target.value)} - disabled={isLoading} - className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-1 focus:ring-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed" - required - /> -
- -
- - handleInputChange(setPassword, e.target.value)} - disabled={isLoading} - className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-1 focus:ring-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed" - required - /> + {error && ( +
+ {error}
+ )} + {oidcProvider ? ( + // OIDC login button - + ) : ( + // Email/password form fallback +
+
+ + handleInputChange(setEmail, e.target.value)} + disabled={isLoading} + className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-1 focus:ring-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed" + required + /> +
+ +
+ + handleInputChange(setPassword, e.target.value)} + disabled={isLoading} + className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-1 focus:ring-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed" + required + /> +
+ + +
+ )}
);