diff --git a/README.md b/README.md index 47179a2..e3e334f 100644 --- a/README.md +++ b/README.md @@ -318,31 +318,223 @@ advertising it — put the account on the hidden plan and it simply gets those l above deliberately lists hidden plans too (marked as such), because the previous behaviour was that a hidden plan was invisible to the operator as well as the customer. -#### Google OAuth +#### Single sign-on (OpenID Connect) -Let users sign in with Google. +Any OIDC provider works — Google, Microsoft/Entra, Okta, Auth0, Keycloak, Authentik, Zitadel — through +one flow: **Authorization Code with PKCE, run server-side**. The browser never talks to the provider +directly, so there is no SDK to load and no third-party script origin to allow in the CSP. -1. Create a project in [Google Cloud Console](https://console.cloud.google.com) -2. Enable the Google Identity API -3. Create OAuth 2.0 credentials (web application) -4. Add `https://yourdomain.com` as an authorized origin +Every login is verified as an **ID token**: signature against the provider's published JWKS, +`iss` exactly as discovered, `aud` (and `azp`) matching your client, `exp`, and a `nonce` this server +generated for that specific login. An access token is never accepted as proof of identity. + +Set the redirect URI at your provider to: + +``` +https://yourdomain.com/api/auth/oidc//callback +``` + +Set `APP_URL` so that origin is pinned — the redirect URI must match your provider's registration +exactly, and deriving it from the request `Host` would both break behind a second hostname and take +its value from the caller. + +**Google** and **Microsoft** need only the variables this README has always documented; their issuer +is filled in for you and their slugs are `google` and `microsoft`: | Variable | Description | |----------|-------------| -| `GOOGLE_CLIENT_ID` | Your Google OAuth client ID | +| `GOOGLE_CLIENT_ID` | OAuth 2.0 client ID from [Google Cloud Console](https://console.cloud.google.com) | +| `MICROSOFT_CLIENT_ID` | Application (client) ID from the [Azure portal](https://portal.azure.com) | +| `MICROSOFT_TENANT_ID` | **Your tenant GUID — required.** `common`/`organizations` are refused | -#### Microsoft OAuth +⚠️ **Multi-tenant Microsoft (`common`) is deliberately refused, and Microsoft sign-in stays disabled +until you set a tenant GUID.** Two reasons that point the same way. It cannot work: Microsoft's +multi-tenant metadata advertises the literal template `https://login.microsoftonline.com/{tenantid}/v2.0`, +so the issuer never matches and every login fails anyway. And the obvious fix is dangerous — accepting +that template means accepting tokens from *every* Azure tenant, which is +[nOAuth](https://www.descope.com/blog/post/noauth): any tenant admin can set an arbitrary, unverified +`email` on one of their own users and be issued a session as that address. Safe multi-tenant support +needs per-tenant pinning (validate `tid` against an allowlist, key accounts on `oid`+`tid` rather than +email) and is not implemented. -Let users sign in with Microsoft/Azure AD. +**Any other provider** is added by slug: -1. Register an app in [Azure Portal](https://portal.azure.com/#blade/Microsoft_AAD_RegisteredApps) -2. Add a web redirect URI: `https://yourdomain.com` -3. Note the Application (client) ID +```bash +OIDC_PROVIDERS=okta,authentik +OIDC_OKTA_ISSUER=https://example.okta.com +OIDC_OKTA_CLIENT_ID=0oa... +OIDC_OKTA_NAME=Okta # optional button label +OIDC_OKTA_CLIENT_SECRET=... # optional — PKCE means a public client works +OIDC_OKTA_SCOPES=openid email profile # optional +``` -| Variable | Description | -|----------|-------------| -| `MICROSOFT_CLIENT_ID` | Your Azure AD application client ID | -| `MICROSOFT_TENANT_ID` | Tenant ID (`common` for multi-tenant) | +The issuer is the base URL whose `/.well-known/openid-configuration` describes the provider; endpoints +and keys are discovered from it and cached. + +**Account rules.** A provider must assert a verified email, because the whole account model keys on +it. An SSO login never takes over an existing account that has a password — the owner signs in +locally and links from Settings. If the provider's stable subject (`sub`) changes for an address, the +login is refused rather than handing an account to a recycled mailbox. + +An account established by one provider is **not** adopted by another. A per-organization provider may +only claim an account that its own organization established, or a `local` account that has never set +a password (an invited user signing in for the first time); anything else is refused with +`account_exists_other_provider`. The earlier rule — "any account without a password may be re-pointed +at whichever provider spoke last" — was safe only while the operator chose every provider, and became +an account-takeover primitive the moment customers could add their own. + +⚠️ **TOTP is not prompted on an SSO login.** Second-factor is the identity provider's job in this +flow, matching the long-standing behaviour of the SSO and API-token paths. + +#### Per-organization SSO (customer-configured) + +The providers above are **instance-wide** — they belong to whoever runs the server and appear as +buttons on the login page for everyone. + +An organization can also bring **its own** identity provider, configured by an org owner or admin in +**Settings → Single sign-on**. No environment variable or restart is involved. + +A per-org provider is **never listed publicly**. It appears only when someone types an email address +at one of that organization's **verified** domains, at which point the login page offers a generic +"Continue with single sign-on" button. The domain lookup answers only whether that domain uses SSO +and whether it is required — never a provider name or slug — so a guessed domain cannot confirm who +a customer is, and the mapping back to a provider happens server-side on submit. Both endpoints are +rate limited. + +**Instance-wide is the default; an organization overrides only its own verified domains.** Type an +address whose domain no organization has verified and you get the local password form plus every +instance provider you configured. Type one that an organization has verified and its own button is +added — and if that organization requires SSO, it becomes the only option. + +Each provider gets a randomly generated redirect URI, shown in Settings, which the admin registers +with their identity provider: + +``` +https://yourdomain.com/api/auth/oidc//callback +``` + +The slug is generated rather than chosen so two customers cannot collide on — or guess — each +other's. A domain may be claimed by only one organization; a second claim is refused. + +⚠️ **A provider may only authenticate emails inside the domains it has VERIFIED.** An organization +supplies its own issuer and client ID, so it controls that identity provider completely and could +otherwise assert any address at all — including another company's, or an administrator's. Confining +assertions to verified domains is what makes customer-configurable SSO safe to offer. + +⚠️ **Public email providers cannot be claimed.** `gmail.com`, `outlook.com`, `yahoo.com`, `icloud.com` +and the rest of the consumer mailboxes are refused (`server/lib/public-email-domains.js`). Claiming +one would offer every Gmail user a "sign in with your organization" button pointing at one tenant's +infrastructure — phishing launched from this product's own login page — and would let one account +deny a public domain to everyone else. + +##### Proving a domain + +A claimed domain **routes nobody and authenticates nobody until DNS proves the organization controls +it.** Typing a domain into a form reserves the name and nothing more. + +Publish this record, then press **Verify**: + +``` +_screentinker-verify.example.com. IN TXT "st-verify=" +``` + +The token is unique per domain, so publishing one proof cannot be replayed to claim a second. A +dedicated `_`-prefixed name is used rather than the apex, where a careless edit would sit alongside +SPF and DMARC and break mail — and where a wildcard `*.example.com` could not be confused for a +proof, since a wildcard answers with its own value and never with the token. + +TXT is the only accepted form. A CNAME alternative would have to point at a wildcard zone this +project operates, answering for every token ever issued; documenting one without running it would +describe a check that can never pass. + +⚠️ **The proof name itself must not be a CNAME.** A TXT lookup follows CNAMEs, and a wildcard +`*.example.com` covers `_screentinker-verify.example.com` too — so a wildcard CNAME would let +whoever controls its target prove the domain, turning an ordinary subdomain takeover into control of +every `@example.com` login. A delegated proof name is refused, which is stricter than ACME's dns-01. + +**An unverified claim lapses after 8 hours, and lapsing RELEASES it.** Pressing Verify on an +expired claim does not reissue it in place — that renewed the clock, so one request per window held +a domain forever. The claim is released, the domain becomes free for anyone else, and re-adding it +is a new claim: new token, and the operator is notified again. A verified domain never expires; +re-proving on a timer would log a customer out over a DNS edit made months afterwards. Squatting is +not made impossible — it is made loud. + +**Deleting a provider releases its domains and returns its accounts to local sign-in**, so the +organization can re-claim its own domain and its people can recover by password reset. Both used to +be stranded: a verified domain row outlived its provider and blocked that domain for everyone +permanently, and its users could neither sign in nor reset. + +Platform admins are emailed whenever a domain is claimed. Verification is what makes an unowned +claim worthless; the notification is what makes an attempt visible. Nothing is ever sent to the +claimed domain itself — that would let any tenant make this product email third parties. + +⚠️ **Instance-wide providers are exempt from all of the above.** `GOOGLE_CLIENT_ID`, `OIDC_*` and +friends are the operator's own configuration, are not domain-restricted, and require no verification. +Domain proof exists because per-organization providers are supplied by CUSTOMERS. + +Signing in through an organization's provider makes the user a member of that organization +(`org_member`). Existing members keep whatever role they already have — logging in never promotes or +demotes anyone. Client secrets are optional (PKCE), and are stored AES-256-GCM encrypted and never +returned by the API. + +##### Requiring single sign-on + +An organization can turn off password sign-in for its verified domains, so its identity provider is +the only way in — which is the point of buying SSO: the IdP holds the MFA, the conditional access +and the instant removal of access, and a password box beside it is a way around all three. + +Settings → Single sign-on → **Require single sign-on**. It needs at least one verified domain, so an +organization cannot leave its own people with no way to sign in, and cannot switch off passwords for +a domain it merely typed. + +When it is on: + +- the login page **hides** the password field for those domains rather than letting someone type a + password that is going to be refused and then send them to reset it; +- `POST /api/auth/login` refuses with `403 sso_required` — distinguishable from a wrong password, + because the page must not tell a user to fix a credential that is not the problem; +- **every other identity provider is refused too**, including the instance's own Google or + Microsoft. Those belong to the operator and are not domain-restricted, so leaving them available + would be a side door straight past the customer's MFA — blocking passwords while leaving + "Continue with Google" is not requiring single sign-on, it is renaming the bypass. + +**Turning it off is a request, not a switch.** That direction re-opens password sign-in, so it is +the direction an attacker who has taken an org admin would take, and it is also what a customer will +demand at their worst moment — identity provider down, nobody can work — which is exactly when a +self-service toggle gets flipped without thinking. The org admin files a request; a **platform admin +approves it**, and nothing changes until they do. + +The approval email deliberately carries **no action link**. A token that acts on its own would turn +every forwarded, archived or auto-previewed copy of that message into a way to switch off a +customer's single sign-on. The decision is made signed in, under Admin. + +⚠️ **`platform_admin` is exempt from enforcement, and that exemption is load-bearing.** The operator +is who approves removal. If the operator's own address sat at an SSO-only domain and that identity +provider broke, nobody could sign in to approve anything and the instance would be bricked with no +way out. It is the break-glass — it applies to the people running the server, never to a customer's +own admins. + +⚠️ **This makes the approval queue an availability dependency.** An organization whose IdP breaks is +locked out until an operator acts. That is the intended trade — deliberate friction on the dangerous +direction — but it should be a decision, not a surprise. + +#### Dependency preflight on boot + +Before anything else is loaded, the server checks that the packages this build declares are actually +installed and that the native database module loads under the running Node. If either is wrong it +repairs it (`npm install --omit=dev`, or `npm rebuild better-sqlite3`) and continues; if it cannot, +it exits saying what to run rather than dying on a `MODULE_NOT_FOUND` naming a file. + +`scripts/upgrade.sh` already installs dependencies, so this is not for the normal path. It is for +the ways a box ends up with the wrong `node_modules`: + +- **rolling back** to an older tag restores that tag's `package.json` but not its packages — and you + are rolling back because something is already wrong; +- **upgrading Node** leaves `better-sqlite3` compiled against the previous ABI, which fails in a way + that reads like database corruption and is not. + +Set `ST_SKIP_DEP_PREFLIGHT=1` on an air-gapped host, or anywhere you manage `node_modules` yourself +and do not want a boot reaching for the registry. #### Email (Microsoft Graph or SMTP) diff --git a/frontend/js/app.js b/frontend/js/app.js index 5cf312a..968cef4 100644 --- a/frontend/js/app.js +++ b/frontend/js/app.js @@ -248,7 +248,7 @@ async function refreshCurrentUser() { // a redirect loop. const hash = window.location.hash || '#/'; if (hasNoAccessibleWorkspace(fresh) - && hash !== '#/no-workspace' && hash !== '#/login' && hash !== '#/change-password') { + && hash !== '#/no-workspace' && !hash.startsWith('#/login') && hash !== '#/change-password') { window.location.hash = '#/no-workspace'; } } catch {} @@ -338,14 +338,29 @@ function route() { // do nothing. The login view reads the token off the hash and shows the new-password form. const isResetRoute = hash.startsWith('#/reset-password'); + /* + * ⚠️ The SAME rule the comment above states, for the login route. + * + * The server finishes every single sign-on by redirecting to `#/login?sso=1` (claim the session) + * or `#/login?sso_error=` (say what went wrong). Matching the hash EXACTLY meant neither + * survived: an unauthenticated browser — the only kind that arrives here — had the hash rewritten + * to a bare `#/login` and the query was gone before the login view ever ran. So a user who + * authenticated perfectly at their identity provider landed back on a clean login page, still + * signed out, with no message; and all sixteen error codes rendered SILENCE, which is worse than + * a wrong message because there is nothing to report or search for. + * + * It took the pre-existing `?verified=1` email-verification toast with it. + */ + const isLoginRoute = hash === '#/login' || hash.startsWith('#/login?'); + // Auth check - redirect to login if not authenticated - if (!isAuthenticated() && hash !== '#/login' && !isResetRoute) { + if (!isAuthenticated() && !isLoginRoute && !isResetRoute) { window.location.hash = '#/login'; return; } // If authenticated and on login page, redirect to dashboard or onboarding - if (isAuthenticated() && (hash === '#/login' || isResetRoute)) { + if (isAuthenticated() && (isLoginRoute || isResetRoute)) { window.location.hash = localStorage.getItem('rd_onboarded') ? '#/' : '#/onboarding'; return; } @@ -422,8 +437,10 @@ function route() { return; } - // Login page (and password-reset links from email) - hide sidebar - if (hash === '#/login' || isResetRoute) { + // Login page (and password-reset links from email) - hide sidebar. + // Matches `#/login?...` too: the single sign-on return carries `?sso=1` / `?sso_error=`, + // and an exact comparison meant the login view was never rendered for either. + if (isLoginRoute || isResetRoute) { sidebar.style.display = 'none'; app.style.marginLeft = '0'; const mb = document.getElementById('mobileMenuBtn'); 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 ab0d8f5..25fc544 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -132,6 +132,93 @@ export default { 'auth.trial_notice': 'New accounts get a 14-day free Pro trial', 'auth.divider_or': 'OR', 'auth.signin_google': 'Sign in with Google', + 'auth.signin_with': 'Continue with {provider}', + 'auth.sso_failed': 'Single sign-on failed. Please try again.', + 'auth.sso_org_hint': 'Your organization uses single sign-on.', + 'auth.signin_sso': 'Continue with single sign-on', + 'sso.title': 'Single sign-on', + 'sso.blurb': 'Let your team sign in with your own identity provider. Anyone using an email address at one of your domains will be sent there instead of being asked for a password.', + 'sso.add': 'Add a provider', + 'sso.none': 'No provider configured yet.', + 'sso.create': 'Add provider', + 'sso.saved': 'Saved', + 'sso.save_failed': 'Could not save that provider.', + 'sso.load_failed': 'Could not load single sign-on settings.', + 'sso.missing_fields': 'Name, issuer and client ID are required.', + 'sso.confirm_delete': 'Remove this provider? Anyone who signs in with it will lose that route.', + 'sso.delete': 'Remove', + 'sso.enable': 'Enable', + 'sso.disable': 'Disable', + '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 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', + 'sso.only_reason_prompt': 'Why do you need password sign-in re-opened? (optional, but it helps the reviewer)', + 'sso.only_requested': 'Request sent. Single sign-on stays required until it is approved.', + 'sso.only_pending': 'A request to stop requiring single sign-on is awaiting approval. Nothing changes until then.', + 'sso.only_cancel': 'Withdraw request', + 'sso.only_cancelled': 'Request withdrawn.', + 'sso.only_failed': 'That did not work.', + 'sso.domain_verified': 'verified', + 'sso.domain_pending': 'not verified — routes nobody yet', + 'sso.unverified_warning': 'Some domains are not verified yet, so nobody is routed to this provider by email address.', + 'sso.verify_now': 'Verify', + 'sso.verifying': 'Checking DNS…', + 'sso.verify_failed': 'Could not verify that domain.', + 'sso.domain_verified_toast': '{domain} is verified.', + 'sso.dns_instructions': 'Publish this TXT record in this domain\u2019s DNS, then click Verify. Claims expire after 8 hours.', + 'sso.callback_label': 'Redirect URI — add this to your provider', + 'sso.f_name': 'Display name', + 'sso.f_issuer': 'Issuer URL', + 'sso.f_issuer_hint': 'The base URL whose /.well-known/openid-configuration describes your provider. We check it before saving.', + 'sso.f_client_id': 'Client ID', + 'sso.f_client_secret': 'Client secret (optional)', + 'sso.f_client_secret_hint': 'Leave blank for a public client — we use PKCE, so a secret is not required. Stored encrypted and never shown again.', + 'sso.f_domains': 'Email domains', + 'sso.f_domains_hint': 'Comma separated. Anyone with an address at these domains is sent to this provider.', + 'sso.edit': 'Edit', + 'sso.save': 'Save changes', + 'sso.cancel': 'Cancel', + 'sso.secret_set': 'A secret is set — leave blank to keep it', + 'sso.secret_none': 'No secret set (public client)', + 'sso.secret_edit_hint': 'Leave blank to keep the current secret. Type a new one to replace it.', + 'sso.secret_clear': 'Remove the stored secret (use a public client)', + 'sso.test': 'Test', + 'sso.testing': 'Checking the provider…', + 'sso.test_failed': 'Could not reach that provider.', + 'sso.check_discovery': 'OpenID configuration', + 'sso.check_endpoints': 'Authorization and token endpoints', + 'sso.check_signing_keys': 'Signing keys', + 'sso.test_caveat': 'This confirms the provider is reachable and its tokens can be verified. It cannot check the client ID, the secret, or that the redirect URI is registered — only a real sign-in does that.', + 'auth.sso_err_expired': 'That sign-in took too long. Please try again.', + 'auth.sso_err_bad_state': 'Sign-in could not be verified. Please start again.', + 'auth.sso_err_no_code': 'The provider did not return an authorization code.', + 'auth.sso_err_no_email': 'Your provider did not share an email address.', + 'auth.sso_err_email_unverified': 'Your provider has not verified that email address.', + 'auth.sso_err_verification_failed': 'We could not verify the sign-in with your provider.', + 'auth.sso_err_provider_refused': 'Your provider declined the sign-in.', + '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 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: + // retrying is exactly what will not help, and the user needs to be told who to talk to instead. + 'auth.sso_required': 'Your organization requires single sign-on. Use \u201cContinue with single sign-on\u201d above \u2014 your password will not work here.', + 'auth.sso_err_sso_required': 'Your organization requires its own single sign-on. Use the single sign-on option for your organization.', + 'auth.sso_err_domain_not_allowed': 'Your organization has not verified that email domain for sign-in. Ask your administrator to verify it in ScreenTinker.', + 'auth.sso_err_account_exists_other_provider': 'An account with this email already exists and signs in through a different provider. Use that provider, or ask your administrator.', 'auth.signin_microsoft': 'Sign in with Microsoft', 'auth.back_to_signin': 'Back to Sign In', // TOTP 2FA challenge (second login step) @@ -1328,6 +1415,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..1ab6791 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,70 @@ 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; + } + // 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) => ` +
+
${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; @@ -276,11 +350,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('')} @@ -291,7 +371,7 @@ async function loadUsers() { ${workspaceCell(u)} - ${u.auth_provider === 'local' && u.id !== currentUser.id ? `` : ''} + ${u.auth_provider === 'local' && u.id !== currentUser.id ? `` : ''} ${!isPlatformAdmin(u) ? `` : `${t('admin.owner')}`} diff --git a/frontend/js/views/login.js b/frontend/js/views/login.js index cdaf376..f1585c0 100644 --- a/frontend/js/views/login.js +++ b/frontend/js/views/login.js @@ -1,5 +1,33 @@ import { showToast } from '../components/toast.js'; import { t } from '../i18n.js'; +import { esc } from '../utils.js'; + + +/* + * A recognisable mark for the providers people expect to see, and an honest generic one for + * everything else. Inline SVG rather than a remote image: an to a provider CDN would put a + * third-party origin back into the CSP, which is precisely what moving the flow server-side removed. + */ +const PROVIDER_ICONS = { + google: ``, + microsoft: ``, +}; + +const GENERIC_ICON = ``; + +const providerIcon = (slug) => PROVIDER_ICONS[slug] || GENERIC_ICON; let authConfig = null; @@ -77,8 +105,19 @@ export async function render(container) {
- + + +
${isSetup ? `
@@ -148,39 +187,32 @@ export async function render(container) {
- ${config.googleEnabled || config.microsoftEnabled ? ` -
+ ${(config.providers || []).length ? ` +
${t('auth.divider_or')}
` : ''} - ${config.googleEnabled ? ` -
- + + +
+ ${(config.providers || []).map((p) => ` + + ${providerIcon(p.slug)} + ${esc(t('auth.signin_with', { provider: p.name }))} + + `).join('')}
- ` : ''} - - ${config.microsoftEnabled ? ` - - ` : ''}
@@ -284,6 +316,12 @@ function setupHandlers(config, isSetup) { body: JSON.stringify({ email, password }) }); const data = await res.json(); + /* + * The organization requires its identity provider, so this is not a credential failure and + * must not read like one — "invalid password" sends the user to reset a password that will + * never work again. Point them at the control that does work. + */ + if (!res.ok && data.code === 'sso_required') { showError(t('auth.sso_required')); return; } if (!res.ok) { showError(data.error); return; } // Unverified account (hosted hard-gate): no session — prompt to check email. if (data.verification_required) { showVerifyNotice(data.email || email); return; } @@ -451,66 +489,214 @@ function setupHandlers(config, isSetup) { } } - // Google Sign-In - if (config.googleEnabled) { - document.getElementById('googleSignInBtn')?.addEventListener('click', async () => { - try { - // Use Google's popup-based sign in - const client = google.accounts.oauth2.initTokenClient({ - client_id: config.googleClientId, - scope: 'email profile', - callback: async (response) => { - if (response.access_token) { - // Get ID token via Google's tokeninfo - const tokenRes = await fetch(`https://oauth2.googleapis.com/tokeninfo?access_token=${response.access_token}`); - const tokenData = await tokenRes.json(); - // Send to our server - const res = await fetch('/api/auth/google', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ credential: response.access_token, email: tokenData.email }) - }); - const data = await res.json(); - if (res.ok) onAuthSuccess(data); - else showError(data.error); - } - } - }); - client.requestAccessToken(); - } catch (err) { - showError(t('auth.error_google_failed')); - } - }); + /* + * SSO is a link, not a script. + * + * The buttons above are anchors to /api/auth/oidc//start, so there is nothing to bind here + * and no SDK to wait for. What DOES need handling is the trip back: the callback redirects to + * #/login carrying either a session token or an error code. + * + * The token rides in the URL FRAGMENT, which browsers never send to servers and proxies never + * log — and it is stripped from the address bar before anything else happens, so a shared screen + * or a copied URL does not carry a live session. + */ + /* + * Email-first SSO for organizations. + * + * Instance-wide providers are always on the page. An ORG provider is different — it belongs to + * one customer — so it is fetched by domain once the address looks complete, and only then. + * + * 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'); + + /* + * Show or hide the password half of the sign-in form. + * + * Presentation only — the server refuses a password for these accounts regardless. Restoring it + * 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) { + /* + * ⚠️ 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. + */ + const show = visible ? '' : 'none'; + for (const id of ['loginPassword', 'loginPasswordLabel', 'loginBtn']) { + 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. + */ + 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) { + const wrap = forgot.parentElement && forgot.parentElement.tagName === 'P' ? forgot.parentElement : forgot; + wrap.style.display = show; + } } - // Microsoft Sign-In - if (config.microsoftEnabled) { - document.getElementById('microsoftSignInBtn')?.addEventListener('click', async () => { - try { - const msalConfig = { - auth: { - clientId: config.microsoftClientId, - authority: `https://login.microsoftonline.com/${config.microsoftTenantId}`, - redirectUri: window.location.origin - } - }; - const msalInstance = new msal.PublicClientApplication(msalConfig); - await msalInstance.initialize(); - const loginResponse = await msalInstance.loginPopup({ scopes: ['User.Read'] }); - if (loginResponse.accessToken) { - const res = await fetch('/api/auth/microsoft', { + async function lookupOrgSso(email) { + const at = String(email || '').lastIndexOf('@'); + const domain = at === -1 ? '' : email.slice(at + 1).trim().toLowerCase(); + const slot = orgSlot(); + if (!slot) return; + // Nothing to ask about until there is a domain with a dot in it. + if (!domain || !domain.includes('.')) { + slot.style.display = 'none'; slot.innerHTML = ''; lastDomainAsked = ''; setPasswordVisible(true); return; + } + if (domain === lastDomainAsked) return; + try { + const res = await fetch(`/api/auth/sso/discover?email=${encodeURIComponent(email)}`); + /* + * ⚠️ Check the STATUS, not just that a body parsed. + * + * The comment below has always said a tripped rate limit must not poison the domain — and it + * did anyway, because a 429 body is perfectly valid JSON: res.json() resolved, `data.sso` + * came back undefined, so the single sign-on button was hidden, the password box restored, + * and `lastDomainAsked` recorded — permanently, for the life of the page. On an SSO-only + * domain that is the worst possible outcome: the password box the user is then offered gets + * 403, and the button they are told to use is not on the screen. Discover is 10/min per IP, + * so a handful of colleagues behind one office address is enough to trigger it. + */ + if (!res.ok) throw new Error(`discover ${res.status}`); + const data = await res.json(); + // Remembered only after a SUCCESSFUL answer. + lastDomainAsked = domain; + if (!data.sso) { slot.style.display = 'none'; slot.innerHTML = ''; setPasswordVisible(true); return; } + /* + * When the organization REQUIRES its identity provider, the password box is not merely going + * to fail — it is the wrong thing to offer. Showing it invites someone to type a password, + * be refused, and go and reset a password that will never work again. Hidden, not disabled, + * so there is one obvious way forward. + */ + setPasswordVisible(!data.required); + /* + * A FORM, not a link, and a deliberately generic label. + * + * The lookup tells us only that this domain uses SSO — never which provider or whose it is, + * because that would identify a customer to anyone who guessed a domain. The server does the + * 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' }, - body: JSON.stringify({ access_token: loginResponse.accessToken }) + headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, + body: JSON.stringify({ email }), }); - const data = await res.json(); - if (res.ok) onAuthSuccess(data); - else showError(data.error); + 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 (err) { - showError(t('auth.error_microsoft_failed')); + }); + } 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. + slot.style.display = 'none'; + slot.innerHTML = ''; + setPasswordVisible(true); + } + } + + document.getElementById('loginEmail')?.addEventListener('input', (e) => { + clearTimeout(ssoLookupTimer); + const value = e.target.value; + ssoLookupTimer = setTimeout(() => lookupOrgSso(value), 400); + }); + + /* + * Completing an SSO login. + * + * The callback no longer hands the session token back in the URL — that was a login-CSRF hole, + * because a crafted link could install an ATTACKER'S token and quietly sign the victim into their + * account. The server now leaves it in a one-shot httpOnly cookie and we exchange it here, which + * a link cannot forge. + * + * Wrapped in an async IIFE because setupHandlers() is not async; `await` at this level is a + * SyntaxError that takes the whole module graph down with it, since app.js imports this file + * statically and there is no bundler to catch it first. + */ + const ssoParams = new URLSearchParams((window.location.hash.split('?')[1] || '')); + const ssoReturning = ssoParams.get('sso') === '1'; + const ssoError = ssoParams.get('sso_error'); + + if (ssoReturning || ssoError) { + // Keep any real query string; only the hash carried the SSO markers. + history.replaceState(null, '', window.location.pathname + window.location.search + '#/login'); + } + + if (ssoReturning) { + (async () => { + try { + const res = await fetch('/api/auth/sso/claim', { method: 'POST' }); + if (!res.ok) throw new Error('claim rejected'); + const data = await res.json(); + onAuthSuccess(data); + } catch { + showToast(t('auth.sso_failed'), 'error'); } - }); + })(); + } else if (ssoError) { + // Every code the callback can emit has a message; an unknown one still says something true + // rather than failing silently, which is how the previous implementation behaved on every click. + const known = ['expired', 'bad_state', 'no_code', 'no_email', 'email_unverified', + 'verification_failed', 'provider_refused', 'provider_unavailable', 'unknown_provider', + 'registration_disabled', 'account_exists_local', 'subject_mismatch', 'server_error', + 'domain_not_allowed', 'account_exists_other_provider', 'sso_required']; + const key = known.includes(ssoError) ? `auth.sso_err_${ssoError}` : 'auth.sso_failed'; + showToast(t(key), 'error'); } } diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index 3bd8efb..2b711d8 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -66,6 +66,35 @@ export async function render(container) {
+ + +

${t('apitoken.title')}

${t('apitoken.desc')}

@@ -634,6 +663,388 @@ export async function render(container) { } }); + /* ── Per-organization SSO ────────────────────────────────────────────────────────────────── + * + * Only an org owner/admin sees this. The server enforces the same rule (and answers 404, not + * 403, so an outsider learns nothing) — this just avoids showing a card the user cannot use. + */ + const orgId = user.current_organization?.id; + const canManageSso = orgId && ['org_owner', 'org_admin'].includes(user.current_org_role); + + async function loadSso() { + const card = document.getElementById('ssoCard'); + if (!card || !canManageSso) return; + card.style.display = ''; + const listEl = document.getElementById('ssoList'); + let providers = []; + try { + const res = await fetch(`/api/organizations/${orgId}/sso`, { + headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }, + }); + if (!res.ok) throw new Error('load failed'); + providers = (await res.json()).providers || []; + } catch { + listEl.innerHTML = `

${esc(t('sso.load_failed'))}

`; + return; + } + + if (!providers.length) { + listEl.innerHTML = `

${esc(t('sso.none'))}

`; + return; + } + + // Requiring SSO is a separate decision from having it, so it gets its own block rather than + // hiding inside a provider — an organization may have several providers and one answer. + let onlyState = null; + try { + const r = await fetch(`/api/organizations/${orgId}/sso-only`, { + headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }, + }); + if (r.ok) onlyState = await r.json(); + } catch { /* the providers still render; the toggle simply does not appear */ } + + const origin = `${window.location.protocol}//${window.location.host}`; + listEl.innerHTML = providers.map((p) => ` +
+
+
+ ${esc(p.name)} + ${p.enabled ? '' : ` — ${esc(t('sso.disabled'))}`} +
${esc(p.issuer)}
+
${esc(t('sso.domains_label'))}: ${esc(p.email_domains || '—')}
+ ${((p.domains || []).some((d) => !d.verified) || (p.domains || []).length === 0) + ? `
⚠️ ${esc(t('sso.unverified_warning'))}
` + : ''} +
+ +
+ + + + +
+
+ +
+
${esc(t('sso.callback_label'))}
+ ${esc(origin + p.callback_url)} +
+ + + + ${(p.domains || []).length ? ` +
+
${esc(t('sso.domains_heading'))}
+ ${p.domains.map((d, di) => ` +
+
+
${esc(d.domain)} + ${d.verified + ? ` — ${esc(t('sso.domain_verified'))}` + : ` — ${esc(t('sso.domain_pending'))}`} +
+ ${d.verified ? '' : ``} +
+ ${d.verified ? '' : ` +
${esc(t('sso.dns_instructions'))}
+ ${esc(d.record_name)} TXT ${esc(d.txt_value)} +`} + +
${d.verified ? '' : esc(d.last_error || '')}
+
`).join('')} +
` : ''} + + + +
`).join(''); + + listEl.querySelectorAll('[data-sso-toggle]').forEach((btn) => { + btn.addEventListener('click', async () => { + await ssoRequest('PUT', `/${btn.dataset.ssoToggle}`, { enabled: btn.dataset.enabled !== '1' }); + }); + }); + /* + * Ask the server to look for the DNS record now. Pull-based on purpose: the admin has just + * edited DNS and wants an answer, and a failure has to say WHICH failure — not published yet, + * published wrong, or the claim expired and the record has changed underneath them. + */ + if (onlyState) { + const pend = onlyState.pending_removal_request; + const box = document.createElement('div'); + box.style.cssText = 'border:1px solid var(--border);border-radius:var(--radius);padding:12px;margin-top:4px'; + box.innerHTML = ` +
${esc(t('sso.only_heading'))}
+
${esc(t('sso.only_help'))}
+ ${onlyState.sso_only ? ` +
✅ ${esc(t('sso.only_on'))}
+ ${pend + ? `
⏳ ${esc(t('sso.only_pending'))}
+ ` + : `
${esc(t('sso.only_remove_help'))}
+ `} + ` : ` +
${esc(t('sso.only_off'))}
+ ${onlyState.verified_domains + ? `` + : `
⚠️ ${esc(t('sso.only_needs_domain'))}
`} + `}`; + listEl.appendChild(box); + + const post = async (url, body, method = 'POST') => { + const r = await fetch(url, { + method, + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}` }, + body: body ? JSON.stringify(body) : undefined, + }); + const j = await r.json().catch(() => ({})); + if (!r.ok) { showToast(j.error || t('sso.only_failed'), 'error'); return null; } + return j; + }; + + const enableBtn = box.querySelector('#ssoOnlyEnable'); + if (enableBtn) enableBtn.addEventListener('click', async () => { + // 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; + 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'); + if (reqBtn) reqBtn.addEventListener('click', async () => { + const reason = window.prompt(t('sso.only_reason_prompt')) || ''; + const r = await post(`/api/organizations/${orgId}/sso-only/removal-request`, { reason }); + if (r) { showToast(t('sso.only_requested'), 'success'); await loadSso(); } + }); + + const cancelBtn = box.querySelector('#ssoOnlyCancel'); + if (cancelBtn) cancelBtn.addEventListener('click', async () => { + const r = await post(`/api/organizations/${orgId}/sso-only/removal-request/${cancelBtn.dataset.req}`, null, 'DELETE'); + if (r) { showToast(t('sso.only_cancelled'), 'success'); await loadSso(); } + }); + } + + listEl.querySelectorAll('[data-sso-verify]').forEach((btn) => { + btn.addEventListener('click', async () => { + const id = btn.dataset.ssoVerify; + const domain = btn.dataset.domain; + // Indexed, not derived from the domain: `a.b.test` and `a-b.test` both slugify to + // `a-b-test`, and getElementById would put one domain's answer in the other's box. + const out = document.getElementById(`ssoVerify-${id}-${btn.dataset.di}`); + btn.disabled = true; + if (out) { out.style.color = 'var(--text-muted)'; out.textContent = t('sso.verifying'); } + try { + const res = await fetch(`/api/organizations/${orgId}/sso/${id}/domains/${encodeURIComponent(domain)}/verify`, { + method: 'POST', + headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }, + }); + const body = await res.json().catch(() => ({})); + if (body.ok) { + showToast(t('sso.domain_verified_toast', { domain }), 'success'); + await loadSso(); // re-render: the domain now routes, and the card must say so + return; + } + // An expired claim has already been reissued server-side, so the records on screen are + // stale — reload rather than leaving the admin publishing a value that no longer matches. + if (body.expired) { + showToast(body.error || t('sso.verify_failed'), 'error'); + await loadSso(); + return; + } + if (out) { out.style.color = 'var(--danger,#b91c1c)'; out.textContent = body.error || t('sso.verify_failed'); } + } catch { + if (out) { out.style.color = 'var(--danger,#b91c1c)'; out.textContent = t('sso.verify_failed'); } + } finally { + btn.disabled = false; + } + }); + }); + listEl.querySelectorAll('[data-sso-test]').forEach((btn) => { + btn.addEventListener('click', async () => { + const id = btn.dataset.ssoTest; + const out = document.getElementById(`ssoTest-${id}`); + if (!out) return; + out.style.display = ''; + out.textContent = t('sso.testing'); + try { + const res = await fetch(`/api/organizations/${orgId}/sso/${id}/test`, { + method: 'POST', + headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }, + }); + const data = await res.json(); + if (!res.ok) { out.textContent = data.error || t('sso.test_failed'); return; } + /* + * Literal keys, never a key built by concatenating a check name. Doing that defeats the + * check in server/test/i18n-keys-exist.js that every key an operator can see is + * translated — and a check name the UI does not know would render as raw key text. The + * fallback keeps an unknown one readable instead. + */ + const CHECK_LABELS = { + discovery: t('sso.check_discovery'), + endpoints: t('sso.check_endpoints'), + signing_keys: t('sso.check_signing_keys'), + }; + const rows = (data.checks || []).map((c) => ` +
${c.ok ? '✅' : '❌'} ${esc(CHECK_LABELS[c.name] || c.name)} — ${esc(c.detail || '')}
`).join(''); + /* + * The caveat is shown on SUCCESS, not tucked away. Discovery and keys prove the provider + * exists and that we could verify a token it signs — they say nothing about whether the + * client id, the secret, or the redirect URI registration are right. A green tick that + * implied "SSO works" would send an admin away from the one thing still to check. + */ + out.innerHTML = rows + (data.ok + ? `
${esc(t('sso.test_caveat'))}
` + : ''); + } catch { + out.textContent = t('sso.test_failed'); + } + }); + }); + listEl.querySelectorAll('[data-sso-edit]').forEach((btn) => { + btn.addEventListener('click', () => { + const panel = document.getElementById(`ssoEdit-${btn.dataset.ssoEdit}`); + if (panel) panel.style.display = panel.style.display === 'none' ? 'block' : 'none'; + }); + }); + listEl.querySelectorAll('[data-sso-cancel]').forEach((btn) => { + btn.addEventListener('click', () => { + const panel = document.getElementById(`ssoEdit-${btn.dataset.ssoCancel}`); + if (panel) panel.style.display = 'none'; + }); + }); + listEl.querySelectorAll('[data-sso-save]').forEach((btn) => { + btn.addEventListener('click', async () => { + const panel = document.getElementById(`ssoEdit-${btn.dataset.ssoSave}`); + if (!panel) return; + const val = (f) => panel.querySelector(`[data-f="${f}"]`)?.value?.trim() ?? ''; + const body = { + name: val('name'), + issuer: val('issuer'), + client_id: val('client_id'), + email_domains: val('email_domains'), + }; + /* + * Three states, and only these three: + * typed a value -> replace the secret + * ticked "remove" -> send '' so the server clears it + * left blank, unticked -> send NOTHING, so the stored secret survives + * Sending '' on every save is the bug this shape exists to avoid. + */ + const typed = panel.querySelector('[data-f="client_secret"]')?.value || ''; + const clearing = panel.querySelector('[data-f="clear_secret"]')?.checked; + if (typed) body.client_secret = typed; + else if (clearing) body.client_secret = ''; + + if (!body.name || !body.issuer || !body.client_id) { + showToast(t('sso.missing_fields'), 'error'); + return; + } + await ssoRequest('PUT', `/${btn.dataset.ssoSave}`, body); + }); + }); + listEl.querySelectorAll('[data-sso-delete]').forEach((btn) => { + btn.addEventListener('click', async () => { + if (!confirm(t('sso.confirm_delete'))) return; + await ssoRequest('DELETE', `/${btn.dataset.ssoDelete}`); + }); + }); + } + + async function ssoRequest(method, path = '', body) { + try { + const res = await fetch(`/api/organizations/${orgId}/sso${path}`, { + method, + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${localStorage.getItem('token')}`, + }, + body: body ? JSON.stringify(body) : undefined, + }); + const data = await res.json().catch(() => ({})); + // 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; } + // "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 { + showToast(t('sso.save_failed'), 'error'); + return false; + } + } + + document.getElementById('ssoCreateBtn')?.addEventListener('click', async () => { + const payload = { + name: document.getElementById('ssoName').value.trim(), + issuer: document.getElementById('ssoIssuer').value.trim(), + client_id: document.getElementById('ssoClientId').value.trim(), + client_secret: document.getElementById('ssoClientSecret').value, + email_domains: document.getElementById('ssoDomains').value.trim(), + }; + if (!payload.name || !payload.issuer || !payload.client_id) { + showToast(t('sso.missing_fields'), 'error'); + return; + } + if (await ssoRequest('POST', '', payload)) { + ['ssoName', 'ssoIssuer', 'ssoClientId', 'ssoClientSecret', 'ssoDomains'] + .forEach((id) => { document.getElementById(id).value = ''; }); + document.getElementById('ssoAddDetails').open = false; + } + }); + + loadSso(); + + document.getElementById('createTokenBtn')?.addEventListener('click', async () => { const name = document.getElementById('tokName').value.trim(); const scope = document.getElementById('tokScope').value; @@ -830,25 +1241,30 @@ async function loadUsers() { ${users.map(u => ` - + + -
${u.name || u.email}
-
${u.email}
+
${esc(u.name || u.email)}
+
${esc(u.email)}
- ${u.auth_provider} + ${esc(u.auth_provider)} - ${u.role} + ${esc(u.role)} - + ${plans.map(p => ``).join('')} - ${u.auth_provider === 'local' && u.id !== currentUser.id ? `` : ''} - ${u.id !== currentUser.id ? `` : `${t('settings.user.you')}`} + ${u.auth_provider === 'local' && u.id !== currentUser.id ? `` : ''} + ${u.id !== currentUser.id ? `` : `${t('settings.user.you')}`} `).join('')} diff --git a/server/config.js b/server/config.js index 7750b1d..c5d62b7 100644 --- a/server/config.js +++ b/server/config.js @@ -84,11 +84,14 @@ module.exports = { return secret; })(), jwtExpiry: '7d', - // Google OAuth - set these in env or here - googleClientId: process.env.GOOGLE_CLIENT_ID || '', - // Microsoft OAuth - set these in env or here - microsoftClientId: process.env.MICROSOFT_CLIENT_ID || '', - microsoftTenantId: process.env.MICROSOFT_TENANT_ID || 'common', + /* + * Google and Microsoft sign-in are configured through lib/oidc-providers.js, which reads + * process.env directly — there is nothing here for it to read, so these fields were dead, and + * `microsoftTenantId` defaulting to 'common' actively contradicted the provider code, which now + * REFUSES 'common' (it advertises a template issuer that can never match, and accepting it means + * accepting tokens from every Azure tenant — nOAuth). Removed rather than left as a trap for the + * next person who greps for where Microsoft SSO is configured. + */ // Stripe (optional - for paid subscriptions) stripeSecretKey: process.env.STRIPE_SECRET_KEY || '', stripeWebhookSecret: process.env.STRIPE_WEBHOOK_SECRET || '', diff --git a/server/db/database.js b/server/db/database.js index 3db48e2..bd5a087 100644 --- a/server/db/database.js +++ b/server/db/database.js @@ -396,6 +396,105 @@ const migrations = [ // Per-telemetry-row rather than on `devices` because a display can be swapped, unplugged or // renegotiated without the player re-registering, and because a dual-output player registers ONE // ROW PER OUTPUT (see output_index) — each row must carry its own screen, not the box's first. + /* + * Per-organization SSO. + * + * Instance-wide providers come from the environment and belong to whoever runs the server. These + * belong to a CUSTOMER: an organization brings its own identity provider, and its people sign in + * with it without the operator touching a config file. + * + * `slug` is globally unique and randomly generated rather than chosen, because it is a URL path + * segment (/api/auth/oidc//start) and two organizations both wanting "okta" must not be + * able to collide — or to guess each other's. The admin only ever sees `name`. + * + * `client_secret_enc` is AES-256-GCM via lib/secretbox, the same at-rest treatment as TOTP + * secrets and BYOK AI keys. PKCE means a secret is optional, so a public client stores NULL. + * + * `email_domains` is the list an admin TYPED, kept for display and for the edit form. It does not + * drive routing — org_sso_domains does, and only its verified rows (see the table below). The two + * are not interchangeable: reading this column to decide who may sign in would let a tenant route + * a domain it never proved. + */ + `CREATE TABLE IF NOT EXISTS org_sso_providers ( + id TEXT PRIMARY KEY, + organization_id TEXT NOT NULL, + slug TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + issuer TEXT NOT NULL, + client_id TEXT NOT NULL, + client_secret_enc TEXT, + scopes TEXT NOT NULL DEFAULT 'openid email profile', + email_domains TEXT NOT NULL DEFAULT '', + enabled INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE + )`, + "CREATE INDEX IF NOT EXISTS idx_org_sso_org ON org_sso_providers(organization_id)", + /* + * Claimed sign-in domains, and the proof that the claimant controls them. + * + * `org_sso_providers.email_domains` used to be the whole story, and first-claim-wins on a text + * field is not a claim — it is a land grab. A tenant could type a domain it had nothing to do + * with and every person at that company typing their work address into the login page would be + * routed to the squatter's identity provider. It also let one account permanently deny a domain + * to its real owner, and strand accounts at addresses it never owned. + * + * So a domain is inert until DNS says otherwise. `verified_at` NULL means claimed but unproven: + * it routes nobody, and the login callback will not accept an assertion for it. The row still + * reserves the name, so two tenants cannot race the same domain, but reserving is all it does. + * + * `token` is what has to appear in DNS. It is per-domain rather than per-organization so that + * publishing one proof cannot be replayed to claim a second domain. + */ + `CREATE TABLE IF NOT EXISTS org_sso_domains ( + id TEXT PRIMARY KEY, + organization_id TEXT NOT NULL, + provider_id TEXT, + domain TEXT NOT NULL UNIQUE, + token TEXT NOT NULL, + -- When the current token was issued. An UNVERIFIED claim is only good for 8 hours from here: + -- past that the token is dead and the reservation lapses, so a domain nobody can prove cannot + -- be held indefinitely by whoever typed it first. Verified rows ignore this entirely. + token_issued_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + verified_at INTEGER, + last_checked_at INTEGER, + last_error TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE, + -- A verified row never expires and domain is globally UNIQUE, so a row that outlives its + -- provider blocks that domain for EVERYONE, forever, while being invisible in the API. The + -- delete handler clears these explicitly; this is the backstop for every other route out + -- (an organization cascade, a manual delete, a future caller that forgets). + FOREIGN KEY (provider_id) REFERENCES org_sso_providers(id) ON DELETE CASCADE + )`, + "CREATE INDEX IF NOT EXISTS idx_org_sso_domains_org ON org_sso_domains(organization_id)", + "CREATE INDEX IF NOT EXISTS idx_org_sso_domains_provider ON org_sso_domains(provider_id)", + /* + * SSO-ONLY: an organization may require its people to use its identity provider, so a password + * is no longer an alternative way in. That is the point of buying SSO — the IdP holds the MFA, + * the conditional access and the instant deprovisioning, and a password box beside it is a way + * around all three. + * + * ⚠️ Asymmetric on purpose. Turning it ON is the safe direction and an org admin does it alone. + * Turning it OFF is how a compromised admin would re-open password login, and it is also what + * an org will demand at its worst moment — IdP down, nobody can work — which is exactly when a + * self-service switch gets flipped under pressure. So removal goes through the operator: the + * request is recorded here and a platform admin has to approve it. + */ + `CREATE TABLE IF NOT EXISTS org_sso_only_requests ( + id TEXT PRIMARY KEY, + organization_id TEXT NOT NULL, + requested_by TEXT, + reason TEXT, + status TEXT NOT NULL DEFAULT 'pending', -- pending | approved | rejected | cancelled + decided_by TEXT, + decided_at INTEGER, + decision_note TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE + )`, + "CREATE INDEX IF NOT EXISTS idx_sso_only_req_status ON org_sso_only_requests(status, organization_id)", "ALTER TABLE device_telemetry ADD COLUMN attached_display TEXT", "ALTER TABLE device_telemetry ADD COLUMN video_mode TEXT", // Panel temperature in Celsius. REAL because the sensor reports fractions, and nullable because @@ -565,6 +664,40 @@ for (const sql of migrations) { } if (_migApplied > 0) console.log(`[migrate] applied ${_migApplied} new column migration(s)`); +/* + * Say something when per-org SSO domains predate the proof requirement. + * + * Domains used to be a comma list an admin typed, and that list routed logins. They now route only + * once DNS proves them, so on an instance upgraded from an earlier build of this feature every one + * of those domains silently stops working — the provider still says "enabled", the typed list is + * still on screen, and every federated user in that organization is locked out with no self-service + * way back. + * + * They are deliberately NOT auto-claimed. A claim now notifies the operator, reserves the name + * against other tenants and starts an 8-hour clock; manufacturing all of that on an admin's behalf, + * for domains nobody ever proved, is not a migration's decision to make. So: name them, loudly, + * once per boot, and let an admin re-add the ones they still want. + */ +try { + const stranded = db.prepare(` + SELECT p.slug, p.name, p.organization_id, p.email_domains + FROM org_sso_providers p + WHERE p.email_domains != '' + AND NOT EXISTS (SELECT 1 FROM org_sso_domains d WHERE d.provider_id = p.id) + `).all(); + if (stranded.length) { + console.warn(`[migrate] ⚠️ ${stranded.length} SSO provider(s) have typed domains that were never verified.`); + console.warn('[migrate] Domains now route only after a DNS TXT record proves them, so these route NOBODY:'); + for (const r of stranded) { + console.warn(`[migrate] ${r.name} (${r.slug}, org ${r.organization_id}): ${r.email_domains}`); + } + console.warn('[migrate] Re-add each domain in Settings to get its record, then Verify. See README, "Proving a domain".'); + } +} catch (e) { + // The table may not exist yet on a first boot; that is not a problem worth a stack trace. + if (!/no such table/i.test(e.message)) console.error('[migrate] SSO domain check failed:', e.message); +} + // #74/#75 per-item schedules: the playlist_item_schedules table is created // idempotently by schema.sql (CREATE TABLE IF NOT EXISTS, run every boot, so it // self-applies on upgrade). Record it in schema_migrations for observability. @@ -825,6 +958,27 @@ migrateGroupSchedules(); // updates workspace_id. ensureMultitenancyMigration(); +/* + * `organizations.sso_only` — added HERE, not in the migrations array above. + * + * That array runs BEFORE ensureMultitenancyMigration(), which is what creates the organizations + * table, so on a fresh install the ALTER hit a table that did not exist yet: `[migrate] FAILED … + * no such table: organizations`, one console.error among ~85 migration lines. The instance then + * ran its entire first boot with the SSO settings screen 500ing and — far worse — + * ssoOnlyForEmail() catching `no such column` and returning "not SSO-only", which is password + * login proceeding for an organization that had switched it off. It self-healed on the second + * boot, which is exactly what makes it easy to miss. + */ +try { + const orgCols = db.prepare('PRAGMA table_info(organizations)').all().map((c) => c.name); + if (orgCols.length && !orgCols.includes('sso_only')) { + db.exec('ALTER TABLE organizations ADD COLUMN sso_only INTEGER NOT NULL DEFAULT 0'); + console.log('[migrate] added organizations.sso_only'); + } +} catch (e) { + console.error('[migrate] could not add organizations.sso_only:', e.message); +} + // Phase 2.2c migration: backfill content_folders.workspace_id from owner's // default workspace. The ALTER lives in the migrations array above; this // one-shot populates the column for any rows that pre-date it. diff --git a/server/lib/domain-verify.js b/server/lib/domain-verify.js new file mode 100644 index 0000000..8aefbfb --- /dev/null +++ b/server/lib/domain-verify.js @@ -0,0 +1,154 @@ +'use strict'; + +/* + * Proving that a tenant controls a sign-in domain. + * + * Per-organization SSO routes everyone at a domain to that organization's identity provider. That + * is exactly right when the organization owns the domain and an account-takeover primitive when it + * does not — and typing a domain into a form is not ownership. A review demonstrated the whole + * chain: claim a company's domain, sign in as a named address there, and the real owner is left + * unable to reach an account bearing their own address. + * + * DNS is the check, because control of a domain's DNS is what "owning a domain" means in the only + * sense that matters here. It is also the mechanism every other vendor uses, so the instructions + * are already familiar to the person who has to follow them. + * + * ONE RECORD FORM — a TXT record at a dedicated name: + * + * _screentinker-verify.example.com. IN TXT "st-verify=" + * + * A CNAME alternative was drafted and dropped. It would have pointed at + * `.verify.screentinker.com`, which requires operating a wildcard DNS zone that answers for + * every token ever issued — infrastructure this project does not have, so the instructions would + * have described a check that could never pass. TXT needs nothing but the customer's own zone. + * + * A dedicated `_`-prefixed name is used rather than the apex on purpose: an apex TXT record sits + * alongside SPF and DMARC, where a careless edit breaks mail, and it is the one record set an + * administrator is most reluctant to touch. + * + * ⚠️ THE PROOF NAME MUST NOT BE A CNAME. A TXT lookup follows CNAMEs transparently, and RFC 4592 + * means a wildcard `*.example.com` synthesizes `_screentinker-verify.example.com` too — so a + * wildcard CNAME pointing anywhere the attacker controls would let them prove a domain they do not + * own. That turns an ordinary subdomain takeover into an apex takeover, and from there into every + * `@example.com` login. ACME's dns-01 permits this delegation deliberately; here the thing being + * delegated is the whole company's sign-in, so it is refused instead. + */ + +const dns = require('dns').promises; +const crypto = require('crypto'); + +const RECORD_PREFIX = '_screentinker-verify'; +const TXT_PREFIX = 'st-verify='; + +// A DNS answer that never arrives must not hold an HTTP request open. The resolver's own retries +// sit under this, so it is a ceiling on the whole lookup rather than on one query. +const LOOKUP_TIMEOUT_MS = 5000; + +/* + * How long an UNVERIFIED claim is worth anything. + * + * A claim reserves the domain so two tenants cannot race it — but a reservation that never lapses + * is squatting with extra steps: type a company's domain, prove nothing, and hold it against its + * real owner forever. Eight hours is comfortably longer than a DNS change takes to publish and + * propagate, and short enough that an unprovable claim is gone by the next working day. + * + * The token dies with the claim. Trying again mints a NEW token, so an old record left in DNS from + * a lapsed attempt proves nothing, and a domain that changed hands cannot be verified with the + * previous holder's value. + * + * A VERIFIED domain is not affected — proof already happened, and re-proving on a timer would log + * out a customer over a DNS edit made months later. + */ +const CLAIM_TTL_S = 8 * 60 * 60; + +/** True when an unverified claim has run out of time and no longer reserves anything. */ +function isClaimExpired(row, nowS = Math.floor(Date.now() / 1000)) { + if (!row) return false; + // `verified_at` is compared to null, NOT tested for truthiness: SQL asks `IS NOT NULL` and a + // stored 0 would otherwise be "verified" to the router and "unverified" here — two definitions of + // the same word, which is how a domain ends up routing while the code believes it cannot. + if (row.verified_at !== null && row.verified_at !== undefined) return false; + return (Number(row.token_issued_at) || 0) + CLAIM_TTL_S <= nowS; +} + +/** Tokens are compared, so they are random and long enough that guessing is not a strategy. */ +const newToken = () => crypto.randomBytes(16).toString('hex'); + +const recordName = (domain) => `${RECORD_PREFIX}.${domain}`; + +/** Exactly what the admin has to publish — shown in the UI, so it is built in one place. */ +function instructions(domain, token) { + return { + record_name: recordName(domain), + txt_value: `${TXT_PREFIX}${token}`, + }; +} + +function withTimeout(promise, ms) { + let timer; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error('DNS lookup timed out')), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); +} + +/* + * Look for the proof. + * + * Both record types are queried together and either one is enough. NXDOMAIN and "no such record" + * are ordinary answers here — the overwhelmingly common case is an admin checking before the record + * has propagated — so they are reported as "not found yet", never as an error to be alarmed by. + * + * ⚠️ Resolution uses the system resolver, which is the same view of DNS the operator already + * trusts. A tenant that can poison that resolver can forge a proof, but a tenant that can do that + * has already won something larger. + */ +async function check(domain, token) { + const name = recordName(domain); + const wantTxt = `${TXT_PREFIX}${token}`; + + /* + * Refuse before looking at the TXT at all if the name is delegated. Checking afterwards would + * still be safe, but doing it first means the answer never depends on what the delegation target + * happens to say. + */ + try { + const cnames = await withTimeout(dns.resolveCname(name), LOOKUP_TIMEOUT_MS); + if (cnames && cnames.length) { + return { + ok: false, + error: `${name} is a CNAME (to ${cnames[0]}). The record must be a TXT record in this ` + + 'domain\u2019s own zone — a delegated name would let whoever controls the target prove this domain.', + }; + } + } catch { /* no CNAME is the normal and wanted case */ } + + let records; + try { + records = await withTimeout(dns.resolveTxt(name), LOOKUP_TIMEOUT_MS); + } catch (e) { + // NXDOMAIN and "no such record" are the ORDINARY answers here — an admin checking before the + // record has propagated — so they are "not found yet", not an error to be alarmed by. + if (/timed out/i.test(e.message)) return { ok: false, error: 'the DNS lookup timed out — try again shortly' }; + return { ok: false, error: `no ${RECORD_PREFIX} record found for ${domain} yet (DNS can take a few minutes)` }; + } + + // resolveTxt returns arrays of string chunks — a value over 255 bytes is split, so join first. + for (const chunks of records) { + if (chunks.join('').trim() === wantTxt) return { ok: true, via: 'TXT' }; + } + + // Present but wrong is a different problem from absent, and the fixes differ: one needs + // correcting, the other needs publishing. A wildcard TXT lands here, which is right — it answers + // with its own value, and that is not a proof of anything. (A wildcard CNAME is refused above.) + if (records.length) { + const found = records.map((c) => c.join('')).join('; '); + return { ok: false, error: `${name} exists but does not match. Found: ${found}` }; + } + return { ok: false, error: `no ${RECORD_PREFIX} record found for ${domain} yet (DNS can take a few minutes)` }; +} + +module.exports = { + check, instructions, newToken, recordName, isClaimExpired, + CLAIM_TTL_S, RECORD_PREFIX, TXT_PREFIX, +}; diff --git a/server/lib/oidc-providers.js b/server/lib/oidc-providers.js new file mode 100644 index 0000000..dd789b2 --- /dev/null +++ b/server/lib/oidc-providers.js @@ -0,0 +1,400 @@ +'use strict'; + +/* + * Which identity providers this instance offers. + * + * Providers are resolved through ONE function on purpose. Instance-wide providers come from the + * environment today; per-organization SSO will come from the database later, and when it does it + * plugs in here rather than growing a second login path. The rest of the app only ever asks + * "give me the provider called X" and never learns where the answer came from. + * + * ── Configuration ──────────────────────────────────────────────────────────────────────────── + * + * OIDC_PROVIDERS=okta,authentik comma-separated slugs to enable + * OIDC_OKTA_ISSUER=https://example.okta.com + * OIDC_OKTA_CLIENT_ID=... + * OIDC_OKTA_CLIENT_SECRET=... optional — PKCE means a public client works + * OIDC_OKTA_NAME=Okta optional button label + * OIDC_OKTA_SCOPES=openid email profile optional + * + * Google and Microsoft are ordinary OIDC providers and are registered automatically from the + * variables the README has always documented (GOOGLE_CLIENT_ID, MICROSOFT_CLIENT_ID + + * MICROSOFT_TENANT_ID), so an existing deployment keeps working without editing anything. They get + * no special code path — the only difference is that their issuer is filled in for you. + */ + +const GOOGLE_ISSUER = 'https://accounts.google.com'; +const DEFAULT_SCOPES = 'openid email profile'; + +/** A slug has to be safe in a URL path and in an env var name. */ +const SLUG_RE = /^[a-z0-9][a-z0-9_-]{0,30}$/; + +/* + * `local` is what users.auth_provider says for a password account, so a provider by that name would + * make every federated login look like a password login to the linking rules — and would put a NULL + * password_hash on rows that POST /login then feeds straight to bcrypt.compareSync. Reserved rather + * than merely discouraged. + */ +const RESERVED_SLUGS = new Set(['local', 'recovery']); + +function envKey(slug, suffix) { + return `OIDC_${slug.toUpperCase().replace(/-/g, '_')}_${suffix}`; +} + +function fromEnv(env, slug) { + const issuer = (env[envKey(slug, 'ISSUER')] || '').trim().replace(/\/+$/, ''); + const clientId = (env[envKey(slug, 'CLIENT_ID')] || '').trim(); + if (!issuer || !clientId) return null; + return { + slug, + name: (env[envKey(slug, 'NAME')] || '').trim() || slug.replace(/[-_]/g, ' '), + issuer, + clientId, + clientSecret: (env[envKey(slug, 'CLIENT_SECRET')] || '').trim() || null, + scopes: (env[envKey(slug, 'SCOPES')] || '').trim() || DEFAULT_SCOPES, + source: 'env', + }; +} + +/** + * Every provider this instance offers, in a stable order. + * + * ⚠️ Never returns clientSecret to a caller that only wants to draw buttons — see publicList(). + */ +function list(env = process.env) { + const out = []; + const seen = new Set(); + + // Back-compat: the two providers the README documented before generic OIDC existed. + const googleId = (env.GOOGLE_CLIENT_ID || '').trim(); + if (googleId) { + out.push({ + slug: 'google', + name: 'Google', + issuer: GOOGLE_ISSUER, + clientId: googleId, + clientSecret: (env.GOOGLE_CLIENT_SECRET || '').trim() || null, + scopes: DEFAULT_SCOPES, + source: 'env', + }); + seen.add('google'); + } + + const msId = (env.MICROSOFT_CLIENT_ID || '').trim(); + if (msId) { + /* + * ⚠️ A TENANT GUID IS REQUIRED. `common` and `organizations` are refused, for two reasons that + * point the same way. + * + * It does not work: Microsoft's multi-tenant metadata advertises + * `https://login.microsoftonline.com/{tenantid}/v2.0` — a literal template — so the issuer can + * never equal the configured URL and every login fails at /start regardless. + * + * And the obvious patch is dangerous: loosening the `iss` comparison to accept the template + * means accepting tokens from EVERY Azure tenant, which is nOAuth — an admin of any tenant can + * set an arbitrary, unverified `email` on one of their own users and be issued a session as that + * address here. Doing multi-tenant Microsoft safely needs per-tenant pinning (validate `tid` + * against an allowlist and key the account on `oid`+`tid`, not on email), which is a feature, + * not a relaxed regex. + * + * So: refuse loudly at boot rather than ship a login that either never works or works too well. + */ + const rawTenant = (env.MICROSOFT_TENANT_ID || '').trim().toLowerCase(); + if (!rawTenant || ['common', 'organizations', 'consumers'].includes(rawTenant)) { + if (!list._warned) { + console.warn('[sso] MICROSOFT_CLIENT_ID is set but MICROSOFT_TENANT_ID is missing or multi-tenant ' + + `(${rawTenant || 'unset'}). Microsoft sign-in is DISABLED: set your tenant GUID. See README.`); + list._warned = true; + } + seen.add('microsoft'); + } else { + out.push({ + slug: 'microsoft', + name: 'Microsoft', + // A tenant GUID narrows the issuer to that tenant, so a token from any other tenant fails + // the `iss` check instead of being quietly accepted. + issuer: `https://login.microsoftonline.com/${rawTenant}/v2.0`, + clientId: msId, + clientSecret: (env.MICROSOFT_CLIENT_SECRET || '').trim() || null, + scopes: DEFAULT_SCOPES, + source: 'env', + }); + seen.add('microsoft'); + } + } + + for (const raw of String(env.OIDC_PROVIDERS || '').split(',')) { + const slug = raw.trim().toLowerCase(); + if (!slug || seen.has(slug)) continue; + if (!SLUG_RE.test(slug) || RESERVED_SLUGS.has(slug)) continue; // ignore rather than crash a boot over a typo + const p = fromEnv(env, slug); + if (p) { out.push(p); seen.add(slug); } + } + + return out; +} + +/** One provider by slug, or null. This is the seam per-org SSO will extend. */ +function get(slug, env = process.env) { + if (!slug || !SLUG_RE.test(String(slug))) return null; + const fromEnvList = list(env).find((p) => p.slug === slug); + if (fromEnvList) return fromEnvList; + // Instance providers win a name clash, which cannot happen in practice (org slugs are random) + // but decides it deterministically if it ever did. + return getOrgProvider(slug); +} + +/** + * What the login page is allowed to know: enough to draw a button and nothing else. + * No client ids, because the browser never talks to the provider directly any more — the redirect + * is built server-side, so there is nothing for the page to do with one. + */ +function publicList(env = process.env) { + return list(env).map((p) => ({ slug: p.slug, name: p.name })); +} + + +/* ──────────────────────────────────────────────────────────────────────────────────────────── + * Per-organization providers. + * + * Loaded lazily so this module stays usable (and testable) without a database — the env-only paths + * above never touch it. An org provider is an ordinary provider once loaded: the login flow cannot + * tell the difference, which is the whole point of resolving everything through get(). + */ + +let _db = null; +function db() { + if (_db === null) { + try { _db = require('../db/database').db; } catch { _db = false; } + } + return _db || null; +} + +function rowToProvider(row, secretbox) { + return { + slug: row.slug, + name: row.name, + issuer: String(row.issuer).replace(/\/+$/, ''), + clientId: row.client_id, + /* + * Fail CLOSED. secretbox.decrypt returns null when the key has rotated, which silently turned a + * confidential client into a public one — the login then fails at the provider with an error + * nobody can act on, while the admin screen still says "a secret is set". + */ + clientSecret: row.client_secret_enc + ? (secretbox.decrypt(row.client_secret_enc) ?? (() => { throw new Error('client secret could not be decrypted — re-enter it'); })()) + : null, + scopes: row.scopes || DEFAULT_SCOPES, + source: 'org', + organizationId: row.organization_id, + /* + * ⚠️ VERIFIED domains only — never org_sso_providers.email_domains. + * + * That column is what an admin typed. This is what they PROVED, by publishing a record in the + * domain's own DNS, and it is the only thing the login callback may confine an assertion to. + * Reading the typed column here would reduce the whole verification feature to a decoration: + * a tenant could type any company's domain and immediately assert addresses in it. + */ + emailDomains: verifiedDomainsFor(row.id).join(','), + }; +} + +/** The domains a provider has actually proved it controls. */ +function verifiedDomainsFor(providerId) { + const conn = db(); + if (!conn) return []; + try { + return conn.prepare('SELECT domain FROM org_sso_domains WHERE provider_id = ? AND verified_at IS NOT NULL') + .all(providerId).map((r) => r.domain); + } catch (e) { + if (/no such table/i.test(e.message)) return []; + throw e; + } +} + +/** One org provider by its (globally unique) slug, or null. */ +function getOrgProvider(slug) { + const conn = db(); + if (!conn || !slug || !SLUG_RE.test(String(slug))) return null; + try { + const row = conn.prepare('SELECT * FROM org_sso_providers WHERE slug = ? AND enabled = 1').get(String(slug)); + if (!row) return null; + return rowToProvider(row, require('./secretbox')); + } catch (e) { + /* + * Only "the table is not there yet" is a null. This catch used to swallow EVERYTHING, which + * turned a secret that could not be decrypted back into a silent success — the exact failure the + * fail-closed check above exists to prevent. Anything else propagates so it is logged and the + * login fails loudly. + */ + if (/no such table/i.test(e.message)) return null; + throw e; + } +} + +/** + * Who owns a provider slug — without decrypting anything, and regardless of whether it is enabled. + * + * The linking rules need to know which ORGANIZATION established an account, not how to talk to its + * provider, and asking get() for that has two problems: it fails closed on an undecryptable secret + * (right for a login, wrong for an ownership question) and it hides disabled rows, which still own + * the accounts they created. + * + * null means "nothing here owns that slug" — either it never existed or the provider has since been + * deleted, and those are deliberately the same answer. + */ +function ownerOf(slug) { + if (!slug || !SLUG_RE.test(String(slug))) return null; + if (list().some((p) => p.slug === slug)) return { source: 'env', organizationId: null }; + const conn = db(); + if (!conn) return null; + try { + const row = conn.prepare('SELECT organization_id FROM org_sso_providers WHERE slug = ?').get(String(slug)); + return row ? { source: 'org', organizationId: row.organization_id } : null; + } catch (e) { + if (/no such table/i.test(e.message)) return null; + throw e; + } +} + +/** + * Which provider, if any, owns an email address. + * + * Domain routing is what makes per-org SSO usable: a customer's staff type their work address and + * are sent to their own identity provider rather than being asked for a password they do not have. + * + * ⚠️ Matched on the domain ONLY, never on whether the address exists. Answering "yes, that domain + * uses SSO" tells an attacker nothing they could not learn from the customer's website; answering + * "yes, that USER exists" would be an account-enumeration oracle on the login page. + */ +function forEmail(email) { + const conn = db(); + if (!conn) return null; + const at = String(email || '').lastIndexOf('@'); + if (at === -1) return null; + const domain = String(email).slice(at + 1).toLowerCase().trim(); + if (!domain) return null; + try { + /* + * Routing is driven by the VERIFIED domain table, not by the text an admin typed, and the JOIN + * is what enforces it — an unverified claim cannot send anyone anywhere. + * + * No ORDER BY: `domain` is UNIQUE, so at most one row can match and there is no tie to break. + * An earlier version ordered here and the comment claimed it decided a race; it decided + * nothing, and saying so invited someone to rely on it. + */ + const row = conn.prepare(` + SELECT p.* FROM org_sso_domains d + JOIN org_sso_providers p ON p.id = d.provider_id + WHERE d.domain = ? AND d.verified_at IS NOT NULL AND p.enabled = 1 + `).get(domain); + if (row) return rowToProvider(row, require('./secretbox')); + } catch (e) { + // Only a missing table is a null — anything else (a secret that will not decrypt, a schema + // drift) must surface rather than silently answering "this domain has no SSO", which is how a + // fail-closed guarantee turns back into a fail-open one. + if (!/no such table/i.test(e.message)) throw e; + } + return null; +} + +/** + * Is this address inside an organization that REQUIRES its identity provider? + * + * Only a VERIFIED domain can compel anyone: an org must not be able to switch off password login + * for a domain it merely typed, which would be a denial-of-service against a company it has nothing + * to do with. Enabled providers only, for the same reason a disabled provider routes nobody. + */ +function ssoOnlyForEmail(email) { + const conn = db(); + if (!conn) return null; + const at = String(email || '').lastIndexOf('@'); + if (at === -1) return null; + // A trailing root dot is the same domain; `acme.test.` slipped the match and let someone + // register at an SSO-only domain (a distinct string, so no squat — but a hole in the gate). + const domain = String(email).slice(at + 1).toLowerCase().trim().replace(/\.+$/, ''); + if (!domain) return null; + try { + return conn.prepare(` + SELECT o.id AS organization_id, o.name AS organization_name, p.slug + FROM org_sso_domains d + JOIN org_sso_providers p ON p.id = d.provider_id + JOIN organizations o ON o.id = d.organization_id + WHERE d.domain = ? AND d.verified_at IS NOT NULL AND p.enabled = 1 AND o.sso_only = 1 + `).get(domain) || null; + } catch (e) { + /* + * ⚠️ FAIL CLOSED. This used to swallow `no such column` and return null — and null means "not + * SSO-only", i.e. password login proceeds. It is the single control stopping a password from + * bypassing a customer's identity provider, so a schema problem must never be the thing that + * quietly switches it off. The sibling forEmail() carries the same warning for the same reason. + * + * `no such table` on the DOMAINS table is different and genuinely means "this instance has no + * per-org SSO at all", so it stays a null. + */ + /* + * "The feature is not installed" and "the schema drifted" are different answers. + * + * A missing per-org SSO table, or no organizations table at all, means this instance has no + * per-organization SSO — nothing is being bypassed, so null is correct and a single-tenant + * install must keep working. A missing sso_only COLUMN on a table that does exist is drift, and + * that is the case that must never quietly answer "not required". + */ + if (/no such table: (org_sso_domains|org_sso_providers|organizations|organization_members)/i.test(e.message)) return null; + console.error('[sso] could not determine SSO-only status, refusing password login:', e.message); + throw e; + } +} + +/** + * Must THIS USER use single sign-on? + * + * ⚠️ Membership, not just the address. ssoOnlyForEmail() answers about a DOMAIN, and a review used + * that gap to walk straight in: any account in the tenant whose address sits outside the verified + * domains kept password login — a contractor, an MSP, the one address nobody remembered. Worse, it + * could be manufactured on demand, because an org admin can create a local password account at any + * address and bind it to their workspace. Enforcing on the domain alone protects the domain; it + * does not protect the ORGANIZATION, which is what the setting claims to do. + * + * So both are asked: the address's domain (which catches people who are not members yet) and every + * organization the user actually belongs to. + */ +function ssoOnlyForUser(user) { + if (!user) return null; + const byDomain = ssoOnlyForEmail(user.email); + if (byDomain) return byDomain; + + const conn = db(); + if (!conn) return null; + try { + /* + * ⚠️ WORKSPACE membership, not just organization_members. + * + * Almost nobody is in `organization_members`: only three places write it (creating an org, + * an org-SSO login, a platform admin creating an org) and nothing ever deletes a row. Every + * INVITED user, every admin-created account and every workspace assignment lands in + * `workspace_members` and nowhere else — so an earlier version of this check covered org + * owners and people who had already used SSO, which is exactly the set the domain check + * already caught. A review invited an outside address into an SSO-only tenant and kept + * password login, then used it to invite more. + */ + return conn.prepare(` + SELECT o.id AS organization_id, o.name AS organization_name + FROM organizations o + WHERE o.sso_only = 1 + AND (EXISTS (SELECT 1 FROM organization_members m WHERE m.organization_id = o.id AND m.user_id = ?) + OR EXISTS (SELECT 1 FROM workspace_members wm + JOIN workspaces w ON w.id = wm.workspace_id + WHERE w.organization_id = o.id AND wm.user_id = ?)) + LIMIT 1 + `).get(user.id, user.id) || null; + } catch (e) { + if (/no such table: (organization_members|organizations|workspace_members|workspaces)/i.test(e.message)) return null; + throw e; // drift on a table that exists — fail closed; the caller refuses the login + } +} + +module.exports = { + list, get, publicList, getOrgProvider, ownerOf, forEmail, + ssoOnlyForEmail, ssoOnlyForUser, DEFAULT_SCOPES, SLUG_RE, +}; diff --git a/server/lib/oidc.js b/server/lib/oidc.js new file mode 100644 index 0000000..b8b5701 --- /dev/null +++ b/server/lib/oidc.js @@ -0,0 +1,332 @@ +'use strict'; + +/* + * OpenID Connect — discovery, key handling and ID-token verification. + * + * This exists because the previous "OAuth" support verified nothing that mattered. The Google path + * asked Google's tokeninfo endpoint whether an ACCESS token was valid and then trusted the email in + * the reply; the Microsoft path handed a bearer token to Graph /me and trusted that. Neither ever + * checked WHO THE TOKEN WAS ISSUED FOR, and an access token is not a proof of identity — it is a + * bearer credential for some resource, minted for some application, and Graph will happily describe + * the user behind a token issued to somebody else's app. Any site a user signs into that asks for + * `email` or `User.Read` could replay that token here and be issued a session as that user. + * + * So identity now comes from an ID TOKEN and nothing else, and the token has to survive: + * + * signature against the provider's published JWKS, restricted to asymmetric algorithms + * iss exactly the issuer discovery advertised + * aud contains our client_id (and azp === client_id when the token carries one) + * exp/nbf inside a small clock skew + * nonce equal to the one WE generated for this login, which is what stops a token + * obtained elsewhere — even a correctly-audienced one — being replayed here + * + * Deliberately dependency-free beyond `jsonwebtoken`: Node can import a JWK straight into a + * KeyObject, so there is no need for jwks-rsa and no second opinion about what a key is. + */ + +const crypto = require('crypto'); +const net = require('net'); +const jwt = require('jsonwebtoken'); + +/* + * `alg: "none"` is the oldest JWT attack there is, and HMAC is nearly as bad here: an HS256 token is + * verified with a SHARED SECRET, and the only "key" we have for a provider is its PUBLIC one — which + * an attacker also has, and could sign with. Only asymmetric families are ever acceptable. + */ +const ALLOWED_ALGS = ['RS256', 'RS384', 'RS512', 'ES256', 'ES384', 'ES512', 'PS256', 'PS384', 'PS512']; + +// Providers rotate keys and publish new ones ahead of use, so a short cache is safe and a miss is +// cheap. Discovery changes far less often but is cached the same way for one reason: a provider +// outage should not be able to stall every login for as long as it lasts. +const DISCOVERY_TTL_MS = 60 * 60 * 1000; // 1 hour +const JWKS_TTL_MS = 10 * 60 * 1000; // 10 minutes +const FETCH_TIMEOUT_MS = 8000; + +const discoveryCache = new Map(); // issuer -> { at, doc } +const jwksCache = new Map(); // jwks_uri -> { at, keys } + +/* + * Every URL this module fetches is ultimately chosen by whoever configured the provider — and since + * per-org SSO, that is a CUSTOMER, not the operator. Discovery, JWKS and the token endpoint are + * therefore server-side request forgery primitives unless they are constrained. + * + * Two rules, both cheap: + * https only — an http:// target is a plaintext credential leak as well as a way to reach + * services that never expected a request from inside the network. + * public hosts only — loopback, RFC1918, CGNAT, link-local (169.254.169.254 is cloud metadata), + * multicast and reserved ranges, in BOTH address families, including the + * IPv4-mapped IPv6 forms that a prefix match misses. + * + * ⚠️ This is a literal-address check, not full SSRF protection: a hostname that RESOLVES to a + * private address still passes, because refusing that needs resolve-then-pin plumbing that Node's + * fetch does not expose. It raises the bar from "type an internal URL" to "control public DNS". + * README.md documents this limitation under per-organization SSO. + */ +/* + * Addresses are parsed as ADDRESSES and compared by range. This started life as a prefix regex, + * which was wrong in both directions: it missed `[::ffff:127.0.0.1]` — the entire IPv4 space + * re-encoded, which WHATWG URL normalises to `[::ffff:7f00:1]` so no dotted-quad prefix can match, + * and a review reached a loopback service straight through it — while also matching plain TEXT, so + * every hostname beginning "fc" or "fd" was refused (fcm.googleapis.com, fcps.edu). + */ +const BLOCKED_V4 = [ + ['0.0.0.0', 8], // "this network" + ['10.0.0.0', 8], // RFC1918 + ['100.64.0.0', 10], // CGNAT / Tailscale + ['127.0.0.0', 8], // loopback + ['169.254.0.0', 16], // link-local — 169.254.169.254 is cloud metadata + ['172.16.0.0', 12], // RFC1918 + ['192.0.0.0', 24], // IETF protocol assignments + ['192.168.0.0', 16], // RFC1918 + ['198.18.0.0', 15], // benchmarking + ['224.0.0.0', 4], // multicast + ['240.0.0.0', 4], // reserved +]; + +const v4ToInt = (ip) => ip.split('.').reduce((acc, o) => (acc * 256) + Number(o), 0); + +function isBlockedV4(ip) { + const addr = v4ToInt(ip); + return BLOCKED_V4.some(([base, bits]) => { + const mask = bits === 0 ? 0 : (-1 << (32 - bits)) >>> 0; + return (addr & mask) >>> 0 === (v4ToInt(base) & mask) >>> 0; + }); +} + +function isBlockedV6(ip) { + const low = ip.toLowerCase(); + // An IPv4-mapped or IPv4-compatible address is an IPv4 address wearing a hat — judge the IPv4. + const mapped = low.match(/^::(ffff:)?(\d+\.\d+\.\d+\.\d+)$/) + || low.match(/^::(ffff:)?([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); + if (mapped) { + if (mapped[2] && mapped[2].includes('.')) return isBlockedV4(mapped[2]); + const hi = parseInt(mapped[2], 16), lo = parseInt(mapped[3], 16); + return isBlockedV4([hi >> 8, hi & 0xff, lo >> 8, lo & 0xff].join('.')); + } + if (low === '::' || low === '::1') return true; // unspecified (= loopback on Linux), loopback + if (/^f[cd]/.test(low)) return true; // fc00::/7 unique-local + if (/^fe[89ab]/.test(low)) return true; // fe80::/10 link-local + if (/^ff/.test(low)) return true; // multicast + return false; +} + +function assertFetchable(url) { + let u; + try { u = new URL(url); } catch { throw new Error(`not a URL: ${url}`); } + if (u.protocol !== 'https:') throw new Error('provider URLs must use https'); + + /* + * A trailing root dot is a legal, fully-qualified spelling of the same name, and WHATWG URL keeps + * it — so `https://localhost./` matched neither alternative below and was ALLOWED. The parser + * normalises the literal-IP forms itself (`127.0.0.1.` becomes `127.0.0.1`), so only the name + * form slipped, and on a resolver that synthesizes `localhost.` it resolves to loopback. + */ + const host = u.hostname.replace(/\.$/, ''); + // URL keeps IPv6 literals in brackets; net.isIP does not want them. + const bare = host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host; + const family = net.isIP(bare); + + const blocked = family === 4 ? isBlockedV4(bare) + : family === 6 ? isBlockedV6(bare) + : /^(localhost|.*\.localhost)$/i.test(host); + + if (blocked) throw new Error('provider host is not publicly routable'); + return u; +} + +/** fetch with a timeout, because a hanging IdP must not hang a login forever. */ +async function getJson(url) { + assertFetchable(url); + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), FETCH_TIMEOUT_MS); + try { + /* + * redirect: 'manual' — following redirects would let an allowlisted host bounce us to a blocked + * one, which defeats the check above entirely. A provider that redirects its own well-known + * document is misconfigured, and saying so is more useful than quietly following it. + */ + const res = await fetch(url, { signal: ctl.signal, redirect: 'manual' }); + if (res.status >= 300 && res.status < 400) throw new Error(`${url} redirected; provider URLs must be final`); + if (!res.ok) throw new Error(`${url} responded ${res.status}`); + return await res.json(); + } finally { + clearTimeout(timer); + } +} + +/** + * The provider's own description of itself. + * + * ⚠️ The discovered `issuer` is checked against the configured one. Discovery is fetched over TLS + * from a URL derived from the issuer, so this is belt-and-braces — but a provider whose document + * claims a DIFFERENT issuer is either misconfigured or hostile, and either way its tokens must not + * be accepted under a name it does not own. + */ +async function discover(issuer) { + const key = String(issuer).replace(/\/+$/, ''); + const hit = discoveryCache.get(key); + if (hit && Date.now() - hit.at < DISCOVERY_TTL_MS) return hit.doc; + + const url = `${key}/.well-known/openid-configuration`; + const doc = await getJson(url); + + const advertised = String(doc.issuer || '').replace(/\/+$/, ''); + if (advertised !== key) { + throw new Error(`discovery issuer mismatch: configured ${key}, document says ${doc.issuer}`); + } + for (const required of ['authorization_endpoint', 'token_endpoint', 'jwks_uri']) { + if (!doc[required]) throw new Error(`discovery for ${key} is missing ${required}`); + } + + discoveryCache.set(key, { at: Date.now(), doc }); + return doc; +} + +/** + * The signing key for one token. + * + * An unknown `kid` forces ONE refresh: that is the normal shape of a key rotation, and refusing to + * refetch would fail every login until the cache expired. It is bounded to one refresh per call so + * a token quoting nonsense cannot be used to hammer the provider. + */ +async function keyForKid(jwksUri, kid) { + let entry = jwksCache.get(jwksUri); + const fresh = entry && Date.now() - entry.at < JWKS_TTL_MS; + + if (!fresh || !entry.keys.some((k) => k.kid === kid)) { + const doc = await getJson(jwksUri); + entry = { at: Date.now(), keys: Array.isArray(doc.keys) ? doc.keys : [] }; + jwksCache.set(jwksUri, entry); + } + + const jwk = entry.keys.find((k) => k.kid === kid) + // A provider with exactly one key may omit kid entirely; anything ambiguous is refused rather + // than guessed, because "try each key until one verifies" is how you accept a key you did not mean to. + || (!kid && entry.keys.length === 1 ? entry.keys[0] : null); + if (!jwk) throw new Error(`no signing key for kid ${kid || '(none)'}`); + + return crypto.createPublicKey({ key: jwk, format: 'jwk' }); +} + +/** + * Verify an ID token and return its claims. + * + * `nonce` is REQUIRED by this function even though the spec makes it conditional. Every flow here + * is a browser login we initiated, so we always have one to compare — and it is the single check + * that distinguishes "a token minted for us, now" from "a token minted for us at some point, + * captured, and replayed". + */ +async function verifyIdToken(idToken, { issuer, clientId, nonce }) { + if (!idToken || typeof idToken !== 'string') throw new Error('no id_token'); + if (!nonce) throw new Error('no nonce to verify against'); + + const decoded = jwt.decode(idToken, { complete: true }); + if (!decoded || !decoded.header) throw new Error('id_token is not a JWT'); + if (!ALLOWED_ALGS.includes(decoded.header.alg)) { + throw new Error(`refusing id_token algorithm ${decoded.header.alg}`); + } + + const doc = await discover(issuer); + const key = await keyForKid(doc.jwks_uri, decoded.header.kid); + + // jsonwebtoken checks signature, exp, nbf, iss and aud. The algorithm allowlist is passed + // explicitly so the header cannot choose how it is verified. + const claims = jwt.verify(idToken, key, { + algorithms: ALLOWED_ALGS, + issuer: doc.issuer, + audience: clientId, + clockTolerance: 60, + }); + + if (claims.nonce !== nonce) throw new Error('id_token nonce does not match this login'); + + /* + * azp names the party the token was issued TO when it differs from the audience. If it is present + * it must be us: a token with our client_id merely in a multi-valued `aud`, issued to a different + * application, is exactly the confused-deputy case this whole file exists to prevent. + */ + if (claims.azp && claims.azp !== clientId) { + throw new Error('id_token was issued to a different application'); + } + if (!claims.sub) throw new Error('id_token has no subject'); + + return claims; +} + +/** PKCE S256. The verifier never leaves us; only its hash goes to the provider. */ +function createPkce() { + const verifier = crypto.randomBytes(32).toString('base64url'); + const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); + return { verifier, challenge, method: 'S256' }; +} + +const randomToken = () => crypto.randomBytes(32).toString('base64url'); + +/** + * Exchange the authorization code. + * + * PKCE means a public client needs no secret, which is what lets a self-hoster configure a provider + * without one. A secret is still sent when configured, because some providers (and some admins) + * require confidential clients. + */ +async function exchangeCode({ issuer, clientId, clientSecret, code, redirectUri, verifier }) { + const doc = await discover(issuer); + const body = new URLSearchParams({ + grant_type: 'authorization_code', + code, + redirect_uri: redirectUri, + client_id: clientId, + code_verifier: verifier, + }); + + const headers = { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json' }; + if (clientSecret) { + // client_secret_basic is the form every provider accepts; client_secret_post is not universal. + headers.Authorization = 'Basic ' + Buffer.from(`${encodeURIComponent(clientId)}:${encodeURIComponent(clientSecret)}`).toString('base64'); + } + + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), FETCH_TIMEOUT_MS); + let payload; + try { + assertFetchable(doc.token_endpoint); + const res = await fetch(doc.token_endpoint, { method: 'POST', headers, body, signal: ctl.signal, redirect: 'manual' }); + payload = await res.json().catch(() => ({})); + if (!res.ok) { + // The provider's own error is far more useful than "exchange failed" — a wrong redirect_uri + // or an unregistered client is the overwhelmingly common setup mistake and it says so here. + throw new Error(payload.error_description || payload.error || `token endpoint responded ${res.status}`); + } + } finally { + clearTimeout(timer); + } + + if (!payload.id_token) throw new Error('provider returned no id_token — is the openid scope requested?'); + return payload; +} + +/** Test seam: drop cached discovery/JWKS so a test can change what a provider claims. */ +function _resetCaches() { + discoveryCache.clear(); + jwksCache.clear(); +} + +/** + * The provider's published keys, straight from the document. Used by the configuration test so an + * admin learns at setup time that a provider publishes no signing keys, rather than at first login. + */ +async function fetchJwks(jwksUri) { + return getJson(jwksUri); +} + +module.exports = { + discover, + assertFetchable, + fetchJwks, + verifyIdToken, + exchangeCode, + createPkce, + randomToken, + ALLOWED_ALGS, + _resetCaches, +}; diff --git a/server/lib/preflight-deps.js b/server/lib/preflight-deps.js new file mode 100644 index 0000000..9be7249 --- /dev/null +++ b/server/lib/preflight-deps.js @@ -0,0 +1,178 @@ +'use strict'; + +/* + * Make sure the dependencies this build needs are actually installed and loadable — BEFORE anything + * requires them. + * + * The normal upgrade path (scripts/upgrade.sh) runs `npm ci --omit=dev`, so this is not for the + * happy case. It is for the three ways a running box ends up with the wrong node_modules: + * + * ROLLBACK checking out an older tag to back out a bad release restores that tag's + * package.json but not its packages, so the server dies on a MODULE_NOT_FOUND for + * something the newer build had removed. That is a bad moment to be reading a + * stack trace: you are already rolling back because something else broke. + * NODE UPGRADE better-sqlite3 is a native module compiled against one ABI. Upgrading Node makes + * every boot fail with NODE_MODULE_VERSION mismatch, which reads like database + * corruption and is not. + * HAND EDITS a `git checkout`, a partly-copied tree, an interrupted install. + * + * All three present as a server that will not start, with an error that names a file rather than + * the action needed. Detecting and repairing is a few seconds; diagnosing is an outage. + * + * ⚠️ Deliberately dependency-free — only Node builtins. Anything it required could be the very + * thing that is missing. + * + * Set ST_SKIP_DEP_PREFLIGHT=1 to turn it off (air-gapped hosts, or an operator who manages + * node_modules themselves and does not want a boot reaching for the network). + */ + +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const SERVER_DIR = path.join(__dirname, '..'); +const NODE_MODULES = path.join(SERVER_DIR, 'node_modules'); +const INSTALL_TIMEOUT_MS = 10 * 60 * 1000; // a cold install on a Pi is genuinely slow + +/** Which declared dependencies are not on disk. */ +function missingDeps() { + let pkg; + try { + pkg = JSON.parse(fs.readFileSync(path.join(SERVER_DIR, 'package.json'), 'utf8')); + } catch { + return []; // no package.json is not our problem to diagnose + } + const declared = Object.keys(pkg.dependencies || {}); + return declared.filter((name) => { + // A scoped or nested name is still one directory below node_modules. + try { return !fs.existsSync(path.join(NODE_MODULES, name, 'package.json')); } catch { return true; } + }); +} + +/** + * Is the native module loadable by THIS Node? + * + * Checked by actually loading it, because the failure is an ABI mismatch that no version string + * comparison catches reliably — a rebuild against the same major can still differ. + */ +function nativeModuleBroken() { + try { + /* + * ⚠️ CONSTRUCT one, do not merely require it. + * + * better-sqlite3's entry point is plain JavaScript and loads the compiled `.node` binding + * lazily, so `require()` alone SUCCEEDS under a Node whose ABI the binary was not built for — + * the first version of this check did exactly that and reported a broken install as healthy, + * verified against a real Node 18 / Node 20 mismatch. Opening an in-memory database is what + * actually pulls the binding in, and it touches no file. + */ + const Database = require('better-sqlite3'); + new Database(':memory:').close(); + return null; + } catch (e) { + const msg = String((e && e.message) || ''); + if (/NODE_MODULE_VERSION|ERR_DLOPEN_FAILED|was compiled against a different/i.test(msg)) return msg; + if (/Cannot find module/i.test(msg)) return msg; + // Anything else is a real error in the module, not an installation problem — let it surface + // later with its own stack rather than being masked by an npm run. + return null; + } +} + +function run(args, label) { + console.log(`[preflight] ${label}: npm ${args.join(' ')}`); + execFileSync('npm', args, { cwd: SERVER_DIR, stdio: 'inherit', timeout: INSTALL_TIMEOUT_MS }); +} + +function fail(reason, hint) { + console.error(`[preflight] ${reason}`); + console.error(`[preflight] ${hint}`); + console.error('[preflight] Set ST_SKIP_DEP_PREFLIGHT=1 to boot without this check.'); + process.exit(1); +} + +function preflight() { + // Same spellings as every other boolean the server accepts, so an operator who writes `true` + // does not silently get a boot that reaches for the registry anyway. + if (['1', 'true', 'yes'].includes(String(process.env.ST_SKIP_DEP_PREFLIGHT || '').toLowerCase())) return; + + const missing = missingDeps(); + const nodeModulesAbsent = !fs.existsSync(NODE_MODULES); + + if (missing.length || nodeModulesAbsent) { + const what = nodeModulesAbsent + ? 'node_modules is missing' + : `${missing.length} dependency/dependencies missing: ${missing.slice(0, 6).join(', ')}${missing.length > 6 ? '…' : ''}`; + console.warn(`[preflight] ${what} — installing before start.`); + try { + /* + * `npm ci` when there is a lockfile and nothing installed: it is reproducible and it is what + * upgrade.sh uses. Otherwise `npm install`, because `ci` DELETES node_modules first and would + * throw away a working tree to fix one missing package. + */ + const hasLock = fs.existsSync(path.join(SERVER_DIR, 'package-lock.json')); + if (hasLock && nodeModulesAbsent) { + /* + * Nothing installed, so `ci` has nothing to destroy and gives a reproducible tree. + * + * `--omit=dev` ONLY when this is plainly a production boot. Applying it unconditionally + * meant a cold start on a developer machine installed 307 packages and left `npm test` + * broken — js-yaml, puppeteer-core and socket.io-client absent — which is the same class of + * surprise as the prune this file already warns about, arriving through the other branch of + * the same `if`. + */ + const prod = process.env.NODE_ENV === 'production'; + run(prod ? ['ci', '--omit=dev', '--no-audit', '--no-fund'] : ['ci', '--no-audit', '--no-fund'], 'installing'); + } else { + /* + * ⚠️ Install ONLY what is missing, by name, and never `--omit=dev` on a populated tree. + * + * `npm install --omit=dev` reconciles the whole tree, which PRUNES devDependencies — so + * merely starting the server deleted socket.io-client, puppeteer-core and js-yaml, and broke + * `npm test`. A review watched it happen. A boot-time repair that quietly removes packages + * is worse than the failure it fixes, so this touches nothing it was not asked to. + * + * `--no-save` because a server starting up has no business editing package.json. + */ + run(['install', '--no-save', '--no-audit', '--no-fund', ...missing], 'installing missing packages'); + } + } catch (e) { + /* + * An install can fail because ANOTHER server started at the same moment and won the race — + * observed as `ENOTEMPTY … rename node_modules/fs-extra`. The tree is complete by the time we + * see the error, so exiting here killed a process that had nothing wrong with it. Re-check + * before giving up; only a genuinely incomplete tree is fatal. + */ + const afterFailure = missingDeps(); + if (afterFailure.length) { + fail(`could not install dependencies: ${e && e.message}`, + 'Run `npm ci --omit=dev` in the server directory, or check network access to the npm registry.'); + } + console.warn(`[preflight] install reported an error but the tree is complete (${e && e.message}) — continuing.`); + } + const still = missingDeps(); + if (still.length) { + fail(`still missing after install: ${still.join(', ')}`, 'Check the npm output above.'); + } + console.log('[preflight] dependencies installed.'); + } + + const nativeProblem = nativeModuleBroken(); + if (nativeProblem) { + console.warn(`[preflight] better-sqlite3 will not load under Node ${process.version} — rebuilding.`); + console.warn(`[preflight] ${nativeProblem.split('\n')[0]}`); + try { + run(['rebuild', 'better-sqlite3'], 'rebuilding native module'); + } catch (e) { + fail(`could not rebuild better-sqlite3: ${e && e.message}`, + `Run \`npm rebuild better-sqlite3\` in the server directory. This usually means Node changed version (now ${process.version}) and the module needs recompiling; a build toolchain (python3, make, g++) must be present.`); + } + if (nativeModuleBroken()) { + fail('better-sqlite3 still will not load after a rebuild.', + 'Delete server/node_modules and run `npm ci --omit=dev`.'); + } + console.log('[preflight] native module rebuilt.'); + } +} + +module.exports = { preflight, missingDeps, nativeModuleBroken }; diff --git a/server/lib/public-email-domains.js b/server/lib/public-email-domains.js new file mode 100644 index 0000000..ec4b64b --- /dev/null +++ b/server/lib/public-email-domains.js @@ -0,0 +1,57 @@ +'use strict'; + +/* + * Email domains nobody may claim for organization SSO. + * + * Per-org SSO routes everyone at a domain to that organization's identity provider. Applied to a + * company domain that is the point. Applied to a CONSUMER domain it is an attack: one tenant claims + * `gmail.com`, and from then on every Gmail user who types their address into this product's login + * page is offered a "sign in with your organization" button that sends them to infrastructure the + * tenant controls — phishing launched from the vendor's own trusted login screen. First-claim-wins + * also lets one cheap account deny a public domain to everyone else, permanently. + * + * ⚠️ This is a floor, not a ceiling. It stops the mass-abuse case; it does NOT stop a tenant + * claiming a domain that belongs to some specific other company. Only proof of control — a DNS TXT + * record, or a challenge to postmaster@ — settles that, and until it exists a claimed domain means + * "nobody else had claimed it", not "they own it". + * + * Kept as data, in one file, because it is a list that will need adding to and that is the cheapest + * possible edit. Matching is exact on the registrable domain, so `mail.google.com` is not blocked by + * `gmail.com` — subdomains of consumer providers are not a realistic sign-in domain anyway. + */ + +const PUBLIC_EMAIL_DOMAINS = new Set([ + // Google + 'gmail.com', 'googlemail.com', + // Microsoft + 'outlook.com', 'outlook.co.uk', 'hotmail.com', 'hotmail.co.uk', 'hotmail.fr', 'hotmail.it', + 'live.com', 'live.co.uk', 'msn.com', 'passport.com', + // Yahoo and friends + 'yahoo.com', 'yahoo.co.uk', 'yahoo.co.jp', 'yahoo.fr', 'yahoo.de', 'yahoo.ca', 'yahoo.com.au', + 'ymail.com', 'rocketmail.com', 'aol.com', 'aim.com', + // Apple + 'icloud.com', 'me.com', 'mac.com', + // Privacy-focused + 'proton.me', 'protonmail.com', 'pm.me', 'tutanota.com', 'tutanota.de', 'tuta.io', 'tuta.com', + 'duck.com', 'hey.com', 'fastmail.com', 'fastmail.fm', + // Other large consumer providers + 'gmx.com', 'gmx.de', 'gmx.net', 'gmx.at', 'gmx.ch', 'web.de', 'mail.com', 'email.com', + 'zoho.com', 'zohomail.com', 'yandex.com', 'yandex.ru', 'ya.ru', 'mail.ru', 'bk.ru', 'inbox.ru', + 'list.ru', 'rambler.ru', + 'qq.com', 'foxmail.com', '163.com', '126.com', 'sina.com', 'sina.cn', 'naver.com', 'daum.net', + 'hanmail.net', 'rediffmail.com', + // ISP-style mailboxes, where the domain belongs to the ISP and not to any customer + 'comcast.net', 'verizon.net', 'att.net', 'sbcglobal.net', 'bellsouth.net', 'cox.net', + 'charter.net', 'earthlink.net', 'juno.com', 'optonline.net', 'roadrunner.com', + 'btinternet.com', 'sky.com', 'virginmedia.com', 'talktalk.net', 'orange.fr', 'wanadoo.fr', + 'free.fr', 'laposte.net', 'libero.it', 'virgilio.it', 'tiscali.it', 'terra.com.br', 'uol.com.br', + 'bol.com.br', 'telus.net', 'shaw.ca', 'rogers.com', 'sympatico.ca', 'bigpond.com', 'optusnet.com.au', + 't-online.de', 'freenet.de', 'arcor.de', +]); + +/** True when this domain is a consumer mailbox provider rather than an organization's own domain. */ +function isPublicEmailDomain(domain) { + return PUBLIC_EMAIL_DOMAINS.has(String(domain || '').trim().toLowerCase()); +} + +module.exports = { PUBLIC_EMAIL_DOMAINS, isPublicEmailDomain }; diff --git a/server/package-lock.json b/server/package-lock.json index dd73fe3..cf07a71 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -15,7 +15,6 @@ "cors": "^2.8.5", "express": "^4.18.2", "express-rate-limit": "^8.3.1", - "google-auth-library": "^10.6.2", "helmet": "^8.1.0", "jsonwebtoken": "^9.0.3", "multer": "^1.4.5-lts.1", @@ -931,6 +930,7 @@ "version": "7.1.4", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-7.1.4.tgz", "integrity": "sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==", + "dev": true, "license": "MIT", "engines": { "node": ">= 14" @@ -1314,15 +1314,6 @@ "prebuild-install": "^7.1.1" } }, - "node_modules/bignumber.js": { - "version": "9.3.1", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.3.1.tgz", - "integrity": "sha512-Ko0uX15oIUS7wJ3Rb30Fs6SkVbLmPBAKdlm7q9+ak9bbIeFf0MwuBsQV6z7+X768/cHsfg+WlysDWJcmthjsjQ==", - "license": "MIT", - "engines": { - "node": "*" - } - }, "node_modules/bindings": { "version": "1.5.0", "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz", @@ -1845,15 +1836,6 @@ "node": ">= 8" } }, - "node_modules/data-uri-to-buffer": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/data-uri-to-buffer/-/data-uri-to-buffer-4.0.1.tgz", - "integrity": "sha512-0R9ikRb668HB7QDxT1vkpuUBtqc53YyAwMwGeUFKRojY/NWKvdZ+9UYtRfGmhqNbRkTSVpMbmyhXipFFv2cb/A==", - "license": "MIT", - "engines": { - "node": ">= 12" - } - }, "node_modules/debug": { "version": "2.6.9", "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", @@ -2347,12 +2329,6 @@ "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", "license": "MIT" }, - "node_modules/extend": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", - "integrity": "sha512-fjquC59cD7CyW6urNXK0FBufkZcoiGG80wTuPujX590cB5Ttln20E2UB4S/WARVqhXffZl2LNgS+gQdPIIim/g==", - "license": "MIT" - }, "node_modules/extract-zip": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/extract-zip/-/extract-zip-2.0.1.tgz", @@ -2415,29 +2391,6 @@ "pend": "~1.2.0" } }, - "node_modules/fetch-blob": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/fetch-blob/-/fetch-blob-3.2.0.tgz", - "integrity": "sha512-7yAQpD2UMJzLi1Dqv7qFYnPbaPx7ZfFK6PiIxQ4PfkGPyNyl2Ugx+a/umUonmKqjhM4DnfbMvdX6otXq83soQQ==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - }, - { - "type": "paypal", - "url": "https://paypal.me/jimmywarting" - } - ], - "license": "MIT", - "dependencies": { - "node-domexception": "^1.0.0", - "web-streams-polyfill": "^3.0.3" - }, - "engines": { - "node": "^12.20 || >= 14.13" - } - }, "node_modules/file-uri-to-path": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", @@ -2491,18 +2444,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/formdata-polyfill": { - "version": "4.0.10", - "resolved": "https://registry.npmjs.org/formdata-polyfill/-/formdata-polyfill-4.0.10.tgz", - "integrity": "sha512-buewHzMvYL29jdeQTVILecSaZKnt/RJWjoZCF5OW60Z67/GmSLBkOFM7qh1PI3zFNtJbaZL5eQu1vLfazOwj4g==", - "license": "MIT", - "dependencies": { - "fetch-blob": "^3.1.2" - }, - "engines": { - "node": ">=12.20.0" - } - }, "node_modules/forwarded": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/forwarded/-/forwarded-0.2.0.tgz", @@ -2550,34 +2491,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/gaxios": { - "version": "7.1.4", - "resolved": "https://registry.npmjs.org/gaxios/-/gaxios-7.1.4.tgz", - "integrity": "sha512-bTIgTsM2bWn3XklZISBTQX7ZSddGW+IO3bMdGaemHZ3tbqExMENHLx6kKZ/KlejgrMtj8q7wBItt51yegqalrA==", - "license": "Apache-2.0", - "dependencies": { - "extend": "^3.0.2", - "https-proxy-agent": "^7.0.1", - "node-fetch": "^3.3.2" - }, - "engines": { - "node": ">=18" - } - }, - "node_modules/gcp-metadata": { - "version": "8.1.2", - "resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-8.1.2.tgz", - "integrity": "sha512-zV/5HKTfCeKWnxG0Dmrw51hEWFGfcF2xiXqcA3+J90WDuP0SvoiSO5ORvcBsifmx/FoIjgQN3oNOGaQ5PhLFkg==", - "license": "Apache-2.0", - "dependencies": { - "gaxios": "^7.0.0", - "google-logging-utils": "^1.0.0", - "json-bigint": "^1.0.0" - }, - "engines": { - "node": ">=18" - } - }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -2717,32 +2630,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/google-auth-library": { - "version": "10.6.2", - "resolved": "https://registry.npmjs.org/google-auth-library/-/google-auth-library-10.6.2.tgz", - "integrity": "sha512-e27Z6EThmVNNvtYASwQxose/G57rkRuaRbQyxM2bvYLLX/GqWZ5chWq2EBoUchJbCc57eC9ArzO5wMsEmWftCw==", - "license": "Apache-2.0", - "dependencies": { - "base64-js": "^1.3.0", - "ecdsa-sig-formatter": "^1.0.11", - "gaxios": "^7.1.4", - "gcp-metadata": "8.1.2", - "google-logging-utils": "1.1.3", - "jws": "^4.0.0" - }, - "engines": { - "node": ">=18" - } - }, - "node_modules/google-logging-utils": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/google-logging-utils/-/google-logging-utils-1.1.3.tgz", - "integrity": "sha512-eAmLkjDjAFCVXg7A1unxHsLf961m6y17QFqXqAXGj/gVkKFrEICfStRfwUlGNfeCEjNRa32JEWOUTlYXPyyKvA==", - "license": "Apache-2.0", - "engines": { - "node": ">=14" - } - }, "node_modules/gopd": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.2.0.tgz", @@ -2857,6 +2744,7 @@ "version": "7.0.6", "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz", "integrity": "sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==", + "dev": true, "license": "MIT", "dependencies": { "agent-base": "^7.1.2", @@ -2870,6 +2758,7 @@ "version": "4.4.3", "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", + "dev": true, "license": "MIT", "dependencies": { "ms": "^2.1.3" @@ -2887,6 +2776,7 @@ "version": "2.1.3", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "dev": true, "license": "MIT" }, "node_modules/iconv-lite": { @@ -3022,15 +2912,6 @@ "js-yaml": "bin/js-yaml.js" } }, - "node_modules/json-bigint": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/json-bigint/-/json-bigint-1.0.0.tgz", - "integrity": "sha512-SiPv/8VpZuWbvLSMtTDU8hEfrZWg/mH/nV/b4o0CYbSxu1UIQPLdwKOCIyLQX+VIPO5vrLX3i8qtqFyhdPSUSQ==", - "license": "MIT", - "dependencies": { - "bignumber.js": "^9.0.0" - } - }, "node_modules/jsonfile": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-6.2.0.tgz", @@ -3371,44 +3252,6 @@ "node": ">=10" } }, - "node_modules/node-domexception": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/node-domexception/-/node-domexception-1.0.0.tgz", - "integrity": "sha512-/jKZoMpw0F8GRwl4/eLROPA3cfcXtLApP0QzLmUT/HuPCZWyB7IY9ZrMeKw2O/nFIqPQB3PVM9aYm0F312AXDQ==", - "deprecated": "Use your platform's native DOMException instead", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - }, - { - "type": "github", - "url": "https://paypal.me/jimmywarting" - } - ], - "license": "MIT", - "engines": { - "node": ">=10.5.0" - } - }, - "node_modules/node-fetch": { - "version": "3.3.2", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-3.3.2.tgz", - "integrity": "sha512-dRB78srN/l6gqWulah9SrxeYnxeddIG30+GOqK/9OlLVyLg3HPnr6SqOWTWOXKRwC2eGYCkZ59NNuSgvSrpgOA==", - "license": "MIT", - "dependencies": { - "data-uri-to-buffer": "^4.0.0", - "fetch-blob": "^3.1.4", - "formdata-polyfill": "^4.0.10" - }, - "engines": { - "node": "^12.20.0 || ^14.13.1 || >=16.0.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/node-fetch" - } - }, "node_modules/node-int64": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz", @@ -4845,15 +4688,6 @@ "node": ">= 0.8" } }, - "node_modules/web-streams-polyfill": { - "version": "3.3.3", - "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.3.3.tgz", - "integrity": "sha512-d2JWLCivmZYTSIoge9MsgFCZrt571BikcWGYkjC1khllbTeDlGqZ2D8vD8E/lJa8WGWbb7Plm8/XJYV7IJHZZw==", - "license": "MIT", - "engines": { - "node": ">= 8" - } - }, "node_modules/webdriver-bidi-protocol": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/webdriver-bidi-protocol/-/webdriver-bidi-protocol-0.4.1.tgz", diff --git a/server/package.json b/server/package.json index 780c65d..c589ffc 100644 --- a/server/package.json +++ b/server/package.json @@ -17,7 +17,6 @@ "cors": "^2.8.5", "express": "^4.18.2", "express-rate-limit": "^8.3.1", - "google-auth-library": "^10.6.2", "helmet": "^8.1.0", "jsonwebtoken": "^9.0.3", "multer": "^1.4.5-lts.1", @@ -32,7 +31,7 @@ }, "devDependencies": { "js-yaml": "^4.2.0", - "socket.io-client": "^4.8.3", - "puppeteer-core": "^24.43.1" + "puppeteer-core": "^24.43.1", + "socket.io-client": "^4.8.3" } } diff --git a/server/routes/admin.js b/server/routes/admin.js index ebf05a3..0e4741d 100644 --- a/server/routes/admin.js +++ b/server/routes/admin.js @@ -3,6 +3,7 @@ const router = express.Router(); const bcrypt = require('bcryptjs'); const { v4: uuidv4 } = require('uuid'); const { db } = require('../db/database'); +const oidcProviders = require('../lib/oidc-providers'); const { canAdminWorkspace } = require('../lib/permissions'); const { requirePlatformAdmin, requireAdmin } = require('../middleware/auth'); const { logActivity, getClientIp } = require('../services/activity'); @@ -20,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; @@ -55,6 +58,53 @@ router.post('/users', (req, res) => { if (!canAdminWorkspace(db, req.user, ws)) { return res.status(403).json({ error: 'Admin access required' }); } + /* + * ⚠️ An SSO-only organization must not have password accounts minted into it. + * + * This route creates a LOCAL account with an admin-chosen password, and it accepts any address — + * so on a tenant that requires single sign-on it was a one-call backdoor: create + * `contractor@somewhere-else.test` bound to the workspace, log in with the password, and every + * control the customer turned SSO-only on for is behind you. A review did exactly that, and the + * account it created could then mint another. + * + * platform_admin keeps the ability, because that is the operator break-glass — the same + * exemption the login gate makes, for the same reason. + */ + if (req.user.role !== 'platform_admin' && ws.organization_id) { + // The table is absent on a single-tenant install; that simply means no organization requires + // single sign-on, so creation proceeds. + let org = null; + try { org = db.prepare('SELECT sso_only, name FROM organizations WHERE id = ?').get(ws.organization_id); } + catch { org = null; } + if (org && org.sso_only) { + return res.status(400).json({ + error: `${org.name || 'This organization'} requires single sign-on, so password accounts cannot be created. Invite the person through your identity provider instead.`, + code: 'sso_only_org', + }); + } + } + + /* + * ⚠️ And the ADDRESS's own domain, wherever it is being created. + * + * Gating only on the target workspace left the squat open through a different door: create your + * own organization, then mint `cfo@theircompany.test` into YOUR workspace. Login is refused, so + * it is not access — but the row now has a password_hash, and an SSO login will not adopt a row + * that has one. The real CFO can then never sign in through their own identity provider, and a + * password reset they CAN complete lands them at a login that refuses them. Permanent, with no + * self-service way out, for any address at any SSO-only customer. + */ + if (req.user.role !== 'platform_admin') { + let ownedBy = null; + try { ownedBy = oidcProviders.ssoOnlyForEmail(email); } catch { ownedBy = { unavailable: true }; } + if (ownedBy) { + return res.status(400).json({ + error: 'That email domain uses single sign-on, so a password account cannot be created for it.', + code: 'sso_only_domain', + }); + } + } + // Stamp the target workspace so the activityLogger middleware (and our // explicit audit row) attribute to the right tenant. req.workspaceId = ws.id; diff --git a/server/routes/auth.js b/server/routes/auth.js index 49b1b45..23a28ff 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -1,9 +1,7 @@ const express = require('express'); const router = express.Router(); const bcrypt = require('bcryptjs'); -const https = require('https'); const { v4: uuidv4 } = require('uuid'); -const { OAuth2Client } = require('google-auth-library'); const { db } = require('../db/database'); const { generateToken, generateMfaPendingToken, verifyMfaPendingToken, requireAuth, requireAdmin, requireSuperAdmin, isPlatformRole, isPlatformStaff, PLATFORM_ROLES } = require('../middleware/auth'); const { resolveTenancy } = require('../lib/tenancy'); @@ -18,6 +16,10 @@ const emailVerify = require('../lib/emailVerify'); const emailSvc = require('../services/email'); const { deleteUserCascade, OrgHasOtherMembersError } = require('../lib/user-deletion'); const config = require('../config'); +const crypto = require('crypto'); +const jwt = require('jsonwebtoken'); +const oidc = require('../lib/oidc'); +const oidcProviders = require('../lib/oidc-providers'); // Phase 2.1: find or create the user's default org+workspace. Returns the // workspace_id to embed in the JWT. Idempotent: if the user already has @@ -100,8 +102,39 @@ 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' }); + /* + * 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' }); @@ -164,11 +197,103 @@ router.post('/login', (req, res) => { const { email, password } = req.body; if (!email || !password) return res.status(400).json({ error: 'Email and password required' }); + /* + * The DOMAIN check runs BEFORE the account lookup, deliberately. + * + * Answering `403 sso_required` only for addresses that exist turned this endpoint into an + * account-existence oracle: a wrong password got 403 for a real address and 401 for an invented + * one. Whether a domain uses single sign-on is already public — /sso/discover answers it for + * 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 }; + } + })(); + 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 + * password is not an alternative way in — otherwise the MFA, conditional access and instant + * deprovisioning the customer bought are all reachable around. + * + * ⚠️ platform_admin is exempt, and that exemption is load-bearing rather than a convenience. The + * operator is the one who approves turning this OFF. If the operator's own address sits at an + * SSO-only domain and that identity provider breaks, nobody can sign in to approve anything and + * the instance is bricked with no path out. The exemption is the break-glass; it applies to the + * people who run the server, never to a customer's own admins. + * + * Said plainly rather than as "invalid email or password": this is not a credential failure and + * pretending otherwise sends the user to reset a password that will never work. The domain + * already answered `sso: true` publicly, so naming it reveals nothing new. + */ + if (user.role !== 'platform_admin') { + /* + * A throw here means we could not determine the answer (schema drift, a broken read). Treat + * that as "SSO is required" rather than letting a 500 escape or, worse, letting the login + * through: the whole point of this gate is that a password must not be an alternative way in, + * and "we could not check" is not "there is nothing to check". + */ + let enforced = null; + try { + // By MEMBERSHIP as well as by domain — an account inside the tenant at an outside address + // was the demonstrated way around this. + enforced = oidcProviders.ssoOnlyForUser(user); + } catch (e) { + console.error('[login] SSO-only status unavailable, refusing password login:', e && e.message); + enforced = { unavailable: true }; + } + if (enforced) { + /* + * Reached only when the ADDRESS's domain is not enforced but the user is a MEMBER of an + * organization that requires single sign-on — an off-domain contractor, say. The generic 401 + * is deliberate: a distinct answer here would put the existence oracle back, for exactly the + * accounts an attacker would most like to enumerate. These people cannot sign in by any + * route (their domain is not verified, so their org's provider will not assert for them + * either), which is why enabling SSO-only now names them to the admin up front instead of + * leaving them to discover it here. + */ + logFailedLogin(email, getClientIp(req), 'Password login refused: member of an SSO-only organization'); + return res.status(401).json({ error: 'Invalid email or password' }); + } + } // Per-ACCOUNT brute-force lockout (lib/login-lockout), on top of the per-IP limiter in // server.js. Checked BEFORE bcrypt so a locked account costs no hashing work. @@ -182,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'); @@ -227,7 +358,7 @@ function issueSession(req, res, user, extra = {}) { // #100: callers pass a SELECT * row. Strip password_hash AND the TOTP internals // (the encrypted secret + the replay counter) so no secret/internal rides in the // response body - "secrets never in responses", same as the API token work. - const { password_hash, totp_secret_enc, totp_last_step, ...safeUser } = user; + const safeUser = publicUser(user); res.json({ token, user: safeUser, current_workspace_id: workspaceId, ...extra }); } @@ -276,12 +407,97 @@ router.post('/resend-verification', (req, res) => { // would turn "read one email" into a full session and quietly bypass MFA. const RESET_GENERIC_OK = { ok: true, message: 'If that address has an account, a reset link is on its way.' }; +/* + * An account whose identity provider no longer exists — and why it may reset a password. + * + * A federated row normally must NOT be resettable: the identity provider owns that account, and + * offering a password would be a way around it. But a provider can be deleted, and the row it + * created outlives it, pointing at a slug nothing answers to. Such an account cannot log in by any + * route: no provider to authenticate against, no password to reset, and registration refuses the + * address as taken. + * + * That is not only an accident. A tenant can claim a domain it does not own (claims are not yet + * verified — see the README), sign in as an address there, delete its provider, and leave the real + * owner permanently unable to reach an account bearing their own address. + * + * Proving control of the MAILBOX is the right way out, and it is strictly stronger evidence than + * the identity-provider assertion that created the row. So an orphaned account may reset, and doing + * so returns it to a local account. A row whose provider still exists is untouched by this. + */ +/* + * What an identity provider is allowed to call an email address. + * + * Exactly one @, no whitespace, no control characters, a domain with at least one dot. Deliberately + * stricter than the RFC — this is not validating what may exist in the world, it is deciding what + * this system will key an ACCOUNT on, and every exotic form is a way for two spellings to look like + * one address to a human and two to the database. + */ +const ASSERTED_EMAIL_RE = /^[^\s@\x00-\x1f]+@[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$/; + +/** + * May this provider speak for this address? + * + * A pure function on purpose: the confinement it implements is the single control standing between + * per-organization SSO and an account-takeover primitive, and a control that can only be exercised + * by standing up a hostile identity provider is a control that does not get tested. It was in fact + * shipped untested once — the test named after it asserted only that a provider row carried two + * fields, and passed with the guard deleted. + * + * `provider.emailDomains` is the VERIFIED set (see rowToProvider), so this cannot be satisfied by a + * domain the tenant merely typed. + */ +function emailAllowedForProvider(provider, email) { + // Instance-wide providers are the operator's own choice and keep the trust they have always had. + if (!provider.organizationId) return true; + const addr = String(email || '').toLowerCase(); + /* + * Malformed addresses are refused rather than tidied. `victim@evil.test@acme.test\n` used to pass + * — lastIndexOf('@') took `acme.test\n`, and trimming turned it into an allowed domain — so an + * address that is not one thing got treated as belonging to a domain it only ended with. Anything + * carrying whitespace, control characters or a second @ is not an address this will reason about. + */ + if (!ASSERTED_EMAIL_RE.test(addr)) return false; + const at = addr.lastIndexOf('@'); + if (at === -1) return false; + const domain = addr.slice(at + 1).trim(); + if (!domain) return false; + // Lowercased on both sides: forEmail lowercases when routing, and a row that differed in case + // would otherwise route a user in and then reject them at the callback. + const allowed = String(provider.emailDomains || '').split(',').map((d) => d.trim().toLowerCase()).filter(Boolean); + return allowed.includes(domain); +} + +function isOrphanedFederated(user) { + if (!user || user.auth_provider === 'local') return false; + /* + * ⚠️ ONLY an organization provider's slug, never an instance one. + * + * This used to ask "does anything answer to that slug?", which cannot tell DELETED apart from + * NOT CURRENTLY CONFIGURED. Unsetting GOOGLE_CLIENT_ID — or fat-fingering MICROSOFT_TENANT_ID to + * `common`, a typo the provider code already refuses — therefore made every account on that + * provider password-resettable instance-wide, and irreversibly: the reset rewrites auth_provider + * to 'local', so restoring the variable does not restore the binding. An organization that chose + * SSO to enforce its IdP's MFA would have had that silently downgraded to mailbox access. + * + * Org slugs are generated as `org` + 12 hex (see org-sso.js), so the shape is decisive: an env + * provider can never match it, and an unconfigured env provider is UNAVAILABLE, not deleted. + * + * Deleting an org provider now returns its users to local accounts outright (org-sso.js), so this + * only catches rows stranded some other way — an interrupted delete, a restored backup. + */ + if (!/^org[0-9a-f]{12}$/.test(String(user.auth_provider))) return false; + return !oidcProviders.ownerOf(user.auth_provider); +} + router.post('/forgot-password', (req, res) => { const email = String(req.body?.email || '').toLowerCase().trim(); // Respond identically no matter what happens below. try { if (email) { - const user = db.prepare("SELECT * FROM users WHERE email = ? AND auth_provider = 'local'").get(email); + const candidate = db.prepare('SELECT * FROM users WHERE email = ?').get(email); + // A local account, or one stranded by a deleted provider — see isOrphanedFederated above. + const user = candidate && (candidate.auth_provider === 'local' || isOrphanedFederated(candidate)) + ? candidate : null; if (user) { if (!emailSvc.isConfigured()) { // Loud, because the user will wait for an email that can never arrive and the @@ -311,7 +527,17 @@ router.post('/reset-password', (req, res) => { // Someone who locked themselves out guessing must not stay locked out after proving // control of the mailbox and choosing a new password. loginLockout.reset(userId); - const u = db.prepare('SELECT email FROM users WHERE id = ?').get(userId); + const u = db.prepare('SELECT email, auth_provider FROM users WHERE id = ?').get(userId); + /* + * Return a stranded federated row to a local account. Without this the reset would "succeed" and + * change nothing anyone can use: POST /login only ever looks at auth_provider = 'local', so the + * new password would be unreachable and the account still lost. + */ + if (isOrphanedFederated(u)) { + db.prepare("UPDATE users SET auth_provider = 'local', provider_id = NULL WHERE id = ?").run(userId); + console.log(`[password-reset] ${u.email} reclaimed from deleted provider ${u.auth_provider}`); + logActivity(userId, 'auth:federated_account_reclaimed', `was ${u.auth_provider}`, null, getClientIp(req)); + } logActivity(userId, 'auth:password_reset_completed', null, null, getClientIp(req)); console.log(`[password-reset] password changed for ${u ? u.email : userId}`); // No session on purpose — see above. @@ -363,7 +589,7 @@ router.get('/totp/status', requireAuth, (req, res) => { // Step 1: mint a pending secret + return the otpauth:// URI + a ready-to-render QR // data URL (drawn server-side with the already-bundled `qrcode` lib, same as the // device-owner provisioning QR). The raw secret is also returned for manual entry. -router.post('/totp/setup', requireAuth, async (req, res) => { +router.post('/totp/setup', requireAuth, asyncRoute(async (req, res) => { const u = db.prepare('SELECT auth_provider, totp_enabled, email FROM users WHERE id = ?').get(req.user.id); if (u.auth_provider !== 'local') return res.status(400).json({ error: 'TOTP is only for password accounts; your identity provider manages MFA.' }); if (u.totp_enabled) return res.status(409).json({ error: 'TOTP already enabled. Disable it first to re-enroll.' }); @@ -380,7 +606,7 @@ router.post('/totp/setup', requireAuth, async (req, res) => { try { qr_data_url = await QRCode.toDataURL(otpauth_uri, { errorCorrectionLevel: 'M', margin: 1, width: 240 }); } catch (e) { /* fall through with qr_data_url = null */ } res.json({ otpauth_uri, secret, qr_data_url }); -}); +})); // Step 2: confirm a code from the user's app, THEN enable + issue recovery codes (once). router.post('/totp/enable', requireAuth, (req, res) => { @@ -456,160 +682,24 @@ router.post('/totp/verify', (req, res) => { // ==================== Google OAuth ==================== -router.post('/google', async (req, res) => { - const { credential } = req.body; - if (!credential) return res.status(400).json({ error: 'Google credential required' }); +/* + * REMOVED 2026-08-10: POST /api/auth/google and POST /api/auth/microsoft. + * + * Both authenticated with an ACCESS token and neither checked who it was issued for. Google's path + * fell back to `tokeninfo?access_token=` and read the email out of the reply; Microsoft's handed the + * bearer token to Graph /me and trusted that. Graph — and tokeninfo — will describe the user behind + * a token minted for SOMEBODY ELSE'S application, so any site a user signed into that requested + * `email` or `User.Read` could replay their token here and be handed a session as them. + * + * Nothing is lost by deleting them: the login page called `google.accounts.oauth2` and + * `new msal.PublicClientApplication`, and neither SDK was ever loaded by any page in this app, so + * both buttons threw ReferenceError on click. The feature had never worked. + * + * Replaced by the OIDC routes at the bottom of this file, which verify an ID token's signature, + * issuer, audience and our own nonce, and which cover Google, Microsoft and any other provider + * through one code path. See lib/oidc.js. + */ - try { - // Verify the Google ID token - const payload = await verifyGoogleToken(credential); - if (!payload) return res.status(401).json({ error: 'Invalid Google token' }); - - const { email, name, picture, sub: googleId } = payload; - - // Find or create user - let user = db.prepare('SELECT * FROM users WHERE email = ?').get(email.toLowerCase()); - const isNewUser = !user; - - if (!user) { - if (!canRegister()) { - return res.status(403).json({ error: 'Public registration is disabled. Contact your administrator.' }); - } - const id = uuidv4(); - const userCount = db.prepare('SELECT COUNT(*) as count FROM users').get().count; - const role = userCount === 0 ? 'platform_admin' : 'user'; - const isFirst = userCount === 0; - const plan = (isFirst && config.selfHosted) ? 'enterprise' : 'pro'; - const trialStarted = isFirst && config.selfHosted ? null : Math.floor(Date.now() / 1000); - - db.prepare(` - INSERT INTO users (id, email, name, auth_provider, provider_id, avatar_url, role, plan_id, trial_started, trial_plan, email_verified) - VALUES (?, ?, ?, 'google', ?, ?, ?, ?, ?, ?, 1) - `).run(id, email.toLowerCase(), name || '', googleId, picture || '', role, plan, trialStarted, trialStarted ? 'pro' : null); - - user = db.prepare('SELECT * FROM users WHERE id = ?').get(id); - } else if (user.auth_provider !== 'google') { - // Existing account with different provider — do NOT silently overwrite auth_provider. - // If they have a local password, require them to log in locally and link from settings. - if (user.password_hash) { - return res.status(409).json({ error: 'An account with this email already exists. Please log in with your password.' }); - } - // No password (e.g. Microsoft → Google switch) — allow linking - db.prepare('UPDATE users SET auth_provider = ?, provider_id = ?, avatar_url = ? WHERE id = ?') - .run('google', googleId, picture || user.avatar_url, user.id); - user = db.prepare('SELECT * FROM users WHERE id = ?').get(user.id); - } - - const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); - const token = generateToken(user, workspaceId); - const { password_hash, ...safeUser } = user; - res.json({ token, user: safeUser, current_workspace_id: workspaceId }); - - // Welcome + admin-notify only when this Google login created a new account. - if (isNewUser) sendSignupEmails(user, req); - } catch (err) { - console.error('Google auth error:', err); - res.status(401).json({ error: 'Google authentication failed' }); - } -}); - -async function verifyGoogleToken(credential) { - const client = new OAuth2Client(config.googleClientId); - try { - const ticket = await client.verifyIdToken({ - idToken: credential, - audience: config.googleClientId || undefined, - }); - return ticket.getPayload(); - } catch (e) { - // Fallback: if credential is an access token, verify via tokeninfo - try { - const res = await fetch(`https://oauth2.googleapis.com/tokeninfo?access_token=${credential}`); - if (!res.ok) throw new Error('Invalid token'); - return await res.json(); - } catch { - throw new Error('Google token verification failed: ' + e.message); - } - } -} - -// ==================== Microsoft OAuth ==================== - -router.post('/microsoft', async (req, res) => { - const { access_token } = req.body; - if (!access_token) return res.status(400).json({ error: 'Microsoft access token required' }); - - try { - // Use the access token to get user profile from Microsoft Graph - const profile = await getMicrosoftProfile(access_token); - if (!profile || !profile.mail && !profile.userPrincipalName) { - return res.status(401).json({ error: 'Could not get Microsoft profile' }); - } - - const email = (profile.mail || profile.userPrincipalName).toLowerCase(); - const name = profile.displayName || ''; - const microsoftId = profile.id; - - // Find or create user - let user = db.prepare('SELECT * FROM users WHERE email = ?').get(email); - const isNewUser = !user; - - if (!user) { - if (!canRegister()) { - return res.status(403).json({ error: 'Public registration is disabled. Contact your administrator.' }); - } - const id = uuidv4(); - const userCount = db.prepare('SELECT COUNT(*) as count FROM users').get().count; - const role = userCount === 0 ? 'platform_admin' : 'user'; - const isFirst = userCount === 0; - const plan = (isFirst && config.selfHosted) ? 'enterprise' : 'pro'; - const trialStarted = isFirst && config.selfHosted ? null : Math.floor(Date.now() / 1000); - - db.prepare(` - INSERT INTO users (id, email, name, auth_provider, provider_id, role, plan_id, trial_started, trial_plan, email_verified) - VALUES (?, ?, ?, 'microsoft', ?, ?, ?, ?, ?, 1) - `).run(id, email, name, microsoftId, role, plan, trialStarted, trialStarted ? 'pro' : null); - - user = db.prepare('SELECT * FROM users WHERE id = ?').get(id); - } else if (user.auth_provider !== 'microsoft') { - // Existing account with different provider — do NOT silently overwrite auth_provider. - if (user.password_hash) { - return res.status(409).json({ error: 'An account with this email already exists. Please log in with your password.' }); - } - db.prepare('UPDATE users SET auth_provider = ?, provider_id = ? WHERE id = ?') - .run('microsoft', microsoftId, user.id); - user = db.prepare('SELECT * FROM users WHERE id = ?').get(user.id); - } - - const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); - const token = generateToken(user, workspaceId); - const { password_hash, ...safeUser } = user; - res.json({ token, user: safeUser, current_workspace_id: workspaceId }); - - // Welcome + admin-notify only when this Microsoft login created a new account. - if (isNewUser) sendSignupEmails(user, req); - } catch (err) { - console.error('Microsoft auth error:', err); - res.status(401).json({ error: 'Microsoft authentication failed' }); - } -}); - -function getMicrosoftProfile(accessToken) { - return new Promise((resolve, reject) => { - const options = { - hostname: 'graph.microsoft.com', - path: '/v1.0/me', - headers: { Authorization: `Bearer ${accessToken}` } - }; - https.get(options, (resp) => { - let data = ''; - resp.on('data', chunk => data += chunk); - resp.on('end', () => { - try { resolve(JSON.parse(data)); } catch (e) { reject(e); } - }); - }).on('error', reject); - }); -} // ==================== User Management ==================== @@ -876,12 +966,19 @@ router.put('/users/:id/password', requireAuth, requireAdmin, (req, res) => { // Get auth config (public - tells frontend which providers are available) router.get('/config', (req, res) => { const userCount = db.prepare('SELECT COUNT(*) as count FROM users').get().count; + /* + * `providers` is the whole SSO surface now: slug + display name, nothing else. The browser no + * longer needs a client id, because it never talks to a provider itself — it follows a link to + * /api/auth/oidc//start and the server builds the authorization request. That is what + * removed the need for a provider SDK on this page, and with it the CSP exception one would need. + */ + const providers = oidcProviders.publicList(); res.json({ - googleEnabled: !!config.googleClientId, - googleClientId: config.googleClientId, - microsoftEnabled: !!config.microsoftClientId, - microsoftClientId: config.microsoftClientId, - microsoftTenantId: config.microsoftTenantId, + providers, + // Kept so a cached older login page hides its buttons rather than drawing dead ones. The client + // ids are deliberately no longer echoed — nothing in the browser has any use for them. + googleEnabled: providers.some((p) => p.slug === 'google'), + microsoftEnabled: providers.some((p) => p.slug === 'microsoft'), localEnabled: true, needsSetup: userCount === 0, registration_enabled: !config.disableRegistration || userCount === 0, @@ -948,4 +1045,593 @@ router.post('/accept-invite/:inviteId', requireAuth, (req, res) => { }); }); + +// ==================== OpenID Connect (generic SSO) ==================== +/* + * ONE flow for every provider — Google, Microsoft, Okta, Keycloak, Authentik, anything that speaks + * OIDC. Authorization Code + PKCE, run server-side, which is why there is no provider SDK on the + * login page and no third-party script origin in the CSP. + * + * It replaces two endpoints that could not tell WHO a token was minted for. Detail in lib/oidc.js; + * the short version is that identity now comes from an ID token whose signature, issuer, audience + * and OUR nonce are all checked, instead of from an access token handed to a userinfo endpoint. + * + * ⚠️ TOTP: an SSO login does not prompt for it, matching the existing documented behaviour at the + * password-login branch above ("The SSO routes and the API-token path never reach here"). The + * second factor is the identity provider's job in this flow. Changing that is a product decision, + * not something this refactor should do silently. + */ + +// The transaction is held in a short-lived signed cookie rather than server memory so that a +// restart mid-login, or a second server process, does not strand the user on a dead state. +const OIDC_TX_COOKIE = 'st_oidc_tx'; +// Holds a completed session for the seconds between the provider redirect and the page claiming it. +const SSO_CLAIM_COOKIE = 'st_sso_claim'; +const OIDC_TX_TTL_S = 600; + +/* + * The only shape of a user row that may leave the server. + * + * Two call sites each stripped `password_hash, totp_secret_enc, totp_last_step` and stopped there, + * so every login response also carried `password_reset_hash` and `email_verify_hash` — live + * credentials for taking the account over, handed to the browser. They are hashes of random tokens + * and only ever went to the account's own page, so this is hygiene rather than a takeover, but it + * means a logged-in XSS reads a working reset hash. Denylisted in ONE place so the next field + * nobody thinks about has somewhere obvious to go. + */ +const PRIVATE_USER_FIELDS = [ + 'password_hash', 'totp_secret_enc', 'totp_last_step', + 'password_reset_hash', 'password_reset_expires', + 'email_verify_hash', 'email_verify_expires', +]; + +function publicUser(row) { + if (!row) return row; + const out = { ...row }; + for (const f of PRIVATE_USER_FIELDS) delete out[f]; + return out; +} + +function readCookie(req, name) { + const raw = req.headers.cookie; + if (!raw) return null; + for (const part of raw.split(';')) { + const eq = part.indexOf('='); + if (eq === -1) continue; + if (part.slice(0, eq).trim() !== name) continue; + const value = part.slice(eq + 1).trim(); + /* + * ⚠️ decodeURIComponent THROWS on a malformed escape — `Cookie: st_oidc_tx=%` is a URIError. + * Anyone can send that, and this function is called before the handler's try block, so the + * throw used to reach the async boundary and take the process down (see asyncRoute below). + * A cookie we cannot decode is a cookie we do not have. + */ + try { return decodeURIComponent(value); } catch { return null; } + } + return null; +} + +/* + * Wrap an async handler so a rejection becomes a 500 instead of killing the server. + * + * Express 4 does not await handlers, so an async one that throws produces an unhandled rejection, + * and server.js turns that into process.exit(1) — one malformed request, one dead instance, on a + * restart loop. This has now bitten three separate times on these routes (a state comparison, a + * cookie decode, a provider whose secret would not decrypt), each time because something threw + * OUTSIDE the handler's own try block. Fixing the individual throws does not fix the shape, so + * every async route here goes through this instead. + */ +function asyncRoute(handler) { + return (req, res, next) => Promise.resolve(handler(req, res, next)).catch((err) => { + console.error(`[auth] unhandled error in ${req.method} ${req.path}:`, err && err.message); + // Wrapped: if responding THROWS, the rejection has no handler and kills the process — the + // guard against process death causing process death. + try { + if (res.headersSent) return; + // These are browser redirects, not API calls; a JSON body would be shown as text. + if (req.path.startsWith('/oidc/')) return backToApp(res, { sso_error: 'server_error' }); + res.status(500).json({ error: 'Something went wrong' }); + } catch (e2) { + console.error('[auth] failed to report an error:', e2 && e2.message); + } + }); +} + +/* + * The origin the provider will redirect back to. APP_URL pins it, exactly as the signup and invite + * mails do, because the redirect_uri must match what is registered with the provider CHARACTER FOR + * CHARACTER — deriving it from the request Host would break the moment someone reaches the box by + * a second name, and would be attacker-controlled input in the bargain. + */ +function publicOrigin(req) { + const configured = (process.env.APP_URL || '').trim().replace(/\/+$/, ''); + if (configured) return configured; + return `${req.protocol}://${req.get('host')}`; +} + +const redirectUriFor = (req, slug) => `${publicOrigin(req)}/api/auth/oidc/${slug}/callback`; + +// Send the browser back to the SPA. Errors travel as a code the login page can translate; the +// token travels in the FRAGMENT, which browsers do not send to servers and proxies do not log. +function backToApp(res, params) { + const qs = new URLSearchParams(params).toString(); + res.redirect(`/app#/login?${qs}`); +} + +// Which providers this instance offers. Public: it is what draws the login buttons. +router.get('/providers', (req, res) => { + res.json({ providers: oidcProviders.publicList() }); +}); + +/* + * Does this email address belong to an organization with its own identity provider? + * + * ⚠️ Answers with a BOOLEAN and nothing else. It deliberately does not return the provider's slug + * or its display name, because both identify a CUSTOMER: a lookup that answered + * "yes — Acme Corp SSO" would turn a guessed domain into confirmation that Acme buys this product, + * and the slug would hand out a working entry point to their tenant's login. + * + * "example.com uses SSO" is the smallest answer that still lets the page draw the right button, and + * it is something anyone could infer by watching an employee log in. The domain-to-provider mapping + * stays server-side: POST /sso/start does the lookup again and redirects, so the browser never + * learns which provider it is being sent to until the provider itself says so. + * + * It also never reveals whether the ACCOUNT exists — only the domain is matched — so this cannot be + * walked to enumerate users. + */ +router.get('/sso/discover', (req, res) => { + const provider = oidcProviders.forEmail(req.query.email); + /* + * `required` says the organization has turned off password sign-in for this domain, so the login + * page can hide the password box instead of letting someone type a password that is going to be + * refused. It is only ever present when `sso` is already true, so it tells an outsider nothing + * they could not learn by asking the same question one field earlier. + * + * ⚠️ Presentation only. The refusal is enforced in POST /login — a hidden field is a courtesy, + * not a control, and anyone can post the form directly. + */ + res.json({ + sso: !!provider, + required: provider ? !!oidcProviders.ssoOnlyForEmail(req.query.email) : false, + }); +}); + +/* + * Begin an organization SSO login for an email address. + * + * POST, so the address travels in a body rather than in a URL that lands in browser history, proxy + * logs and any Referer sent by the provider's page. The lookup happens here rather than in the + * browser for the reason above: the slug is never published. + */ +router.post('/sso/start', express.urlencoded({ extended: false }), (req, res) => { + const provider = oidcProviders.forEmail((req.body && req.body.email) || req.query.email); + /* + * ⚠️ 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) => { + const provider = oidcProviders.get(req.params.slug); + if (!provider) return backToApp(res, { sso_error: 'unknown_provider' }); + + try { + const doc = await oidc.discover(provider.issuer); + const pkce = oidc.createPkce(); + const nonce = oidc.randomToken(); + const state = oidc.randomToken(); + + const tx = jwt.sign( + { typ: 'oidc-tx', slug: provider.slug, nonce, verifier: pkce.verifier, state }, + 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. + { expiresIn: OIDC_TX_TTL_S, algorithm: 'HS256' }, + ); + res.cookie(OIDC_TX_COOKIE, tx, { + httpOnly: true, + sameSite: 'lax', // the provider returns via a top-level GET, which Lax allows + secure: req.protocol === 'https', + maxAge: OIDC_TX_TTL_S * 1000, + path: '/api/auth', + }); + + const url = new URL(doc.authorization_endpoint); + url.searchParams.set('response_type', 'code'); + url.searchParams.set('client_id', provider.clientId); + url.searchParams.set('redirect_uri', redirectUriFor(req, provider.slug)); + url.searchParams.set('scope', provider.scopes); + url.searchParams.set('state', state); + url.searchParams.set('nonce', nonce); + url.searchParams.set('code_challenge', pkce.challenge); + 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' }); + } +})); + +router.get('/oidc/:slug/callback', asyncRoute(async (req, res) => { + const provider = oidcProviders.get(req.params.slug); + if (!provider) return backToApp(res, { sso_error: 'unknown_provider' }); + + // The provider itself can refuse (consent declined, admin policy). That is not an error here. + if (req.query.error) { + console.warn(`[oidc] ${provider.slug} returned ${req.query.error}`); + return backToApp(res, { sso_error: 'provider_refused' }); + } + + const raw = readCookie(req, OIDC_TX_COOKIE); + res.clearCookie(OIDC_TX_COOKIE, { path: '/api/auth' }); + if (!raw) return backToApp(res, { sso_error: 'expired' }); + + let tx; + try { + tx = jwt.verify(raw, config.jwtSecret, { algorithms: ['HS256'] }); + if (tx.typ !== 'oidc-tx') throw new Error('not a login transaction'); + } catch { + return backToApp(res, { sso_error: 'expired' }); + } + + // CSRF: the state we minted, in the cookie only we could set, must match the one coming back. + // Compared in constant time so a wrong state cannot be discovered a character at a time. + const got = String(req.query.state || ''); + const want = String(tx.state || ''); + /* + * Compared as BYTES, not characters. + * + * `got.length` is UTF-16 code units; Buffer.from() produces UTF-8 bytes. A state of 43 characters + * containing one multi-byte character is 43 chars but 44 bytes, so the guard passed and + * timingSafeEqual threw ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH — inside an async handler, which + * Express 4 does not catch, which server.js turns into process.exit. One crafted request per + * restart was enough to take an instance down. + */ + const gotBuf = Buffer.from(got, 'utf8'); + const wantBuf = Buffer.from(want, 'utf8'); + if (gotBuf.length !== wantBuf.length || !crypto.timingSafeEqual(gotBuf, wantBuf)) { + return backToApp(res, { sso_error: 'bad_state' }); + } + if (tx.slug !== provider.slug) return backToApp(res, { sso_error: 'bad_state' }); + if (!req.query.code) return backToApp(res, { sso_error: 'no_code' }); + + let claims; + try { + const tokens = await oidc.exchangeCode({ + issuer: provider.issuer, + clientId: provider.clientId, + clientSecret: provider.clientSecret, + code: String(req.query.code), + redirectUri: redirectUriFor(req, provider.slug), + verifier: tx.verifier, + }); + claims = await oidc.verifyIdToken(tokens.id_token, { + issuer: provider.issuer, + clientId: provider.clientId, + nonce: tx.nonce, + }); + } catch (err) { + console.error(`[oidc] ${provider.slug} verification failed:`, err.message); + return backToApp(res, { sso_error: 'verification_failed' }); + } + + const email = String(claims.email || '').toLowerCase().trim(); + if (!email) return backToApp(res, { sso_error: 'no_email' }); + + /* + * ⚠️ AN ORGANIZATION'S PROVIDER MAY ONLY SPEAK FOR ITS OWN DOMAINS. + * + * Without this, per-org SSO is an account-takeover primitive, demonstrated end to end twice in + * review: any org owner can point us at an identity provider they fully control, and such a + * provider can assert ANY email with email_verified:true — including a platform_admin's. Every + * check passes honestly, because the attacker IS the issuer. + * + * Instance-wide providers are exempt: the OPERATOR chose them, which is the trust they have + * always had. An org provider is chosen by a customer, so it is confined to the domains that + * customer registered — and a domain cannot be registered while another organization holds it. + * + * The domains are the VERIFIED ones — proved by a DNS record published in the domain itself — so + * this is confinement to what the tenant demonstrably controls, not to what they typed. + */ + /* + * SSO-ONLY applies to EVERY route in, not just the password box. + * + * Confinement stops an org provider speaking for domains it does not own. This is the mirror + * image: when an organization requires its identity provider, no OTHER provider may speak for + * its people either — including the instance's own Google or Microsoft, which are not + * domain-confined and would otherwise be an open side door around the MFA and deprovisioning the + * customer turned this on for. Blocking passwords while leaving "Continue with Google" is not + * requiring single sign-on; it is renaming the bypass. + */ + const enforcedOrg = oidcProviders.ssoOnlyForEmail(email); + if (enforcedOrg && enforcedOrg.slug !== provider.slug) { + console.warn(`[oidc] ${provider.slug} asserted ${email}, but that organization requires ${enforcedOrg.slug}`); + return backToApp(res, { sso_error: 'sso_required' }); + } + + if (!emailAllowedForProvider(provider, email)) { + console.warn(`[oidc] ${provider.slug} asserted ${email}, outside its verified domains [${provider.emailDomains}]`); + return backToApp(res, { sso_error: 'domain_not_allowed' }); + } + /* + * An unverified email is refused. The whole account model keys on email — linking, invites, + * password reset — so accepting an address the provider itself will not vouch for would let + * anyone who can type an address into a sloppy IdP arrive as its owner. Providers that omit the + * claim entirely are treated as "not asserted", which is the same answer. + */ + // `=== false` accepted an OMITTED claim, which is the opposite of what the comment above says and + // what Azure AD v2 actually sends (it omits it). Absent means not asserted, which is not verified. + if (claims.email_verified !== true) return backToApp(res, { sso_error: 'email_unverified' }); + + try { + const result = upsertFederatedUser({ claims, email, provider, req }); + if (result.error) return backToApp(res, { sso_error: result.error }); + const { user, isNew } = result; + + /* + * A provider that belongs to an ORGANIZATION vouches for its own people, so anyone who signs in + * through it becomes a member of that organization — otherwise a customer would configure SSO, + * their staff would authenticate successfully, and each would land in a fresh empty org of their + * own, which is the opposite of what they asked for. + * + * Membership is added, never changed: an existing member keeps whatever role they already have, + * so an org_owner cannot be demoted by logging in, and a plain member cannot be promoted by one. + * Instance-wide providers do none of this — they say nothing about which tenant anyone is in. + */ + if (provider.organizationId) { + const already = db.prepare( + 'SELECT 1 FROM organization_members WHERE organization_id = ? AND user_id = ?' + ).get(provider.organizationId, user.id); + 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)); + } + } + + logSuccessfulLogin(user.id, user.email, getClientIp(req)); + const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); + const token = generateToken(user, workspaceId); + if (isNew) sendSignupEmails(user, req); + + /* + * ⚠️ The session token is NOT put in the redirect URL. + * + * An earlier version returned it in the fragment. That is a login-CSRF hole: anyone could send + * a victim `/app#/login?sso_token=` and the page would install it, silently + * signing that person into the ATTACKER'S account — after which their uploads, playlists and + * settings all land somewhere the attacker can read. + * + * Instead the token goes into a one-shot httpOnly cookie that only this origin can set, and the + * page exchanges it at /sso/claim. A link cannot forge that cookie, so a token can only be + * claimed by the browser that actually completed the login. + */ + /* + * The cookie carries a CLAIM token, not the session token itself. Two reasons, both learned: + * every token here is signed with the same secret, so a token minted for another purpose (a + * pre-TOTP `mfa_pending` one, say) was accepted by /sso/claim and returned the full user row; + * and the session token lives for days, so a copy of it sitting in a Set-Cookie header is worth + * stealing long after the login. This wrapper is good for 120 seconds and for nothing else. + */ + const claimToken = jwt.sign( + { typ: 'sso-claim', tok: token, wsp: workspaceId || null }, + config.jwtSecret, + { algorithm: 'HS256', expiresIn: 120 }, + ); + res.cookie(SSO_CLAIM_COOKIE, claimToken, { + httpOnly: true, + sameSite: 'lax', + secure: req.protocol === 'https', + maxAge: 120 * 1000, + path: '/api/auth', + }); + backToApp(res, { sso: '1' }); + } catch (err) { + console.error(`[oidc] ${provider.slug} sign-in failed:`, err.message); + backToApp(res, { sso_error: 'server_error' }); + } +})); + +/* + * Exchange the one-shot cookie for the session token. + * + * POST so it cannot be triggered by a link or an , and the cookie is cleared on the way out. + * + * ⚠️ Clearing a cookie asks the BROWSER to forget it; it does not invalidate anything. What bounds + * a leaked copy is the claim token's own 120-second expiry, which is why the session token is + * wrapped rather than handed over directly. Do not restore the comment that used to claim this was + * "already spent" — it was not, and a review demonstrated the same cookie claiming twice. + */ +router.post('/sso/claim', (req, res) => { + const token = readCookie(req, SSO_CLAIM_COOKIE); + res.clearCookie(SSO_CLAIM_COOKIE, { path: '/api/auth' }); + if (!token) return res.status(401).json({ error: 'No sign-in to complete' }); + + let claims; + try { + // Pinned algorithm and an explicit `typ`: two token kinds signed with one secret must never be + // interchangeable, and this endpoint accepted anything the secret had touched. + claims = jwt.verify(token, config.jwtSecret, { algorithms: ['HS256'] }); + } catch { + return res.status(401).json({ error: 'That sign-in has expired' }); + } + if (claims.typ !== 'sso-claim' || !claims.tok) { + return res.status(401).json({ error: 'That sign-in has expired' }); + } + + let session; + try { + session = jwt.verify(claims.tok, config.jwtSecret, { algorithms: ['HS256'] }); + } catch { + return res.status(401).json({ error: 'That sign-in has expired' }); + } + /* + * The wrapped token must be an ordinary SESSION token. Forging the wrapper needs the signing + * secret, so this is not exploitable — but a `mfa_pending` token nested inside a valid wrapper + * was accepted, which is the same interchangeability the outer typ check was added to close. + * A session token carries no `aud` and no `mfa_pending`; anything else is a different kind. + */ + if (session.mfa_pending || session.aud || !session.id) { + return res.status(401).json({ error: 'That sign-in has expired' }); + } + const user = db.prepare('SELECT * FROM users WHERE id = ?').get(session.id); + if (!user) return res.status(401).json({ error: 'That sign-in has expired' }); + + const safeUser = publicUser(user); + res.json({ token: claims.tok, user: safeUser, current_workspace_id: claims.wsp || null }); +}); + +/* + * Find or create the account behind a verified set of claims. + * + * The linking rule is the one the Google path already used, kept deliberately: an existing account + * WITH a password is never taken over by an SSO login — the owner proves control by logging in + * locally and linking from Settings. An account with no password (already federated) is re-pointed + * at whichever provider just authenticated it. + */ +function upsertFederatedUser({ claims, email, provider, req }) { + const existing = db.prepare('SELECT * FROM users WHERE email = ?').get(email); + + if (!existing) { + if (!canRegister()) return { error: 'registration_disabled' }; + const id = uuidv4(); + const userCount = db.prepare('SELECT COUNT(*) as count FROM users').get().count; + const isFirst = userCount === 0; + const role = isFirst ? 'platform_admin' : 'user'; + const plan = (isFirst && config.selfHosted) ? 'enterprise' : 'pro'; + const trialStarted = isFirst && config.selfHosted ? null : Math.floor(Date.now() / 1000); + db.prepare(` + INSERT INTO users (id, email, name, auth_provider, provider_id, avatar_url, role, plan_id, trial_started, trial_plan, email_verified) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1) + `).run(id, email, claims.name || '', provider.slug, String(claims.sub), claims.picture || '', + role, plan, trialStarted, trialStarted ? 'pro' : null); + return { user: db.prepare('SELECT * FROM users WHERE id = ?').get(id), isNew: true }; + } + + if (existing.auth_provider !== provider.slug) { + /* + * 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 + * last" — fine when the operator chose them all, an account takeover once a customer can add + * one. An ORG provider therefore never adopts an account another provider established; the user + * links it deliberately instead. + */ + if (provider.organizationId) { + /* + * `existing.auth_provider && … !== 'local'` failed OPEN on an empty string, and compared + * SLUGS, which got the two interesting cases backwards: + * + * - a customer replacing their identity provider (or an admin who deleted one and made + * another) got a new random slug, so their own org could no longer sign its own people in + * — every SSO account in the tenant bricked, with no recovery route; + * - meanwhile an account owned by a DELETED provider looked adoptable to everyone. + * + * Ownership is therefore asked of the ORGANIZATION behind the slug, and the only states an + * org provider may take over are its own org's, and `local` with no password — an invited + * user who has not set one yet, which is a real and wanted case. + * + * An account established by a provider that no longer exists is deliberately NOT adoptable: + * see the squatting note in the callback. It is recovered by proving control of the email + * through password reset, not by another identity provider asserting it. + */ + const owner = oidcProviders.ownerOf(existing.auth_provider); + const sameOrg = !!(owner && owner.organizationId && owner.organizationId === provider.organizationId); + const neverFederated = existing.auth_provider === 'local'; + if (!sameOrg && !neverFederated) return { error: 'account_exists_other_provider' }; + } + db.prepare('UPDATE users SET auth_provider = ?, provider_id = ?, avatar_url = ? WHERE id = ?') + .run(provider.slug, String(claims.sub), claims.picture || existing.avatar_url, existing.id); + return { user: db.prepare('SELECT * FROM users WHERE id = ?').get(existing.id), isNew: false }; + } + + /* + * Same provider, but a DIFFERENT subject. `sub` is the provider's stable id and the email is not: + * addresses get reassigned, especially inside companies. Refusing here is what stops a recycled + * address inheriting the previous holder's account. + */ + if (existing.provider_id && String(existing.provider_id) !== String(claims.sub)) { + return { error: 'subject_mismatch' }; + } + if (!existing.provider_id) { + db.prepare('UPDATE users SET provider_id = ? WHERE id = ?').run(String(claims.sub), existing.id); + } + return { user: db.prepare('SELECT * FROM users WHERE id = ?').get(existing.id), isNew: false }; +} + + module.exports = router; +// Exported for tests: these two carry the security decisions of the SSO flow, and testing them +// through a live identity provider only is how they shipped unverified the first time. +module.exports.emailAllowedForProvider = emailAllowedForProvider; +module.exports.upsertFederatedUser = upsertFederatedUser; +module.exports.isOrphanedFederated = isOrphanedFederated; diff --git a/server/routes/org-sso.js b/server/routes/org-sso.js new file mode 100644 index 0000000..d9a3802 --- /dev/null +++ b/server/routes/org-sso.js @@ -0,0 +1,939 @@ +'use strict'; + +/* + * Per-organization SSO — the customer-facing half of single sign-on. + * + * Instance-wide providers live in the environment and belong to whoever runs the server. These + * belong to a CUSTOMER: an organization points ScreenTinker at its own identity provider, and its + * people sign in with it without the operator editing a config file. + * + * The login flow is unchanged. A provider configured here is resolved by exactly the same + * oidc-providers.get(slug) the environment ones go through, so there is one authorization request + * builder, one token exchange and one verifier — not a second, less-tested path for tenants. + */ + +const express = require('express'); +const crypto = require('crypto'); +const router = express.Router(); +const { db } = require('../db/database'); +const { requireAuth } = require('../middleware/auth'); +const { resolveTenancy } = require('../lib/tenancy'); +const secretbox = require('../lib/secretbox'); +const oidc = require('../lib/oidc'); +const { logActivity, getClientIp } = require('../services/activity'); +const { isPublicEmailDomain } = require('../lib/public-email-domains'); +const domainVerify = require('../lib/domain-verify'); +const emailSvc = require('../services/email'); + +/* + * Only an org owner/admin may configure how their people sign in — it is the most security-relevant + * setting a tenant has. Platform staff are deliberately NOT given a bypass here: this is customer + * configuration, and an operator who needs to change it can do so as a member of that organization. + */ +function requireOrgAdmin(req, res, next) { + const orgId = req.params.orgId; + if (!orgId) return res.status(400).json({ error: 'organization required' }); + const row = db.prepare( + 'SELECT role FROM organization_members WHERE organization_id = ? AND user_id = ?' + ).get(orgId, req.user.id); + if (!row || (row.role !== 'org_owner' && row.role !== 'org_admin')) { + // 404 rather than 403: an outsider should not learn that an organization id exists. + return res.status(404).json({ error: 'Not found' }); + } + req.orgId = orgId; + next(); +} + +/* + * Configuring SSO requires a VERIFIED email address, on top of being an org admin. + * + * Everything else here rests on the identity of the person doing it: they claim domains, they point + * the organization at an identity provider, and they are who the operator's claim notification + * names. An unverified address is an assertion nobody has checked, so without this the entire + * feature — including domain claims — is reachable by anyone who can type an address into the + * signup form and never open the mail. + * + * Reads are deliberately NOT gated: seeing your own organization's configuration changes nothing, + * and locking an admin out of the screen that explains why sign-in is broken helps no one. + */ +function requireVerifiedAdmin(req, res, next) { + const row = db.prepare('SELECT email_verified FROM users WHERE id = ?').get(req.user.id); + if (!row || !row.email_verified) { + return res.status(403).json({ + error: 'Verify your email address before configuring single sign-on.', + code: 'email_unverified', + }); + } + next(); +} + +/* + * The slug is a URL path segment and is generated, never chosen. + * + * Two customers both wanting "okta" must not collide, and one must not be able to guess or squat + * another's. It is random and globally unique; the admin only ever sees the display name. + */ +const newSlug = () => `org${crypto.randomBytes(6).toString('hex')}`; + +/** Never let a secret out of the API, in either direction of a round trip. */ +function toPublic(row) { + return { + id: row.id, + slug: row.slug, + name: row.name, + issuer: row.issuer, + client_id: row.client_id, + has_client_secret: !!row.client_secret_enc, + scopes: row.scopes, + email_domains: row.email_domains, + enabled: !!row.enabled, + login_url: `/api/auth/oidc/${row.slug}/start`, + callback_url: `/api/auth/oidc/${row.slug}/callback`, + // Domains and their proof state. A provider whose domains are all unverified can be saved and + // looks configured, but routes nobody — the UI needs this to say so rather than imply success. + domains: domainsFor(row.id), + }; +} + +/* + * Domains are the routing key, so they are normalised hard: lowercased, de-duplicated, stripped of + * a leading @ or scheme someone pasted, and validated as something that can actually be the right + * hand side of an address. A wildcard is refused — "*" would route every unrecognised address at + * one customer's IdP. + */ +const MAX_DOMAINS = 50; + +function normaliseDomains(raw) { + const seen = new Set(); + for (const part of String(raw || '').split(/[,\s]+/)) { + /* + * Capped. Uncapped, one verified org admin could POST 20,000 domains inside the 12 MB body + * limit: 20,000 rows inserted under a single write lock (stalling every other query on the + * instance) and 20,000 notification emails per platform admin. No real organization signs in + * from fifty domains, and an org that does can create a second provider. + */ + if (seen.size >= MAX_DOMAINS) { + const e = new Error(`at most ${MAX_DOMAINS} sign-in domains per provider`); + e.status = 400; + throw e; + } + let d = part.trim().toLowerCase().replace(/^@/, '').replace(/^https?:\/\//, '').replace(/\/.*$/, ''); + if (!d) continue; + if (!/^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$/.test(d)) { + throw new Error(`"${part.trim()}" is not a valid email domain`); + } + /* + * A consumer mailbox provider is never an organization's sign-in domain, and claiming one is an + * attack rather than a mistake: every Gmail or Outlook user typing their address into this + * product's login page would be offered a "sign in with your organization" button pointing at + * one tenant's infrastructure. It also lets one cheap account deny a public domain to everyone. + */ + if (isPublicEmailDomain(d)) { + const e = new Error(`${d} is a public email provider and cannot be used as a sign-in domain. ` + + 'Use a domain your organization owns.'); + e.status = 400; + throw e; + } + seen.add(d); + } + return [...seen].join(','); +} + +/** + * A domain may belong to ONE organization. + * + * Without this, a second tenant could claim a domain already routed elsewhere and quietly capture + * that company's logins — the worst failure this feature could have. First claim wins; the loser is + * told which domain clashed and nothing about who holds it. + */ +function assertDomainsFree(domains, orgId, excludeProviderId) { + if (!domains) return; + for (const d of domains.split(',')) { + const row = db.prepare('SELECT * FROM org_sso_domains WHERE domain = ?').get(d); + if (!row) continue; + if (row.provider_id && row.provider_id === excludeProviderId) continue; + /* + * A lapsed unverified claim reserves nothing. Clearing it here rather than on a timer means the + * domain frees itself the moment someone else asks for it, and there is no sweeper to forget to + * run — a squatter's unprovable claim simply stops being an obstacle. + */ + if (domainVerify.isClaimExpired(row)) { + db.prepare('DELETE FROM org_sso_domains WHERE id = ?').run(row.id); + continue; + } + // Same-org duplicates were allowed and should not have been: two providers claiming one + // domain makes routing depend on table-scan order, so half a company's staff get sent to an + // identity provider that has never heard of them. + const e = new Error(row.organization_id === orgId + ? `the domain ${d} is already used by another of your providers` + : `the domain ${d} is already used for sign-in by another organization`); + e.status = 409; + throw e; + } +} + +/* + * Bring the claimed-domain rows in line with what the admin typed. + * + * A newly claimed domain arrives UNVERIFIED and stays inert until DNS proves the claim — it routes + * nobody and the login callback refuses assertions for it. Re-typing an existing domain must not + * reset that proof, which is why this diffs rather than deleting and re-inserting: a save on the + * name field would otherwise silently un-verify every domain the customer had already proved, and + * log their whole company out. + * + * Runs inside the caller's transaction so a domain cannot be reserved by two organizations at once. + */ +function syncDomains(providerId, orgId, domains) { + const wanted = domains ? domains.split(',').filter(Boolean) : []; + const existing = db.prepare('SELECT * FROM org_sso_domains WHERE provider_id = ?').all(providerId); + const stale = new Map(existing.map((r) => [r.domain, r])); + const claimed = []; // newly claimed, for the operator notification — sent AFTER the transaction + + for (const d of wanted) { + const mine = stale.get(d); + if (mine && !domainVerify.isClaimExpired(mine)) { + stale.delete(d); // already ours and still live — keep any proof that happened + continue; + } + if (mine) { + /* + * A LAPSED claim is not renewed in place. Renewing silently is what made the 8-hour limit + * meaningless: a review held a domain indefinitely at one request per window, and because a + * renewal was not a new claim, the operator was told exactly once, on day zero. + * + * So the row is dropped and re-created: the token changes (a record left over from the + * abandoned attempt cannot satisfy the new one), and it counts as a fresh claim, which means + * it is notified again. Squatting is not made impossible — it is made loud. + */ + stale.delete(d); + db.prepare('DELETE FROM org_sso_domains WHERE id = ?').run(mine.id); + } + assertDomainsFree(d, orgId, providerId); + db.prepare(`INSERT INTO org_sso_domains (id, organization_id, provider_id, domain, token) + VALUES (?, ?, ?, ?, ?)`) + .run(crypto.randomUUID(), orgId, providerId, d, domainVerify.newToken()); + claimed.push(d); + } + for (const row of stale.values()) { + db.prepare('DELETE FROM org_sso_domains WHERE id = ?').run(row.id); + } + return claimed; +} + +/* + * Tell the operator that a tenant has claimed a domain. + * + * DNS verification makes a claim worthless without control of the domain, so this is not what stops + * abuse — it is what makes abuse VISIBLE. A tenant claiming `microsoft.com` will never verify it, + * but an operator still wants to know somebody tried, and the notification is the difference + * between finding that out now and finding it out from the company involved. + * + * Deliberately NOT sent to postmaster@ the claimed domain. That would mean this product emails + * third parties who never signed up for it, on input any tenant can supply — a spam cannon with a + * ScreenTinker return address. The operator can contact a domain owner; the server should not do it + * unprompted. + * + * Failure to send is logged and swallowed: a mail outage must not stop a customer configuring SSO. + */ +function notifyOperatorOfClaim(req, { domains, orgId, providerName }) { + try { + if (!domains || !domains.length) return; + // Always log, even with no mail transport — otherwise an instance without email has no record + // of a claim at all, and those are exactly the instances least likely to notice. + console.log(`[org-sso] domain(s) claimed by org ${orgId} (${providerName}): ${domains.join(', ')}`); + if (!emailSvc.isConfigured()) return; + // COALESCE, because email_alerts is nullable in practice on older rows and `= 1` silently + // excludes NULL — the activation-nudge query already defends this way. + const admins = db.prepare("SELECT email FROM users WHERE role = 'platform_admin' AND COALESCE(email_alerts, 1) = 1").all(); + if (!admins.length) return; + const org = db.prepare('SELECT name FROM organizations WHERE id = ?').get(orgId); + const who = req.user && req.user.email ? req.user.email : 'an administrator'; + // ONE message per save listing every domain, not one per domain per admin — fifty admins + // claiming ten domains was five hundred messages from a single request. + const body = [ + `${who} claimed ${domains.length} sign-in domain(s):`, + '', + ...domains.map((d) => ` ${d}`), + '', + `Organization: ${org ? org.name : orgId} (${orgId})`, + `Provider: ${providerName}`, + '', + 'A claimed domain routes nobody until a DNS TXT record proves it, and the claim lapses after', + '8 hours if it is not proved. No action is needed unless this looks wrong.', + ].join('\n'); + const subject = domains.length === 1 + ? `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)); + } + } catch (e) { + console.error('[org-sso] claim notification failed:', e && e.message); + } +} + +/* + * Tell the operator that a customer wants password login re-opened. + * + * The mail deliberately carries NO action link. A token that acts on its own turns every forwarded, + * archived or auto-previewed copy of this message into a way to switch off a customer's single + * sign-on; the decision belongs to a signed-in platform admin, so the mail only says where to make + * it. Logged unconditionally, because an instance with no mail transport still needs a record that + * somebody asked. + */ +function notifyOperatorOfRemovalRequest(req, { id, orgId, orgName, reason }) { + try { + const who = req.user && req.user.email ? req.user.email : 'an administrator'; + console.warn(`[org-sso] SSO-ONLY REMOVAL REQUESTED for org ${orgName || orgId} (${orgId}) by ${who} — request ${id}`); + if (!emailSvc.isConfigured()) return; + const admins = db.prepare("SELECT email FROM users WHERE role = 'platform_admin' AND COALESCE(email_alerts, 1) = 1").all(); + if (!admins.length) return; + const body = [ + `${who} has asked to stop requiring single sign-on for ${orgName || orgId}.`, + '', + 'Approving this RE-OPENS password sign-in for everyone at that organization\u2019s verified', + 'domains. Until it is approved, nothing changes.', + '', + reason ? `Reason given: ${reason}` : 'No reason was given.', + '', + `Organization: ${orgName || ''} (${orgId})`, + `Request: ${id}`, + '', + 'Review it in ScreenTinker under Admin. There is no link in this email on purpose — the', + 'decision has to be made while signed in as a platform admin, so a forwarded copy of this', + 'message cannot turn off a customer\u2019s single sign-on.', + ].join('\n'); + for (const a of admins) { + Promise.resolve(emailSvc.sendEmail({ + to: a.email, + subject: `Approval needed: stop requiring SSO for ${orgName || orgId}`, + text: body, + })).catch((e) => console.error('[org-sso] removal notification failed:', e && e.message)); + } + } catch (e) { + console.error('[org-sso] removal notification failed:', e && e.message); + } +} + +/* + * An SSO-only organization may not dismantle its own enforcement sideways. + * + * `sso_only` is honoured only while a provider is enabled AND a domain is verified, so disabling + * the provider, clearing its domains, or deleting it all switch enforcement off — with `sso_only` + * still reading `true`, no request filed and the operator never told. A review used each of the + * three, and the delete variant additionally rewrites every federated account to `local`, after + * which a password reset takes over accounts the identity provider was supposed to own. + * + * That made the approval workflow decorative: anyone who could file a request could instead just + * turn the provider off. So the same interlock guards every route that would leave the tenant with + * nothing enforcing, and points at the request as the way through. + */ +/** The domains that currently ENFORCE for an organization: verified, on an enabled provider. */ +function enforcingDomains(orgId, excludeProviderId = null) { + return db.prepare(` + SELECT d.domain FROM org_sso_domains d + JOIN org_sso_providers p ON p.id = d.provider_id + WHERE d.organization_id = ? AND d.verified_at IS NOT NULL AND p.enabled = 1 + AND (? IS NULL OR p.id != ?) + `).all(orgId, excludeProviderId, excludeProviderId).map((r) => r.domain); +} + +/* + * An SSO-only organization may not shrink the set of domains that enforce. + * + * The first version of this asked "would ANY provider still enforce?", which was the wrong + * question twice over, and a review defeated it both ways: + * + * SWAP it only fired when the resulting domain list was EMPTY, so replacing + * `acme.test` with `decoy.test` removed every proof and sailed through — two PUTs + * and the customer's domain no longer required anything. + * SIBLING it counted PROVIDERS, so with two configured you could disable the one that owns + * your staff's domain while the other, covering a domain nobody signs in at, kept the + * answer "yes, something still enforces". + * + * The question that matters is per-DOMAIN: after this change, is every domain that enforces today + * still enforcing? Losing one is exactly what needs the operator, whichever route gets you there. + */ +function assertEnforcementNotReduced(orgId, providerId, nextDomainsFor, what) { + const org = db.prepare('SELECT sso_only FROM organizations WHERE id = ?').get(orgId); + if (!org || !org.sso_only) return; + + const before = new Set(enforcingDomains(orgId)); + const after = new Set(enforcingDomains(orgId, providerId)); + // Whatever this provider will still contribute afterwards, as VERIFIED domains only — a domain + // being re-added is unverified, so it does not count as still enforcing. + for (const d of nextDomainsFor) after.add(d); + + const lost = [...before].filter((d) => !after.has(d)); + if (!lost.length) return; + + const e = new Error(`Your organization requires single sign-on, so ${what} would stop ` + + `${lost.join(', ')} from being covered and leave those people unable to sign in. ` + + 'Ask the people who run this server to approve stopping the requirement first.'); + e.status = 409; + e.code = 'sso_only_locked'; + throw e; +} + +/** A provider's domains, with the DNS record each unverified one still needs. *//** A provider's domains, with the DNS record each unverified one still needs. */ +function domainsFor(providerId) { + return db.prepare('SELECT * FROM org_sso_domains WHERE provider_id = ? ORDER BY domain').all(providerId) + .map((r) => ({ + domain: r.domain, + verified: !!r.verified_at, + verified_at: r.verified_at, + last_checked_at: r.last_checked_at, + last_error: r.verified_at ? null : r.last_error, + // The token is not a secret — it only means anything published in that domain's own DNS. + ...domainVerify.instructions(r.domain, r.token), + })); +} + +/* + * What an admin is told when discovery fails. + * + * The temptation is to hand back the underlying message, because it is genuinely the most useful + * thing for a real misconfiguration. But the issuer is caller-supplied and fetched server-side, so + * that message is an SSRF read primitive: `https://internal-host:8080 responded 403` and + * `discovery issuer mismatch: … document says ` both report on services the caller cannot reach + * directly. The jwks branch of the /test endpoint was already genericised for exactly this reason; + * these paths were not, which left the scanner intact one line above the comment saying not to. + * + * So: the shape of the failure, never the upstream's answer. The full message goes to the log. + */ +function discoveryErrorMessage(e, issuer) { + const raw = String((e && e.message) || ''); + console.warn(`[org-sso] discovery failed for ${issuer}: ${raw}`); + if (/must use https|not publicly routable|not a URL/i.test(raw)) return raw; // our own guard, no upstream data + if (/issuer mismatch/i.test(raw)) return 'that URL is not the OpenID issuer it claims to be'; + if (/is missing /i.test(raw)) return 'that issuer published an incomplete OpenID configuration'; + if (/redirected/i.test(raw)) return 'that issuer redirected; the URL must be the final one'; + if (/abort|timeout/i.test(raw)) return 'that issuer did not respond in time'; + return 'no OpenID configuration could be read from that URL'; +} + +/* + * Wrap an async handler so a rejection is a 500 rather than a dead server. Express 4 does not await + * handlers and server.js turns an unhandled rejection into process.exit — see the longer note on + * asyncRoute in routes/auth.js, which is the same guard for the same reason. + */ +function asyncRoute(handler) { + return (req, res, next) => Promise.resolve(handler(req, res, next)).catch((err) => { + console.error(`[org-sso] unhandled error in ${req.method} ${req.path}:`, err && err.message); + // Wrapped for the same reason as in routes/auth.js: a throw while REPORTING an error would + // become an unhandled rejection and take the process down. + try { + if (!res.headersSent) res.status(500).json({ error: 'Something went wrong' }); + } catch (e2) { + console.error('[org-sso] failed to report an error:', e2 && e2.message); + } + }); +} + +router.use(requireAuth, resolveTenancy); + +// List an organization's providers. +router.get('/:orgId/sso', requireOrgAdmin, (req, res) => { + const rows = db.prepare('SELECT * FROM org_sso_providers WHERE organization_id = ? ORDER BY created_at').all(req.orgId); + res.json({ providers: rows.map(toPublic) }); +}); + +router.post('/:orgId/sso', requireOrgAdmin, requireVerifiedAdmin, asyncRoute(async (req, res) => { + const { name, issuer, client_id: clientId, client_secret: clientSecret, scopes, email_domains: domains } = req.body || {}; + if (!name || !issuer || !clientId) { + return res.status(400).json({ error: 'name, issuer and client_id are required' }); + } + + let cleanDomains; + try { + cleanDomains = normaliseDomains(domains); + assertDomainsFree(cleanDomains, req.orgId, null); + } catch (e) { + return res.status(e.status || 400).json({ error: e.message }); + } + + /* + * The issuer is checked against the live provider BEFORE anything is stored. A typo here would + * otherwise be discovered by a user staring at a failed login, and the error they would see says + * nothing useful. Discovery also proves the URL is an OIDC issuer at all rather than a company + * home page someone pasted. + */ + try { + await oidc.discover(String(issuer).trim().replace(/\/+$/, '')); + } catch (e) { + return res.status(400).json({ error: `Could not read OpenID configuration from that issuer: ${discoveryErrorMessage(e, issuer)}` }); + } + + const id = crypto.randomUUID(); + const slug = newSlug(); + let newlyClaimed = []; + /* + * Re-check the domains INSIDE the transaction. The first check happened before `await + * oidc.discover()`, which yields the event loop for a network round trip the caller's own IdP + * controls the length of — two admins racing that window both passed and both got the domain, + * after which routing became whichever row the scan reached first. + */ + try { + db.transaction(() => { + assertDomainsFree(cleanDomains, req.orgId, null); + db.prepare(` + INSERT INTO org_sso_providers (id, organization_id, slug, name, issuer, client_id, client_secret_enc, scopes, email_domains, enabled) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 1) + `).run(id, req.orgId, slug, String(name).trim(), String(issuer).trim().replace(/\/+$/, ''), String(clientId).trim(), + clientSecret ? secretbox.encrypt(String(clientSecret)) : null, + String(scopes || 'openid email profile').trim(), cleanDomains); + newlyClaimed = syncDomains(id, req.orgId, cleanDomains); + })(); + } catch (e) { + if (e.status) return res.status(e.status).json({ error: e.message }); + console.error('[org-sso] create failed:', e.message); + return res.status(500).json({ error: 'Could not save that provider' }); + } + + // Notified after the transaction commits, so an operator is never told about a claim that rolled + // back — and never inside it, where a slow mail path would hold a write lock. + notifyOperatorOfClaim(req, { domains: newlyClaimed, orgId: req.orgId, providerName: String(name).trim() }); + + // (userId, action, details, deviceId, ipAddress, workspaceId) — the org id is NOT the 4th arg. + logActivity(req.user.id, 'org_sso_created', `${name} (${slug}) org=${req.orgId}`, null, getClientIp(req)); + res.status(201).json(toPublic(db.prepare('SELECT * FROM org_sso_providers WHERE id = ?').get(id))); +})); + +router.put('/:orgId/sso/:id', requireOrgAdmin, requireVerifiedAdmin, asyncRoute(async (req, res) => { + const existing = db.prepare('SELECT * FROM org_sso_providers WHERE id = ? AND organization_id = ?').get(req.params.id, req.orgId); + if (!existing) return res.status(404).json({ error: 'Not found' }); + + const { name, issuer, client_id: clientId, client_secret: clientSecret, scopes, email_domains: domains, enabled } = req.body || {}; + + let cleanDomains = existing.email_domains; + // `!== undefined` let a null through, and null took the destructive branch: normaliseDomains(null) + // is '', which deleted every claimed domain and every DNS proof with it. A client that sends the + // field as null on an unrelated save must not log a customer's whole company out. + const domainsSupplied = domains !== undefined && domains !== null; + if (domainsSupplied) { + try { + cleanDomains = normaliseDomains(domains); + assertDomainsFree(cleanDomains, req.orgId, existing.id); + } catch (e) { + return res.status(e.status || 400).json({ error: e.message }); + } + } + + const nextIssuer = issuer !== undefined ? String(issuer).trim().replace(/\/+$/, '') : existing.issuer; + if (nextIssuer !== existing.issuer) { + try { await oidc.discover(nextIssuer); } + catch (e) { return res.status(400).json({ error: `Could not read OpenID configuration from that issuer: ${discoveryErrorMessage(e, nextIssuer)}` }); } + } + + /* + * An absent client_secret LEAVES THE STORED ONE ALONE; an empty string clears it. The API never + * returns the secret, so a UI that round-trips a form would otherwise blank it on every save — + * the classic way a settings page silently breaks the thing it is editing. + */ + // Disabling this provider, or removing the domains it enforces through, is the same act as + // turning the requirement off — and that needs the operator. + try { + const willBeEnabled = enabled === undefined ? !!existing.enabled : !!enabled; + /* + * What this provider still covers afterwards: nothing if it is being disabled, otherwise the + * domains it keeps that are ALREADY verified. A domain typed back in arrives unverified and + * enforces nobody, which is precisely how the swap bypass worked. + */ + let keeps = []; + if (willBeEnabled) { + const kept = domainsSupplied ? new Set(cleanDomains.split(',').filter(Boolean)) : null; + keeps = db.prepare("SELECT domain FROM org_sso_domains WHERE provider_id = ? AND verified_at IS NOT NULL") + .all(existing.id).map((r) => r.domain) + .filter((d) => (kept ? kept.has(d) : true)); + } + const what = !willBeEnabled ? 'disabling this provider' : 'changing its sign-in domains'; + assertEnforcementNotReduced(req.orgId, existing.id, keeps, what); + } catch (e) { + return res.status(e.status || 400).json({ error: e.message, code: e.code }); + } + + let newlyClaimed = []; + const secretEnc = clientSecret === undefined ? existing.client_secret_enc + : (clientSecret === '' ? null : secretbox.encrypt(String(clientSecret))); + + /* + * Same transaction, same re-check, and for the same reason as the create path above — this one was + * missed when that was fixed, which left the race fully open on the route an attacker would + * actually pick: `await oidc.discover()` on a CHANGED issuer is a round trip whose length the + * caller's own IdP decides, so it can be held open for the full fetch timeout while a victim + * organization claims the domain legitimately. The UNIQUE constraint on `domain` is the hard + * backstop now; this keeps the failure a clean 409 rather than a constraint error. + */ + try { + db.transaction(() => { + if (domainsSupplied) assertDomainsFree(cleanDomains, req.orgId, existing.id); + db.prepare(` + UPDATE org_sso_providers + SET name = ?, issuer = ?, client_id = ?, client_secret_enc = ?, scopes = ?, email_domains = ?, enabled = ?, + updated_at = strftime('%s','now') + WHERE id = ? + `).run( + name !== undefined ? String(name).trim() : existing.name, + nextIssuer, + clientId !== undefined ? String(clientId).trim() : existing.client_id, + secretEnc, + scopes !== undefined ? String(scopes).trim() : existing.scopes, + cleanDomains, + enabled === undefined ? existing.enabled : (enabled ? 1 : 0), + existing.id, + ); + if (domainsSupplied) newlyClaimed = syncDomains(existing.id, req.orgId, cleanDomains); + })(); + } catch (e) { + // A thrown assertDomainsFree carries its own status; anything else is ours and stays generic + // rather than returning a raw SQLite message to the caller. + if (e.status) return res.status(e.status).json({ error: e.message }); + console.error('[org-sso] update failed:', e.message); + return res.status(500).json({ error: 'Could not save that provider' }); + } + + notifyOperatorOfClaim(req, { domains: newlyClaimed, orgId: req.orgId, providerName: existing.name }); + + logActivity(req.user.id, 'org_sso_updated', `${existing.name} (${existing.slug}) org=${req.orgId}`, null, getClientIp(req)); + res.json(toPublic(db.prepare('SELECT * FROM org_sso_providers WHERE id = ?').get(existing.id))); +})); + +/* + * Check a provider without making anyone log in. + * + * The overwhelmingly common failure is a configuration one — an issuer that is a company home page + * rather than an OIDC issuer, a provider that is unreachable from the server, a JWKS with no signing + * keys — and every one of those currently surfaces as a user staring at a failed login with an + * error that says nothing useful. This turns that into an answer at configuration time. + * + * ⚠️ It is deliberately honest about its limits. Discovery and JWKS prove the provider EXISTS and + * that we could verify a token it signed. They cannot prove the client id is right, that the secret + * matches, or that the redirect URI is registered — only a real authorization round trip does that, + * and the response says so rather than implying a green tick means "SSO works". + */ +router.post('/:orgId/sso/:id/test', requireOrgAdmin, requireVerifiedAdmin, asyncRoute(async (req, res) => { + const row = db.prepare('SELECT * FROM org_sso_providers WHERE id = ? AND organization_id = ?').get(req.params.id, req.orgId); + if (!row) return res.status(404).json({ error: 'Not found' }); + + const checks = []; + let doc = null; + try { + doc = await oidc.discover(row.issuer); + checks.push({ name: 'discovery', ok: true, detail: doc.issuer }); + } catch (e) { + checks.push({ name: 'discovery', ok: false, detail: discoveryErrorMessage(e, row.issuer) }); + return res.json({ ok: false, checks }); + } + + checks.push({ + name: 'endpoints', + ok: !!(doc.authorization_endpoint && doc.token_endpoint), + detail: doc.authorization_endpoint || 'missing authorization_endpoint', + }); + + try { + const jwks = await oidc.fetchJwks(doc.jwks_uri); + const signing = (jwks.keys || []).filter((k) => !k.use || k.use === 'sig'); + checks.push({ + name: 'signing_keys', + ok: signing.length > 0, + detail: signing.length ? `${signing.length} key(s)` : 'the provider published no signing keys', + }); + } catch (e) { + // Deliberately generic. `jwks_uri` comes from the CALLER'S OWN discovery document, so echoing + // the upstream status here turned this endpoint into a readable internal port scanner. + checks.push({ name: 'signing_keys', ok: false, detail: 'could not read the provider keys' }); + } + + // What the admin must have registered at the provider — the single most common thing to get + // wrong, and something we can state exactly rather than ask them to guess. + const origin = (process.env.APP_URL || '').trim().replace(/\/+$/, '') || `${req.protocol}://${req.get('host')}`; + res.json({ + ok: checks.every((c) => c.ok), + checks, + redirect_uri: `${origin}/api/auth/oidc/${row.slug}/callback`, + // Said plainly so a passing test is not mistaken for a working login. + note: 'unverifiable_by_test', + }); +})); + +/* + * Check DNS for the proof, and record the answer. + * + * Verification is the whole point of the domain table: until this succeeds the domain routes nobody + * and the login callback refuses to accept an assertion for it, so a claim on a domain the tenant + * does not control buys them nothing at all. + * + * Deliberately pull-based rather than a background sweep. The admin has just edited DNS and wants to + * know now, and a per-request check means there is no scheduler to fall over quietly and no window + * where a verified domain sits unnoticed. + */ +router.post('/:orgId/sso/:id/domains/:domain/verify', requireOrgAdmin, requireVerifiedAdmin, asyncRoute(async (req, res) => { + const provider = db.prepare('SELECT * FROM org_sso_providers WHERE id = ? AND organization_id = ?') + .get(req.params.id, req.orgId); + if (!provider) return res.status(404).json({ error: 'Not found' }); + + const row = db.prepare('SELECT * FROM org_sso_domains WHERE provider_id = ? AND domain = ?') + .get(provider.id, String(req.params.domain).toLowerCase()); + if (!row) return res.status(404).json({ error: 'Not found' }); + + if (row.verified_at) return res.json({ ok: true, domain: row.domain, verified: true, already: true }); + + /* + * A lapsed claim is RELEASED, not reissued. + * + * Reissuing here renewed the clock, so pressing Verify once per window held a domain forever — + * the exact squatting the time limit exists to stop, performed through the endpoint meant to + * enforce it. Releasing it frees the domain for anyone else, and re-adding it is an ordinary new + * claim: new token, and the operator is notified again. + */ + if (domainVerify.isClaimExpired(row)) { + db.prepare('DELETE FROM org_sso_domains WHERE id = ?').run(row.id); + logActivity(req.user.id, 'org_sso_domain_lapsed', `${row.domain} org=${req.orgId}`, null, getClientIp(req)); + return res.status(409).json({ + ok: false, + domain: row.domain, + verified: false, + expired: true, + error: 'That claim expired and has been released. Add the domain again to get a new record.', + }); + } + + const result = await domainVerify.check(row.domain, row.token); + + if (result.ok) { + db.prepare("UPDATE org_sso_domains SET verified_at = strftime('%s','now'), last_checked_at = strftime('%s','now'), last_error = NULL WHERE id = ?") + .run(row.id); + logActivity(req.user.id, 'org_sso_domain_verified', `${row.domain} via ${result.via} org=${req.orgId}`, null, getClientIp(req)); + console.log(`[org-sso] ${row.domain} verified via ${result.via} for org ${req.orgId}`); + return res.json({ ok: true, domain: row.domain, verified: true, via: result.via }); + } + + db.prepare("UPDATE org_sso_domains SET last_checked_at = strftime('%s','now'), last_error = ? WHERE id = ?") + .run(result.error, row.id); + res.status(400).json({ + ok: false, + domain: row.domain, + verified: false, + error: result.error, + ...domainVerify.instructions(row.domain, row.token), + }); +})); + +/* ──────────────────────────────────────────────────────────────────────────────────────────── + * SSO-only: requiring the organization's identity provider. + */ + +/** Only a VERIFIED domain can compel anyone — see the note on ssoOnlyForEmail. */ +function verifiedDomainCount(orgId) { + return db.prepare(` + SELECT COUNT(*) AS n FROM org_sso_domains d + JOIN org_sso_providers p ON p.id = d.provider_id + WHERE d.organization_id = ? AND d.verified_at IS NOT NULL AND p.enabled = 1 + `).get(orgId).n; +} + +router.get('/:orgId/sso-only', requireOrgAdmin, (req, res) => { + const org = db.prepare('SELECT sso_only FROM organizations WHERE id = ?').get(req.orgId); + const pending = db.prepare( + "SELECT id, requested_by, reason, created_at FROM org_sso_only_requests WHERE organization_id = ? AND status = 'pending' ORDER BY created_at DESC" + ).get(req.orgId); + res.json({ + sso_only: !!(org && org.sso_only), + verified_domains: verifiedDomainCount(req.orgId), + pending_removal_request: pending || null, + }); +}); + +/* + * Turn it ON. An org admin does this alone: it can only ever reduce the ways into their own tenant, + * and the people affected are their own. + */ +router.post('/:orgId/sso-only', requireOrgAdmin, requireVerifiedAdmin, (req, res) => { + /* + * Refuse when nothing is proved. Otherwise an organization could switch off password login for + * accounts it cannot offer any other way in for — locking its own people out of a product they + * can then only reach by asking the operator to undo it. + */ + if (!verifiedDomainCount(req.orgId)) { + return res.status(400).json({ + error: 'Verify at least one sign-in domain before requiring single sign-on — otherwise nobody could sign in.', + code: 'no_verified_domain', + }); + } + + /* + * ⚠️ Do not let the person pressing this button lock themselves out. + * + * The commonest onboarding shape is: sign up with a personal or consultancy address, create the + * organization, verify the company domain, turn this on. Enforcement then covers them (they are + * a member) while their own address is outside the verified domains — so passwords are refused + * AND their org's provider will not assert for them either, because assertions are confined to + * verified domains. There is no self-service way back: no route removes a membership, and + * password reset succeeds but login still refuses. A review walked into it on the happy path. + * + * They are told exactly which address is the problem, and what to do about it. + */ + const domains = enforcingDomains(req.orgId); + const at = String(req.user.email || '').lastIndexOf('@'); + const ownDomain = at === -1 ? '' : String(req.user.email).slice(at + 1).toLowerCase().replace(/\.+$/, ''); + if (!domains.includes(ownDomain)) { + return res.status(400).json({ + error: `Your own address (${req.user.email}) is not at a verified domain (${domains.join(', ')}), ` + + 'so requiring single sign-on would lock you out with no way back. Verify that domain first, ' + + 'or hand ownership to someone whose address is covered.', + code: 'would_lock_out_actor', + }); + } + + /* + * Everyone ELSE in the same position is reported rather than refused — they may be exactly the + * contractors this is meant to shut out. But the admin must find out here, not from a support + * ticket after the fact. + */ + const stranded = db.prepare(` + SELECT DISTINCT u.email FROM users u + WHERE u.id IN ( + SELECT m.user_id FROM organization_members m WHERE m.organization_id = ? + UNION + SELECT wm.user_id FROM workspace_members wm JOIN workspaces w ON w.id = wm.workspace_id + WHERE w.organization_id = ? + ) + `).all(req.orgId, req.orgId) + .map((r) => r.email) + .filter((e) => { + const i = String(e).lastIndexOf('@'); + return i === -1 || !domains.includes(String(e).slice(i + 1).toLowerCase().replace(/\.+$/, '')); + }); + db.prepare('UPDATE organizations SET sso_only = 1 WHERE id = ?').run(req.orgId); + logActivity(req.user.id, 'org_sso_only_enabled', + `org=${req.orgId} stranded=${stranded.length}`, null, getClientIp(req)); + console.log(`[org-sso] SSO-only ENABLED for org ${req.orgId} by ${req.user.email}` + + (stranded.length ? ` — ${stranded.length} member(s) outside the verified domains: ${stranded.join(', ')}` : '')); + res.json({ sso_only: true, stranded_members: stranded }); +}); + +/* + * Turning it OFF is a REQUEST, not a switch. + * + * This is the direction that re-opens password login, so it is the direction an attacker who has + * taken an org admin would take, and it is also what a customer will demand at their worst moment — + * identity provider down, nobody can work — which is precisely when a self-service toggle gets + * flipped without thinking. A platform admin has to approve it. + */ +router.post('/:orgId/sso-only/removal-request', requireOrgAdmin, requireVerifiedAdmin, (req, res) => { + const org = db.prepare('SELECT sso_only, name FROM organizations WHERE id = ?').get(req.orgId); + if (!org || !org.sso_only) return res.status(400).json({ error: 'Single sign-on is not required for this organization' }); + + const existing = db.prepare("SELECT id FROM org_sso_only_requests WHERE organization_id = ? AND status = 'pending'").get(req.orgId); + if (existing) return res.status(409).json({ error: 'A removal request is already awaiting approval', request_id: existing.id }); + + const id = crypto.randomUUID(); + const reason = String((req.body && req.body.reason) || '').slice(0, 500); + db.prepare('INSERT INTO org_sso_only_requests (id, organization_id, requested_by, reason) VALUES (?, ?, ?, ?)') + .run(id, req.orgId, req.user.id, reason); + + notifyOperatorOfRemovalRequest(req, { id, orgId: req.orgId, orgName: org.name, reason }); + logActivity(req.user.id, 'org_sso_only_removal_requested', `org=${req.orgId} id=${id}`, null, getClientIp(req)); + res.status(202).json({ status: 'pending', request_id: id }); +}); + +/** Withdrawing your own request needs nobody's approval — it only ever keeps SSO required. */ +router.delete('/:orgId/sso-only/removal-request/:id', requireOrgAdmin, requireVerifiedAdmin, (req, res) => { + const row = db.prepare("SELECT * FROM org_sso_only_requests WHERE id = ? AND organization_id = ? AND status = 'pending'") + .get(req.params.id, req.orgId); + if (!row) return res.status(404).json({ error: 'Not found' }); + db.prepare("UPDATE org_sso_only_requests SET status = 'cancelled', decided_at = strftime('%s','now'), decided_by = ? WHERE id = ?") + .run(req.user.id, row.id); + res.json({ status: 'cancelled' }); +}); + +/* + * The operator's side. + * + * Approval is an authenticated platform_admin action, NOT a link in an email: a token that acts on + * its own turns every forwarded or archived message into a way to re-open password login for a + * customer. The mail says what happened and where to go; the decision is made signed in. + */ +function requirePlatformAdmin(req, res, next) { + if (!req.user || req.user.role !== 'platform_admin') return res.status(404).json({ error: 'Not found' }); + next(); +} + +router.get('/sso-only/removal-requests', requirePlatformAdmin, (req, res) => { + const rows = db.prepare(` + SELECT r.id, r.organization_id, r.reason, r.created_at, o.name AS organization_name, u.email AS requested_by_email + FROM org_sso_only_requests r + LEFT JOIN organizations o ON o.id = r.organization_id + LEFT JOIN users u ON u.id = r.requested_by + WHERE r.status = 'pending' + ORDER BY r.created_at + `).all(); + res.json({ requests: rows }); +}); + +router.post('/sso-only/removal-requests/:id/:decision', requirePlatformAdmin, (req, res) => { + const decision = req.params.decision === 'approve' ? 'approved' + : req.params.decision === 'reject' ? 'rejected' : null; + if (!decision) return res.status(400).json({ error: 'decision must be approve or reject' }); + + const row = db.prepare("SELECT * FROM org_sso_only_requests WHERE id = ? AND status = 'pending'").get(req.params.id); + if (!row) return res.status(404).json({ error: 'Not found' }); + + const note = String((req.body && req.body.note) || '').slice(0, 500); + db.transaction(() => { + db.prepare("UPDATE org_sso_only_requests SET status = ?, decided_by = ?, decided_at = strftime('%s','now'), decision_note = ? WHERE id = ?") + .run(decision, req.user.id, note, row.id); + // Only an approval changes anything. A rejection leaves SSO required, which is the safe state. + if (decision === 'approved') db.prepare('UPDATE organizations SET sso_only = 0 WHERE id = ?').run(row.organization_id); + })(); + + logActivity(req.user.id, `org_sso_only_${decision}`, `org=${row.organization_id} id=${row.id}`, null, getClientIp(req)); + console.log(`[org-sso] SSO-only removal ${decision} for org ${row.organization_id} by ${req.user.email}`); + res.json({ status: decision, organization_id: row.organization_id }); +}); + +router.delete('/:orgId/sso/:id', requireOrgAdmin, requireVerifiedAdmin, (req, res) => { + const existing = db.prepare('SELECT * FROM org_sso_providers WHERE id = ? AND organization_id = ?').get(req.params.id, req.orgId); + if (!existing) return res.status(404).json({ error: 'Not found' }); + /* + * Take the domain rows and the accounts with it, in one transaction. + * + * Deleting only the provider left both behind, and both were unrecoverable in the product: + * + * - `domain` is globally UNIQUE and a VERIFIED row never expires, so an orphaned row blocked + * its own domain forever — for this organization and for every other one — while being + * invisible in the API and routing nobody. Re-claiming your own domain returned 409. The only + * way out was SQL. + * - the users this provider established kept pointing at a slug nothing answers to. They could + * not sign in (no provider), could not use a password (auth_provider is not 'local') and + * could not register (address taken). + * + * Both are handled here, at the moment the intent is known, rather than inferred later from the + * absence of configuration — which is what made an unset GOOGLE_CLIENT_ID look like a deletion. + */ + try { + assertEnforcementNotReduced(req.orgId, existing.id, [], 'deleting this provider'); + } catch (e) { + return res.status(e.status || 400).json({ error: e.message, code: e.code }); + } + + const freed = db.transaction(() => { + const domains = db.prepare('DELETE FROM org_sso_domains WHERE provider_id = ?').run(existing.id).changes; + // Back to a local account, so the owner can recover it by proving the mailbox — strictly + // stronger evidence than the identity-provider assertion that created it. + const users = db.prepare("UPDATE users SET auth_provider = 'local', provider_id = NULL WHERE auth_provider = ?") + .run(existing.slug).changes; + db.prepare('DELETE FROM org_sso_providers WHERE id = ?').run(existing.id); + return { domains, users }; + })(); + + logActivity(req.user.id, 'org_sso_deleted', + `${existing.name} (${existing.slug}) org=${req.orgId} domains=${freed.domains} users_reset=${freed.users}`, + null, getClientIp(req)); + console.log(`[org-sso] deleted ${existing.slug}: released ${freed.domains} domain(s), returned ${freed.users} account(s) to local`); + res.json({ success: true, domains_released: freed.domains, accounts_returned_to_local: freed.users }); +}); + +module.exports = router; 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 808d3a2..981fd25 100644 --- a/server/server.js +++ b/server/server.js @@ -1,3 +1,14 @@ +/* + * FIRST, before any dependency is required: make sure they are installed and loadable. + * + * A rollback restores an older package.json but not its packages, and a Node upgrade leaves the + * native database module compiled against the wrong ABI. Both present as a server that will not + * start, with an error naming a file rather than the action needed — and the rollback case happens + * precisely when something else has already gone wrong. Repairing takes seconds; diagnosing at 2am + * does not. ST_SKIP_DEP_PREFLIGHT=1 turns it off. + */ +require('./lib/preflight-deps').preflight(); + const express = require('express'); const http = require('http'); const https = require('https'); @@ -173,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); @@ -522,6 +556,52 @@ app.use('/socket.io-client', express.static( // safe because the callback runs at request time, which is a subtle thing to depend on. const limiterTelemetry = require('./lib/limiter-telemetry'); const rateLimits = new Map(); +/* + * The bucket key is the SHAPE of the endpoint, never the spelling the caller chose. + * + * Two failures drove this. Express routes non-strictly, so `/api/auth/login/` was a different key + * and bought a fresh ten password attempts. And `/api/organizations//...` carries three + * caller-chosen segments, so every request minted its own bucket — 120 calls with unique ids gave + * zero 429s against the limit that exists to bound outbound OIDC discovery and live DNS lookups. + * + * ⚠️ Fold by EXPLICIT shape, not with a clever catch-all. A single regex that collapsed "anything + * else" put every unknown path in one bucket WITH the real endpoints, so flooding nonsense URLs + * exhausted the limit for `/sso-only` — trading a bypass for a denial of service. Known shapes get + * their own keys; everything else shares one, separate from all of them. + */ +const LIMIT_PATH_SHAPES = [ + [/^\/api\/auth\/oidc\/[^/]+\/(start|callback)$/, (m) => `/api/auth/oidc/:slug/${m[1]}`], + [/^\/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'], + [/^\/api\/organizations\/[^/]+\/sso$/, () => '/api/organizations/:id/sso'], +]; + +function canonicalLimitPath(rawPath) { + const p = rawPath + .replace(/\/{2,}/g, '/') // collapse doubled separators + .replace(/\/+$/, '') // a trailing slash is the same endpoint + .toLowerCase() + || '/'; + for (const [re, to] of LIMIT_PATH_SHAPES) { + const m = p.match(re); + if (m) return to(m); + } + // Unrecognised, but still under a mount whose ids are caller-chosen: one shared bucket, kept + // apart from every real endpoint so flooding it cannot starve them. + if (p.startsWith('/api/organizations/')) return '/api/organizations/:unmatched'; + return p; +} + function rateLimit(windowMs, maxRequests) { return (req, res, next) => { // #100: key on the FULL path, not req.path. These limiters are mounted via @@ -529,7 +609,18 @@ function rateLimit(windowMs, maxRequests) { // req.path was '/' for ALL of them - i.e. /login, /register, /totp/verify shared // ONE per-IP counter (coupled limits; the /totp/verify brute-force limit wasn't // actually independent). originalUrl keeps each endpoint's limit separate. - const key = getClientIp(req) + (req.originalUrl || req.url || req.path).split('?')[0]; + /* + * ⚠️ NORMALISE THE PATH, or the key is caller-controlled and the limit is decorative. + * + * Express routes non-strictly, so `/api/auth/login/` reaches the same handler as + * `/api/auth/login` — with a different originalUrl, hence a different bucket, hence a fresh ten + * attempts. A review walked straight past the login limiter that way. Any path segment the + * caller chooses does the same thing, and `/api/auth/oidc/:slug/...` has one by design, so the + * slug is folded out too: one bucket per IP per ENDPOINT, not per spelling of it. + */ + const rawPath = (req.originalUrl || req.url || req.path).split('?')[0]; + const normalisedPath = canonicalLimitPath(rawPath); + const key = getClientIp(req) + normalisedPath; const now = Date.now(); const windowStart = now - windowMs; let hits = rateLimits.get(key) || []; @@ -573,6 +664,15 @@ app.use('/api/auth/register', rateLimit(60000, 5)); // 5 registrations per minut app.use('/api/auth/totp/verify', rateLimit(60000, 10)); // Email-verification resend: cap so it can't be used to spray mail at an address. app.use('/api/auth/resend-verification', rateLimit(60000, 5)); +// Domain lookup is unauthenticated by necessity (it runs before login). Rate limited so it +// cannot be walked to enumerate which customers use SSO. +app.use('/api/auth/sso/discover', rateLimit(60000, 10)); +// 10/min was wrong for this one: org SSO is used by companies behind a SINGLE corporate egress IP, +// and this is their only entry point, so the 11th employee of the morning met a raw JSON 429 with no +// login page. It is a redirect, not a credential check. +app.use('/api/auth/sso/start', rateLimit(60000, 120)); +// The OIDC endpoints had no limit at all, which left the callback's parsing as a free amplifier. +app.use('/api/auth/oidc', rateLimit(60000, 120)); // Self-service password reset. The request endpoint is the spray surface (it sends mail to // an address the caller supplies), so it gets the tighter cap; the redeem endpoint is a // 32-byte-token guess, capped mostly to keep the bcrypt work bounded. @@ -583,6 +683,14 @@ app.use('/api/auth/reset-password', rateLimit(60000, 10)); // path prefix first, so this fires before /api/auth catches the request. app.use('/api/auth/users', rateLimit(60000, 20)); app.use('/api/auth', require('./routes/auth')); +// Per-organization SSO configuration. Mounted under /api/organizations so the org id is the +// route's own subject, which is what the org_owner/org_admin check keys on. +/* + * Rate-limited because these routes do outbound work on caller-supplied input: OIDC discovery on a + * customer-chosen issuer, and a live DNS lookup per domain verification. Everything under + * /api/auth/* already had a limit; this router was mounted without one. + */ +app.use('/api/organizations', rateLimit(60000, 60), require('./routes/org-sso')); // Rate limit pairing to prevent brute force (5 attempts per minute per IP). // #88: bind this to the whole /api/provision surface, not just /pair - the bare // POST /api/provision (routes/provisioning.js) is a second pairing endpoint that diff --git a/server/test/admin-users.test.js b/server/test/admin-users.test.js index 4a690bf..0abf80d 100644 --- a/server/test/admin-users.test.js +++ b/server/test/admin-users.test.js @@ -72,7 +72,11 @@ db.exec(` ); CREATE TABLE organizations ( id TEXT PRIMARY KEY, name TEXT NOT NULL, - owner_user_id TEXT, plan_id TEXT, subscription_status TEXT + owner_user_id TEXT, plan_id TEXT, subscription_status TEXT, + -- Mirrors the real schema. Login refuses when it cannot determine whether an organization + -- requires single sign-on, so a fixture missing this column fails closed — correctly, but it + -- is the fixture that is wrong, not the guard. + sso_only INTEGER NOT NULL DEFAULT 0 ); CREATE TABLE activity_log ( id INTEGER PRIMARY KEY AUTOINCREMENT, diff --git a/server/test/oidc-sso.test.js b/server/test/oidc-sso.test.js new file mode 100644 index 0000000..d5fd37a --- /dev/null +++ b/server/test/oidc-sso.test.js @@ -0,0 +1,774 @@ +'use strict'; + +/* + * The SSO that shipped before this verified nothing that mattered, and had no tests at all. + * + * Google's path asked `tokeninfo?access_token=` whether a token was valid and trusted the email in + * the answer; Microsoft's handed a bearer token to Graph /me and trusted that. Neither asked WHO + * THE TOKEN WAS ISSUED FOR. An access token is a bearer credential for a resource, minted for some + * application — so any site a user signed into that requested `email` or `User.Read` could replay + * their token and be handed a session as them. + * + * These tests exist so that cannot come back. Every one of them describes an attack that the old + * code would have waved through, and they run against a REAL RSA keypair and a REAL JWKS document + * so the verifier is exercised the way a provider would exercise it — not against a stub that + * agrees with us. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const crypto = require('node:crypto'); +const fs = require('node:fs'); +const jwt = require('jsonwebtoken'); + +const oidc = require('../lib/oidc'); +const providers = require('../lib/oidc-providers'); + +// --------------------------------------------------------------------------------------------- +// A pretend identity provider: one keypair, one JWKS, one discovery document. + +const ISSUER = 'https://idp.example.com'; +const CLIENT_ID = 'screentinker-test-client'; +const KID = 'test-key-1'; + +const { publicKey, privateKey } = crypto.generateKeyPairSync('rsa', { modulusLength: 2048 }); +const JWKS = { keys: [{ ...publicKey.export({ format: 'jwk' }), kid: KID, use: 'sig', alg: 'RS256' }] }; + +// A second keypair nobody should trust — the "signed by someone else" case. +const rogue = crypto.generateKeyPairSync('rsa', { modulusLength: 2048 }); + +function discoveryDoc(issuer = ISSUER) { + return { + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + jwks_uri: `${issuer}/jwks`, + }; +} + +/** Point global fetch at the pretend provider. Returns a restore function. */ +function mockProvider({ doc = discoveryDoc(), jwks = JWKS } = {}) { + const real = global.fetch; + global.fetch = async (url) => { + const u = String(url); + if (u.endsWith('/.well-known/openid-configuration')) { + return { ok: true, status: 200, json: async () => doc }; + } + if (u.endsWith('/jwks')) { + return { ok: true, status: 200, json: async () => jwks }; + } + return { ok: false, status: 404, json: async () => ({}) }; + }; + oidc._resetCaches(); + return () => { global.fetch = real; oidc._resetCaches(); }; +} + +const idToken = (claims = {}, { key = privateKey, alg = 'RS256', kid = KID } = {}) => jwt.sign( + { iss: ISSUER, aud: CLIENT_ID, sub: 'user-123', email: 'a@example.com', nonce: 'NONCE', ...claims }, + key, { algorithm: alg, keyid: kid, expiresIn: '5m' }, +); + +const verify = (token, over = {}) => + oidc.verifyIdToken(token, { issuer: ISSUER, clientId: CLIENT_ID, nonce: 'NONCE', ...over }); + +// --------------------------------------------------------------------------------------------- + +test('a well-formed token from the right provider verifies', async () => { + const restore = mockProvider(); + try { + const claims = await verify(idToken()); + assert.equal(claims.sub, 'user-123'); + assert.equal(claims.email, 'a@example.com'); + } finally { restore(); } +}); + +test('THE OLD BUG: a token minted for a DIFFERENT application is refused', async () => { + // This is the whole reason the previous implementation was unsafe. Same provider, same user, + // real signature — but issued to somebody else's client. It must not buy a session here. + const restore = mockProvider(); + try { + await assert.rejects(() => verify(idToken({ aud: 'someone-elses-client' })), /audience/i); + } finally { restore(); } +}); + +test('...and neither is one that merely LISTS us alongside its real audience', async () => { + // aud can be an array. azp names who it was actually issued to, and if that is not us then we + // are a bystander in someone else's token — the confused-deputy case. + const restore = mockProvider(); + try { + await assert.rejects( + () => verify(idToken({ aud: [CLIENT_ID, 'other'], azp: 'other' })), + /issued to a different application/i, + ); + } finally { restore(); } +}); + +test('a token captured from an earlier login cannot be replayed', async () => { + // The nonce is minted per login and kept in a signed cookie. Without this check a correctly + // audienced token, obtained any way at all, would be reusable forever. + const restore = mockProvider(); + try { + await assert.rejects(() => verify(idToken({ nonce: 'A-DIFFERENT-LOGIN' })), /nonce/i); + } finally { restore(); } +}); + +test('alg:none is refused', async () => { + const restore = mockProvider(); + try { + // Hand-built, because jsonwebtoken will not sign 'none' for you. + const header = Buffer.from(JSON.stringify({ alg: 'none', typ: 'JWT', kid: KID })).toString('base64url'); + const body = Buffer.from(JSON.stringify({ + iss: ISSUER, aud: CLIENT_ID, sub: 'x', email: 'a@example.com', nonce: 'NONCE', + exp: Math.floor(Date.now() / 1000) + 300, + })).toString('base64url'); + await assert.rejects(() => verify(`${header}.${body}.`), /algorithm/i); + } finally { restore(); } +}); + +test('an HMAC-signed token is refused even though the "key" is public', async () => { + // HS256 verifies with a shared secret. The only key we hold for a provider is its PUBLIC one, + // which the attacker also has — so accepting HMAC would let anyone sign their own identity. + const restore = mockProvider(); + try { + const forged = jwt.sign( + { iss: ISSUER, aud: CLIENT_ID, sub: 'x', email: 'admin@example.com', nonce: 'NONCE' }, + publicKey.export({ type: 'spki', format: 'pem' }), + { algorithm: 'HS256', keyid: KID, expiresIn: '5m' }, + ); + await assert.rejects(() => verify(forged), /algorithm/i); + } finally { restore(); } +}); + +test('a token signed by the wrong key is refused', async () => { + const restore = mockProvider(); + try { + await assert.rejects(() => verify(idToken({}, { key: rogue.privateKey })), /signature/i); + } finally { restore(); } +}); + +test('an expired token is refused', async () => { + const restore = mockProvider(); + try { + const stale = jwt.sign( + { iss: ISSUER, aud: CLIENT_ID, sub: 'x', email: 'a@example.com', nonce: 'NONCE', + exp: Math.floor(Date.now() / 1000) - 3600 }, + privateKey, { algorithm: 'RS256', keyid: KID }, + ); + await assert.rejects(() => verify(stale), /expired/i); + } finally { restore(); } +}); + +test('a provider whose discovery claims a different issuer is refused', async () => { + // Discovery is fetched from a URL derived from the configured issuer, so a document naming a + // DIFFERENT one is either broken or hostile. Either way its tokens must not be accepted under a + // name it does not own. + const restore = mockProvider({ doc: discoveryDoc('https://evil.example.com') }); + try { + await assert.rejects(() => verify(idToken()), /issuer mismatch/i); + } finally { restore(); } +}); + +test('verification cannot be skipped by omitting the nonce', async () => { + // Belt and braces: the caller must always have a nonce to compare, so a coding mistake that + // forgets to pass one fails closed rather than accepting anything. + const restore = mockProvider(); + try { + await assert.rejects(() => verify(idToken(), { nonce: undefined }), /nonce/i); + } finally { restore(); } +}); + +test('an unknown kid triggers exactly one JWKS refresh, then gives up', async () => { + // Key rotation is normal and must not fail every login until a cache expires; a token quoting + // nonsense must not become a way to hammer the provider either. + let jwksFetches = 0; + const real = global.fetch; + global.fetch = async (url) => { + const u = String(url); + if (u.endsWith('/.well-known/openid-configuration')) return { ok: true, status: 200, json: async () => discoveryDoc() }; + if (u.endsWith('/jwks')) { jwksFetches++; return { ok: true, status: 200, json: async () => JWKS }; } + return { ok: false, status: 404, json: async () => ({}) }; + }; + oidc._resetCaches(); + try { + await assert.rejects(() => verify(idToken({}, { kid: 'no-such-kid' })), /no signing key/i); + assert.equal(jwksFetches, 1, 'one refresh, not a loop'); + } finally { global.fetch = real; oidc._resetCaches(); } +}); + +// --------------------------------------------------------------------------------------------- +// PKCE + +test('PKCE uses S256 and never sends the verifier', () => { + const { verifier, challenge, method } = oidc.createPkce(); + assert.equal(method, 'S256'); + assert.notEqual(verifier, challenge, 'a plain challenge would make PKCE pointless'); + const expected = crypto.createHash('sha256').update(verifier).digest('base64url'); + assert.equal(challenge, expected); + assert.ok(verifier.length >= 43, 'RFC 7636 wants at least 43 characters of entropy'); +}); + +test('every login gets fresh values', () => { + const a = oidc.createPkce(); const b = oidc.createPkce(); + assert.notEqual(a.verifier, b.verifier); + assert.notEqual(oidc.randomToken(), oidc.randomToken()); +}); + +// --------------------------------------------------------------------------------------------- +// The provider registry + +test('Google registers from the variable the README always documented', () => { + const [g] = providers.list({ GOOGLE_CLIENT_ID: 'g' }); + assert.equal(g.issuer, 'https://accounts.google.com'); +}); + +test('a single-tenant Microsoft app narrows the issuer, so another tenant fails iss', () => { + const [ms] = providers.list({ MICROSOFT_CLIENT_ID: 'm', MICROSOFT_TENANT_ID: 'abc-123' }); + assert.equal(ms.issuer, 'https://login.microsoftonline.com/abc-123/v2.0'); +}); + +test('MULTI-TENANT MICROSOFT IS REFUSED, not silently broken', () => { + /* + * Two reasons pointing the same way. It cannot work: Microsoft's `common` metadata advertises the + * literal template `https://login.microsoftonline.com/{tenantid}/v2.0`, so the issuer can never + * equal the configured URL and every login fails at /start anyway. + * + * And the obvious patch is dangerous: loosening the iss comparison accepts tokens from EVERY + * Azure tenant, which is nOAuth — any tenant admin can set an arbitrary unverified `email` on + * their own user and be issued a session as that address here. + */ + for (const tenant of ['common', 'organizations', 'consumers', '']) { + assert.deepEqual(providers.list({ MICROSOFT_CLIENT_ID: 'm', MICROSOFT_TENANT_ID: tenant }), [], + `MICROSOFT_TENANT_ID=${tenant || '(unset)'} must not register a provider`); + } +}); + +test('any OIDC provider can be added by env', () => { + const list = providers.list({ + OIDC_PROVIDERS: 'authentik', + OIDC_AUTHENTIK_ISSUER: 'https://id.example.com/application/o/st/', + OIDC_AUTHENTIK_CLIENT_ID: 'abc', + OIDC_AUTHENTIK_NAME: 'Company SSO', + }); + assert.equal(list.length, 1); + assert.equal(list[0].slug, 'authentik'); + assert.equal(list[0].name, 'Company SSO'); + assert.equal(list[0].issuer, 'https://id.example.com/application/o/st', 'trailing slash normalised'); + assert.equal(list[0].clientSecret, null, 'PKCE means a public client is fine'); +}); + +test('an incomplete or malformed provider is ignored rather than crashing boot', () => { + assert.equal(providers.list({ OIDC_PROVIDERS: 'broken' }).length, 0, 'no issuer/client id'); + assert.equal(providers.list({ + OIDC_PROVIDERS: '../etc/passwd', + OIDC_ISSUER: 'https://x', OIDC_CLIENT_ID: 'y', + }).length, 0, 'a slug that is not URL-safe never becomes a route'); +}); + +test('the browser is told slugs and names only — never a client id or secret', () => { + const pub = providers.publicList({ + GOOGLE_CLIENT_ID: 'super-secret-id', + OIDC_PROVIDERS: 'okta', OIDC_OKTA_ISSUER: 'https://x.okta.com', + OIDC_OKTA_CLIENT_ID: 'id', OIDC_OKTA_CLIENT_SECRET: 'shh', + }); + const serialised = JSON.stringify(pub); + assert.ok(!serialised.includes('super-secret-id')); + assert.ok(!serialised.includes('shh')); + assert.deepEqual(Object.keys(pub[0]).sort(), ['name', 'slug']); +}); + +// --------------------------------------------------------------------------------------------- +// Per-organization SSO. +// +// Instance providers belong to whoever runs the server; these belong to a CUSTOMER. Two properties +// matter more than the feature itself: one organization must not be able to capture another's +// logins, and the login page must not become a way to enumerate who the customers are. + +const Database = require('better-sqlite3'); + +function orgDb() { + const d = new Database(':memory:'); + d.exec(` + CREATE TABLE org_sso_providers ( + id TEXT PRIMARY KEY, organization_id TEXT NOT NULL, slug TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, issuer TEXT NOT NULL, client_id TEXT NOT NULL, client_secret_enc TEXT, + scopes TEXT NOT NULL DEFAULT 'openid email profile', email_domains TEXT NOT NULL DEFAULT '', + enabled INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL DEFAULT 0, updated_at INTEGER NOT NULL DEFAULT 0); + CREATE TABLE organizations (id TEXT PRIMARY KEY, name TEXT, sso_only INTEGER NOT NULL DEFAULT 0); + CREATE TABLE org_sso_domains ( + id TEXT PRIMARY KEY, organization_id TEXT NOT NULL, provider_id TEXT, domain TEXT NOT NULL UNIQUE, + token TEXT NOT NULL, token_issued_at INTEGER NOT NULL DEFAULT 0, verified_at INTEGER, + last_checked_at INTEGER, last_error TEXT, created_at INTEGER NOT NULL DEFAULT 0); + `); + return d; +} + +/* + * `domains` are VERIFIED (DNS proof recorded); `pending` are claimed but unproven. The distinction + * is the whole point of the domain table, so the harness makes it impossible to write a test that + * blurs the two: a test that wants routing must say which state it is testing. + */ +function withOrgDb(rows, fn) { + const d = orgDb(); + let n = 0; + for (const r of rows) { + d.prepare('INSERT OR IGNORE INTO organizations (id, name, sso_only) VALUES (?, ?, ?)') + .run(r.org, r.org, r.ssoOnly ? 1 : 0); + const typed = [...(r.domains || '').split(','), ...(r.pending || '').split(',')].filter(Boolean).join(','); + d.prepare(`INSERT INTO org_sso_providers (id, organization_id, slug, name, issuer, client_id, email_domains, enabled) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`) + .run(r.id, r.org, r.slug, r.name, r.issuer || ISSUER, r.clientId || 'cid', typed, r.enabled === undefined ? 1 : r.enabled); + const addDomain = (dom, verifiedAt) => d.prepare( + `INSERT INTO org_sso_domains (id, organization_id, provider_id, domain, token, token_issued_at, verified_at) + VALUES (?, ?, ?, ?, ?, ?, ?)` + ).run(`dom${++n}`, r.org, r.id, dom, `tok${n}`, Math.floor(Date.now() / 1000), verifiedAt); + // Verified in claim order unless the test pins it, so "who proved it first" stays testable. + for (const dom of (r.domains || '').split(',').filter(Boolean)) addDomain(dom, (r.verifiedAt || 1000) + n); + for (const dom of (r.pending || '').split(',').filter(Boolean)) addDomain(dom, null); + } + // Swap the module's lazily-resolved connection for this in-memory one. + const real = require('../db/database'); + const saved = real.db; + real.db = d; + delete require.cache[require.resolve('../lib/oidc-providers')]; + const mod = require('../lib/oidc-providers'); + try { return fn(mod); } finally { + real.db = saved; + delete require.cache[require.resolve('../lib/oidc-providers')]; + } +} + +test('an org provider is found by the email DOMAIN', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme SSO', domains: 'acme.com,acme.co.uk' }], (m) => { + assert.equal(m.forEmail('someone@acme.com').name, 'Acme SSO'); + assert.equal(m.forEmail('someone@ACME.CO.UK').name, 'Acme SSO', 'case-insensitive'); + assert.equal(m.forEmail('someone@other.com'), null); + assert.equal(m.forEmail('not-an-email'), null); + }); +}); + +test('a disabled provider stops answering for its domain', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme', domains: 'acme.com', enabled: 0 }], (m) => { + assert.equal(m.forEmail('x@acme.com'), null); + assert.equal(m.getOrgProvider('orgaaa'), null, 'and cannot be started directly either'); + }); +}); + +test('ORG PROVIDERS ARE NEVER PUBLISHED to the whole internet', () => { + // The login page lists instance-wide providers only. Listing a customer's IdP would both offer it + // to people it does not belong to and leak the customer list. + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme SSO', domains: 'acme.com' }], (m) => { + const pub = m.publicList({ GOOGLE_CLIENT_ID: 'g' }); + assert.deepEqual(pub.map((p) => p.slug), ['google']); + assert.ok(!JSON.stringify(pub).includes('Acme'), 'no customer name anywhere in the public list'); + }); +}); + +test('an org provider is still resolvable by slug, so the shared login flow can run it', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme SSO', domains: 'acme.com' }], (m) => { + const p = m.get('orgaaa', {}); + assert.equal(p.name, 'Acme SSO'); + assert.equal(p.organizationId, 'org-a', 'carries its org so the callback can grant membership'); + assert.equal(p.source, 'org'); + }); +}); + +test('an instance provider wins a slug clash with an org one', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'google', name: 'Impostor', domains: 'evil.com' }], (m) => { + // Org slugs are randomly generated so this cannot happen by accident — but if it ever did, a + // tenant must not be able to shadow the platform's own Google button. + assert.equal(m.get('google', { GOOGLE_CLIENT_ID: 'real' }).name, 'Google'); + }); +}); + +test('THE SHIPPED SCHEMA makes one domain, one organization a constraint', () => { + /* + * Uniqueness was enforced only by a check in the route, which a race defeated twice in review. + * It is now a UNIQUE constraint, so a second claim cannot exist even if the check is bypassed. + * + * ⚠️ Read from server/db/database.js, NOT from the test harness. The earlier version of this + * test asserted against the harness's own CREATE TABLE and therefore stayed green when UNIQUE was + * removed from the shipped schema — it tested a copy of the thing it was named after. + */ + const schema = fs.readFileSync(require.resolve('../db/database.js'), 'utf8'); + const table = schema.slice(schema.indexOf('CREATE TABLE IF NOT EXISTS org_sso_domains')); + const body = table.slice(0, table.indexOf('`,')); + assert.match(body, /domain\s+TEXT\s+NOT NULL\s+UNIQUE/, 'org_sso_domains.domain must be UNIQUE'); + // And the row must not outlive its provider: a verified row never expires, so an orphan would + // block its domain for every organization, forever, while being invisible in the API. + assert.match(body, /FOREIGN KEY \(provider_id\) REFERENCES org_sso_providers\(id\) ON DELETE CASCADE/, + 'a domain row must be removed with its provider'); +}); + +test('no database means no org providers, and no crash', () => { + // The env-only paths must keep working on an instance where the table has not been migrated yet. + const m = require('../lib/oidc-providers'); + assert.doesNotThrow(() => m.publicList({ GOOGLE_CLIENT_ID: 'g' })); +}); + + +// --------------------------------------------------------------------------------------------- +// Regressions for defects found in security review. Each one was demonstrated end to end against a +// running server before it was fixed; none of them was hypothetical. + +test('TAKEOVER: an org provider may not assert an email outside its own domains', () => { + /* + * The worst defect in this feature. An org admin supplies the issuer and client id, so they + * control the IdP completely and can mint a token asserting ANY email with email_verified:true — + * including a platform_admin's. Every cryptographic check passes honestly, because the attacker + * IS the issuer. Three reviewers demonstrated a full session as the victim independently. + * + * The confinement lives in the callback; this pins the data it depends on, so a provider loaded + * from the database always carries the domains its assertions are checked against. + */ + withOrgDb([{ id: '1', org: 'org-evil', slug: 'orgevil', name: 'Evil', domains: 'evil.test' }], (m) => { + const p = m.getOrgProvider('orgevil'); + assert.equal(p.emailDomains, 'evil.test', 'the callback cannot confine what it cannot see'); + assert.ok(!p.emailDomains.includes('victim'), 'and only ever the domains that were PROVED'); + assert.equal(p.organizationId, 'org-evil', 'and must know this is a tenant provider, not the operator\'s'); + }); +}); + +test('an INSTANCE provider carries no organization, so it is not domain-confined', () => { + // Operator-chosen providers keep the trust they have always had; confinement targets tenants. + const [g] = providers.list({ GOOGLE_CLIENT_ID: 'g' }); + assert.equal(g.organizationId, undefined); + assert.equal(g.source, 'env'); +}); + +test('a domain routes to exactly the provider that verified it', () => { + /* + * forEmail used an unordered SELECT over a comma column, so deleting and re-adding a provider + * silently flipped which IdP a whole domain routed to. + * + * Two earlier versions of this test were hollow: one asserted only that two calls agreed with + * each other (an unordered scan satisfies that within a process), the next used two DIFFERENT + * domains so no ordering was exercised at all. The property that actually matters is that a + * domain reaches its OWN provider and never a sibling's, so assert that. + */ + withOrgDb([ + { id: 'a', org: 'org-a', slug: 'orgaaa', name: 'Alpha', domains: 'alpha.test' }, + { id: 'b', org: 'org-b', slug: 'orgbbb', name: 'Beta', domains: 'beta.test', pending: 'gamma.test' }, + ], (m) => { + assert.equal(m.forEmail('x@alpha.test').name, 'Alpha'); + assert.equal(m.forEmail('x@beta.test').name, 'Beta'); + assert.equal(m.forEmail('x@gamma.test'), null, 'unverified, so it belongs to nobody'); + // Beta must not inherit Alpha's domain through a join or an ordering accident. + assert.equal(m.getOrgProvider('orgbbb').emailDomains, 'beta.test'); + assert.equal(m.getOrgProvider('orgaaa').emailDomains, 'alpha.test'); + }); +}); + +test('AN UNVERIFIED DOMAIN ROUTES NOBODY', () => { + /* + * The point of DNS verification. A tenant may type any domain — including a company they have + * nothing to do with — and until a record proves control it must buy them nothing: no routing, + * and (see the callback tests) no ability to assert an address inside it. + */ + withOrgDb([{ id: '1', org: 'org-x', slug: 'orgxxx', name: 'Squatter', pending: 'victim-corp.test' }], (m) => { + assert.equal(m.forEmail('ceo@victim-corp.test'), null, 'a claim is not a proof'); + const p = m.getOrgProvider('orgxxx'); + assert.equal(p.emailDomains, '', 'and the callback is given nothing it may confine to'); + }); +}); + +test('verifying one domain does not carry over to the others claimed with it', () => { + withOrgDb([{ + id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme', + domains: 'acme.test', pending: 'acme-partner.test', + }], (m) => { + assert.equal(m.forEmail('x@acme.test').name, 'Acme'); + assert.equal(m.forEmail('x@acme-partner.test'), null); + assert.equal(m.getOrgProvider('orgaaa').emailDomains, 'acme.test'); + }); +}); + +test('a secret that cannot be decrypted fails CLOSED', () => { + // decrypt() returns null after a JWT_SECRET rotation, which silently downgraded a confidential + // client to a public one — the login then failed at the provider with an error nobody could act + // on, while the admin screen still said "a secret is set". + withOrgDb([{ id: '1', org: 'o', slug: 'orgsec', name: 'X', domains: 'x.test' }], (m) => { + const real = require('../db/database'); + real.db.prepare('UPDATE org_sso_providers SET client_secret_enc = ? WHERE id = ?').run('not-decryptable', '1'); + assert.throws(() => m.getOrgProvider('orgsec'), /could not be decrypted/); + }); +}); + +test('a tenant cannot claim a public email provider as its sign-in domain', () => { + /* + * Demonstrated in review: a tenant claimed gmail.com, after which /sso/discover answered + * {"sso":true} for every Gmail address and the login page offered "sign in with your + * organization" — a phishing hop launched from the vendor's own login screen, pointed at + * infrastructure the tenant controls. First-claim-wins also meant one cheap account could deny a + * public domain to everyone else. + */ + const { isPublicEmailDomain } = require('../lib/public-email-domains'); + for (const d of ['gmail.com', 'outlook.com', 'hotmail.co.uk', 'yahoo.com', 'icloud.com', + 'proton.me', 'qq.com', 'mail.ru', 'comcast.net', 'gmx.de']) { + assert.ok(isPublicEmailDomain(d), `${d} must be refused as an org sign-in domain`); + } + // ...and a real company domain is still fine, or the feature would be pointless. + for (const d of ['acme.com', 'bigcorp.io', 'my-company.co.uk', 'mail.acme.com']) { + assert.equal(isPublicEmailDomain(d), false, `${d} must remain claimable`); + } +}); + +test('the blocklist is case- and whitespace-insensitive', () => { + // Domains arrive from a form. ` GMAIL.COM ` must not slip through a lowercase-only comparison. + const { isPublicEmailDomain } = require('../lib/public-email-domains'); + assert.ok(isPublicEmailDomain(' GMAIL.COM ')); + assert.ok(isPublicEmailDomain('Outlook.Com')); +}); + +// --------------------------------------------------------------------------------------------- +// The confinement itself. +// +// Everything above tests the DATA the callback confines against. These test the DECISION, which is +// what actually stops the takeover — and each one below was checked by reverting the guard and +// confirming the test goes red. A security test that passes against the vulnerable code is worse +// than no test, because it is read as coverage. + +const authRoutes = require('../routes/auth'); +const { emailAllowedForProvider } = authRoutes; + +const orgProvider = (domains) => ({ slug: 'orgabc', organizationId: 'org-a', emailDomains: domains }); + +test('CONFINEMENT: an org provider may only assert inside its verified domains', () => { + const p = orgProvider('acme.test'); + assert.equal(emailAllowedForProvider(p, 'staff@acme.test'), true); + assert.equal(emailAllowedForProvider(p, 'victim@other.test'), false, 'THE TAKEOVER'); + assert.equal(emailAllowedForProvider(p, 'admin@screentinker.com'), false); +}); + +test('CONFINEMENT: a provider with nothing verified may assert NOTHING', () => { + // The squatting case. A tenant types a domain, proves nothing, and must get nowhere — including + // for the domain they typed. + const p = orgProvider(''); + assert.equal(emailAllowedForProvider(p, 'ceo@victim-corp.test'), false); + assert.equal(emailAllowedForProvider(p, 'anyone@anywhere.test'), false); +}); + +test('CONFINEMENT: the domain cannot be smuggled past the check', () => { + const p = orgProvider('acme.test'); + for (const evil of [ + 'victim@other.test', // plainly outside + 'victim@acme.test.evil.test', // suffix, not the domain + 'victim@evil.test@acme.test\n', // trailing newline + 'victim@sub.acme.test', // subdomain is a different domain + 'victim@acme.test.', // trailing dot + 'victim@ACME.TEST.EVIL.TEST', + 'no-at-sign', + 'victim@', + '', + ]) { + assert.equal(emailAllowedForProvider(p, evil), false, `must refuse: ${JSON.stringify(evil)}`); + } + // ...while the legitimate forms still work, including the ones case normalisation must handle. + assert.equal(emailAllowedForProvider(p, 'Staff@Acme.Test'), true); + assert.equal(emailAllowedForProvider(p, 'a.b+tag@acme.test'), true); +}); + +test('CONFINEMENT: an INSTANCE provider is exempt, because the operator chose it', () => { + // Per-org verification is for tenant-supplied providers only. The instance's own Google or Okta + // is the operator's decision and is not domain-restricted — the same trust it has always had. + const instance = { slug: 'google', emailDomains: '' }; + assert.equal(emailAllowedForProvider(instance, 'anyone@anywhere.test'), true); + assert.equal(emailAllowedForProvider(instance, 'admin@gmail.com'), true); +}); + +// --------------------------------------------------------------------------------------------- +// Domain ownership. +// +// A claim is not a proof. These pin the part that makes that true: an unverified domain routes +// nobody, a claim lapses so it cannot be held forever, and a lapsed claim's token is dead so a +// record left behind from an earlier attempt cannot satisfy a later one. + +const domainVerify = require('../lib/domain-verify'); +const NOW = 1800000000; + +test('an unverified claim lapses after 8 hours; a verified one never does', () => { + const claim = (agoS, verified) => ({ token_issued_at: NOW - agoS, verified_at: verified ? NOW - 99 : null }); + assert.equal(domainVerify.isClaimExpired(claim(60, false), NOW), false, 'a minute old'); + assert.equal(domainVerify.isClaimExpired(claim(8 * 3600 - 30, false), NOW), false, 'just inside'); + assert.equal(domainVerify.isClaimExpired(claim(8 * 3600 + 1, false), NOW), true, 'just outside'); + // Proof does not rot. Re-verifying on a timer would log a customer out over a DNS edit made + // months after they legitimately proved the domain. + assert.equal(domainVerify.isClaimExpired(claim(365 * 86400, true), NOW), false, 'verified, a year old'); +}); + +test('the DNS record is per-domain and per-claim, so an old record proves nothing', () => { + const a = domainVerify.newToken(); + const b = domainVerify.newToken(); + assert.notEqual(a, b, 'two claims never share a token'); + assert.ok(a.length >= 32, 'not guessable'); + + const one = domainVerify.instructions('acme.test', a); + const two = domainVerify.instructions('acme.test', b); + assert.equal(one.record_name, '_screentinker-verify.acme.test'); + assert.notEqual(one.txt_value, two.txt_value, 'reissuing changes what must be published'); + // TXT is the only accepted form: a CNAME alternative would need a wildcard zone this project + // does not operate, so offering one would document a check that could never pass. + assert.equal(one.cname_value, undefined, 'no CNAME form is advertised'); + // The record lives at a dedicated name, never the apex, where it would sit beside SPF and DMARC. + assert.ok(!domainVerify.instructions('acme.test', a).record_name.startsWith('acme.test')); +}); + +test('a lapsed claim frees the domain for someone else', () => { + // The anti-squat property: a domain nobody can prove cannot be held indefinitely by whoever typed + // it first. Modelled here on the same predicate the route uses to decide whether a row blocks. + const squatter = { domain: 'victim-corp.test', token_issued_at: NOW - (9 * 3600), verified_at: null }; + const owner = { domain: 'victim-corp.test', token_issued_at: NOW - 60, verified_at: NOW }; + assert.equal(domainVerify.isClaimExpired(squatter, NOW), true, 'the squatter no longer blocks it'); + assert.equal(domainVerify.isClaimExpired(owner, NOW), false, 'the real owner, having proved it, does'); +}); + +/* + * The proof name must not be delegated. + * + * A TXT lookup follows CNAMEs transparently, and RFC 4592 means a wildcard `*.victim.com` + * synthesizes `_screentinker-verify.victim.com` as well. So a wildcard CNAME pointing at anything + * the attacker controls lets them publish the token in THEIR zone and prove a domain they do not + * own — turning an ordinary subdomain takeover into the whole company's sign-in. A review did + * exactly this against a real authoritative zone. + * + * The resolver is stubbed rather than mocked at the network layer: `dns.promises` is a singleton, + * so replacing the two methods is enough and the real check() runs unmodified. + */ +const dnsPromises = require('node:dns').promises; + +function withStubbedDns({ cname, txt }, fn) { + const realCname = dnsPromises.resolveCname; + const realTxt = dnsPromises.resolveTxt; + const nx = () => { const e = new Error('queryTxt ENOTFOUND'); e.code = 'ENOTFOUND'; throw e; }; + dnsPromises.resolveCname = async () => (cname ? cname : nx()); + dnsPromises.resolveTxt = async () => (txt ? txt : nx()); + return Promise.resolve(fn()).finally(() => { + dnsPromises.resolveCname = realCname; + dnsPromises.resolveTxt = realTxt; + }); +} + +test('DELEGATION: a CNAME at the proof name is refused, even when the TXT matches', async () => { + const token = 'deadbeefdeadbeefdeadbeefdeadbeef'; + // The attacker owns takeover.attacker.test and publishes a perfect token there; victim.test has + // a wildcard CNAME pointing at it. Without the refusal this returns ok:true. + const r = await withStubbedDns( + { cname: ['takeover.attacker.test'], txt: [[`st-verify=${token}`]] }, + () => domainVerify.check('victim.test', token), + ); + assert.equal(r.ok, false, 'a delegated proof name must never verify'); + assert.match(r.error, /CNAME/, 'and the admin is told exactly why'); +}); + +test('an ordinary TXT proof in the domain\'s own zone still verifies', async () => { + const token = 'cafebabecafebabecafebabecafebabe'; + const r = await withStubbedDns({ cname: null, txt: [[`st-verify=${token}`]] }, + () => domainVerify.check('acme.test', token)); + assert.equal(r.ok, true); + assert.equal(r.via, 'TXT'); +}); + +test('a wildcard TXT answers with its own value, which is not a proof', async () => { + const r = await withStubbedDns({ cname: null, txt: [['v=spf1 -all']] }, + () => domainVerify.check('victim.test', 'sometoken')); + assert.equal(r.ok, false); + assert.match(r.error, /does not match/); +}); + +test('a 255-byte-split TXT record is joined before comparing', async () => { + // resolveTxt returns one array of chunks per record; a long value arrives split. + const token = 'a'.repeat(32); + const full = `st-verify=${token}`; + const r = await withStubbedDns({ cname: null, txt: [[full.slice(0, 5), full.slice(5)]] }, + () => domainVerify.check('acme.test', token)); + assert.equal(r.ok, true, 'chunks of ONE record are concatenated'); +}); + +test('chunks are never joined ACROSS records', async () => { + const token = 'b'.repeat(32); + const full = `st-verify=${token}`; + const r = await withStubbedDns({ cname: null, txt: [[full.slice(0, 5)], [full.slice(5)]] }, + () => domainVerify.check('acme.test', token)); + assert.equal(r.ok, false, 'two unrelated records must not add up to a proof'); +}); + +test('SSRF: a trailing root dot is the same host, and does not slip the guard', () => { + // `https://localhost./` is a legal fully-qualified spelling that WHATWG URL preserves, so it + // matched neither `localhost` nor `*.localhost` and was allowed. The literal-IP forms were never + // affected — the parser normalises those itself. + for (const u of ['https://localhost./', 'https://LOCALHOST./', 'https://foo.localhost./']) { + assert.throws(() => oidc.assertFetchable(u), /not publicly routable/, u); + } + // and a real host that merely ends in a dot is still fine + for (const u of ['https://accounts.google.com./', 'https://fcm.googleapis.com./']) { + assert.doesNotThrow(() => oidc.assertFetchable(u), u); + } +}); + +test('a user object never leaves the server carrying a reset or verify hash', () => { + /* + * Two call sites each stripped three columns and stopped, so every login response also carried + * `password_reset_hash` and `email_verify_hash` — live credentials for taking the account over. + * The sanitiser is one function now; this pins the list so the next column added to `users` has + * to be considered rather than shipped. + */ + const src = fs.readFileSync(require.resolve('../routes/auth.js'), 'utf8'); + const block = src.slice(src.indexOf('const PRIVATE_USER_FIELDS'), src.indexOf('function publicUser')); + for (const field of ['password_hash', 'totp_secret_enc', 'totp_last_step', + 'password_reset_hash', 'password_reset_expires', 'email_verify_hash', 'email_verify_expires']) { + assert.ok(block.includes(`'${field}'`), `${field} must never be serialised to a client`); + } + // And nothing may hand-roll the old partial strip again. + assert.ok(!/totp_last_step,\s*\.\.\.safeUser/.test(src), 'use publicUser(), not an inline destructure'); +}); + +// --------------------------------------------------------------------------------------------- +// SSO-only: an organization requiring its own identity provider. + +test('SSO-ONLY applies to a VERIFIED domain', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme', domains: 'acme.test', ssoOnly: true }], (m) => { + const hit = m.ssoOnlyForEmail('staff@acme.test'); + assert.ok(hit, 'password login must be refused for this address'); + assert.equal(hit.organization_id, 'org-a'); + assert.equal(m.ssoOnlyForEmail('staff@ACME.TEST').organization_id, 'org-a', 'case-insensitive'); + assert.equal(m.ssoOnlyForEmail('someone@elsewhere.test'), null, 'and nobody else is affected'); + }); +}); + +test('SSO-ONLY CANNOT be imposed through a domain that was only claimed', () => { + /* + * The dangerous shape: switching off password login for a domain the tenant never proved would + * be a denial-of-service against a company they have nothing to do with — every account at that + * address locked out of a product the squatter does not own. + */ + withOrgDb([{ id: '1', org: 'org-x', slug: 'orgxxx', name: 'Squatter', pending: 'victim-corp.test', ssoOnly: true }], (m) => { + assert.equal(m.ssoOnlyForEmail('ceo@victim-corp.test'), null, 'an unproved domain compels nobody'); + }); +}); + +test('SSO-ONLY stops applying when the provider is disabled', () => { + // Otherwise disabling a broken provider would leave its users with no way in at all: no SSO + // (disabled) and no password (still enforced). + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme', domains: 'acme.test', enabled: 0, ssoOnly: true }], (m) => { + assert.equal(m.ssoOnlyForEmail('staff@acme.test'), null); + }); +}); + +test('SSO-ONLY is off unless the organization turned it on', () => { + withOrgDb([{ id: '1', org: 'org-a', slug: 'orgaaa', name: 'Acme', domains: 'acme.test' }], (m) => { + assert.equal(m.ssoOnlyForEmail('staff@acme.test'), null, 'having SSO is not the same as requiring it'); + }); +}); + +test('the login gate exempts platform_admin, and that exemption is deliberate', () => { + /* + * The operator approves turning SSO-only OFF. If the operator's own address sat at an SSO-only + * domain and that identity provider broke, nobody could sign in to approve anything and the + * instance would be bricked. Pinned as source because it is a security-relevant exemption that + * must not be "tidied away" by someone who reads it as a convenience. + */ + const src = fs.readFileSync(require.resolve('../routes/auth.js'), 'utf8'); + assert.match(src, /user\.role !== 'platform_admin'[\s\S]{0,600}ssoOnlyForUser/, + 'the break-glass exemption must guard the SSO-only check'); + assert.match(src, /code: 'sso_required'/, 'and the refusal must be distinguishable from a bad password'); +}); diff --git a/server/test/preflight-deps.test.js b/server/test/preflight-deps.test.js new file mode 100644 index 0000000..550738b --- /dev/null +++ b/server/test/preflight-deps.test.js @@ -0,0 +1,83 @@ +'use strict'; + +/* + * The boot-time dependency check. + * + * It exists for the moments nobody is at their best: a rollback that restores an older + * package.json but not its packages, and a Node upgrade that leaves the native database module + * compiled against the wrong ABI. Both present as "server will not start", with an error naming a + * file rather than the action needed. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const preflight = require('../lib/preflight-deps'); + +test('a healthy install reports nothing missing and nothing broken', () => { + assert.deepEqual(preflight.missingDeps(), [], 'this tree should be complete'); + assert.equal(preflight.nativeModuleBroken(), null, 'and the native module should load'); +}); + +test('THE NATIVE CHECK CONSTRUCTS A DATABASE, it does not merely require the module', () => { + /* + * better-sqlite3's entry point is plain JavaScript that loads the compiled binding lazily, so + * `require()` SUCCEEDS under a Node whose ABI the binary was never built for. The first version + * of this check stopped at require and therefore reported a genuinely broken install — verified + * against a real Node 18 / Node 20 mismatch — as healthy. + * + * Pinned as source because the failure is invisible: the check keeps passing, on every machine + * where nothing is wrong, right up until the one where something is. + */ + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'preflight-deps.js'), 'utf8'); + const fn = src.slice(src.indexOf('function nativeModuleBroken'), src.indexOf('function run(')); + assert.match(fn, /new Database\(':memory:'\)/, + 'nativeModuleBroken must open a database, or an ABI mismatch goes undetected'); + assert.ok(!/^\s*require\('better-sqlite3'\);\s*$/m.test(fn), 'a bare require is not a load test'); +}); + +test('missingDeps agrees with what is actually on disk', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, '..', 'package.json'), 'utf8')); + const declared = Object.keys(pkg.dependencies || {}); + assert.ok(declared.length > 0, 'the server declares dependencies'); + const reported = preflight.missingDeps(); + for (const name of declared) { + const present = fs.existsSync(path.join(__dirname, '..', 'node_modules', name, 'package.json')); + assert.equal(present, !reported.includes(name), `${name}: presence and report disagree`); + } +}); + +test('the preflight uses only Node builtins', () => { + /* + * It runs BEFORE dependencies are installed, so anything it imported could be the very thing + * that is missing — and the failure would be the one it exists to prevent, with an extra layer + * of confusion on top. + */ + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'preflight-deps.js'), 'utf8'); + const requires = [...src.matchAll(/require\('([^']+)'\)/g)].map((m) => m[1]); + const builtins = new Set(['fs', 'path', 'child_process', 'os', 'crypto', 'util']); + for (const r of requires) { + // better-sqlite3 is the thing being TESTED for loadability, not a dependency of this file. + if (r === 'better-sqlite3') continue; + assert.ok(builtins.has(r) || r.startsWith('.'), `preflight must not depend on ${r}`); + } +}); + +test('it can be turned off for an air-gapped host', () => { + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'preflight-deps.js'), 'utf8'); + assert.match(src, /ST_SKIP_DEP_PREFLIGHT/, 'an operator who manages node_modules must be able to opt out'); + const saved = process.env.ST_SKIP_DEP_PREFLIGHT; + process.env.ST_SKIP_DEP_PREFLIGHT = '1'; + try { assert.doesNotThrow(() => preflight.preflight(), 'opting out must be a clean no-op'); } + finally { if (saved === undefined) delete process.env.ST_SKIP_DEP_PREFLIGHT; else process.env.ST_SKIP_DEP_PREFLIGHT = saved; } +}); + +test('server.js runs the preflight BEFORE requiring anything', () => { + const src = fs.readFileSync(path.join(__dirname, '..', 'server.js'), 'utf8'); + const preflightAt = src.indexOf("require('./lib/preflight-deps')"); + const firstDep = src.indexOf("require('express')"); + assert.ok(preflightAt > -1, 'server.js must run the preflight'); + assert.ok(preflightAt < firstDep, 'it must come before the first dependency, or it cannot help'); +});