From 355b7a2b869113304d1ad3052a08251d55abcd7e Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 11 Aug 2026 08:48:53 -0500 Subject: [PATCH] SSO: build the operator approval screen, and close the last of the QA findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The approval workflow had no front door. The notification email told the operator to "review it in ScreenTinker under Admin" and that screen did not exist — the only way to approve was curl, while the tenant sat locked out of their own product. Admin now leads with a removal-request section: who asked, for which organization, the reason they gave, what approving does, and Approve/Reject. It hides itself when the queue is empty. Approving is confirmed; rejecting is not, because rejecting only leaves the safe state. REGISTRATION BYPASSED SSO-ONLY AND SQUATTED ADDRESSES /register had no domain awareness: it issued a working session at an SSO-only domain, and the account then held that address forever, because an SSO login will not adopt a row that has a password. Registering ceo@acme.test before the real CEO's first login left the address dead in both directions with no self-service way out. Refused now, and "Create Account" is hidden on the login page for those domains — it was the only action left on the card, so the page was inviting the one thing that cannot work. THE NEW RATE LIMIT WAS DECORATIVE /api/organizations carries three caller-chosen segments, and only the OIDC slug was folded — so every request minted its own bucket. Measured: 120 calls with unique org ids produced ZERO 429s, unauthenticated, against the limit that exists to bound outbound discovery and live DNS. Now 60/60. The general problem was named in the previous commit's own comment and then not applied to the mount it added. XSS IN THE TOAST showToast built innerHTML from server strings, including ones that reflect input verbatim — a reviewer typed `` as an issuer and got script execution in the admin's session. Escaped. ALSO - the org SSO button sat BETWEEN the "Password" label and its input, so the label described the button and the field had none; moved below the input, with a for= - the OR divider survived when the providers under it were hidden - provider action buttons were clipped off-screen at 375px with no way to scroll to them — "Remove" was unreachable; the row wraps now 1609 tests. Verified in real Chrome: 13/13 on the approval loop and the login states, including approving a request and watching password login re-open for that org. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A --- frontend/js/components/toast.js | 14 ++++++- frontend/js/i18n/en.js | 10 +++++ frontend/js/views/admin.js | 72 +++++++++++++++++++++++++++++++++ frontend/js/views/login.js | 27 ++++++++++--- frontend/js/views/settings.js | 6 ++- server/routes/auth.js | 22 ++++++++++ server/server.js | 13 ++++++ 7 files changed, 156 insertions(+), 8 deletions(-) diff --git a/frontend/js/components/toast.js b/frontend/js/components/toast.js index 966f9e6..2bff52a 100644 --- a/frontend/js/components/toast.js +++ b/frontend/js/components/toast.js @@ -1,3 +1,15 @@ +/* + * ⚠️ Messages are ESCAPED. This builds innerHTML, and callers pass server error strings straight + * in — including ones that reflect user input verbatim, such as the OIDC issuer in + * `not a URL: `. A review typed `` as an issuer and got script + * execution in the admin's own session. A toast is a place text goes, never markup. + */ +function esc(v) { + return String(v == null ? '' : v) + .replace(/&/g, '&').replace(//g, '>') + .replace(/"/g, '"').replace(/'/g, '''); +} + export function showToast(message, type = 'info', duration = 4000) { const container = document.getElementById('toastContainer'); const toast = document.createElement('div'); @@ -10,7 +22,7 @@ export function showToast(message, type = 'info', duration = 4000) { type === 'error' ? '' : ''} - ${message} + ${esc(message)} `; container.appendChild(toast); setTimeout(() => { diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 0a2ae83..31ab7aa 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1413,6 +1413,16 @@ export default { 'admin.orgs.ws_deleted': 'Workspace "{name}" deleted', 'admin.access_denied': 'Access Denied', 'admin.access_denied_desc': 'Platform admin access required.', + 'admin.sso_only.title': 'Single sign-on removal requests', + 'admin.sso_only.desc': 'An organization has asked to stop requiring its identity provider. Until you approve, nothing changes for them.', + 'admin.sso_only.requested_by': 'Requested by {who}', + 'admin.sso_only.effect': 'Approving re-opens password sign-in for everyone at this organization\u2019s verified domains.', + 'admin.sso_only.approve': 'Approve removal', + 'admin.sso_only.reject': 'Reject', + 'admin.sso_only.confirm': 'Re-open password sign-in for this organization?\n\nTheir identity provider will no longer be the only way in. Approve only if you are satisfied the request is genuine.', + 'admin.sso_only.approved': 'Approved. Password sign-in is re-opened for that organization.', + 'admin.sso_only.rejected': 'Rejected. Single sign-on is still required.', + 'admin.sso_only.failed': 'That did not work.', 'admin.all_users': 'All Users', 'admin.plans': 'Subscription Plans', 'admin.col.accounts': 'Accounts', diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index 0647949..1e7856a 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -79,6 +79,15 @@ export async function render(container) { + + +

${t('admin.all_users')}

${t('common.loading')}

@@ -137,6 +146,7 @@ export async function render(container) { loadUsers(); loadOrgs(); + loadSsoOnlyRequests(); loadBranding(); loadPlans(); loadSystem(); @@ -146,6 +156,68 @@ export async function render(container) { // #36: list organizations with owner + resource counts; platform admin can // cascade-delete an org or an individual workspace (type-the-name confirm). +/* + * Pending "stop requiring single sign-on" requests. + * + * The notification email tells the operator to review this under Admin, and for a while it did not + * exist — the only way to approve was curl, while the customer sat locked out. The section hides + * itself when there is nothing pending so it is never noise. + */ +async function loadSsoOnlyRequests() { + const section = document.getElementById('ssoOnlySection'); + const host = document.getElementById('ssoOnlyRequests'); + if (!section || !host) return; + // NB: `api` is a map of named methods, not a generic client — there is no api.get(), and calling + // one silently hid this whole section behind the catch below. + const authed = (path, init = {}) => fetch(`/api${path}`, { + ...init, + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...(init.headers || {}) }, + }); + + let requests = []; + try { + const res = await authed('/organizations/sso-only/removal-requests'); + if (!res.ok) throw new Error(String(res.status)); + requests = (await res.json()).requests || []; + } catch { + section.style.display = 'none'; + return; + } + if (!requests.length) { section.style.display = 'none'; return; } + section.style.display = ''; + + host.innerHTML = requests.map((r) => ` +
+
${esc(r.organization_name || r.organization_id)}
+
+ ${esc(t('admin.sso_only.requested_by', { who: r.requested_by_email || 'unknown' }))} +
+ ${r.reason ? `
${esc(r.reason)}
` : ''} +
${esc(t('admin.sso_only.effect'))}
+
+ + +
+
`).join(''); + + const decide = async (id, decision) => { + try { + const res = await authed(`/organizations/sso-only/removal-requests/${id}/${decision}`, { method: 'POST', body: '{}' }); + if (!res.ok) throw new Error(((await res.json().catch(() => ({}))).error) || String(res.status)); + showToast(t(decision === 'approve' ? 'admin.sso_only.approved' : 'admin.sso_only.rejected'), 'success'); + await loadSsoOnlyRequests(); + } catch (e) { + showToast((e && e.message) || t('admin.sso_only.failed'), 'error'); + } + }; + // Approving RE-OPENS password sign-in for a whole organization, so it is confirmed; rejecting + // only leaves the safe state in place and is not. + host.querySelectorAll('[data-sso-approve]').forEach((b) => b.addEventListener('click', () => { + if (window.confirm(t('admin.sso_only.confirm'))) decide(b.dataset.ssoApprove, 'approve'); + })); + host.querySelectorAll('[data-sso-reject]').forEach((b) => b.addEventListener('click', () => decide(b.dataset.ssoReject, 'reject'))); +} + async function loadOrgs() { const el = document.getElementById('orgsTable'); if (!el) return; diff --git a/frontend/js/views/login.js b/frontend/js/views/login.js index 9c0552d..35f330e 100644 --- a/frontend/js/views/login.js +++ b/frontend/js/views/login.js @@ -105,13 +105,19 @@ export async function render(container) {
- + + - - + list off the login page. + + ⚠️ BELOW the input, inside the same group. Above it, the button sat between the + "Password" label and its field — so the label described the SSO button and the + password box had none at all. It has to stay INSIDE the group, because hiding the + group is how the password is hidden and the button must survive that... which is + exactly why setPasswordVisible() hides the FIELD, never the container. --> +
${isSetup ? `
@@ -182,7 +188,7 @@ export async function render(container) {
${(config.providers || []).length ? ` -
+
${t('auth.divider_or')}
@@ -533,6 +539,17 @@ function setupHandlers(config, isSetup) { */ const instance = document.getElementById('instanceProviders'); if (instance) instance.style.display = show; + /* + * "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. + */ + 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. const forgot = document.getElementById('forgotLink'); if (forgot) { diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index d08da70..cc80010 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -706,7 +706,7 @@ export async function render(container) { const origin = `${window.location.protocol}//${window.location.host}`; listEl.innerHTML = providers.map((p) => `

-
+
${esc(p.name)} ${p.enabled ? '' : ` — ${esc(t('sso.disabled'))}`} @@ -716,7 +716,9 @@ export async function render(container) { ? `
⚠️ ${esc(t('sso.unverified_warning'))}
` : ''}
-
+ +