Skip to content

feat(auth): support multiple Google hosted domains#605

Merged
nicholasjjlim merged 4 commits into
mainfrom
feat/google-multi-hosted-domain
Jul 1, 2026
Merged

feat(auth): support multiple Google hosted domains#605
nicholasjjlim merged 4 commits into
mainfrom
feat/google-multi-hosted-domain

Conversation

@nicholasjjlim

@nicholasjjlim nicholasjjlim commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two related changes to Google sign-in:

  1. Multi-domain allowlist. GOOGLE_HOSTED_DOMAIN now accepts a comma-separated list of allowed Google Workspace domains (e.g. moe.edu.sg,hci.edu.sg). A single value behaves exactly as before.
  2. Domain-specific rejection message. When a sign-in is rejected because the account's domain isn't allowed, the learner login page now shows an inline message instead of silently bouncing back.

Related: #603

Changes

Multi-domain allowlist

  • getAllowedHostedDomains() — parses GOOGLE_HOSTED_DOMAIN by splitting on commas, trimming whitespace, and dropping empties. Returns [] when unset (no restriction).
  • verifyIdToken (the security gate) — rejects unless the token's signed hd claim is one of the configured domains; a missing hd claim is still rejected when any domain is configured. Personal @gmail.com accounts (no hd claim) and other Workspace domains are rejected.
  • generateAuthURL (UI hint only) — passes the single domain when one is configured, and falls back to * for multiple, since Google's hd parameter accepts only one value. Per Google's docs, hd is an account-chooser hint and not a security control — enforcement lives entirely in verifyIdToken.

Domain-specific rejection message

  • HostedDomainMismatchError — a subclass of InvalidIdTokenError thrown specifically for the hosted-domain mismatch (and missing-hd) case. Existing catch-all handling still treats it as a verification failure; callers that care can distinguish it.
  • Learner Google callback — maps HostedDomainMismatchError to /login?error=domain_not_allowed (logged at warn, since an outside-domain sign-in is expected behavior, not a fault). All other verification failures still funnel to the generic oauth2_callback_failed.
  • Login page — reads the error param and renders an inline message: "You can only sign in with an approved organisation account. Please try again with your work email."

Testing

google.test.ts (10 tests): single / multiple / no configured domains for both functions, comma whitespace tolerance, missing-hd rejection, and that the rejection throws HostedDomainMismatchError (and is still an InvalidIdTokenError).

  • pnpm test run src/lib/server/auth/google.test.ts — 10 passed
  • pnpm check — 0 errors, 0 warnings
  • Manually verified end-to-end: a non-allowlisted account is rejected and the login page shows the message.

Notes / out of scope

  • The admin Google callback uses the same verifyIdToken but still funnels all failures to its generic error; it could mirror the domain-specific message later if desired.
  • The callback branch and login-page banner aren't unit-tested (no existing harness for that route); covered by the unit test on the error type plus manual verification.

GOOGLE_HOSTED_DOMAIN now accepts a comma-separated list (e.g.
"moe.edu.sg,hci.edu.sg"). verifyIdToken rejects any token whose hd
claim is not one of the configured domains, and a missing hd claim is
still rejected when any domain is set. The auth-URL hd hint passes the
single domain when one is configured and falls back to "*" for
multiple, since hd accepts only one value; enforcement remains in
verifyIdToken (hd is a UI hint, not a security control).

Related: #603

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicholasjjlim nicholasjjlim self-assigned this Jun 30, 2026
@nicholasjjlim
nicholasjjlim requested a review from santosral June 30, 2026 09:30
nicholasjjlim and others added 2 commits June 30, 2026 17:44
Add HostedDomainMismatchError (a subclass of InvalidIdTokenError) thrown
when a token's hosted domain is not in GOOGLE_HOSTED_DOMAIN. The learner
Google callback maps it to /login?error=domain_not_allowed (logged at
warn, since an outside-domain sign-in is expected), and the login page
renders an inline message telling the user to use an approved
organisation account. Other verification failures still funnel to the
generic oauth2_callback_failed with no banner.

Related: #603

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address code-review feedback on #605:

- Document that verifyIdToken enforces the restriction on the verified
  hd (Workspace) claim, not the email domain, so any account in an
  allow-listed Workspace is admitted regardless of its email's literal
  domain. No behavior change.
- Generalize the login page so every error code surfaces a message:
  domain_not_allowed keeps its tailored text, and any other code (e.g.
  oauth2_callback_failed) now shows a generic retry message instead of a
  bare page.

Related: #603

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicholasjjlim
nicholasjjlim marked this pull request as ready for review June 30, 2026 09:58
Comment thread src/lib/server/auth/google.ts Outdated
@nicholasjjlim

Copy link
Copy Markdown
Collaborator Author

Code Review Summary

Severity Count
Critical 0
High 0
Medium 0
Low 1

One Low finding posted inline on src/lib/server/auth/google.ts: domain matching normalises whitespace but not case, so a mis-cased config entry would silently lock out all users of that domain.

What Looks Good

  • HostedDomainMismatchError subclassing InvalidIdTokenError is the right call — existing catch-all handlers keep rejecting it, while the callback can branch on it for a tailored message; and the callback correctly checks the subclass before the generic case.
  • The hd account-chooser hint logic (undefined for none, the domain for one, * for many) is sound, and the inline comment correctly notes the real enforcement still lives in verifyIdToken — the hint is never trusted as the gate.
  • Strong test coverage: single vs. multi-domain accept/reject, whitespace tolerance, missing-hd rejection, and the unconfigured pass-through case are all exercised, including asserting the error is both a HostedDomainMismatchError and an InvalidIdTokenError.
  • The login page maps known error codes to tailored copy and falls back to a generic message for any other non-empty code, with an accessible role="alert" container.

🤖 aif-code-review · claude-opus-4-8[1m]

@nicholasjjlim nicholasjjlim added the skill:aif-code-review Reviewed with the aif-code-review skill label Jun 30, 2026
Google's hd claim is always lower-case, so a config like
GOOGLE_HOSTED_DOMAIN=MOE.edu.sg would silently reject every user of that
domain. Lower-case allow-list entries when parsing so matching is
case-insensitive.

Addresses code-review feedback on #605.
Related: #603

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicholasjjlim

Copy link
Copy Markdown
Collaborator Author

LGTM 👍


🤖 aif-code-review · claude-opus-4-8[1m]

@nicholasjjlim
nicholasjjlim merged commit d30cc2e into main Jul 1, 2026
8 checks passed
@nicholasjjlim
nicholasjjlim deleted the feat/google-multi-hosted-domain branch July 1, 2026 02:19
@nicholasjjlim nicholasjjlim mentioned this pull request Jul 1, 2026
wondopamine added a commit to transformteamsg/onward-design that referenced this pull request Jul 1, 2026
The multi-hosted-domain auth work merged from main (String-dxd#605) added a login
error notice styled with raw palette utilities (bg-red-50 / text-red-700),
which token-audit [COL-2] flags. Re-point it to the semantic layer to match
the app's canonical error-notice pattern (unit/[id] uses the same tokens):
- bg-red-50  → bg-destructive/10
- text-red-700 → text-destructive
Behaviour and layout unchanged; keeps the token-audit gate clean app-wide
after the rebase onto v1.16.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WrZm9PZ693J1R2j5tV7cEF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skill:aif-code-review Reviewed with the aif-code-review skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants