diff --git a/apps/api/src/openapi.ts b/apps/api/src/openapi.ts index 70c9d7c..2c23878 100644 --- a/apps/api/src/openapi.ts +++ b/apps/api/src/openapi.ts @@ -1242,23 +1242,36 @@ export const VoteToKickReason = z.object({ }) /** - * `GET|POST /api/PlayerReporting/v1/moderationBlockDetails` — always the "not blocked" - * answer (no ban storage yet), mirroring the reference server's stub - * `ReturnModerationBlockDetails()`. `ReportCategory` is `Unknown` (-1) rather than 0, - * which is a real category, and `Message` is null — the client distinguishes "no - * message" from a blank one, so we send null where the reference sends an empty string. - * `IsVoiceModAutoban`/`TimeoutStartedAt` are on the DTO but unset by that stub, so - * they carry their C# defaults (false / null). + * `GET|POST /api/PlayerReporting/v1/moderationBlockDetails` — the caller's block. With an + * account-wide ban in force (a `report` row with `banned` set) it describes that ban: + * `IsBan` true, the report's `ReportCategory`, `Duration` in seconds left (0 for a + * permanent ban, which has no end) and a fixed `Message` of "Rule violation". Otherwise it is the "not + * blocked" answer, mirroring the reference server's stub `ReturnModerationBlockDetails()`: + * `ReportCategory` is `Unknown` (-1) rather than 0, which is a real category, and + * `Message` is null — the client distinguishes "no message" from a blank one, so we send + * null where the reference sends an empty string. `IsVoiceModAutoban`/`TimeoutStartedAt` + * are on the DTO but unset by that stub, so they carry their C# defaults (false / null). */ export const ModerationBlockDetails = z.object({ - ReportCategory: z.int().describe('-1 = ReportCategory.Unknown (0 is a real category)'), - Duration: z.int(), + ReportCategory: z + .int() + .describe( + 'The category the ban’s report was filed under; -1 = ReportCategory.Unknown when not blocked (0 is a real category)' + ), + Duration: z + .int() + .describe( + 'Seconds left on the block; 0 for a permanent ban (no end) and when not blocked — `IsBan` marks the block' + ), GameSessionId: z.int(), - IsBan: z.boolean(), + IsBan: z.boolean().describe('True when an account-wide ban is in force'), IsHostKick: z.boolean(), IsVoiceModAutoban: z.boolean(), - Message: z.string().nullable(), - PlayerIdReporter: z.int().nullable(), + Message: z.string().nullable().describe('“Rule violation” on a ban; null when not blocked'), + PlayerIdReporter: z + .int() + .nullable() + .describe('Always null — the reporter is not shown to the reported'), TimeoutStartedAt: z.string().nullable(), }) diff --git a/apps/api/src/reports-db.ts b/apps/api/src/reports-db.ts index 791a1ef..f529306 100644 --- a/apps/api/src/reports-db.ts +++ b/apps/api/src/reports-db.ts @@ -20,14 +20,14 @@ * one polymorphic id because the keys differ in TYPE: two numbers and a guid. * * A report is also where an ACCOUNT-WIDE ban lives: acting on a report sets `banned` - * on that same row (see `banFromReport`), so the ban carries the evidence for it. Two - * workers read it — `match` refuses every matchmake for a banned player, and `auth` - * refuses to issue them a token at all — both via `isPlayerBanned`. This is distinct - * from the per-room `room_ban` table the rooms worker owns: that one keeps a player - * out of ONE room, this one out of the game. - * - * `/api/PlayerReporting/v1/moderationBlockDetails` is NOT wired to it yet and still - * answers "not blocked" unconditionally. + * on that same row (see `banFromReport`), so the ban carries the evidence for it. It is + * ENFORCED by `match`, which refuses every matchmake for a banned player, and DESCRIBED + * by `/api/PlayerReporting/v1/moderationBlockDetails`, which tells the banned player why + * (via `getActiveBan`). `auth` still issues a banned account a token — that is what lets + * the client reach the block screen — and reads this table only for ban EVASION (an + * account sharing a device or network with a banned one; see bans-db). This is distinct + * from the per-room `room_ban` table the rooms worker owns: that one keeps a player out + * of ONE room, this one out of the game. */ /** @@ -199,9 +199,9 @@ export async function getActiveBan( } /** - * Whether a player is banned right now. The hot-path form of `getActiveBan` — `match` - * calls it on every matchmake and `auth` on every token grant, and neither has anything - * to say about WHICH report did it. + * Whether a player is banned right now. The hot-path form of `getActiveBan`, for a caller + * that has nothing to say about WHICH report did it. `moderationBlockDetails` is the + * caller that does, and reads `getActiveBan` itself. */ export async function isPlayerBanned( db: D1Database, diff --git a/apps/api/src/routes/moderation.ts b/apps/api/src/routes/moderation.ts index 64fe006..5eb37a0 100644 --- a/apps/api/src/routes/moderation.ts +++ b/apps/api/src/routes/moderation.ts @@ -35,12 +35,13 @@ import { VoteToKickReason, VoteToKickRequest, } from '../openapi' -import { createReport } from '../reports-db' +import { createReport, getActiveBan } from '../reports-db' import { createWarning } from '../warnings-db' import type { Context } from 'hono' import type { ModerationKickPayload } from '../../../notify/src/notification-payloads' import type { App } from '../context' +import type { ReportRow } from '../reports-db' /** * Roles allowed to hand down a warning — the operator-granted elevated roles the auth @@ -223,16 +224,76 @@ async function pushVoteToKick(c: Context, message: VoteToKickMessage): Prom } } +/** + * `Duration` on a permanent ban: 0, "no end". `IsBan` is what says the player is blocked; + * `Duration` only says for how long, and a ban with no expiry has no length to give. Not + * the int32-max sentinel E12354 uses — that reads as a 68-year countdown. + */ +const PERMANENT_BAN_DURATION = 0 + +/** The "not blocked" answer — the reference server's stub `ReturnModerationBlockDetails()`. */ +const NOT_BLOCKED = { + ReportCategory: -1, + Duration: 0, + GameSessionId: 0, + IsBan: false, + IsHostKick: false, + IsVoiceModAutoban: false, + Message: null, + PlayerIdReporter: null, + TimeoutStartedAt: null, +} + +/** + * The block details for a ban in force — the `report` row a moderator set `banned` on. + * + * `Duration` is the seconds left on the ban (rounded up, so a ban with a second to run + * doesn't read as over), or `PERMANENT_BAN_DURATION` (0, no end) when `ban_expires` is + * NULL — `IsBan` alone marks the block, so the "not blocked" answer and a permanent ban + * share a `Duration` of 0 without being confused. The + * category is the one the report was filed under, so the client's ban screen names the + * reason. `Message` is a fixed "Rule violation" rather than the report's `details` — + * those are the REPORTER's words, and the banned player isn't shown them, for the same + * reason `PlayerIdReporter` stays null: the reporter is not a host who kicked them, and + * naming them would tell the banned player who reported them. `IsHostKick`, + * `IsVoiceModAutoban` and `TimeoutStartedAt` describe the OTHER kinds of block, none of + * which this server hands out. + */ +function banBlockDetails(ban: ReportRow, now: Date) { + const duration = + ban.ban_expires === null + ? PERMANENT_BAN_DURATION + : Math.max(1, Math.ceil((Date.parse(ban.ban_expires) - now.getTime()) / 1000)) + return { + ReportCategory: ban.report_category, + Duration: duration, + GameSessionId: 0, + IsBan: true, + IsHostKick: false, + IsVoiceModAutoban: false, + Message: 'Rule violation', + PlayerIdReporter: null, + TimeoutStartedAt: null, + } +} + // ---- Player reporting ------------------------------------------------------ export const moderationRoutes = new Hono({ strict: false }) - // Whether the caller is currently blocked (banned / timed out / host-kicked). Bans - // are stored (a report row with `banned` set) and enforced at matchmake and at login, - // but this endpoint is not wired to them, so it's always the "not blocked" answer — - // the reference server's stub `ReturnModerationBlockDetails()`. - // `ReportCategory` is `Unknown` (-1) rather than 0, which is a real category; - // `Message` is null, not the empty string that stub sends — the client distinguishes - // "no message" from a blank one. `IsVoiceModAutoban`/`TimeoutStartedAt` are on the - // DTO but left unset there, so they go out with their C# defaults. + // Whether the caller is currently blocked (banned / timed out / host-kicked). The one + // kind of block this server has is the account-wide ban — a `report` row with `banned` + // set (see `getActiveBan`), the same row matchmake refuses on — so a caller with one in + // force gets it described here, and everyone else gets the "not blocked" answer of the + // reference server's stub `ReturnModerationBlockDetails()`. This is the screen a banned + // player is shown, which is why `auth` still issues them a token: without one the client + // never gets here, and the ban reads as a failed sign-in. + // Only the caller's OWN account is consulted, not the evasion arms `resolveBan` adds + // at matchmake and login: this screen explains a ban handed to this account, and a + // player blocked for sharing a network with a banned one has no report row to show. + // In the "not blocked" answer `ReportCategory` is `Unknown` (-1) rather than 0, which + // is a real category, and `Message` is null, not the empty string that stub sends — + // the client distinguishes "no message" from a blank one. `IsVoiceModAutoban` / + // `TimeoutStartedAt` are on the DTO but unset there, so they go out with their C# + // defaults. // The newer client POSTs this with no body despite it being a pure read; it answers // GET too, so the path is reachable from either build. .on( @@ -242,29 +303,34 @@ export const moderationRoutes = new Hono({ strict: false }) tags: ['Moderation'], summary: 'Whether the caller is blocked', description: - 'Ban / timeout / host-kick state for the caller. Bans are stored (a `report` row ' + - 'with `banned` set) and enforced at matchmake and at login, but this endpoint is ' + - 'not wired to them, so it is always the “not blocked” answer, following the ' + - 'reference server’s stub: `ReportCategory` is `Unknown` (-1) rather than 0, which ' + - 'is a real category, and `Message` is null rather than the empty string that stub ' + - 'sends — the client distinguishes “no message” from a blank one. ' + - '`IsVoiceModAutoban` and `TimeoutStartedAt` are on the DTO but unset by that ' + - 'stub, so they carry their defaults. Answers GET or POST: the newer client POSTs ' + - 'it with no body.', - responses: { 200: json(ModerationBlockDetails, 'Always “not blocked”') }, + 'Ban / timeout / host-kick state for the caller. The one block this server hands ' + + 'out is the account-wide ban — a `report` row with `banned` set, the same row ' + + 'matchmake refuses on (login still issues a token, so the client can reach this ' + + 'screen) — so a caller with one in force gets ' + + '`IsBan: true`, the `ReportCategory` the report was filed under, `Duration` as the ' + + 'seconds left (0 for a permanent ban, which has no end) and the fixed ' + + '`Message` “Rule violation”. `PlayerIdReporter` stays null: it names a kicking ' + + 'host, and the reporter is not shown to the player they reported. Only the ' + + 'caller’s own account is consulted, not the ban-evasion arms.\n\n' + + 'Everyone else gets the reference server’s stub “not blocked” answer: ' + + '`ReportCategory` is `Unknown` (-1) rather than 0, which is a real category, and ' + + '`Message` is null rather than the empty string that stub sends — the client ' + + 'distinguishes “no message” from a blank one. `IsVoiceModAutoban` and ' + + '`TimeoutStartedAt` are on the DTO but unset by that stub, so they carry their ' + + 'defaults. Answers GET or POST: the newer client POSTs it with no body.', + security: AUTHED, + responses: { + 200: json(ModerationBlockDetails, 'The caller’s block, or “not blocked”'), + 401: UNAUTHORIZED_RESPONSE, + }, }), - (c) => - c.json({ - ReportCategory: -1, - Duration: 0, - GameSessionId: 0, - IsBan: false, - IsHostKick: false, - IsVoiceModAutoban: false, - Message: null, - PlayerIdReporter: null, - TimeoutStartedAt: null, - }) + async (c) => { + const id = await authedId(c) + if (id === null) return unauthorized(c) + const now = new Date() + const ban = await getActiveBan(c.env.DB, id, now) + return c.json(ban ? banBlockDetails(ban, now) : NOT_BLOCKED) + } ) // The reasons the client offers when a player starts a vote-to-kick. Order matters — // the client renders them in the order they arrive — and the list is grouped by the @@ -350,10 +416,10 @@ export const moderationRoutes = new Hono({ strict: false }) tags: ['Moderation'], summary: 'Submit a player report', description: - 'Records a player report in the `report` table; nothing dedupes the rows, and ' + - '`moderationBlockDetails` still answers “not blocked” unconditionally. A report ' + + 'Records a player report in the `report` table; nothing dedupes the rows. A report ' + 'is filed unbanned — a moderator converts one into an account-wide ban by setting ' + - '`banned` on the row, which is what matchmaking and `/connect/token` refuse on.\n\n' + + '`banned` on the row, which is what matchmaking refuses on and what ' + + '`moderationBlockDetails` describes to the banned player.\n\n' + 'The reporter is the caller (from the bearer token), NOT a body field. Only ' + '`PlayerIdReported` is required; the client omits whatever it has no value for ' + '(a report raised outside a room carries no `RoomId`), and those are stored as ' + diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 10bc18b..a61940f 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -750,32 +750,6 @@ describe('public endpoints', () => { expect(charadesWordsFor(new Date('2026-04-01T23:59:59Z'))).toBe(april) }) - // The client POSTs this with no body, despite it being a pure read; the route answers - // GET as well, and both methods serve the same body. - test.each(['GET', 'POST'])( - '%s /api/PlayerReporting/v1/moderationBlockDetails reports "not blocked"', - async (method) => { - const res = await exports.default.fetch( - `${ORIGIN}/api/PlayerReporting/v1/moderationBlockDetails`, - { method } - ) - expect(res.status).toBe(200) - // ReportCategory -1 = Unknown (0 is a real category). Message is null, not the - // reference stub's empty string — the client tells "no message" from a blank one. - expect(await res.json()).toEqual({ - ReportCategory: -1, - Duration: 0, - GameSessionId: 0, - IsBan: false, - IsHostKick: false, - IsVoiceModAutoban: false, - Message: null, - PlayerIdReporter: null, - TimeoutStartedAt: null, - }) - } - ) - // A fixed list, in render order — the client shows the buttons in the order they // arrive, so the order is part of the contract, not just the contents. test('GET /api/PlayerReporting/v1/voteToKickReasons serves the reasons in order', async () => { @@ -4531,6 +4505,98 @@ describe('player reports', () => { test('banFromReport returns null for an unknown report', async () => { expect(await banFromReport(env.DB, 999_999)).toBeNull() }) + + // What the banned player is TOLD. The block screen reads this; it's the same row + // matchmake and login refuse on, described rather than merely enforced. + describe('moderationBlockDetails', () => { + const NOT_BLOCKED = { + ReportCategory: -1, + Duration: 0, + GameSessionId: 0, + IsBan: false, + IsHostKick: false, + IsVoiceModAutoban: false, + Message: null, + PlayerIdReporter: null, + TimeoutStartedAt: null, + } + const details = async (method: string, sub: string) => { + const res = await exports.default.fetch( + `${ORIGIN}/api/PlayerReporting/v1/moderationBlockDetails`, + { method, headers: await bearer(sub) } + ) + expect(res.status).toBe(200) + return res.json() + } + + // The client POSTs this with no body, despite it being a pure read; the route + // answers GET as well, and both methods serve the same body. + test.each(['GET', 'POST'])( + '%s reports "not blocked" for an unbanned player', + async (method) => { + // A report against them that nobody acted on is not a block. + await submit({ PlayerIdReported: '220' }, await bearer()) + // ReportCategory -1 = Unknown (0 is a real category). Message is null, not the + // reference stub's empty string — the client tells "no message" from a blank one. + expect(await details(method, '220')).toEqual(NOT_BLOCKED) + } + ) + + test('401s without a bearer token', async () => { + const res = await exports.default.fetch( + `${ORIGIN}/api/PlayerReporting/v1/moderationBlockDetails` + ) + expect(res.status).toBe(401) + }) + + // A permanent ban has no end, so Duration is 0 — IsBan is what marks the block. + test('describes a permanent ban', async () => { + await submit( + { PlayerIdReported: '221', ReportCategory: '102', Details: 'slurs' }, + await bearer() + ) + const [row] = await getReportsAgainst(env.DB, 221) + await banFromReport(env.DB, row!.id) + + expect(await details('POST', '221')).toEqual({ + ReportCategory: 102, + Duration: 0, + GameSessionId: 0, + IsBan: true, + IsHostKick: false, + IsVoiceModAutoban: false, + // A fixed message — the report's `details` are the reporter's words, and + // the reporter is not shown to the player they reported, hence null. + Message: 'Rule violation', + PlayerIdReporter: null, + TimeoutStartedAt: null, + }) + }) + + // A timed ban reports the seconds LEFT, not its original length. + test('describes a timed ban with the seconds remaining', async () => { + await submit({ PlayerIdReported: '222', ReportCategory: '103' }, await bearer()) + const [row] = await getReportsAgainst(env.DB, 222) + await banFromReport(env.DB, row!.id, { + banExpires: new Date(Date.now() + 3600 * 1000).toISOString(), + }) + + const body = await details('GET', '222') + expect(body).toMatchObject({ ReportCategory: 103, IsBan: true, Message: 'Rule violation' }) + expect(body.Duration).toBeGreaterThan(3500) + expect(body.Duration).toBeLessThanOrEqual(3600) + }) + + // A ban that has served its time is not a block, even though the row still says + // `banned = 1` — the same rule `getActiveBan` applies for matchmake and login. + test('reports "not blocked" once a ban has expired', async () => { + await submit({ PlayerIdReported: '223' }, await bearer()) + const [row] = await getReportsAgainst(env.DB, 223) + await banFromReport(env.DB, row!.id, { banExpires: '2020-01-01T00:00:00.000Z' }) + + expect(await details('GET', '223')).toEqual(NOT_BLOCKED) + }) + }) }) describe('player warnings', () => { diff --git a/apps/auth/src/auth.app.ts b/apps/auth/src/auth.app.ts index 2a7d4dd..7f48b1e 100644 --- a/apps/auth/src/auth.app.ts +++ b/apps/auth/src/auth.app.ts @@ -71,19 +71,16 @@ const TOKEN_SCOPE = 'offline_access profile rn rn.accounts rn.accounts.gc rn.api rn.chat rn.clubs rn.commerce rn.match.read rn.match.write rn.notify rn.rooms rn.storage' /** - * The `error_description` a banned account's grant is refused with. A fixed sentence, - * never interpolated with the expiry, because `www`'s shared auth-messages table keys on - * this exact string to put a real sentence in front of a player — anything varying would - * fall through to the generic "you could not be signed in". Keep the two in sync. - */ -const BANNED_DESCRIPTION = 'this account is banned' - -/** - * The refusal when it is not THIS account that is banned but one it shares an identity - * with (see bans-db's linked arms). Deliberately a different, vaguer sentence: the - * account being refused may be an innocent housemate of a banned player, so telling them - * "this account is banned" would be a lie, and naming the account we matched them to - * would hand out somebody else's moderation record. + * The `error_description` a grant is refused with when the caller's account is not itself + * banned but shares an identity with one that is (see bans-db's linked arms). A fixed + * sentence, because `www`'s shared auth-messages table keys on this exact string to put a + * real sentence in front of a player — anything varying would fall through to the generic + * "you could not be signed in". Keep the two in sync. Deliberately vague: the account + * being refused may be an innocent housemate of a banned player, so telling them "this + * account is banned" would be a lie, and naming the account we matched them to would hand + * out somebody else's moderation record. + * + * A DIRECTLY banned account is not refused here at all — see the token grant. */ const BLOCKED_DESCRIPTION = 'this device or network is blocked' @@ -608,19 +605,22 @@ const app = new Hono() 'the `rn.privilege` CLAIM (`BanVChat`, `BanRmChat`) — scope-shaped name, but the', 'client reads it as a claim beside `role`, and it is absent for everyone else.', '', - '**Bans.** Once the grant has resolved an account, a BANNED account is refused a', - 'token at all (`invalid_grant`) — every grant, including a refresh. A ban is a', - '`report` row with `banned` set (the `api` worker owns that table); it lifts on its', - 'own when `ban_expires` passes, and never if that is null.', + '**Bans.** A BANNED account still gets a token — every grant, including a refresh.', + 'A ban is a `report` row with `banned` set (the `api` worker owns that table); it', + 'lifts on its own when `ban_expires` passes, and never if that is null. The token', + 'is what lets the client reach `api`’s `/api/PlayerReporting/v1/moderationBlockDetails`', + 'and show the player the block screen that explains the ban; the ban itself is', + 'enforced by `match`, which refuses every matchmake for a banned player, so a token', + 'gets them as far as that screen and no further.', '', - 'The refusal follows the player, not just the account: it also catches an account', - 'that shares a PROVEN platform identity (a `platform_account` link) or an IP', - '(`signupIp`/`lastLoginIp`, or the address this request came from) with a banned', - 'one, and a `create_account` carrying either is refused BEFORE it mints anything.', - 'Those two arms are the operator’s `BAN_EVASION_MATCH` knob (`ip`, `platform`, or', - '`off`); the ban on the account itself is always enforced. A linked match answers a', - 'deliberately vaguer description than a direct one — the account refused may belong', - 'to a housemate of the banned player rather than to them.', + 'What IS refused here (`invalid_grant`) is ban EVASION: an account that shares a', + 'PROVEN platform identity (a `platform_account` link) or an IP (`signupIp`/', + '`lastLoginIp`, or the address this request came from) with a banned one, and a', + '`create_account` carrying either, which is refused BEFORE it mints anything. Such', + 'an account has no ban of its own for the block screen to describe, so there is', + 'nothing to let it in for. Those two arms are the operator’s `BAN_EVASION_MATCH`', + 'knob (`ip`, `platform`, or `off`). The description is deliberately vague — the', + 'account refused may belong to a housemate of the banned player rather than to them.', ].join('\n'), requestBody: form( TokenRequest, @@ -633,7 +633,7 @@ const app = new Hono() [ 'Unusable grant: bad credentials, an unverifiable platform or platform_auth, an', 'invalid/expired refresh token, a missing account identifier, a signup cap reached,', - 'or a banned account', + 'or an account sharing a banned one’s device or network', ].join(' ') ), 500: json( @@ -789,9 +789,9 @@ const app = new Hono() // via create_account or /account/me/changepassword. let accountId: string if (grantType === 'create_account') { - // A banned player's next move is a new account, so the ban is checked BEFORE - // one is minted — against the only identity a signup has, the IP it came from - // and the platform identity it just proved. Refusing after the fact (as the + // A banned player's next move is a new account, so the evasion arms are checked + // BEFORE one is minted — against the only identity a signup has, the IP it came + // from and the platform identity it just proved. Refusing after the fact (as the // shared check below would) still refuses the token, but leaves the account // row behind and burns a slot off both signup caps, so the evader gets to keep // making them. @@ -987,14 +987,19 @@ const app = new Hono() await setLoginContext(c.env.DB, resolvedId, { deviceId, deviceClass, ip: clientIp }) } - // A banned player gets no token — and with no token every other worker is shut to - // them, so this is the outer wall of a ban; matchmaking's refusal is the inner - // one, which still has to exist because a token issued before the ban stays valid - // until it expires. + // A DIRECTLY banned account still gets its token. The client needs one to reach + // `api`'s moderationBlockDetails, which is where the player is TOLD they are banned + // (category, time left, "Rule violation") — refused here, they would only ever see + // a failed sign-in. The ban is enforced by matchmaking instead, which refuses every + // matchmake for a banned player, so the token gets them as far as the block screen + // and no further. Logged, so the operator can see a banned player signing in. // - // Checked once here, after the grant has resolved an account, so it covers every - // grant: password, cached_login and a refresh_token redeemed by a client that has - // been running since before the ban. Deliberately AFTER the credential checks — + // Ban EVASION is still refused here: an account that merely shares a device or + // network with a banned one has no ban of its own for that screen to describe, so + // there is nothing to let it in for — and letting it in is exactly what the evader + // wants. Checked once here, after the grant has resolved an account, so it covers + // every grant: password, cached_login and a refresh_token redeemed by a client that + // has been running since before the ban. Deliberately AFTER the credential checks — // a wrong password is still "invalid account_id or password", so this can't be // used to probe whether an account exists or is banned without knowing it. // @@ -1007,8 +1012,8 @@ const app = new Hono() identity: { ip: clientIp, platform: verifiedPlatform, platformId: verifiedPlatformId }, arms: banEvasionMatch(c.env.BAN_EVASION_MATCH), }) - if (ban) { - logger.info('token refused: player banned', { + if (ban && ban.via !== 'account') { + logger.info('token refused: ban evasion', { accountId, grantType, via: ban.via, @@ -1016,13 +1021,15 @@ const app = new Hono() reportId: ban.ban.id, banExpires: ban.ban.ban_expires, }) - return c.json( - { - error: 'invalid_grant', - error_description: ban.via === 'account' ? BANNED_DESCRIPTION : BLOCKED_DESCRIPTION, - }, - 400 - ) + return c.json({ error: 'invalid_grant', error_description: BLOCKED_DESCRIPTION }, 400) + } + if (ban) { + logger.info('token issued to banned account', { + accountId, + grantType, + reportId: ban.ban.id, + banExpires: ban.ban.ban_expires, + }) } // Never sign with an empty key. An empty JWT_SECRET (misconfigured/missing diff --git a/apps/auth/src/test/integration/api.test.ts b/apps/auth/src/test/integration/api.test.ts index f2bd711..2626120 100644 --- a/apps/auth/src/test/integration/api.test.ts +++ b/apps/auth/src/test/integration/api.test.ts @@ -89,8 +89,8 @@ beforeAll(async () => { IsDorm: false, SubRooms: [{ SubRoomId: 23, UnitySceneId: ORIENTATION_SCENE, MaxPlayers: 1 }], }) - // Report table (owned by the api worker) — a banned account is refused a token, and - // a ban is a report row with `banned` set. + // Report table (owned by the api worker) — a ban is a report row with `banned` set; + // the token grant reads it for the evasion arms. for (const stmt of REPORTS_SCHEMA_DDL) await env.DB.prepare(stmt).run() }) @@ -1436,38 +1436,35 @@ describe('CORS', () => { }) }) -// A banned account is refused a token at all — the outer wall of a ban, since with no -// token every other worker is shut to it. The ban is a `report` row with `banned` set -// (the api worker owns that table); matchmaking enforces the same ban on tokens issued -// before it was handed down. +// A banned account is still issued a token: the game client needs one to reach the api +// worker's moderationBlockDetails, which is where the player is shown WHY they are +// blocked. The ban is a `report` row with `banned` set (the api worker owns that table) +// and is enforced by matchmaking, which refuses every matchmake for a banned player — so +// the token gets them to the block screen and no further. describe('banned accounts', () => { - test('POST /connect/token refuses a password grant from a banned account', async () => { + test('POST /connect/token issues a token to a banned account', async () => { await seedAccount(6101, 'BannedPlayer') await banAccount(6101) const res = await postToken(`account_id=6101&password=${LOGIN_PASSWORD}`) - expect(res.status).toBe(400) - expect(res.json.error).toBe('invalid_grant') - // The exact sentence www's shared auth-messages table keys on to put a real - // message in front of the player — changing it silently downgrades that to the - // generic "you could not be signed in". - expect(res.json.error_description).toBe('this account is banned') + expect(res.status).toBe(200) + expect(decodePayload(res.json.access_token as string).sub).toBe('6101') }) - test('POST /connect/token refuses a username login from a banned account', async () => { + test('POST /connect/token issues a token to a banned account logging in by username', async () => { await seedAccount(6102, 'BannedByName') await banAccount(6102) const res = await postToken( `grant_type=password&username=BannedByName&password=${LOGIN_PASSWORD}` ) - expect(res.status).toBe(400) - expect(res.json.error_description).toBe('this account is banned') + expect(res.status).toBe(200) + expect(decodePayload(res.json.access_token as string).sub).toBe('6102') }) - // A client that was already signed in when the ban landed still holds a valid refresh - // token; redeeming it must not renew the session. - test('POST /connect/token refuses to refresh a banned account’s session', async () => { + // A client that was already signed in when the ban landed refreshes as normal — its + // next matchmake is what refuses it, and moderationBlockDetails says why. + test('POST /connect/token refreshes a banned account’s session', async () => { await seedAccount(6103, 'BannedLater') const login = await postToken(`account_id=6103&password=${LOGIN_PASSWORD}`) expect(login.status).toBe(200) @@ -1477,13 +1474,12 @@ describe('banned accounts', () => { const refreshed = await postToken( `grant_type=refresh_token&refresh_token=${encodeURIComponent(refreshToken)}` ) - expect(refreshed.status).toBe(400) - expect(refreshed.json.error_description).toBe('this account is banned') + expect(refreshed.status).toBe(200) + expect(decodePayload(refreshed.json.access_token as string).sub).toBe('6103') }) - // The ban check runs AFTER the credential check, so a wrong password on a banned - // account still answers the ordinary bad-credential refusal — it can't be used to - // find out whether an account exists or is banned without knowing its password. + // A ban does not loosen the credential check: a wrong password on a banned account is + // the ordinary bad-credential refusal. test('a wrong password on a banned account is still a credential refusal', async () => { await seedAccount(6104, 'BannedWrongPw') await banAccount(6104) @@ -1493,23 +1489,13 @@ describe('banned accounts', () => { expect(res.json.error_description).toBe('invalid account_id or password') }) - // A timed ban lifts itself when its expiry passes; nothing clears the flag. - test('an expired ban lets the account sign in again', async () => { - await seedAccount(6105, 'ServedTime') - await banAccount(6105, '2020-01-01T00:00:00.000Z') - - const res = await postToken(`account_id=6105&password=${LOGIN_PASSWORD}`) - expect(res.status).toBe(200) - expect(decodePayload(res.json.access_token as string).sub).toBe('6105') - }) - - test('a ban that has not expired yet still refuses the login', async () => { + test('a ban that has not expired yet still issues a token', async () => { await seedAccount(6106, 'StillServing') await banAccount(6106, new Date(Date.now() + 3_600_000).toISOString()) const res = await postToken(`account_id=6106&password=${LOGIN_PASSWORD}`) - expect(res.status).toBe(400) - expect(res.json.error_description).toBe('this account is banned') + expect(res.status).toBe(200) + expect(decodePayload(res.json.access_token as string).sub).toBe('6106') }) // A report is not a ban until a moderator converts it. @@ -1534,8 +1520,10 @@ describe('banned accounts', () => { // The ban follows the player past the account it was written on: a login from an account // that shares a proven platform identity or an IP with a banned one is refused, and a -// signup carrying either is refused before it mints anything. See the api worker's -// bans-db.ts for the arms and the BAN_EVASION_MATCH knob. +// signup carrying either is refused before it mints anything. Unlike the banned account +// itself, such an account has no ban of its own for the block screen to describe, so +// there is nothing to let it in for. See the api worker's bans-db.ts for the arms and +// the BAN_EVASION_MATCH knob. describe('ban evasion at the token endpoint', () => { /** Seed a loginable account carrying the IPs it signed up / last logged in from. */ const account = async (id: number, name: string, ips: Record = {}) => { @@ -1615,7 +1603,7 @@ describe('ban evasion at the token endpoint', () => { }) // The knob an operator reaches for when the IP arm locks out real players. - test('BAN_EVASION_MATCH=platform drops the IP arm but keeps the direct ban', async () => { + test('BAN_EVASION_MATCH=platform drops the IP arm but keeps the platform one', async () => { const original = env.BAN_EVASION_MATCH await account(6320, 'KnobBanned', { signupIp: '203.0.113.50' }) await linkPlatformIdentity(env.DB, 6320, 0, 'steam-knobevader') @@ -1633,10 +1621,9 @@ describe('ban evasion at the token endpoint', () => { env.BAN_EVASION_MATCH = 'off' expect((await login(6322)).status).toBe(200) - // The banned account itself is refused whatever the knob says. - const banned = await login(6320) - expect(banned.status).toBe(400) - expect(banned.json.error_description).toBe('this account is banned') + // The banned account itself signs in whatever the knob says — its ban is + // enforced at matchmake, and the knob only governs the linked arms. + expect((await login(6320)).status).toBe(200) } finally { env.BAN_EVASION_MATCH = original } diff --git a/apps/www/src/auth-messages.ts b/apps/www/src/auth-messages.ts index f648712..e1c6071 100644 --- a/apps/www/src/auth-messages.ts +++ b/apps/www/src/auth-messages.ts @@ -32,13 +32,11 @@ const AUTH_MESSAGES: Record = { '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.', - // Deliberately says nothing about when it lifts: auth sends one fixed description for - // every ban (see its BANNED_DESCRIPTION), permanent or timed, so there is no expiry - // here to quote. - 'this account is banned': 'This account is banned and cannot be signed in to.', - // Not this account, but one it shares a device or network with. Phrased for BOTH the + // An account that shares a device or network with a BANNED one. Phrased for BOTH the // person evading a ban and the housemate of one — the IP arm cannot tell them apart — - // and for both forms, since signup and sign-in send the same description. + // and for both forms, since signup and sign-in send the same description. A directly + // banned account is not refused a sign-in at all (auth issues it a token so the game + // client can show the block screen), which is why there is no "banned" entry here. 'this device or network is blocked': 'This device or network is blocked. If you think that is a mistake, contact the server operator.', }