[rooms] fix get subroom save endpoints for co-owners

This commit is contained in:
Devin Zuczek
2026-10-06 10:48:22 -04:00
parent ae0f2f4cc9
commit 7dd88a0198
3 changed files with 37 additions and 31 deletions
+7 -6
View File
@@ -118,12 +118,13 @@ inconsistency here without checking the client first.
`GET …/saves/:saveId` is the detail behind a list row, under the same gate, but in the `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 CAMELCASE projection the room save's response uses — not the PascalCase rows the list
serves. Three shapes of one save; keep them straight. serves. Three shapes of one save; keep them straight.
- Both save reads (`rooms`: `…/saves` and `…/saves/:saveId`) are auth-gated and readable by - The save reads (`rooms`: `…/saves`, `…/saves/no_unity_assets` and `…/saves/:saveId`) are
the room's CREATOR or by anyone whose live `presence` row puts them in that room — not by auth-gated and readable by the room's creator, a CO-OWNER, or anyone whose live `presence`
co-owners as such (a co-owner passes only by standing there). They list unpublished row puts them in that room. They list unpublished staged saves, so they aren't public; but
staged saves, so they aren't public; but a visitor resolves which version an instance is a co-owner opens the save history from the room's settings without being in the room
running from this list, so creator-only locks them out of loading the room. The grant (creator-only showed them the buttons and a list that 403'd), and a visitor resolves which
expires with the presence row. version an instance is running from this list, so managers-only locks them out of loading
the room. The presence grant expires with the presence row.
- A room save writes ONLY to the subroom and its save row — never to the room. Everything - 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 the body carries describes that one revision: `Description` is the save comment shown in
`…/saves`, and `PersistenceVersion`/`InventionUsage` describe the scene just saved (the `…/saves`, and `PersistenceVersion`/`InventionUsage` describe the scene just saved (the
+22 -17
View File
@@ -338,18 +338,22 @@ async function handlePhotonAccessToken(c: Context<App>) {
} }
/** /**
* May this caller read the room's saves? The room's creator always may. So may anyone * May this caller read the room's saves? The room's creator and its co-owners always may:
* whose live presence puts them IN the room: they are already loading its scene, and the * they are the ones who write and publish saves (`canManageRoom` gates both), and the
* client resolves which version to load — the published one or the creator's latest — from * client shows them the save history from the room's settings, wherever they are — a
* the save list, so refusing everyone but the creator leaves a visitor unable to load what * co-owner refused here sees the buttons and a list that never loads.
* the instance is actually running.
* *
* Presence is the shared `presence` table the `match` heartbeat maintains, so this grant * 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 latest —
* from the save list, so refusing everyone but the room's managers leaves a visitor unable
* to load what the instance is actually running.
*
* Presence is the shared `presence` table the `match` heartbeat maintains, so that grant
* lasts only as long as the player is actually there (rows carry an absolute expiry and * 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 * expired ones don't read back).
* 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. * The presence read only happens for someone who doesn't manage the room, so the owner's
* and a co-owner's path stays one query.
*/ */
async function canReadSaves( async function canReadSaves(
c: Context<App>, c: Context<App>,
@@ -357,7 +361,7 @@ async function canReadSaves(
roomId: number, roomId: number,
accountId: number accountId: number
): Promise<boolean> { ): Promise<boolean> {
if (room.CreatorAccountId === accountId) return true if (canManageRoom(room, accountId)) return true
const instance = (await getPresence<PresenceView>(c.env.DB, accountId))?.roomInstance const instance = (await getPresence<PresenceView>(c.env.DB, accountId))?.roomInstance
return instance?.roomId === roomId return instance?.roomId === roomId
} }
@@ -3437,7 +3441,8 @@ const app = new Hono<App>()
// A subroom's saved-data versions — the room-history / "restore a save" list. Every // 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 // 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 readable by the // 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`). // room's creator, a co-owner, or anyone whose presence puts them in the room (see
// `canReadSaves`).
.get( .get(
'/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/saves', '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/saves',
describeRoute({ describeRoute({
@@ -3451,8 +3456,8 @@ const app = new Hono<App>()
'(0 Windows, 2 Android/Quest). `unityAssetVersion` is accepted and ignored.', '(0 Windows, 2 Android/Quest). `unityAssetVersion` is accepted and ignored.',
'', '',
'The list includes STAGED saves that were never published, so it is not public:', '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', 'the room’s creator or a co-owner may read it, and so may anyone standing IN the room',
'presence says so). Anyone else is a 403. It is what the client reads to resolve', '(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', '“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.', 'visitor who cannot read it cannot load what the instance is running.',
'', '',
@@ -3525,8 +3530,8 @@ const app = new Hono<App>()
'paged wrapper, with no `{ success, error, value }` envelope around it.', 'paged wrapper, with no `{ success, error, value }` envelope around it.',
'', '',
'Gated exactly like `…/saves`, and for the same reason: the list includes STAGED', 'Gated exactly like `…/saves`, and for the same reason: the list includes STAGED',
'saves that were never published, so it is the room’s creator or anyone whose live', 'saves that were never published, so it is the room’s creator, a co-owner, or anyone',
'presence puts them in the room, and anyone else is a 403.', 'whose live presence puts them in the room, and anyone else is a 403.',
'', '',
'`TotalResults` and `TotalCount` carry the same number — the client’s paged DTO and', '`TotalResults` and `TotalCount` carry the same number — the client’s paged DTO and',
'the reference disagree on the name, so both are emitted.', 'the reference disagree on the name, so both are emitted.',
@@ -3588,8 +3593,8 @@ const app = new Hono<App>()
'save by guessing an id: a save that belongs elsewhere is a 404, same as an unknown', 'save by guessing an id: a save that belongs elsewhere is a 404, same as an unknown',
'one.', 'one.',
'', '',
'Gated like the list it details — the room’s creator, or anyone whose presence puts', 'Gated like the list it details — the room’s creator, a co-owner, or anyone whose',
'them in the room. A save id resolves whether or not it was ever published, so this', 'presence puts them in the room. A save id resolves whether or not it was ever published, so this',
'reads unpublished work.', 'reads unpublished work.',
].join(' '), ].join(' '),
security: AUTHED, security: AUTHED,
+8 -8
View File
@@ -5306,18 +5306,18 @@ describe('rooms endpoints', () => {
expect(await empty.json()).toEqual({ Results: [], TotalResults: 0, TotalCount: 0 }) expect(await empty.json()).toEqual({ Results: [], TotalResults: 0, TotalCount: 0 })
// The list exposes unpublished saves, so it isn't public: no token → 401, and a // 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 // valid token from someone who neither manages the room nor is in it → 403.
// 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`)).status).toBe(401)
expect( expect(
(await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') })) (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('999') }))
.status .status
).toBe(403) ).toBe(403)
// Account 2 is a co-owner (Role 30 on the seeded rooms) and reads it from anywhere:
// the client opens the save history from the room's settings, not from inside it.
expect( expect(
(await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('2') })) (await SELF.fetch(`${ORIGIN}/rooms/2/subrooms/2/saves`, { headers: await bearer('2') }))
.status .status
).toBe(403) ).toBe(200)
// …but a player standing IN the room reads it: the client resolves which version to // …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. // load from this list, so a visitor who can't read it can't load the instance.
@@ -5409,7 +5409,7 @@ describe('rooms endpoints', () => {
// Same gate as the list it mirrors — it exposes the same unpublished saves. // Same gate as the list it mirrors — it exposes the same unpublished saves.
expect((await get(light)).status).toBe(401) expect((await get(light)).status).toBe(401)
expect((await get(light, '999')).status).toBe(403) expect((await get(light, '999')).status).toBe(403)
expect((await get(light, '2')).status).toBe(403) expect((await get(light, '2')).status).toBe(200)
await putInRoom(999, 2) await putInRoom(999, 2)
expect((await get(light, '999')).status).toBe(200) expect((await get(light, '999')).status).toBe(200)
await clearPresence(999) await clearPresence(999)
@@ -5460,12 +5460,12 @@ describe('rooms endpoints', () => {
expect((await get('/rooms/99999/subrooms/2/saves/1', '1')).status).toBe(404) 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) expect((await get('/rooms/2/subrooms/99999/saves/1', '1')).status).toBe(404)
// Same gate as the list it details: 401 unauthed, 403 for someone who is neither the // Same gate as the list it details: 401 unauthed, 403 for someone who neither manages
// creator nor in the room (a co-owner included) — it reads unpublished saves. // the room nor is in it — it reads unpublished saves. A co-owner (account 2) passes.
const detail = `/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}` const detail = `/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}`
expect((await get(detail)).status).toBe(401) expect((await get(detail)).status).toBe(401)
expect((await get(detail, '999')).status).toBe(403) expect((await get(detail, '999')).status).toBe(403)
expect((await get(detail, '2')).status).toBe(403) expect((await get(detail, '2')).status).toBe(200)
// A player standing in the room reads it, for as long as they're there. // A player standing in the room reads it, for as long as they're there.
await putInRoom(999, 2) await putInRoom(999, 2)
expect((await get(detail, '999')).status).toBe(200) expect((await get(detail, '999')).status).toBe(200)