From e27c1ca06c71fed6595b4dbbb4293533882bed3d Mon Sep 17 00:00:00 2001 From: Bert Hausmans Date: Wed, 20 May 2026 23:27:52 +0200 Subject: [PATCH] chore(auth): non-blocking follow-ups from final review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - /api/stats: add verifyCsrf middleware (defense-in-depth; no-op for GETs) - VerifyEmailPage: useRef guard to prevent React StrictMode double-fire of the single-use verify token in dev - router.tsx: route-level code splitting via React.lazy + Suspense; initial bundle drops from 397 KB to 224 KB with per-route chunks (0.3–14 KB each) - e2e: wait for verify-email completion before login; bump Account-menu timeout to handle Vite cold-chunk compile --- e2e/auth.spec.ts | 4 +- e2e/smoke.spec.ts | 8 +- packages/backend/src/app.ts | 2 +- .../frontend/src/pages/auth/VerifyEmail.tsx | 7 +- packages/frontend/src/router.tsx | 103 ++++++++++++------ 5 files changed, 80 insertions(+), 44 deletions(-) diff --git a/e2e/auth.spec.ts b/e2e/auth.spec.ts index adb595f..176fd16 100644 --- a/e2e/auth.spec.ts +++ b/e2e/auth.spec.ts @@ -26,12 +26,14 @@ test('admin invites user; user accepts and logs in', async ({ page }) => { await page.getByRole('button', { name: /Account aanmaken/ }).click(); await expect(page.getByText(/bevestigingsmail/i)).toBeVisible(); await page.goto(await fetchLink(adminEmail, 'verify-email')); + await expect(page.getByRole('link', { name: 'Naar inloggen' })).toBeVisible({ timeout: 10_000 }); await page.goto('/login'); await page.getByLabel(/E-mailadres/).fill(adminEmail); await page.getByLabel(/Wachtwoord/).fill(adminPw); await page.getByRole('button', { name: 'Inloggen' }).click(); - await expect(page.getByRole('button', { name: 'Account menu' })).toBeVisible(); + // Cold Vite chunks can take a few seconds to compile on the first run. + await expect(page.getByRole('button', { name: 'Account menu' })).toBeVisible({ timeout: 15_000 }); await page.goto('/admin/users'); const inviteeEmail = `invitee+${Date.now()}@example.com`; diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts index 13f22f3..5bc6e29 100644 --- a/e2e/smoke.spec.ts +++ b/e2e/smoke.spec.ts @@ -27,16 +27,14 @@ test('register → verify → login → create lesson → add card → practice const link = await fetchVerifyLink(email); await page.goto(link); - // Verify endpoint is called on mount; React StrictMode in dev triggers it twice - // (second call fails because token is already consumed). The DB is updated by - // the first call, so we can safely proceed regardless of UI state. - await expect(page.getByRole('heading', { name: 'E-mailverificatie' })).toBeVisible(); + // Wait for the verify POST to finish before logging in. + await expect(page.getByRole('link', { name: 'Naar inloggen' })).toBeVisible({ timeout: 10_000 }); await page.goto('/login'); await page.getByLabel(/E-mailadres/).fill(email); await page.getByLabel(/Wachtwoord/).fill(password); await page.getByRole('button', { name: 'Inloggen' }).click(); - await expect(page.getByRole('button', { name: 'Account menu' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Account menu' })).toBeVisible({ timeout: 15_000 }); await page.goto('/admin'); await page.getByPlaceholder(/Nieuwe wortel-les/).fill('E2E les'); diff --git a/packages/backend/src/app.ts b/packages/backend/src/app.ts index 733e79e..9e59c42 100644 --- a/packages/backend/src/app.ts +++ b/packages/backend/src/app.ts @@ -32,7 +32,7 @@ export function createApp(db: Db): Express { app.use('/api/lessons', requireAuth, verifyCsrf, lessonsRouter(db)); app.use('/api', requireAuth, verifyCsrf, cardsRouter(db)); app.use('/api/sessions', requireAuth, verifyCsrf, sessionsRouter(db)); - app.use('/api/stats', requireAuth, statsRouter(db)); + app.use('/api/stats', requireAuth, verifyCsrf, statsRouter(db)); app.use('/api/admin/users', requireAuth, requireRole('sysadmin'), verifyCsrf, adminUsersRouter(db)); // Static frontend in production diff --git a/packages/frontend/src/pages/auth/VerifyEmail.tsx b/packages/frontend/src/pages/auth/VerifyEmail.tsx index 86ff4ce..ef81402 100644 --- a/packages/frontend/src/pages/auth/VerifyEmail.tsx +++ b/packages/frontend/src/pages/auth/VerifyEmail.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { Link, useSearchParams } from 'react-router-dom'; import { authApi } from '../../api/auth.js'; import { ApiClientError } from '../../api/client.js'; @@ -9,9 +9,14 @@ export function VerifyEmailPage() { const token = params.get('token'); const [state, setState] = useState<'pending' | 'ok' | 'err'>('pending'); const [message, setMessage] = useState(''); + // Single-fire guard: React StrictMode double-invokes effects in dev, + // which would consume the single-use token twice and show a misleading error. + const fired = useRef(false); useEffect(() => { if (!token) { setState('err'); setMessage('Token ontbreekt.'); return; } + if (fired.current) return; + fired.current = true; (async () => { try { await authApi.verifyEmail({ token }); diff --git a/packages/frontend/src/router.tsx b/packages/frontend/src/router.tsx index ac78925..9340a2c 100644 --- a/packages/frontend/src/router.tsx +++ b/packages/frontend/src/router.tsx @@ -1,25 +1,56 @@ +import { lazy, Suspense, type ComponentType } from 'react'; import { createBrowserRouter, Navigate } from 'react-router-dom'; import { Layout } from './components/Layout.js'; import { AuthBoundary } from './components/AuthBoundary.js'; import { RoleGuard } from './components/RoleGuard.js'; -import { DashboardPage } from './pages/Dashboard.js'; -import { AdminPage } from './pages/Admin.js'; -import { AdminLessonPage } from './pages/AdminLesson.js'; -import { PracticeSetupPage } from './pages/PracticeSetup.js'; -import { PracticePage } from './pages/Practice.js'; -import { PracticeDonePage } from './pages/PracticeDone.js'; -import { StatsPage } from './pages/Stats.js'; -import { StatsLessonPage } from './pages/StatsLesson.js'; -import { StatsCardPage } from './pages/StatsCard.js'; -import { SettingsPage } from './pages/Settings.js'; -import { LoginPage } from './pages/auth/Login.js'; -import { RegisterPage } from './pages/auth/Register.js'; -import { VerifyEmailPage } from './pages/auth/VerifyEmail.js'; -import { ForgotPasswordPage } from './pages/auth/ForgotPassword.js'; -import { ResetPasswordPage } from './pages/auth/ResetPassword.js'; -import { AcceptInvitePage } from './pages/auth/AcceptInvite.js'; -import { ProfilePage } from './pages/Profile.js'; -import { AdminUsersPage } from './pages/AdminUsers.js'; + +// Tiny loading placeholder reused for every lazy route boundary. +function PageFallback() { + return ( +
+
+
+ ); +} + +// `React.lazy` requires a default export; our pages use named exports, so we +// adapt with a small helper that picks the named export from the module. +function lazyPage( + loader: () => Promise>, + name: K, +): ComponentType { + const Component = lazy(async () => { + const mod = await loader(); + return { default: mod[name] as ComponentType }; + }); + return function LazyPage() { + return ( + }> + + + ); + }; +} + +const Dashboard = lazyPage(() => import('./pages/Dashboard.js'), 'DashboardPage'); +const Admin = lazyPage(() => import('./pages/Admin.js'), 'AdminPage'); +const AdminLesson = lazyPage(() => import('./pages/AdminLesson.js'), 'AdminLessonPage'); +const PracticeSetup = lazyPage(() => import('./pages/PracticeSetup.js'), 'PracticeSetupPage'); +const Practice = lazyPage(() => import('./pages/Practice.js'), 'PracticePage'); +const PracticeDone = lazyPage(() => import('./pages/PracticeDone.js'), 'PracticeDonePage'); +const Stats = lazyPage(() => import('./pages/Stats.js'), 'StatsPage'); +const StatsLesson = lazyPage(() => import('./pages/StatsLesson.js'), 'StatsLessonPage'); +const StatsCard = lazyPage(() => import('./pages/StatsCard.js'), 'StatsCardPage'); +const Settings = lazyPage(() => import('./pages/Settings.js'), 'SettingsPage'); +const Profile = lazyPage(() => import('./pages/Profile.js'), 'ProfilePage'); +const AdminUsers = lazyPage(() => import('./pages/AdminUsers.js'), 'AdminUsersPage'); + +const Login = lazyPage(() => import('./pages/auth/Login.js'), 'LoginPage'); +const Register = lazyPage(() => import('./pages/auth/Register.js'), 'RegisterPage'); +const VerifyEmail = lazyPage(() => import('./pages/auth/VerifyEmail.js'), 'VerifyEmailPage'); +const ForgotPassword = lazyPage(() => import('./pages/auth/ForgotPassword.js'), 'ForgotPasswordPage'); +const ResetPassword = lazyPage(() => import('./pages/auth/ResetPassword.js'), 'ResetPasswordPage'); +const AcceptInvite = lazyPage(() => import('./pages/auth/AcceptInvite.js'), 'AcceptInvitePage'); export const router = createBrowserRouter([ { @@ -27,32 +58,32 @@ export const router = createBrowserRouter([ element: , children: [ // Public auth routes - { path: 'login', element: }, - { path: 'register', element: }, - { path: 'verify-email', element: }, - { path: 'forgot-password', element: }, - { path: 'reset-password', element: }, - { path: 'accept-invite', element: }, + { path: 'login', element: }, + { path: 'register', element: }, + { path: 'verify-email', element: }, + { path: 'forgot-password', element: }, + { path: 'reset-password', element: }, + { path: 'accept-invite', element: }, // Authenticated routes { element: , children: [ - { index: true, element: }, - { path: 'admin', element: }, - { path: 'admin/lessons/:id', element: }, - { path: 'practice/:lessonId/setup', element: }, - { path: 'practice/:lessonId', element: }, - { path: 'practice/:lessonId/done', element: }, - { path: 'stats', element: }, - { path: 'stats/lessons/:id', element: }, - { path: 'stats/cards/:id', element: }, - { path: 'settings', element: }, - { path: 'profile', element: }, + { index: true, element: }, + { path: 'admin', element: }, + { path: 'admin/lessons/:id', element: }, + { path: 'practice/:lessonId/setup', element: }, + { path: 'practice/:lessonId', element: }, + { path: 'practice/:lessonId/done', element: }, + { path: 'stats', element: }, + { path: 'stats/lessons/:id', element: }, + { path: 'stats/cards/:id', element: }, + { path: 'settings', element: }, + { path: 'profile', element: }, { element: , children: [ - { path: 'admin/users', element: }, + { path: 'admin/users', element: }, ], }, { path: '*', element: },