From fd4264d982ade88388e727fbd3393ddbde25c8eb Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Thu, 17 Sep 2026 10:43:31 -0400 Subject: [PATCH] [rooms] improve room ban/unban, history [api] mod kick [match] allow banned users into dorm --- apps/api/src/openapi.ts | 12 + apps/api/src/reports-db.ts | 81 ++++ apps/api/src/routes/moderation.ts | 191 ++++---- apps/api/src/test/integration/api.test.ts | 214 ++++++++- apps/auth/src/auth.app.ts | 85 ++-- apps/auth/src/test/integration/api.test.ts | 63 +-- apps/match/src/context.ts | 10 +- apps/match/src/match.app.ts | 54 ++- apps/match/src/test/integration/api.test.ts | 91 +++- .../0018_room_ban_reason_expiry.sql | 13 + .../migrations/0019_room_ban_history.sql | 30 ++ apps/rooms/src/openapi.ts | 62 +++ apps/rooms/src/rooms.app.ts | 334 +++++++++++-- apps/rooms/src/test/integration/api.test.ts | 439 +++++++++++++++++- apps/rooms/vitest.config.ts | 8 +- apps/www/src/staff.ts | 67 +-- apps/www/src/test/integration/api.test.ts | 41 ++ packages/domain/src/rooms-db.ts | 269 +++++++++-- 18 files changed, 1804 insertions(+), 260 deletions(-) create mode 100644 apps/rooms/migrations/0018_room_ban_reason_expiry.sql create mode 100644 apps/rooms/migrations/0019_room_ban_history.sql diff --git a/apps/api/src/openapi.ts b/apps/api/src/openapi.ts index 3debbb5..ff32091 100644 --- a/apps/api/src/openapi.ts +++ b/apps/api/src/openapi.ts @@ -1477,6 +1477,18 @@ export const InstantKickRequest = z.object({ PlayerIds: z.array(z.int()).describe('Account ids to kick out of that instance'), }) +/** `POST /api/PlayerReporting/v1/roomModKick` form body — one player out of one instance. */ +export const RoomModKickRequest = z.object({ + PlayerId: z.string().describe('Account id of the player to kick'), + GameSessionId: z + .string() + .describe('The room instance (game session) to eject them from — they must be standing in it'), + Reason: z + .string() + .optional() + .describe('Free text, shown to the kicked player in the kick message'), +}) + /** `POST /api/PlayerReporting/v1/deviceId` form body — the id rotation the client reports. */ export const DeviceIdRequest = z.object({ oldDeviceId: z.string().optional().describe('The id the client thinks we hold'), diff --git a/apps/api/src/reports-db.ts b/apps/api/src/reports-db.ts index e666555..087abc6 100644 --- a/apps/api/src/reports-db.ts +++ b/apps/api/src/reports-db.ts @@ -467,3 +467,84 @@ export async function getBansInForce( .all() return results } + +// ---- Block details ---------------------------------------------------------- + +/** + * `Duration` on a permanent ban. The client's field is a 32-bit int of seconds that PAIRS + * with `TimeoutStartedAt` — start + duration is the end of the block — so a ban with no + * end gets the largest value the field holds, 68 years past its start. + */ +export const PERMANENT_BAN_DURATION = 2_147_483_647 + +/** + * The "not blocked" answer — the reference server's stub `ReturnModerationBlockDetails()`, + * widened to every key the client's `ModerationBlockDetail` decoder names (16 on the wire; + * the 2025 build's formatter reads them all). The ones past the stub's nine are the block + * kinds and screen dressings this server never uses — a device ban, a warning, the + * vote-kick reason, an associated account, the creator code of conduct, the top/bottom + * message overrides — so they carry their "none" values on every answer. + */ +export const NOT_BLOCKED = { + ReportCategory: -1, + Duration: 0, + GameSessionId: 0, + IsHostKick: false, + Message: null, + PlayerIdReporter: null, + IsBan: false, + IsVoiceModAutoban: false, + IsDeviceBan: false, + IsWarning: false, + VoteKickReason: null, + TimeoutStartedAt: null, + AssociatedAccountUsername: null, + ShowCreatorCodeOfConduct: false, + TopMessageOverride: null, + BottomMessageOverride: null, +} + +/** + * The block details for a ban in force — the `report` row a moderator set `banned` on. + * + * Shared by the two places a player meets an account ban: `api`'s `moderationBlockDetails` + * (the screen at sign-in) and `www`'s live `ModerationKick` frame (the screen when the ban is + * handed down mid-session). Built once here so both screens describe the same ban. + * + * `Duration` and `TimeoutStartedAt` are a PAIR in the client: the block runs from the + * start for the duration. The start is `banned_at`, the instant the ban was handed down + * (see 0020_report_ban_audit.sql), and the duration is the seconds from there to + * `ban_expires`, so the two sum to the expiry; or `PERMANENT_BAN_DURATION` when there is + * none. + * + * It falls back to the report's `created_at` for a row banned before that column existed. + * The two can be months apart, and using `created_at` as the start — which is what this + * did before there was anything else to use — misreports both halves of the pair: a 7-day + * ban applied to a 30-day-old report told the player their block began a month ago and + * ended three weeks ago. Every pre-migration row still reads exactly as it used to, which + * is the point of the fallback rather than a coalesce to now. + * + * 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. Everything else keeps its `NOT_BLOCKED` value: the other block kinds and + * screen dressings, none of which this server hands out. + */ +export function banBlockDetails(ban: ReportRow) { + const startedAtIso = ban.banned_at ?? ban.created_at + const startedAt = Date.parse(startedAtIso) + const duration = + ban.ban_expires === null + ? PERMANENT_BAN_DURATION + : Math.max(1, Math.ceil((Date.parse(ban.ban_expires) - startedAt) / 1000)) + return { + ...NOT_BLOCKED, + ReportCategory: ban.report_category, + Duration: duration, + IsBan: true, + Message: 'Rule violation', + TimeoutStartedAt: startedAtIso, + } +} diff --git a/apps/api/src/routes/moderation.ts b/apps/api/src/routes/moderation.ts index ad169b3..aff7847 100644 --- a/apps/api/src/routes/moderation.ts +++ b/apps/api/src/routes/moderation.ts @@ -2,9 +2,11 @@ import { Hono } from 'hono' import { describeRoute } from 'hono-openapi' import { + canManageRoom, canModerateRoom, deletePresence, getPlayerIdsInInstance, + getPresence, getPresences, getRoomById, getStoredRoomInstance, @@ -30,18 +32,18 @@ import { JsonArray, jsonBody, ModerationBlockDetails, + RoomModKickRequest, SuccessErrorEnvelope, UNAUTHORIZED_RESPONSE, VoteToKickReason, VoteToKickRequest, } from '../openapi' -import { createReport, getActiveBan } from '../reports-db' +import { banBlockDetails, createReport, getActiveBan, NOT_BLOCKED } 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 @@ -131,14 +133,17 @@ async function pushInstantKick( playerIds: number[], gameSessionId: number, roomName: string, - moderatorId: number + moderatorId: number, + { reason = '', isHostKick = true }: { reason?: string; isHostKick?: boolean } = {} ): Promise { const frame: ModerationKickPayload = { ReportCategory: KickReportCategory.Moderator, Duration: 0, GameSessionId: gameSessionId, - IsHostKick: true, - Message: `You have been kicked from ${roomName}.`, + IsHostKick: isHostKick, + Message: reason + ? `You have been kicked from ${roomName}. Reason: ${reason}` + : `You have been kicked from ${roomName}.`, PlayerIdReporter: moderatorId, IsBan: false, IsVoiceModAutoban: false, @@ -224,81 +229,6 @@ async function pushVoteToKick(c: Context, message: VoteToKickMessage): Prom } } -/** - * `Duration` on a permanent ban. The client's field is a 32-bit int of seconds that PAIRS - * with `TimeoutStartedAt` — start + duration is the end of the block — so a ban with no - * end gets the largest value the field holds, 68 years past its start. - */ -const PERMANENT_BAN_DURATION = 2_147_483_647 - -/** - * The "not blocked" answer — the reference server's stub `ReturnModerationBlockDetails()`, - * widened to every key the client's `ModerationBlockDetail` decoder names (16 on the wire; - * the 2025 build's formatter reads them all). The ones past the stub's nine are the block - * kinds and screen dressings this server never uses — a device ban, a warning, the - * vote-kick reason, an associated account, the creator code of conduct, the top/bottom - * message overrides — so they carry their "none" values on every answer. - */ -const NOT_BLOCKED = { - ReportCategory: -1, - Duration: 0, - GameSessionId: 0, - IsHostKick: false, - Message: null, - PlayerIdReporter: null, - IsBan: false, - IsVoiceModAutoban: false, - IsDeviceBan: false, - IsWarning: false, - VoteKickReason: null, - TimeoutStartedAt: null, - AssociatedAccountUsername: null, - ShowCreatorCodeOfConduct: false, - TopMessageOverride: null, - BottomMessageOverride: null, -} - -/** - * The block details for a ban in force — the `report` row a moderator set `banned` on. - * - * `Duration` and `TimeoutStartedAt` are a PAIR in the client: the block runs from the - * start for the duration. The start is `banned_at`, the instant the ban was handed down - * (see 0020_report_ban_audit.sql), and the duration is the seconds from there to - * `ban_expires`, so the two sum to the expiry; or `PERMANENT_BAN_DURATION` when there is - * none. - * - * It falls back to the report's `created_at` for a row banned before that column existed. - * The two can be months apart, and using `created_at` as the start — which is what this - * did before there was anything else to use — misreports both halves of the pair: a 7-day - * ban applied to a 30-day-old report told the player their block began a month ago and - * ended three weeks ago. Every pre-migration row still reads exactly as it used to, which - * is the point of the fallback rather than a coalesce to now. - * - * 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. Everything else keeps its `NOT_BLOCKED` value: the other block kinds and - * screen dressings, none of which this server hands out. - */ -function banBlockDetails(ban: ReportRow) { - const startedAtIso = ban.banned_at ?? ban.created_at - const startedAt = Date.parse(startedAtIso) - const duration = - ban.ban_expires === null - ? PERMANENT_BAN_DURATION - : Math.max(1, Math.ceil((Date.parse(ban.ban_expires) - startedAt) / 1000)) - return { - ...NOT_BLOCKED, - ReportCategory: ban.report_category, - Duration: duration, - IsBan: true, - Message: 'Rule violation', - TimeoutStartedAt: startedAtIso, - } -} - // ---- Player reporting ------------------------------------------------------ export const moderationRoutes = new Hono({ strict: false }) // Whether the caller is currently blocked (banned / timed out / host-kicked). The one @@ -700,6 +630,107 @@ export const moderationRoutes = new Hono({ strict: false }) } ) + // ONE player kicked out of ONE live instance, from the in-room moderation menu, with a + // reason. The single-player sibling of `instantKick`, but form-encoded and gated more + // narrowly: the room's owner or a co-owner, or an account holding the staff role — not + // the room's Moderator (20) tier, which `instantKick` lets through. + // + // The cross-room guard is the same and it is the point: the instance names the room, the + // room is what the caller's authority is checked against, and the target's LIVE presence + // must put them in that very instance. Naming a player standing in some other room — even + // another instance of this one — kicks nobody, so authority over one room can never reach + // a session in another. + .post( + '/api/PlayerReporting/v1/roomModKick', + describeRoute({ + tags: ['Moderation'], + summary: 'Kick one player out of a room instance, with a reason', + description: + 'Ejects `PlayerId` from the live room instance `GameSessionId` (a `roomInstanceId`). ' + + 'Form-encoded, unlike `instantKick`’s JSON; `Reason` is optional free text shown to ' + + 'the kicked player in the kick message.\n\n' + + 'Gated to the instance’s room: its creator or a co-owner, or any account whose token ' + + 'carries the `moderator` / `developer` role. A room Moderator (20) or Host is NOT ' + + 'enough here. Anyone else with a valid token gets a 403.\n\n' + + 'The player must be standing in **that** instance — their live `presence` row must ' + + 'name it. Otherwise nothing happens and the call answers `success: false`, so an ' + + 'account id cannot reach into a session in another room, or another instance of this ' + + 'one. The caller need not be in the instance themselves. The caller cannot kick ' + + 'themselves, and nobody can kick the room’s owner or a co-owner.\n\n' + + 'A kick deletes the player’s presence row (they read offline at once and the instance ' + + 'frees a slot, its fullness recomputed) and sends them an EPHEMERAL `ModerationKick` ' + + 'frame (id 22) with `IsBan: false` — nothing stops them rejoining. `IsHostKick` is ' + + 'true when the caller runs the room, false for a staff account acting in a room it ' + + 'does not.\n\n' + + 'Answers the lowercase `{ success, error }` envelope; the shape is unverified against ' + + 'the real service.', + security: AUTHED, + requestBody: form(RoomModKickRequest, 'The session, the player and a reason'), + responses: { + 200: json( + SuccessErrorEnvelope, + '`{ success: true, error: "" }`, or `success: false` when the player cannot be kicked' + ), + 400: json(SuccessErrorEnvelope, 'No `GameSessionId` or `PlayerId`'), + 401: UNAUTHORIZED_RESPONSE, + 403: json(SuccessErrorEnvelope, 'The caller neither runs the room nor holds a staff role'), + 404: json(SuccessErrorEnvelope, 'No such game session'), + }, + }), + async (c) => { + const moderatorId = await authedId(c) + if (moderatorId === null) return unauthorized(c) + + const body = await c.req.parseBody().catch(() => ({}) as Record) + const gameSessionId = asInt(formField(body, c, 'GameSessionId')) + if (gameSessionId === null) { + return c.json({ success: false, error: 'GameSessionId is required' }, 400) + } + const playerId = asInt(formField(body, c, 'PlayerId')) + if (playerId === null) return c.json({ success: false, error: 'PlayerId is required' }, 400) + const reason = formField(body, c, 'Reason')?.trim() ?? '' + + // The instance names the room, and the room carries the roles this is gated on. + const instance = await getStoredRoomInstance(c.env.DB, gameSessionId) + const room = instance && (await getRoomById(c.env.DB, instance.roomId)) + if (!room) return c.json({ success: false, error: 'This game session does not exist!' }, 404) + + // The room's own owners first; the token's roles are only read when that fails. + const runsRoom = canManageRoom(room, moderatorId) + if (!runsRoom) { + const roles = await authedRoles(c) + if (!roles?.some((role) => MODERATOR_ROLES.has(role))) { + return c.json({ success: false, error: 'Forbidden' }, 403) + } + } + + if (playerId === moderatorId) { + return c.json({ success: false, error: 'You cannot kick yourself!' }) + } + // Otherwise a co-owner, or a staffer, could throw the room's owner out of it. + if (canManageRoom(room, playerId)) { + return c.json({ success: false, error: 'You cannot kick an owner of this room!' }) + } + + // The cross-room guard: their LIVE presence must name this exact instance. Offline, + // expired, in another room, or in another instance of this room are all "not here". + const presence = await getPresence<{ roomInstanceId?: number }>(c.env.DB, playerId) + if (presence?.roomInstance?.roomInstanceId !== gameSessionId) { + return c.json({ success: false, error: 'That player is not in this game session!' }) + } + + await deletePresence(c.env.DB, playerId) + await refreshInstanceFullness(c.env.DB, gameSessionId) + const roomName = typeof room.Name === 'string' ? room.Name : 'this room' + await pushInstantKick(c, [playerId], gameSessionId, roomName, moderatorId, { + reason, + isHostKick: runsRoom, + }) + + return c.json({ success: true, error: '' }) + } + ) + // A warning handed down by a moderator — the staff-side counterpart to a report. // Gated on the `moderator` role in the token, not just a valid one. .post( diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 190bf43..a0fd017 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -103,7 +103,8 @@ const TEST_ROOMS = [ }, { // The instant kick's room. Owned by account 42 (the default test token); 43 holds - // Moderator (20) and 44 only Host (10) — the tier just below that gate. + // Moderator (20) and 44 only Host (10) — the tier just below that gate. 45 is a + // CoOwner (30), for `roomModKick`, which is gated on owners rather than moderators. RoomId: 4, Name: 'KickRoom', IsDorm: false, @@ -112,6 +113,7 @@ const TEST_ROOMS = [ Roles: [ { AccountId: 43, Role: 20, LastChangedByAccountId: null, InvitedRole: 0 }, { AccountId: 44, Role: 10, LastChangedByAccountId: null, InvitedRole: 0 }, + { AccountId: 45, Role: 30, LastChangedByAccountId: null, InvitedRole: 0 }, ], }, ] @@ -4688,6 +4690,215 @@ describe('instant kick', () => { }) }) +describe('room mod kick', () => { + // A session of KickRoom (room 4, owned by 42, co-owned by 45), a second session of the + // same room, and a session of RecCenter (room 2, owned by 1) — somebody else's room. + const SESSION = 1074859 + const SAME_ROOM_SESSION = 1074860 + const OTHER_ROOM_SESSION = 1074861 + + const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') + + const seedInstance = async ( + roomInstanceId: number, + roomId: number, + maxCapacity = 0, + isFull = false + ) => + env.DB.prepare('INSERT OR REPLACE INTO room_instance (data) VALUES (?1)') + .bind( + JSON.stringify({ + roomInstanceId, + ownerAccountId: 42, + roomId, + subRoomId: roomId, + maxCapacity, + isFull, + gameVersion: GAME_VERSION, + createdAt: new Date().toISOString(), + }) + ) + .run() + + const standIn = async (accountId: number, roomInstanceId: number, roomId = 4) => + env.DB.prepare('INSERT OR REPLACE INTO presence (data) VALUES (?1)') + .bind( + JSON.stringify({ + accountId, + roomInstance: { roomInstanceId, roomId }, + expiresAt: Math.floor(Date.now() / 1000) + PRESENCE_TTL_SECONDS, + }) + ) + .run() + + const isPresent = async (accountId: number) => + (await env.DB.prepare('SELECT COUNT(*) AS n FROM presence WHERE account_id = ?1') + .bind(accountId) + .first<{ n: number }>())!.n === 1 + + // The client's form body. + const modKick = async (fields: Record, sub = '42', roles?: string[]) => + exports.default.fetch(`${ORIGIN}/api/PlayerReporting/v1/roomModKick`, { + method: 'POST', + headers: { + ...(await bearer(sub, roles)), + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams(fields).toString(), + }) + + const frames = async () => + (await (await hub().fetch('http://do/all')).json()) as Array<{ + playerIds?: number[] + ephemeral?: boolean + data: Record + }> + + test('the room’s owner kicks a player out of the session they are in, with a reason', async () => { + await hub().fetch('http://do/all', { method: 'DELETE' }) + // A full one-seat instance that 205 is filling. + await seedInstance(SESSION, 4, 1, true) + await standIn(205, SESSION) + + // The client's body, verbatim. + const res = await modKick({ PlayerId: '205', GameSessionId: String(SESSION), Reason: 'test' }) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ success: true, error: '' }) + + expect(await isPresent(205)).toBe(false) + const full = await env.DB.prepare('SELECT is_full AS f FROM room_instance WHERE id = ?1') + .bind(SESSION) + .first<{ f: number }>() + expect(full!.f).toBe(0) + + expect(await frames()).toEqual([ + expect.objectContaining({ + playerIds: [205], + ephemeral: true, + data: expect.objectContaining({ + GameSessionId: SESSION, + IsHostKick: true, + IsBan: false, + PlayerIdReporter: 42, + Message: 'You have been kicked from KickRoom. Reason: test', + }), + }), + ]) + }) + + // The whole reason for the presence check: authority over one room must never reach a + // session somewhere else. + test('a player who is not standing in that exact session is not kicked', async () => { + await hub().fetch('http://do/all', { method: 'DELETE' }) + await seedInstance(SESSION, 4) + await seedInstance(SAME_ROOM_SESSION, 4) + await seedInstance(OTHER_ROOM_SESSION, 2) + + // In another instance of the SAME room, in a different room entirely, and offline. + await standIn(206, SAME_ROOM_SESSION) + await standIn(207, OTHER_ROOM_SESSION, 2) + for (const playerId of ['206', '207', '208']) { + const res = await modKick({ PlayerId: playerId, GameSessionId: String(SESSION) }, '42') + expect(res.status, playerId).toBe(200) + expect(await res.json(), playerId).toEqual({ + success: false, + error: 'That player is not in this game session!', + }) + } + expect(await isPresent(206)).toBe(true) + expect(await isPresent(207)).toBe(true) + + // Naming the OTHER room's session instead doesn't help: 42 has no authority there. + const cross = await modKick( + { PlayerId: '207', GameSessionId: String(OTHER_ROOM_SESSION) }, + '42' + ) + expect(cross.status).toBe(403) + expect(await isPresent(207)).toBe(true) + expect(await frames()).toEqual([]) + }) + + test('a co-owner or a staff role may kick; a room moderator, a host and a stranger may not', async () => { + await seedInstance(SESSION, 4) + + // 45 co-owns the room. + await standIn(209, SESSION) + expect( + await (await modKick({ PlayerId: '209', GameSessionId: String(SESSION) }, '45')).json() + ).toEqual({ success: true, error: '' }) + expect(await isPresent(209)).toBe(false) + + // 999 has no role on the room but carries the staff `moderator` role — and is not the + // host, so the frame says so. + await hub().fetch('http://do/all', { method: 'DELETE' }) + await standIn(209, SESSION) + const staff = await modKick({ PlayerId: '209', GameSessionId: String(SESSION) }, '999', [ + 'gameClient', + 'moderator', + ]) + expect(await staff.json()).toEqual({ success: true, error: '' }) + expect(await isPresent(209)).toBe(false) + expect((await frames())[0].data).toMatchObject({ IsHostKick: false, PlayerIdReporter: 999 }) + + // 43 is the room's Moderator (20) — enough for instantKick, not for this. 44 is a Host, + // 99 holds nothing. + await standIn(209, SESSION) + for (const sub of ['43', '44', '99']) { + const res = await modKick({ PlayerId: '209', GameSessionId: String(SESSION) }, sub) + expect(res.status, sub).toBe(403) + expect(await res.json()).toEqual({ success: false, error: 'Forbidden' }) + } + expect(await isPresent(209)).toBe(true) + + const anon = await exports.default.fetch(`${ORIGIN}/api/PlayerReporting/v1/roomModKick`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: `PlayerId=209&GameSessionId=${SESSION}`, + }) + expect(anon.status).toBe(401) + }) + + test('nobody kicks themselves or an owner of the room', async () => { + await seedInstance(SESSION, 4) + await standIn(42, SESSION) + await standIn(45, SESSION) + + expect( + await (await modKick({ PlayerId: '42', GameSessionId: String(SESSION) }, '42')).json() + ).toEqual({ success: false, error: 'You cannot kick yourself!' }) + // A co-owner cannot throw out the owner, and staff cannot throw out a co-owner. + expect( + await (await modKick({ PlayerId: '42', GameSessionId: String(SESSION) }, '45')).json() + ).toEqual({ success: false, error: 'You cannot kick an owner of this room!' }) + expect( + await ( + await modKick({ PlayerId: '45', GameSessionId: String(SESSION) }, '999', ['moderator']) + ).json() + ).toEqual({ success: false, error: 'You cannot kick an owner of this room!' }) + expect(await isPresent(42)).toBe(true) + expect(await isPresent(45)).toBe(true) + }) + + test('an unknown session 404s, and the body must name a session and a player', async () => { + const unknown = await modKick({ PlayerId: '205', GameSessionId: '999999' }) + expect(unknown.status).toBe(404) + expect(await unknown.json()).toEqual({ + success: false, + error: 'This game session does not exist!', + }) + + for (const [fields, error] of [ + [{ PlayerId: '205' }, 'GameSessionId is required'], + [{ PlayerId: '205', GameSessionId: 'nope' }, 'GameSessionId is required'], + [{ GameSessionId: String(SESSION) }, 'PlayerId is required'], + ] as Array<[Record, string]>) { + const res = await modKick(fields) + expect(res.status, error).toBe(400) + expect(await res.json()).toEqual({ success: false, error }) + } + }) +}) + describe('vote to kick', () => { // Two live sessions, so a vote called in one can be checked against a player in the // other. Nothing reads `room_instance` here — the gate is presence alone. @@ -8474,6 +8685,7 @@ describe('openapi', () => { 'POST /api/PlayerReporting/v1/instantKick', 'POST /api/PlayerReporting/v1/moderationBlockDetails', 'POST /api/PlayerReporting/v1/referee', + 'POST /api/PlayerReporting/v1/roomModKick', 'POST /api/PlayerReporting/v3/create', 'POST /api/PlayerReporting/v3/voteToKick', 'POST /api/avatar/v1/lockeditems/bulk', diff --git a/apps/auth/src/auth.app.ts b/apps/auth/src/auth.app.ts index 7901483..09126f5 100644 --- a/apps/auth/src/auth.app.ts +++ b/apps/auth/src/auth.app.ts @@ -72,8 +72,8 @@ 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 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 + * The `error_description` a SIGNUP is refused with when it shares an identity with a banned + * account (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 @@ -81,7 +81,9 @@ const TOKEN_SCOPE = * 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. + * Only `create_account` is refused this way. SIGNING IN is never refused for a ban, its own + * or a linked one — see the token grant: the client needs a token to reach the block screen, + * and `match` is what holds a blocked player to their dorm. */ const BLOCKED_DESCRIPTION = 'this device or network is blocked' @@ -651,22 +653,25 @@ 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.** A BANNED account still gets a token — every grant, including a refresh.', + '**Bans.** A BLOCKED account still gets a token — every grant, including a refresh,', + 'and whether the ban is its own or reaches it through the evasion arms.', '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.', + 'enforced by `match`, which refuses every matchmake but the caller’s own dorm, so a', + 'token gets them as far as that screen and no further.', '', - '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.', + 'Ban EVASION — an account sharing a PROVEN platform identity (a `platform_account`', + 'link) or an IP (`signupIp`/`lastLoginIp`, or the address this request came from)', + 'with a banned one — signs in the same way, and is stopped at matchmaking like any', + 'other blocked account. Those two arms are the operator’s `BAN_EVASION_MATCH` knob', + '(`ip`, `platform`, or `off`).', + '', + 'What IS refused here (`invalid_grant`) is a `create_account` carrying either link,', + 'refused BEFORE it mints anything: making a NEW account to evade with is a different', + 'act from signing into one. The description is deliberately vague — the signup', + 'refused may belong to a housemate of the banned player rather than to them.', ].join('\n'), requestBody: form( TokenRequest, @@ -1033,33 +1038,36 @@ const app = new Hono() await setLoginContext(c.env.DB, resolvedId, { deviceId, deviceClass, ip: clientIp }) } - // 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. + // EVERY account gets its token, banned or not — a ban of its own, or a link to + // somebody else's through the evasion arms. The client needs one to reach `api`'s + // moderationBlockDetails, which is where a banned player is TOLD they are banned + // (category, time left, "Rule violation"); refused here they would only ever see a + // sign-in that fails, with nothing saying why. The ban is enforced by `match`, which + // refuses every matchmake but the caller's own dorm, so a token gets them as far as + // the block screen and no further. // - // 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. + // That includes ban EVASION, which this used to refuse. An evader signing in lands + // in their dorm and can enter no room — the same place the ban leaves them — and + // refusing the token instead only ever produced a client stuck on a failed login. + // `create_account` is still refused before it mints anything (above): that is making + // a NEW account to evade with, which is a different act from signing into one. + // + // Logged either way, and the arm is logged with it: "banned" and "shares a network + // with somebody banned" are very different things to be reading in a log. + // + // 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. // // The request's own IP and proven identity are passed alongside the account, so a - // ban also reaches an old, clean account logged into from the banned player's - // device or network — the stored ips alone would only catch that on the SECOND - // login. create_account was already refused before it minted anything (above); - // this still runs for it, so a signup that raced one is refused too. + // ban also reaches an old, clean account logged into from the banned player's device + // or network — the stored ips alone would only catch that on the SECOND login. const ban = await resolveBan(c.env.DB, Number(accountId), { identity: { ip: clientIp, platform: verifiedPlatform, platformId: verifiedPlatformId }, arms: banEvasionMatch(c.env.BAN_EVASION_MATCH), }) - if (ban && ban.via !== 'account') { - logger.info('token refused: ban evasion', { + if (ban) { + logger.info('token issued to a blocked account', { accountId, grantType, via: ban.via, @@ -1067,15 +1075,6 @@ const app = new Hono() reportId: ban.ban.id, banExpires: ban.ban.ban_expires, }) - 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 2f50f74..dd6dad2 100644 --- a/apps/auth/src/test/integration/api.test.ts +++ b/apps/auth/src/test/integration/api.test.ts @@ -1662,12 +1662,12 @@ 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. 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. +// The ban follows the player past the account it was written on. SIGNING IN is not where +// that is enforced: a linked account gets its token like any other, and `match` holds it to +// its own dorm — refusing the token only ever left the client stuck on a failed login. What +// IS refused here is a SIGNUP carrying either link, before it mints anything: making a new +// account to evade with is a different act from signing into one. 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 = {}) => { @@ -1686,38 +1686,38 @@ describe('ban evasion at the token endpoint', () => { const login = (id: number, ip?: string) => postToken(`account_id=${id}&password=${LOGIN_PASSWORD}`, ip) - test('an account sharing a banned account’s platform identity cannot log in', async () => { + test('an account sharing a banned account’s platform identity still signs in', async () => { await account(6301, 'EvaderOne') await linkPlatformIdentity(env.DB, 6301, 0, 'steam-tokenevader') await banAccount(6301) await account(6302, 'EvaderTwo') await linkPlatformIdentity(env.DB, 6302, 0, 'steam-tokenevader') + // A token, like anyone else: the client has to load in before it can be told anything, + // and `match` is what keeps this account out of every room but its dorm. const res = await login(6302) - expect(res.status).toBe(400) - // A vaguer sentence than a direct ban: this account may belong to somebody else. - expect(res.json.error_description).toBe('this device or network is blocked') + expect(res.status).toBe(200) + expect(decodePayload(res.json.access_token as string).sub).toBe('6302') }) - test('an account sharing a banned account’s IP cannot log in', async () => { + test('an account sharing a banned account’s IP still signs in', async () => { await account(6303, 'SameHouseBanned', { signupIp: '203.0.113.30' }) await banAccount(6303) await account(6304, 'SameHouseClean', { signupIp: '203.0.113.30' }) - const res = await login(6304) - expect(res.status).toBe(400) - expect(res.json.error_description).toBe('this device or network is blocked') + expect((await login(6304)).status).toBe(200) }) - // The address the request arrives from counts, so an account that never logged in - // from the banned network before is caught on the first attempt rather than the second. - test('the request’s own IP is matched even when the account has none stored', async () => { + // The address the request arrives from counts, so a SIGNUP from the banned network is + // caught on the first attempt rather than the second. + test('the request’s own IP is matched at signup even when the account has none stored', async () => { await account(6305, 'BannedAtHome', { signupIp: '203.0.113.31' }) await banAccount(6305) await account(6306, 'CleanElsewhere') - expect((await login(6306, '203.0.113.31')).status).toBe(400) - // The same account from any other network signs in normally. + expect((await postToken('grant_type=create_account', '203.0.113.31')).status).toBe(400) + // Signing in from that same network is not refused — only minting a new account is. + expect((await login(6306, '203.0.113.31')).status).toBe(200) expect((await login(6306, '198.51.100.31')).status).toBe(200) }) @@ -1746,7 +1746,8 @@ describe('ban evasion at the token endpoint', () => { expect(res.status).toBe(200) }) - // The knob an operator reaches for when the IP arm locks out real players. + // The knob an operator reaches for when the IP arm locks out real players. It governs the + // linked arms wherever they are applied — at signup here, and at every matchmake in `match`. 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' }) @@ -1758,16 +1759,22 @@ describe('ban evasion at the token endpoint', () => { try { env.BAN_EVASION_MATCH = 'platform' - expect((await login(6321)).status).toBe(200) - expect((await login(6322)).status).toBe(400) - // And signup from that network is open again. + // Signup from the banned IP is open again, since that arm is off... expect((await postToken('grant_type=create_account', '203.0.113.50')).status).toBe(200) + // ...while the platform arm still refuses one from the banned headset. + expect( + (await postToken('grant_type=create_account&platform=0&platform_id=steam-knobevader')) + .status + ).toBe(400) - env.BAN_EVASION_MATCH = 'off' - expect((await login(6322)).status).toBe(200) - // 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) + // Signing in is not what the knob gates: every one of these gets a token whatever it + // says — the housemate, the evader, and the banned account itself. + for (const arms of ['platform', 'off']) { + env.BAN_EVASION_MATCH = arms + for (const id of [6320, 6321, 6322]) { + expect((await login(id)).status, `${arms}/${id}`).toBe(200) + } + } } finally { env.BAN_EVASION_MATCH = original } diff --git a/apps/match/src/context.ts b/apps/match/src/context.ts index 72403e5..bdad737 100644 --- a/apps/match/src/context.ts +++ b/apps/match/src/context.ts @@ -91,7 +91,15 @@ export type Env = SharedHonoEnv & { } /** Variables can be extended */ -export type Variables = SharedHonoVariables +export type Variables = SharedHonoVariables & { + /** + * Set by the `/matchmake/*` ban gate when it lets a BANNED account through to its own + * dorm. Only `/matchmake/dorm` and `/matchmake/none` are let through, and `none` reads + * this to answer the dorm rather than whatever instance the caller's presence names — + * which for a stale row is a public room the ban must keep them out of. + */ + bannedToDorm?: boolean +} export interface App extends HonoApp { Bindings: Env diff --git a/apps/match/src/match.app.ts b/apps/match/src/match.app.ts index 8191a94..0aa6c15 100644 --- a/apps/match/src/match.app.ts +++ b/apps/match/src/match.app.ts @@ -1527,6 +1527,23 @@ async function playerDormInstance(c: Context, accountId: number): Promise /^\/matchmake\/(dorm|none)\/?$/.test(path) + const app = new Hono() .use( '*', @@ -1555,6 +1572,26 @@ const app = new Hono() // only — it's the one refusal the client renders as "you are banned" instead of a room // that mysteriously fails to load, and it's what the enum offers. // + // ONE exception: a BLOCKED player may still matchmake into their own DORM. + // `auth` already issues them a token, because the client has to be signed in to reach + // `api`'s moderationBlockDetails — the screen that tells them they are banned, for how long + // and why. Refused every matchmake, the client never finishes loading in and never draws + // that screen, so a banned player saw nothing but a game that would not start. Their dorm + // is private to them and reachable by nobody else, so letting them sit in it costs nothing + // and is where they read the ban. Everything else — every public room, club, event, + // instance, invite and follow — is still refused, which is the ban. + // + // Only the two load-in calls, `/matchmake/dorm` and `/matchmake/none` — which one the + // client makes depends on the build. `dorm` resolves the CALLER'S dorm and nothing else; + // `none` would otherwise hand back whatever instance their presence names, so for a banned + // caller its handler answers the dorm outright rather than trusting a stale row. + // + // Ban EVASION gets the dorm too. `auth` signs those accounts in like any other, so refusing + // them the dorm would leave the client stuck on load-in — the exact failure this exception + // exists to fix — and they are held to it by the same gate. They see no block screen (that + // describes the caller's OWN ban, and they have none), but a player sitting in their dorm + // unable to enter any room is where a ban leaves them either way. + // // Unauthenticated requests fall through untouched: the route's own `authedId` answers // 401, which mustn't turn into "banned" just because the token was missing. .use('/matchmake/*', async (c, next) => { @@ -1564,7 +1601,7 @@ const app = new Hono() identity: { ip: c.req.header('cf-connecting-ip') }, arms: banEvasionMatch(c.env.BAN_EVASION_MATCH), }) - if (match) { + if (match && !isDormMatchmake(c.req.path)) { logger.info('matchmake refused: player banned', { accountId: id, via: match.via, @@ -1573,6 +1610,16 @@ const app = new Hono() }) return matchmakeResult(c, BANNED_FROM_ROOM, null) } + if (match) { + // Tells `/matchmake/none` to answer the dorm rather than the caller's presence. + c.set('bannedToDorm', true) + logger.info('blocked player allowed into their own dorm', { + accountId: id, + via: match.via, + reportId: match.ban.id, + banExpires: match.ban.ban_expires, + }) + } } await next() }) @@ -2763,7 +2810,10 @@ const app = new Hono() const id = await authedId(c) if (id === null) return unauthorized(c) - const presence = await getPresence(c.env.DB, id) + // A BANNED caller gets their dorm whatever their presence says. The ban gate let them + // this far so the client can finish loading in and draw the block screen; answering + // the instance a stale presence row names would walk them back into a public room. + const presence = c.get('bannedToDorm') ? null : await getPresence(c.env.DB, id) const current = presence?.roomInstance ?? (await playerDormInstance(c, id)) await enterRoom(c, id, current) return matchmakeResult(c, 0, current) diff --git a/apps/match/src/test/integration/api.test.ts b/apps/match/src/test/integration/api.test.ts index b5b1fbf..091d567 100644 --- a/apps/match/src/test/integration/api.test.ts +++ b/apps/match/src/test/integration/api.test.ts @@ -3349,14 +3349,13 @@ describe('account bans', () => { headers: await bearer(player), }) - test('every matchmake route is refused for a banned account', async () => { + test('every matchmake route but the load-in ones is refused for a banned account', async () => { await banAccount(6001) // One live instance of room 2 and one club membership, so each route would // otherwise have somewhere to put them. for (const path of [ '/matchmake/room/2', '/matchmake/room/77/34', - '/matchmake/dorm', '/matchmake/club/4', '/matchmake/player/9701', '/matchmake/instance/1', @@ -3367,6 +3366,94 @@ describe('account bans', () => { } }) + // The client has to finish loading in to draw the block screen that tells the player they + // are banned — `auth` issues them a token for exactly that reason. Refused their own dorm + // too, they never get that far and just see a game that will not start. + test('a banned account may still matchmake into its own dorm', async () => { + await banAccount(6008) + const res = await matchmake('/matchmake/dorm', '6008') + expect(res.status).toBe(200) + const body = (await res.json()) as { + errorCode: number + roomInstance: { roomInstanceId: number; roomId: number } | null + } + expect(body.errorCode).toBe(0) + expect(body.roomInstance).not.toBeNull() + + // Their OWN dorm, the same one every time — not a room anyone else can reach. + const again = (await (await matchmake('/matchmake/dorm', '6008')).json()) as { + roomInstance: { roomInstanceId: number } | null + } + expect(again.roomInstance?.roomInstanceId).toBe(body.roomInstance!.roomInstanceId) + + // And it is only the dorm — the ban is otherwise untouched. + expect(await (await matchmake('/matchmake/room/2', '6008')).json()).toEqual(refused(55)) + }) + + // `/matchmake/none` is the other load-in call, and which one the client makes depends on + // the build. It normally answers the instance the caller's presence names — so for a banned + // caller it must answer the DORM instead, or a stale presence row walks them straight back + // into the public room they were standing in. + test('a banned account’s /matchmake/none answers the dorm, not their stale presence', async () => { + await banAccount(6011) + // Standing in a live instance of room 2, the way a player banned mid-session is. + const joined = (await (await matchmake('/matchmake/room/2', '9701')).json()) as { + roomInstance: { roomInstanceId: number; roomId: number } + } + await env.DB.prepare('INSERT OR REPLACE INTO presence (data) VALUES (?1)') + .bind( + JSON.stringify({ + accountId: 6011, + roomInstance: joined.roomInstance, + expiresAt: Math.floor(Date.now() / 1000) + 900, + }) + ) + .run() + + const body = (await (await matchmake('/matchmake/none', '6011')).json()) as { + errorCode: number + roomInstance: { roomInstanceId: number; roomId: number } | null + } + expect(body.errorCode).toBe(0) + expect(body.roomInstance).not.toBeNull() + // NOT the room they were standing in. + expect(body.roomInstance?.roomInstanceId).not.toBe(joined.roomInstance.roomInstanceId) + expect(body.roomInstance?.roomId).not.toBe(joined.roomInstance.roomId) + // It is their dorm — the same instance `/matchmake/dorm` answers. + const dorm = (await (await matchmake('/matchmake/dorm', '6011')).json()) as { + roomInstance: { roomInstanceId: number } | null + } + expect(dorm.roomInstance?.roomInstanceId).toBe(body.roomInstance?.roomInstanceId) + }) + + // An evader signs in like anyone else (`auth` issues them a token), so they must be able to + // finish loading in — otherwise the client hangs on a failed login, which is the failure the + // dorm exception exists to fix. They are held to the dorm by the same gate. + test('an account caught by ban evasion gets the dorm and nothing else', async () => { + const linkTo = async (id: number, platformId: string) => + env.DB.prepare( + `INSERT OR IGNORE INTO platform_account (account_id, platform, platform_id, linked_at) + VALUES (?1, 0, ?2, ?3)` + ) + .bind(id, platformId, new Date().toISOString()) + .run() + await linkTo(6009, 'steam-dorm-evader') + await banAccount(6009) + // The replacement account: different id, same headset. + await linkTo(6010, 'steam-dorm-evader') + + const dorm = (await (await matchmake('/matchmake/dorm', '6010')).json()) as { + errorCode: number + roomInstance: unknown + } + expect(dorm.errorCode).toBe(0) + expect(dorm.roomInstance).not.toBeNull() + + // Every room is still closed to them — which is the ban. + expect(await (await matchmake('/matchmake/room/2', '6010')).json()).toEqual(refused(55)) + expect(await (await matchmake('/matchmake/club/4', '6010')).json()).toEqual(refused(55)) + }) + // The refusal is the ban's, not the room's: nothing is entered, so no presence is // written and the player stays where they were (nowhere). test('a refused matchmake leaves no presence behind', async () => { diff --git a/apps/rooms/migrations/0018_room_ban_reason_expiry.sql b/apps/rooms/migrations/0018_room_ban_reason_expiry.sql new file mode 100644 index 0000000..32bdf00 --- /dev/null +++ b/apps/rooms/migrations/0018_room_ban_reason_expiry.sql @@ -0,0 +1,13 @@ +-- A room ban can now carry a reason and lapse on its own. +-- +-- Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) — keep in sync. +-- +-- `reason` is the issuer's free text, NULL when none was given. `expires_at` is ISO-8601 UTC +-- (the same format `created_at` is written in, so the two compare as strings), and NULL is a +-- permanent ban — which is what every row written before this migration is, so no backfill. +-- +-- A lapsed row is not deleted when it lapses: nothing runs at that moment. Every read that +-- asks whether a player is banned filters on `expires_at IS NULL OR expires_at > now` instead, +-- and the next ban or unban of that player overwrites or removes the stale row. +ALTER TABLE room_ban ADD COLUMN reason TEXT; +ALTER TABLE room_ban ADD COLUMN expires_at TEXT; diff --git a/apps/rooms/migrations/0019_room_ban_history.sql b/apps/rooms/migrations/0019_room_ban_history.sql new file mode 100644 index 0000000..e44f38f --- /dev/null +++ b/apps/rooms/migrations/0019_room_ban_history.sql @@ -0,0 +1,30 @@ +-- Room bans that have ENDED, so `GET /rooms/{roomId}/bans/history` has a past to report. +-- +-- Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) — keep in sync. +-- +-- `room_ban` holds only the ban in force: one row per (room, player), DELETEd on unban and +-- overwritten on re-ban. Everything that asks "is this player banned" — `match` among them — +-- reads that table and is unchanged. This table is append-only and written at the moment a +-- ban stops being the live one: +-- +-- * lifted by an unban -> status 2 (Lifted), ended_at = the unban, unbanned_by_account_id set +-- * lapsed, then cleared -> status 1 (Elapsed), ended_at = its expiry, unbanned_by_account_id NULL +-- (by an unban of the stale row, or by a new ban replacing it) +-- +-- A ban still in force that is re-issued is AMENDED in place, not ended, so it writes nothing. +-- Nothing is backfilled: bans lifted before this migration were deleted and left no record. +CREATE TABLE IF NOT EXISTS room_ban_history ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + room_id INTEGER NOT NULL, + banned_player_id INTEGER NOT NULL, + ban_mask INTEGER NOT NULL DEFAULT 0, + banned_by_account_id INTEGER NOT NULL, + created_at TEXT NOT NULL, + reason TEXT, + expires_at TEXT, + status INTEGER NOT NULL, + ended_at TEXT, + unbanned_by_account_id INTEGER + ); +CREATE INDEX IF NOT EXISTS idx_room_ban_history_player + ON room_ban_history (room_id, banned_player_id); diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index b47547a..989d3d8 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -440,6 +440,39 @@ export const IsBannedPascalEnvelope = z.object({ error_id: z.string().nullable().describe('Null. Lowercase, unlike its three siblings'), }) +/** + * One ban in `GET /rooms/{roomId}/bans/history`. Seven keys, in the client's order — derived + * members before base ones, so `AccountId` is fifth. + */ +export const RoomBanRecordDto = z.object({ + Status: z.int().describe('0 Active · 1 Elapsed (ran out) · 2 Lifted (unbanned early)'), + UnbannedByAccountId: z.int().nullable().describe('Who lifted it; null unless Lifted'), + BanEndTime: z + .string() + .nullable() + .describe( + 'ISO 8601 UTC. Active: scheduled expiry, null when permanent. Elapsed: its expiry. Lifted: when it was lifted' + ), + Reason: z.string().describe('`""` when no reason was given — never null'), + AccountId: z.int().describe('The banned player'), + BannedByAccountId: z.int().nullable().describe('Who issued the ban'), + BanStartTime: z.string().describe('ISO 8601 UTC, when the ban was issued'), +}) + +/** `GET /rooms/{roomId}/bans/history` — the PascalCase envelope, `error_id` lowercase. */ +export const RoomBanHistoryEnvelope = z.object({ + Value: z + .object({ + ActiveBan: RoomBanRecordDto.nullable().describe('The ban in force, or null'), + PreviousBans: RoomBanRecordDto.array().describe('Every ban that has ended, newest first'), + }) + .nullable() + .describe('Null only when `id` is missing or not a number'), + Success: z.boolean(), + Error: z.string().nullable(), + error_id: z.string().nullable().describe('Null. Lowercase, unlike its three siblings'), +}) + /** The bare JSON string the bulk lookups answer when the id list is over the cap. */ export const TooManyLookupIds = z .string() @@ -608,6 +641,16 @@ export const BanRequest = z.object({ .string() .optional() .describe('Stored verbatim; meaning unknown — the client sends `0`. Defaults to 0'), + reason: z + .string() + .optional() + .describe('Free text, stored on the ban and shown in the kick message'), + durationMinutes: z + .string() + .optional() + .describe( + 'Minutes until the ban lapses. Absent, empty or `0` is permanent; anything that is not a non-negative whole number is refused' + ), }) /** A stored room ban — what `POST /rooms/{roomId}/bans` answers in `value`. */ @@ -617,6 +660,8 @@ export const RoomBanDto = z.object({ BanMask: z.int(), BannedByAccountId: z.int().describe('Who issued the ban'), CreatedAt: z.string(), + Reason: z.string().nullable().describe('Null when no reason was given'), + ExpiresAt: z.string().nullable().describe('ISO 8601 UTC when the ban lapses; null is permanent'), }) /** @@ -630,6 +675,23 @@ export const RoomBanEntryDto = z.object({ banStartTime: z.string().describe('ISO 8601 UTC, when the ban was issued'), }) +/** `DELETE /rooms/{roomId}/bans/bulk` — the players whose bans to lift. */ +export const UnbanBulkRequest = z.object({ + id: z + .union([z.string(), z.array(z.string())]) + .describe('Account id(s) to unban — repeated `id=` fields, or one comma-separated value'), + banMask: z.string().optional().describe('Sent by the client; ignored'), +}) + +/** What the bulk unban answers — the ban envelope, with `value` the removed bans. */ +export const RoomBansRemovedEnvelope = z.object({ + success: z.boolean(), + error: z.string().describe('Empty on success'), + value: RoomBanDto.array() + .nullable() + .describe('The bans removed, in request order; null on a rejection'), +}) + /** The envelope the ban write answers — same shape as the room writes, `value` is the ban. */ export const RoomBanEnvelope = z.object({ success: z.boolean(), diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index b9ddf73..1919abe 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -15,6 +15,7 @@ import { countRoomsByCreator, createNotification, createSubRoom, + deletePresence, deleteRoom, deleteRoomLeaderboard, deleteSubRoom, @@ -31,6 +32,7 @@ import { getPresence, getPublicRoomsByCreator, getRecommendedRooms, + getRoomBanHistory, getRoomBans, getRoomById, getRoomByName, @@ -48,6 +50,7 @@ import { MessageType, modifySubRoom, publishSubRoomSave, + refreshInstanceFullness, removeCheer, removeFavorite, removeRoomRole, @@ -124,6 +127,8 @@ import { RoleRequest, RoomBanEntryDto, RoomBanEnvelope, + RoomBanHistoryEnvelope, + RoomBansRemovedEnvelope, RoomDto, RoomEnvelope, RoomExperience, @@ -151,11 +156,12 @@ import { UNAUTHORIZED_EMPTY, UNAUTHORIZED_ENVELOPE, UNAUTHORIZED_RESPONSE, + UnbanBulkRequest, WarningRequest, } from './openapi' import type { Context } from 'hono' -import type { RoomBan, RoomPermission } from '@repo/domain' +import type { RoomBan, RoomBanRecord, RoomPermission } from '@repo/domain' import type { MessageReceivedPayload } from '../../notify/src/notification-payloads' import type { App } from './context' @@ -615,43 +621,109 @@ async function pushRoleInvite( const REPORT_CATEGORY_MODERATOR = -1 /** - * Eject a player from the room they're in — a `ModerationKick` push (id 22), the frame - * the client acts on to remove someone. Sent on a ban: the row keeps them out of future - * matchmakes, this gets them out of the instance they're in right now. + * Take a freshly banned player out of the room's LIVE PRESENCE, so the ban takes effect on + * everything that reads presence right now rather than when their row next expires. The kick + * push ejects their client, but until the row goes they still count toward the instance's + * player total (keeping a full room full), still show as standing in the room to friends, and + * still pass the presence-based grants — reading the room's staged saves among them. * - * The payload is the client's moderation shape, camelCase, in wire order: - * `reportCategory`, `duration`, `gameSessionId`, `isHostKick`, `message`, - * `playerIdReporter`, `isBan`, `isVoiceModAutoban`. `duration` is 0 (a room ban has no - * expiry — it's lifted by DELETE, not by time) and `gameSessionId` is 0 (nothing here - * tracks one). + * Only when their presence puts them in THIS room. A player banned from a room they are not + * standing in keeps their presence: a ban here is no reason to knock them offline from wherever + * they actually are. The same rule the instant kick in `api` applies to its instance. * - * `isHostKick` says the room's HOST ejected the player, as opposed to the room - * majority vote-kicking them. There is no vote-kick path yet, so the only false case - * here is a staff moderator acting in a room they don't host. `playerIdReporter` is - * whoever caused it — the host today, and the player who started the vote once - * vote-kicks exist (those will carry `reportCategory` 10 and `isHostKick` false). + * The instance they left gets its fullness recomputed, as every other presence removal does, so + * a room they were filling opens back up. Best-effort like the push: the ban row is already + * committed, and a failure here must not turn a ban that happened into an error response. * - * Like {@link pushRoomUpdate}, hub failures are logged and swallowed: the ban row has - * already committed, so a hub hiccup must not fail the request. + * Returns the instance they were removed from, or null when they were not standing in the + * room — which is what decides whether they get a kick frame at all. + */ +async function evictBannedPlayer(c: Context, ban: RoomBan): Promise { + try { + const instance = (await getPresence(c.env.DB, ban.BannedPlayerId))?.roomInstance + if (instance?.roomId !== ban.RoomId) return null + await deletePresence(c.env.DB, ban.BannedPlayerId) + if (instance.roomInstanceId == null) return null + await refreshInstanceFullness(c.env.DB, instance.roomInstanceId) + return instance.roomInstanceId + } catch (err) { + logger.error('failed to clear a banned player’s presence', { + playerId: ban.BannedPlayerId, + roomId: ban.RoomId, + error: err instanceof Error ? err.message : String(err), + }) + return null + } +} + +/** An ISO timestamp as the client's ban record carries it: UTC, to the second. */ +function toBanTime(iso: string): string { + return iso.replace(/\.\d+Z$/, 'Z') +} + +/** + * One ban as the history route serves it. Seven keys, and the ORDER is the client's: the + * derived members before the base ones, so `AccountId` is fifth. `Reason` is a non-nullable + * string on the client, so a ban issued without one is `""`, not null. + */ +function toBanRecordDto(ban: RoomBanRecord) { + return { + Status: ban.Status, + UnbannedByAccountId: ban.UnbannedByAccountId, + BanEndTime: ban.EndedAt === null ? null : toBanTime(ban.EndedAt), + Reason: ban.Reason ?? '', + AccountId: ban.AccountId, + BannedByAccountId: ban.BannedByAccountId, + BanStartTime: toBanTime(ban.StartedAt), + } +} + +/** + * Eject a room-banned player from the instance they are standing in — a `ModerationKick` + * frame (id 22), the one the client acts on to leave a room. The `room_ban` row is what keeps + * them out afterwards: `match` refuses their matchmakes into this room with `BannedFromRoom` + * (55). This frame only gets them out of the session they are in right now. + * + * It must NOT look like an account ban, and it once did. `IsBan: true` on this frame is what + * the client's moderation screen reads as a ban from the GAME — it is exactly what the staff + * account ban (`www`) sends — so a room ban sent that way showed players a game-wide ban + * screen, and with a non-zero `duration` a timed one, when nothing but one room was closed to + * them. So, like `api`'s instant kick: + * + * - `isBan` false and `duration` 0. The frame is a kick; the ban is server-side. + * - EPHEMERAL, never queued. A queued frame is delivered on the player's next connect, which + * would eject them from some unrelated session later, or greet them with it on login. + * - Sent only when they were standing in THIS room — `gameSessionId` is the instance they were + * evicted from. A player banned from a room they are not in has nothing to be ejected from + * and hears nothing; they meet the ban as a refused matchmake if they try to join. + * + * The payload is the client's moderation shape, camelCase, in wire order. `isHostKick` says the + * room's HOST ejected them rather than a room vote-kick; the only false case today is a staff + * moderator acting in a room they do not host. `playerIdReporter` is whoever caused it. + * + * Best-effort: the ban row has already committed, so a hub hiccup must not fail the request. */ async function pushRoomBan( c: Context, ban: RoomBan, + gameSessionId: number, roomName: string, isHostKick: boolean ): Promise { try { - await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayer( - ban.BannedPlayerId, + await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayersEphemeral( + [ban.BannedPlayerId], NotificationType.ModerationKick, { reportCategory: REPORT_CATEGORY_MODERATOR, duration: 0, - gameSessionId: 0, + gameSessionId, isHostKick, - message: `You have been banned from ${roomName}.`, + message: ban.Reason + ? `You have been banned from ${roomName}. Reason: ${ban.Reason}` + : `You have been banned from ${roomName}.`, playerIdReporter: ban.BannedByAccountId, - isBan: true, + isBan: false, isVoiceModAutoban: false, } ) @@ -2401,7 +2473,98 @@ const app = new Hono() } ) - // Ban a player from a room (form body `id` + `banMask`). Auth-gated (401), then + // One player's ban history in one room (`?id=` is the player). The owner's view of + // whether someone is banned now and how often they have been before — so gated like the + // ban list: auth (401), then the room's owner/co-owner or a staff token (403). + // + // Answers the PascalCase `{ Value, Success, Error, error_id }` envelope the unprefixed + // isBanned uses — `error_id` lowercase among its PascalCase siblings — with `Value` exactly + // `{ ActiveBan, PreviousBans }`. The record's seven keys are in the client's order: the + // derived members (`Status`, `UnbannedByAccountId`, `BanEndTime`, `Reason`) come BEFORE the + // base ones (`AccountId`, `BannedByAccountId`, `BanStartTime`), which is why `AccountId` + // is fifth. Built key by key in that order rather than by spreading a domain object. + .get( + '/rooms/:roomId{[0-9]+}/bans/history', + describeRoute({ + tags: ['Room settings'], + summary: 'A player’s ban history in a room', + description: [ + 'Whether `id` is banned from the room right now, and every ban of theirs there that has', + 'ended. `Value` has exactly two keys: `ActiveBan` (the ban in force, or null) and', + '`PreviousBans` (newest first).', + '', + 'Each record is seven keys in this order: `Status` (0 Active, 1 Elapsed, 2 Lifted),', + '`UnbannedByAccountId` (who lifted it; null unless Lifted), `BanEndTime`, `Reason`', + '(`""` when none was given — never null), `AccountId`, `BannedByAccountId`,', + '`BanStartTime`. `BanEndTime` is the scheduled expiry of an Active ban (null for a', + 'permanent one), the expiry of an Elapsed ban, and the moment a Lifted ban was lifted.', + 'Timestamps are ISO 8601 UTC to the second.', + '', + 'A timed ban that has run out is not active: it is reported as an Elapsed previous ban.', + 'A ban re-issued while still in force is AMENDED, not ended, so it appears once, with', + 'its latest reason and times. History starts with this server’s ban-history migration;', + 'bans lifted before it left no record.', + '', + 'Gated like the ban list: the room’s creator or a co-owner, or an account whose token', + 'carries the `developer` / `moderator` role. An unknown room answers an empty history.', + 'A missing or non-numeric `id` answers `Success: false` with a null `Value`.', + ].join('\n'), + security: AUTHED, + parameters: [ + roomIdParam, + { + name: 'id', + in: 'query', + required: true, + description: 'The account whose ban history to read', + schema: { type: 'string', pattern: '^[0-9]+$' }, + }, + ], + responses: { + 200: json(RoomBanHistoryEnvelope, 'The player’s active ban and previous bans'), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) return unauthorized(c) + + const playerId = Number.parseInt(c.req.query('id') ?? '', 10) + if (Number.isNaN(playerId)) { + return c.json({ + Value: null, + Success: false, + Error: 'You must provide a valid player!', + error_id: null, + }) + } + + const roomId = Number.parseInt(c.req.param('roomId'), 10) + const room = await getRoomById(c.env.DB, roomId) + // No room → no bans, the same answer the ban list gives, so this is not a way to + // probe which room ids exist. + if (room && !canManageRoom(room, accountId) && !(await isStaff(c))) { + return c.body(null, 403) + } + const history = room + ? await getRoomBanHistory(c.env.DB, roomId, playerId) + : { active: null, previous: [] } + + return c.json({ + Value: { + ActiveBan: history.active ? toBanRecordDto(history.active) : null, + PreviousBans: history.previous.map(toBanRecordDto), + }, + Success: true, + Error: null, + error_id: null, + }) + } + ) + + // Ban a player from a room (form body `id` + `banMask`, optionally `reason` and + // `durationMinutes`). Auth-gated (401), then // gated to the room's owner/co-owner OR a staff token (403). One row per // (room, player) — re-banning rewrites it, so the call is idempotent. .post( @@ -2423,16 +2586,25 @@ const app = new Hono() '`banMask` is stored verbatim and nothing interprets it — the client sends `0` and', 'what it selects is not known yet. It defaults to 0 when absent.', '', - 'The BANNED player (not the caller) gets a `ModerationKick` push (id 22) — the frame', - 'the client acts on to eject someone — so a ban takes effect immediately rather than', - 'only at their next matchmake. `isBan` is true, `duration` 0 (a room ban has no', - 'expiry; it is lifted by DELETE, not by time) and `reportCategory` -1 (Moderator).', + '`reason` is optional free text, stored on the ban and appended to the kick message.', + '`durationMinutes` makes the ban lapse on its own; absent, empty or `0` is permanent.', + 'A duration that is present but not a non-negative whole number is a rejection, never', + 'read as permanent. A lapsed ban stops counting everywhere — matchmaking, `isBanned`,', + 'the ban list — without anyone lifting it. Re-banning REPLACES the reason and', + 'duration and restarts the clock; it does not extend the remaining time.', '', - '`isHostKick` means the room’s HOST ejected them rather than the room majority', - 'vote-kicking them; with no vote-kick path yet the only false case is a staff', - 'moderator acting in a room they do not host. `playerIdReporter` is whoever caused', - 'it — the host today, the player who started the vote once vote-kicks exist. The hub', - 'queues the frame if they are offline.', + 'If the banned player’s live presence puts them in THIS room, that presence row is', + 'deleted and the instance’s fullness recomputed, so they stop counting as standing in', + 'the room immediately. Presence in some other room is left alone.', + '', + 'A banned player standing in the room also gets a `ModerationKick` frame (id 22),', + 'the frame the client acts on to leave, with `gameSessionId` the instance they were', + 'removed from. It is a KICK, not a ban, as far as the client is concerned: `isBan` is', + 'false and `duration` 0, because `isBan: true` is what the client shows as a ban from', + 'the whole game. The ban itself is the row, enforced when `match` refuses them the room', + '(`BannedFromRoom`, 55). The frame is ephemeral, never queued, and a player not in the', + 'room gets none. `isHostKick` is false only for a staff moderator acting in a room they', + 'do not host; `playerIdReporter` is whoever issued the ban.', '', 'Answers the same lowercase `{ success, error, value }` envelope the room writes use,', 'but `value` is the BAN, not the room — a ban is not part of the room the client', @@ -2475,10 +2647,27 @@ const app = new Hono() // Absent or unparseable → 0, the value the client sends. const banMask = Number.parseInt(str(body.banMask), 10) || 0 - const ban = await banPlayerFromRoom(c.env.DB, roomId, bannedPlayerId, banMask, accountId) - // The banned player is told, not the caller — their client acts on the kick. - const roomName = typeof room.Name === 'string' ? room.Name : 'this room' - await pushRoomBan(c, ban, roomName, isHostKick) + // Absent or empty is a PERMANENT ban. A duration that is present but is not a + // non-negative whole number is refused rather than read as permanent: a typo turning + // "30 minutes" into "forever" is the one mistake here that is worse than failing. + const rawDuration = str(body.durationMinutes).trim() + const durationMinutes = rawDuration === '' ? 0 : Number(rawDuration) + if (!Number.isInteger(durationMinutes) || durationMinutes < 0) { + return banEnvelope(c, null, 'You must provide a valid ban duration!') + } + + const ban = await banPlayerFromRoom(c.env.DB, roomId, bannedPlayerId, banMask, accountId, { + reason: str(body.reason), + durationMinutes, + }) + // Out of the room's presence first, so by the time their client acts on the kick + // nothing server-side still counts them as standing in it. Only a player who WAS + // standing in the room is kicked — anyone else has nothing to be ejected from. + const evictedFrom = await evictBannedPlayer(c, ban) + if (evictedFrom !== null) { + const roomName = typeof room.Name === 'string' ? room.Name : 'this room' + await pushRoomBan(c, ban, evictedFrom, roomName, isHostKick) + } return banEnvelope(c, ban) } ) @@ -2581,6 +2770,79 @@ const app = new Hono() } ) + // Lift bans in BULK (form body `id`, repeated or comma-separated, plus the `banMask` the + // client sends and nothing reads). Same gate as issuing one: auth-gated (401), then the + // room's owner/co-owner OR a staff token (403). Declared before the `/:playerId` route + // only for reading order — `bulk` is not a number, so the two never compete for a path. + .delete( + '/rooms/:roomId{[0-9]+}/bans/bulk', + describeRoute({ + tags: ['Room settings'], + summary: 'Unban players from a room in bulk', + description: [ + 'Removes the `room_ban` row of every player named by `id`, so they can matchmake into', + 'the room again. `id` may repeat (`id=205&id=206`) or be comma-separated; `banMask`', + 'is accepted because the client sends it, and ignored — it described the ban, not the', + 'unban. The body is form-encoded, on a DELETE.', + '', + 'Gated exactly like issuing a ban: the room’s creator or a co-owner, or an account', + 'whose token carries the `developer` / `moderator` role.', + '', + 'Unlike the single-player DELETE, a player who is not banned is SKIPPED rather than', + 'failing the call — one stale id in a list must not sink the rest. Only a body naming', + 'no valid id at all is a rejection. A ban that has already lapsed counts as not banned.', + '', + 'Answers the ban envelope with `value` the ARRAY of bans actually removed, in the order', + 'asked; empty when none of the players named were banned. No notification is pushed.', + 'This shape is unverified against the real service.', + ].join('\n'), + security: AUTHED, + parameters: [roomIdParam], + requestBody: form(UnbanBulkRequest, 'The players to unban'), + responses: { + 200: json( + RoomBansRemovedEnvelope, + 'The removed bans, or a rejection with `success: false`' + ), + 401: UNAUTHORIZED_RESPONSE, + 403: FORBIDDEN_RESPONSE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) return unauthorized(c) + + const roomId = Number.parseInt(c.req.param('roomId'), 10) + const room = await getRoomById(c.env.DB, roomId) + if (!room) return banEnvelope(c, null, 'This room does not exist!') + if (!canManageRoom(room, accountId) && !(await isStaff(c))) return c.body(null, 403) + + // `all: true` keeps a repeated `id` from collapsing to its last value, which would + // unban one player out of the list and report success. + const body = await c.req + .parseBody({ all: true }) + .catch(() => ({}) as Record) + const ids = [ + ...new Set( + [body.id] + .flat() + .filter((v): v is string => typeof v === 'string') + .flatMap((v) => v.split(',')) + .map((v) => Number.parseInt(v.trim(), 10)) + .filter((n) => !Number.isNaN(n)) + ), + ] + if (ids.length === 0) return banEnvelope(c, null, 'You must provide a valid player to unban!') + + const removed: RoomBan[] = [] + for (const playerId of ids) { + const ban = await unbanPlayerFromRoom(c.env.DB, roomId, playerId, accountId) + if (ban) removed.push(ban) + } + return c.json({ success: true, error: '', value: removed }) + } + ) + // Lift a player's ban on a room. Same gate as issuing one: auth-gated (401), then the // room's owner/co-owner OR a staff token (403). .delete( @@ -2617,7 +2879,7 @@ const app = new Hono() if (!canManageRoom(room, accountId) && !(await isStaff(c))) return c.body(null, 403) const playerId = Number.parseInt(c.req.param('playerId'), 10) - const removed = await unbanPlayerFromRoom(c.env.DB, roomId, playerId) + const removed = await unbanPlayerFromRoom(c.env.DB, roomId, playerId, accountId) if (!removed) return banEnvelope(c, null, 'This player is not banned from this room!') return banEnvelope(c, removed) } diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index 4858d2d..4ef3275 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -2507,8 +2507,154 @@ describe('rooms endpoints', () => { await env.DB.prepare('DELETE FROM room_leaderboard WHERE room_id IN (2, 3)').run() }) - it('POST /rooms/:id/bans kicks the banned player', async () => { - type Sent = { playerId: number; notificationType: string | number; data: unknown } + it('POST /rooms/:id/bans takes a reason and a duration', async () => { + type Ban = { Reason: string | null; ExpiresAt: string | null; CreatedAt: string } + type Sent = { playerIds: number[]; data: { duration: number; isBan: boolean; message: string } } + const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') + await hub().fetch('http://do/all', { method: 'DELETE' }) + // Standing in the room, so there is a session to kick them out of. + await putInRoom(310, 2) + + // The client's body, verbatim. + const res = await postForm( + '/rooms/2/bans', + { banMask: '0', id: '310', reason: 'test', durationMinutes: '30' }, + '1' + ) + const body = (await envOf(res)) as unknown as { success: boolean; value: Ban } + expect(body).toMatchObject({ success: true, value: { BannedPlayerId: 310, Reason: 'test' } }) + // Lapses thirty minutes after it was issued, to the second. + expect(Date.parse(body.value.ExpiresAt!) - Date.parse(body.value.CreatedAt)).toBe(30 * 60_000) + + // The kick carries the reason in its message — but NOT the ban's length, and is not a + // ban frame: a duration with `isBan` is what the client shows as a timed GAME ban. + const [kick] = (await (await hub().fetch('http://do/all')).json()) as Sent[] + expect(kick.playerIds).toEqual([310]) + expect(kick.data).toMatchObject({ duration: 0, isBan: false }) + expect(kick.data.message).toMatch(/Reason: test$/) + + // Neither field is required: no duration is a permanent ban, and it still pushes 0. + const permanent = (await envOf( + await postForm('/rooms/2/bans', { banMask: '0', id: '311' }, '1') + )) as unknown as { value: Ban } + expect(permanent.value).toMatchObject({ Reason: null, ExpiresAt: null }) + for (const d of ['', '0']) { + const v = (await envOf( + await postForm('/rooms/2/bans', { id: '312', durationMinutes: d }, '1') + )) as unknown as { value: Ban } + expect(v.value.ExpiresAt, d).toBeNull() + } + + // A re-ban REPLACES the duration and reason — it neither extends nor keeps the old ones. + const rebanned = (await envOf( + await postForm('/rooms/2/bans', { id: '310' }, '1') + )) as unknown as { value: Ban } + expect(rebanned.value).toMatchObject({ Reason: null, ExpiresAt: null }) + }) + + it('POST /rooms/:id/bans deletes the banned player’s presence in that room', async () => { + const presenceOf = (accountId: number) => + env.DB.prepare('SELECT room_id FROM presence WHERE account_id = ?1') + .bind(accountId) + .first<{ room_id: number }>() + const instanceId = 1000002 // what `putInRoom` puts room 2's players in + const isFull = async () => + ( + await env.DB.prepare('SELECT is_full FROM room_instance WHERE id = ?1') + .bind(instanceId) + .first<{ is_full: number }>() + )?.is_full + + // A one-seat instance of room 2 that 320 is filling. + await env.DB.prepare('INSERT INTO room_instance (data) VALUES (?1)') + .bind( + JSON.stringify({ + roomInstanceId: instanceId, + roomId: 2, + subRoomId: 2, + maxCapacity: 1, + isFull: true, + }) + ) + .run() + await putInRoom(320, 2) + // 321 is standing in a DIFFERENT room. + await putInRoom(321, 4) + + expect((await postForm('/rooms/2/bans', { banMask: '0', id: '320' }, '1')).status).toBe(200) + // Gone from the room they were banned from, and the seat they held is free again. + expect(await presenceOf(320)).toBeNull() + expect(await isFull()).toBe(0) + + // A ban from room 2 does not knock someone offline from room 4. + expect((await postForm('/rooms/2/bans', { banMask: '0', id: '321' }, '1')).status).toBe(200) + expect(await presenceOf(321)).toEqual({ room_id: 4 }) + + await clearPresence(321) + await env.DB.prepare('DELETE FROM room_instance WHERE id = ?1').bind(instanceId).run() + }) + + it('POST /rooms/:id/bans refuses a duration it cannot read, rather than banning forever', async () => { + for (const durationMinutes of ['thirty', '-5', '1.5']) { + expect( + await envOf(await postForm('/rooms/2/bans', { id: '313', durationMinutes }, '1')), + durationMinutes + ).toMatchObject({ success: false, error: 'You must provide a valid ban duration!' }) + } + const row = await env.DB.prepare( + 'SELECT 1 FROM room_ban WHERE room_id = 2 AND banned_player_id = 313' + ).first() + expect(row).toBeNull() + }) + + it('a lapsed room ban stops counting everywhere without being lifted', async () => { + // Written straight to the table: an hour-long ban issued two hours ago. + const issued = new Date(Date.now() - 2 * 3_600_000) + await env.DB.prepare( + `INSERT INTO room_ban + (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, expires_at) + VALUES (2, 314, 0, 1, ?1, 'old', ?2)` + ) + .bind(issued.toISOString(), new Date(issued.getTime() + 3_600_000).toISOString()) + .run() + + // `isBanned` — the same check `match` refuses matchmakes on. + const isBanned = await SELF.fetch(`${ORIGIN}/rooms/2/bans/314/isBanned`, { + headers: await bearer('314'), + }) + expect(await isBanned.json()).toMatchObject({ Value: false }) + + // Not in the ban list. + const list = (await ( + await SELF.fetch(`${ORIGIN}/rooms/2/bans`, { headers: await bearer('1') }) + ).json()) as Array<{ accountId: number }> + expect(list.some((b) => b.accountId === 314)).toBe(false) + + // Unbanning it is "not banned", like any other player who isn't. + const unban = await SELF.fetch(`${ORIGIN}/rooms/2/bans/314`, { + method: 'DELETE', + headers: await bearer('1'), + }) + expect(await unban.json()).toMatchObject({ success: false }) + + // A ban still in force DOES count — the filter is on the expiry, not on having one. + expect( + (await postForm('/rooms/2/bans', { id: '315', durationMinutes: '30' }, '1')).status + ).toBe(200) + const stillBanned = await SELF.fetch(`${ORIGIN}/rooms/2/bans/315/isBanned`, { + headers: await bearer('315'), + }) + expect(await stillBanned.json()).toMatchObject({ Value: true }) + }) + + it('POST /rooms/:id/bans kicks a banned player out of the room — without banning them from the game', async () => { + type Sent = { + playerId?: number + playerIds?: number[] + ephemeral?: boolean + notificationType: string | number + data: unknown + } const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') const sentSince = async (): Promise => (await (await hub().fetch('http://do/all')).json()) as Sent[] @@ -2516,27 +2662,30 @@ describe('rooms endpoints', () => { // The room's current name, read rather than hardcoded — earlier tests rename it. const { Name } = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as { Name: string } + await putInRoom(207, 2) await hub().fetch('http://do/all', { method: 'DELETE' }) expect((await postForm('/rooms/2/bans', { banMask: '0', id: '207' }, '1')).status).toBe(200) - // A ModerationKick (id 22) to the BANNED player, not the caller — it ejects them - // from the instance they're in now; the row keeps them out of future matchmakes. + // One EPHEMERAL ModerationKick to the banned player, out of the instance they were in. // Asserted against the enum rather than a literal: the ids are notify's to change. expect(await sentSince()).toEqual([ { - playerId: 207, + playerIds: [207], + ephemeral: true, notificationType: NotificationType.ModerationKick, // The client's moderation payload, camelCase, in wire order. data: { reportCategory: -1, // Moderator — a person acted, not the system - duration: 0, // a room ban has no expiry - gameSessionId: 0, - // The host ejected them (as opposed to a room vote-kick, which doesn't - // exist yet). Account 1 owns RecCenter, so it hosts it. + duration: 0, + gameSessionId: 1000002, // the instance `putInRoom` stood them in + // Account 1 owns RecCenter, so it hosts it. isHostKick: true, message: `You have been banned from ${Name}.`, playerIdReporter: 1, - isBan: true, + // FALSE. `isBan: true` is what the client shows as a ban from the whole game + // — the account ban's frame. A room ban is enforced by the row, when `match` + // refuses them the room; this frame only ejects them from the session. + isBan: false, isVoiceModAutoban: false, }, }, @@ -2544,14 +2693,26 @@ describe('rooms endpoints', () => { // A staff moderator doesn't host the room, so it isn't a host kick — and // `playerIdReporter` is still whoever caused it. + await putInRoom(208, 2) await hub().fetch('http://do/all', { method: 'DELETE' }) expect( (await postForm('/rooms/2/bans', { id: '208' }, '999', ['gameClient', 'moderator'])).status ).toBe(200) expect((await sentSince())[0]).toMatchObject({ - playerId: 208, - data: { isHostKick: false, playerIdReporter: 999 }, + playerIds: [208], + data: { isHostKick: false, playerIdReporter: 999, isBan: false }, }) + + // A player who is NOT in the room has nothing to be kicked out of, and hears NOTHING — + // not now, and not queued for their next connect, which is how a room ban once greeted + // players with a ban screen on login. + await putInRoom(216, 4) + await hub().fetch('http://do/all', { method: 'DELETE' }) + expect((await postForm('/rooms/2/bans', { id: '216' }, '1')).status).toBe(200) + expect((await postForm('/rooms/2/bans', { id: '217' }, '1')).status).toBe(200) + expect(await sentSince()).toEqual([]) + + for (const id of [216]) await clearPresence(id) }) it('GET /rooms/:id/bans lists the room’s bans, under the same gate', async () => { @@ -2586,6 +2747,258 @@ describe('rooms endpoints', () => { expect(await (await list('/rooms/99999/bans', '1')).json()).toEqual([]) }) + it('GET /rooms/:id/bans/history serves the client’s exact shape', async () => { + const history = async (playerId: number) => + (await ( + await SELF.fetch(`${ORIGIN}/rooms/2/bans/history?id=${playerId}`, { + headers: await bearer('1'), + }) + ).json()) as { + Value: { + ActiveBan: Record | null + PreviousBans: Array> + } + Success: boolean + Error: string | null + error_id: string | null + } + + expect( + (await postForm('/rooms/2/bans', { banMask: '0', id: '340', reason: 'test' }, '1')).status + ).toBe(200) + const body = await history(340) + + // The envelope, PascalCase with a lowercase `error_id`, and a Value of exactly two keys. + expect(Object.keys(body)).toEqual(['Value', 'Success', 'Error', 'error_id']) + expect(body).toMatchObject({ Success: true, Error: null, error_id: null }) + expect(Object.keys(body.Value)).toEqual(['ActiveBan', 'PreviousBans']) + + // Seven keys in the CLIENT'S order — derived members first, so AccountId is fifth. + const active = body.Value.ActiveBan! + expect(Object.keys(active)).toEqual([ + 'Status', + 'UnbannedByAccountId', + 'BanEndTime', + 'Reason', + 'AccountId', + 'BannedByAccountId', + 'BanStartTime', + ]) + expect(active).toMatchObject({ + Status: 0, // Active + UnbannedByAccountId: null, + BanEndTime: null, // permanent + Reason: 'test', + AccountId: 340, + BannedByAccountId: 1, + }) + // UTC to the second, as the client's example carries it. + expect(active.BanStartTime).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/) + expect(body.Value.PreviousBans).toEqual([]) + + // A ban with no reason is `""`, never null; a timed one carries its scheduled end. + expect( + (await postForm('/rooms/2/bans', { id: '341', durationMinutes: '30' }, '1')).status + ).toBe(200) + const timed = (await history(341)).Value.ActiveBan! + expect(timed.Reason).toBe('') + expect(Date.parse(timed.BanEndTime as string) - Date.parse(timed.BanStartTime as string)).toBe( + 30 * 60_000 + ) + + // Someone never banned has no history at all. + expect((await history(349)).Value).toEqual({ ActiveBan: null, PreviousBans: [] }) + }) + + it('GET /rooms/:id/bans/history keeps lifted and elapsed bans, newest first', async () => { + type Rec = { + Status: number + UnbannedByAccountId: number | null + BanEndTime: string | null + Reason: string + } + const history = async (playerId: number) => + ( + (await ( + await SELF.fetch(`${ORIGIN}/rooms/2/bans/history?id=${playerId}`, { + headers: await bearer('1'), + }) + ).json()) as { Value: { ActiveBan: Rec | null; PreviousBans: Rec[] } } + ).Value + const unban = async (playerId: number, sub: string) => + SELF.fetch(`${ORIGIN}/rooms/2/bans/${playerId}`, { + method: 'DELETE', + headers: await bearer(sub), + }) + + // LIFTED: banned by the owner, unbanned early by the co-owner. + expect((await postForm('/rooms/2/bans', { id: '342', reason: 'first' }, '1')).status).toBe(200) + expect((await unban(342, '2')).status).toBe(200) + let h = await history(342) + expect(h.ActiveBan).toBeNull() + expect(h.PreviousBans).toHaveLength(1) + expect(h.PreviousBans[0]).toMatchObject({ Status: 2, UnbannedByAccountId: 2, Reason: 'first' }) + expect(h.PreviousBans[0].BanEndTime).not.toBeNull() + + // Banned and lifted again: two previous bans, the newest first. + expect((await postForm('/rooms/2/bans', { id: '342', reason: 'second' }, '1')).status).toBe(200) + expect((await unban(342, '1')).status).toBe(200) + h = await history(342) + expect(h.PreviousBans.map((b) => b.Reason)).toEqual(['second', 'first']) + + // ELAPSED: a one-hour ban issued two hours ago, never touched since. It is not active, + // and it is already reported as a previous ban before anything files it. + const issued = new Date(Date.now() - 2 * 3_600_000) + const expired = new Date(issued.getTime() + 3_600_000).toISOString() + await env.DB.prepare( + `INSERT INTO room_ban + (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, expires_at) + VALUES (2, 343, 0, 1, ?1, 'lapsed', ?2)` + ) + .bind(issued.toISOString(), expired) + .run() + h = await history(343) + expect(h.ActiveBan).toBeNull() + expect(h.PreviousBans).toEqual([ + expect.objectContaining({ Status: 1, UnbannedByAccountId: null, Reason: 'lapsed' }), + ]) + + // A new ban replaces the stale row — and FILES the lapsed one, exactly once, rather than + // losing it or counting it twice. + expect((await postForm('/rooms/2/bans', { id: '343', reason: 'again' }, '1')).status).toBe(200) + h = await history(343) + expect(h.ActiveBan).toMatchObject({ Status: 0, Reason: 'again' }) + expect(h.PreviousBans).toEqual([ + expect.objectContaining({ Status: 1, UnbannedByAccountId: null, Reason: 'lapsed' }), + ]) + // Elapsed at its expiry, not at the moment it was replaced. + expect(Date.parse(h.PreviousBans[0].BanEndTime!)).toBe( + Math.floor(Date.parse(expired) / 1000) * 1000 + ) + + // Re-issuing a ban still in force AMENDS it: one active ban, nothing filed. + expect((await postForm('/rooms/2/bans', { id: '344', reason: 'v1' }, '1')).status).toBe(200) + expect((await postForm('/rooms/2/bans', { id: '344', reason: 'v2' }, '2')).status).toBe(200) + h = await history(344) + expect(h.ActiveBan).toMatchObject({ Reason: 'v2' }) + expect(h.PreviousBans).toEqual([]) + + // The bulk unban files a lift the same way the single one does. + await SELF.fetch(`${ORIGIN}/rooms/2/bans/bulk`, { + method: 'DELETE', + headers: { 'content-type': 'application/x-www-form-urlencoded', ...(await bearer('1')) }, + body: 'banMask=0&id=344', + }) + h = await history(344) + expect(h.ActiveBan).toBeNull() + expect(h.PreviousBans).toEqual([ + expect.objectContaining({ Status: 2, UnbannedByAccountId: 1, Reason: 'v2' }), + ]) + }) + + it('GET /rooms/:id/bans/history is gated like the ban list', async () => { + const get = async (path: string, sub?: string, roles?: string[]) => + SELF.fetch(`${ORIGIN}${path}`, { headers: sub ? await bearer(sub, roles) : {} }) + expect((await postForm('/rooms/2/bans', { id: '345' }, '1')).status).toBe(200) + + expect((await get('/rooms/2/bans/history?id=345')).status).toBe(401) + expect((await get('/rooms/2/bans/history?id=345', '999')).status).toBe(403) + // The co-owner, and a staff token with no role on the room. + for (const [sub, roles] of [ + ['2', undefined], + ['999', ['gameClient', 'moderator']], + ] as const) { + const res = await get('/rooms/2/bans/history?id=345', sub, roles as string[] | undefined) + expect(res.status, sub).toBe(200) + expect( + ((await res.json()) as { Value: { ActiveBan: unknown } }).Value.ActiveBan + ).not.toBeNull() + } + + // An unknown room is an empty history, not an error. + expect(await (await get('/rooms/99999/bans/history?id=345', '999')).json()).toEqual({ + Value: { ActiveBan: null, PreviousBans: [] }, + Success: true, + Error: null, + error_id: null, + }) + // A missing or non-numeric id. + for (const q of ['', '?id=nope']) { + expect(await (await get(`/rooms/2/bans/history${q}`, '1')).json(), q).toEqual({ + Value: null, + Success: false, + Error: 'You must provide a valid player!', + error_id: null, + }) + } + }) + + it('DELETE /rooms/:id/bans/bulk removes the named bans, under the same gate', async () => { + type Removed = { + success: boolean + error: string + value: Array<{ BannedPlayerId: number }> | null + } + const unban = async (body: string, sub?: string, roles?: string[]) => + SELF.fetch(`${ORIGIN}/rooms/2/bans/bulk`, { + method: 'DELETE', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + ...(sub ? await bearer(sub, roles) : {}), + }, + body, + }) + const banned = async (playerId: number) => + (await env.DB.prepare('SELECT 1 FROM room_ban WHERE room_id = 2 AND banned_player_id = ?1') + .bind(playerId) + .first()) !== null + for (const id of ['330', '331', '332', '333']) { + expect((await postForm('/rooms/2/bans', { id }, '1')).status).toBe(200) + } + + // No token → 401; a valid token with no standing in the room → 403. Nothing removed. + expect((await unban('banMask=0&id=330')).status).toBe(401) + expect((await unban('banMask=0&id=330', '999')).status).toBe(403) + expect(await banned(330)).toBe(true) + + // The client's body, verbatim, from the owner. + const one = await unban('banMask=0&id=330', '1') + expect(one.status).toBe(200) + expect((await one.json()) as Removed).toMatchObject({ + success: true, + error: '', + value: [{ BannedPlayerId: 330 }], + }) + expect(await banned(330)).toBe(false) + + // Repeated `id` lifts every one of them — not just the last — from a co-owner, and a player + // who is not banned (330, already lifted) is skipped rather than failing the others. + const many = (await (await unban('banMask=0&id=331&id=330&id=332', '2')).json()) as Removed + expect(many).toMatchObject({ success: true }) + expect(many.value!.map((b) => b.BannedPlayerId)).toEqual([331, 332]) + expect(await banned(331)).toBe(false) + expect(await banned(332)).toBe(false) + + // A staff token works in a room it has no role on, and a comma-separated id is read too. + expect( + (await (await unban('id=333,334', '999', ['gameClient', 'moderator'])).json()) as Removed + ).toMatchObject({ success: true, value: [{ BannedPlayerId: 333 }] }) + + // Nothing banned among the names is still a success, with nothing removed. + expect((await (await unban('id=330', '1')).json()) as Removed).toMatchObject({ + success: true, + value: [], + }) + // A body naming no valid player at all is the one rejection. + for (const body of ['banMask=0', 'id=nope']) { + expect((await (await unban(body, '1')).json()) as Removed, body).toMatchObject({ + success: false, + error: 'You must provide a valid player to unban!', + value: null, + }) + } + }) + it('DELETE /rooms/:id/bans/:playerId lifts a ban, under the same gate', async () => { const del = async (path: string, sub?: string, roles?: string[]) => SELF.fetch(`${ORIGIN}${path}`, { @@ -4654,6 +5067,7 @@ describe('rooms endpoints', () => { ) expect([...documented].sort()).toEqual([ 'DELETE /rooms/{roomId}', + 'DELETE /rooms/{roomId}/bans/bulk', 'DELETE /rooms/{roomId}/bans/{playerId}', 'DELETE /rooms/{roomId}/interactionby/me/cheer', 'DELETE /rooms/{roomId}/interactionby/me/favorite', @@ -4683,6 +5097,7 @@ describe('rooms endpoints', () => { 'GET /rooms/visitedby/{playerId}', 'GET /rooms/{roomId}', 'GET /rooms/{roomId}/bans', + 'GET /rooms/{roomId}/bans/history', 'GET /rooms/{roomId}/bans/{playerId}/isBanned', 'GET /rooms/{roomId}/experience', 'GET /rooms/{roomId}/experience/player', diff --git a/apps/rooms/vitest.config.ts b/apps/rooms/vitest.config.ts index 9cef654..8cf36f2 100644 --- a/apps/rooms/vitest.config.ts +++ b/apps/rooms/vitest.config.ts @@ -23,7 +23,9 @@ export default defineConfig({ durableObjects: { RECFLARE_NOTIFICATIONS_HUB: 'NotificationsHub' }, // notifyPlayer records every call so tests can assert the notifications the // worker pushed (type + payload). GET /all for the whole list, DELETE to - // reset it between assertions. + // reset it between assertions. notifyPlayersEphemeral is recorded too, with + // `playerIds` and `ephemeral: true`: whether a frame is queued for an offline + // player is worth asserting — a queued ModerationKick lands on their next login. script: ` import { DurableObject } from 'cloudflare:workers' export class NotificationsHub extends DurableObject { @@ -32,6 +34,10 @@ export default defineConfig({ this.sent.push({ playerId, notificationType, data }) return { delivered: 0, queued: true } } + async notifyPlayersEphemeral(playerIds, notificationType, data) { + this.sent.push({ playerIds, ephemeral: true, notificationType, data }) + return { delivered: 0 } + } async broadcast() { return { delivered: 0 } } async fetch(request) { if (request.method === 'DELETE') { diff --git a/apps/www/src/staff.ts b/apps/www/src/staff.ts index f7b0474..f09732b 100644 --- a/apps/www/src/staff.ts +++ b/apps/www/src/staff.ts @@ -8,6 +8,7 @@ import { validateAndGetAccountId, validateAndGetRoles } from '@repo/jwt' // module comment below. import { banEvasionMatch, linkedAccounts } from '../../api/src/bans-db' import { + banBlockDetails, banFromReport, createReport, getActiveBan, @@ -20,11 +21,10 @@ import { import { getWarningsAgainst } from '../../api/src/warnings-db' // The notification ids and the kick frame's recovered shape, owned by `notify`. Both are // imported as values/types with no runtime dependencies. -import { KickReportCategory } from '../../notify/src/notification-payloads' import { NotificationType } from '../../notify/src/notification-types' import type { Context, MiddlewareHandler } from 'hono' -import type { ReportSearch } from '../../api/src/reports-db' +import type { ReportRow, ReportSearch } from '../../api/src/reports-db' import type { ModerationKickPayload } from '../../notify/src/notification-payloads' import type { App, Env } from './context' @@ -152,46 +152,59 @@ function banExpiry( } /** - * Throw the banned player out of whatever instance they are standing in. + * Tell a player they have been banned from the GAME, and throw them out of wherever they are. * - * Without this a ban only takes effect on the player's NEXT matchmake: `match` refuses a - * banned player, but nothing revisits a session already in progress, so someone banned - * mid-session keeps playing until they leave on their own. + * Without this a ban only takes effect on the player's NEXT sign-in or matchmake: `match` + * refuses a banned player and `moderationBlockDetails` blocks them at login, but nothing + * revisits a session already in progress, so someone banned mid-session keeps playing. * - * Mirrors `api`'s instant kick — presence row deleted (they read offline at once and the - * instance frees a slot), instance fullness recomputed, and a `ModerationKick` frame (id - * 22) pushed EPHEMERALLY. `IsBan` is true here, which is what makes the client's screen - * name it as a ban rather than a kick, and an offline player needs no frame: they meet the - * ban at `moderationBlockDetails` when they next sign in. + * The frame is a `ModerationKick` (id 22) with `IsBan: true` — the frame the client's + * moderation screen shows as a ban from the game. This is the ONE place that frame belongs: a + * ROOM ban must never send it (`rooms` sends `IsBan: false` and is enforced by refusing the + * room), because the client cannot tell the two apart and shows a game-wide ban screen. * - * Entirely best-effort. The ban row is already committed by the time this runs; a hub - * hiccup or a missing presence must not fail a ban that has been handed down. + * Its contents are {@link banBlockDetails} — the same `ReportCategory`, `Message` and the + * `Duration`/`TimeoutStartedAt` pair `moderationBlockDetails` answers for this ban — so the + * screen a player sees mid-session is the screen they see when they next sign in, not a + * generic "banned" with no length. + * + * Sent to anyone ONLINE, not only to a player standing in an instance: someone in a menu is + * as banned as someone in a room. `GameSessionId` is their instance, or 0 when they are in + * none. EPHEMERAL, because an offline player needs no frame — they meet the same screen at + * `moderationBlockDetails` when they next sign in — and a queued one would fire again then. + * + * Their presence row is deleted (they read offline at once), and the instance they were in, + * if any, has its fullness recomputed so a full room frees a slot. + * + * Entirely best-effort. The ban row is already committed by the time this runs; a hub hiccup + * or a missing presence must not fail a ban that has been handed down. */ -async function kickBannedPlayer(c: Context, playerId: number, moderatorId: number) { +async function kickBannedPlayer(c: Context, report: ReportRow, moderatorId: number) { + const playerId = report.reported_player_id try { const presence = (await getPresences<{ roomInstanceId?: number }>(c.env.DB, [playerId])).get( playerId ) - const gameSessionId = presence?.roomInstance?.roomInstanceId - // Not online, or online but not in an instance (the lobby) — nothing to eject them - // from. Their presence row still goes, so they read offline immediately. - if (presence) await deletePresence(c.env.DB, playerId) - if (gameSessionId === undefined) return + // Offline: nothing to eject and nobody to tell. The sign-in check has them. + if (!presence) return - await refreshInstanceFullness(c.env.DB, gameSessionId) + await deletePresence(c.env.DB, playerId) + const gameSessionId = presence.roomInstance?.roomInstanceId + if (gameSessionId !== undefined) await refreshInstanceFullness(c.env.DB, gameSessionId) + const block = banBlockDetails(report) const frame: ModerationKickPayload = { - ReportCategory: KickReportCategory.Moderator, - Duration: 0, - GameSessionId: gameSessionId, + ReportCategory: block.ReportCategory, + Duration: block.Duration, + GameSessionId: gameSessionId ?? 0, IsHostKick: false, - Message: 'You have been banned.', + Message: block.Message, PlayerIdReporter: null, IsBan: true, IsVoiceModAutoban: false, IsWarning: false, VoteKickReason: '', - TimeoutStartedAt: null, + TimeoutStartedAt: block.TimeoutStartedAt, } await c.env.RECFLARE_NOTIFICATIONS_HUB.getByName(HUB_INSTANCE).notifyPlayersEphemeral( [playerId], @@ -199,7 +212,7 @@ async function kickBannedPlayer(c: Context, playerId: number, moderatorId: { ...frame } ) } catch (err) { - logger.error('failed to kick a banned player out of their instance', { + logger.error('failed to tell a banned player they are banned', { playerId, moderatorId, error: err instanceof Error ? err.message : String(err), @@ -346,7 +359,7 @@ export async function banReportHandler(c: Context) { banExpires: report.ban_expires, }) - if (banned) await kickBannedPlayer(c, report.reported_player_id, moderatorId) + if (banned) await kickBannedPlayer(c, report, moderatorId) return c.json(report) } diff --git a/apps/www/src/test/integration/api.test.ts b/apps/www/src/test/integration/api.test.ts index 9854f13..e72cf4d 100644 --- a/apps/www/src/test/integration/api.test.ts +++ b/apps/www/src/test/integration/api.test.ts @@ -937,6 +937,16 @@ it('throws a banned player out of the instance they are standing in', async () = expect(frames[0].data.IsBan).toBe(true) expect(frames[0].data.GameSessionId).toBe(77001) + // The SAME ban the sign-in screen describes, not a generic one: its category, message and + // the Duration/TimeoutStartedAt pair, three days from when it was handed down. + const banned = await getReportById(env.DB, report.id) + expect(frames[0].data).toMatchObject({ + ReportCategory: banned!.report_category, + Message: 'Rule violation', + TimeoutStartedAt: banned!.banned_at, + Duration: 3 * 86_400, + }) + // And their presence row is gone, so they read offline at once and the instance frees // a slot. const presence = await env.DB.prepare( @@ -945,6 +955,37 @@ it('throws a banned player out of the instance they are standing in', async () = expect(presence?.n).toBe(0) }) +// Online but not standing in any room — in a menu — is still banned from the game, and is +// told so now rather than at their next sign-in. +it('tells a banned player who is online but in no instance', async () => { + const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') + await hub().fetch('http://do/all', { method: 'DELETE' }) + + await setPresence(env.DB, { + accountId: 8195, + roomInstance: null, + statusVisibility: 0, + deviceClass: 0, + vrMovementMode: 0, + platform: 4, + appVersion: 'test', + }) + const report = await createReport(env.DB, { reporterPlayerId: 8196, reportedPlayerId: 8195 }) + expect( + (await staffPost(`/api/staff/reports/${report.id}/ban`, 8110, { permanent: true })).status + ).toBe(200) + + const frames = (await (await hub().fetch('http://do/all')).json()) as Array<{ + playerIds?: number[] + ephemeral?: boolean + data: Record + }> + expect(frames).toHaveLength(1) + expect(frames[0]).toMatchObject({ playerIds: [8195], ephemeral: true }) + // No session to name, and a permanent ban carries the largest duration the field holds. + expect(frames[0].data).toMatchObject({ IsBan: true, GameSessionId: 0, Duration: 2_147_483_647 }) +}) + // The ban row is committed before the kick is attempted, so a player who is offline (or // a hub that can't be reached) must not fail a ban that has been handed down. it('bans an offline player without a kick to push', async () => { diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index 23d8196..3707321 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -101,9 +101,33 @@ export const ROOM_SCHEMA_DDL: string[] = [ ban_mask INTEGER NOT NULL DEFAULT 0, banned_by_account_id INTEGER NOT NULL, created_at TEXT NOT NULL, + -- migrations/0018_room_ban_reason_expiry.sql. NULL expires_at is a permanent ban. + reason TEXT, + expires_at TEXT, PRIMARY KEY (room_id, banned_player_id) )`, `CREATE INDEX IF NOT EXISTS idx_room_ban_player ON room_ban (banned_player_id)`, + // Room bans that have ENDED (migrations/0019_room_ban_history.sql). `room_ban` above holds + // only the ban in force — one row per (room, player), deleted on unban and overwritten on + // re-ban — so without this a lifted or lapsed ban leaves no trace. A row is written here + // at the moment a ban stops being the live one: when it is lifted (`status` 2, Lifted, + // `ended_at` the lift and `unbanned_by_account_id` who lifted it) or when a lapsed ban is + // replaced by a new one (`status` 1, Elapsed, `ended_at` its expiry). Append-only. + `CREATE TABLE IF NOT EXISTS room_ban_history ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + room_id INTEGER NOT NULL, + banned_player_id INTEGER NOT NULL, + ban_mask INTEGER NOT NULL DEFAULT 0, + banned_by_account_id INTEGER NOT NULL, + created_at TEXT NOT NULL, + reason TEXT, + expires_at TEXT, + status INTEGER NOT NULL, + ended_at TEXT, + unbanned_by_account_id INTEGER + )`, + `CREATE INDEX IF NOT EXISTS idx_room_ban_history_player + ON room_ban_history (room_id, banned_player_id)`, // Per-room leaderboard definitions (migrations/0016_room_leaderboard.sql). One row per // (room, leaderboard): `leaderboard_id` is the client's slot number — small ordinals // (1, 2, 3…), unique only within the room — so the pair is the key, and re-posting a @@ -289,6 +313,10 @@ export interface RoomBan { BanMask: number BannedByAccountId: number CreatedAt: string + /** Free text from whoever issued the ban. Null when none was given. */ + Reason: string | null + /** When the ban lapses (ISO-8601 UTC). Null is a permanent ban, lifted only by DELETE. */ + ExpiresAt: string | null } interface RoomBanRow { @@ -297,6 +325,8 @@ interface RoomBanRow { ban_mask: number banned_by_account_id: number created_at: string + reason: string | null + expires_at: string | null } const toRoomBan = (row: RoomBanRow): RoomBan => ({ @@ -305,68 +335,253 @@ const toRoomBan = (row: RoomBanRow): RoomBan => ({ BanMask: row.ban_mask, BannedByAccountId: row.banned_by_account_id, CreatedAt: row.created_at, + Reason: row.reason, + ExpiresAt: row.expires_at, }) +/** + * The SQL that keeps a ban row in force: permanent, or not yet lapsed. An expired row is not + * deleted when it lapses — nothing runs at that moment — so EVERY read that asks "is this + * player banned" has to apply this, or a 30-minute ban would last forever. the time is bound + * rather than using SQLite's clock so it compares in the same ISO-8601 format the rows are + * written in (`datetime('now')` has no `T` or `Z` and would sort wrongly against them). + * `now` names the placeholder the caller binds the current time to. + */ +const banInForce = (now: string): string => `(expires_at IS NULL OR expires_at > ${now})` + +/** Where a ban stands, as the client's `Status` enum numbers it. */ +export const RoomBanStatus = { + /** In force now. Only ever the `room_ban` row. */ + Active: 0, + /** Ran out on its own. */ + Elapsed: 1, + /** Ended early by someone unbanning the player. */ + Lifted: 2, +} as const + +/** + * One ban in a player's history, whatever state it is in. Timestamps are ISO-8601 as stored. + * `EndedAt` is when the ban stopped counting — its expiry if it lapsed, the moment it was + * lifted if it was lifted — and for an Active ban the expiry it is scheduled to reach, null + * when it is permanent. + */ +export interface RoomBanRecord { + Status: (typeof RoomBanStatus)[keyof typeof RoomBanStatus] + UnbannedByAccountId: number | null + EndedAt: string | null + Reason: string | null + AccountId: number + BannedByAccountId: number + StartedAt: string +} + +interface RoomBanHistoryRow { + banned_player_id: number + banned_by_account_id: number + created_at: string + reason: string | null + status: RoomBanRecord['Status'] + ended_at: string | null + unbanned_by_account_id: number | null +} + +/** + * A player's ban history in one room: the ban in force, if any, and every ban that has ended, + * newest first. + * + * A `room_ban` row that has LAPSED is not active — but nothing moves it into + * `room_ban_history` until the player is banned or unbanned again, so it is reported here as + * an Elapsed previous ban rather than dropped. Only the row still in force is `active`. + */ +export async function getRoomBanHistory( + db: D1Database, + roomId: number, + playerId: number +): Promise<{ active: RoomBanRecord | null; previous: RoomBanRecord[] }> { + const now = new Date().toISOString() + const [current, ended] = await db.batch([ + db + .prepare('SELECT * FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2') + .bind(roomId, playerId), + db + .prepare( + `SELECT * FROM room_ban_history WHERE room_id = ?1 AND banned_player_id = ?2 + ORDER BY created_at DESC, id DESC` + ) + .bind(roomId, playerId), + ]) + + const previous = (ended.results as RoomBanHistoryRow[]).map((row): RoomBanRecord => ({ + Status: row.status, + UnbannedByAccountId: row.unbanned_by_account_id, + EndedAt: row.ended_at, + Reason: row.reason, + AccountId: row.banned_player_id, + BannedByAccountId: row.banned_by_account_id, + StartedAt: row.created_at, + })) + + const row = (current.results as RoomBanRow[])[0] + if (!row) return { active: null, previous } + const inForce = row.expires_at === null || row.expires_at > now + const record: RoomBanRecord = { + Status: inForce ? RoomBanStatus.Active : RoomBanStatus.Elapsed, + UnbannedByAccountId: null, + EndedAt: row.expires_at, + Reason: row.reason, + AccountId: row.banned_player_id, + BannedByAccountId: row.banned_by_account_id, + StartedAt: row.created_at, + } + if (inForce) return { active: record, previous } + // Newer than anything already filed, since filing it is what a later ban would do. + return { active: null, previous: [record, ...previous] } +} + +/** Options for {@link banPlayerFromRoom}. */ +export interface RoomBanOptions { + /** Shown to nobody yet; kept on the row. Empty is stored as null. */ + reason?: string | null + /** Minutes until the ban lapses. Absent, null or 0 is permanent. */ + durationMinutes?: number | null +} + /** * Ban a player from a room, returning the stored ban. One row per (room, player): - * re-banning someone already banned rewrites their row with the new mask and issuer - * rather than appending a second one, so the call is idempotent. + * re-banning someone already banned rewrites their row with the new mask, issuer, reason and + * expiry rather than appending a second one, so the call is idempotent. + * + * A re-ban REPLACES the duration rather than extending it, and the clock restarts from now: + * banning someone for 30 minutes who has 5 left leaves them with 30, and re-banning with no + * duration makes the ban permanent. The latest ban issued is the one in force. + * + * History: if the row being overwritten had already LAPSED, that ban is over and is filed in + * `room_ban_history` as Elapsed first. A ban still in force is AMENDED rather than ended — the + * player has been banned continuously — so it is overwritten without leaving a history row. + * Both statements run as one batch, so the old ban is never lost between them. */ export async function banPlayerFromRoom( db: D1Database, roomId: number, bannedPlayerId: number, banMask: number, - bannedByAccountId: number + bannedByAccountId: number, + options: RoomBanOptions = {} ): Promise { - const row = await db - .prepare( - `INSERT INTO room_ban (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at) - VALUES (?1, ?2, ?3, ?4, ?5) - ON CONFLICT(room_id, banned_player_id) DO UPDATE SET - ban_mask = ?3, banned_by_account_id = ?4, created_at = ?5 - RETURNING *` - ) - .bind(roomId, bannedPlayerId, banMask, bannedByAccountId, new Date().toISOString()) - .first() + const now = new Date() + const minutes = options.durationMinutes ?? 0 + const expiresAt = minutes > 0 ? new Date(now.getTime() + minutes * 60_000).toISOString() : null + const reason = options.reason?.trim() || null + + const [, upserted] = await db.batch([ + db + .prepare( + `INSERT INTO room_ban_history + (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, + expires_at, status, ended_at, unbanned_by_account_id) + SELECT room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, + expires_at, ?4, expires_at, NULL + FROM room_ban + WHERE room_id = ?1 AND banned_player_id = ?2 AND NOT ${banInForce('?3')}` + ) + .bind(roomId, bannedPlayerId, now.toISOString(), RoomBanStatus.Elapsed), + db + .prepare( + `INSERT INTO room_ban + (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, expires_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7) + ON CONFLICT(room_id, banned_player_id) DO UPDATE SET + ban_mask = ?3, banned_by_account_id = ?4, created_at = ?5, reason = ?6, expires_at = ?7 + RETURNING *` + ) + .bind( + roomId, + bannedPlayerId, + banMask, + bannedByAccountId, + now.toISOString(), + reason, + expiresAt + ), + ]) // RETURNING always yields the upserted row. - return toRoomBan(row!) + return toRoomBan((upserted.results as RoomBanRow[])[0]) } /** * Lift a player's ban on a room, returning the ban that was removed — or null when - * they weren't banned, which lets the caller tell a real unban from a no-op. + * they weren't banned, which lets the caller tell a real unban from a no-op. A ban that has + * already LAPSED counts as not banned; its stale row is deleted all the same. + * + * History: the removed ban is filed in `room_ban_history` in the same batch as the delete. A + * ban still in force is Lifted, ended now, by `unbannedByAccountId`. A lapsed one is Elapsed, + * ended at its expiry, by nobody — deleting the stale row is housekeeping, not a lift. */ export async function unbanPlayerFromRoom( db: D1Database, roomId: number, - bannedPlayerId: number + bannedPlayerId: number, + unbannedByAccountId: number ): Promise { - const row = await db - .prepare('DELETE FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2 RETURNING *') - .bind(roomId, bannedPlayerId) - .first() - return row ? toRoomBan(row) : null + const now = new Date().toISOString() + const inForce = banInForce('?3') + const [, deleted] = await db.batch([ + db + .prepare( + `INSERT INTO room_ban_history + (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, + expires_at, status, ended_at, unbanned_by_account_id) + SELECT room_id, banned_player_id, ban_mask, banned_by_account_id, created_at, reason, + expires_at, + CASE WHEN ${inForce} THEN ?5 ELSE ?6 END, + CASE WHEN ${inForce} THEN ?3 ELSE expires_at END, + CASE WHEN ${inForce} THEN ?4 ELSE NULL END + FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2` + ) + .bind( + roomId, + bannedPlayerId, + now, + unbannedByAccountId, + RoomBanStatus.Lifted, + RoomBanStatus.Elapsed + ), + db + .prepare('DELETE FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2 RETURNING *') + .bind(roomId, bannedPlayerId), + ]) + const row = (deleted.results as RoomBanRow[])[0] + if (!row) return null + const ban = toRoomBan(row) + return ban.ExpiresAt !== null && ban.ExpiresAt <= new Date().toISOString() ? null : ban } -/** Everyone banned from a room, most recently banned first. */ +/** Everyone CURRENTLY banned from a room, most recently banned first. Lapsed bans are left out. */ export async function getRoomBans(db: D1Database, roomId: number): Promise { const { results } = await db - .prepare('SELECT * FROM room_ban WHERE room_id = ?1 ORDER BY created_at DESC') - .bind(roomId) + .prepare( + `SELECT * FROM room_ban WHERE room_id = ?1 AND ${banInForce('?2')} ORDER BY created_at DESC` + ) + .bind(roomId, new Date().toISOString()) .all() return results.map(toRoomBan) } -/** Whether a player is banned from a room. */ +/** + * Whether a player is banned from a room RIGHT NOW. A timed ban that has lapsed is not a ban. + * This is the check `match` refuses matchmakes on, so the expiry has to be applied here. + */ export async function isPlayerBannedFromRoom( db: D1Database, roomId: number, playerId: number ): Promise { const row = await db - .prepare('SELECT 1 AS hit FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2') - .bind(roomId, playerId) + .prepare( + `SELECT 1 AS hit FROM room_ban + WHERE room_id = ?1 AND banned_player_id = ?2 AND ${banInForce('?3')}` + ) + .bind(roomId, playerId, new Date().toISOString()) .first<{ hit: number }>() return row !== null }