SSO: build the operator approval screen, and close the last of the QA findings

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 `<img src=x onerror=alert(1)>` 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
This commit is contained in:
ScreenTinker 2026-08-11 08:48:53 -05:00
parent 983bee31b7
commit 355b7a2b86
7 changed files with 156 additions and 8 deletions

View file

@ -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: <value>`. A review typed `<img src=x onerror=alert(1)>` 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, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
.replace(/"/g, '&quot;').replace(/'/g, '&#39;');
}
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' ? '<circle cx="12" cy="12" r="10"/><line x1="15" y1="9" x2="9" y2="15"/><line x1="9" y1="9" x2="15" y2="15"/>' :
'<circle cx="12" cy="12" r="10"/><line x1="12" y1="16" x2="12" y2="12"/><line x1="12" y1="8" x2="12.01" y2="8"/>'}
</svg>
<span>${message}</span>
<span>${esc(message)}</span>
`;
container.appendChild(toast);
setTimeout(() => {

View file

@ -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',

View file

@ -79,6 +79,15 @@ export async function render(container) {
</div>
</div>
<!-- Single sign-on removal approvals. First, because it is the only screen on this page an
operator is DIRECTED to by an email, and because a tenant is locked out of their own
product while it sits here. -->
<div class="settings-section" id="ssoOnlySection" style="display:none">
<h3>${t('admin.sso_only.title')}</h3>
<p style="color:var(--text-muted);font-size:12px;margin-bottom:12px">${t('admin.sso_only.desc')}</p>
<div id="ssoOnlyRequests"><p style="color:var(--text-muted)">${t('common.loading')}</p></div>
</div>
<div class="settings-section">
<h3>${t('admin.all_users')}</h3>
<div id="allUsersTable"><p style="color:var(--text-muted)">${t('common.loading')}</p></div>
@ -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) => `
<div style="border:1px solid var(--border);border-radius:var(--radius);padding:12px;margin-bottom:8px">
<div><strong>${esc(r.organization_name || r.organization_id)}</strong></div>
<div style="font-size:12px;color:var(--text-muted);margin-top:2px">
${esc(t('admin.sso_only.requested_by', { who: r.requested_by_email || 'unknown' }))}
</div>
${r.reason ? `<div style="font-size:12px;margin-top:6px">${esc(r.reason)}</div>` : ''}
<div style="font-size:12px;color:var(--warning,#b45309);margin-top:8px">${esc(t('admin.sso_only.effect'))}</div>
<div style="display:flex;gap:6px;flex-wrap:wrap;margin-top:10px">
<button class="btn btn-danger btn-sm" data-sso-approve="${esc(r.id)}">${esc(t('admin.sso_only.approve'))}</button>
<button class="btn btn-secondary btn-sm" data-sso-reject="${esc(r.id)}">${esc(t('admin.sso_only.reject'))}</button>
</div>
</div>`).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;

View file

@ -105,13 +105,19 @@ export async function render(container) {
<input type="email" id="loginEmail" class="input" placeholder="${t('auth.placeholder_email')}" autocomplete="email">
</div>
<div class="form-group">
<label id="loginPasswordLabel">${t('auth.password')}</label>
<label id="loginPasswordLabel" for="loginPassword">${t('auth.password')}</label>
<input type="password" id="loginPassword" class="input" placeholder="${t('auth.placeholder_password')}" autocomplete="current-password">
<!-- Filled in only when the typed email belongs to an organization that has configured
its own identity provider. A customer's IdP is never listed to everyone: the button
appears for the people it belongs to and nobody else, which also keeps the customer
list off the login page. -->
<div id="orgSsoSlot" style="display:none;margin-bottom:12px"></div>
<input type="password" id="loginPassword" class="input" placeholder="${t('auth.placeholder_password')}" autocomplete="current-password">
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. -->
<div id="orgSsoSlot" style="display:none;margin-top:12px"></div>
</div>
${isSetup ? `
<div class="form-group">
@ -182,7 +188,7 @@ export async function render(container) {
<div id="ssoBlock">
${(config.providers || []).length ? `
<div style="display:flex;align-items:center;gap:12px;margin:20px 0">
<div id="ssoDivider" style="display:flex;align-items:center;gap:12px;margin:20px 0">
<hr style="flex:1;border-color:var(--border)">
<span style="color:var(--text-muted);font-size:12px">${t('auth.divider_or')}</span>
<hr style="flex:1;border-color:var(--border)">
@ -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 <p>; hide the wrapper so no empty gap is left.
const forgot = document.getElementById('forgotLink');
if (forgot) {

View file

@ -706,7 +706,7 @@ export async function render(container) {
const origin = `${window.location.protocol}//${window.location.host}`;
listEl.innerHTML = providers.map((p) => `
<div style="border:1px solid var(--border);border-radius:var(--radius);padding:12px;margin-bottom:8px">
<div style="display:flex;justify-content:space-between;align-items:center;gap:8px">
<div style="display:flex;justify-content:space-between;align-items:center;gap:8px;flex-wrap:wrap">
<div>
<strong>${esc(p.name)}</strong>
${p.enabled ? '' : `<span style="font-size:11px;color:var(--text-muted)"> — ${esc(t('sso.disabled'))}</span>`}
@ -716,7 +716,9 @@ export async function render(container) {
? `<div style="font-size:12px;color:var(--warning,#b45309);margin-top:2px">⚠️ ${esc(t('sso.unverified_warning'))}</div>`
: ''}
</div>
<div style="display:flex;gap:6px;flex-shrink:0">
<!-- wrap, do not shrink-to-clip: at 375px this row ran to x=417 on a 375px viewport and
the page does not scroll horizontally, so "Remove" was simply unreachable. -->
<div style="display:flex;gap:6px;flex-wrap:wrap;justify-content:flex-end">
<button class="btn btn-secondary btn-sm" data-sso-test="${esc(p.id)}">${esc(t('sso.test'))}</button>
<button class="btn btn-secondary btn-sm" data-sso-edit="${esc(p.id)}">${esc(t('sso.edit'))}</button>
<button class="btn btn-secondary btn-sm" data-sso-toggle="${esc(p.id)}" data-enabled="${p.enabled ? '1' : '0'}">

View file

@ -104,6 +104,28 @@ router.post('/register', (req, res) => {
if (!email || !password) return res.status(400).json({ error: 'Email and password required' });
if (password.length < 8) return res.status(400).json({ error: 'Password must be at least 8 characters' });
/*
* An organization that requires single sign-on must not have password accounts created at its
* domains not even by a stranger. Two things went wrong without this: the account was issued a
* working session immediately (a bypass), and it then held the address forever, because
* upsertFederatedUser refuses to adopt a row that has a password. Registering ceo@acme.test
* before the real CEO's first login left that address dead in BOTH directions with no
* self-service way out.
*/
let ssoOnlyOrg = null;
try {
ssoOnlyOrg = oidcProviders.ssoOnlyForEmail(email);
} catch (e) {
console.error('[register] SSO-only status unavailable, refusing registration:', e && e.message);
ssoOnlyOrg = { unavailable: true };
}
if (ssoOnlyOrg) {
return res.status(403).json({
error: 'That domain uses single sign-on. Sign in with your organization instead of creating a password.',
code: 'sso_required',
});
}
const existing = db.prepare('SELECT id FROM users WHERE email = ?').get(email.toLowerCase());
if (existing) return res.status(409).json({ error: 'Email already registered' });

View file

@ -555,6 +555,19 @@ function rateLimit(windowMs, maxRequests) {
.replace(/\/+$/, '') // ignore a trailing slash
.toLowerCase()
.replace(/^(\/api\/auth\/oidc)\/[^/]+/, '$1') // the slug is not a distinct endpoint
/*
* /api/organizations/<orgId>/... carries THREE caller-chosen segments (org id, provider
* id, domain). Folding only the OIDC slug left this mount with a fresh bucket per request:
* a review measured 120 unauthenticated calls with unique org ids and got zero 429s, while
* the same path 120 times correctly produced 60. The limiter runs before requireAuth, so an
* anonymous caller could mint buckets for free and this limit exists specifically to bound
* outbound OIDC discovery and live DNS lookups.
*
* Ids are collapsed to a placeholder so the SHAPE of the endpoint is the key.
*/
.replace(/^(\/api\/organizations)\/[^/]+/, '$1/:id')
.replace(/^(\/api\/organizations\/:id\/sso)\/[^/]+/, '$1/:id')
.replace(/(\/domains)\/[^/]+/, '$1/:id')
|| '/';
const key = getClientIp(req) + normalisedPath;
const now = Date.now();