From 6bf5d218dd441a08f422b65b6a1fc4d3ce630df7 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Wed, 16 Sep 2026 13:39:06 -0400 Subject: [PATCH] [auth] wire up new developer/moderator endpoint with proper response --- apps/auth/src/auth.app.ts | 81 +++++++++++++++++----- apps/auth/src/openapi.ts | 43 +++++++++++- apps/auth/src/test/integration/api.test.ts | 76 ++++++++++++++++++++ 3 files changed, 183 insertions(+), 17 deletions(-) diff --git a/apps/auth/src/auth.app.ts b/apps/auth/src/auth.app.ts index 7f48b1e..7901483 100644 --- a/apps/auth/src/auth.app.ts +++ b/apps/auth/src/auth.app.ts @@ -47,6 +47,7 @@ import { PlatformIdsRequest, PlatformType, RestrictionDto, + roleFilter, roleLookup, TokenRequest, TokenResponse, @@ -200,6 +201,51 @@ async function authedId(c: Context): Promise { return validateAndGetAccountId(c.req.raw, await c.env.JWT_SECRET.get()) } +/** Whether one account row holds a role. Absent/false both mean no role. */ +function holdsRole(account: Account, role: 'developer' | 'moderator'): boolean { + return (role === 'developer' ? account.isDeveloper : account.isModerator) === true +} + +/** + * The ONE-player lookup (`/role/{role}/{id}`): a BARE JSON boolean — the client reads the body + * as a bool, not as an object — or an empty 404 for a player this server has no account for, + * mirroring the reference API. An id that is not a number is answered as unknown (404) rather + * than as a 400: `/role/developer/nonsense` is just an id that matches nothing. + */ +async function roleAnswer(c: Context, role: 'developer' | 'moderator', id: string) { + logger.info(`${role} role lookup`, { id }) + const accountId = Number.parseInt(id, 10) + const account = Number.isNaN(accountId) ? null : await getAccount(c.env.DB, accountId) + if (!account) return c.body(null, 404) + return c.json(holdsRole(account, role)) +} + +/** + * The BULK form (`/role/{role}?id=1&id=2`): WHICH of the players asked about hold the role, as + * an array of ints. A different question from the single lookup and so a different shape — + * never that route's boolean. + * + * A FILTER, so nothing here is an error: an id that names no account, or names one without the + * role, is left out, and a query naming no ids answers `[]`. A 404 would be wrong — the + * question "which of these are developers" has the answer "none of them", not "no such thing". + * + * Answered in the order the query asked, de-duplicated, so a caller can read the result + * against its own list. One round trip however many ids are named: `getAccountsByIds` splits + * the read across statements when it exceeds D1's bound-parameter limit. + */ +async function roleFilterAnswer(c: Context, role: 'developer' | 'moderator') { + // `queries` and not `query`: the ids arrive as a REPEATED parameter, and `query` would + // collapse them to the first and silently answer about one player out of the set. + const asked = c.req.queries('id') ?? [] + const ids = [...new Set(asked.map((v) => Number.parseInt(v, 10)).filter(Number.isInteger))] + logger.info(`${role} role filter`, { asked: asked.length, ids: ids.length }) + if (ids.length === 0) return c.json([]) + + const accounts = await getAccountsByIds(c.env.DB, ids) + const holders = new Set(accounts.filter((a) => holdsRole(a, role)).map((a) => a.accountId)) + return c.json(ids.filter((id) => holders.has(id))) +} + /** * The role names beyond `gameClient` for an account's token `role` claim. Base roles * (gameClient) are added by generateToken. `screenshare` rides on EVERY token — the @@ -1172,26 +1218,29 @@ const app = new Hono() // is off by default and only an operator grants it (via `runx admin grant-developer`, // which sets the account's isDeveloper flag); it also rides in the token's `role` // claim (see accountRoles). - .get('/role/developer/:id', describeRoute(roleLookup('developer')), async (c) => { - const { id } = c.req.param() - logger.info('developer role lookup', { id }) - const accountId = Number.parseInt(id, 10) - const account = Number.isNaN(accountId) ? null : await getAccount(c.env.DB, accountId) - if (!account) return c.body(null, 404) - return c.json(account.isDeveloper === true) - }) + .get('/role/developer/:id', describeRoute(roleLookup('developer')), (c) => + roleAnswer(c, 'developer', c.req.param('id')) + ) + + // The BULK form — `/role/developer?id=2&id=3` answers WHICH of those players are developers, + // as an array of ints. A route of its own rather than an optional `:id?` segment, and a + // different answer shape from the single lookup above: both are kept because the client asks + // both ways (the path form is the 2023 one). + .get('/role/developer', describeRoute(roleFilter('developer')), (c) => + roleFilterAnswer(c, 'developer') + ) // Moderator role lookup, mirroring developer (bare boolean, 404 for unknown player). // Operator-granted only (via `runx admin grant-moderator`); the flag also rides in // the token's `role` claim. - .get('/role/moderator/:id', describeRoute(roleLookup('moderator')), async (c) => { - const { id } = c.req.param() - logger.info('moderator role lookup', { id }) - const accountId = Number.parseInt(id, 10) - const account = Number.isNaN(accountId) ? null : await getAccount(c.env.DB, accountId) - if (!account) return c.body(null, 404) - return c.json(account.isModerator === true) - }) + .get('/role/moderator/:id', describeRoute(roleLookup('moderator')), (c) => + roleAnswer(c, 'moderator', c.req.param('id')) + ) + + // Moderator's bulk form, kept in step with developer's above. + .get('/role/moderator', describeRoute(roleFilter('moderator')), (c) => + roleFilterAnswer(c, 'moderator') + ) // @guess Oculus nonce. The client asks for this before a Meta login; the exact shape // it expects hasn't been observed, so this mints a fresh 64-char hex nonce (the length diff --git a/apps/auth/src/openapi.ts b/apps/auth/src/openapi.ts index 6dded37..1531ad4 100644 --- a/apps/auth/src/openapi.ts +++ b/apps/auth/src/openapi.ts @@ -239,7 +239,9 @@ export function roleLookup(role: 'developer' | 'moderator') { description: `Returns a bare JSON boolean (\`true\`/\`false\`), not an object. Off by default and ` + `granted only by an operator via \`runx admin grant-${role}\`. The same flag also rides ` + - `in the access token's \`role\` claim, so the client rarely needs this route.`, + `in the access token's \`role\` claim, so the client rarely needs this route.\n\n` + + `This is the ONE-player form. \`/role/${role}?id=\` is a different question with a ` + + `different answer shape — which of several players hold the role, as an array.`, parameters: [ { name: 'id', @@ -256,6 +258,45 @@ export function roleLookup(role: 'developer' | 'moderator') { } } +/** + * The BULK form: `/role/{role}?id=1&id=2` answers which of those players hold the role, as an + * array of ints. Not the single lookup's boolean — a different question, so a different shape; + * the two live side by side because the client asks both ways. + */ +export function roleFilter(role: 'developer' | 'moderator') { + return { + tags: ['Roles'], + summary: `Which of these players have the ${role} role`, + description: + `The subset of the accounts named by \`?id=\` that hold the ${role} role, as an ARRAY ` + + `OF INTS — not a boolean, and not an object. Ids are repeated (\`?id=1&id=2\`), not ` + + `comma-joined.\n\n` + + `It is a FILTER, so a player who does not hold the role is simply absent from the ` + + `answer, as is an id matching no account — neither is an error, and a query naming no ` + + `ids answers \`[]\`. Results keep the order the query asked in, and a repeated id ` + + `yields at most one entry.\n\n` + + `The role is off by default and granted only by an operator via ` + + `\`runx admin grant-${role}\`.`, + parameters: [ + { + name: 'id', + in: 'query' as const, + required: false, + description: + 'An account id to test, repeated once per player. Ids that are non-numeric, unknown ' + + 'or lack the role are left out of the answer rather than erroring.', + style: 'form' as const, + explode: true, + schema: { type: 'array' as const, items: { type: 'string' as const } }, + example: ['2', '42'], + }, + ], + responses: { + 200: json(z.int().array(), `The ids, of those asked for, that hold the ${role} role`), + }, + } +} + /** Bulk cached-login lookup form body: repeated `id=` fields. */ export const PlatformIdsRequest = z.object({ id: z.union([z.string(), z.array(z.string())]).describe('Repeated `id=` form fields'), diff --git a/apps/auth/src/test/integration/api.test.ts b/apps/auth/src/test/integration/api.test.ts index dfefcdd..2f50f74 100644 --- a/apps/auth/src/test/integration/api.test.ts +++ b/apps/auth/src/test/integration/api.test.ts @@ -1396,6 +1396,80 @@ describe('auth worker routes', () => { expect((await exports.default.fetch(`${ORIGIN}/role/moderator/99999`)).status).toBe(404) }) + // `/role/{role}?id=` is a BULK FILTER — which of these players hold the role, as an array + // of ints — not the single lookup's boolean. Different question, different shape. + test('GET /role/developer?id= answers which of the players are developers', async () => { + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4242, username: 'DevPlayer', isDeveloper: true })) + .run() + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4244, username: 'DevPlayer2', isDeveloper: true })) + .run() + + // INTS, not strings, and not a boolean. + const one = await exports.default.fetch(`${ORIGIN}/role/developer?id=4242`) + expect(one.status).toBe(200) + expect(await one.json()).toEqual([4242]) + + // The subset that holds the role: 42 exists without the flag and 99999 not at all, and + // neither is an error — a filter answers "none of them", never 404. + const many = await exports.default.fetch( + `${ORIGIN}/role/developer?id=42&id=4242&id=99999&id=4244` + ) + expect(many.status).toBe(200) + expect(await many.json()).toEqual([4242, 4244]) + + // Ordered as asked, so a caller can read the answer against its own list. + const reversed = await exports.default.fetch(`${ORIGIN}/role/developer?id=4244&id=4242`) + expect(await reversed.json()).toEqual([4244, 4242]) + + // A repeated id yields at most one entry. + const repeated = await exports.default.fetch(`${ORIGIN}/role/developer?id=4242&id=4242`) + expect(await repeated.json()).toEqual([4242]) + }) + + test('GET /role/developer?id= answers [] rather than erroring', async () => { + // No ids, only unknown ids, only un-flagged ids, and a non-numeric id all answer an empty + // array with a 200 — nothing about this question is a 400 or a 404. + for (const query of ['', '?id=99999', '?id=42', '?id=nonsense', '?id=']) { + const res = await exports.default.fetch(`${ORIGIN}/role/developer${query}`) + expect(res.status, query).toBe(200) + expect(await res.json(), query).toEqual([]) + } + }) + + // The repeated parameter is the whole point: reading only the first would silently answer + // about one player out of the set. + test('GET /role/developer?id= reads every repeated id, not just the first', async () => { + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4242, username: 'DevPlayer', isDeveloper: true })) + .run() + const res = await exports.default.fetch(`${ORIGIN}/role/developer?id=42&id=4242`) + expect(await res.json()).toEqual([4242]) + }) + + test('GET /role/moderator?id= filters on the moderator flag alone', async () => { + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4343, username: 'ModPlayer', isModerator: true })) + .run() + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4242, username: 'DevPlayer', isDeveloper: true })) + .run() + // The two roles are independent flags: a developer is not implicitly a moderator. + const res = await exports.default.fetch(`${ORIGIN}/role/moderator?id=4242&id=4343&id=42`) + expect(await res.json()).toEqual([4343]) + }) + + // The single lookup keeps its own shape — the bulk form must not have changed it. + test('GET /role/developer/:id is still a bare boolean, not an array', async () => { + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 4242, username: 'DevPlayer', isDeveloper: true })) + .run() + expect(await (await exports.default.fetch(`${ORIGIN}/role/developer/4242`)).json()).toBe(true) + expect(await (await exports.default.fetch(`${ORIGIN}/role/developer/42`)).json()).toBe(false) + expect((await exports.default.fetch(`${ORIGIN}/role/developer/99999`)).status).toBe(404) + }) + test('unknown path returns 404', async () => { const res = await exports.default.fetch(`${ORIGIN}/nope`) expect(res.status).toBe(404) @@ -1426,7 +1500,9 @@ describe('auth worker routes', () => { 'GET /eac/challenge', 'GET /oculus/nonce', 'GET /privileges/me/restrictions', + 'GET /role/developer', 'GET /role/developer/{id}', + 'GET /role/moderator', 'GET /role/moderator/{id}', 'POST /account/me/changepassword', 'POST /cachedlogin/forplatformid/{platform}/{id}',