diff --git a/docker-compose.e2e.yml b/docker-compose.e2e.yml index a03122c2..cfee6c22 100644 --- a/docker-compose.e2e.yml +++ b/docker-compose.e2e.yml @@ -46,7 +46,7 @@ services: # the in-network hostname and the host-port loopback. Without this # the CORS middleware (scoped to /api) rejects any non-GET request # because .env's APP_URL points at the production domain. - CORS_ORIGINS: "http://pediatric-ai-scribe-e2e:3000,http://host.docker.internal:3553,http://localhost:3553" + CORS_ORIGINS: "http://pediatric-ai-scribe-e2e:3000,http://host.docker.internal:3553,http://localhost:3553,http://127.0.0.1:3553" volumes: - scribe-logs-e2e:/app/data/logs depends_on: diff --git a/e2e/fixtures.js b/e2e/fixtures.js index 880632a5..5cec0802 100644 --- a/e2e/fixtures.js +++ b/e2e/fixtures.js @@ -13,12 +13,17 @@ const base = require('@playwright/test'); // ── Environment ────────────────────────────────────────────── -const E2E_BASE_INTERNAL = 'http://pediatric-ai-scribe-e2e:3000'; -const E2E_BASE_EXTERNAL = 'http://host.docker.internal:3553'; -const E2E_BASE = process.env.E2E_AUTH_BASE_URL || E2E_BASE_INTERNAL; +// Loopback, not the container hostname. Anything else is an insecure context, +// where crypto.randomUUID does not exist and the app cannot complete a sign-in +// — see the note in playwright.config.js. +const E2E_BASE = process.env.E2E_AUTH_BASE_URL || 'http://127.0.0.1:3553'; const TEST_EMAIL = process.env.E2E_TEST_EMAIL || 'e2e-user@ped-ai.test'; const TEST_PASSWORD = process.env.E2E_TEST_PASSWORD || 'E2E-testPassword123!'; +// Seeded with the admin role by e2e/seed.js. Kept as a separate account rather +// than promoting the ordinary user, so a test that asserts something is denied +// to a non-admin still has a non-admin to assert it with. +const ADMIN_EMAIL = process.env.E2E_ADMIN_EMAIL || 'e2e-admin@ped-ai.test'; const USE_REAL_AI = process.env.E2E_USE_REAL_AI === '1' || process.env.E2E_USE_REAL_AI === 'true'; @@ -39,25 +44,35 @@ function isAllowedConsoleNoise(text) { } // ── Auth — module-scoped token cache ──────────────────────── -// Keeps one login per worker to avoid the 10/15-min login rate-limiter. -let _tokenCache = null; -async function getAuthToken(request) { - if (_tokenCache) return _tokenCache; +// Keeps one login per account per worker to avoid the 10/15-min login +// rate-limiter. Keyed by email, because there is more than one account now and +// a single slot would have each login evicting the other's token. +const _tokenCache = new Map(); +async function tokenFor(request, email) { + if (_tokenCache.has(email)) return _tokenCache.get(email); const r = await request.post(E2E_BASE + '/api/auth/login', { - data: { email: TEST_EMAIL, password: TEST_PASSWORD }, + data: { email, password: TEST_PASSWORD }, }); if (!r.ok()) { const text = await r.text(); - throw new Error(`E2E login failed (status ${r.status()}): ${text}`); + // The overwhelmingly likely cause is an unseeded database, and saying so + // beats leaving someone to work back from a 401. + throw new Error( + `E2E login failed for ${email} (status ${r.status()}): ${text}\n` + + 'If the account does not exist, seed it: docker exec pediatric-ai-scribe-e2e node e2e/seed.js' + ); } const body = await r.json(); if (!body.token) throw new Error('Login response missing token: ' + JSON.stringify(body)); - _tokenCache = body.token; - return _tokenCache; + _tokenCache.set(email, body.token); + return body.token; } -async function loginAs(context, request) { - const token = await getAuthToken(request); +async function getAuthToken(request) { return tokenFor(request, TEST_EMAIL); } +async function getAdminToken(request) { return tokenFor(request, ADMIN_EMAIL); } + +async function loginAs(context, request, email = TEST_EMAIL) { + const token = await tokenFor(request, email); const url = new URL(E2E_BASE); await context.addCookies([{ name: 'ped_auth', @@ -149,6 +164,13 @@ const test = base.test.extend({ await loginAs(context, request); await use(page); }, + + // The same thing signed in as an administrator, for the screens an ordinary + // account cannot reach at all. + adminPage: async ({ page, context, request }, use) => { + await loginAs(context, request, ADMIN_EMAIL); + await use(page); + }, }); const expect = base.expect; @@ -159,8 +181,10 @@ module.exports = { E2E_BASE, TEST_EMAIL, TEST_PASSWORD, + ADMIN_EMAIL, loginAs, getAuthToken, + getAdminToken, mockAI, USE_REAL_AI, }; diff --git a/e2e/playwright.config.js b/e2e/playwright.config.js index fa2861dc..9fafb833 100644 --- a/e2e/playwright.config.js +++ b/e2e/playwright.config.js @@ -1,8 +1,23 @@ // Playwright config — runs smoke tests against the already-running PedScribe -// container (no dev server spin-up). Expects BASE_URL (default -// http://host.docker.internal:3552 when run via scripts/e2e.sh). +// container (no dev server spin-up). Expects BASE_URL, which scripts/e2e.sh +// supplies. const { defineConfig, devices } = require('@playwright/test'); +// BASE_URL must be a loopback origin (127.0.0.1 / localhost), not a container +// hostname. The app is a secure context in production and is written on that +// assumption: AccountBoundary mints a session generation with +// crypto.randomUUID() on every sign-in. Over plain http on a hostname that is +// not loopback the browser provides no crypto.randomUUID at all, so that call +// throws, the boot handler's catch swallows it, and every test lands on the +// login screen no matter how valid its session is — which is exactly what the +// whole browser suite was doing. +// +// Chrome's --unsafely-treat-insecure-origin-as-secure was tried first and does +// not work here: Playwright rejects the --user-data-dir it has to be paired +// with, and the flag alone leaves isSecureContext false. Loopback needs no +// flags, so scripts/e2e.sh runs the browser on the host network and reaches the +// app through its published port instead. + module.exports = defineConfig({ testDir: './tests', timeout: 30_000, @@ -12,7 +27,7 @@ module.exports = defineConfig({ workers: 1, reporter: [['list']], use: { - baseURL: process.env.BASE_URL || 'http://host.docker.internal:3552', + baseURL: process.env.BASE_URL || 'http://127.0.0.1:3553', trace: 'retain-on-failure', screenshot: 'only-on-failure', actionTimeout: 5_000, diff --git a/e2e/seed.js b/e2e/seed.js new file mode 100644 index 00000000..a3f5a980 --- /dev/null +++ b/e2e/seed.js @@ -0,0 +1,82 @@ +// ============================================================ +// E2E ACCOUNT SEED +// ============================================================ +// Run inside the app container, which is where the database credentials live: +// +// docker exec pediatric-ai-scribe-e2e node e2e/seed.js +// +// Before this existed the e2e user was a registration someone did by hand once +// and the shared Postgres happened to keep. That was enough to log in and no +// more: there was no admin account, so nothing under /api/admin could be tested +// through a real request at all, and the Search Sources screen had to be +// checked by reading its markup. +// +// Reconciles rather than only creating. An account left over from an earlier +// run with the wrong role, an unverified address, a disabled flag or a +// different password is repaired in place, so the suite cannot fail for a +// reason that has nothing to do with the code under test. +// +// The domain guard is the important part. This script updates passwords and +// grants the admin role, so it refuses to touch any address outside +// @ped-ai.test — a mistyped environment variable can then do nothing worse +// than create another test account. +// ============================================================ + +// The entrypoint fetches secrets from OpenBao and exports them into the server +// process, and nowhere else — not into the image config, not into an env file. +// `docker exec` therefore starts with none of them and the database connection +// refuses on localhost. Borrowing PID 1's environment is what makes this +// runnable the documented way; without it the script only works on a stack +// whose credentials happen to be in plain compose environment. +require('fs').readFileSync('/proc/1/environ', 'utf8').split('\0').forEach(function (pair) { + var i = pair.indexOf('='); + if (i > 0 && !process.env[pair.slice(0, i)]) process.env[pair.slice(0, i)] = pair.slice(i + 1); +}); + +var db = require('../src/db/database'); +var bcrypt = require('bcryptjs'); + +var TEST_DOMAIN = '@ped-ai.test'; +var PASSWORD = process.env.E2E_TEST_PASSWORD || 'E2E-testPassword123!'; + +var ACCOUNTS = [ + { email: process.env.E2E_TEST_EMAIL || 'e2e-user' + TEST_DOMAIN, name: 'E2E User', role: 'user' }, + { email: process.env.E2E_ADMIN_EMAIL || 'e2e-admin' + TEST_DOMAIN, name: 'E2E Admin', role: 'admin' } +]; + +async function seed(account) { + var email = String(account.email || '').toLowerCase().trim(); + if (email.slice(-TEST_DOMAIN.length) !== TEST_DOMAIN) { + throw new Error('refusing to seed ' + email + ': only ' + TEST_DOMAIN + ' addresses may be seeded'); + } + var hash = await bcrypt.hash(PASSWORD, 12); + var existing = await db.get('SELECT id, role, email_verified, disabled FROM users WHERE email = ?', [email]); + if (!existing) { + await db.run( + 'INSERT INTO users (email, password, name, role, email_verified, disabled) VALUES (?, ?, ?, ?, true, false)', + [email, hash, account.name, account.role] + ); + console.log('created ' + email + ' (' + account.role + ')'); + return; + } + await db.run( + 'UPDATE users SET password = ?, name = ?, role = ?, email_verified = true, disabled = false WHERE id = ?', + [hash, account.name, account.role, existing.id] + ); + var drift = []; + if (existing.role !== account.role) drift.push('role ' + existing.role + '→' + account.role); + if (!existing.email_verified) drift.push('verified'); + if (existing.disabled) drift.push('re-enabled'); + console.log('repaired ' + email + ' (' + (drift.length ? drift.join(', ') : 'password reset') + ')'); +} + +(async function () { + try { + for (var i = 0; i < ACCOUNTS.length; i++) await seed(ACCOUNTS[i]); + console.log('e2e accounts ready'); + process.exit(0); + } catch (err) { + console.error('e2e seed failed: ' + err.message); + process.exit(1); + } +})(); diff --git a/e2e/tests/admin-search-sources.spec.js b/e2e/tests/admin-search-sources.spec.js new file mode 100644 index 00000000..c53fea0e --- /dev/null +++ b/e2e/tests/admin-search-sources.spec.js @@ -0,0 +1,138 @@ +// ============================================================ +// ADMIN — SEARCH SOURCES +// ============================================================ +// The screen that decides whether a generated resource may search the web or +// PubMed, and holds the keys for both. +// +// This exists because that card could not previously be tested at all: there +// was no admin account to log in as, so it was checked by reading its markup +// and confirming the element ids matched the handlers. That verifies the wiring +// and nothing about whether an administrator can actually reach the screen, +// whether an ordinary user is kept off it, or whether a saved key survives a +// round trip. +// +// Two things are asserted that are easy to get wrong and expensive to get +// wrong: the routes are admin-only, and a key is never sent back to the browser +// in readable form. +// ============================================================ + +const { test, expect, E2E_BASE, TEST_EMAIL, ADMIN_EMAIL, loginAs, getAuthToken, getAdminToken } = require('../fixtures'); + +test.describe('Search Sources', () => { + let adminToken, userToken; + + test.beforeAll(async ({ request }) => { + adminToken = await getAdminToken(request); + userToken = await getAuthToken(request); + }); + + const auth = t => ({ Authorization: 'Bearer ' + t, 'Content-Type': 'application/json' }); + + test('an ordinary account cannot read or change search settings', async ({ request }) => { + const read = await request.get(E2E_BASE + '/api/admin/websearch', { headers: auth(userToken) }); + expect(read.status(), 'a non-admin must not read the settings').toBeGreaterThanOrEqual(400); + + const write = await request.put(E2E_BASE + '/api/admin/websearch', { + headers: auth(userToken), data: { enabled: 'true', provider: 'tavily' }, + }); + expect(write.status(), 'nor change them').toBeGreaterThanOrEqual(400); + }); + + test('an administrator reads the settings, and no key comes back readable', async ({ request }) => { + const r = await request.get(E2E_BASE + '/api/admin/websearch', { headers: auth(adminToken) }); + expect(r.ok(), await r.text()).toBeTruthy(); + const config = (await r.json()).config; + expect(config, 'settings come back under config').toBeTruthy(); + + // Both sources are represented, so the screen has something to render. + // Bracketed, not toHaveProperty: these key names contain dots, and a dotted + // string is read as a path into the object rather than as one key. + for (const key of ['websearch.enabled', 'websearch.provider', 'pubmed.enabled']) { + expect(Object.keys(config), key + ' is missing').toContain(key); + } + + // A key is either absent or masked. Anything else means a secret is being + // handed to the browser, which is the one failure here worth catching. + for (const key of ['websearch.api_key', 'pubmed.api_key']) { + const value = config[key]; + if (value) expect(value, key + ' must be masked').toMatch(/^•+/); + } + }); + + test('the Test button reports each source separately', async ({ request }) => { + const r = await request.post(E2E_BASE + '/api/admin/websearch/test', { + headers: auth(adminToken), data: { query: 'bronchiolitis high flow' }, + }); + expect(r.ok(), await r.text()).toBeTruthy(); + const body = await r.json(); + + // One press has to say which of the two works, so each reports either a + // count or a reason — never nothing at all. + for (const source of ['web', 'pubmed']) { + expect(body[source], source + ' must be reported').toBeTruthy(); + const reported = typeof body[source].count === 'number' || Boolean(body[source].reason); + expect(reported, source + ' reported neither a count nor a reason').toBeTruthy(); + } + }); + + // Admin is not a tab on the rail; it is an item in the account-card menu that + // is only created when the signed-in user has the admin role. So opening it + // and finding the menu item missing are the same assertion from both sides. + async function openAdmin(page) { + await page.locator('.account-card-btn').first().click(); + await page.locator('[data-account-tab="admin"]').first().click(); + await page.waitForFunction(() => { + const el = document.getElementById('admin-tab'); + return el && el.classList.contains('active') && el.innerHTML.trim().length > 100; + }, { timeout: 20000 }); + } + + // AccountBoundary allows one verified owner per document and freezes the page + // rather than letting a second account in, so each account is checked in its + // own browser context. Swapping the cookie inside one context is not a + // shortcut here — it is the thing the app deliberately refuses. + async function pageFor(browser, email) { + const context = await browser.newContext(); + await loginAs(context, context.request, email); + const page = await context.newPage(); + await page.goto(E2E_BASE + '/'); + await page.waitForSelector('button.tab-btn', { timeout: 20000 }); + // At phone width the rail — and the account card with it — is behind the + // menu toggle, the same way the other specs open it. + const vp = page.viewportSize(); + if (vp && vp.width <= 768) await page.click('#btn-menu-toggle').catch(() => {}); + return { page, context }; + } + + test('the account menu offers Admin to an administrator only', async ({ browser }) => { + const mine = await pageFor(browser, TEST_EMAIL); + await mine.page.locator('.account-card-btn').first().click(); + await expect(mine.page.locator('[data-account-tab="admin"]'), + 'an ordinary account is never offered Admin').toHaveCount(0); + await mine.context.close(); + + const theirs = await pageFor(browser, ADMIN_EMAIL); + await theirs.page.locator('.account-card-btn').first().click(); + await expect(theirs.page.locator('[data-account-tab="admin"]').first()).toBeVisible(); + await theirs.context.close(); + }); + + test('the card renders for an administrator, with both sources', async ({ browser }) => { + const { page: adminPage, context } = await pageFor(browser, ADMIN_EMAIL); + await openAdmin(adminPage); + await adminPage.waitForSelector('#ws-provider', { timeout: 20000 }); + + // Every control the save handler reads must exist, which is the failure the + // static id linter catches and this confirms in a real render. + for (const id of ['ws-enabled', 'ws-provider', 'ws-api-key', 'ws-base-url', + 'pm-enabled', 'pm-api-key', 'pm-email', 'ws-status']) { + await expect(adminPage.locator('#' + id), '#' + id + ' is missing').toHaveCount(1); + } + + // Anything typed into a key field must not be a readable input. + for (const id of ['ws-api-key', 'pm-api-key']) { + await expect(adminPage.locator('#' + id)).toHaveAttribute('type', 'password'); + } + await context.close(); + }); +}); diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 211440fd..8ed8f602 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -17,13 +17,26 @@ echo "==> Static reference lint" docker run --rm -v "$PWD:/work" -w /work node:20-alpine \ node scripts/lint-references.js -# Attach the Playwright container to the same Docker network as the app so it -# can resolve pediatric-ai-scribe by service name. Internal container port is 3000. -NETWORK="${E2E_NETWORK:-ped-ai_default}" -BASE_URL="${BASE_URL:-http://pediatric-ai-scribe:3000}" +# --- SEED: the accounts the fixtures log in as --- +# Idempotent, and the only place the admin account comes from. Run inside the +# app container because that is where the database credentials are: the +# entrypoint exports them from OpenBao into the Node process and nowhere else. +# Non-fatal, so a run against a stack that is already seeded is not blocked by +# a container that happens not to be up. +E2E_CONTAINER="${E2E_CONTAINER:-pediatric-ai-scribe-e2e}" +echo "==> Seeding e2e accounts" +if docker exec "$E2E_CONTAINER" node e2e/seed.js; then + : +else + echo " seed skipped ($E2E_CONTAINER not running or not seedable); tests will fail on login if the accounts are missing" >&2 +fi + +# Host network and a loopback URL, because the browser only treats loopback as +# a secure context over plain http, and the app cannot sign in without one. +BASE_URL="${BASE_URL:-http://127.0.0.1:3553}" docker run --rm --ipc=host \ - --network="$NETWORK" \ + --network=host \ -v "$PWD/e2e":/work \ -w /work \ -e BASE_URL="$BASE_URL" \ diff --git a/test/e2e-harness.test.js b/test/e2e-harness.test.js new file mode 100644 index 00000000..bedde3c6 --- /dev/null +++ b/test/e2e-harness.test.js @@ -0,0 +1,60 @@ +// ============================================================ +// E2E HARNESS +// ============================================================ +// The browser suite is the only thing that exercises the app the way a person +// uses it, so the two ways it silently stops doing that are worth pinning. +// +// Both were found the same day: every browser-driving spec was failing, and had +// been, because the harness could not sign in at all. +// ============================================================ + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const path = require('path'); +const read = p => fs.readFileSync(path.join(__dirname, '..', p), 'utf8'); + +test('the browser suite runs against a loopback origin, because the app needs a secure context', () => { + // AccountBoundary mints a session generation with crypto.randomUUID() on + // every sign-in. On a non-loopback http origin the browser provides no + // crypto.randomUUID at all, so that call throws, the boot handler's catch + // swallows it, and every test lands on the login screen with a valid session + // in hand. Measured: isSecureContext false and randomUUID undefined on + // http://pediatric-ai-scribe-e2e:3000, both true on http://127.0.0.1:3553. + assert.match(read('public/js/accountBoundary.js'), /crypto\.randomUUID\(\)/, + 'if this call is gone, the loopback requirement may have gone with it'); + + const config = read('e2e/playwright.config.js'); + const fixtures = read('e2e/fixtures.js'); + const runner = read('scripts/e2e.sh'); + // host.docker.internal is a hostname, not loopback, and was the old default + // in all three places. + for (const [name, src] of [['config', config], ['fixtures', fixtures], ['runner', runner]]) { + assert.ok(!/host\.docker\.internal/.test(src), name + ' must not default to a non-loopback origin'); + } + assert.match(config, /127\.0\.0\.1:3553/, 'playwright baseURL is loopback'); + assert.match(fixtures, /127\.0\.0\.1:3553/, 'the fixtures authenticate against loopback'); + assert.match(runner, /--network=host/, 'which needs the host network to reach the published port'); + assert.match(read('docker-compose.e2e.yml'), /http:\/\/127\.0\.0\.1:3553/, 'and CORS has to allow it'); +}); + +test('the e2e accounts are seeded, and the seed cannot touch a real one', () => { + const seed = read('e2e/seed.js'); + // Both roles. Without an admin account nothing under /api/admin could be + // tested through a real request, which is how the Search Sources screen came + // to be checked by reading its markup instead. + assert.match(seed, /role: 'user'/); + assert.match(seed, /role: 'admin'/); + // It resets passwords and grants the admin role, so the domain guard is the + // part that matters: a mistyped environment variable must not be able to + // reach a real account. + assert.match(seed, /refusing to seed/); + assert.match(seed, /TEST_DOMAIN = '@ped-ai\.test'/); + assert.match(seed, /email\.slice\(-TEST_DOMAIN\.length\) !== TEST_DOMAIN/); + // Reconciles rather than only creating, so a leftover account with the wrong + // role cannot fail the suite for a reason unrelated to the code. + assert.match(seed, /UPDATE users SET password = \?, name = \?, role = \?, email_verified = true, disabled = false/); + + // And the runner seeds before it tests, so nobody has to remember to. + assert.match(read('scripts/e2e.sh'), /node e2e\/seed\.js/); +});