From 3dd6d6420b32e295764623418a8d0402a278fb98 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Fri, 7 Aug 2026 10:48:52 -0400 Subject: [PATCH] open up room save endpoints to users with presence --- CLAUDE.md | 15 +++-- apps/rooms/src/rooms.app.ts | 51 +++++++++++---- apps/rooms/src/test/integration/api.test.ts | 72 ++++++++++++++++++--- 3 files changed, 114 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a564e84..2977988 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -99,12 +99,17 @@ inconsistency here without checking the client first. publish: no publish step exists in the client for them. Saves live in the `subroom_save` table with globally-unique ids (a bare id has to resolve — `StagedSubRoomDataSaveId` carries no subroom context), and nothing is overwritten, so - `…/saves` is real history and `publish_save` doubles as restore-a-save. `…/saves` is - auth-gated and CREATOR-only (not co-owners) — it lists unpublished staged saves. There + `…/saves` is real history and `publish_save` doubles as restore-a-save. There is no `GET …/subrooms/:sid/data`; only the POST (the room save) exists on that path. - `GET …/saves/:saveId` is the detail behind a list row, under the same creator-only gate, - but in the CAMELCASE projection the room save's response uses — not the PascalCase rows - the list serves. Three shapes of one save; keep them straight. + `GET …/saves/:saveId` is the detail behind a list row, under the same gate, but in the + CAMELCASE projection the room save's response uses — not the PascalCase rows the list + serves. Three shapes of one save; keep them straight. +- Both save reads (`rooms`: `…/saves` and `…/saves/:saveId`) are auth-gated and readable by + the room's CREATOR or by anyone whose live `presence` row puts them in that room — not by + co-owners as such (a co-owner passes only by standing there). They list unpublished + staged saves, so they aren't public; but a visitor resolves which version an instance is + running from this list, so creator-only locks them out of loading the room. The grant + expires with the presence row. - A room save writes ONLY to the subroom and its save row — never to the room. Everything the body carries describes that one revision: `Description` is the save comment shown in `…/saves`, and `PersistenceVersion`/`InventionUsage` describe the scene just saved (the diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index dc4618b..95026d5 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -159,6 +159,7 @@ const MAKER_PEN_ACCOUNT_IDS = new Set([1, 2, 3]) */ interface PresenceView { roomInstanceId?: number + roomId?: number subRoomId?: number } @@ -243,6 +244,31 @@ async function handlePhotonAccessToken(c: Context) { return c.json(photonAccessToken(accountId, instance?.roomInstanceId ?? null, overrides)) } +/** + * May this caller read the room's saves? The room's creator always may. So may anyone + * whose live presence puts them IN the room: they are already loading its scene, and the + * client resolves which version to load — the published one or the creator's latest — from + * the save list, so refusing everyone but the creator leaves a visitor unable to load what + * the instance is actually running. + * + * Presence is the shared `presence` table the `match` heartbeat maintains, so this grant + * lasts only as long as the player is actually there (rows carry an absolute expiry and + * expired ones don't read back). Co-owners get nothing extra from being co-owners — a + * co-owner standing in the room passes because of where they are, not what they hold. + * + * The presence read only happens for a non-creator, so the owner's own path stays one query. + */ +async function canReadSaves( + c: Context, + room: Record, + roomId: number, + accountId: number +): Promise { + if (room.CreatorAccountId === accountId) return true + const instance = (await getPresence(c.env.DB, accountId))?.roomInstance + return instance?.roomId === roomId +} + /** The Bearer token's account id (`sub`), or null when there's no valid token. */ async function authedAccountId(c: Context): Promise { return validateAndGetAccountId(c.req.raw, await c.env.JWT_SECRET.get()) @@ -1874,8 +1900,8 @@ const app = new Hono() // A subroom's saved-data versions — the room-history / "restore a save" list. Every // save is its own `subroom_save` row (nothing is overwritten), so this is real - // history, newest first, paged by skip/take. Auth-gated (401) and creator-only (403): - // the list exposes unpublished saves, which only the owner is entitled to see. + // history, newest first, paged by skip/take. Auth-gated (401), and readable by the + // room's creator or anyone whose presence puts them in the room (see `canReadSaves`). .get( '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/saves', describeRoute({ @@ -1887,9 +1913,11 @@ const app = new Hono() 'only when the subroom has never been saved.', '`unityAssetTarget`/`unityAssetVersion` are accepted and ignored.', '', - 'Owner-only (403 otherwise) — the list includes STAGED saves that were never', - 'published, so it is not public. It is what the client reads to offer the owner', - '“load the latest or the published version?” when they enter a private instance.', + 'The list includes STAGED saves that were never published, so it is not public:', + 'the room’s creator may read it, and so may anyone standing IN the room (their live', + 'presence says so). Anyone else is a 403. It is what the client reads to resolve', + '“load the latest or the published version?” on entering a private instance — a', + 'visitor who cannot read it cannot load what the instance is running.', '', '`TotalResults` and `TotalCount` carry the same number: the client’s paged DTO and', 'the reference disagree on the name, so both are emitted.', @@ -1920,7 +1948,7 @@ const app = new Hono() if (!room || !findSubRoom(room, subRoomId)) { return c.json({ Results: [], TotalResults: 0, TotalCount: 0 }) } - if (room.CreatorAccountId !== accountId) return c.body(null, 403) + if (!(await canReadSaves(c, room, roomId, accountId))) return c.body(null, 403) const saves = await getSubRoomSaves(c.env.DB, subRoomId) const skip = Number.parseInt(c.req.query('skip') ?? '', 10) @@ -1933,8 +1961,8 @@ const app = new Hono() ) // One of a subroom's saves by id — the detail behind a row of the `…/saves` list. - // Same gate as that list (auth-gated, creator-only): a save id resolves whether or not - // it was ever published, so this exposes the same unpublished work the list does. + // Same gate as that list: a save id resolves whether or not it was ever published, so + // this exposes the same unpublished work, to the same readers. .get( '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/saves/:saveId{[0-9]+}', describeRoute({ @@ -1947,8 +1975,9 @@ const app = new Hono() 'save by guessing an id: a save that belongs elsewhere is a 404, same as an unknown', 'one.', '', - 'Creator-only, like the list it details — a save id resolves whether or not it was', - 'ever published, so this reads unpublished work.', + 'Gated like the list it details — the room’s creator, or anyone whose presence puts', + 'them in the room. A save id resolves whether or not it was ever published, so this', + 'reads unpublished work.', ].join(' '), security: AUTHED, parameters: [roomIdParam, subRoomIdParam, saveIdParam], @@ -1971,7 +2000,7 @@ const app = new Hono() // be used to read its saves. const room = await getRoomById(c.env.DB, roomId) if (!room || !findSubRoom(room, subRoomId)) return c.notFound() - if (room.CreatorAccountId !== accountId) return c.body(null, 403) + if (!(await canReadSaves(c, room, roomId, accountId))) return c.body(null, 403) const save = await getSubRoomSaveById(c.env.DB, subRoomId, saveId) return save ? c.json(toSaveResponse(save)) : c.notFound() diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index f6aab4f..a117692 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -50,6 +50,33 @@ async function bearer(sub: string, roles?: string[]): Promise { + const now = Math.floor(Date.now() / 1000) + await env.DB.prepare('INSERT OR REPLACE INTO presence (data) VALUES (?1)') + .bind( + JSON.stringify({ + accountId, + roomInstance: { roomInstanceId: 1000000 + roomId, roomId, subRoomId: roomId }, + expiresAt: expired ? now - 1 : now + 900, + }) + ) + .run() +} + +/** Take a player back out of whatever room they were in. */ +async function clearPresence(accountId: number): Promise { + await env.DB.prepare('DELETE FROM presence WHERE account_id = ?1').bind(accountId).run() +} + // Apply the schema + seed the imported rooms into the test D1 (mirrors the migrations). beforeAll(async () => { // Seed the shared JWT signing key into the local Secrets Store so .get() resolves. @@ -2579,18 +2606,41 @@ describe('rooms endpoints', () => { }) expect(await empty.json()).toEqual({ Results: [], TotalResults: 0, TotalCount: 0 }) - // The list exposes unpublished saves, so it is owner-only: no token → 401, and a - // valid token that isn't the room's creator → 403. + // The list exposes unpublished saves, so it isn't public: no token → 401, and a + // valid token from someone who is neither the creator nor in the room → 403. Account + // 2 is a co-owner (Role 30 on the seeded rooms) and is refused too — holding a role + // grants nothing here; being in the room does (see below). expect((await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`)).status).toBe(401) expect( (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') })) .status ).toBe(403) - // Even a co-owner (account 2 holds Role 30 on the seeded rooms) is refused. expect( (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('2') })) .status ).toBe(403) + + // …but a player standing IN the room reads it: the client resolves which version to + // load from this list, so a visitor who can't read it can't load the instance. + await putInRoom(999, 2) + expect( + (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') })) + .status + ).toBe(200) + // Presence in a DIFFERENT room is not presence in this one. + await putInRoom(999, 5) + expect( + (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') })) + .status + ).toBe(403) + // And the grant lasts only as long as the presence does — an expired row reads as + // absent, so the visitor is refused again the moment they leave. + await putInRoom(999, 2, { expired: true }) + expect( + (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') })) + .status + ).toBe(403) + await clearPresence(999) }) it('GET /rooms/:id/subrooms/:sid/saves/:saveId is the detail behind a history row', async () => { @@ -2637,11 +2687,17 @@ describe('rooms endpoints', () => { expect((await get('/rooms/99999/subrooms/2/saves/1', '1')).status).toBe(404) expect((await get('/rooms/2/subrooms/99999/saves/1', '1')).status).toBe(404) - // Same gate as the list it details: 401 unauthed, 403 for a non-creator, and 403 - // even for a co-owner — it reads unpublished saves. - expect((await get(`/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}`)).status).toBe(401) - expect((await get(`/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}`, '999')).status).toBe(403) - expect((await get(`/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}`, '2')).status).toBe(403) + // Same gate as the list it details: 401 unauthed, 403 for someone who is neither the + // creator nor in the room (a co-owner included) — it reads unpublished saves. + const detail = `/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}` + expect((await get(detail)).status).toBe(401) + expect((await get(detail, '999')).status).toBe(403) + expect((await get(detail, '2')).status).toBe(403) + // A player standing in the room reads it, for as long as they're there. + await putInRoom(999, 2) + expect((await get(detail, '999')).status).toBe(200) + await clearPresence(999) + expect((await get(detail, '999')).status).toBe(403) }) it('GET /openapi.json documents every route', async () => {