diff --git a/src/lib/server/auth/google.test.ts b/src/lib/server/auth/google.test.ts new file mode 100644 index 00000000..7ef35f5c --- /dev/null +++ b/src/lib/server/auth/google.test.ts @@ -0,0 +1,130 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +// Mutable env object: google.ts reads `env.GOOGLE_HOSTED_DOMAIN` at call time, +// so mutating this between tests is enough — no need to re-import the module. +const env: Record = {}; +vi.mock('$env/dynamic/private', () => ({ env })); + +const mockVerifyIdToken = vi.fn(); +const mockGenerateAuthUrl = vi.fn(); +const mockGetToken = vi.fn(); +vi.mock('google-auth-library', () => ({ + CodeChallengeMethod: { S256: 'S256' }, + OAuth2Client: vi.fn().mockImplementation(() => ({ + verifyIdToken: mockVerifyIdToken, + generateAuthUrl: mockGenerateAuthUrl, + getToken: mockGetToken, + })), +})); + +function payloadWithHd(hd: string | undefined) { + return { + getPayload: () => ({ sub: 'user-id', email: 'a@b.com', name: 'Test', picture: null, hd }), + }; +} + +beforeEach(() => { + delete env.GOOGLE_HOSTED_DOMAIN; +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe('verifyIdToken hosted-domain enforcement', () => { + test('accepts a token whose hd matches the single configured domain', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('moe.edu.sg')); + + const { verifyIdToken } = await import('./google.js'); + await expect(verifyIdToken('token')).resolves.toMatchObject({ email: 'a@b.com' }); + }); + + test('rejects a token whose hd does not match the single configured domain', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('gmail.com')); + + const { verifyIdToken, HostedDomainMismatchError, InvalidIdTokenError } = await import( + './google.js' + ); + // A subclass of InvalidIdTokenError so existing catch-all handling still applies, + // but distinguishable so the callback can show a domain-specific message. + await expect(verifyIdToken('token')).rejects.toBeInstanceOf(HostedDomainMismatchError); + await expect(verifyIdToken('token')).rejects.toBeInstanceOf(InvalidIdTokenError); + }); + + test('accepts a token whose hd matches any of multiple configured domains', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg,hci.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('hci.edu.sg')); + + const { verifyIdToken } = await import('./google.js'); + await expect(verifyIdToken('token')).resolves.toMatchObject({ email: 'a@b.com' }); + }); + + test('rejects a token whose hd is in none of the multiple configured domains', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg,hci.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('nus.edu.sg')); + + const { verifyIdToken, HostedDomainMismatchError } = await import('./google.js'); + await expect(verifyIdToken('token')).rejects.toBeInstanceOf(HostedDomainMismatchError); + }); + + test('tolerates whitespace around comma-separated domains', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg , hci.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('hci.edu.sg')); + + const { verifyIdToken } = await import('./google.js'); + await expect(verifyIdToken('token')).resolves.toMatchObject({ email: 'a@b.com' }); + }); + + test('matches domains case-insensitively', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'MOE.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd('moe.edu.sg')); + + const { verifyIdToken } = await import('./google.js'); + await expect(verifyIdToken('token')).resolves.toMatchObject({ email: 'a@b.com' }); + }); + + test('rejects a token with no hd claim when a domain is configured', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg'; + mockVerifyIdToken.mockResolvedValue(payloadWithHd(undefined)); + + const { verifyIdToken, HostedDomainMismatchError } = await import('./google.js'); + await expect(verifyIdToken('token')).rejects.toBeInstanceOf(HostedDomainMismatchError); + }); + + test('accepts any hd when no domain is configured', async () => { + mockVerifyIdToken.mockResolvedValue(payloadWithHd('gmail.com')); + + const { verifyIdToken } = await import('./google.js'); + await expect(verifyIdToken('token')).resolves.toMatchObject({ email: 'a@b.com' }); + }); +}); + +describe('generateAuthURL hosted-domain hint', () => { + function hdArg() { + return mockGenerateAuthUrl.mock.calls[0][0].hd; + } + + async function callGenerate() { + const { generateAuthURL } = await import('./google.js'); + generateAuthURL({ origin: 'https://app.example.com', state: 's', codeVerifier: 'v' }); + } + + test('passes the single configured domain as the hd hint', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg'; + await callGenerate(); + expect(hdArg()).toBe('moe.edu.sg'); + }); + + test('passes the wildcard hint when multiple domains are configured', async () => { + env.GOOGLE_HOSTED_DOMAIN = 'moe.edu.sg,hci.edu.sg'; + await callGenerate(); + expect(hdArg()).toBe('*'); + }); + + test('passes no hd hint when no domain is configured', async () => { + await callGenerate(); + expect(hdArg()).toBeUndefined(); + }); +}); diff --git a/src/lib/server/auth/google.ts b/src/lib/server/auth/google.ts index e0fda22c..b15735b6 100644 --- a/src/lib/server/auth/google.ts +++ b/src/lib/server/auth/google.ts @@ -28,6 +28,21 @@ export interface GoogleProfile { */ const GOOGLE_REDIRECT_URL_PATH = '/auth/google/callback'; +/** + * Parses `GOOGLE_HOSTED_DOMAIN` into the list of allowed Google Workspace + * domains. Accepts a single domain or a comma-separated list; whitespace around + * entries is ignored and entries are lower-cased so matching is + * case-insensitive (Google's `hd` claim is always lower-case, so a config like + * `MOE.edu.sg` would otherwise silently reject every user). Returns an empty + * array when unset, meaning no hosted-domain restriction is enforced. + */ +function getAllowedHostedDomains(): string[] { + return (env.GOOGLE_HOSTED_DOMAIN ?? '') + .split(',') + .map((domain) => domain.trim().toLowerCase()) + .filter((domain) => domain.length > 0); +} + function getOAuth2Client() { return new OAuth2Client({ client_id: env.GOOGLE_CLIENT_ID, @@ -67,6 +82,14 @@ export function generateAuthURL({ }): string { const client = getOAuth2Client(); + // `hd` is only an account-chooser hint and accepts a single value. With one + // configured domain we pass it directly; with several we fall back to `*`, + // which limits the chooser to managed Workspace accounts. The actual + // restriction is enforced in `verifyIdToken`. + const allowedDomains = getAllowedHostedDomains(); + const hd = + allowedDomains.length === 0 ? undefined : allowedDomains.length === 1 ? allowedDomains[0] : '*'; + return client.generateAuthUrl({ redirect_uri: origin + GOOGLE_REDIRECT_URL_PATH, scope: [ @@ -76,7 +99,7 @@ export function generateAuthURL({ state, code_challenge: createHash('sha256').update(codeVerifier).digest('base64url'), code_challenge_method: CodeChallengeMethod.S256, - hd: env.GOOGLE_HOSTED_DOMAIN, + hd, prompt, }); } @@ -127,11 +150,20 @@ export async function exchangeCodeForIdToken({ * Verifies the Google ID token and extracts the profile information. * * Throws `InvalidIdTokenError` when the token cannot be trusted — bad - * signature, expired, missing claims, or hosted-domain mismatch. + * signature, expired, or missing claims — and `HostedDomainMismatchError` + * (a subclass) when the token's hosted domain is not allow-listed. + * + * The hosted-domain restriction is enforced on the Google-verified `hd` + * (Workspace) claim, not on the email address. `hd` is the account's Workspace + * primary domain, so any account belonging to an allow-listed Workspace is + * admitted regardless of its email's literal domain (e.g. a secondary or alias + * domain). This matches the `GOOGLE_HOSTED_DOMAIN` semantics — it lists + * Workspace domains, not email domains — and is Google's recommended gate. * * @param idToken - The Google ID token to verify. * @returns The Google profile. * @throws InvalidIdTokenError + * @throws HostedDomainMismatchError */ export async function verifyIdToken(idToken: string): Promise { const client = getOAuth2Client(); @@ -153,9 +185,12 @@ export async function verifyIdToken(idToken: string): Promise { throw new InvalidIdTokenError(`Google ID token missing claim: ${missing}`); } - if (env.GOOGLE_HOSTED_DOMAIN && payload.hd !== env.GOOGLE_HOSTED_DOMAIN) { - throw new InvalidIdTokenError( - 'Google ID token hosted domain does not match the configured hosted domain', + // Gate on the verified `hd` (Workspace) claim, not the email domain — see the + // function docstring for why. + const allowedDomains = getAllowedHostedDomains(); + if (allowedDomains.length > 0 && (!payload.hd || !allowedDomains.includes(payload.hd))) { + throw new HostedDomainMismatchError( + 'Google ID token hosted domain does not match a configured hosted domain', ); } @@ -171,3 +206,11 @@ export class InvalidIdTokenError extends Error { this.name = this.constructor.name; } } + +/** + * Thrown when the token is otherwise valid but its hosted domain is not one of + * the domains configured in `GOOGLE_HOSTED_DOMAIN`. A subclass of + * `InvalidIdTokenError` so existing catch-all handling still rejects it, while + * callers that care can distinguish it to surface a domain-specific message. + */ +export class HostedDomainMismatchError extends InvalidIdTokenError {} diff --git a/src/routes/(main)/auth/google/callback/+server.ts b/src/routes/(main)/auth/google/callback/+server.ts index 3c953e99..2e48bd9f 100644 --- a/src/routes/(main)/auth/google/callback/+server.ts +++ b/src/routes/(main)/auth/google/callback/+server.ts @@ -4,6 +4,7 @@ import { HOME_PATH } from '$lib/helpers'; import { exchangeCodeForIdToken, type GoogleProfile, + HostedDomainMismatchError, learnerAuth, verifyIdToken, } from '$lib/server/auth/index.js'; @@ -72,6 +73,10 @@ export const GET: RequestHandler = async (event) => { try { profile = await verifyIdToken(idToken); } catch (err) { + if (err instanceof HostedDomainMismatchError) { + logger.warn({ err }, 'Rejected sign-in from a non-allowed hosted domain'); + return redirect(302, '/login?error=domain_not_allowed'); + } logger.error({ err }, 'Failed to verify ID token'); return redirect(302, '/login?error=oauth2_callback_failed'); } diff --git a/src/routes/(main)/login/+page.svelte b/src/routes/(main)/login/+page.svelte index 32677c88..7f2ba9c4 100644 --- a/src/routes/(main)/login/+page.svelte +++ b/src/routes/(main)/login/+page.svelte @@ -2,6 +2,22 @@ import { page } from '$app/state'; import { LinkButton } from '$lib/components/Button/index.js'; import { HOME_PATH } from '$lib/helpers/index.js'; + + const ERROR_MESSAGES: Record = { + domain_not_allowed: + 'You can only sign in with an approved organisation account. Please try again with your work email.', + }; + const GENERIC_ERROR_MESSAGE = 'Something went wrong while signing you in. Please try again.'; + + // Map the `error` query param to a user-facing message. Known codes get a + // tailored message; any other non-empty code falls back to the generic one. + const errorMessage = $derived.by(() => { + const error = page.url.searchParams.get('error'); + if (!error) { + return null; + } + return ERROR_MESSAGES[error] ?? GENERIC_ERROR_MESSAGE; + });
@@ -22,6 +38,15 @@
+ {#if errorMessage} + + {/if} +