mirror of
https://github.com/djdevin/recflare.git
synced 2026-09-08 14:41:28 -07:00
fix too many accounts error because the worker was behind a Cloudflare call, add username change
This commit is contained in:
@@ -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: () => <UsernameForm account={account} onChange={onChange} />,
|
||||
},
|
||||
{
|
||||
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 (
|
||||
<section className="card">
|
||||
<h2>Username</h2>
|
||||
<p className="muted">
|
||||
What you sign in with, here and in the game — and what other players see you by.
|
||||
</p>
|
||||
<form
|
||||
onSubmit={(e) => {
|
||||
e.preventDefault()
|
||||
void run(async () => {
|
||||
const updated = await api<SelfAccount>('/api/username', { username })
|
||||
onChange(updated)
|
||||
setUsername(updated.username)
|
||||
return `You are now @${updated.username}.`
|
||||
})
|
||||
}}
|
||||
>
|
||||
<label>
|
||||
Username
|
||||
<input
|
||||
type="text"
|
||||
value={username}
|
||||
autoComplete="username"
|
||||
disabled={spent}
|
||||
onChange={(e) => setUsername(e.target.value)}
|
||||
required
|
||||
/>
|
||||
<span className="hint">
|
||||
{remaining === undefined
|
||||
? 'Changing your username uses up one of a limited number of changes.'
|
||||
: spent
|
||||
? 'You have no username changes remaining, so this can no longer be changed.'
|
||||
: `You have ${remaining} username change${remaining === 1 ? '' : 's'} remaining — this one is permanent once used.`}
|
||||
</span>
|
||||
</label>
|
||||
{error && <p className="error">{error}</p>}
|
||||
{done && <p className="ok">{done}</p>}
|
||||
<button type="submit" disabled={pending || spent || unchanged}>
|
||||
{pending ? 'Changing…' : 'Change username'}
|
||||
</button>
|
||||
</form>
|
||||
</section>
|
||||
)
|
||||
}
|
||||
|
||||
function EmailForm({
|
||||
account,
|
||||
onChange,
|
||||
|
||||
@@ -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.<DOMAIN> — 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
|
||||
|
||||
@@ -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.<DOMAIN> — 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',
|
||||
|
||||
@@ -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<string, string>,
|
||||
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<string, string>,
|
||||
bearer?: string
|
||||
): Promise<Response> => 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<string, string>,
|
||||
bearer?: string
|
||||
): Promise<Response> => 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.<DOMAIN> 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<string, string>,
|
||||
opts: { bearer?: string; clientIp?: string } = {}
|
||||
): Promise<Response> {
|
||||
const headers: Record<string, string> = {
|
||||
'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. */
|
||||
|
||||
+83
-20
@@ -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<App>()
|
||||
|
||||
// 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<App>()
|
||||
// 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<App>()
|
||||
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<App>()
|
||||
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<string, unknown>
|
||||
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<App>()
|
||||
.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)
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.<DOMAIN>
|
||||
// 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": {
|
||||
|
||||
Reference in New Issue
Block a user