diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index c0cb764..cdf64d6 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -401,6 +401,23 @@ export const IsBannedEnvelope = z.object({ value: z.boolean().describe('Whether that player is banned from that room'), }) +/** + * `GET /rooms/{roomId}/bans/{playerId}/isBanned` — the same check, in the shape the client + * reads on the UNPREFIXED path. + * + * PascalCase, and deliberately not unified with {@link IsBannedEnvelope}: the two paths are + * two calls the client makes with two different decoders, and its decoder drops members it + * does not recognise silently, so a `value` served where it wants `Value` reads as `false` — + * a banned player looking unbanned — rather than as an error. `error_id` stays lowercase + * even here; that is how it comes off the wire, not a slip. + */ +export const IsBannedPascalEnvelope = z.object({ + Value: z.boolean().describe('Whether that player is banned from that room'), + Success: z.literal(true).describe('The check ran; whether the player is banned is `Value`'), + Error: z.string().nullable().describe('Null — the check itself does not fail'), + 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() @@ -437,7 +454,9 @@ export const FeaturedRoomGroupDto = z.object({ name: z.string(), StartAt: z.string(), EndAt: z.string(), - Rooms: z.array(FeaturedRoomDto).describe('Randomly ordered — no editorial curation yet'), + Rooms: z + .array(FeaturedRoomDto) + .describe('Randomly ordered, at most 10 — no editorial curation yet'), }) // ---- Envelopes ------------------------------------------------------------- diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index ac04d5e..a0431a8 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -85,6 +85,7 @@ import { InteractionDto, intQuery, IsBannedEnvelope, + IsBannedPascalEnvelope, json, jsonBody, LoadScreenRequest, @@ -361,6 +362,21 @@ async function isStaff(c: Context): Promise { return roles?.some((role) => STAFF_ROLES.has(role)) ?? false } +/** + * Whether the `:playerId` in the path is banned from the `:roomId` in it — the read behind + * both `isBanned` routes, which differ only in the envelope they wrap the answer in. + * + * Reads the same `room_ban` rows the ban writes make and `match` refuses matchmakes on, so + * the answer is what would actually happen. A room that does not exist simply has no ban + * rows and comes back false: the question is about the ban, not about the room. + */ +async function pathBan(c: Context): Promise { + // Both routes constrain these to `[0-9]+`, so neither is ever missing — the fallback is + // only here because a helper is typed against the whole app rather than one route. + const id = (name: string) => Number.parseInt(c.req.param(name) ?? '', 10) + return isPlayerBannedFromRoom(c.env.DB, id('roomId'), id('playerId')) +} + /** 401 for the auth-gated `*by/me` endpoints — no stub-account fallback. */ function unauthorized(c: Context) { return c.json({ error: 'Unauthorized' }, 401) @@ -899,8 +915,8 @@ const app = new Hono() ) // Featured rooms — a single always-active group whose `Rooms` are a randomly - // ordered set of public, non-dorm rooms. No real curation yet, so `current` - // just returns a shuffled list of eligible rooms in the featured-group shape. + // ordered set of public, non-dorm rooms, at most ten of them. No real curation yet, so + // `current` just returns a shuffled sample of eligible rooms in the featured-group shape. // // Gated on the caller's BUILD, not just their token: this payload breaks the 2023 // client — its other room listings start failing with NREs, apparently because the @@ -917,7 +933,9 @@ const app = new Hono() summary: 'Featured rooms', description: [ 'A single always-active group of featured rooms: a random shuffle of eligible public', - 'rooms, since there is no editorial curation yet.', + 'rooms, since there is no editorial curation yet. Capped at 10 rooms — a featured', + 'group is a short selection, not the whole room list — and the cap is applied after', + 'the shuffle, so each request serves a different sample.', '', 'Restricted by CLIENT BUILD. Serving this to the 2023 client breaks its other room', 'listings (NREs, apparently from the featured-room load corrupting its room cache), so', @@ -2008,12 +2026,50 @@ const app = new Hono() const accountId = await authedAccountId(c) if (accountId === null) return unauthorized(c) - const banned = await isPlayerBannedFromRoom( - c.env.DB, - Number.parseInt(c.req.param('roomId'), 10), - Number.parseInt(c.req.param('playerId'), 10) - ) - return c.json({ success: true, error: null, error_id: null, value: banned }) + return c.json({ success: true, error: null, error_id: null, value: await pathBan(c) }) + } + ) + + // The SAME check on the unprefixed path (`GET /rooms/112/bans/1/isBanned`), which is the + // spelling the client uses on the rooms host itself. Registered as its own route rather + // than as an alias because the answer is not the same bytes: this one is PascalCase + // (`Value`/`Success`/`Error`, `error_id` still lowercase), and the client's decoder drops + // members it doesn't know silently — a lowercase `value` here would read as `false` and + // show a banned player as unbanned. + // + // Same gate as its sibling: auth-gated, not owner-gated. A ban is not a secret from the + // player it stops, and the client asks this before offering a room action so it can grey + // it out rather than let the attempt fail. + .get( + '/rooms/:roomId{[0-9]+}/bans/:playerId{[0-9]+}/isBanned', + describeRoute({ + tags: ['Room settings'], + summary: 'Whether a player is banned from a room (unprefixed path)', + description: [ + 'The same `room_ban` check as the `/Room_server/` route, on the path the client uses', + 'against the rooms host directly — and in the PascalCase envelope it reads there:', + '`{ Value, Success, Error, error_id }`, with `error_id` lowercase.', + '', + 'The two envelopes are NOT unified. The client’s decoder drops members it does not', + 'recognise, so serving the other route’s lowercase `value` here would decode as', + '`false` — a banned player shown as unbanned — rather than fail.', + '', + 'Auth-gated, but any authenticated caller may ask: a ban is not a secret from the', + 'player it stops. A room that does not exist has no bans, so it answers', + '`Value: false` rather than 404ing — the check is about the ban row, not the room.', + ].join('\n'), + security: AUTHED, + parameters: [roomIdParam, bannedPlayerIdParam], + responses: { + 200: json(IsBannedPascalEnvelope, 'Whether that player is banned from that room'), + 401: UNAUTHORIZED_RESPONSE, + }, + }), + async (c) => { + const accountId = await authedAccountId(c) + if (accountId === null) return unauthorized(c) + + return c.json({ Value: await pathBan(c), Success: true, Error: null, error_id: null }) } ) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index f011773..dc3b63e 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1125,6 +1125,38 @@ describe('rooms endpoints', () => { expect(body.Rooms.some((r) => r.RoomId === 1)).toBe(false) }) + it('GET /featuredrooms/current serves at most 10 rooms', async () => { + // Seed more eligible rooms than the cap, so the length is decided by the cap rather + // than by how many rooms the suite happens to have. + const ids = Array.from({ length: 14 }, (_, i) => 30900 + i) + for (const RoomId of ids) { + await env.DB.prepare('INSERT INTO room (data) VALUES (?1)') + .bind( + JSON.stringify({ + RoomId, + Name: `Featurable${RoomId}`, + CreatorAccountId: 831, + Accessibility: 1, + IsDorm: false, + SubRooms: [], + }) + ) + .run() + } + + const res = await SELF.fetch(`${ORIGIN}/featuredrooms/current`, { + headers: await bearer('1', undefined, '20250718.01'), + }) + const body = (await res.json()) as { Rooms: Array<{ RoomId: number }> } + // A featured group is a short selection, not the whole room list. + expect(body.Rooms).toHaveLength(10) + // The cap TRIMS the shuffled list rather than sampling with replacement, so no room + // can appear twice in one group. + expect(new Set(body.Rooms.map((r) => r.RoomId)).size).toBe(10) + + await env.DB.prepare(`DELETE FROM room WHERE room_id IN (${ids.join(', ')})`).run() + }) + it('GET /featuredrooms/current withholds the group from other client builds', async () => { // The 2023 build gets the 404 it got while the route was parked — the state in which // its room listings work. Same for a token with no `rn.ver` at all. @@ -1667,6 +1699,62 @@ describe('rooms endpoints', () => { .run() }) + it('GET /rooms/:id/bans/:playerId/isBanned answers the same check, PascalCase', async () => { + const isBanned = async (roomId: number, playerId: number) => + SELF.fetch(`${ORIGIN}/rooms/${roomId}/bans/${playerId}/isBanned`, { + headers: await bearer('300'), + }) + + // Same gate as the `/Room_server/` spelling: a token is needed, any token will do. + expect((await SELF.fetch(`${ORIGIN}/rooms/112/bans/1/isBanned`)).status).toBe(401) + + const clean = await isBanned(112, 1) + expect(clean.status).toBe(200) + // PascalCase — `Value`/`Success`/`Error`, and `error_id` lowercase all the same. The + // client's decoder drops members it doesn't recognise, so the other route's lowercase + // keys here would decode as `false` rather than fail. Room 112 does not exist and the + // answer is still a clean `false`: the question is about the ban row, not the room. + expect(await clean.json()).toEqual({ + Value: false, + Success: true, + Error: null, + error_id: null, + }) + + // The same `room_ban` rows the other route and `match` read. + await env.DB.prepare( + 'INSERT INTO room_ban (room_id, banned_player_id, ban_mask, banned_by_account_id, created_at)' + + " VALUES (?1, ?2, 0, 1, '2026-01-01T00:00:00Z')" + ) + .bind(112, 1) + .run() + expect(await (await isBanned(112, 1)).json()).toEqual({ + Value: true, + Success: true, + Error: null, + error_id: null, + }) + + // Per (room, player), like every other read of these rows. + expect(await (await isBanned(113, 1)).json()).toMatchObject({ Value: false }) + expect(await (await isBanned(112, 2)).json()).toMatchObject({ Value: false }) + + // …and the prefixed route sees the same ban, in its own lowercase envelope. + const prefixed = await SELF.fetch(`${ORIGIN}/Room_server/rooms/112/bans/1/isBanned`, { + headers: await bearer('300'), + }) + expect(await prefixed.json()).toEqual({ + success: true, + error: null, + error_id: null, + value: true, + }) + + await env.DB.prepare('DELETE FROM room_ban WHERE room_id = ?1 AND banned_player_id = ?2') + .bind(112, 1) + .run() + }) + it('POST /rooms/:id/bans kicks the banned player', async () => { type Sent = { playerId: number; notificationType: string | number; data: unknown } const hub = () => env.RECFLARE_NOTIFICATIONS_HUB.getByName('global') @@ -3609,6 +3697,7 @@ describe('rooms endpoints', () => { 'GET /rooms/visitedby/{playerId}', 'GET /rooms/{roomId}', 'GET /rooms/{roomId}/bans', + 'GET /rooms/{roomId}/bans/{playerId}/isBanned', 'GET /rooms/{roomId}/experience', 'GET /rooms/{roomId}/experience/player', 'GET /rooms/{roomId}/interactionby/me', diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index c68b624..014dd59 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -2470,11 +2470,19 @@ export interface FeaturedRoomGroup { Rooms: FeaturedRoom[] } +/** How many rooms one featured group carries. See {@link getFeaturedRooms}. */ +export const FEATURED_ROOM_LIMIT = 10 + /** * Featured rooms group: public, non-dorm rooms not excluded from lists, in random * order. There's no editorial curation behind this yet, so "featured" is just a * random shuffle of the eligible rooms wrapped in a single always-active group. * Eligibility is filtered in SQL ({@link LISTABLE_WHERE}); the rest is in memory. + * + * At most {@link FEATURED_ROOM_LIMIT} rooms — a featured group is a short editorial + * selection, not the whole room list. The cap is applied AFTER the shuffle, so it is a + * random SAMPLE that varies between requests; a `LIMIT` in the SQL would instead pin the + * same handful of rooms forever and make the shuffle cosmetic. */ export async function getFeaturedRooms(db: D1Database): Promise { const { results } = await db @@ -2486,6 +2494,7 @@ export async function getFeaturedRooms(db: D1Database): Promise (typeof v === 'string' ? v : '') const num = (v: unknown): number => (typeof v === 'number' ? v : 0)