From bfab8513648ce519714f943da4e982255c407904 Mon Sep 17 00:00:00 2001 From: Piotr Baranek Date: Thu, 7 May 2026 13:33:43 +0200 Subject: [PATCH 1/3] test(red): requireRole logging, junk-role defense, probe routes, login redirect - requireRole must log user_id, role, path on 403 - requireRole must reject junk roles even when present in allowedRoles - probe routes /api/_probe/doctor-only and /patient-only - e2e: logged-in user visiting /login redirected to dashboard Closes #3 Co-Authored-By: Claude Sonnet 4.6 --- apps/api/src/middleware/require-auth.test.ts | 83 +++++++++++++++++++ .../src/routes/_probe/probe-router.test.ts | 55 ++++++++++++ test/e2e/auth.spec.ts | 24 ++++++ 3 files changed, 162 insertions(+) create mode 100644 apps/api/src/middleware/require-auth.test.ts create mode 100644 apps/api/src/routes/_probe/probe-router.test.ts diff --git a/apps/api/src/middleware/require-auth.test.ts b/apps/api/src/middleware/require-auth.test.ts new file mode 100644 index 0000000..a68bb3b --- /dev/null +++ b/apps/api/src/middleware/require-auth.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect } from 'vitest'; +import { Hono } from 'hono'; +import { requireRole } from './require-auth'; +import type { AppVariables, UserRole } from '../domain/auth/types'; +import type { Logger } from '../lib/logger'; + +function makeLogger(): Logger & { warns: Array<[string, unknown?]> } { + const warns: Array<[string, unknown?]> = []; + return { + info: () => {}, + warn: (event: string, ctx?: unknown) => warns.push([event, ctx]), + error: () => {}, + debug: () => {}, + warns, + }; +} + +function makeApp( + contextRole: string, + contextUserId: string, + allowedRoles: unknown[], + logger: Logger, +) { + const app = new Hono<{ Variables: AppVariables }>(); + app.get('/test', async (c, next) => { + c.set('userId', contextUserId); + c.set('userRole', contextRole as UserRole); + await next(); + }); + app.get('/test', requireRole(allowedRoles as UserRole[], { logger }), (c) => + c.json({ ok: true }), + ); + return app; +} + +describe('requireRole middleware', () => { + it('allows a doctor when role is doctor', async () => { + const logger = makeLogger(); + const app = makeApp('doctor', 'uid-1', ['doctor'], logger); + const res = await app.request('/test'); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it('returns 403 when patient hits a doctor-only route', async () => { + const logger = makeLogger(); + const app = makeApp('patient', 'uid-2', ['doctor'], logger); + const res = await app.request('/test'); + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: 'Forbidden' }); + }); + + it('allows a patient when role is patient', async () => { + const logger = makeLogger(); + const app = makeApp('patient', 'uid-3', ['patient'], logger); + const res = await app.request('/test'); + expect(res.status).toBe(200); + }); + + it('returns 403 when doctor hits a patient-only route', async () => { + const logger = makeLogger(); + const app = makeApp('doctor', 'uid-4', ['patient'], logger); + const res = await app.request('/test'); + expect(res.status).toBe(403); + }); + + it('logs auth.rbac.denied with user_id, role, and path on 403', async () => { + const logger = makeLogger(); + const app = makeApp('patient', 'uid-5', ['doctor'], logger); + await app.request('/test'); + expect(logger.warns).toHaveLength(1); + const [event, ctx] = logger.warns[0]!; + expect(event).toBe('auth.rbac.denied'); + expect(ctx).toMatchObject({ user_id: 'uid-5', role: 'patient', path: '/test' }); + }); + + it('rejects a junk role even when it appears in allowedRoles (defense-in-depth)', async () => { + const logger = makeLogger(); + const app = makeApp('admin', 'uid-6', ['admin', 'doctor', 'patient'], logger); + const res = await app.request('/test'); + expect(res.status).toBe(403); + }); +}); diff --git a/apps/api/src/routes/_probe/probe-router.test.ts b/apps/api/src/routes/_probe/probe-router.test.ts new file mode 100644 index 0000000..b63d3f5 --- /dev/null +++ b/apps/api/src/routes/_probe/probe-router.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest'; +import { signJwt } from '../../lib/jwt'; +import { createProbeRouter } from './probe-router'; + +const SECRET = 'test-secret-that-is-at-least-32-chars-long'; + +function makeLogger() { + return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; +} + +function makeRouter() { + return createProbeRouter({ config: { JWT_SECRET: SECRET }, logger: makeLogger() }); +} + +function authHeader(role: 'doctor' | 'patient') { + return { Authorization: `Bearer ${signJwt({ sub: 'uid-probe', role }, SECRET)}` }; +} + +describe('GET /doctor-only probe route', () => { + it('returns 200 { ok: true } for a doctor', async () => { + const router = makeRouter(); + const res = await router.request('/doctor-only', { headers: authHeader('doctor') }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it('returns 403 for a patient', async () => { + const router = makeRouter(); + const res = await router.request('/doctor-only', { headers: authHeader('patient') }); + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: 'Forbidden' }); + }); + + it('returns 401 with no token', async () => { + const router = makeRouter(); + const res = await router.request('/doctor-only'); + expect(res.status).toBe(401); + }); +}); + +describe('GET /patient-only probe route', () => { + it('returns 200 { ok: true } for a patient', async () => { + const router = makeRouter(); + const res = await router.request('/patient-only', { headers: authHeader('patient') }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it('returns 403 for a doctor', async () => { + const router = makeRouter(); + const res = await router.request('/patient-only', { headers: authHeader('doctor') }); + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: 'Forbidden' }); + }); +}); diff --git a/test/e2e/auth.spec.ts b/test/e2e/auth.spec.ts index 7f4a6bf..3dbbc2c 100644 --- a/test/e2e/auth.spec.ts +++ b/test/e2e/auth.spec.ts @@ -95,4 +95,28 @@ test.describe('Authentication', () => { await page.waitForURL('**/patient', { timeout: 5_000 }); }); + + test('logged-in doctor visiting /login is redirected to /doctor', async ({ page }) => { + await page.goto('/login'); + await page.getByLabel('Email').fill(DOCTOR_EMAIL); + await page.getByLabel('Password').fill(DOCTOR_PASSWORD); + await page.getByRole('button', { name: /sign in/i }).click(); + await page.waitForURL('**/doctor'); + + await page.goto('/login'); + + await page.waitForURL('**/doctor', { timeout: 5_000 }); + }); + + test('logged-in patient visiting /login is redirected to /patient', async ({ page }) => { + await page.goto('/login'); + await page.getByLabel('Email').fill(PATIENT_EMAIL); + await page.getByLabel('Password').fill(PATIENT_PASSWORD); + await page.getByRole('button', { name: /sign in/i }).click(); + await page.waitForURL('**/patient'); + + await page.goto('/login'); + + await page.waitForURL('**/patient', { timeout: 5_000 }); + }); }); From 035822ccf087b11e30e6d6b2a001967ecf32b1b2 Mon Sep 17 00:00:00 2001 From: Piotr Baranek Date: Thu, 7 May 2026 13:35:08 +0200 Subject: [PATCH 2/3] feat(green): requireRole hardening, probe routes, /login redirect guard - requireRole logs user_id/role/path; rejects junk roles via VALID_ROLES set - GET /api/_probe/doctor-only and /patient-only wired through requireAuth+requireRole - probe contract entries added to apiContract - /login beforeLoad redirects authenticated users to their dashboard Closes #3 Co-Authored-By: Claude Sonnet 4.6 --- apps/api/src/app.ts | 2 ++ apps/api/src/middleware/require-auth.test.ts | 1 - apps/api/src/middleware/require-auth.ts | 12 ++++++-- .../src/routes/_probe/probe-router.test.ts | 2 +- apps/api/src/routes/_probe/probe-router.ts | 30 +++++++++++++++++++ apps/web/src/routes/login.tsx | 10 ++++++- packages/contracts/src/contract.ts | 2 ++ packages/contracts/src/probe.ts | 28 +++++++++++++++++ 8 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 apps/api/src/routes/_probe/probe-router.ts create mode 100644 packages/contracts/src/probe.ts diff --git a/apps/api/src/app.ts b/apps/api/src/app.ts index 03eba80..f243879 100644 --- a/apps/api/src/app.ts +++ b/apps/api/src/app.ts @@ -2,6 +2,7 @@ import { Hono } from 'hono'; import { cors } from 'hono/cors'; import { healthRouter } from './routes/health'; import { createAuthRouter } from './routes/auth/auth-router'; +import { createProbeRouter } from './routes/_probe/probe-router'; import type { Db } from './infrastructure/db'; import type { AppConfig } from './config'; import type { Logger } from './lib/logger'; @@ -18,6 +19,7 @@ export function createApp(deps?: AppDeps): Hono { app.route('/', healthRouter); if (deps) { app.route('/', createAuthRouter(deps)); + app.route('/api/_probe', createProbeRouter(deps)); } return app; } diff --git a/apps/api/src/middleware/require-auth.test.ts b/apps/api/src/middleware/require-auth.test.ts index a68bb3b..9196393 100644 --- a/apps/api/src/middleware/require-auth.test.ts +++ b/apps/api/src/middleware/require-auth.test.ts @@ -10,7 +10,6 @@ function makeLogger(): Logger & { warns: Array<[string, unknown?]> } { info: () => {}, warn: (event: string, ctx?: unknown) => warns.push([event, ctx]), error: () => {}, - debug: () => {}, warns, }; } diff --git a/apps/api/src/middleware/require-auth.ts b/apps/api/src/middleware/require-auth.ts index e0f93eb..f70c0fb 100644 --- a/apps/api/src/middleware/require-auth.ts +++ b/apps/api/src/middleware/require-auth.ts @@ -36,14 +36,20 @@ export function requireAuth(deps: { }; } +const VALID_ROLES = new Set(['doctor', 'patient']); + export function requireRole( roles: UserRole[], deps: { logger: Logger }, ): MiddlewareHandler { return async (c, next) => { - const role = c.get('userRole'); - if (!role || !roles.includes(role)) { - deps.logger.warn('auth.rbac.denied', { role, requiredRoles: roles }); + const role = c.get('userRole') as string | undefined; + if (!role || !VALID_ROLES.has(role) || !roles.includes(role as UserRole)) { + deps.logger.warn('auth.rbac.denied', { + user_id: c.get('userId'), + role, + path: c.req.path, + }); return c.json({ error: 'Forbidden' }, 403); } await next(); diff --git a/apps/api/src/routes/_probe/probe-router.test.ts b/apps/api/src/routes/_probe/probe-router.test.ts index b63d3f5..4d1933c 100644 --- a/apps/api/src/routes/_probe/probe-router.test.ts +++ b/apps/api/src/routes/_probe/probe-router.test.ts @@ -5,7 +5,7 @@ import { createProbeRouter } from './probe-router'; const SECRET = 'test-secret-that-is-at-least-32-chars-long'; function makeLogger() { - return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; + return { info: () => {}, warn: () => {}, error: () => {} }; } function makeRouter() { diff --git a/apps/api/src/routes/_probe/probe-router.ts b/apps/api/src/routes/_probe/probe-router.ts new file mode 100644 index 0000000..8b52270 --- /dev/null +++ b/apps/api/src/routes/_probe/probe-router.ts @@ -0,0 +1,30 @@ +import { Hono } from 'hono'; +import type { AppConfig } from '../../config'; +import type { Logger } from '../../lib/logger'; +import type { AppVariables } from '../../domain/auth/types'; +import { requireAuth, requireRole } from '../../middleware/require-auth'; + +export interface ProbeDeps { + config: Pick; + logger: Logger; +} + +export function createProbeRouter(deps: ProbeDeps): Hono<{ Variables: AppVariables }> { + const router = new Hono<{ Variables: AppVariables }>(); + + router.get( + '/doctor-only', + requireAuth(deps), + requireRole(['doctor'], deps), + (c) => c.json({ ok: true as const }), + ); + + router.get( + '/patient-only', + requireAuth(deps), + requireRole(['patient'], deps), + (c) => c.json({ ok: true as const }), + ); + + return router; +} diff --git a/apps/web/src/routes/login.tsx b/apps/web/src/routes/login.tsx index bf70db2..0cab3df 100644 --- a/apps/web/src/routes/login.tsx +++ b/apps/web/src/routes/login.tsx @@ -1,5 +1,5 @@ import { type FormEvent, useState } from 'react'; -import { createRoute, useNavigate } from '@tanstack/react-router'; +import { createRoute, useNavigate, redirect, isRedirect } from '@tanstack/react-router'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import { rootRoute } from './__root'; import { apiClient } from '../lib/api-client'; @@ -9,6 +9,14 @@ import { currentUserQueryOptions } from '../features/auth/queries'; export const loginRoute = createRoute({ getParentRoute: () => rootRoute, path: '/login', + beforeLoad: async ({ context: { queryClient } }) => { + try { + const user = await queryClient.ensureQueryData(currentUserQueryOptions); + throw redirect({ to: `/${user.role}` }); + } catch (e) { + if (isRedirect(e)) throw e; + } + }, component: LoginPage, }); diff --git a/packages/contracts/src/contract.ts b/packages/contracts/src/contract.ts index 726c8cc..76e0956 100644 --- a/packages/contracts/src/contract.ts +++ b/packages/contracts/src/contract.ts @@ -6,6 +6,7 @@ import { documentsContract } from './documents'; import { doctorContract } from './doctor'; import { appointmentsContract } from './appointments'; import { visitsContract } from './visits'; +import { probeContract } from './probe'; const c = initContract(); @@ -20,6 +21,7 @@ export const apiContract = c.router({ doctor: doctorContract, appointments: appointmentsContract, visits: visitsContract, + probe: probeContract, }); export type ApiContract = typeof apiContract; diff --git a/packages/contracts/src/probe.ts b/packages/contracts/src/probe.ts new file mode 100644 index 0000000..ac63474 --- /dev/null +++ b/packages/contracts/src/probe.ts @@ -0,0 +1,28 @@ +import { initContract } from '@ts-rest/core'; +import { z } from 'zod'; + +const c = initContract(); + +const probeOkSchema = z.object({ ok: z.literal(true) }); +const errorResponseSchema = z.object({ error: z.string() }); + +export const probeContract = c.router({ + doctorOnly: { + method: 'GET', + path: '/api/_probe/doctor-only', + responses: { + 200: probeOkSchema, + 401: errorResponseSchema, + 403: errorResponseSchema, + }, + }, + patientOnly: { + method: 'GET', + path: '/api/_probe/patient-only', + responses: { + 200: probeOkSchema, + 401: errorResponseSchema, + 403: errorResponseSchema, + }, + }, +}); From 6fd2a83ef4983b31ae435b791d14ab4744d3a468 Mon Sep 17 00:00:00 2001 From: Piotr Baranek Date: Thu, 7 May 2026 13:36:32 +0200 Subject: [PATCH 3/3] fix(green): load root .env in API entry point, fix ts-rest api-client URL and body serialisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - index.ts: load root .env before checkEnv() so JWT_SECRET is available when Playwright starts the API server via pnpm (no shell env inheritance) - api-client.ts: ts-rest's custom api() receives the full URL in `path` and a pre-JSON.stringify'd body; using `\${baseUrl}\${path}` doubled the URL and JSON.stringify(body) double-encoded the payload, causing every browser API call to fail (health → "API: down", login → 422) Co-Authored-By: Claude Sonnet 4.6 --- apps/api/src/index.ts | 15 +++++++++++++++ apps/web/src/lib/api-client.ts | 7 +++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 6b25df1..8dbe0ba 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -1,3 +1,6 @@ +import { readFileSync, existsSync } from 'node:fs'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { serve } from '@hono/node-server'; import { createApp } from './app'; import { loadConfig } from './config'; @@ -5,6 +8,18 @@ import { checkEnv } from './lib/env-guard'; import { createLogger } from './lib/logger'; import { createDb } from './infrastructure/db'; +const rootEnv = resolve(dirname(fileURLToPath(import.meta.url)), '../../..', '.env'); +if (existsSync(rootEnv)) { + for (const line of readFileSync(rootEnv, 'utf-8').split('\n')) { + if (!line || line.startsWith('#')) continue; + const eq = line.indexOf('='); + if (eq === -1) continue; + const key = line.slice(0, eq).trim(); + const val = line.slice(eq + 1).trim(); + if (key && !(key in process.env)) process.env[key] = val; + } +} + checkEnv(); const config = loadConfig(); const logger = createLogger(); diff --git a/apps/web/src/lib/api-client.ts b/apps/web/src/lib/api-client.ts index 5294532..dc1f670 100644 --- a/apps/web/src/lib/api-client.ts +++ b/apps/web/src/lib/api-client.ts @@ -9,16 +9,15 @@ export const apiClient = initClient(apiContract, { baseHeaders: {}, api: async ({ path, method, headers, body }) => { const token = getToken(); - const response = await fetch(`${baseUrl}${path}`, { + const response = await fetch(path, { method, headers: { - 'Content-Type': 'application/json', ...headers, ...(token ? { Authorization: `Bearer ${token}` } : {}), }, - body: body !== null && body !== undefined ? JSON.stringify(body) : undefined, + body: body ?? undefined, }); - const data = await response.json().catch(() => null) as unknown; + const data = (await response.json().catch(() => null)) as unknown; return { status: response.status, body: data, headers: response.headers }; }, });