diff --git a/apps/www/src/client/App.tsx b/apps/www/src/client/App.tsx index 83ec386..48dc469 100644 --- a/apps/www/src/client/App.tsx +++ b/apps/www/src/client/App.tsx @@ -16,6 +16,12 @@ interface SelfAccount { username: string displayName: string email: string | null + /** + * Username changes left on the account — each change spends one, and an account + * starts with one. Absent on an older self DTO, which reads as "unknown": the form + * stays usable and lets the server be the one to refuse. + */ + availableUsernameChanges?: number /** Whether this session may use admin controls (from the token's role claim). */ isAdmin?: boolean } @@ -806,6 +812,11 @@ function Dashboard({ // The dashboard sections, shown one at a time via the left tab rail. Admin-only // sections are appended when the session carries an admin role. const sections = [ + { + id: 'username', + label: 'Username', + render: () => , + }, { id: 'email', label: 'Email', @@ -936,6 +947,78 @@ function MaintenanceForm() { ) } +/** + * Change the account's username — the name used to sign in, here and in the game. + * + * Changes are rationed (an account starts with one), so the count is stated up front and + * the form locks itself once none are left rather than letting someone spend the attempt + * finding out. The server is still the one that decides: an unknown count leaves the form + * open, and a name taken since the page loaded is refused upstream. + * + * The response is the caller's whole self account, re-read after the write, so the + * remaining count on screen is the stored one and not a guess. + */ +function UsernameForm({ + account, + onChange, +}: { + account: SelfAccount + onChange: (a: SelfAccount) => void +}) { + const [username, setUsername] = useState(account.username) + const { pending, error, done, run } = useAction() + + const remaining = account.availableUsernameChanges + const spent = remaining !== undefined && remaining <= 0 + // Retyping the current name would be refused upstream anyway ("already taken" is + // waived for your own name, but it would still spend a change). + const unchanged = username.trim() === account.username + + return ( +
+

Username

+

+ What you sign in with, here and in the game — and what other players see you by. +

+
{ + e.preventDefault() + void run(async () => { + const updated = await api('/api/username', { username }) + onChange(updated) + setUsername(updated.username) + return `You are now @${updated.username}.` + }) + }} + > + + {error &&

{error}

} + {done &&

{done}

} + + +
+ ) +} + function EmailForm({ account, onChange, diff --git a/apps/www/src/context.ts b/apps/www/src/context.ts index 3bb45b0..a43d885 100644 --- a/apps/www/src/context.ts +++ b/apps/www/src/context.ts @@ -6,6 +6,16 @@ export type Env = SharedHonoEnv & { DOMAIN: string /** Static-asset fetcher for the built React SPA (see wrangler.jsonc `assets`). */ ASSETS: Fetcher + /** + * Service binding to the `auth` worker — how the BFF reaches it, so the browser's real + * IP survives the hop (see wrangler.jsonc and src/upstream.ts `postAuthForm`). + * + * OPTIONAL because a deployed www always has it (it's declared in wrangler.jsonc) but + * standalone local dev doesn't: `vite dev` runs www on its own against a deployed + * DOMAIN, with no `auth` session to bind to. Absent, `postAuthForm` falls back to + * fetching auth. — the pre-binding behaviour, correct except for the IP. + */ + AUTH?: Fetcher /** * The Turnstile widget's public site key. Public by design — it ships to the browser so * the widget can render — but it lives in the Secrets Store beside its secret, so one diff --git a/apps/www/src/test/integration/api.test.ts b/apps/www/src/test/integration/api.test.ts index 70a6225..f79d665 100644 --- a/apps/www/src/test/integration/api.test.ts +++ b/apps/www/src/test/integration/api.test.ts @@ -4,7 +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 { postAuthForm, readAuthError } from '../../upstream' import type { Env } from '../../context' @@ -163,6 +163,52 @@ it('explains a refused signup instead of relaying invalid_grant', async () => { expect(html.upstream).toBe('HTTP 502') }) +// The signup cap counts auth's `CF-Connecting-IP` as the account's immutable `signupIp`, +// and www used to reach auth over https://auth. — a Worker subrequest, which +// re-enters the Cloudflare edge, which REPLACES that header with Cloudflare's own +// address. Every browser signup therefore shared one IP, and the cap (3, never decaying) +// refused the fourth web account ever created, for everyone. The service binding skips +// the edge, so the header set here is the one auth reads. +// +// Checked directly rather than through /api/signup: the pass path would call Cloudflare's +// siteverify for real (see the Turnstile tests above). +it('carries the browser IP across to auth instead of losing it to the edge', async () => { + const seen: Request[] = [] + const withAuth = (fetcher?: Fetcher) => + ({ + DOMAIN: 'rec.example.com', + AUTH: fetcher, + }) as unknown as Env + const capture = { + fetch: async (request: Request) => { + seen.push(request) + return new Response('{}', { headers: { 'content-type': 'application/json' } }) + }, + } as unknown as Fetcher + + await postAuthForm( + withAuth(capture), + '/connect/token', + { grant_type: 'create_account', password: 'hunter2' }, + { clientIp: '203.0.113.7' } + ) + + // The binding is used in preference to the hostname, and the real IP rides along. + expect(seen).toHaveLength(1) + expect(seen[0]!.headers.get('cf-connecting-ip')).toBe('203.0.113.7') + // Still the same host/path/body auth already answers — only the transport changed. + expect(seen[0]!.url).toBe('https://auth.rec.example.com/connect/token') + const body = await seen[0]!.formData() + expect(body.get('grant_type')).toBe('create_account') + expect(body.get('password')).toBe('hunter2') + + // A call with no IP to forward must not invent one: an absent header leaves auth's + // own `clientIp` empty, which SKIPS the cap, rather than counting everyone together. + await postAuthForm(withAuth(capture), '/account/me/changepassword', {}, { bearer: 'tok' }) + expect(seen[1]!.headers.get('cf-connecting-ip')).toBeNull() + expect(seen[1]!.headers.get('authorization')).toBe('Bearer tok') +}) + it('requires credentials to log in', async () => { const res = await SELF.fetch('https://example.com/api/login', { method: 'POST', @@ -173,6 +219,33 @@ it('requires credentials to log in', async () => { expect(await res.json()).toEqual({ error: 'Username and password are required.' }) }) +it('rejects an unauthenticated username change', async () => { + const res = await SELF.fetch('https://example.com/api/username', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ username: 'newname' }), + }) + expect(res.status).toBe(401) + expect(await res.json()).toEqual({ error: 'not signed in' }) +}) + +// The accounts worker answers a refused username change at HTTP 200, in its own +// `{ success, error, value }` envelope — so an empty name reaching it would come back +// looking like a success to a browser that keys off the status. Rejected here instead, +// before the upstream call, and in the `{ error }` + 4xx shape every other endpoint uses. +it('refuses a username change with no username', async () => { + const res = await SELF.fetch('https://example.com/api/username', { + method: 'POST', + // Any cookie value gets past the session check — www holds no signing key and + // only forwards the token, so this stops at the empty-name check without ever + // reaching accounts. + headers: { 'content-type': 'application/json', cookie: 'rf_token=stub' }, + body: JSON.stringify({ username: ' ' }), + }) + expect(res.status).toBe(400) + expect(await res.json()).toEqual({ error: 'A username is required.' }) +}) + it('rejects an unauthenticated maintenance broadcast', async () => { const res = await SELF.fetch('https://example.com/api/maintenance', { method: 'POST', diff --git a/apps/www/src/upstream.ts b/apps/www/src/upstream.ts index 2ec8a42..d0a23c8 100644 --- a/apps/www/src/upstream.ts +++ b/apps/www/src/upstream.ts @@ -15,11 +15,12 @@ export const apiBase = (env: Env): string => `https://api.${env.DOMAIN}` export const imgBase = (env: Env): string => `https://img.${env.DOMAIN}` /** - * POST a form-urlencoded body to an upstream worker. The auth/accounts endpoints + * Send a form-urlencoded body to an upstream worker. The auth/accounts endpoints * read their inputs via Hono's `parseBody()`, so they expect form fields (not * JSON). `bearer`, when given, authenticates the caller. */ -export async function postForm( +async function sendForm( + method: 'POST' | 'PUT', url: string, fields: Record, bearer?: string @@ -29,10 +30,70 @@ export async function postForm( } if (bearer) headers.authorization = `Bearer ${bearer}` return fetch(url, { + method, + headers, + body: new URLSearchParams(fields).toString(), + }) +} + +/** POST a form-urlencoded body to an upstream worker (see `sendForm`). */ +export const postForm = ( + url: string, + fields: Record, + bearer?: string +): Promise => sendForm('POST', url, fields, bearer) + +/** + * PUT a form-urlencoded body to an upstream worker (see `sendForm`). The accounts + * worker's profile mutations are split by verb — the username change is a PUT, and + * posting to it 404s rather than failing loudly. + */ +export const putForm = ( + url: string, + fields: Record, + bearer?: string +): Promise => sendForm('PUT', url, fields, bearer) + +/** + * POST a form body to the `auth` worker, carrying the browser's real IP across. + * + * Every other upstream is reached over its public hostname, but auth can't be: it reads + * the caller's address from `CF-Connecting-IP` and counts it as the account's immutable + * `signupIp`, and a Worker subrequest to https://auth. re-enters the Cloudflare + * edge, which REPLACES that header with Cloudflare's own address. Every web signup + * therefore recorded one shared IP, and auth's per-IP cap — 3 accounts, never decaying — + * refused the fourth web account ever created, for everybody. The web path asserts no + * platform, so that arm was also the only cap actually in front of it. + * + * Going through the service binding skips the edge, so the header set here is the one + * auth reads. That is safe precisely because the edge does overwrite it on the public + * route: a game client posting `/connect/token` directly still cannot spoof its own IP, + * so no shared secret is needed to tell the two callers apart. + * + * `clientIp` is the caller's own edge-set `cf-connecting-ip`, and must never be anything + * a browser supplied. Omit it on calls that aren't IP-sensitive. + * + * Falls back to the public hostname when the binding is absent (local `vite dev` — see + * `Env.AUTH`); the edge then overwrites the header again, which is the old behaviour. + */ +export async function postAuthForm( + env: Env, + path: string, + fields: Record, + opts: { bearer?: string; clientIp?: string } = {} +): Promise { + const headers: Record = { + 'content-type': 'application/x-www-form-urlencoded', + } + if (opts.bearer) headers.authorization = `Bearer ${opts.bearer}` + if (opts.clientIp) headers['cf-connecting-ip'] = opts.clientIp + + const request = new Request(`${authBase(env)}${path}`, { method: 'POST', headers, body: new URLSearchParams(fields).toString(), }) + return env.AUTH ? env.AUTH.fetch(request) : fetch(request) } /** Which grant www was making, so a shared refusal reads right on either form. */ diff --git a/apps/www/src/www.app.ts b/apps/www/src/www.app.ts index 1a0766d..6153bbf 100644 --- a/apps/www/src/www.app.ts +++ b/apps/www/src/www.app.ts @@ -11,11 +11,12 @@ import { turnstileKeys, verifyTurnstile } from './turnstile' import { accountsBase, apiBase, - authBase, authUnreachable, imgBase, notifyBase, + postAuthForm, postForm, + putForm, readAuthError, } from './upstream' @@ -235,12 +236,10 @@ const app = new Hono() // The IP Turnstile cross-checks the token against — set by the edge, so the client // can't spoof it (unlike X-Forwarded-For). `auth` records the same header as the - // account's signup IP. - const verified = await verifyTurnstile( - keys.secretKey, - turnstileToken, - c.req.header('cf-connecting-ip') - ) + // account's signup IP, which is why it's forwarded to the grant below rather than + // left to the edge: see `postAuthForm`. + const clientIp = c.req.header('cf-connecting-ip') + const verified = await verifyTurnstile(keys.secretKey, turnstileToken, clientIp) // 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) @@ -248,10 +247,12 @@ const app = new Hono() // 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, - }).catch(() => null) + const res = await postAuthForm( + c.env, + '/connect/token', + { grant_type: 'create_account', password }, + { clientIp } + ).catch(() => null) if (res === null) { logger.error('could not reach auth to create an account') return c.json({ error: authUnreachable('signup') }, 502) @@ -270,12 +271,14 @@ const app = new Hono() return c.json({ error: 'Username and password are required.' }, 400) } - const res = await postForm(`${authBase(c.env)}/connect/token`, { - grant_type: 'password', - username, - platform: WEB_PLATFORM, - password, - }).catch(() => null) + // The IP goes along here too: auth refreshes `lastLoginIp` on every successful + // grant, and without it every web login would stamp the same Cloudflare address. + const res = await postAuthForm( + c.env, + '/connect/token', + { grant_type: 'password', username, platform: WEB_PLATFORM, password }, + { clientIp: c.req.header('cf-connecting-ip') } + ).catch(() => null) if (res === null) { logger.error('could not reach auth to sign in') return c.json({ error: authUnreachable('login') }, 502) @@ -327,6 +330,63 @@ const app = new Hono() return c.json({ ...account, isAdmin: isAdminToken(token) }) }) + // Change the signed-in account's username. + // + // The accounts worker answers this one in its own envelope — `{ success, error, value }` + // at HTTP 200 even when it refused (taken name, no changes left) — so relaying it + // verbatim would read as a success to the browser, which keys off the status. It's + // translated to the same `{ error }` + 4xx shape as every other endpoint here instead; + // the sentences accounts writes are already player-facing, so they pass through as-is. + // + // On success the caller's SELF account is re-fetched rather than returning the + // envelope's `value`: that's the PUBLIC DTO, and it carries no + // `availableUsernameChanges` — the very field the form needs to know whether another + // change is left. (An account starts with one; it's spent by this call.) + .post('/api/username', async (c) => { + const token = sessionToken(c) + if (!token) return c.json({ error: 'not signed in' }, 401) + + const { username } = await c.req + .json<{ username?: string }>() + .catch(() => ({}) as { username?: string }) + const wanted = typeof username === 'string' ? username.trim() : '' + if (wanted === '') return c.json({ error: 'A username is required.' }, 400) + + const res = await putForm( + `${accountsBase(c.env)}/account/me/username`, + { username: wanted }, + token + ) + if (!res.ok) return relay(c, res) + + const result = (await res.json().catch(() => null)) as { + success?: boolean + error?: unknown + } | null + if (!result) { + logger.error('accounts answered a username change with a body that was not JSON') + return c.json({ error: 'Your username could not be changed. Please try again later.' }, 502) + } + const refusal = typeof result.error === 'string' ? result.error : '' + if (refusal !== '') return c.json({ error: refusal }, 400) + + const me = await fetch(`${accountsBase(c.env)}/account/me`, { + headers: { authorization: `Bearer ${token}` }, + }) + // The name has already changed by now, so this can't be reported as a failure — + // telling them to retry would spend the change they no longer have. A reload shows + // the new name either way. + if (!me.ok) { + logger.error('failed to reload the account after a username change', { status: me.status }) + return c.json( + { error: 'Your username was changed, but reloading your account failed. Reload the page.' }, + 502 + ) + } + const account = (await me.json()) as Record + return c.json({ ...account, isAdmin: isAdminToken(token) }) + }) + // Set the signed-in account's email. .post('/api/email', async (c) => { const token = sessionToken(c) @@ -349,10 +409,13 @@ const app = new Hono() .catch(() => ({}) as { oldPassword?: string; newPassword?: string }) if (!newPassword) return c.json({ error: 'A new password is required.' }, 400) - const res = await postForm( - `${authBase(c.env)}/account/me/changepassword`, + // No `clientIp`: auth reads no IP on this path — it's routed through the binding + // only so every auth call goes one way. + const res = await postAuthForm( + c.env, + '/account/me/changepassword', { oldPassword: oldPassword ?? '', newPassword }, - token + { bearer: token } ) return relay(c, res) }) diff --git a/apps/www/vitest.config.ts b/apps/www/vitest.config.ts index 08a9356..187e856 100644 --- a/apps/www/vitest.config.ts +++ b/apps/www/vitest.config.ts @@ -6,6 +6,22 @@ export default defineConfig({ cloudflareTest({ wrangler: { configPath: `${__dirname}/wrangler.jsonc` }, miniflare: { + // Stands in for the `auth` service binding wrangler.jsonc declares — the real + // worker isn't part of this project's test run, and without an override the + // runtime refuses to start ("no such service is defined"). It echoes the + // forwarded `cf-connecting-ip` back so a test can assert the browser's IP + // actually survives the hop (see src/upstream.ts `postAuthForm`); every other + // auth call in the tests fails before reaching it. + serviceBindings: { + AUTH: (request: Request) => + new Response( + JSON.stringify({ + error: 'invalid_grant', + error_description: request.headers.get('cf-connecting-ip') ?? 'no ip', + }), + { status: 400, headers: { 'content-type': 'application/json' } } + ), + }, bindings: { ENVIRONMENT: 'VITEST', // The Turnstile keypair is NOT bound here: both keys come from the Secrets diff --git a/apps/www/wrangler.jsonc b/apps/www/wrangler.jsonc index 6a0e501..a6823ef 100644 --- a/apps/www/wrangler.jsonc +++ b/apps/www/wrangler.jsonc @@ -54,6 +54,17 @@ "secret_name": "TURNSTILE_SECRET_KEY" } ], + // The `auth` worker, reached directly instead of over its public hostname. This is + // about the CLIENT IP, not latency: a Worker subrequest to https://auth. + // re-enters the Cloudflare edge, which overwrites CF-Connecting-IP with Cloudflare's + // own address — so auth recorded the SAME `signupIp` for every browser signup and its + // per-IP cap (3 by default) locked out every player after the third account ever + // created. A service binding skips the edge, so the real browser IP www forwards on + // that header survives (see src/upstream.ts `postAuthForm`). + // + // Only auth is bound: it's the only upstream whose behaviour depends on the caller's + // IP. accounts/api/img/notify still go over their public hostnames. + "services": [{ "binding": "AUTH", "service": "auth" }], "upload_source_maps": true, "observability": { "logs": {