Skip to content

fix(platform): route SSO login to the org matching the user's email (#2082)#2125

Open
larryro wants to merge 2 commits into
mainfrom
tale/xs7c4b1dx9ta9msqtpmq5ym3ad8981xd
Open

fix(platform): route SSO login to the org matching the user's email (#2082)#2125
larryro wants to merge 2 commits into
mainfrom
tale/xs7c4b1dx9ta9msqtpmq5ym3ad8981xd

Conversation

@larryro

@larryro larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #2082 — OIDC/OAuth2 SSO login ignored the user's email and always routed to the first enabled connection, so on multi-org deployments second-org users landed at the wrong org's IdP.

Each SSO connection is scoped per organization (the tenant boundary), but the login flow never told the server which org to use: handleSsoLogin hit /authorize with no email/org, and both the authorize and callback handlers called resolveSignInConfig({}) which falls through to loadSingleEnabled (first enabled row). The email was only forwarded as an OIDC login_hint. The email-domain router (discoverByEmail / POST /api/sso/discover) already existed but had zero frontend callers.

Changes

  • app/routes/_auth/log-in.tsxhandleSsoLogin now POSTs the entered email to /api/sso/discover first, then routes to the resolved org. It threads organizationId to /authorize (and org to SAML login), and branches SAML-vs-OIDC on the discovered protocol, falling back to the deployment-global status when discovery doesn't resolve (empty email / single-org / network failure).
  • convex/enterprise_sso/login/authorize_handler.ts — reads organizationId from the query, scopes resolveSignInConfig to it, and carries the resolved org in the signed state.
  • convex/enterprise_sso/login/callback_handler.ts — recovers organizationId from state and resolves the same connection (including on seamless/step-up retries) so the code is exchanged against the correct IdP.
  • convex/enterprise_sso/internal_queries.test.ts — new tests for per-org resolveSignInConfig scoping and discoverByEmail domain routing + fallback.

Verification

  • tsc --noEmit — clean
  • oxlint --type-aware on changed files — clean
  • vitest --project server enterprise_sso — 46 passed (incl. 3 new)

Single-org self-hosted deployments are unaffected: discovery falls back to the single enabled connection exactly as before.

Tale Agent added 2 commits June 24, 2026 16:15
…2082)

Multi-org deployments key each OIDC/OAuth2 connection per organization, but the login flow ignored the entered email and the server fell back to the first enabled connection, so second-org users landed at the wrong IdP.

- log-in: resolve the org+protocol from the email via /api/sso/discover before starting the flow; thread organizationId to /authorize and SAML login; branch SAML-vs-OIDC on the discovered protocol with the global status as fallback.
- authorize_handler: scope resolveSignInConfig by organizationId and carry the resolved org in the signed state.
- callback_handler: resolve the same org from state (and on seamless/step-up retries) so the code is exchanged against the correct IdP.
- add internal_queries tests for per-org resolveSignInConfig and discoverByEmail routing.
@larryro

larryro commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Desk Review — #2082 multi-org SSO routing

Verdict: READY TO MERGE.

Reviewed branch tale/xs7c4b1dx9ta9msqtpmq5ym3ad8981xd (PR #2125) with a two-round fan-out (5 breadth subagents across correctness / robustness+security / tests / elegance / resolution, then independent confirmation of every blocking candidate).

CI & tests

  • CI: green. gh pr checks — all required checks pass (Type check, Lint, Unit, Build platform/convex, Knip, Format, full Playwright platform 1–16/16, Smoke). No red, no pending; remaining entries are skipping fork/container jobs.
  • Tests run locally: cd services/platform && bunx vitest --run --project server enterprise_sso/40 passed (6 files), including the 3 new internal_queries.test.ts cases.

What the fix does (all three issue points implemented)

  1. handleSsoLogin now POSTs the entered email to /api/sso/discover and uses the returned organizationId + protocol (app/routes/_auth/log-in.tsx). The previously-unwired /api/sso/discover endpoint now has a caller.
  2. organizationId is threaded through the signed state: authorize_handler.ts:99 stores the resolved config.organizationId; callback_handler.ts:189 resolves the same org from verified state (not first-enabled). Both error-retry paths (silent-auth :84, step-up :107) re-thread it.
  3. The SAML-vs-OIDC branch uses discovery's protocol (log-in.tsx:303,307), with ?org= forwarded to the SAML login handler (which already reads it).

Correctness — verified across branches

  • authorize always pins a resolved org into state (explicit param or single-enabled fallback); callback re-resolves the same one. loadConnection(org) and loadSingleEnabled read the same configCache slice, so single-org behavior is unchanged.
  • Empty email / ssoEnabled:false / discovery network-or-parse failure all fall back to the deployment-global status without blocking sign-in (console.warn, no empty catch — repo rule respected).
  • If a connection is disabled between authorize and callback, resolveSignInConfig returns null and the callback redirects with a clean error.
  • The fallback resolvedProtocol = protocol ?? ssoConfig?.providerType reproduces the exact pre-fix branch behavior (SAML connections surface providerType:'saml' via isConfigured), so there is no regression.

Security

  • The organizationId/org query param to /authorize is attacker-controllable but benign: the resolved config is non-secret by design, secrets are fetched server-side per resolved org, and identity comes from the IdP. The org used at callback comes from the signed state, not user input — no cross-org confusion.
  • Email is carried via URLSearchParams (correctly encoded); used only as OIDC login_hint.

Non-blocking notes (for awareness, not required for merge)

  1. Domain routing requires domains to be configured. In a multi-org deployment where an org has no domain set, or where two orgs share a domain, discoverByEmail falls back to / tie-breaks on the first enabled connection in (non-deterministic) iteration order. This is inherent to domain-based routing and outside the reported bug (which is about routing when the domain matches). Consider enforcing unique, configured domains per deployment, or documenting the requirement.
  2. Test scope. The new tests cover the query layer (org-scoped resolve, domain routing, no-match fallback). The handler-level state threading is not unit-tested — consistent with the repo, where no SSO HTTP handler is unit-tested (those paths are covered by the green Playwright e2e suite). An optional resolveSignInConfig({}) (no-org) case and an email-without-@ case would round out the edge coverage.
  3. The PR also carries an unrelated oxfmt formatting commit on desk-process.json (separate commit, no logic change) — harmless.

None of the above blocks merge.

READY TO MERGE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: OIDC/OAuth2 SSO login ignores the user's email and always routes to the first enabled connection (multi-org IdP collision)

1 participant