From e58aa1b9960c9adfc1471623feffcb763bc0140b Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 13 Sep 2026 06:11:00 +0200 Subject: [PATCH] refactor: sign-in codes and registration invitations leave; the SSO has both MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sign-in is email → code at sso.pedshub.com, and new accounts come from an invitation link minted there, so the app's own code emails and invite codes recorded a path nobody can take. Gone: the login-code routes and their rate limiters, the invite admin API and card, the invite field on the register form, the "email me a code / use my password" choice on the sign-in screen (an email now leads straight to the password), both utility modules, and the invite-only setting. A migration drops login_codes and registration_invites. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- docs/api-reference.md | 25 +- docs/authentication.md | 83 +------ docs/database.md | 37 --- e2e/seed.js | 6 +- e2e/tests/auth-screen.spec.js | 15 +- .../1781100000000_drop-sign-in-codes.js | 15 ++ public/components/admin.html | 36 +-- public/index.html | 25 -- public/js/admin.js | 197 +--------------- public/js/auth.js | 88 +------- public/js/authFetch.js | 6 +- server.js | 17 -- src/routes/adminConfig.js | 61 +---- src/routes/auth.js | 125 +--------- src/utils/adminLockdown.js | 1 - src/utils/loginCodes.js | 150 ------------ src/utils/openapiRoutes.js | 15 +- src/utils/registrationInvites.js | 213 ------------------ test/backend-hardening.test.js | 82 +------ test/crypto-context-binding.test.js | 9 - test/login-codes.test.js | 96 -------- 21 files changed, 55 insertions(+), 1247 deletions(-) create mode 100644 migrations/1781100000000_drop-sign-in-codes.js delete mode 100644 src/utils/loginCodes.js delete mode 100644 src/utils/registrationInvites.js delete mode 100644 test/login-codes.test.js diff --git a/docs/api-reference.md b/docs/api-reference.md index cc0664ab..35fc939e 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1574,7 +1574,7 @@ Get all application configuration settings. ### PUT /api/admin/config/:key -Update one application configuration setting. The key must match an allowed prefix: `announcement.`, `feature.`, `email.`, `prompt.`, `registration_enabled`, `registration_invite_only`, `site.`, `smtp.`, `models.`, `tts.`, `stt.`, `clinical_assistant.`, or `my_resources.`. Anything else is rejected with 400. +Update one application configuration setting. The key must match an allowed prefix: `announcement.`, `feature.`, `email.`, `prompt.`, `registration_enabled`, `site.`, `smtp.`, `models.`, `tts.`, `stt.`, `clinical_assistant.`, or `my_resources.`. Anything else is rejected with 400. Some keys are refused here even when allowed: `models.*` must go through the validated model endpoints, `feature.*` values must be `true` or `false`, and any key under lockdown returns 403. @@ -2050,18 +2050,6 @@ The signed-in user's own active sessions. `src/routes/sessions.js`. | `GET` | `/api/sessions` | | `DELETE` | `/api/sessions/:id` | -### Registration Invites (Admin) - -Invite codes for invite-only registration. `DELETE /invites/spent` clears codes that are used or expired-and-not-revoked; a live code is revoked first. `src/routes/adminConfig.js`. - -| Method | Path | -|---|---| -| `GET` | `/api/admin/invites` | -| `POST` | `/api/admin/invites` | -| `DELETE` | `/api/admin/invites/:id` | -| `POST` | `/api/admin/invites/:id/revoke` | -| `DELETE` | `/api/admin/invites/spent` | - ### Admin - Documentation Viewer Serves this `docs/` tree inside the Admin panel. `src/routes/adminDocs.js`. @@ -2092,12 +2080,9 @@ Per-workflow image generation settings. ### Authentication (additional) -`/api/auth/login-code/request` and `/api/auth/login-code/verify` are the -passwordless sign-in path: a 6-digit code is emailed, hashed at rest, valid -for 10 minutes, single use, 5 attempts. Password sign-in always remains -available, so a code that never arrives is never a lockout. The rest manage -2FA backup codes and password changes. `src/routes/auth.js`, -`src/utils/loginCodes.js`. +These manage 2FA backup codes and password changes for local accounts. +Sign-in codes are the SSO's (`sso.pedshub.com`), not the app's. +`src/routes/auth.js`. | Method | Path | |---|---| @@ -2105,8 +2090,6 @@ available, so a code that never arrives is never a lockout. The rest manage | `GET` | `/api/auth/2fa/backup-codes/count` | | `POST` | `/api/auth/change-password` | | `POST` | `/api/auth/check-password` | -| `POST` | `/api/auth/login-code/request` | -| `POST` | `/api/auth/login-code/verify` | Remaining endpoints not listed above are additional admin configuration, model/STT/TTS discovery and test calls, and the per-feature AI helpers (`/api/dont-miss`, `/api/suggest-codes`, `/api/generate-pe-narrative`, `/api/hospital-course-update`, `/api/hospital-course-clarify`, `/api/well-visit/note`, `/api/milestones-data`, `/api/user/features`, `/api/logs/client-error`, `/api/logs/client-event`, `/api/generated-images/:id`, `/api/image-jobs/:workflow`). diff --git a/docs/authentication.md b/docs/authentication.md index 1d908216..363d64e9 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -103,8 +103,6 @@ Providers tested: Authentik, Azure AD, Okta, Keycloak, Google, PocketID. |---|---| | `/api/*` general | 200 req / min / IP | | `/api/auth/login` | 10 / 15 min | -| `/api/auth/login-code/request` | 5 / hour | -| `/api/auth/login-code/verify` | 10 / 15 min | | `/api/auth/register` | 5 / hour | | `/api/auth/forgot-password` | 5 / hour | | `/api/auth/resend-verification` | 3 / 15 min | @@ -113,15 +111,6 @@ Providers tested: Authentik, Azure AD, Okta, Keycloak, Google, PocketID. Limits are per-IP (`express-rate-limit`). A clinic behind a single NAT shares the bucket; increase or switch to per-user keying if that becomes a problem. -Requesting a sign-in code is limited more tightly than attempting one, because -each request sends mail to somebody else's address — the cost of abuse lands on -the mailbox owner, not the caller. `LOGIN_CODE_RATE_LIMIT_MAX` overrides it. - -These are separate limiters rather than covered by the `/api/auth/login` one: -Express matches `app.use` paths on segment boundaries, so `/api/auth/login` does -**not** match `/api/auth/login-code/...`. A new sign-in endpoint needs its own -entry or it has no limit at all. - ## Login enumeration resistance `/api/auth/login` returns `"Invalid credentials"` for: @@ -132,74 +121,12 @@ entry or it has no limit at all. `"Email not verified"` is still returned for unverified accounts — deemed a necessary UX tradeoff over perfect indistinguishability. -## Sign-in codes +## Sign-in codes and invitations -A six-digit code emailed to the address being signed in with, offered beside the -password rather than instead of it. The screen asks for the email first, then -shows both routes: the code depends on mail being delivered and the password -does not, so neither is allowed to be the only way in. - -`POST /api/auth/login-code/request` → `POST /api/auth/login-code/verify`. - -What makes it a front door rather than a weaker side entrance: - -| | | -|---|---| -| Storage | bcrypt hash only, in `login_codes` — a code read out of the database is not a working credential | -| Lifetime | 10 minutes | -| Reuse | single use, marked used **before** the session is issued so a replay cannot race it | -| Supersession | requesting a new code deletes the previous one | -| Guessing | 5 wrong attempts burn the code; six digits is a million possibilities, which is plenty against a person and nothing against a script with unlimited tries at one code | -| Two-factor | still applies — a code proves you can read the mailbox, which is one factor, and an account that asked for a second still wants it | - -Generation uses rejection sampling on `crypto.randomBytes`, not modulo, which -would make low digits slightly likelier. - -`loginCodes.sweep()` clears codes more than a day past expiry. It is fire and -forget: housekeeping never fails a request. - -Verified end to end against the running server with SMTP configured: a code is -requested from the sign-in screen, the mail is sent, and typing the code returns -200 from `/login-code/verify` and enters the app. - -### Frontend note - -`public/js/authFetch.js` keeps an allowlist of `/api` paths callable with no -verified account owner, and rejects everything else **before it is sent**. A new -pre-auth endpoint must be added there or it fails as a "Connection error" with -no request ever leaving the browser. - -## Registration invitations - -`registration_invite_only` sits between open and closed registration: people may -register, but only with a code. It is subordinate to `registration_enabled` — with -registration disabled entirely, nobody can register, code or not. - -`registration_invites` holds codes that let someone register while -`registration_invite_only` is on. Only the hash is stored; the code is shown -once, at creation. - -Four states: `active`, `used`, `expired`, `revoked`. - -**Revoke** stops a live code and leaves the row, marked. **Delete** removes the -row, and is only permitted once the code can no longer be redeemed: - -```sql -(used_at IS NOT NULL OR (revoked_at IS NULL AND expires_at <= NOW())) -``` - -Deleting a code that could still be redeemed takes it off the list without -taking it out of anybody's inbox: the holder keeps something that looks valid, -it quietly stops working, and nothing is left to say who had it. Revoked rows -are kept because revoking records a decision somebody took. - -The condition is written to match the status the admin list displays. The -simpler `used OR expires_at <= NOW()` would also catch a revoked code whose date -had since passed — a row the screen still labels revoked and offers no delete -on, so button and query would disagree about the same row. - -`DELETE /api/admin/invites/spent` clears them in bulk under the same rule. It is -declared **before** `/invites/:id` or Express reads `spent` as an id. +Both live at the SSO (`sso.pedshub.com`, Authentik) rather than in this app: +sign-in is email → code, and new accounts come from an invitation link minted +with `authentik-pedshub/invite.py`. The app's own sign-in codes and +registration invites were removed once sign-in became SSO-only. ## Turnstile (Cloudflare bot protection) diff --git a/docs/database.md b/docs/database.md index 1e1b973f..4246dc13 100644 --- a/docs/database.md +++ b/docs/database.md @@ -306,43 +306,6 @@ Saved diagrams: the Mermaid source plus the user's own notes. | title, source, notes | TEXT NOT NULL | | | created_at, updated_at | TIMESTAMPTZ NOT NULL | | -### `login_codes` - -Passwordless sign-in codes. Only the bcrypt hash is stored, so a code read out -of the database is not a working credential. Ten-minute TTL, single use -(`used_at`), five attempts (`attempts`). See -[`authentication.md`](authentication.md). - -| Column | Type | Notes | -|---|---|---| -| id | SERIAL PK | | -| user_id | INTEGER NOT NULL | | -| code_hash | TEXT NOT NULL | bcrypt | -| attempts | INTEGER NOT NULL | Refused at 5 | -| expires_at | TIMESTAMPTZ NOT NULL | 10 minutes | -| used_at | TIMESTAMPTZ | Set once; a used code never verifies again | -| created_at | TIMESTAMPTZ NOT NULL | | - -### `registration_invites` - -Invite codes for invite-only registration. Only the hash is stored; `code_hint` -is the fragment shown in the admin list so a code can be recognised without -being recoverable. - -A code is *spent* when `used_at IS NOT NULL`, or when it has expired and was not -revoked. `DELETE /api/admin/invites/spent` deletes exactly that set — a live -code has to be revoked before it can be removed, so no code disappears while it -could still be redeemed. - -| Column | Type | Notes | -|---|---|---| -| id | SERIAL PK | | -| code_hash | TEXT NOT NULL | | -| code_hint | TEXT NOT NULL | Display fragment only | -| note | TEXT NOT NULL | Why it was issued | -| created_by, used_by, revoked_by | INTEGER | User ids | -| created_at, expires_at | TIMESTAMPTZ NOT NULL | | -| used_at, revoked_at | TIMESTAMPTZ | | ### `user_phone_extensions` diff --git a/e2e/seed.js b/e2e/seed.js index acbbc5cb..9361ceab 100644 --- a/e2e/seed.js +++ b/e2e/seed.js @@ -70,10 +70,8 @@ var SETTINGS = { 'stt.model': 'e2e-stt', 'tts.model': 'e2e-tts', 'tts.voice': 'e2e-voice', - // Registration closed and invite-only, which is what production runs and - // what the auth-screen spec asserts the sign-in page reflects. - 'registration_enabled': 'true', - 'registration_invite_only': 'true' + // Registration open, so the auth-screen spec can see the register link. + 'registration_enabled': 'true' }; async function seedSettings() { diff --git a/e2e/tests/auth-screen.spec.js b/e2e/tests/auth-screen.spec.js index 90cda26d..dc395082 100644 --- a/e2e/tests/auth-screen.spec.js +++ b/e2e/tests/auth-screen.spec.js @@ -22,41 +22,34 @@ test.describe('Unauthenticated auth screen', () => { await expect(page.locator('#btn-login-continue')).toBeVisible(); // Later steps are present but not yet offered. await expect(page.locator('#login-password')).toBeHidden(); - await expect(page.locator('#login-code')).toBeHidden(); await expect(page.locator('#btn-local-login')).toBeHidden(); // main app body must be hidden while unauthenticated await expect(page.locator('#main-app')).toBeHidden(); }); - test('an email leads to the choice, and choosing a password reveals it', async ({ page }) => { + test('an email leads straight to the password', async ({ page }) => { await page.goto(E2E_BASE + '/'); await page.waitForSelector('#auth-screen', { timeout: 10000 }); await page.fill('#login-email', 'someone@ped-ai.test'); await page.click('#btn-login-continue'); - await expect(page.locator('#login-choice')).toBeVisible(); // The address is fixed once the flow has moved past it; "use a different // email" is how you go back, and it only appears after the first step. await expect(page.locator('#login-email')).toHaveJSProperty('readOnly', true); await expect(page.locator('#login-change-email')).toBeVisible(); - - await page.click('#btn-login-use-password'); await expect(page.locator('#login-password')).toBeVisible(); await expect(page.locator('#btn-local-login')).toBeVisible(); }); test('the register link follows the registration setting', async ({ page }) => { - // Hidden by default and shown only when registration is enabled. The seed - // sets registration_enabled true and invite_only true, which is what - // production runs: registration is open, but a code is required — so the - // link shows and the invite field is required. + // Hidden by default and shown only when registration is enabled, which the + // seed turns on. No invitation field: invitations are the SSO's. await page.goto(E2E_BASE + '/'); await page.waitForSelector('#auth-screen', { timeout: 10000 }); await expect(page.locator('#show-register')).toBeVisible(); await expect(page.locator('#register-form')).toHaveCount(1); await page.click('#show-register'); - await expect(page.locator('#reg-invite')).toBeVisible(); - await expect(page.locator('#reg-invite')).toHaveJSProperty('required', true); + await expect(page.locator('#reg-invite')).toHaveCount(0); }); test('register form DOM is wired correctly if manually unhidden', async ({ page }) => { diff --git a/migrations/1781100000000_drop-sign-in-codes.js b/migrations/1781100000000_drop-sign-in-codes.js new file mode 100644 index 00000000..70e98422 --- /dev/null +++ b/migrations/1781100000000_drop-sign-in-codes.js @@ -0,0 +1,15 @@ +// Sign-in codes and registration invitations are gone. Sign-in and sign-up +// happen at the SSO (sso.pedshub.com), which emails its own codes and issues +// its own invitation links, so these two tables recorded a path nobody can +// take any more. The invite-only switch goes with them. + +exports.up = pgm => pgm.sql(` + DROP TABLE IF EXISTS login_codes; + DROP TABLE IF EXISTS registration_invites; + DELETE FROM app_settings WHERE key = 'registration_invite_only'; +`); + +// The tables can be recreated by the migrations that introduced them; the +// codes they held were single-use and short-lived, so there is nothing to +// restore. +exports.down = () => {}; diff --git a/public/components/admin.html b/public/components/admin.html index bb2ab01e..c73cc848 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -19,11 +19,9 @@

Accounts

- +

Registration
@@ -32,34 +30,6 @@
-
- Invite only -
- -
-
- -
- -
- - - -
-
-

Codes stay readable here so you can copy one again — an invitation usually has to be given to somebody later than the moment it was made. A code that has been used, revoked or expired can be deleted.

-
- -
- -
- -
diff --git a/public/index.html b/public/index.html index c3eea3a2..5eea6cc9 100644 --- a/public/index.html +++ b/public/index.html @@ -70,28 +70,8 @@ - - - - - - -
diff --git a/public/js/admin.js b/public/js/admin.js index 9191d75a..79eeaaae 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -366,6 +366,8 @@ function adminTabActive() { if (!data.success) return; var cfg = {}; (data.config || []).forEach(function(row) { cfg[row.key] = row.value; }); + // Painted after the panel has rendered, from the same response. + if (typeof window.applyAdminLockdown === 'function') window.applyAdminLockdown(data.lockdown); // Announcement var annEnabled = document.getElementById('cms-ann-enabled'); @@ -1891,7 +1893,7 @@ initImageSettings(); // filling in a field that was never going to save. Settings stay visible, so // the configuration can still be read. // -// The state rides along on the invites response rather than a request of its +// The state rides along on the config response rather than a request of its // own: the admin panel already makes enough calls on open. // ============================================================ { @@ -1916,7 +1918,7 @@ initImageSettings(); // Everything inside the panel, except the controls that stay operational and // the buttons that only read. function lockdownFields(panel, state) { - var editable = ['admin-invite', 'btn-create-invite', 'cms-ann', 'announcement', + var editable = ['cms-ann', 'announcement', 'admin-users-search', 'registration']; panel.querySelectorAll('input, select, textarea, button').forEach(function(el) { var id = el.id || ''; @@ -1929,197 +1931,6 @@ initImageSettings(); } } -// ============================================================ -// ADMIN REGISTRATION INVITES -// The code exists in readable form exactly once: in the response to creating -// it. Everything after works from the id and the last four characters. -// ============================================================ -{ - document.addEventListener('tabChanged', function(e) { - if (e.detail && e.detail.tab === 'admin') loadInvites(); - }); - if (adminTabActive()) loadInvites(); - - document.addEventListener('click', function(e) { - if (e.target.closest('#btn-create-invite')) createInvite(); - var revoke = e.target.closest('.admin-invite-revoke'); - if (revoke) inviteAction(revoke.dataset.id, 'revoke'); - var del = e.target.closest('.admin-invite-delete'); - if (del) inviteAction(del.dataset.id, 'delete'); - if (e.target.closest('#btn-clear-used-invites')) clearUsedInvites(); - var copy = e.target.closest('.admin-invite-copy'); - if (copy && navigator.clipboard) { - navigator.clipboard.writeText(copy.dataset.code) - .then(function() { showToast('Invitation code copied', 'success'); }) - .catch(function() { showToast('Could not copy; select it by hand', 'error'); }); - } - }); - document.addEventListener('change', function(e) { - if (e.target.id === 'admin-invite-only') setInviteOnly(e.target.checked); - }); - - const esc = adminEscapeHtml; - - function loadInvites() { - fetch('/api/admin/invites', { headers: getAuthHeaders() }) - .then(function(r) { return r.json(); }) - .then(function(data) { - if (!data.success) return; - var toggle = document.getElementById('admin-invite-only'); - if (toggle) toggle.checked = !!data.inviteOnly; - renderInvites(data.invites || []); - // Painted after the panel has rendered, from the same response. - if (typeof window.applyAdminLockdown === 'function') window.applyAdminLockdown(data.lockdown); - }) - .catch(function() {}); - } - - function setInviteOnly(on) { - fetch('/api/admin/config/' + encodeURIComponent('registration_invite_only'), { - method: 'PUT', headers: getAuthHeaders(), body: JSON.stringify({ value: on ? 'true' : 'false' }) - }) - .then(function(r) { return r.json(); }) - .then(function(data) { - if (!data.success) throw new Error(data.error || 'Could not save'); - showToast(on ? 'Registration now requires an invitation' : 'Registration no longer requires an invitation', 'success'); - }) - .catch(function(err) { showToast(err.message, 'error'); loadInvites(); }); - } - - function createInvite() { - var note = (document.getElementById('admin-invite-note') || {}).value || ''; - var days = (document.getElementById('admin-invite-days') || {}).value || '7'; - var out = document.getElementById('admin-invite-new'); - fetch('/api/admin/invites', { - method: 'POST', headers: getAuthHeaders(), body: JSON.stringify({ note: note, days: days }) - }) - .then(function(r) { return r.json(); }) - .then(function(data) { - if (!data.success) throw new Error(data.error || 'Could not create'); - if (out) { - out.innerHTML = '
' + - '' + esc(data.code) + '' + - '' + - 'Valid ' + esc(String(data.days)) + ' days. This is the only time it is shown.' + - '
'; - } - var noteEl = document.getElementById('admin-invite-note'); - if (noteEl) noteEl.value = ''; - loadInvites(); - }) - .catch(function(err) { showToast(err.message, 'error'); }); - } - - function inviteAction(id, action) { - var request = action === 'delete' - ? fetch('/api/admin/invites/' + encodeURIComponent(id), { method: 'DELETE', headers: getAuthHeaders() }) - : fetch('/api/admin/invites/' + encodeURIComponent(id) + '/revoke', { method: 'POST', headers: getAuthHeaders() }); - request.then(function(r) { return r.json(); }) - .then(function(data) { - if (!data.success) throw new Error(data.error || 'Failed'); - showToast(action === 'delete' ? 'Invitation deleted' : 'Invitation revoked', 'info'); - loadInvites(); - }) - .catch(function(err) { showToast(err.message, 'error'); }); - } - - // Used, or expired without being redeemed. A revoked code keeps its row: it - // records a decision somebody took, and it is not cluttering anything the way - // a pile of expired codes does. - // Spent: it can no longer be redeemed, whatever ended it. Revoked was held - // back at first — revoke stops a code and leaves the row — but a revoked code - // is already dead, so keeping it only fills the list. - var SPENT_STATUS = ['used', 'expired', 'revoked']; - - // Spent invitations in one go, which is what a cluttered list actually wants. - // Confirmed first: it is a delete, even if everything it removes is finished. - function clearUsedInvites() { - showConfirm('Delete every used, revoked and expired invitation? Ones that can still be redeemed are kept.', function() { - fetch('/api/admin/invites/spent', { method: 'DELETE', headers: getAuthHeaders() }) - .then(function(r) { return r.json(); }) - .then(function(data) { - if (!data.success) throw new Error(data.error || 'Could not clear them'); - showToast('Removed ' + data.removed + ' spent invitation' + (data.removed === 1 ? '' : 's'), 'success'); - loadInvites(); - }) - .catch(function(err) { showToast(err.message, 'error'); }); - }, { danger: true, confirmText: 'Delete' }); - } - - function renderInvites(rows) { - var container = document.getElementById('admin-invites-list'); - if (!container) return; - if (!rows.length) { - container.innerHTML = '

No invitations yet.

'; - return; - } - var colours = { active: 'var(--green)', used: 'var(--g400)', expired: 'var(--amber)', revoked: 'var(--red)' }; - container.innerHTML = rows.map(function(row) { - var when = row.status === 'used' ? 'used ' + new Date(row.used_at).toLocaleDateString() - : row.status === 'revoked' ? 'revoked' - : 'expires ' + new Date(row.expires_at).toLocaleDateString(); - var who = row.used_by_email ? ' by ' + esc(row.used_by_email) : ''; - return '
' + - '' + esc(row.status) + '' + - // The code itself when it is still recoverable, with a button to copy - // it: an invitation has to be given to somebody, usually later than the - // moment it was made. A row from before codes were kept shows the four - // characters it has. - (row.code - ? '' + esc(row.code) + '' + - '' - : '****-' + esc(row.code_hint) + '') + - '' + esc(row.note || '') + '' + - '' + esc(when) + who + '' + - (row.status === 'active' ? '' : '') + - // Delete is offered on spent codes only — used, or expired unredeemed. - // One that could still be redeemed may be sitting in somebody's inbox: - // taking it off this list would not take it out of their hands, and - // nothing would then say who held it. Revoking is what stops a live - // code, and it leaves the row behind, marked. - (SPENT_STATUS.indexOf(row.status) !== -1 - ? '' - : '') + - '
'; - }).join(''); - - container.querySelectorAll('.admin-invite-copy').forEach(function(btn) { - btn.addEventListener('click', function() { - var code = btn.dataset.code || ''; - // The async clipboard API needs a secure context and permission; the - // textarea fallback is what works everywhere else, including plain http - // on a local network. - var done = function() { - var icon = btn.querySelector('i'); - if (icon) { icon.className = 'fas fa-check'; setTimeout(function() { icon.className = 'fas fa-copy'; }, 1200); } - showToast('Code copied', 'success'); - }; - if (navigator.clipboard && window.isSecureContext) { - navigator.clipboard.writeText(code).then(done).catch(fallback); - } else { fallback(); } - function fallback() { - var box = document.createElement('textarea'); - box.value = code; - box.setAttribute('readonly', ''); - box.style.cssText = 'position:fixed;top:-1000px;opacity:0;'; - document.body.appendChild(box); - box.select(); - try { document.execCommand('copy'); done(); } - catch (e) { showToast('Could not copy — select the code and copy it', 'error'); } - box.remove(); - } - }); - }); - - var spent = rows.filter(function(row) { return SPENT_STATUS.indexOf(row.status) !== -1; }).length; - var clear = document.getElementById('btn-clear-used-invites'); - if (clear) { - clear.hidden = spent === 0; - clear.textContent = 'Clear ' + spent + ' spent'; - } - } -} - // ============================================================ // ADMIN IMAGE MODEL MANAGEMENT // ============================================================ diff --git a/public/js/auth.js b/public/js/auth.js index f6b9a470..ad68f40c 100644 --- a/public/js/auth.js +++ b/public/js/auth.js @@ -271,7 +271,7 @@ document.addEventListener('DOMContentLoaded', function() { if (data.disableLocalAuth) { // Hide local login form fields, only show SSO var localFields = document.querySelectorAll('#login-form .form-group, #btn-local-login, ' + - '#btn-login-continue, #login-choice, #login-change-email, #show-register, #show-forgot'); + '#btn-login-continue, #login-change-email, #show-register, #show-forgot'); localFields.forEach(function(el) { el.style.display = 'none'; }); if (ssoDivider) ssoDivider.style.display = 'none'; } @@ -741,20 +741,16 @@ document.addEventListener('DOMContentLoaded', function() { // element stayed hidden forever. Measured on the "use a different email" // link, which never reappeared. if (on) el.classList.remove('hidden'); else el.classList.add('hidden'); - el.style.display = on ? (id === 'login-choice' ? 'flex' : '') : 'none'; + el.style.display = on ? '' : 'none'; } function loginStep(step) { reveal('btn-login-continue', step === 'email'); - reveal('login-choice', step === 'choice'); - reveal('login-code-group', step === 'code'); reveal('login-password-group', step === 'password'); - reveal('btn-local-login', step === 'code' || step === 'password'); + reveal('btn-local-login', step === 'password'); reveal('login-change-email', step !== 'email'); var email = document.getElementById('login-email'); if (email) email.readOnly = step !== 'email'; - var submit = document.getElementById('btn-local-login'); - if (submit) submit.textContent = step === 'code' ? 'Sign in with code' : 'Sign In'; loginMode = step; } var loginMode = 'email'; @@ -763,13 +759,15 @@ document.addEventListener('DOMContentLoaded', function() { if (continueBtn) continueBtn.addEventListener('click', function () { var email = (document.getElementById('login-email') || {}).value || ''; if (!email.trim() || email.indexOf('@') === -1) { showToast('Enter your email', 'error'); return; } - loginStep('choice'); + loginStep('password'); + var pw = document.getElementById('login-password'); + if (pw) pw.focus(); }); var changeEmail = document.getElementById('login-change-email'); if (changeEmail) changeEmail.addEventListener('click', function (e) { e.preventDefault(); - ['login-code', 'login-password', 'login-totp'].forEach(function (id) { + ['login-password', 'login-totp'].forEach(function (id) { var el = document.getElementById(id); if (el) el.value = ''; }); reveal('totp-group', false); @@ -778,66 +776,7 @@ document.addEventListener('DOMContentLoaded', function() { if (email) email.focus(); }); - var usePassword = document.getElementById('btn-login-use-password'); - if (usePassword) usePassword.addEventListener('click', function () { - loginStep('password'); - var pw = document.getElementById('login-password'); - if (pw) pw.focus(); - }); - - var sendCode = document.getElementById('btn-login-send-code'); - if (sendCode) sendCode.addEventListener('click', function () { - var email = ((document.getElementById('login-email') || {}).value || '').trim(); - if (!email) { showToast('Enter your email', 'error'); return; } - sendCode.disabled = true; - fetch('/api/auth/login-code/request', { - method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ email: email }) - }) - .then(function (r) { return r.json(); }) - .then(function (data) { - // The server answers the same way whether or not the account exists, so - // this says what was done, not what was found. - loginStep('code'); - var hint = document.getElementById('login-code-hint'); - if (hint) { - hint.textContent = data && data.error - ? data.error - : 'If ' + email + ' has an account, a code is on its way. It expires in 10 minutes. ' + - 'No code? Use your password instead.'; - } - var box = document.getElementById('login-code'); - if (box) box.focus(); - }) - .catch(function () { showToast('Connection error', 'error'); }) - .finally(function () { sendCode.disabled = false; }); - }); - // ---- LOGIN FORM SUBMIT ---- - function submitCode() { - var email = document.getElementById('login-email').value.trim(); - var code = (document.getElementById('login-code') || {}).value || ''; - var totpEl = document.getElementById('login-totp'); - var totpCode = totpEl ? totpEl.value.trim() : ''; - if (!code.trim()) { showToast('Enter the code from your email', 'error'); return false; } - - var body = { email: email, code: code }; - if (totpCode) body.totpCode = totpCode; - var attempt = authState(); - return fetch('/api/auth/login-code/verify', { - method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body) - }) - .then(function (r) { return r.json(); }) - .then(function (data) { return handleSignIn(data, attempt); }) - .catch(function (err) { - if (boundary.blocked() || err.name === 'AbortError') return; - showToast('Connection error', 'error'); - }); - } - - // Shared by both routes in, so a code sign-in gets the same 2FA prompt, the - // same verification notice and the same welcome as a password sign-in. function handleSignIn(data, attempt) { if (boundary.blocked() && !(data.success && data.token && data.user)) return; if (data.requires2FA) { @@ -935,7 +874,7 @@ document.addEventListener('DOMContentLoaded', function() { if (loginForm) loginForm.addEventListener('submit', function(e) { e.preventDefault(); e.stopPropagation(); - authenticate(loginMode === 'code' ? submitCode : submitLogin, 'Signing in...'); + authenticate(submitLogin, 'Signing in...'); }); loginStep('email'); @@ -1002,10 +941,7 @@ document.addEventListener('DOMContentLoaded', function() { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ - name: name, email: email, password: password, turnstileToken: regToken, - // Sent whether or not the field is showing; the server decides - // whether it is required. - inviteCode: (document.getElementById('reg-invite') || {}).value || '' + name: name, email: email, password: password, turnstileToken: regToken }) }); var attempt = authState(); @@ -1317,12 +1253,6 @@ document.addEventListener('DOMContentLoaded', function() { var link = document.getElementById('show-register'); if (link) link.style.display = ''; } - // Only ask for a code when one is actually required, so ordinary - // registration is not cluttered by a field nobody can fill in. - var inviteGroup = document.getElementById('reg-invite-group'); - var inviteInput = document.getElementById('reg-invite'); - if (inviteGroup) inviteGroup.classList.toggle('hidden', !data.inviteOnly); - if (inviteInput) inviteInput.required = !!data.inviteOnly; }) .catch(function() {}); diff --git a/public/js/authFetch.js b/public/js/authFetch.js index 17792ff5..fed89498 100644 --- a/public/js/authFetch.js +++ b/public/js/authFetch.js @@ -9,7 +9,6 @@ if (!window.__fetchAuthIntercepted) { // "Connection error" with no request ever leaving the browser. var authPaths = new Set([ '/api/auth/login', '/api/auth/register', '/api/auth/logout', - '/api/auth/login-code/request', '/api/auth/login-code/verify', '/api/auth/forgot-password', '/api/auth/reset-password', '/api/auth/verify-email', '/api/auth/resend-verification', '/api/auth/oidc', '/api/auth/oidc-status', '/api/auth/registration-status' @@ -44,10 +43,7 @@ if (!window.__fetchAuthIntercepted) { if (!ticket && !authPaths.has(url.pathname) && url.pathname !== '/api/auth/me' && url.pathname !== '/api/models') { return Promise.reject(boundary.error()); } - // Signing in with a code establishes a session exactly as a password does, - // so the boundary has to be prepared for a new owner the same way. - var login = (url.pathname === '/api/auth/login' || url.pathname === '/api/auth/register' - || url.pathname === '/api/auth/login-code/verify') + var login = (url.pathname === '/api/auth/login' || url.pathname === '/api/auth/register') && String((init && init.method) || (input && input.method) || 'GET').toUpperCase() === 'POST'; try { if (login) boundary.startLogin(); } catch (e) { return Promise.reject(e); } var revision = boundary.revision(); diff --git a/server.js b/server.js index 7b17087c..d8e72c52 100644 --- a/server.js +++ b/server.js @@ -103,23 +103,6 @@ app.use('/api/auth/login', rateLimit({ message: { error: 'Too many login attempts. Try again in 15 minutes.' }, standardHeaders: true, legacyHeaders: false })); -// Asking for a code sends mail to someone else's address, so it is limited more -// tightly than a login attempt: the cost of abuse lands on the mailbox owner. -// Note these are separate limiters rather than covered by the one above — -// Express matches app.use paths on segment boundaries, so '/api/auth/login' -// does not match '/api/auth/login-code'. -app.use('/api/auth/login-code/request', rateLimit({ - windowMs: 60 * 60 * 1000, - max: parseInt(process.env.LOGIN_CODE_RATE_LIMIT_MAX || '5', 10), - message: { error: 'Too many code requests. Try again later, or sign in with your password.' }, - standardHeaders: true, legacyHeaders: false -})); -app.use('/api/auth/login-code/verify', rateLimit({ - windowMs: 15 * 60 * 1000, - max: parseInt(process.env.LOGIN_RATE_LIMIT_MAX || '10', 10), - message: { error: 'Too many attempts. Try again in 15 minutes.' }, - standardHeaders: true, legacyHeaders: false -})); app.use('/api/auth/register', rateLimit({ windowMs: 60 * 60 * 1000, max: 5, message: { error: 'Too many registration attempts.' }, diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index d83dfdc4..70d61ea7 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -841,62 +841,6 @@ router.get('/citation-audit', async function(req, res) { } catch (e) { return serverError(res, 'Citation audit', e, 'Could not read citation quality'); } }); -// ── Registration invites ───────────────────────────────────────────────── -// A code is shown once, at creation. Only its hash and last four characters -// are stored, so this endpoint is the only time it can be read. -var invites = require('../utils/registrationInvites'); - -router.get('/invites', async function(req, res) { - try { - // list() returns the code already decrypted, so an invitation can be copied - // again rather than only at the moment it was made; the cipher never leaves - // the server. A row created before codes were kept simply has no code, and - // its four-character hint is all there is to show. - res.json({ success: true, invites: await invites.list(), inviteOnly: await invites.inviteOnly(), lockdown: lockdown.state() }); - } catch (e) { return serverError(res, 'Invites list', e, 'Could not list invitations'); } -}); - -router.post('/invites', async function(req, res) { - try { - var created = await invites.create(req.user.id, { days: req.body.days, note: req.body.note }); - logger.audit(req.user.id, 'invite_create', 'Created a registration invite valid ' + created.days + ' days', req, { category: 'admin' }); - res.json({ success: true, code: created.code, days: created.days }); - } catch (e) { return serverError(res, 'Invite create', e, 'Could not create an invitation'); } -}); - -router.post('/invites/:id/revoke', async function(req, res) { - try { - var done = await invites.revoke(req.params.id, req.user.id); - if (!done) return res.status(400).json({ error: 'Only an unused, unrevoked invitation can be revoked' }); - logger.audit(req.user.id, 'invite_revoke', 'Revoked invitation ' + req.params.id, req, { category: 'admin' }); - res.json({ success: true }); - } catch (e) { return serverError(res, 'Invite revoke', e, 'Could not revoke the invitation'); } -}); - -// Clearing away spent invitations — used, or expired without being used. A code -// that could still be redeemed is never deleted: that would take it off the list -// without taking it out of anybody's inbox, leaving nothing to say who held it. -// Revoke is what stops a live code, and it leaves the row behind, marked. -router.delete('/invites/spent', async function(req, res) { - try { - var removed = await invites.removeSpent(); - logger.audit(req.user.id, 'invite_delete', 'Cleared ' + removed + ' spent invitations', req, { category: 'admin' }); - res.json({ success: true, removed: removed }); - } catch (e) { return serverError(res, 'Invite clear', e, 'Could not clear spent invitations'); } -}); - -router.delete('/invites/:id', async function(req, res) { - try { - if (!await invites.remove(req.params.id)) { - // Said plainly rather than as "not found": the row is very likely there, - // and the reason it cannot go is worth knowing. - return res.status(409).json({ error: 'That invitation can still be used. Revoke it instead.' }); - } - logger.audit(req.user.id, 'invite_delete', 'Deleted invitation ' + req.params.id, req, { category: 'admin' }); - res.json({ success: true }); - } catch (e) { return serverError(res, 'Invite delete', e, 'Could not delete the invitation'); } -}); - // ── GET discover STT models from provider ──────────────────────────────── router.get('/config/stt/discover', async function(req, res) { try { @@ -985,7 +929,7 @@ router.put('/config/:key(*)', async function(req, res) { } // Security: only allow known key prefixes - var allowed = ['announcement.', 'feature.', 'email.', 'prompt.', 'registration_enabled', 'registration_invite_only', 'site.', 'smtp.', 'models.', 'tts.', 'stt.', 'clinical_assistant.', 'my_resources.']; + var allowed = ['announcement.', 'feature.', 'email.', 'prompt.', 'registration_enabled', 'site.', 'smtp.', 'models.', 'tts.', 'stt.', 'clinical_assistant.', 'my_resources.']; var isAllowed = allowed.some(function(p) { return key === p || key.startsWith(p); }); if (!isAllowed) { return res.status(400).json({ error: 'Unknown config key' }); @@ -1032,9 +976,6 @@ router.put('/config/:key(*)', async function(req, res) { if (key === 'clinical_assistant.show_sources' && !['true', 'false'].includes(String(value))) { return res.status(400).json({ error: 'Show sources must be true or false' }); } - if (key === 'registration_invite_only' && !['true', 'false'].includes(String(value))) { - return res.status(400).json({ error: 'Invite-only must be true or false' }); - } // The admin's image-model roster: comma-separated gateway model ids. if (key === 'clinical_assistant.image_model_roster') { var rosterIds = String(value).split(',').map(function(s) { return s.trim(); }).filter(Boolean); diff --git a/src/routes/auth.js b/src/routes/auth.js index 9323d44d..e605ca91 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -9,8 +9,6 @@ const QRCode = require('qrcode'); const crypto = require('crypto'); const db = require('../db/database'); const { JWT_SECRET, authMiddleware } = require('../middleware/auth'); -const invites = require('../utils/registrationInvites'); -const loginCodes = require('../utils/loginCodes'); const { hashToken, parseUserAgent, generateSessionId } = require('../utils/sessions'); const { notifyNewLogin, notifyPasswordChanged, notifyNewRegistration } = require('../utils/notify'); var logger = require('../utils/logger'); @@ -184,16 +182,9 @@ router.post('/register', requireLocalAuth, async (req, res) => { return res.status(403).json({ error: 'Registration is currently disabled. Contact an administrator.' }); } - var { email, password, name, turnstileToken, inviteCode } = req.body; + var { email, password, name, turnstileToken } = req.body; if (!email || !password || !name) return res.status(400).json({ error: 'All fields required' }); - // Invite-only sits between "open" and "closed": anyone with a code, nobody - // without one. Checked before the work of hashing a password, and claimed - // atomically once the account exists. - var inviteRequired = await invites.inviteOnly(); - if (inviteRequired && !String(inviteCode || '').trim()) { - return res.status(400).json({ error: 'An invitation code is required to register.' }); - } if (password.length < 8) return res.status(400).json({ error: 'Password must be 8+ characters' }); // Cloudflare Turnstile verification @@ -228,17 +219,6 @@ router.post('/register', requireLocalAuth, async (req, res) => { var userId = result.lastInsertRowid; - // Claimed only now, so a code is never spent on a registration that failed. - // The claim is a single conditional UPDATE, so two people racing the same - // code cannot both win — the loser's account is removed again rather than - // left behind as a free registration. - if (inviteRequired) { - var claimedInvite = await invites.claim(inviteCode, userId); - if (!claimedInvite) { - await db.run('DELETE FROM users WHERE id = ?', [userId]); - return res.status(400).json({ error: 'That invitation code is not valid. It may have expired, been revoked, or already been used.' }); - } - } var verifyUrl = safeAppUrl() + '/api/auth/verify-email?token=' + verifyToken; var verifySubject = await db.getSetting('email.verify.subject') || 'Verify your Pediatric AI Scribe account'; var verifyBody = await db.getSetting('email.verify.body') || 'Great to have you on Pediatric AI Scribe. Please verify your email address by clicking the button below.'; @@ -317,104 +297,6 @@ router.post('/resend-verification', async (req, res) => { // ============================================================ // SIGN-IN CODE -// ============================================================ -// Signing in with a code emailed to you, offered alongside the password rather -// than instead of it: the code depends on mail being delivered and the password -// does not, so neither can be the only way in. -// -// Requesting one answers identically whether or not the address exists. A -// sign-in screen that says "no such account" is a way of finding out who has -// one, and that is worth more to an attacker than the code is. -router.post('/login-code/request', requireLocalAuth, async (req, res) => { - var email = String((req.body && req.body.email) || '').toLowerCase().trim(); - // Said the same way on every path below, including the ones that do nothing. - var GENERIC = { success: true, message: 'If that account exists, a code is on its way.' }; - try { - if (!email || email.length > 320) return res.json(GENERIC); - - var user = await db.get('SELECT id, email, name, disabled FROM users WHERE email = ?', [email]); - if (!user || user.disabled) { - console.warn('[Auth] login-code: no deliverable account (ip=' + req.ip + ')'); - return res.json(GENERIC); - } - - var code = await loginCodes.issue(db, user.id); - loginCodes.sweep(db); - // Fire and forget: whether the mail went is not something the response may - // reveal, and the password is still there if it did not. - // Through the same wrapper as the rest of the mail, so a sign-in code looks - // like it came from the same place as the verification and reset emails. - // It said 'PedAI' while every other message says SITE_NAME. - var codeSiteName = process.env.SITE_NAME || 'Pediatric AI Scribe'; - sendEmail(user.email, 'Your sign-in code for ' + codeSiteName, - emailWrapper(loginCodes.emailBody(code, codeSiteName, user.email))) - .catch(function (err) { console.warn('[Auth] login-code send failed:', err.message); }); - - await db.run('INSERT INTO audit_log (user_id, action, ip_address) VALUES (?, ?, ?)', - [user.id, 'login_code_requested', req.ip]).catch(function () {}); - res.json(GENERIC); - } catch (err) { - console.error('[Auth] login-code request error:', err.message); - res.json(GENERIC); - } -}); - -router.post('/login-code/verify', requireLocalAuth, async (req, res) => { - try { - var email = String((req.body && req.body.email) || '').toLowerCase().trim(); - var code = String((req.body && req.body.code) || ''); - var totpCode = (req.body && req.body.totpCode) || ''; - // One message for every failure. Which of them it was is not the caller's - // business, and saying would turn this into an account oracle. - var REFUSED = { error: 'That code is not valid. Request a new one, or sign in with your password.' }; - if (!email || !code) return res.status(400).json(REFUSED); - - var user = await db.get('SELECT * FROM users WHERE email = ?', [email]); - if (!user || user.disabled) { - console.warn('[Auth] login-code verify: no deliverable account (ip=' + req.ip + ')'); - return res.status(401).json(REFUSED); - } - - var ok = await loginCodes.consume(db, user.id, code); - if (!ok) { - logger.access(user.id, 'login_code', req, false); - return res.status(401).json(REFUSED); - } - - // A code proves you can read the mailbox, which is one factor. An account - // that asked for a second still wants it. - if (user.totp_enabled) { - if (!totpCode) return res.json({ requires2FA: true }); - var totpInput = String(totpCode).trim(); - var verified = speakeasy.totp.verify({ secret: user.totp_secret, encoding: 'base32', token: totpInput, window: 1 }); - if (!verified) { - var consumed = await tryConsumeBackupCode(user.id, totpInput); - if (!consumed) return res.status(401).json({ error: 'Invalid 2FA code' }); - } - } - - var token = signAuthToken(user.id, req); - await db.run('INSERT INTO audit_log (user_id, action, ip_address) VALUES (?, ?, ?)', - [user.id, 'login_code', req.ip]); - logger.access(user.id, 'login_code', req, true); - - var sessionId = generateSessionId(); - await db.run('INSERT INTO user_sessions (id, user_id, token_hash, ip_address, user_agent, device_label) VALUES (?, ?, ?, ?, ?, ?)', - [sessionId, user.id, hashToken(token), req.ip, req.headers['user-agent'] || '', parseUserAgent(req.headers['user-agent'])]); - notifyNewLogin(user.id, parseUserAgent(req.headers['user-agent']), req.ip); - - setAuthCookie(res, token); - res.json({ - success: true, token: token, sessionId: sessionId, - user: { id: user.id, email: user.email, name: user.name, role: user.role, - totp_enabled: user.totp_enabled, email_verified: user.email_verified } - }); - } catch (err) { - console.error('[Auth] login-code verify error:', err.message); - res.status(500).json({ error: 'Sign-in failed' }); - } -}); - // ============================================================ // LOGIN (checks disabled status) // ============================================================ @@ -801,10 +683,7 @@ router.get('/me', authMiddleware, async (req, res) => { router.get('/registration-status', async (req, res) => { try { var enabled = await db.getSetting('registration_enabled'); - res.json({ - registrationEnabled: enabled !== 'false' && !await isSSOOnly(), - inviteOnly: await invites.inviteOnly() - }); + res.json({ registrationEnabled: enabled !== 'false' && !await isSSOOnly() }); } catch (err) { res.json({ registrationEnabled: false }); } }); diff --git a/src/utils/adminLockdown.js b/src/utils/adminLockdown.js index eb50d466..ba5d4bcd 100644 --- a/src/utils/adminLockdown.js +++ b/src/utils/adminLockdown.js @@ -29,7 +29,6 @@ var LOCKED_PREFIXES = Object.freeze([ var EDITABLE_WHEN_LOCKED = Object.freeze([ 'announcement.', 'registration_enabled', - 'registration_invite_only', 'feature.', 'site.' ]); diff --git a/src/utils/loginCodes.js b/src/utils/loginCodes.js deleted file mode 100644 index 56dde832..00000000 --- a/src/utils/loginCodes.js +++ /dev/null @@ -1,150 +0,0 @@ -// ============================================================ -// SIGN-IN CODES -// ============================================================ -// A six-digit code emailed to you, as an alternative to typing a password — -// not a replacement for one. Both are always offered, because the code depends -// on mail being delivered and the password does not. -// -// The rules that make this safe rather than a second, weaker front door: -// -// - Only a hash is stored, so a code read out of the database is not a -// working credential. -// - Ten minutes, single use, and requesting a new one invalidates the old. -// - Five wrong guesses burns the code. Six digits is a million possibilities, -// which is plenty against a person and nothing against a script with -// unlimited tries at one code. -// - Requesting a code says the same thing whether or not the address exists. -// A sign-in screen that answers "no such account" is an oracle for finding -// out who has one. -// - Two-factor still applies. A code proves you can read the mailbox, which -// is one factor, and an account that asked for a second still wants it. - -var crypto = require('crypto'); -var bcrypt = require('bcryptjs'); - -var TTL_MINUTES = 10; -var MAX_ATTEMPTS = 5; -var LENGTH = 6; - -// Uniform across the full range, unlike % on a random byte, which would make -// low digits slightly likelier. -function generate() { - var max = Math.pow(10, LENGTH); - var value; - do { - value = crypto.randomBytes(4).readUInt32BE(0); - } while (value >= Math.floor(0xFFFFFFFF / max) * max); - return String(value % max).padStart(LENGTH, '0'); -} - -function normalise(input) { - return String(input || '').replace(/\D/g, '').slice(0, LENGTH); -} - -async function issue(db, userId) { - var code = generate(); - // One live code per account: asking for a new one must not leave the old one - // working, or a code forwarded to the wrong place stays usable. - await db.run('DELETE FROM login_codes WHERE user_id = ?', [userId]); - await db.run( - 'INSERT INTO login_codes (user_id, code_hash, expires_at) VALUES (?, ?, NOW() + INTERVAL \'' + - TTL_MINUTES + ' minutes\')', - [userId, await bcrypt.hash(code, 10)] - ); - return code; -} - -/** - * Returns true only for a live, unused, correctly-guessed code. - * - * Every failure path looks the same to the caller. Telling someone that a code - * was right but expired, or that there was no code at all, is information they - * did not have. - */ -async function consume(db, userId, input) { - var code = normalise(input); - if (code.length !== LENGTH) return false; - - var row = await db.get( - 'SELECT id, code_hash, attempts FROM login_codes ' + - 'WHERE user_id = ? AND used_at IS NULL AND expires_at > NOW() ' + - 'ORDER BY created_at DESC LIMIT 1', - [userId] - ); - if (!row) return false; - if (row.attempts >= MAX_ATTEMPTS) { - await db.run('DELETE FROM login_codes WHERE id = ?', [row.id]); - return false; - } - - var ok = await bcrypt.compare(code, row.code_hash); - if (!ok) { - await db.run('UPDATE login_codes SET attempts = attempts + 1 WHERE id = ?', [row.id]); - return false; - } - // Marked used before the caller issues a session, so a replay cannot race it. - var claimed = await db.get( - 'UPDATE login_codes SET used_at = NOW() WHERE id = ? AND used_at IS NULL RETURNING id', - [row.id] - ); - return Boolean(claimed); -} - -// Expired and spent codes are of no use to anyone; this keeps the table from -// growing without bound. Safe to call and ignore. -async function sweep(db) { - try { - await db.run("DELETE FROM login_codes WHERE expires_at < NOW() - INTERVAL '1 day'"); - } catch (e) { /* housekeeping never fails a request */ } -} - -// The address is the one thing here that did not come from us, and it is -// written into HTML, so it is escaped. The code cannot carry markup — it is six -// digits from a generator — but escaping only the interesting half is how the -// other half becomes interesting later. -function escapeHtml(value) { - return String(value == null ? '' : value) - .replace(/&/g, '&').replace(//g, '>') - .replace(/"/g, '"').replace(/'/g, '''); -} - -/** - * The body of the sign-in code email. - * - * Written for someone holding a phone in one hand: the code is the only thing - * on the screen that is large, it sits in a box of its own so it is obvious - * what to copy, and it is monospaced so a 0 cannot be mistaken for an O while - * being retyped. Everything else is quiet. - * - * It names the address the code signs into. A code that arrives at a shared - * mailbox, or to someone with two accounts, is otherwise a number with no - * indication of what it opens — and it is the one detail that lets a person - * notice a sign-in they did not start. - * - * Goes inside emailWrapper with the other mail, which supplies the wordmark, - * the rules and the footer — including the line about ignoring it, which is why - * there is not a second one here. - */ -function emailBody(code, appName, email) { - var name = escapeHtml(appName || 'PedAI'); - return '' + - '

Sign in to ' + name + '

' + - (email - ? '

' + - 'Enter this code to sign in as ' + escapeHtml(email) + '.

' - : '

Enter this code to sign in.

') + - - // A table, not a div: Outlook ignores padding and border-radius on a div, - // and this box is the whole point of the message. - '' + - '
' + - '' + escapeHtml(code) + '' + - '
' + - - '

' + - 'It expires in ' + TTL_MINUTES + ' minutes and can be used once.

'; -} - -module.exports = { issue, consume, sweep, generate, normalise, emailBody, - TTL_MINUTES, MAX_ATTEMPTS, LENGTH }; diff --git a/src/utils/openapiRoutes.js b/src/utils/openapiRoutes.js index bac1fa2b..9ce8b433 100644 --- a/src/utils/openapiRoutes.js +++ b/src/utils/openapiRoutes.js @@ -39,22 +39,13 @@ var operations = { description: 'Returns a token and sets the ped_auth cookie. Rate limited.', public: true }, - 'POST /api/auth/login-code/request': { - summary: 'Email a single-use sign-in code', - description: 'Always answers the same way whether or not the address has an account, so it cannot be used to discover who is registered.', - public: true - }, - 'POST /api/auth/login-code/verify': { - summary: 'Exchange a sign-in code for a session', - public: true - }, 'POST /api/auth/register': { - summary: 'Create an account', - description: 'Requires an invitation code while registration is invite-only.', + summary: 'Create a local password account', + description: 'Refused while sign-in is SSO-only; accounts are then created at the SSO from an invitation link.', public: true }, 'GET /api/auth/registration-status': { - summary: 'Whether registration is open, and whether it needs an invitation', + summary: 'Whether local registration is open', description: 'Read by the sign-in screen to decide whether to offer the register link.', public: true }, diff --git a/src/utils/registrationInvites.js b/src/utils/registrationInvites.js deleted file mode 100644 index 1d20621a..00000000 --- a/src/utils/registrationInvites.js +++ /dev/null @@ -1,213 +0,0 @@ -// ============================================================ -// REGISTRATION INVITES -// registration_enabled is open-or-closed. This is the middle setting: open to -// people you invited. One module owns the rules so the register route and the -// admin routes cannot disagree about what "valid" means. -// -// A code is single-use, expires, and can be revoked. It is stored hashed, -// because an invite grants account creation and a database dump should not -// hand someone a working one. -// ============================================================ - -var crypto = require('crypto'); -var cryptoUtil = require('./crypto'); - -// Required lazily. The code-generation and hashing helpers are pure, and -// requiring the database at module load made simply importing this file open a -// connection — which is why a test that only checked those helpers hung. -function db() { - return require('../db/database'); -} - -var DEFAULT_TTL_DAYS = 7; -var MAX_TTL_DAYS = 90; - -// Crockford-style: no I, L, O or U, so a code read aloud or copied off a screen -// is hard to mistype and cannot spell anything unfortunate. -var ALPHABET = '23456789ABCDEFGHJKMNPQRSTVWXYZ'; - -function generateCode() { - var bytes = crypto.randomBytes(16); - var out = ''; - for (var i = 0; i < 16; i++) { - out += ALPHABET[bytes[i] % ALPHABET.length]; - if (i % 4 === 3 && i !== 15) out += '-'; - } - return out; // XXXX-XXXX-XXXX-XXXX -} - -function normalize(code) { - return String(code == null ? '' : code).toUpperCase().replace(/[^0-9A-Z]/g, ''); -} - -function hash(code) { - return crypto.createHash('sha256').update(normalize(code)).digest('hex'); -} - -// Encrypted with DATA_ENCRYPTION_KEY, like every other recoverable secret here. -// Without a key configured the code is simply not kept — the invitation still -// works, it just cannot be shown again, which is exactly the old behaviour. -// -// Bound to the row's own code_hash, so a cipher lifted onto another invitation -// will not open there. The hash is already stored beside the cipher, so using -// it as the binding reveals nothing new, and unlike the row id it exists at the -// moment of the INSERT. -function codeContext(codeHash) { - return cryptoUtil.context('registration_invites', 'code_cipher', codeHash); -} - -function encryptCode(code) { - try { - if (!cryptoUtil.hasKey()) return null; - return cryptoUtil.encryptString(normalize(code), codeContext(hash(code))); - } catch (e) { return null; } -} - -// Codes made before binding have no context; decryptString ignores the one -// passed for those, so both forms read here. -function decryptCode(cipher, codeHash) { - if (!cipher) return null; - try { return format(cryptoUtil.decryptString(cipher, codeContext(codeHash))); } - catch (e) { return null; } -} - -// XXXX-XXXX-XXXX-XXXX from the stored, normalised form. -function format(normalized) { - var plain = String(normalized || ''); - if (plain.length !== 16) return plain || null; - return plain.replace(/(.{4})(?=.)/g, '$1-'); -} - -function ttlDays(requested) { - var days = Number(requested); - if (!Number.isFinite(days) || days < 1) return DEFAULT_TTL_DAYS; - return Math.min(Math.floor(days), MAX_TTL_DAYS); -} - -// Returns the code, and keeps it. Three things are stored: the hash, which is -// what a claim looks up; the last four characters, which are what an older row -// has; and the code encrypted, which is what makes it copyable later. -// -// An invitation has to be given to somebody, usually later than the moment it -// was made, and sometimes twice. Show-once was the wrong shape for that. -async function create(adminUserId, options) { - var opts = options || {}; - var code = generateCode(); - var days = ttlDays(opts.days); - var note = String(opts.note || '').trim().slice(0, 200); - await db().run( - "INSERT INTO registration_invites (code_hash, code_hint, code_cipher, note, created_by, expires_at) " + - "VALUES ($1, $2, $3, $4, $5, NOW() + ($6 || ' days')::interval)", - [hash(code), normalize(code).slice(-4), encryptCode(code), note, adminUserId, String(days)] - ); - return { code: code, days: days, note: note }; -} - -// Rows for the admin screen, with the code already decrypted. The cipher and -// the hash stay in here: the caller has no use for either, and the hash is now -// half of what opens the cipher. -async function list() { - var rows = await db().all( - "SELECT i.id, i.code_hint, i.code_cipher, i.code_hash, i.note, i.created_at, i.expires_at, i.used_at, i.revoked_at, " + - " c.email AS created_by_email, u.email AS used_by_email, " + - " CASE WHEN i.revoked_at IS NOT NULL THEN 'revoked' " + - " WHEN i.used_at IS NOT NULL THEN 'used' " + - " WHEN i.expires_at <= NOW() THEN 'expired' " + - " ELSE 'active' END AS status " + - "FROM registration_invites i " + - "LEFT JOIN users c ON c.id = i.created_by " + - "LEFT JOIN users u ON u.id = i.used_by " + - "ORDER BY i.created_at DESC LIMIT 200", [] - ); - return (rows || []).map(function (row) { - var code = decryptCode(row.code_cipher, row.code_hash); - delete row.code_cipher; - delete row.code_hash; - return Object.assign(row, { code: code }); - }); -} - -async function revoke(id, adminUserId) { - // Revoking an already-used code would rewrite history, so only a live one. - var result = await db().run( - 'UPDATE registration_invites SET revoked_at = NOW(), revoked_by = $1 ' + - 'WHERE id = $2 AND revoked_at IS NULL AND used_at IS NULL', - [adminUserId, id] - ); - return result.changes > 0; -} - -// Spent: used, revoked, or run out of time. The one thing never deletable is a -// code that could still be redeemed — deleting that takes it off the list -// without taking it out of anybody's inbox, so the holder keeps something that -// looks valid, it quietly stops working, and nothing is left to say who had it. -// -// Revoked codes were held back from this at first, on the reasoning that revoke -// stops a code and leaves the row. But a revoked code is already dead: it -// cannot be redeemed, and keeping it only fills the list. Revoke and delete are -// two steps of the same thought, and the second was missing. -var SPENT = '(used_at IS NOT NULL OR revoked_at IS NOT NULL OR expires_at <= NOW())'; - -/** - * Delete a spent invitation. - * - * Returns false for one that is still live, which the caller reports rather - * than treating as a missing row. - */ -async function remove(id) { - var result = await db().run( - 'DELETE FROM registration_invites WHERE id = $1 AND ' + SPENT, [id]); - return result.changes > 0; -} - -// Every spent invitation at once, which is what "they clutter the list" asks -// for. Same rule: nothing still redeemable is touched. -async function removeSpent() { - var result = await db().run('DELETE FROM registration_invites WHERE ' + SPENT); - return result.changes || 0; -} - -/** - * Claim a code for a registration, atomically. - * - * The UPDATE carries every condition, so two registrations racing the same code - * cannot both succeed: the second matches no row. Checking first and updating - * after would leave exactly that gap. - * - * Returns the invite id, or null if the code is unusable for any reason — - * unknown, expired, revoked or already used. The caller must not say which: - * distinguishing them tells someone probing codes which guesses were closer. - */ -async function claim(code, userId) { - var normalized = normalize(code); - if (!normalized) return null; - var rows = await db().all( - 'UPDATE registration_invites SET used_at = NOW(), used_by = $1 ' + - 'WHERE code_hash = $2 AND used_at IS NULL AND revoked_at IS NULL AND expires_at > NOW() ' + - 'RETURNING id', - [userId, hash(normalized)] - ); - return rows && rows.length ? rows[0].id : null; -} - -// True when a code must be supplied to register at all. -async function inviteOnly() { - return String(await db().getSetting('registration_invite_only') || 'false') === 'true'; -} - -module.exports = { - formatCode: format, - DEFAULT_TTL_DAYS, - MAX_TTL_DAYS, - generateCode, - normalize, - hash, - ttlDays, - create, - list, - revoke, - remove, - removeSpent, - claim, - inviteOnly -}; diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index 774d064f..ca3799dc 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -147,83 +147,6 @@ test('.env.example documents every variable the app reads', () => { assert.deepEqual(missing, [], 'undocumented variables: ' + missing.join(', ')); }); -// registration_enabled is open-or-closed. Invite-only is the middle setting, -// and it has to hold up against someone probing codes. -test('an invitation is deletable only once it can no longer be used', () => { - const src = read('src/utils/registrationInvites.js'); - const route = read('src/routes/adminConfig.js'); - - // A code that could still be redeemed may be sitting in somebody's inbox. - // Deleting the row takes it off the list without taking it out of their - // hands: it quietly stops working and nothing is left to say who held it. - // Revoke is what stops such a code — and a revoked one is then dead, so it - // is deletable like any other spent code. - assert.match(src, /var SPENT = '\(used_at IS NOT NULL OR revoked_at IS NOT NULL OR expires_at <= NOW\(\)\)';/); - assert.match(src, /DELETE FROM registration_invites WHERE id = \$1 AND ' \+ SPENT/); - assert.match(src, /DELETE FROM registration_invites WHERE ' \+ SPENT/, 'and in bulk'); - // Scoped to the rule itself. revoked_at IS NULL appears elsewhere and - // belongs there: revoke() will not re-revoke, and claim() will not redeem a - // revoked code. It is only in *this* rule that it meant "keep it forever". - const spentRule = src.slice(src.indexOf('var SPENT ='), src.indexOf('var SPENT =') + 120); - assert.doesNotMatch(spentRule, /revoked_at IS NULL/, 'a revoked code is spent, not protected'); - assert.match(spentRule, /revoked_at IS NOT NULL/); - - // Refused with the reason, not as a missing row: the row is very likely there. - assert.match(route, /That invitation can still be used\. Revoke it instead\./); - assert.match(route, /res\.status\(409\)/); - // The bulk route is declared before /invites/:id, or "spent" is read as an id. - assert.ok(route.indexOf("router.delete('/invites/spent'") < route.indexOf("router.delete('/invites/:id'")); - - // And the button is only offered where it can work. - const js = read('public/js/admin.js'); - assert.match(js, /var SPENT_STATUS = \['used', 'expired', 'revoked'\];/); - assert.match(js, /SPENT_STATUS\.indexOf\(row\.status\) !== -1\s*\n?\s*\? '