From 94e1273ecd1b72ed3e35ca331e8a52a0b5525f18 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 11 Aug 2026 10:36:32 -0500 Subject: [PATCH] Fix: per-organization SSO was blocked by our own CSP and had never worked in a browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit THE HEADLINE FEATURE COULD NOT RUN. "Continue with single sign-on" was a
that redirected on to the customer's identity provider. Chrome applies `form-action` across the WHOLE redirect chain, and the dashboard sets `form-action 'self'`, so the hop to the provider was aborted — silently. The user clicked and nothing happened: no navigation, no toast, no spinner, a byte-identical page. Combined with SSO-only it was a total lockout: password login answers 403 "use the single sign-on button", pointing at a button that cannot work. Every test I ran on this feature checked the button RENDERED. None clicked it. The provider origins cannot be allowlisted — customers supply them at runtime. So the page now fetches the destination and navigates itself; a script-initiated navigation is not governed by form-action. The redirect answer is kept for a caller without JavaScript, where the chain stays same-origin until the provider takes over. The slug in the JSON is not a disclosure: following the old redirect put it in the address bar and history anyway. Verified in Chrome: the provider start endpoint is reached, zero CSP violations, zero aborted requests — where before it was ERR_ABORTED plus a console violation. STORED XSS IN THE PLATFORM ADMIN'S SESSION admin.js interpolated user name, email and auth_provider into innerHTML unescaped, and /register accepted an address whose local part was an img tag with an onerror handler — no spaces, so it slipped the asserted-email check too. A reviewer registered anonymously and got script execution on #/admin: the page operators are now emailed to. Escaped, and registration refuses addresses that are not addresses. (The render bug predates this branch; the reachability and the significance of that screen do not.) ALSO - the org SSO button is secondary while a password still works; two identical blue buttons stacked sent people to their IdP by muscle memory after typing a password. 1609 tests, three clean runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A --- frontend/js/views/admin.js | 12 ++++++++--- frontend/js/views/login.js | 41 ++++++++++++++++++++++++++++++++------ server/routes/auth.js | 39 ++++++++++++++++++++++++++++++++---- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index 1e7856a..1fb86be 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -348,11 +348,17 @@ async function loadUsers() { ${users.map(u => ` -
${u.name || u.email}
${u.email}
- ${u.auth_provider} + +
${esc(u.name || u.email)}
${esc(u.email)}
+ ${esc(u.auth_provider)} ${u.last_login ? new Date(u.last_login * 1000).toLocaleString() : t('common.never')} - ${PLATFORM_ROLE_OPTIONS.map(r => ``).join('')} diff --git a/frontend/js/views/login.js b/frontend/js/views/login.js index e31965d..f1585c0 100644 --- a/frontend/js/views/login.js +++ b/frontend/js/views/login.js @@ -601,17 +601,46 @@ function setupHandlers(config, isSetup) { * mapping again on submit, so the slug is never published to the page. POST keeps the address * out of the URL, browser history and any Referer the provider's page would send. */ + /* + * A BUTTON that fetches and then navigates — not a form that submits. + * + * The dashboard's CSP is `form-action 'self'`, and Chrome applies it across the whole + * redirect chain, so a form POST that 302s on to the customer's identity provider was + * ABORTED with nothing shown to the user at all. The provider origins cannot be allowlisted + * because customers supply them. A script-initiated navigation is not covered by + * form-action, so the page asks the server where to go and goes there. + * + * Styled secondary: "Sign In" is the primary action while a password still works, and two + * identical blue buttons stacked one above the other sent people to their IdP by muscle + * memory after typing a password. + */ slot.innerHTML = ` - - - - +
${t('auth.sso_org_hint')}
`; slot.style.display = ''; + + const btn = slot.querySelector('#orgSsoBtn'); + if (btn) btn.addEventListener('click', async () => { + btn.disabled = true; + try { + const r = await fetch('/api/auth/sso/start', { + method: 'POST', + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ email }), + }); + const body = await r.json().catch(() => ({})); + if (!r.ok || !body.start_url) throw new Error(body.error || `start ${r.status}`); + window.location.assign(body.start_url); + } catch { + btn.disabled = false; + showError(t('auth.sso_err_provider_unavailable')); + } + }); } catch { // A failed lookup must never block a password login — the form still works, and the password // box comes back rather than leaving someone staring at a form with no way to submit it. diff --git a/server/routes/auth.js b/server/routes/auth.js index 27338e8..b8e7d24 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -102,6 +102,15 @@ router.post('/register', (req, res) => { } const { email, password, name, createOrg } = req.body; if (!email || !password) return res.status(400).json({ error: 'Email and password required' }); + /* + * Registration accepted anything with an @ in it, so `@acme.test` + * became a real row — markup with no spaces, which is why it also slipped the asserted-email + * check. Rendering is escaped now, but an address that is not an address has no business being + * stored: it is displayed on operator screens, put in emails, and compared against domains. + */ + if (!ASSERTED_EMAIL_RE.test(String(email).toLowerCase()) || /[<>"'`\\]/.test(String(email))) { + return res.status(400).json({ error: 'Enter a valid email address' }); + } if (password.length < 8) return res.status(400).json({ error: 'Password must be at least 8 characters' }); /* @@ -1168,10 +1177,32 @@ router.get('/sso/discover', (req, res) => { */ router.post('/sso/start', express.urlencoded({ extended: false }), (req, res) => { const provider = oidcProviders.forEmail((req.body && req.body.email) || req.query.email); - // An unknown domain is answered exactly like a known one that is disabled: back to the login page - // with nothing learned. - if (!provider) return res.redirect('/app#/login?sso_error=unknown_provider'); - res.redirect(`/api/auth/oidc/${encodeURIComponent(provider.slug)}/start`); + /* + * ⚠️ ANSWER WITH JSON when the page asks for it, rather than a redirect. + * + * This used to be a plain
that 302'd on to the provider. Chrome applies + * `form-action` to the WHOLE redirect chain, and the dashboard's CSP sets `form-action 'self'` + * (server.js), so the hop to the identity provider was aborted — silently. The user clicked + * "Continue with single sign-on" and NOTHING happened: no navigation, no error, an unchanged + * page. Per-organization SSO, the whole point of this feature, could never work in a browser. + * + * The origins cannot simply be allowlisted: they are supplied by customers at runtime. So the + * page fetches this, then navigates itself — a script-initiated navigation is not governed by + * form-action. The redirect is kept for a caller without JavaScript, where the chain is + * same-origin up to the point the provider's own page takes over. + * + * The slug in the answer is not a disclosure: following the old redirect put it in the address + * bar, the network log and history anyway. What stays private is the mapping for a domain the + * caller cannot name — an unknown domain answers exactly like a disabled one. + */ + const wantsJson = String(req.get('accept') || '').includes('application/json'); + if (!provider) { + if (wantsJson) return res.status(404).json({ error: 'unknown_provider', code: 'unknown_provider' }); + return res.redirect('/app#/login?sso_error=unknown_provider'); + } + const startUrl = `/api/auth/oidc/${encodeURIComponent(provider.slug)}/start`; + if (wantsJson) return res.json({ start_url: startUrl }); + res.redirect(startUrl); }); router.get('/oidc/:slug/start', asyncRoute(async (req, res) => {