From 853050c2f072a2c3bce83e52a4aa52d0b3f5d566 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Tue, 6 Oct 2026 23:25:23 -0400 Subject: [PATCH] [rooms] fix permissions update --- apps/rooms/src/openapi.ts | 5 +- apps/rooms/src/rooms.app.ts | 32 ++++++++---- apps/rooms/src/test/integration/api.test.ts | 56 +++++++++++++++------ 3 files changed, 65 insertions(+), 28 deletions(-) diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index 1863796..a8d40f3 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -855,8 +855,9 @@ export const MoveSubRoomRequest = z.object({ * A room in the PascalCase `{ Value, Success, Error, error_id }` envelope — the same mixed * casing the unprefixed isBanned check has ({@link IsBannedPascalEnvelope}), NOT the * lowercase `{ success, error, value }` most room mutations answer. The newer settings - * routes (the subroom move, the max-player calculation mode) answer this one; the client - * decodes them with a different reader, so the two are kept apart deliberately. + * routes (the subroom move, the max-player calculation mode, the subroom permissions) + * answer this one; the client decodes them with a different reader, so the two are kept + * apart deliberately. */ export const RoomPascalEnvelope = z.object({ Value: RoomDto.nullable().describe('The room as it now stands; null on a rejection'), diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index 6f6ab73..e04d9db 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -4067,7 +4067,12 @@ const app = new Hono() // creator or a co-owner (403), the same `canManageRoom` the other room-admin writes // take; a creator-only check here locked out co-owners the client shows the screen to, // and an owner whose room names them in `Roles` rather than `CreatorAccountId`. Answers - // an EMPTY 200 — the client fires this and re-reads nothing, so there is no envelope. + // the whole ROOM under `Value` in the PascalCase `{ Value, Success, Error, error_id }` + // envelope — the same reader the subroom move and max-player mode routes feed, and the + // same Room DTO `GET /rooms/{id}` serves (the permissions themselves aren't on it). A + // rejection is HTTP 200 with `Success: false` and the message in `Error`; only a + // missing token is a 401. This once answered an empty 200, which the client's reader + // couldn't decode. .put( '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/permissions', describeRoute({ @@ -4093,33 +4098,40 @@ const app = new Hono() 'to the subroom the caller is standing in, resolved from presence.', '', 'Gated to the room’s managers — its creator or a co-owner (`canManageRoom`), as the', - 'other room-admin writes are. The response body is EMPTY: the client doesn’t read one.', + 'other room-admin writes are. Answers the whole ROOM under `Value` in the PascalCase', + '`{ Value, Success, Error, error_id }` envelope — the same Room DTO `GET /rooms/{id}`', + 'serves (the permission table itself is not part of it), and the same envelope the', + 'subroom move and `max_player_calculation_mode` answer, NOT the lowercase', + '`{ success, error, value }` the other subroom mutations use. A rejection is HTTP 200', + 'with `Success: false`, `Value: null` and the message in `Error`; only a missing', + 'token is a 401.', ].join('\n'), security: AUTHED, parameters: [roomIdParam, subRoomIdParam], requestBody: jsonBody(SubRoomPermissionsRequest, 'The permission entries to set'), responses: { - 200: { description: 'Stored (empty body)' }, - 401: UNAUTHORIZED_RESPONSE, - 403: FORBIDDEN_RESPONSE, - 404: { description: 'No such room or subroom' }, + 200: json(RoomPascalEnvelope, 'The room, or a rejection with `Success: false`'), + 401: { description: 'No bearer token (empty body)' }, }, }), async (c) => { const accountId = await authedAccountId(c) - if (accountId === null) return unauthorized(c) + if (accountId === null) return c.body(null, 401) + const refuse = (error: string) => + c.json({ Value: null, Success: false, Error: error, error_id: null }) const roomId = Number.parseInt(c.req.param('roomId'), 10) const subRoomId = Number.parseInt(c.req.param('subRoomId'), 10) // Scoped through the room so a subroom id from another room can't be written. const room = await getRoomById(c.env.DB, roomId) - if (!room || !findSubRoom(room, subRoomId)) return c.notFound() - if (!canManageRoom(room, accountId)) return c.body(null, 403) + if (!room) return refuse('This room does not exist!') + if (!findSubRoom(room, subRoomId)) return refuse('This subroom does not exist!') + if (!canManageRoom(room, accountId)) return refuse('You are not the owner of this room!') const permissions = parseRoomPermissions(await c.req.json().catch(() => null)) await setSubRoomPermissions(c.env.DB, subRoomId, permissions) - return c.body(null, 200) + return c.json({ Value: room, 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 754ea6b..cb0eda4 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -4818,42 +4818,66 @@ describe('rooms endpoints', () => { .run() } + type Envelope = { + Value: { RoomId: number; SubRooms: unknown[] } | null + Success: boolean + Error: string | null + error_id: null + } + const refusal = async (res: Response): Promise => { + expect(res.status).toBe(200) + const env = (await res.json()) as Envelope + expect(env.Success).toBe(false) + expect(env.Value).toBeNull() + expect(env.error_id).toBeNull() + return env.Error + } + it('is auth-gated and open to the room’s managers only', async () => { const body = [ { Permission: 'CAN_SAVE_INVENTIONS', Role: 30, Override: false, Type: 0, Value: 'True' }, ] // No token → 401. expect((await putPermissions('/rooms/2/subrooms/2/permissions', body)).status).toBe(401) - // A valid token that isn't the room's creator or a co-owner → 403. - expect((await putPermissions('/rooms/2/subrooms/2/permissions', body, '999')).status).toBe( - 403 - ) + // A valid token that isn't the room's creator or a co-owner → a `Success: false` + // rejection, as the other PascalCase-envelope routes answer. + expect( + await refusal(await putPermissions('/rooms/2/subrooms/2/permissions', body, '999')) + ).toBe('You are not the owner of this room!') // A co-owner may: account 2 holds Role 30 on the seeded rooms. (This body clears a // pair that was never stored, so it changes nothing.) expect((await putPermissions('/rooms/2/subrooms/2/permissions', body, '2')).status).toBe(200) // A Host (a lower tier) may not. await setRoles() - expect((await putPermissions('/rooms/2/subrooms/2/permissions', body, '748')).status).toBe( - 403 - ) - // Unknown room / unknown subroom → 404. - expect((await putPermissions('/rooms/99999/subrooms/2/permissions', body, '1')).status).toBe( - 404 - ) + expect( + await refusal(await putPermissions('/rooms/2/subrooms/2/permissions', body, '748')) + ).toBe('You are not the owner of this room!') + // Unknown room / unknown subroom. + expect( + await refusal(await putPermissions('/rooms/99999/subrooms/2/permissions', body, '1')) + ).toBe('This room does not exist!') // A subroom id belonging to another room doesn't resolve either. - expect((await putPermissions('/rooms/2/subrooms/9999/permissions', body, '1')).status).toBe( - 404 - ) + expect( + await refusal(await putPermissions('/rooms/2/subrooms/9999/permissions', body, '1')) + ).toBe('This subroom does not exist!') }) - it('answers an empty 200 — the client reads no body', async () => { + it('answers the whole room in the PascalCase `{ Value, Success, Error, error_id }` envelope', async () => { const res = await putPermissions( '/rooms/2/subrooms/2/permissions', [{ Permission: 'CAN_SPAWN_INVENTIONS', Role: 30, Override: true, Type: 0, Value: 'True' }], '1' ) expect(res.status).toBe(200) - expect(await res.text()).toBe('') + const env = (await res.json()) as Envelope + expect(env.Success).toBe(true) + expect(env.Error).toBeNull() + expect(env.error_id).toBeNull() + // `Value` is the same Room DTO `GET /rooms/2` serves — subrooms re-attached and all. + const room = (await ( + await SELF.fetch(`${ORIGIN}/rooms/2`, { headers: await bearer('1') }) + ).json()) as Envelope['Value'] + expect(env.Value).toEqual(room) }) it('a Role 0 grant reaches a visitor; a Role 30 grant does not', async () => {