diff --git a/apps/auth/migrations/0010_platform_account_role.sql b/apps/auth/migrations/0010_platform_account_role.sql new file mode 100644 index 0000000..8fff15b --- /dev/null +++ b/apps/auth/migrations/0010_platform_account_role.sql @@ -0,0 +1,37 @@ +-- Record the roles a linked identity held in its platform's community, for the one +-- platform that has any: `PlatformType.Discord` (101), written by the website's benefits +-- claim from the member record Discord itself served +-- (`GET /users/@me/guilds/{guild}/member`). Steam and Meta links leave it NULL — there is +-- no such thing there — so the column is nullable and has no default. +-- +-- Until now the claim asked Discord for the member's roles, tested them against the +-- configured qualifying list, and threw the answer away; all that survived was `hasPlus`, +-- which records "held A qualifying role once" and not WHICH one. That is the question an +-- operator has to answer by hand in Discord for every player who writes in — and the only +-- way to tell a supporter from a booster from staff after the fact, or to see that a tier +-- was granted under a role that has since been retired. +-- +-- A JSON ARRAY of role id snowflakes (`["1077…001","1077…002"]`), not one id: a member +-- holds as many roles as they hold, and the claim qualifies on ANY of them, so storing a +-- single one would have to pick arbitrarily. Snowflakes stay STRINGS — one exceeds 2^53, +-- so a JSON number would round and stop matching the real role. The list is the whole +-- member record's roles, not just the qualifying ones: it is a snapshot of who they were +-- in the guild, and filtering it to the config of the day would make it unreadable the +-- moment the config changed. +-- +-- It is a SNAPSHOT, taken at claim time and refreshed only when the same player claims +-- again. Nothing sweeps it: reading a member's roles needs that member's own OAuth token +-- (this design deliberately holds no bot token — see apps/www/src/discord.ts), so the +-- server cannot re-read them unprompted. Read it as "the roles they held when they +-- claimed", the same tense `hasPlus` carries, and never as "the roles they hold". +-- +-- No index. Nothing queries BY role yet; when something does, it will want +-- `json_each(role)` and an expression index built for that query, not a plain one over +-- the array text, which could only serve an exact-whole-array match. Kept in sync with +-- PLATFORM_SCHEMA_DDL in src/platform-db.ts. +-- +-- Safe to run before or after the deploy that ships it: an added nullable column is +-- invisible to the currently-deployed code, which names its columns explicitly and never +-- SELECTs *. + +ALTER TABLE platform_account ADD COLUMN role TEXT; diff --git a/apps/auth/src/platform-db.ts b/apps/auth/src/platform-db.ts index 48f3ab2..527b97f 100644 --- a/apps/auth/src/platform-db.ts +++ b/apps/auth/src/platform-db.ts @@ -27,17 +27,27 @@ * `verifyPlatformProof` answers `unsupported` for anything but Steam and Meta, so a * `cached_login` naming platform 101 is refused — and the picker must not offer it * either. See {@link CACHED_LOGIN_PLATFORMS}, which is what keeps those two in step. + * + * That entitlement is also the one thing a link carries beyond the identity itself: the + * `role` column (migration 0010) records the Discord roles the claim read, so what a + * player was granted Plus FOR survives the claim instead of collapsing into a bare + * `hasPlus`. See {@link PlatformLink.roles}. */ import { PlatformType } from '@repo/domain/src/enums' -/** Schema DDL (mirror of migrations/0007_platform_accounts.sql, sans the backfill). */ +/** + * Schema DDL (mirror of migrations/0007_platform_accounts.sql, sans the backfill, plus + * 0010's `role` column — which is an ALTER on prod and a plain column here, since this + * builds the table from nothing). + */ export const PLATFORM_SCHEMA_DDL: string[] = [ `CREATE TABLE IF NOT EXISTS platform_account ( account_id INTEGER NOT NULL, platform INTEGER NOT NULL, platform_id TEXT NOT NULL, linked_at TEXT NOT NULL, + role TEXT, PRIMARY KEY (platform, platform_id, account_id) )`, // The picker's lookup: "which accounts does this identity open?". Covered by the @@ -113,6 +123,17 @@ export interface PlatformLink { platformId: string /** ISO-8601 time the link was made. */ linkedAt: string + /** + * The roles this identity held in its platform's community when the link was last + * written — Discord role id snowflakes, kept as strings (one exceeds 2^53). Empty for + * every platform that has no such thing, which is all of them but Discord. + * + * A SNAPSHOT from the claim, not a live read: nothing here can re-read a member's roles + * without their own OAuth token (see apps/www/src/discord.ts, which deliberately holds + * no bot token), so this says what they held when they claimed. Stored as a JSON array + * in the `role` column (migration 0010). + */ + roles: string[] } interface LinkRow { @@ -120,10 +141,39 @@ interface LinkRow { platform: number platformId: string linkedAt: string + /** The `role` column: a JSON array of role ids, or NULL for a link that has none. */ + role: string | null } const SELECT_LINK = `SELECT account_id AS accountId, platform, platform_id AS platformId, - linked_at AS linkedAt FROM platform_account` + linked_at AS linkedAt, role FROM platform_account` + +/** + * A stored `role` array back into role ids. Tolerant by design — every failure mode is + * "this link has no roles recorded", which is the truth for every Steam and Meta link and + * the honest answer for a row written before 0010. A NULL, a value that isn't JSON, or a + * JSON value that isn't an array of strings all land there rather than throwing: the roles + * are a record, and no caller's correctness depends on them, so a bad one must not take + * down the login picker that reads the same rows. + */ +const parseRoles = (raw: string | null): string[] => { + if (raw === null || raw === '') return [] + try { + const parsed: unknown = JSON.parse(raw) + return Array.isArray(parsed) ? parsed.filter((r): r is string => typeof r === 'string') : [] + } catch { + return [] + } +} + +/** One row as the rest of the codebase sees it. */ +const toLink = (row: LinkRow): PlatformLink => ({ + accountId: row.accountId, + platform: row.platform, + platformId: row.platformId, + linkedAt: row.linkedAt, + roles: parseRoles(row.role), +}) /** * Link a verified platform identity to an account. Idempotent — re-logging in on the @@ -132,22 +182,49 @@ const SELECT_LINK = `SELECT account_id AS accountId, platform, platform_id AS pl * * Callers must pass an identity the platform itself proved. Nothing in here can tell * a verified id from a spoofed one. + * + * `roles` records what that identity held in its platform's community — Discord role ids, + * from the benefits claim's member read. Omitting it (every login path: Steam and Meta + * have no such thing) leaves the column NULL rather than writing an empty array, so + * "never had roles" stays distinguishable in the table from "claimed holding none". An + * EMPTY array is a real value and is stored as one. + * + * Unlike the rest of the row, roles are REFRESHED on a repeat link: they're a snapshot of + * a membership that moves, and the later claim is the fresher reading. That is also why + * the update is separate from the insert rather than an upsert — the insert's + * `meta.changes` is what tells a new link from an existing one, and an + * `ON CONFLICT DO UPDATE` reports a change either way, which would make every re-login + * look like a first link to {@link linkLoginIdentity}'s primary-identity write. The second + * statement only runs on the Discord path, and only when the row was already there. */ export async function linkPlatformIdentity( db: D1Database, accountId: number, platform: number, - platformId: string + platformId: string, + roles?: readonly string[] ): Promise { if (platformId === '') return false + const role = roles === undefined ? null : JSON.stringify([...roles]) const res = await db .prepare( - `INSERT OR IGNORE INTO platform_account (account_id, platform, platform_id, linked_at) - VALUES (?1, ?2, ?3, ?4)` + `INSERT OR IGNORE INTO platform_account (account_id, platform, platform_id, linked_at, role) + VALUES (?1, ?2, ?3, ?4, ?5)` ) - .bind(accountId, platform, platformId, new Date().toISOString()) + .bind(accountId, platform, platformId, new Date().toISOString(), role) .run() - return res.meta.changes > 0 + if (res.meta.changes > 0) return true + + if (role !== null) { + await db + .prepare( + `UPDATE platform_account SET role = ?4 + WHERE account_id = ?1 AND platform = ?2 AND platform_id = ?3` + ) + .bind(accountId, platform, platformId, role) + .run() + } + return false } /** @@ -170,7 +247,7 @@ export async function getLinksForPlatformIdentity( ) .bind(platform, platformId) .all() - return results + return results.map(toLink) } /** @@ -193,7 +270,7 @@ export async function getLinksForPlatformId( ) .bind(platformId, ...CACHED_LOGIN_PLATFORMS) .all() - return results + return results.map(toLink) } /** Every platform identity linked to an account (a player's PC and headset, say). */ @@ -205,7 +282,7 @@ export async function getLinksForAccount( .prepare(`${SELECT_LINK} WHERE account_id = ?1 ORDER BY linked_at, platform`) .bind(accountId) .all() - return results + return results.map(toLink) } /** diff --git a/apps/auth/src/test/integration/api.test.ts b/apps/auth/src/test/integration/api.test.ts index ee87ef2..4226ed1 100644 --- a/apps/auth/src/test/integration/api.test.ts +++ b/apps/auth/src/test/integration/api.test.ts @@ -473,6 +473,60 @@ describe('auth worker routes', () => { ).resolves.toBe(1) }) + // The `role` column (migration 0010): what the benefits claim read about a member, kept + // so "why does this account have Plus" is answerable later. Exercised through the helper + // rather than the claim route, which runs through discord.com. + test('records the roles a claimed Discord identity held, and refreshes them on a re-claim', async () => { + const discordId = '308994132968210434' + const supporter = '1077000000000000001' + const booster = '1077000000000000002' + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 31400, username: 'RoleClaimer', hasPlus: true })) + .run() + + await linkPlatformIdentity(env.DB, 31400, PlatformType.Discord, discordId, [supporter]) + const first = await getLinksForAccount(env.DB, 31400) + // Snowflakes come back as STRINGS — parsed as numbers they'd round past 2^53 and stop + // matching the real role. + expect(first.map((l) => l.roles)).toEqual([[supporter]]) + + // A re-claim is the same row (INSERT OR IGNORE, so `linkedAt` is the first claim's) + // with a FRESH role list: a membership moves, and this reading is the later one. + await linkPlatformIdentity(env.DB, 31400, PlatformType.Discord, discordId, [supporter, booster]) + const second = await getLinksForAccount(env.DB, 31400) + expect(second).toHaveLength(1) + expect(second[0]?.roles).toEqual([supporter, booster]) + expect(second[0]?.linkedAt).toBe(first[0]?.linkedAt) + + // Losing every role still stores the reading. Empty is a fact about the claim, not an + // absence of one — and it is what distinguishes this from a Steam link, below. + await linkPlatformIdentity(env.DB, 31400, PlatformType.Discord, discordId, []) + expect((await getLinksForAccount(env.DB, 31400))[0]?.roles).toEqual([]) + const stored = await env.DB.prepare( + 'SELECT role FROM platform_account WHERE account_id = 31400' + ).first<{ role: string | null }>() + expect(stored?.role).toBe('[]') + }) + + // Every login path omits the argument, because Steam and Meta have no such thing. The + // column stays NULL there rather than becoming `[]`, so the table can still tell "never + // had roles" from "claimed holding none". + test('leaves the role column null for a link with no roles to record', async () => { + const steamId = '76561197962463299' + await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') + .bind(JSON.stringify({ accountId: 31401, username: 'NoRoles' })) + .run() + await linkPlatformIdentity(env.DB, 31401, PlatformType.Steam, steamId) + + const row = await env.DB.prepare( + 'SELECT role FROM platform_account WHERE account_id = 31401' + ).first<{ role: string | null }>() + expect(row?.role).toBeNull() + // A NULL reads back as no roles rather than throwing — which is also how a row written + // before 0010 reads. + expect((await getLinksForAccount(env.DB, 31401))[0]?.roles).toEqual([]) + }) + // The 20250424.01 build POSTs the picker lookup with a platform-attestation form body // instead of GETting it. Nothing reads that body yet, so both methods must answer the // same list — otherwise the newer client's login screen comes up empty. diff --git a/apps/www/src/www.app.ts b/apps/www/src/www.app.ts index c949800..a6f0cd9 100644 --- a/apps/www/src/www.app.ts +++ b/apps/www/src/www.app.ts @@ -429,7 +429,21 @@ const app = new Hono() // Both writes are idempotent: the link is INSERT OR IGNORE (so `linkedAt` keeps the // FIRST claim's time), and the flag is already true on a re-claim. - await linkPlatformIdentity(c.env.DB, accountId, PlatformType.Discord, membership.userId) + // + // The member's roles ride along onto the link's `role` column — the WHOLE list Discord + // served, not just the ones that qualified. `hasPlus` records only that a qualifying + // role was held; this records which, so an operator can tell a supporter from a + // booster from staff without going and asking Discord, and can still read a grant made + // under a role that has since been retired from the config. Roles are the one part of + // the row a re-claim REFRESHES, because they're a snapshot of a membership that moves + // and this reading is the fresher one. + await linkPlatformIdentity( + c.env.DB, + accountId, + PlatformType.Discord, + membership.userId, + membership.roles + ) await updateAccount(c.env.DB, accountId, { hasPlus: true }) logger.info('granted plus from a discord benefits claim', { accountId })