diff --git a/frontend/js/api.js b/frontend/js/api.js index 112a7f9..574398b 100644 --- a/frontend/js/api.js +++ b/frontend/js/api.js @@ -210,6 +210,9 @@ export const api = { // TOTP 2FA (#100) — opt-in per-user, local accounts only. See routes/auth.js. totpStatus: () => request('/auth/totp/status'), + // Unlink an instance-wide SSO provider. The new password is required in the same call: + // the account must never sit between credentials. + ssoUnlink: (password) => request('/auth/oidc/unlink', { method: 'POST', body: JSON.stringify({ password }) }), totpSetup: () => request('/auth/totp/setup', { method: 'POST' }), totpEnable: (code) => request('/auth/totp/enable', { method: 'POST', body: JSON.stringify({ code }) }), totpDisable: (code) => request('/auth/totp/disable', { method: 'POST', body: JSON.stringify({ code }) }), diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 25fc544..375981c 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -116,6 +116,8 @@ export default { 'common.unknown': 'Unknown', // Auth (login view) + 'auth.next': 'Next', + 'auth.error_email_required': 'Enter your email address', 'auth.sign_in': 'Sign In', 'auth.sign_out': 'Sign out', 'auth.create_account': 'Create Account', @@ -836,6 +838,33 @@ export default { 'settings.save_profile': 'Save Profile', 'settings.email_alerts': 'Email me when devices go offline', 'settings.change_password': 'Change Password', + // Sign-in method (#258). The link warning is deliberately explicit about destruction of the + // password — that is the part users miss, and it is not reversible without setting a new one. + 'settings.signin_method': 'Sign-in method', + 'settings.signin_password_now': 'This account signs in with a password. You can link it to a single sign-on provider instead.', + 'settings.signin_password_only': 'This account signs in with a password. No single sign-on providers are configured on this server.', + 'settings.signin_link': 'Link {provider}', + 'settings.signin_link_warning': 'You are linking this account to {provider}.\n\nYour local password will be DELETED. After this you sign in with {provider} only.\n\nTo go back to a password later, unlink {provider} and set a new one.', + 'settings.signin_linked': 'This account signs in with {provider}. It has no password.', + 'settings.signin_unlink': 'Unlink {provider}', + 'settings.signin_unlink_desc': 'Set a password to sign in with instead. It takes effect immediately and {provider} is unlinked in the same step.', + 'settings.signin_unlink_confirm': 'Set password and unlink', + 'settings.signin_unlinked_toast': 'Unlinked. You now sign in with your password.', + 'settings.passwords_dont_match': 'The two passwords do not match', + 'settings.signin_linked_toast': 'Linked. You now sign in with {provider}, and your password has been removed.', + 'settings.signin_err_link_email_mismatch': 'That provider account uses a different email address than this account. Sign in to the provider with the same address and try again.', + 'settings.signin_err_link_already_used': 'That provider account is already linked to a different ScreenTinker account.', + 'settings.signin_err_not_linkable': 'Only the providers configured on this server can be linked to an account.', + 'settings.signin_err_no_email': 'The provider did not supply an email address, so the account could not be linked.', + 'settings.signin_err_email_unverified': 'The provider would not confirm that email address is verified.', + 'settings.signin_err_verification_failed': 'The sign-in could not be verified. Nothing was changed.', + 'settings.signin_err_provider_unavailable': 'The provider could not be reached. Nothing was changed.', + 'settings.signin_err_provider_refused': 'The provider refused the request. Nothing was changed.', + 'settings.signin_err_unknown_provider': 'That provider is not configured on this server.', + 'settings.signin_err_expired': 'That took too long. Start the link again.', + 'settings.signin_err_bad_state': 'The response did not match the request. Start the link again.', + 'settings.signin_err_no_code': 'The provider returned no authorization code. Start the link again.', + 'settings.signin_err_server_error': 'Something went wrong. Nothing was changed.', 'settings.password_min_8': 'Must be at least 8 characters.', 'settings.current_password': 'Current Password', 'settings.new_password': 'New Password', diff --git a/frontend/js/views/login.js b/frontend/js/views/login.js index f1585c0..1acf2e2 100644 --- a/frontend/js/views/login.js +++ b/frontend/js/views/login.js @@ -287,7 +287,15 @@ function setupHandlers(config, isSetup) { if (isSetup) { document.getElementById('loginBtn')?.addEventListener('click', () => doRegister(true)); } else { - document.getElementById('loginBtn')?.addEventListener('click', doLogin); + /* + * Identifier-first. The button is "Next" until an address has been submitted: we ask the server + * what that address uses BEFORE offering a credential, so an SSO-only user is never shown a + * password box that is going to be refused, and the org lookup has somewhere to happen. + */ + document.getElementById('loginBtn')?.addEventListener('click', () => { + if (identified && !ssoOnlyDomain) return doLogin(); + identify(); + }); document.getElementById('showRegisterBtn')?.addEventListener('click', () => { document.getElementById('localAuthForm').style.display = 'none'; document.getElementById('registerForm').style.display = 'block'; @@ -304,6 +312,40 @@ function setupHandlers(config, isSetup) { if (e.key === 'Enter') isSetup ? doRegister(true) : doLogin(); }); + /* + * Enter in the EMAIL field advances rather than submitting. During first-run setup both fields + * are needed at once, so identifier-first is skipped entirely there. + */ + document.getElementById('loginEmail')?.addEventListener('keydown', (e) => { + if (e.key !== 'Enter') return; + if (isSetup) return doRegister(true); + if (identified && !ssoOnlyDomain) return doLogin(); + identify(); + }); + + /* + * Editing the address after identifying returns to the identifier step. Someone who mistypes + * their domain must get a fresh answer rather than keep the previous domain's one. + */ + document.getElementById('loginEmail')?.addEventListener('input', () => { + if (!identified) return; + identified = false; + applyFormState(); + }); + + /* + * Ask what this address uses, then show the right thing. The lookup itself sets ssoOnlyDomain via + * setPasswordVisible(), so this only has to decide that we now know who is signing in. + */ + async function identify() { + const email = document.getElementById('loginEmail').value.trim(); + if (!email || !email.includes('@')) { showError(t('auth.error_email_required')); return; } + try { await lookupOrgSso(email); } catch { /* lookup failures fall through to the password box */ } + identified = true; + applyFormState(); + if (!ssoOnlyDomain) document.getElementById('loginPassword')?.focus(); + } + async function doLogin() { const email = document.getElementById('loginEmail').value.trim(); const password = document.getElementById('loginPassword').value; @@ -509,7 +551,6 @@ function setupHandlers(config, isSetup) { * Debounced because this fires while someone types, and the endpoint is rate limited; asking on * every keystroke would spend a user's whole budget before they finished their own address. */ - let ssoLookupTimer = null; let lastDomainAsked = ''; const orgSlot = () => document.getElementById('orgSsoSlot'); @@ -520,44 +561,68 @@ function setupHandlers(config, isSetup) { * on every negative answer matters as much as hiding it: someone who types an SSO-only address, * then corrects it to their own, must get the password box back. */ - function setPasswordVisible(visible) { + /* + * Password visibility has TWO independent drivers, and conflating them is how this got confusing: + * + * identified — identifier-first. The password box does not exist until an address has been + * submitted, because until then we do not know whether this account uses a + * password at all. This is what lets the org lookup happen before we offer the + * wrong thing. + * ssoOnlyDomain — the address belongs to an organization that REQUIRES its own provider. Then a + * password box is not merely going to fail, it is the wrong thing to show. + * + * The field appears only when identified AND not SSO-only. Kept as one function so the two can + * never disagree about what is on screen. + */ + let identified = false; + let ssoOnlyDomain = false; + + function applyFormState() { + const showPassword = identified && !ssoOnlyDomain; + const show = showPassword ? '' : 'none'; /* * ⚠️ Hide the password FIELD, never its .form-group — the organization SSO slot lives inside - * that same group, so hiding the container took the single sign-on button down with it and left - * a login page whose only action was "Create Account". Found by looking at a screenshot. + * that same group, so hiding the container took the single sign-on button down with it. */ - const show = visible ? '' : 'none'; - for (const id of ['loginPassword', 'loginPasswordLabel', 'loginBtn']) { + for (const id of ['loginPassword', 'loginPasswordLabel']) { const el = document.getElementById(id); if (el) el.style.display = show; } + /* - * The instance's own providers go too. They are the operator's, not this organization's, and - * they are not domain-confined — so offering "Continue with Google" to someone whose company - * requires its own identity provider is offering them the bypass. The server refuses it either - * way; this stops the page inviting it. + * The primary button is "Next" until an address has been submitted, then "Sign in". One button + * rather than two, so there is never a choice about which to press. */ - const instance = document.getElementById('instanceProviders'); - if (instance) instance.style.display = show; + const btn = document.getElementById('loginBtn'); + if (btn) btn.textContent = identified && !ssoOnlyDomain ? t('auth.sign_in') : t('auth.next'); + if (btn) btn.style.display = ssoOnlyDomain ? 'none' : ''; + /* - * "Create Account" goes too. Registration at an SSO-only domain is refused by the server, and - * leaving the button was worse than useless: it was the ONLY action left on the card, so the - * page invited the one thing that cannot work. + * The instance's own providers stay visible at ALL times, by explicit decision: they are the + * operator's, they are offered to everyone, and the server refuses them for an SSO-only + * organization anyway. (Previously they were hidden for such domains so the page would not + * invite the bypass; the cost was a login page that changed shape while you typed.) + */ + + /* + * "Create Account" and "Forgot your password?" DO go for an SSO-only domain: registration there + * is refused by the server, and a password reset produces one that can never be used. */ const reg = document.getElementById('showRegisterBtn'); - if (reg) reg.style.display = show; - // The OR divider sits outside #instanceProviders, so hiding those alone left a dangling rule - // with nothing beneath it. - const divider = document.getElementById('ssoDivider'); - if (divider) divider.style.display = show; - // "Forgot your password?" sits in its own

; hide the wrapper so no empty gap is left. + if (reg) reg.style.display = ssoOnlyDomain ? 'none' : ''; const forgot = document.getElementById('forgotLink'); if (forgot) { const wrap = forgot.parentElement && forgot.parentElement.tagName === 'P' ? forgot.parentElement : forgot; - wrap.style.display = show; + wrap.style.display = ssoOnlyDomain ? 'none' : ''; } } + // Kept for the org lookup below, which reasons about SSO-only rather than about identification. + function setPasswordVisible(visible) { + ssoOnlyDomain = !visible; + applyFormState(); + } + async function lookupOrgSso(email) { const at = String(email || '').lastIndexOf('@'); const domain = at === -1 ? '' : email.slice(at + 1).trim().toLowerCase(); @@ -650,11 +715,21 @@ function setupHandlers(config, isSetup) { } } - document.getElementById('loginEmail')?.addEventListener('input', (e) => { - clearTimeout(ssoLookupTimer); - const value = e.target.value; - ssoLookupTimer = setTimeout(() => lookupOrgSso(value), 400); - }); + /* + * The lookup now runs on SUBMIT (identify()), not on every keystroke. + * + * Identifier-first made the debounced version both redundant and wrong: redundant because nothing + * is shown until an address is submitted anyway, and wrong because it would answer for a + * half-typed domain and change the form under someone mid-address. It also spent a rate-limit + * budget of 10/min per IP on people who had not finished typing — an office behind one address + * could exhaust it without a single sign-in attempt. + * + * ⚠️ Applied HERE, after the `let identified` / `let ssoOnlyDomain` declarations above. Called any + * earlier it would throw on the temporal dead zone, which on this page means a login form that + * never renders. + */ + if (isSetup) identified = true; // first-run setup needs both fields at once + applyFormState(); /* * Completing an SSO login. diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index 7a01f6d..5819cb0 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -62,6 +62,16 @@ export async function render(container) {

${t('settings.sso_note', { provider: esc(user.auth_provider || 'SSO') })}

`} + +
+

${t('settings.signin_method')}

+

+
+

${t('settings.2fa_title')}

@@ -526,6 +536,87 @@ export async function render(container) { // ==================== Two-factor authentication (#100) ==================== // Drives the merged TOTP backend (/api/auth/totp/*). Re-renders #twoFactorBlock // for each state: SSO note / disabled+enroll / recovery-codes / enabled+manage. + /* + * Sign-in method: password OR one instance-wide provider, never both. + * + * The warning on the link button is the whole UX: the local password is DELETED, not kept as a + * fallback, and someone who does not read that will think they gained a second way in. Unlink + * asks for the new password up front for the same reason — the account must never sit between + * credentials. + * + * Only instance-wide providers appear. An organization's provider is chosen by a customer and + * must not be attachable to a platform account; the server refuses it too. + */ + async function loadSsoLink() { + const block = document.getElementById('ssoLinkBlock'); + if (!block) return; + const head = `

${t('settings.signin_method')}

`; + const muted = 'color:var(--text-muted);font-size:12px'; + const paint = (inner) => { block.innerHTML = head + inner; }; + + let me; + try { me = await api.getMe(); } + catch (e) { paint(`

${esc(e.message)}

`); return; } + + let providers = []; + try { + const res = await fetch('/api/auth/providers'); + if (res.ok) providers = (await res.json()).providers || []; + } catch { /* offline: fall through to the no-providers copy */ } + + if (me.auth_provider && me.auth_provider !== 'local') { + const name = providers.find((p) => p.slug === me.auth_provider)?.name || me.auth_provider; + paint(` +

${t('settings.signin_linked', { provider: esc(name) })}

+ + + `); + document.getElementById('unlinkBtn').onclick = () => { + document.getElementById('unlinkForm').style.display = ''; + document.getElementById('unlinkBtn').style.display = 'none'; + document.getElementById('unlinkPw').focus(); + }; + document.getElementById('unlinkConfirmBtn').onclick = async () => { + const pw = document.getElementById('unlinkPw').value; + const pw2 = document.getElementById('unlinkPw2').value; + if (pw !== pw2) return showToast(t('settings.passwords_dont_match'), 'error'); + try { + await api.ssoUnlink(pw); + showToast(t('settings.signin_unlinked_toast'), 'success'); + loadSsoLink(); + } catch (e) { showToast(e.message, 'error'); } + }; + return; + } + + if (!providers.length) { + paint(`

${t('settings.signin_password_only')}

`); + return; + } + paint(` +

${t('settings.signin_password_now')}

+
+ ${providers.map((p) => ``).join('')} +
+ `); + block.querySelectorAll('[data-link-slug]').forEach((btn) => { + btn.onclick = () => { + const slug = btn.dataset.linkSlug; + const name = providers.find((p) => p.slug === slug)?.name || slug; + // Deliberately blunt: the password is destroyed, and that is the part people miss. + if (!window.confirm(t('settings.signin_link_warning', { provider: name }))) return; + window.location.href = `/api/auth/oidc/${encodeURIComponent(slug)}/link/start`; + }; + }); + } + async function load2FA() { const block = document.getElementById('twoFactorBlock'); if (!block) return; @@ -660,6 +751,32 @@ export async function render(container) { loadTokens(); load2FA(); + loadSsoLink(); + + /* + * Report the outcome of a link round trip. + * + * The callback returns to #/settings rather than the login page — an authenticated user bounced + * to a login screen to be told "that did not work" reads as having been signed out. Params are + * stripped afterwards so a refresh or a copied URL does not replay the message. + */ + (function reportLinkOutcome() { + const q = new URLSearchParams((location.hash.split('?')[1] || '')); + const linked = q.get('sso_linked'); + const err = q.get('sso_error'); + if (!linked && !err) return; + if (linked) { + showToast(t('settings.signin_linked_toast', { provider: linked }), 'success'); + } else { + const known = ['link_email_mismatch', 'link_already_used', 'not_linkable', 'no_email', + 'email_unverified', 'verification_failed', 'provider_unavailable', 'provider_refused', + 'unknown_provider', 'expired', 'bad_state', 'no_code', 'server_error']; + const key = known.includes(err) ? `settings.signin_err_${err}` : 'auth.sso_failed'; + showToast(t(key), 'error'); + } + history.replaceState(null, '', location.pathname + location.search + '#/settings'); + loadSsoLink(); + }()); // #73: agency scope reveals a playlist picker (the token's allowlist). Loaded lazily once. const tokScopeSel = document.getElementById('tokScope'); diff --git a/server/routes/auth.js b/server/routes/auth.js index 6b7fa53..1155179 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -1158,6 +1158,13 @@ function backToApp(res, params) { res.redirect(`/app#/login?${qs}`); } +// A link attempt starts from Settings while signed in, so it must end there — bouncing an +// authenticated user to the login page to report the outcome reads as "you were signed out". +function backToSettings(res, params) { + const qs = new URLSearchParams(params).toString(); + res.redirect(`/app#/settings?${qs}`); +} + // Which providers this instance offers. Public: it is what draws the login buttons. router.get('/providers', (req, res) => { res.json({ providers: oidcProviders.publicList() }); @@ -1233,10 +1240,15 @@ router.post('/sso/start', express.urlencoded({ extended: false }), (req, res) => res.redirect(startUrl); }); -router.get('/oidc/:slug/start', asyncRoute(async (req, res) => { - const provider = oidcProviders.get(req.params.slug); - if (!provider) return backToApp(res, { sso_error: 'unknown_provider' }); - +/** + * Begin an OIDC round trip. + * + * `extra` is merged into the signed transaction, which is how LINK mode is carried: the tx is + * server-signed and lives in an httpOnly cookie, so the browser can neither read nor forge which + * account a link is for. Login and link therefore share one flow — the same PKCE, state, nonce and + * verification — instead of a second copy that drifts. + */ +async function beginOidc(req, res, provider, extra = {}, onError = backToApp) { try { const doc = await oidc.discover(provider.issuer); const pkce = oidc.createPkce(); @@ -1244,7 +1256,7 @@ router.get('/oidc/:slug/start', asyncRoute(async (req, res) => { const state = oidc.randomToken(); const tx = jwt.sign( - { typ: 'oidc-tx', slug: provider.slug, nonce, verifier: pkce.verifier, state }, + { typ: 'oidc-tx', slug: provider.slug, nonce, verifier: pkce.verifier, state, ...extra }, config.jwtSecret, // HS256 explicitly, and a `typ` the session verifier does not accept: two token kinds signed // with one secret must never be interchangeable, even if today only `slug` happens to stop it. @@ -1269,9 +1281,61 @@ router.get('/oidc/:slug/start', asyncRoute(async (req, res) => { url.searchParams.set('code_challenge_method', pkce.method); res.redirect(url.toString()); } catch (err) { - console.error(`[oidc] ${req.params.slug} start failed:`, err.message); - backToApp(res, { sso_error: 'provider_unavailable' }); + console.error(`[oidc] ${provider.slug} start failed:`, err.message); + onError(res, { sso_error: 'provider_unavailable' }); } +} + +router.get('/oidc/:slug/start', asyncRoute(async (req, res) => { + const provider = oidcProviders.get(req.params.slug); + if (!provider) return backToApp(res, { sso_error: 'unknown_provider' }); + await beginOidc(req, res, provider); +})); + +/* + * Link an EXISTING account to an instance-wide provider. + * + * Signing in with a provider never adopts an account that has a password — that would let anyone who + * can make a provider assert an address inherit the account behind it. So the owner proves they are + * the owner first, by being signed in, and starts the link themselves. The account is taken from the + * SESSION, never from the email in the returned token. + * + * ⚠️ INSTANCE-WIDE PROVIDERS ONLY. An organization's provider is chosen by a customer; letting one + * attach itself to a platform account would hand that customer whatever the account can do. Org + * membership arrives through the normal org SSO path, which is domain-confined. + */ +/* + * Unlink, and set a password in the SAME operation. + * + * Not two steps. An account whose only credential is a provider has nothing to fall back on the + * moment that link is removed, so "unlink now, set a password next" leaves a window — and a failure + * in between leaves an account nobody can sign into at all. The new password is therefore required + * up front and written in one transaction with the unlink. + */ +router.post('/oidc/unlink', requireAuth, (req, res) => { + const password = String((req.body || {}).password || ''); + const user = db.prepare('SELECT id, email, auth_provider, password_hash FROM users WHERE id = ?').get(req.user.id); + if (!user) return res.status(404).json({ error: 'Account not found' }); + if (user.auth_provider === 'local') { + return res.status(400).json({ error: 'This account already signs in with a password' }); + } + if (password.length < passwordReset.MIN_PASSWORD_LENGTH) { + return res.status(400).json({ error: `Password must be at least ${passwordReset.MIN_PASSWORD_LENGTH} characters` }); + } + + const was = user.auth_provider; + db.prepare("UPDATE users SET auth_provider = 'local', provider_id = NULL, password_hash = ? WHERE id = ?") + .run(bcrypt.hashSync(password, 10), user.id); + logActivity(user.id, 'auth:sso_unlinked', `was ${was}`, null, getClientIp(req)); + console.log(`[oidc] ${was} unlinked from ${user.email} (password set)`); + res.json({ ok: true, auth_provider: 'local' }); +}); + +router.get('/oidc/:slug/link/start', requireAuth, asyncRoute(async (req, res) => { + const provider = oidcProviders.get(req.params.slug); + if (!provider) return backToSettings(res, { sso_error: 'unknown_provider' }); + if (provider.organizationId) return backToSettings(res, { sso_error: 'not_linkable' }); + await beginOidc(req, res, provider, { link: req.user.id }, backToSettings); })); router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => { @@ -1338,7 +1402,9 @@ router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => { } const email = String(claims.email || '').toLowerCase().trim(); - if (!email) return backToApp(res, { sso_error: 'no_email' }); + const linking = !!tx.link; + const fail = linking ? backToSettings : backToApp; + if (!email) return fail(res, { sso_error: 'no_email' }); /* * ⚠️ AN ORGANIZATION'S PROVIDER MAY ONLY SPEAK FOR ITS OWN DOMAINS. @@ -1387,7 +1453,39 @@ router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => { // explicit false is still refused, and an org-configured provider still cannot assume anything. // See oidcProviders.emailIsVerified() for why that division is the safe one. if (!oidcProviders.emailIsVerified(claims, provider)) { - return backToApp(res, { sso_error: 'email_unverified' }); + return fail(res, { sso_error: 'email_unverified' }); + } + + /* + * LINK: attach this provider to the account that STARTED the link, and drop its password. + * + * The account comes from the signed transaction (i.e. from the session that began this), never + * from the returned email — otherwise "linking" would be the very email-keyed takeover the login + * path refuses. The email must still match the account's own, because login resolves an account by + * the address the provider asserts: linking a different address would produce an account that + * cannot be signed into, or would collide with someone else's. + * + * The password is DELETED rather than kept alongside. One credential at a time is the whole point + * — a password left behind is a second way in that the user believes they replaced. + */ + if (linking) { + const target = db.prepare('SELECT id, email, auth_provider FROM users WHERE id = ?').get(tx.link); + if (!target) return backToSettings(res, { sso_error: 'server_error' }); + if (target.email.toLowerCase() !== email) { + console.warn(`[oidc] link refused: ${provider.slug} asserted ${email} for account ${target.email}`); + return backToSettings(res, { sso_error: 'link_email_mismatch' }); + } + // Someone else already signed in with this provider identity. Two accounts must never share one + // provider subject, or whoever signs in second silently takes the first one's place. + const taken = db.prepare('SELECT id FROM users WHERE provider_id = ? AND auth_provider = ? AND id != ?') + .get(String(claims.sub), provider.slug, target.id); + if (taken) return backToSettings(res, { sso_error: 'link_already_used' }); + + db.prepare('UPDATE users SET auth_provider = ?, provider_id = ?, password_hash = NULL, avatar_url = COALESCE(?, avatar_url) WHERE id = ?') + .run(provider.slug, String(claims.sub), claims.picture || null, target.id); + logActivity(target.id, 'auth:sso_linked', `provider=${provider.slug}`, null, getClientIp(req)); + console.log(`[oidc] ${provider.slug} linked to ${target.email} (password cleared)`); + return backToSettings(res, { sso_linked: provider.slug }); } try { diff --git a/server/test/login-identifier-first.test.js b/server/test/login-identifier-first.test.js new file mode 100644 index 0000000..3a8d872 --- /dev/null +++ b/server/test/login-identifier-first.test.js @@ -0,0 +1,63 @@ +'use strict'; + +/* + * Identifier-first login (#258). + * + * The password box does not exist until an address has been submitted. That is what lets the + * organization lookup happen BEFORE a credential is offered, so someone whose company requires its + * own identity provider is never shown a password box that is going to be refused. + * + * Verified in a real browser as well (password hidden -> submit -> visible + focused -> edit the + * address -> hidden again); these assertions stop the wiring being removed silently. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const LOGIN = fs.readFileSync(path.join(__dirname, '..', '..', 'frontend', 'js', 'views', 'login.js'), 'utf8'); + +test('password visibility depends on BOTH identification and SSO-only', () => { + assert.match(LOGIN, /const showPassword = identified && !ssoOnlyDomain;/, + 'the two drivers must be combined in one place so they cannot disagree'); +}); + +test('the primary button advances before it signs in', () => { + assert.match(LOGIN, /if \(identified && !ssoOnlyDomain\) return doLogin\(\);\s*\n\s*identify\(\);/, + 'the button must identify first and only sign in once an address is known'); + assert.match(LOGIN, /btn\.textContent = identified && !ssoOnlyDomain \? t\('auth\.sign_in'\) : t\('auth\.next'\)/); +}); + +test('editing the address returns to the identifier step', () => { + assert.match(LOGIN, /if \(!identified\) return;\s*\n\s*identified = false;/, + 'a corrected address must get a fresh answer, not the previous domain\'s'); +}); + +test('the per-keystroke lookup is gone', () => { + assert.doesNotMatch(LOGIN, /ssoLookupTimer/, + 'the debounced lookup answered for half-typed domains and burned a 10/min budget'); + assert.match(LOGIN, /async function identify\(\)[\s\S]{0,400}await lookupOrgSso\(email\)/, + 'the lookup now runs on submit'); +}); + +test('instance-wide providers are never hidden', () => { + // Deliberate: they are the operator's, offered to everyone, and the server refuses them for an + // SSO-only organization anyway. Hiding them made the page change shape while typing. + assert.doesNotMatch(LOGIN, /getElementById\('instanceProviders'\)[\s\S]{0,120}style\.display/, + 'nothing may hide #instanceProviders'); +}); + +test('first-run setup skips identifier-first', () => { + assert.match(LOGIN, /if \(isSetup\) identified = true;/, + 'creating the first admin needs both fields at once'); +}); + +test('the initial state is applied after its declarations (temporal dead zone)', () => { + const decl = LOGIN.indexOf('let identified = false;'); + const call = LOGIN.lastIndexOf('\n applyFormState();'); + assert.ok(decl !== -1 && call !== -1, 'both the declaration and the init call must exist'); + assert.ok(call > decl, + 'applyFormState() must be called AFTER the let declarations — earlier throws on the TDZ, which ' + + 'on this page means a login form that never renders'); +}); diff --git a/server/test/oidc-account-linking.test.js b/server/test/oidc-account-linking.test.js new file mode 100644 index 0000000..a128e64 --- /dev/null +++ b/server/test/oidc-account-linking.test.js @@ -0,0 +1,99 @@ +'use strict'; + +/* + * Linking an existing account to an instance-wide provider (#258). + * + * Signing in with a provider never adopts an account that already has a password — that is the + * takeover the login path exists to refuse. The README promised an escape hatch ("the owner signs + * in locally and links from Settings") that was never built, so an account created with a password + * could never use SSO at all. + * + * The rules this pins down, all of which are load-bearing: + * - the account being linked comes from the SIGNED TRANSACTION (i.e. the session that started the + * link), never from the email in the returned token. Otherwise "linking" is the same email-keyed + * takeover under a friendlier name; + * - the provider's email must still equal the account's, because login resolves accounts by the + * asserted address; + * - one provider subject may not be linked to two accounts; + * - linking DELETES the password: one credential at a time; + * - unlinking SETS a password in the same statement, so the account is never between credentials; + * - ORG providers are not linkable at all. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const AUTH = fs.readFileSync(path.join(__dirname, '..', 'routes', 'auth.js'), 'utf8'); + +/** Body of a route handler, from its `router.(''` to the next `router.`. */ +function handler(verb, route) { + const start = AUTH.indexOf(`router.${verb}('${route}'`); + assert.notEqual(start, -1, `route ${verb.toUpperCase()} ${route} not found`); + const rest = AUTH.slice(start + 1); + const end = rest.indexOf('\nrouter.'); + return end === -1 ? rest : rest.slice(0, end); +} + +test('link start requires authentication and refuses org providers', () => { + const body = handler('get', '/oidc/:slug/link/start'); + assert.match(AUTH, /router\.get\('\/oidc\/:slug\/link\/start', requireAuth/, + 'the link must be startable only by someone already signed in — that is the proof of ownership'); + assert.match(body, /provider\.organizationId.*not_linkable/s, + "an organization's provider must never attach itself to a platform account"); + assert.match(body, /link: req\.user\.id/, + 'the account must come from the session, not from anything the browser can set'); +}); + +test('the linked account is taken from the transaction, never from the returned email', () => { + const cb = handler('get', '/oidc/:slug/callback'); + assert.match(cb, /WHERE id = \?'\)\.get\(tx\.link\)/, + 'the target account is looked up by tx.link (the session that started it)'); + // The email is still checked, but as a constraint on the link — not as the way the account is found. + assert.match(cb, /target\.email\.toLowerCase\(\) !== email/, 'email must match the account being linked'); + assert.match(cb, /link_email_mismatch/); +}); + +test('one provider subject cannot be linked to two accounts', () => { + const cb = handler('get', '/oidc/:slug/callback'); + assert.match(cb, /provider_id = \? AND auth_provider = \? AND id != \?/, + 'must check whether this provider identity already belongs to another account'); + assert.match(cb, /link_already_used/); +}); + +test('linking deletes the password — one credential at a time', () => { + const cb = handler('get', '/oidc/:slug/callback'); + assert.match(cb, /UPDATE users SET auth_provider = \?, provider_id = \?, password_hash = NULL/, + 'the password must be cleared in the same statement that attaches the provider'); +}); + +test('unlinking sets a password in the SAME statement', () => { + const body = handler('post', '/oidc/unlink'); + assert.match(body, /UPDATE users SET auth_provider = 'local', provider_id = NULL, password_hash = \?/, + 'unlink and set-password must be one write — never unlink first and set a password after'); + assert.match(body, /password\.length < passwordReset\.MIN_PASSWORD_LENGTH/, + 'the replacement password must meet the same minimum as a reset'); + assert.match(body, /auth_provider === 'local'/, 'refuse unlinking an account that has no provider'); +}); + +test('both link and unlink are recorded in the activity log', () => { + assert.match(handler('get', '/oidc/:slug/callback'), /logActivity\([^)]*'auth:sso_linked'/); + assert.match(handler('post', '/oidc/unlink'), /logActivity\([^)]*'auth:sso_unlinked'/); +}); + +test('link failures return to Settings, not the login page', () => { + const cb = handler('get', '/oidc/:slug/callback'); + assert.match(cb, /const fail = linking \? backToSettings : backToApp/, + 'an authenticated user must not be bounced to a login screen to be told the link failed'); + assert.match(AUTH, /function backToSettings\(res, params\)[\s\S]{0,200}#\/settings/); +}); + +test('login and link share one flow, so verification cannot drift between them', () => { + // beginOidc is the single place PKCE/state/nonce are minted; both entry points call it. + assert.match(AUTH, /async function beginOidc\(req, res, provider, extra = \{\}/); + const login = handler('get', '/oidc/:slug/start'); + const link = handler('get', '/oidc/:slug/link/start'); + assert.match(login, /beginOidc\(req, res, provider\)/); + assert.match(link, /beginOidc\(req, res, provider, \{ link: req\.user\.id \}/); +});