diff --git a/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md b/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md new file mode 100644 index 0000000..fdd2b53 --- /dev/null +++ b/.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md @@ -0,0 +1,88 @@ +--- +phase: 12-initial-setup-wizard +plan: 05 +subsystem: setup-wizard-frontend +tags: [setup, pwa, uat-gap-closure, a11y] +requires: + - "GET /api/setup/status { setupComplete, dbName } (Plan 06)" + - "SetupStatusResponse.dbName?: string | null typed field (Plan 06)" +provides: + - "Instance step intro copy trimmed (no DB-vs-env-file aside)" + - "Read-only, disabled DB-name field under App URL, populated from status dbName" + - "Instance field values lifted to SetupPage so Back navigation preserves them" +affects: + - apps/pwa setup wizard Instance step (SetupPage.tsx) +tech-stack: + added: [] + patterns: + - "useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus }) reads non-secret dbName into a read-only field" + - "Step-level field values lifted to the parent (SetupPage) so step unmount no longer drops entries" + - "Sensitive app password deliberately NOT lifted — stays in Step3 local state, cleared on unmount (T-12-15)" +key-files: + created: [] + modified: + - apps/pwa/src/routes/SetupPage.tsx + - apps/pwa/src/routes/SetupPage.test.tsx +decisions: + - "D-12-05-LIFT: only the four non-secret Instance fields are lifted to SetupPage; the Fastmail app password is never lifted or persisted (T-12-15 preserved)." + - "D-12-05-DBNAME: DB-name field renders the dbName value only; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text, never as values (T-12-3DB)." + - "D-12-05-VALSTATE: Step2 validation state (db/oidc/vapid pass flags) is intentionally NOT lifted — only field values persist across Back; operator re-runs Save & Validate after returning." +metrics: + duration_minutes: 9 + completed: 2026-06-16 +--- + +# Phase 12 Plan 05: Instance-Step Gap Closure (copy trim, DB-name field, Back persistence) Summary + +Closed UAT gaps 1, 3 (frontend half), and 4 on the PWA Instance step (`SetupPage.tsx`): dropped the confusing DB-vs-env-file implementation aside, added a read-only env-derived DB-name field so the "database connection verified" row has an on-screen referent, and lifted the four Instance field values into `SetupPage` so navigating Back from the Calendar step no longer wipes entered config. + +## What Was Built + +### Task 1 — Drop DB-vs-env aside + add read-only DB-name field (gaps 1, 3-frontend) +Commit `35db5c5`. + +- **Gap 1**: Removed the sentence "These are written to the database — not your environment file." from the Instance step intro `

`, keeping the first sentence ("Enter your instance's connection details."). +- **Gap 3 (frontend)**: Added a labelled, `readOnly` + `disabled` input ("Database") directly under the App URL field, populated from `fetchSetupStatus().dbName` via `useQuery({ queryKey: ['setupStatus'], staleTime: 0, retry: false })`. The field is greyed out (`--color-surface-dim` background, `--color-text-secondary` text), carries `aria-readonly="true"` and `tabIndex={-1}`, and shows `—` while loading/null. Helper text explains the DB is configured via the server's Docker environment (DB_HOST/DB_PORT/DB_USER/DB_PASSWORD as static names) and is not entered here. The existing "Database connection verified." ValidationRow is unchanged. +- Tests assert the dropped sentence is absent, the DB field renders `readOnly`/`disabled`/`aria-readonly` with the mocked `dbName: 'familysync'`, and the existing DB validation row still appears on Save & Validate. + +### Task 2 — Preserve Instance fields across Back navigation (gap 4) +Commit `a13fc11`. + +- Introduced an `InstanceFields` shape (`appUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey`) owned by `SetupPage` (`instanceFields` / `setInstanceFields`), passed to `Step2Config` as `fields` / `setFields` props. `Step2Config` now reads/writes these through the lifted setters instead of its own local `useState`. Validation/mutation logic is unchanged. +- The Fastmail app password (Step 3) is **not** lifted — it remains in `Step3Credential` local state and is cleared on unmount when navigating away (T-12-15 preserved). +- Tests: filling the Instance step, validating to GREEN, advancing to the Calendar step, then clicking Back restores all four Instance values; a second test confirms a typed app password is empty after Back→forward (Step 3 re-mounts fresh). + +## Verification + +- `cd apps/pwa && pnpm test -- SetupPage` → **263 passed (22 files)**. +- `cd apps/pwa && pnpm typecheck` → clean (tsc + e2e tsconfig). +- `grep -c "not your environment file" apps/pwa/src/routes/SetupPage.tsx` → **0**. +- `grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` → **0**. +- `grep -nE "sessionStorage|localStorage" apps/pwa/src/routes/SetupPage.tsx` → **no matches** (no client-side persistence of any field, secret or otherwise). +- `grep -c "readOnly" apps/pwa/src/routes/SetupPage.tsx` → **1** (the DB-name field). +- DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear only as static env-var names in helper/error copy — never fetched or rendered as values. + +## Deviations from Plan + +None — plan executed exactly as written. Implementation note: the two tasks both restructure the `Step2Config` signature/body and the same intro paragraph, so they were authored together and then committed as two atomic, individually-GREEN commits (Task 1 commit verified GREEN with 261 tests before Task 2's state-lifting and Back-navigation tests were added). + +## Threat Surface + +| Threat ID | Disposition | Outcome | +|-----------|-------------|---------| +| T-12-15 (app password disclosure) | mitigate | Preserved — password stays in Step3 local state, type="password", NOT lifted, NOT persisted to storage; cleared on unmount. Test asserts it is empty after Back→forward. | +| T-12-14 (XSS in Instance/DB copy) | mitigate | All new copy + dbName rendered as plain-text JSX children; `dangerouslySetInnerHTML` grep = 0. | +| T-12-3DB (DB secret/topology disclosure) | mitigate | Only `dbName` value is fetched and rendered; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text. | + +No new security-relevant surface introduced beyond the planned `threat_model`. + +## Known Stubs + +None. + +## Self-Check: PASSED + +- `apps/pwa/src/routes/SetupPage.tsx` — modified, exists. +- `apps/pwa/src/routes/SetupPage.test.tsx` — modified, exists. +- Commit `35db5c5` (Task 1) — FOUND in git log. +- Commit `a13fc11` (Task 2) — FOUND in git log. diff --git a/apps/pwa/src/routes/SetupPage.test.tsx b/apps/pwa/src/routes/SetupPage.test.tsx index 32ca942..9634506 100644 --- a/apps/pwa/src/routes/SetupPage.test.tsx +++ b/apps/pwa/src/routes/SetupPage.test.tsx @@ -210,9 +210,14 @@ describe('SetupPage — Already Locked screen', () => { describe('SetupPage — Step 2 VAPID validation (CR-01 gap)', () => { let queryClient: QueryClient; - beforeEach(() => { + beforeEach(async () => { queryClient = makeQueryClient(); vi.resetAllMocks(); + const { fetchSetupStatus } = await import('../api/client.js'); + (fetchSetupStatus as ReturnType).mockResolvedValue({ + setupComplete: false, + dbName: 'familysync', + }); }); /** @@ -305,3 +310,141 @@ describe('SetupPage — Step 2 VAPID validation (CR-01 gap)', () => { }); }); }); + +// ── Tests: Step 2 Instance copy + read-only DB-name field (gaps 1, 3-frontend) ── + +describe('SetupPage — Step 2 Instance copy + DB-name field (gaps 1, 3)', () => { + let queryClient: QueryClient; + + beforeEach(async () => { + queryClient = makeQueryClient(); + vi.resetAllMocks(); + const { fetchSetupStatus } = await import('../api/client.js'); + (fetchSetupStatus as ReturnType).mockResolvedValue({ + setupComplete: false, + dbName: 'familysync', + }); + }); + + async function advanceToStep2() { + renderSetupPage(queryClient); + const continueBtn = await screen.findByRole('button', { name: 'Continue' }); + fireEvent.click(continueBtn); + await screen.findByText('Instance Configuration'); + } + + it('Instance step intro no longer contains the DB-vs-env-file aside (gap 1)', async () => { + await advanceToStep2(); + expect(screen.queryByText(/not your environment file/i)).toBeNull(); + // First sentence is preserved + expect(screen.getByText(/Enter your instance/i)).toBeInTheDocument(); + }); + + it('renders a read-only, disabled DB-name field populated from status dbName (gap 3)', async () => { + await advanceToStep2(); + const dbField = await screen.findByLabelText('Database'); + await waitFor(() => { + expect(dbField).toHaveValue('familysync'); + }); + expect(dbField).toHaveAttribute('readonly'); + expect(dbField).toBeDisabled(); + expect(dbField).toHaveAttribute('aria-readonly', 'true'); + }); + + it('keeps the existing "Database connection verified" validation row available', async () => { + const { postSetupConfig, validateSetupDb, validateSetupOidc, validateSetupVapid } = + await import('../api/client.js'); + (postSetupConfig as ReturnType).mockResolvedValue(undefined); + (validateSetupDb as ReturnType).mockResolvedValue(undefined); + (validateSetupOidc as ReturnType).mockResolvedValue(undefined); + (validateSetupVapid as ReturnType).mockResolvedValue(undefined); + + await advanceToStep2(); + fireEvent.change(screen.getByLabelText('App URL'), { target: { value: 'https://app.example.com' } }); + fireEvent.change(screen.getByLabelText('OIDC issuer URL'), { target: { value: 'https://auth.example.com' } }); + fireEvent.change(screen.getByLabelText('OIDC client ID'), { target: { value: 'familysync' } }); + fireEvent.change(screen.getByLabelText('VAPID public key'), { target: { value: 'BHtest123' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' })); + + await waitFor(() => { + expect(screen.getByText('Database connection verified.')).toBeInTheDocument(); + }); + }); +}); + +// ── Tests: Back navigation preserves Instance fields (gap 4) ────────────────── + +describe('SetupPage — Back navigation preserves Instance fields (gap 4)', () => { + let queryClient: QueryClient; + + beforeEach(async () => { + queryClient = makeQueryClient(); + vi.resetAllMocks(); + const { fetchSetupStatus } = await import('../api/client.js'); + (fetchSetupStatus as ReturnType).mockResolvedValue({ + setupComplete: false, + dbName: 'familysync', + }); + }); + + /** + * Helper: fill the four Instance fields, run validation to GREEN, advance to + * the Calendar step (step 3). + */ + async function fillStep2AndAdvance() { + const { postSetupConfig, validateSetupDb, validateSetupOidc, validateSetupVapid } = + await import('../api/client.js'); + (postSetupConfig as ReturnType).mockResolvedValue(undefined); + (validateSetupDb as ReturnType).mockResolvedValue(undefined); + (validateSetupOidc as ReturnType).mockResolvedValue(undefined); + (validateSetupVapid as ReturnType).mockResolvedValue(undefined); + + renderSetupPage(queryClient); + fireEvent.click(await screen.findByRole('button', { name: 'Continue' })); + await screen.findByText('Instance Configuration'); + + fireEvent.change(screen.getByLabelText('App URL'), { target: { value: 'https://app.example.com' } }); + fireEvent.change(screen.getByLabelText('OIDC issuer URL'), { target: { value: 'https://auth.example.com' } }); + fireEvent.change(screen.getByLabelText('OIDC client ID'), { target: { value: 'familysync' } }); + fireEvent.change(screen.getByLabelText('VAPID public key'), { target: { value: 'BHtest123' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' })); + + // After all validations pass, the Continue button appears + fireEvent.click(await screen.findByRole('button', { name: 'Continue' })); + await screen.findByText('Fastmail Credential'); + } + + it('restores all four Instance field values after navigating Back from Calendar step', async () => { + await fillStep2AndAdvance(); + + // Now on step 3 (Calendar). Navigate Back. + fireEvent.click(screen.getByRole('button', { name: 'Back' })); + await screen.findByText('Instance Configuration'); + + expect(screen.getByLabelText('App URL')).toHaveValue('https://app.example.com'); + expect(screen.getByLabelText('OIDC issuer URL')).toHaveValue('https://auth.example.com'); + expect(screen.getByLabelText('OIDC client ID')).toHaveValue('familysync'); + expect(screen.getByLabelText('VAPID public key')).toHaveValue('BHtest123'); + }); + + it('does NOT persist the Fastmail app password across Back/forward navigation (T-12-15)', async () => { + await fillStep2AndAdvance(); + + // On step 3: type a password into the app password field. + const pwField = screen.getByLabelText('App password'); + fireEvent.change(pwField, { target: { value: 'super-secret-pw' } }); + expect(pwField).toHaveValue('super-secret-pw'); + + // Back to step 2 — field values are preserved, but validation state is not + // lifted, so re-run Save & Validate to surface Continue, then advance to step 3. + fireEvent.click(screen.getByRole('button', { name: 'Back' })); + await screen.findByText('Instance Configuration'); + // Fields are still populated (gap 4), so just re-validate. + fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' })); + fireEvent.click(await screen.findByRole('button', { name: 'Continue' })); + await screen.findByText('Fastmail Credential'); + + // Step 3 re-mounted with fresh local state — the app password is NOT persisted. + expect(screen.getByLabelText('App password')).toHaveValue(''); + }); +}); diff --git a/apps/pwa/src/routes/SetupPage.tsx b/apps/pwa/src/routes/SetupPage.tsx index 92f7757..b5652f3 100644 --- a/apps/pwa/src/routes/SetupPage.tsx +++ b/apps/pwa/src/routes/SetupPage.tsx @@ -21,9 +21,10 @@ */ import { useState, useRef, useEffect } from 'react'; -import { useMutation } from '@tanstack/react-query'; +import { useMutation, useQuery } from '@tanstack/react-query'; import { ShieldCheck, CheckCircle, AlertCircle, Loader2 } from 'lucide-react'; import { + fetchSetupStatus, postSetupConfig, validateSetupDb, validateSetupOidc, @@ -429,19 +430,41 @@ function Step1Welcome({ onContinue, stepHeadingRef }: Step1Props) { // ── Step 2: Instance Configuration ─────────────────────────────────────────── +interface InstanceFields { + appUrl: string; + oidcIssuer: string; + oidcClientId: string; + vapidPublicKey: string; +} + interface Step2Props { onBack: () => void; onSuccess: () => void; stepHeadingRef: React.RefObject; + /** Lifted to SetupPage so values survive step unmount (gap 4: Back preserves entries). */ + fields: InstanceFields; + setFields: React.Dispatch>; } -function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) { - const [appUrl, setAppUrl] = useState(''); - const [oidcIssuer, setOidcIssuer] = useState(''); - const [oidcClientId, setOidcClientId] = useState(''); - const [vapidPublicKey, setVapidPublicKey] = useState(''); +function Step2Config({ onBack, onSuccess, stepHeadingRef, fields, setFields }: Step2Props) { + const { appUrl, oidcIssuer, oidcClientId, vapidPublicKey } = fields; + const setAppUrl = (v: string) => setFields((f) => ({ ...f, appUrl: v })); + const setOidcIssuer = (v: string) => setFields((f) => ({ ...f, oidcIssuer: v })); + const setOidcClientId = (v: string) => setFields((f) => ({ ...f, oidcClientId: v })); + const setVapidPublicKey = (v: string) => setFields((f) => ({ ...f, vapidPublicKey: v })); const [fieldError, setFieldError] = useState(null); + // Gap 3 (frontend): fetch the env-derived, non-secret DB name so the + // "database connection verified" row below has an on-screen referent. + // Only dbName is surfaced — DB_HOST/DB_USER/DB_PASSWORD are never fetched (T-12-3DB). + const { data: setupStatus } = useQuery({ + queryKey: ['setupStatus'], + queryFn: fetchSetupStatus, + staleTime: 0, + retry: false, + }); + const dbName = setupStatus?.dbName ?? ''; + const [validationRows, setValidationRows] = useState>({ db: 'idle', oidc: 'idle', @@ -553,8 +576,7 @@ function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) { lineHeight: 1.5, }} > - Enter your instance's connection details. These are written to the database — not - your environment file. + Enter your instance's connection details.

{/* App URL */} @@ -574,6 +596,32 @@ function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) {
The public URL where FamilySync is reachable.
+ {/* Database (read-only, env-derived) — gives the DB validation row below a referent */} +
+ + +
+ Configured via the server's Docker environment (DB_HOST,{' '} + DB_PORT, DB_USER, DB_PASSWORD) — not entered here. +
+
+ {/* OIDC Issuer */}