diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js
index 31ab7aa..25fc544 100644
--- a/frontend/js/i18n/en.js
+++ b/frontend/js/i18n/en.js
@@ -152,12 +152,14 @@ export default {
'sso.disabled': 'disabled',
'sso.domains_label': 'Email domains',
'sso.domains_heading': 'Sign-in domains',
+ 'sso.removed': 'Removed.',
+ 'sso.only_stranded': 'Single sign-on is now required.\n\nThese members are not at a verified domain, so they can no longer sign in at all:\n\n{list}\n\nVerify their domain, or remove them from this organization.',
'sso.only_heading': 'Require single sign-on',
'sso.only_help': 'When required, people at your verified domains can only sign in through your identity provider — a password will not work. Your provider keeps control of MFA and of removing access.',
'sso.only_on': 'Single sign-on is required for your verified domains.',
'sso.only_off': 'Password sign-in is still allowed alongside single sign-on.',
'sso.only_enable': 'Require single sign-on',
- 'sso.only_confirm': 'Require single sign-on for everyone at your verified domains?\n\nPasswords will stop working for them immediately. Turning this back off needs approval from the people who run this server, so make sure your identity provider is working first.',
+ 'sso.only_confirm': 'Require single sign-on for everyone at your verified domains?\n\nPasswords will stop working for them at their next sign-in; sessions already open continue until they expire. Turning this back off needs approval from the people who run this server, so make sure your identity provider is working first.',
'sso.only_needs_domain': 'Verify a sign-in domain first — otherwise nobody would be able to sign in.',
'sso.only_remove_help': 'Turning this off re-opens password sign-in, so it needs approval from the people who run this server.',
'sso.only_request': 'Request to stop requiring single sign-on',
@@ -208,7 +210,7 @@ export default {
'auth.sso_err_provider_unavailable': 'That provider is not reachable right now.',
'auth.sso_err_unknown_provider': 'That sign-in provider is not configured.',
'auth.sso_err_registration_disabled': 'New accounts are disabled on this instance.',
- 'auth.sso_err_account_exists_local': 'An account with this email already exists. Sign in with your password, then link your provider in Settings.',
+ 'auth.sso_err_account_exists_local': 'An account with this email already exists and uses a password. Sign in with your password instead.',
'auth.sso_err_subject_mismatch': 'This email is already linked to a different account at your provider.',
'auth.sso_err_server_error': 'Something went wrong completing sign-in.',
// Both of these used to fall through to "please try again", which is advice that can never work:
diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js
index 1fb86be..1ab6791 100644
--- a/frontend/js/views/admin.js
+++ b/frontend/js/views/admin.js
@@ -183,7 +183,9 @@ async function loadSsoOnlyRequests() {
section.style.display = 'none';
return;
}
- if (!requests.length) { section.style.display = 'none'; return; }
+ // Clear as well as hide: leaving the last decided request in the tree kept its live
+ // Approve/Reject listeners attached to a request that no longer exists.
+ if (!requests.length) { host.innerHTML = ''; section.style.display = 'none'; return; }
section.style.display = '';
host.innerHTML = requests.map((r) => `
@@ -369,7 +371,7 @@ async function loadUsers() {
${workspaceCell(u)}
`
: ''}
@@ -843,7 +843,20 @@ export async function render(container) {
// Confirmed, because it removes the only way in for everyone at these domains, and the way
// back needs the operator rather than this button.
if (!window.confirm(t('sso.only_confirm'))) return;
- if (await post(`/api/organizations/${orgId}/sso-only`)) { showToast(t('sso.only_on'), 'success'); await loadSso(); }
+ const r = await post(`/api/organizations/${orgId}/sso-only`);
+ if (r) {
+ showToast(t('sso.only_on'), 'success');
+ /*
+ * Name the people who just lost their only way in. The server reports them precisely so
+ * the admin finds out HERE rather than from a support ticket — and it was being thrown
+ * away, which made the whole warning pointless.
+ */
+ const stranded = r.stranded_members || [];
+ if (stranded.length) {
+ window.alert(t('sso.only_stranded', { list: stranded.join('\n') }));
+ }
+ await loadSso();
+ }
});
const reqBtn = box.querySelector('#ssoOnlyRequest');
@@ -1000,7 +1013,8 @@ export async function render(container) {
// The server's message is the useful one here — a bad issuer or a domain already claimed by
// another organization both say exactly what went wrong, and a generic failure would not.
if (!res.ok) { showToast(data.error || t('sso.save_failed'), 'error'); return false; }
- showToast(t('sso.saved'), 'success');
+ // "Saved" for a DELETE read as though nothing had been destroyed.
+ showToast(t(method === 'DELETE' ? 'sso.removed' : 'sso.saved'), 'success');
await loadSso();
return true;
} catch {
@@ -1227,25 +1241,30 @@ async function loadUsers() {
${users.map(u => `
-
`).join('')}
diff --git a/server/routes/admin.js b/server/routes/admin.js
index a8b267a..0e4741d 100644
--- a/server/routes/admin.js
+++ b/server/routes/admin.js
@@ -21,7 +21,9 @@ const { platformDefaultRow, HARDCODED_BRANDING, PLATFORM_DEFAULT_ID } = require(
// have no user/role-management power (#13).
// Same email shape the invite-create endpoint validates against (workspaces.js).
-const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+// Markup characters are not legal here. The looser form admitted < > " ' and an admin-
+// chosen email became stored XSS in the platform admin's user list.
+const EMAIL_RE = /^[^\s@<>"'`\\;,()\[\]]+@[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)+$/;
const WORKSPACE_ROLES = ['workspace_admin', 'workspace_editor', 'workspace_viewer'];
// Mirror the server-side minimum enforced by PUT /api/auth/me and register.
const MIN_PASSWORD_LENGTH = 8;
diff --git a/server/routes/auth.js b/server/routes/auth.js
index b8e7d24..23a28ff 100644
--- a/server/routes/auth.js
+++ b/server/routes/auth.js
@@ -206,26 +206,48 @@ router.post('/login', (req, res) => {
* anyone — so refusing on the domain alone reveals nothing new, and it reveals it identically
* for addresses that exist and addresses that do not.
*/
+ /*
+ * SSO-only refusal, arranged so it is neither an account-existence oracle NOR a way to brick the
+ * instance.
+ *
+ * Two constraints pull against each other. Answering 403 only for addresses that EXIST turned
+ * this into an enumeration oracle. But hoisting the check above the account lookup — the obvious
+ * cure — silently killed the platform_admin break-glass, because role is not known until the row
+ * is read. That is worse than it sounds: on a self-hosted instance the operator IS the org owner,
+ * and the guard that stops an admin locking themselves out GUARANTEES their address is inside the
+ * enforced set. Approving a removal request needs a platform admin to be signed in, so the
+ * recovery loop closed on itself and the only way back was a shell.
+ *
+ * Both hold if the operator is let through on a CORRECT PASSWORD and nothing else: every wrong
+ * answer is the identical 403, whether the address exists, does not exist, or belongs to the
+ * operator. The only observable difference needs the password, which an enumerator does not have.
+ */
const domainEnforced = (() => {
try { return oidcProviders.ssoOnlyForEmail(email); } catch (e) {
console.error('[login] SSO-only status unavailable, refusing password login:', e && e.message);
return { unavailable: true };
}
})();
- if (domainEnforced) {
+ const ssoRefusal = () => {
logFailedLogin(email, getClientIp(req), 'Password login refused: domain requires SSO');
return res.status(403).json({
error: 'Your organization requires single sign-on. Use the single sign-on button to continue.',
code: 'sso_required',
sso_start: '/api/auth/sso/start',
});
- }
+ };
const user = db.prepare('SELECT * FROM users WHERE email = ? AND auth_provider = ?').get(email.toLowerCase(), 'local');
if (!user) {
+ // An unknown address at an enforced domain answers exactly like a known one — see above.
+ if (domainEnforced) return ssoRefusal();
logFailedLogin(email, getClientIp(req), 'User not found');
return res.status(401).json({ error: 'Invalid email or password' });
}
+ // The break-glass: the operator may still sign in with a password at an enforced domain, but a
+ // WRONG password answers with the same refusal everyone else gets, so nothing is learned.
+ const breakGlass = domainEnforced && user.role === 'platform_admin' && !domainEnforced.unavailable;
+ if (domainEnforced && !breakGlass) return ssoRefusal();
/*
* SSO-ONLY. The organization that owns this VERIFIED domain requires its identity provider, so a
@@ -285,7 +307,13 @@ router.post('/login', (req, res) => {
return res.status(401).json({ error: 'Invalid email or password' });
}
- if (!bcrypt.compareSync(password, user.password_hash)) {
+ if (!user.password_hash || !bcrypt.compareSync(password, user.password_hash)) {
+ if (breakGlass) {
+ // Same answer as every other address at this domain: the operator's existence is not a fact
+ // this endpoint gives away to someone who cannot type their password.
+ loginLockout.recordFailure(user.id);
+ return ssoRefusal();
+ }
const rec = loginLockout.recordFailure(user.id);
if (rec.lockedUntil) logActivity(null, 'auth:login_locked', `${email} - locked after repeated failures`, null, getClientIp(req));
logFailedLogin(email, getClientIp(req), 'Wrong password');
@@ -1379,6 +1407,25 @@ router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => {
if (!already) {
db.prepare("INSERT INTO organization_members (organization_id, user_id, role) VALUES (?, ?, 'org_member')")
.run(provider.organizationId, user.id);
+ /*
+ * ⚠️ And a WORKSPACE, or they land somewhere else entirely.
+ *
+ * ensureDefaultOrgForUser (below) looks for a workspace_members row, not an
+ * organization_members one — so writing only the org membership left it finding nothing and
+ * minting the user a brand-new personal organization, which then became their CURRENT one.
+ * The customer's Members page still read "Members (1)": their staff signed in successfully
+ * and were invisible to the admin, managing a private org of their own. That is precisely
+ * the outcome the comment above says this code exists to prevent.
+ */
+ const target = db.prepare(
+ 'SELECT id FROM workspaces WHERE organization_id = ? ORDER BY created_at LIMIT 1'
+ ).get(provider.organizationId);
+ if (target) {
+ db.prepare("INSERT OR IGNORE INTO workspace_members (workspace_id, user_id, role) VALUES (?, ?, 'workspace_viewer')")
+ .run(target.id, user.id);
+ } else {
+ console.warn(`[oidc] org ${provider.organizationId} has no workspace; ${user.email} has no place to land`);
+ }
// (userId, action, details, deviceId, ipAddress, workspaceId) — the org id is NOT the 4th
// arg; it was landing in device_id, which has no FK to catch it.
logActivity(user.id, 'org_sso_joined', `via ${provider.name} org=${provider.organizationId}`, null, getClientIp(req));
@@ -1505,7 +1552,33 @@ function upsertFederatedUser({ claims, email, provider, req }) {
}
if (existing.auth_provider !== provider.slug) {
- if (existing.password_hash) return { error: 'account_exists_local' };
+ /*
+ * An account WITH a password is normally never taken over by an SSO login — the owner proves
+ * control by signing in locally. There is exactly one case where refusing is worse than
+ * adopting, and it is a trap the previous design walked into:
+ *
+ * an organization that REQUIRES single sign-on, asserting an address at a domain it has PROVED
+ * by DNS. There, the password is already refused by policy (403 sso_required), so refusing the
+ * SSO login too shuts both doors — the member cannot sign in by any route, password reset
+ * "succeeds" and changes nothing, and if that member is the last org admin the removal request
+ * that would undo it can never be filed. A review locked an admin out of their own tenant this
+ * way, with no route back short of SQL.
+ *
+ * Adopting is safe precisely because of what the two conditions already establish: the tenant
+ * proved control of the domain (a DNS record they published), and the confinement check above
+ * has already refused anything outside it. This is what every hosted identity product does with
+ * a verified domain, and it is the only reading under which "requires single sign-on" is a
+ * statement about the domain rather than about whoever happened to register first.
+ */
+ const ssoOnlyAdoption = !!provider.organizationId
+ && !!oidcProviders.ssoOnlyForEmail(email)
+ && emailAllowedForProvider(provider, email);
+ if (existing.password_hash && !ssoOnlyAdoption) return { error: 'account_exists_local' };
+ if (existing.password_hash && ssoOnlyAdoption) {
+ // The password is dead by policy; clear it rather than leave a credential nobody may use.
+ db.prepare('UPDATE users SET password_hash = NULL WHERE id = ?').run(existing.id);
+ console.log(`[oidc] ${provider.slug} adopted ${email} (organization requires SSO for its verified domain)`);
+ }
/*
* `password_hash IS NULL` was the wrong test for "safe to relink". Every SSO-created account has
* a null password, so it meant "any federated account may be adopted by whichever provider spoke
diff --git a/server/routes/org-sso.js b/server/routes/org-sso.js
index a821fdc..d9a3802 100644
--- a/server/routes/org-sso.js
+++ b/server/routes/org-sso.js
@@ -262,8 +262,8 @@ function notifyOperatorOfClaim(req, { domains, orgId, providerName }) {
'8 hours if it is not proved. No action is needed unless this looks wrong.',
].join('\n');
const subject = domains.length === 1
- ? `[ScreenTinker] SSO domain claimed: ${domains[0]}`
- : `[ScreenTinker] ${domains.length} SSO domains claimed`;
+ ? `SSO domain claimed: ${domains[0]}`
+ : `${domains.length} SSO domains claimed`; // services/email.js adds the [ScreenTinker] prefix
for (const a of admins) {
Promise.resolve(emailSvc.sendEmail({ to: a.email, subject, text: body }))
.catch((e) => console.error('[org-sso] claim notification failed:', e && e.message));
@@ -307,7 +307,7 @@ function notifyOperatorOfRemovalRequest(req, { id, orgId, orgName, reason }) {
for (const a of admins) {
Promise.resolve(emailSvc.sendEmail({
to: a.email,
- subject: `[ScreenTinker] Approval needed: stop requiring SSO for ${orgName || orgId}`,
+ subject: `Approval needed: stop requiring SSO for ${orgName || orgId}`,
text: body,
})).catch((e) => console.error('[org-sso] removal notification failed:', e && e.message));
}
diff --git a/server/routes/workspaces.js b/server/routes/workspaces.js
index 86f092a..fdfb0ff 100644
--- a/server/routes/workspaces.js
+++ b/server/routes/workspaces.js
@@ -14,7 +14,9 @@ const { sendEmail } = require('../services/email');
const NAME_MAX = 80;
const SLUG_MAX = 60;
const SLUG_RE = /^[a-z0-9]+(?:-[a-z0-9]+)*$/;
-const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+// Markup characters are not legal here. The looser form admitted < > " ' and an admin-
+// chosen email became stored XSS in the platform admin's user list.
+const EMAIL_RE = /^[^\s@<>"'`\\;,()\[\]]+@[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)+$/;
const WORKSPACE_ROLES = ['workspace_admin', 'workspace_editor', 'workspace_viewer'];
// Operational policy - env-configurable with conservative defaults. Restart
diff --git a/server/server.js b/server/server.js
index b3cf602..981fd25 100644
--- a/server/server.js
+++ b/server/server.js
@@ -184,6 +184,29 @@ app.post('/api/stripe/webhook', express.raw({ type: 'application/json' }), strip
// 12mb so AI-designed signs with embedded generated images (base64 data URLs)
// can be published. #41 follow-up: upload generated images to the content store
// and reference by URL instead of embedding, to keep widget configs small.
+/*
+ * Collapse duplicate slashes in the PATH before anything routes on it.
+ *
+ * Express normalises the mount boundary for a router, so `/api/auth//login` still reaches the login
+ * handler — but `app.use('/api/auth/login', rateLimit(...))` does NOT match it, so the limiter
+ * never runs. One extra slash therefore removed EVERY per-endpoint limit under /api/auth: unlimited
+ * password guesses (a review got a real session after 60 unthrottled attempts), unlimited TOTP
+ * codes, unlimited password-reset mail to any address, and the SSO discovery cap that exists to
+ * stop customer enumeration. It also made the per-account lockout a denial-of-service tool.
+ *
+ * Fixing it inside the limiter's key is not enough — the middleware is never invoked. The path has
+ * to be one canonical thing before routing, which is what this does. Query and body are untouched.
+ */
+app.use((req, res, next) => {
+ const q = req.url.indexOf('?');
+ const path = q === -1 ? req.url : req.url.slice(0, q);
+ if (path.includes('//')) {
+ const collapsed = path.replace(/\/{2,}/g, '/');
+ req.url = q === -1 ? collapsed : collapsed + req.url.slice(q);
+ }
+ next();
+});
+
app.use(express.json({ limit: '12mb' }));
const { sanitizeBody } = require('./middleware/sanitize');
app.use(sanitizeBody);
@@ -551,7 +574,12 @@ const LIMIT_PATH_SHAPES = [
[/^\/api\/organizations\/sso-only\/removal-requests\/[^/]+\/[^/]+$/, () => '/api/organizations/sso-only/removal-requests/:id/:decision'],
[/^\/api\/organizations\/sso-only\/removal-requests$/, () => '/api/organizations/sso-only/removal-requests'],
[/^\/api\/organizations\/[^/]+\/sso-only\/removal-request\/[^/]+$/, () => '/api/organizations/:id/sso-only/removal-request/:id'],
+ [/^\/api\/organizations\/[^/]+\/sso-only\/removal-request$/, () => '/api/organizations/:id/sso-only/removal-request'],
[/^\/api\/organizations\/[^/]+\/sso-only$/, () => '/api/organizations/:id/sso-only'],
+ // The reset/target routes mint a bucket per TARGET without this, which is the same
+ // caller-chosen-segment defect, at the mount next door.
+ [/^\/api\/auth\/users\/[^/]+\/(.+)$/, (m) => `/api/auth/users/:id/${m[1]}`],
+ [/^\/api\/content\/[^/]+$/, () => '/api/content/:id'],
[/^\/api\/organizations\/[^/]+\/sso\/[^/]+\/domains\/[^/]+\/verify$/, () => '/api/organizations/:id/sso/:id/domains/:domain/verify'],
[/^\/api\/organizations\/[^/]+\/sso\/[^/]+\/test$/, () => '/api/organizations/:id/sso/:id/test'],
[/^\/api\/organizations\/[^/]+\/sso\/[^/]+$/, () => '/api/organizations/:id/sso/:id'],