diff --git a/apps/api/src/routes/me.ts b/apps/api/src/routes/me.ts index 655e425..83b6082 100644 --- a/apps/api/src/routes/me.ts +++ b/apps/api/src/routes/me.ts @@ -167,35 +167,36 @@ const meNoEchoHook = (result: { success: boolean }, c: Context) => { } }; -meRouter.post( - '/credential', - zValidator('json', meCredentialSchema, meNoEchoHook), - async (c) => { - // Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body. - const currentUserId = await resolveUserId(c); - if (!currentUserId) { - return c.json({ error: 'Unauthorized' }, 401); +meRouter.post('/credential', zValidator('json', meCredentialSchema, meNoEchoHook), async (c) => { + // Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body. + const currentUserId = await resolveUserId(c); + if (!currentUserId) { + return c.json({ error: 'Unauthorized' }, 401); + } + + const { fastmailEmail, appPassword, providerType } = c.req.valid('json'); + // T-10-10: NEVER log appPassword or c.req.valid('json') here + + try { + // D-07: identical validate→encrypt→store→sync path as admin, but always with + // currentUserId (not a body userId). Admin passes the target member's userId; + // self-service passes the authenticated session userId. Same helper, same argument order. + await validateEncryptAndStoreCredential( + currentUserId, + fastmailEmail, + appPassword, + providerType, + ); + } catch (err) { + if (err instanceof CredentialValidationError) { + return c.json({ error: 'Invalid request' }, 400); } + console.error( + '[me/POST /credential] Unexpected error:', + err instanceof Error ? err.message : String(err), + ); + return c.json({ error: 'Service unavailable' }, 503); + } - const { fastmailEmail, appPassword, providerType } = c.req.valid('json'); - // T-10-10: NEVER log appPassword or c.req.valid('json') here - - try { - // D-07: identical validate→encrypt→store→sync path as admin, but always with - // currentUserId (not a body userId). Admin passes the target member's userId; - // self-service passes the authenticated session userId. Same helper, same argument order. - await validateEncryptAndStoreCredential(currentUserId, fastmailEmail, appPassword, providerType); - } catch (err) { - if (err instanceof CredentialValidationError) { - return c.json({ error: 'Invalid request' }, 400); - } - console.error( - '[me/POST /credential] Unexpected error:', - err instanceof Error ? err.message : String(err), - ); - return c.json({ error: 'Service unavailable' }, 503); - } - - return c.json({ ok: true }, 200); - }, -); + return c.json({ ok: true }, 200); +}); diff --git a/apps/api/tests/auth/user.test.ts b/apps/api/tests/auth/user.test.ts index dfaf314..1f809b6 100644 --- a/apps/api/tests/auth/user.test.ts +++ b/apps/api/tests/auth/user.test.ts @@ -314,7 +314,15 @@ describe('upsertUser', () => { } // Re-fetch after insert return makeSelectChain([ - { id: 10, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[0], isAdmin: true, createdAt: new Date() }, + { + id: 10, + oidcIss: iss, + oidcSub: sub, + displayName: null, + color: COLOR_PALETTE[0], + isAdmin: true, + createdAt: new Date(), + }, ]); }); mockDb.insert.mockReturnValue(makeInsertChain([{ id: 10 }])); @@ -344,7 +352,15 @@ describe('upsertUser', () => { return makeSelectChain([{ count: 1 }]); } return makeSelectChain([ - { id: 11, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[1], isAdmin: false, createdAt: new Date() }, + { + id: 11, + oidcIss: iss, + oidcSub: sub, + displayName: null, + color: COLOR_PALETTE[1], + isAdmin: false, + createdAt: new Date(), + }, ]); }); mockDb.insert.mockReturnValue(makeInsertChain([{ id: 11 }])); diff --git a/apps/api/tests/lib/requireAdmin.test.ts b/apps/api/tests/lib/requireAdmin.test.ts index b231de8..16cea75 100644 --- a/apps/api/tests/lib/requireAdmin.test.ts +++ b/apps/api/tests/lib/requireAdmin.test.ts @@ -24,7 +24,13 @@ vi.mock('../../src/db/client.js', () => ({ // Bring in the ContextVariableMap augmentation (sets up c.get('user') typing) vi.mock('../../src/auth/devBypass.js', () => ({ - DEV_USER: { id: 1, oidcIss: 'dev', oidcSub: 'dev-user', displayName: 'Dev User', color: '#4A90D9' }, + DEV_USER: { + id: 1, + oidcIss: 'dev', + oidcSub: 'dev-user', + displayName: 'Dev User', + color: '#4A90D9', + }, devAuthBypass: () => async (_c: unknown, next: () => Promise) => next(), COLOR_PALETTE: ['#4A90D9'], })); diff --git a/apps/api/tests/routes/me.test.ts b/apps/api/tests/routes/me.test.ts index 646a7a1..9747976 100644 --- a/apps/api/tests/routes/me.test.ts +++ b/apps/api/tests/routes/me.test.ts @@ -167,13 +167,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () = let callCount = 0; vi.mocked(db.select).mockImplementation(() => { callCount++; - const limitFn = callCount === 1 - ? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup - : vi.fn().mockResolvedValue([]); // memberCredentials lookup (none) + const limitFn = + callCount === 1 + ? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup + : vi.fn().mockResolvedValue([]); // memberCredentials lookup (none) return { from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ limit: limitFn }), - innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), + innerJoin: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }), + }), }), // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; @@ -183,7 +186,9 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () = const res = await app.request('/api/me'); expect(res.status).toBe(200); - const body = (await res.json()) as { user: { id: number; isAdmin: boolean; needsProviderSetup: boolean } }; + const body = (await res.json()) as { + user: { id: number; isAdmin: boolean; needsProviderSetup: boolean }; + }; expect(body.user).toHaveProperty('isAdmin'); expect(body.user.isAdmin).toBe(true); // DB returns true, not hardcoded }); @@ -194,13 +199,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () = let callCount = 0; vi.mocked(db.select).mockImplementation(() => { callCount++; - const limitFn = callCount === 1 - ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup - : vi.fn().mockResolvedValue([]); // no member_credentials row + const limitFn = + callCount === 1 + ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup + : vi.fn().mockResolvedValue([]); // no member_credentials row return { from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ limit: limitFn }), - innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), + innerJoin: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }), + }), }), // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; @@ -221,13 +229,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () = let callCount = 0; vi.mocked(db.select).mockImplementation(() => { callCount++; - const limitFn = callCount === 1 - ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup - : vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row + const limitFn = + callCount === 1 + ? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup + : vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row return { from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ limit: limitFn }), - innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }), + innerJoin: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }), + }), }), // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; diff --git a/apps/pwa/e2e/admin.spec.ts b/apps/pwa/e2e/admin.spec.ts index 3384a0b..f81bc7c 100644 --- a/apps/pwa/e2e/admin.spec.ts +++ b/apps/pwa/e2e/admin.spec.ts @@ -100,9 +100,7 @@ test.describe('Non-admin user — admin nav entry hidden + /admin redirect', () await page.waitForURL(/\/calendar/, { timeout: 10_000 }); // Should have landed on /calendar const url = new URL(page.url()); - expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch( - /^\/(calendar)?$/, - ); + expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch(/^\/(calendar)?$/); // "Admin Settings" heading must NOT be present await expect(page.getByRole('heading', { name: 'Admin Settings' })).toHaveCount(0); }); diff --git a/apps/pwa/src/components/CredentialSheet.tsx b/apps/pwa/src/components/CredentialSheet.tsx index 623535a..f2d0157 100644 --- a/apps/pwa/src/components/CredentialSheet.tsx +++ b/apps/pwa/src/components/CredentialSheet.tsx @@ -296,7 +296,9 @@ export function CredentialSheet({ id="credential-helper" style={{ fontSize: 'var(--text-label-size, 13px)', - color: validationError ? 'var(--color-destructive, #DC2626)' : 'var(--color-text-secondary)', + color: validationError + ? 'var(--color-destructive, #DC2626)' + : 'var(--color-text-secondary)', lineHeight: 1.4, marginBottom: 'var(--space-6, 24px)', display: 'flex', diff --git a/apps/pwa/src/components/SetupBanner.tsx b/apps/pwa/src/components/SetupBanner.tsx index fc88d67..a5e5d42 100644 --- a/apps/pwa/src/components/SetupBanner.tsx +++ b/apps/pwa/src/components/SetupBanner.tsx @@ -30,6 +30,7 @@ import { CredentialSheet } from './CredentialSheet.js'; export function SetupBanner() { const [sheetOpen, setSheetOpen] = useState(false); + // Use HTMLButtonElement for the ref (assignable to the CredentialSheet's HTMLElement trigger) const ctaRef = useRef(null); const meQuery = useQuery({ @@ -124,7 +125,7 @@ export function SetupBanner() { onClose={() => setSheetOpen(false)} mode="self-service" memberName={memberName} - triggerRef={ctaRef as React.RefObject} + triggerRef={ctaRef} /> ); diff --git a/apps/pwa/src/routes/AdminPage.tsx b/apps/pwa/src/routes/AdminPage.tsx index 379d1f7..df9e5c5 100644 --- a/apps/pwa/src/routes/AdminPage.tsx +++ b/apps/pwa/src/routes/AdminPage.tsx @@ -77,8 +77,7 @@ export function AdminPage() { }); // Derive current saved shared calendar id from the data - const currentSharedId = - calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null; + const currentSharedId = calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null; // Effective selected = user pick OR fallback to current saved const effectiveSelected = selectedCalendarId ?? currentSharedId; @@ -97,8 +96,7 @@ export function AdminPage() { // Open credential sheet for a member function openSheet(member: AdminMember, buttonRef: React.RefObject) { // Capture the button so focus can return on close - (triggerRef as React.MutableRefObject).current = - buttonRef.current; + (triggerRef as React.MutableRefObject).current = buttonRef.current; setSheetMember(member); setSheetMode(member.hasCredential ? 'admin-rotate' : 'admin-add'); setSheetOpen(true); @@ -298,7 +296,7 @@ export function AdminPage() { mode={sheetMode} memberName={sheetMember.displayName} memberId={sheetMember.id} - triggerRef={triggerRef as React.RefObject} + triggerRef={triggerRef} /> )}