diff --git a/apps/www/src/client/App.tsx b/apps/www/src/client/App.tsx index c7c18ce..83ec386 100644 --- a/apps/www/src/client/App.tsx +++ b/apps/www/src/client/App.tsx @@ -30,10 +30,22 @@ interface SiteConfig { turnstileSiteKey: string | null } +/** + * An OAuth machine code (`invalid_grant`, `server_error`) rather than a sentence — a + * lower_snake_case word with no spaces. Bodies relayed from a worker that speaks OAuth + * put one of these in `error`, where the readable reason is in `error_description`. + */ +const isErrorCode = (s: string) => /^[a-z][a-z\d]*(_[a-z\d]+)+$/.test(s) + /** * Call a www BFF endpoint. GET when no body is given, else POST JSON. Throws with * the upstream error message (auth uses `error`/`error_description`, the account * mutations use `error`) so callers can surface it. + * + * `error` wins, since that's where www puts the message it wrote for the player — but + * NOT when it's a bare OAuth code: showing "invalid_grant" tells nobody anything, so a + * relayed OAuth body falls through to its description. www translates the signup/login + * grants itself (see `readAuthError`); this covers the endpoints that still relay. */ async function api(path: string, body?: unknown): Promise { const res = await fetch(path, { @@ -43,9 +55,12 @@ async function api(path: string, body?: unknown): Promise { }) const data = (await res.json().catch(() => ({}))) as Record if (!res.ok) { + const error = typeof data.error === 'string' ? data.error : '' + const description = typeof data.error_description === 'string' ? data.error_description : '' const message = - (typeof data.error === 'string' && data.error) || - (typeof data.error_description === 'string' && data.error_description) || + (error && !(isErrorCode(error) && description) && error) || + description || + error || `Request failed (${res.status})` throw new Error(message) } diff --git a/apps/www/src/test/integration/api.test.ts b/apps/www/src/test/integration/api.test.ts index 5501645..70a6225 100644 --- a/apps/www/src/test/integration/api.test.ts +++ b/apps/www/src/test/integration/api.test.ts @@ -4,6 +4,7 @@ import { beforeAll, expect, it } from 'vitest' import { DOCUMENTED_SERVICES } from '../../docs' import { DISCORD_INVITE, ISSUES_URL, PRIVACY_EMAIL } from '../../links' import { turnstileKeys } from '../../turnstile' +import { readAuthError } from '../../upstream' import type { Env } from '../../context' @@ -110,6 +111,58 @@ it('refuses a signup with no password', async () => { expect(await res.json()).toEqual({ error: 'A password is required.' }) }) +// A refused grant reaches the form as a sentence, never as the OAuth code. auth answers +// `{ error: 'invalid_grant', error_description: }`, and www used to +// relay that untouched — so every failed signup, including one the player could act on +// (the per-network cap), read simply "invalid_grant". Checked directly because the pass +// path can't be reached from here (it would call the real auth worker). +it('explains a refused signup instead of relaying invalid_grant', async () => { + const refused = (description: string, status = 400) => + new Response(JSON.stringify({ error: 'invalid_grant', error_description: description }), { + status, + headers: { 'content-type': 'application/json' }, + }) + + const capped = await readAuthError( + refused('too many accounts created from this network'), + 'signup' + ) + expect(capped.status).toBe(400) + expect(capped.message).toContain('Too many accounts have already been created from your network') + // The raw pair still reaches the operator's log line. + expect(capped.upstream).toBe('invalid_grant: too many accounts created from this network') + + const badPassword = await readAuthError(refused('invalid account_id or password'), 'login') + expect(badPassword.message).toBe('That username or password is incorrect.') + + // A description auth grew since this table was written must not leak through as-is: + // it's written for an operator, so an unmapped one falls back to the generic sentence. + const unmapped = await readAuthError(refused('some new internal reason'), 'signup') + expect(unmapped.message).not.toContain('some new internal reason') + expect(unmapped.message).toContain('could not be created') + + // Nothing about the form was wrong — auth couldn't proceed (an unset JWT_SECRET). Don't + // send them back to re-check their details, and don't answer 400 for our own fault. + const broken = await readAuthError( + new Response( + JSON.stringify({ + error: 'server_error', + error_description: 'token signing is not configured', + }), + { status: 500, headers: { 'content-type': 'application/json' } } + ), + 'signup' + ) + expect(broken.status).toBe(502) + expect(broken.message).toContain('problem on our end') + + // A body from something in front of auth (an edge error page) is not JSON at all. + const html = await readAuthError(new Response('502', { status: 502 }), 'signup') + expect(html.status).toBe(502) + expect(html.message).toContain('problem on our end') + expect(html.upstream).toBe('HTTP 502') +}) + it('requires credentials to log in', async () => { const res = await SELF.fetch('https://example.com/api/login', { method: 'POST', diff --git a/apps/www/src/upstream.ts b/apps/www/src/upstream.ts index e07db26..2ec8a42 100644 --- a/apps/www/src/upstream.ts +++ b/apps/www/src/upstream.ts @@ -34,3 +34,90 @@ export async function postForm( body: new URLSearchParams(fields).toString(), }) } + +/** Which grant www was making, so a shared refusal reads right on either form. */ +export type AuthAction = 'signup' | 'login' + +/** A rejected `/connect/token` grant, translated for the browser. */ +export interface AuthFailure { + /** The sentence to put in front of the player. */ + message: string + /** 400 when the grant was refused, 502 when `auth` itself couldn't proceed. */ + status: 400 | 502 + /** The raw `error`/`error_description` pair, for the operator's log line only. */ + upstream: string +} + +/** + * What each of auth's `error_description`s means to somebody filling in a form. + * + * Keyed on the exact string auth sends (see its `/connect/token` handler). Only a few + * are reachable from the web — signup posts `create_account` with no platform, login + * posts `password` with no `platform_auth` — but the platform arms are mapped anyway so + * a future web flow that does assert one can't regress to a bare code. + */ +const AUTH_MESSAGES: Record = { + 'too many accounts created from this network': + 'Too many accounts have already been created from your network. Try again later, or from a different connection.', + 'account limit reached for this platform account': + 'This platform account has already created as many accounts as it is allowed.', + 'invalid account_id or password': 'That username or password is incorrect.', + 'account_id or username is required': 'Username and password are required.', + 'invalid or missing platform_auth': 'Your platform sign-in could not be verified.', + 'unsupported platform; only Steam and Meta can be verified': + 'That platform cannot be verified — only Steam and Meta are supported.', + 'no linked account for this platform identity': + 'No account is linked to this platform sign-in yet. Sign in with your password once to link it.', + 'refresh_token is invalid or expired': 'Your session has expired. Please sign in again.', +} + +/** Fallbacks when nothing above matched, so a player never reads an OAuth code. */ +const GENERIC_MESSAGES: Record = { + signup: { + rejected: 'Your account could not be created. Please check your details and try again.', + broken: + 'Accounts cannot be created right now. This is a problem on our end — please try again later.', + }, + login: { + rejected: 'You could not be signed in. Please check your details and try again.', + broken: + 'Sign-in is unavailable right now. This is a problem on our end — please try again later.', + }, +} + +/** Message for an `auth` that couldn't be reached at all (the fetch itself threw). */ +export const authUnreachable = (action: AuthAction): string => GENERIC_MESSAGES[action].broken + +/** + * Read a failed `auth` `/connect/token` response into something worth showing. + * + * auth answers the OAuth shape — `{ error: 'invalid_grant', error_description: … }` — + * where `error` is one of three machine codes and the DESCRIPTION carries the actual + * reason. Relaying that body verbatim put "invalid_grant" on screen for every failure, + * including the ones a player can act on (the per-network signup cap), so the + * description is matched to a sentence here instead. An unrecognised body — or a + * non-JSON one from something in front of auth — falls back to the generic line for + * the action rather than leaking whatever it did say. + */ +export async function readAuthError(res: Response, action: AuthAction): Promise { + const parsed = (await res.json().catch(() => null)) as { + error?: unknown + error_description?: unknown + } | null + const body = parsed ?? {} + const code = typeof body.error === 'string' ? body.error : '' + const description = typeof body.error_description === 'string' ? body.error_description : '' + + // A 5xx (or a `server_error`) is an operator misconfiguration — an unset JWT_SECRET, + // an unset META_APP_SECRET — not something the player got wrong. Don't send them back + // to re-check a form that was fine; the real reason is in auth's log, not theirs. + const broken = res.status >= 500 || code === 'server_error' + const generic = GENERIC_MESSAGES[action] + + return { + message: + (!broken && AUTH_MESSAGES[description]) || (broken ? generic.broken : generic.rejected), + status: broken ? 502 : 400, + upstream: description ? `${code || 'unknown'}: ${description}` : code || `HTTP ${res.status}`, + } +} diff --git a/apps/www/src/www.app.ts b/apps/www/src/www.app.ts index 2d25b7e..1a0766d 100644 --- a/apps/www/src/www.app.ts +++ b/apps/www/src/www.app.ts @@ -8,11 +8,21 @@ import { NotificationType } from '../../notify/src/notification-types' import { docsPage, fetchSpec } from './docs' import { privacyPage } from './privacy' import { turnstileKeys, verifyTurnstile } from './turnstile' -import { accountsBase, apiBase, authBase, imgBase, notifyBase, postForm } from './upstream' +import { + accountsBase, + apiBase, + authBase, + authUnreachable, + imgBase, + notifyBase, + postForm, + readAuthError, +} from './upstream' import type { Context } from 'hono' import type { CookieOptions } from 'hono/utils/cookie' import type { App } from './context' +import type { AuthAction } from './upstream' /** * www — the first frontend worker. It serves the React SPA (create account, set @@ -92,13 +102,32 @@ async function relay(c: Context, res: Response) { * `email`, when given, is saved onto the new account before that fetch, so the account * comes back already carrying it. `create_account` takes no email — the accounts worker * owns that field — which is why this is a second call rather than another grant field. + * + * A refused grant is translated (see `readAuthError`) rather than relayed: auth answers + * the OAuth shape, whose `error` is always a code like `invalid_grant`, and that code is + * what the form used to show for every failure — including the per-network signup cap, + * which the player could otherwise understand. The raw pair is logged for the operator. */ -async function establishSession(c: Context, tokenResponse: Response, email?: string) { - if (!tokenResponse.ok) return relay(c, tokenResponse) +async function establishSession( + c: Context, + action: AuthAction, + tokenResponse: Response, + email?: string +) { + if (!tokenResponse.ok) { + const failure = await readAuthError(tokenResponse, action) + logger.info('auth refused a token grant', { + action, + status: tokenResponse.status, + upstream: failure.upstream, + }) + return c.json({ error: failure.message }, failure.status) + } const token = (await tokenResponse.json()) as { access_token?: string; expires_in?: number } if (!token.access_token) { - return c.json({ error: 'auth did not return an access token' }, 502) + logger.error('auth answered a token grant with no access_token', { action }) + return c.json({ error: authUnreachable(action) }, 502) } setCookie( @@ -130,7 +159,21 @@ async function establishSession(c: Context, tokenResponse: Response, email? const me = await fetch(`${accountsBase(c.env)}/account/me`, { headers: { authorization: `Bearer ${token.access_token}` }, }) - if (!me.ok) return c.json({ error: 'failed to load account after auth' }, 502) + // The session cookie is already set, so this is the one failure where telling them to + // retry would be wrong: on signup the account exists (and a second attempt spends + // another slot against auth's per-IP cap), and either way a reload finds them signed in. + if (!me.ok) { + logger.error('failed to load the account after a token grant', { action, status: me.status }) + return c.json( + { + error: + action === 'signup' + ? 'Your account was created, but loading it failed. Reload the page — you are already signed in.' + : 'You are signed in, but loading your account failed. Please reload the page.', + }, + 502 + ) + } const account = (await me.json()) as Record return c.json({ account: { ...account, isAdmin: isAdminToken(token.access_token) } }) } @@ -201,11 +244,19 @@ const app = new Hono() // A token is single-use, so the client resets its widget before letting them retry. if (!verified) return c.json({ error: 'Bot check failed. Please try again.' }, 403) + // A throw here is auth being unreachable, not a rejected signup — answered as such + // rather than falling through to the generic 500 handler, whose "internal server + // error" tells the player nothing about whether they now have an account (they don't: + // nothing was created). const res = await postForm(`${authBase(c.env)}/connect/token`, { grant_type: 'create_account', password, - }) - return establishSession(c, res, signupEmail || undefined) + }).catch(() => null) + if (res === null) { + logger.error('could not reach auth to create an account') + return c.json({ error: authUnreachable('signup') }, 502) + } + return establishSession(c, 'signup', res, signupEmail || undefined) }) // Log in with a username + password, then start a session. The auth password grant @@ -224,8 +275,12 @@ const app = new Hono() username, platform: WEB_PLATFORM, password, - }) - return establishSession(c, res) + }).catch(() => null) + if (res === null) { + logger.error('could not reach auth to sign in') + return c.json({ error: authUnreachable('login') }, 502) + } + return establishSession(c, 'login', res) }) // Clear the session cookie.