diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index 2279fdc..4290f93 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -64,12 +64,6 @@ export const FORBIDDEN_RESPONSE = { description: 'A valid token, but not the room’s creator or a co-owner (empty body)', } -/** The 403 the friends-only routes return (empty body). */ -export const NOT_FRIENDS_RESPONSE = { - description: - 'A valid token, but the caller is not that player (nor a friend of theirs) (empty body)', -} - // ---- Parameters ------------------------------------------------------------ /** A digits-only id path parameter (the route patterns constrain these to `[0-9]+`). */ diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index f0afd03..2beed05 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -6,7 +6,6 @@ import { Accessibility, answerRoomRoleInvite, applyRoomTagEdit, - areFriends, autocompleteRoomSearch, banPlayerFromRoom, canManageRoom, @@ -33,6 +32,7 @@ import { getPlayerIdsInRoom, getPresence, getPublicRoomsByCreator, + getPublicVisitedRooms, getRecommendedRooms, getRoomBanHistory, getRoomBans, @@ -122,7 +122,6 @@ import { ModifySubRoomRequest, MoveSubRoomRequest, NameRequest, - NOT_FRIENDS_RESPONSE, PagedRooms, pageParams, PhotonAccessTokenDto, @@ -1570,42 +1569,52 @@ const app = new Hono() } ) - // Another player's visited rooms — what the client shows on a friend's profile. - // Auth-gated (401), and FRIENDS-ONLY: a valid token for someone who isn't that - // player and isn't a mutual friend of theirs is a 403, since where a player has - // been is not public. Registered after `visitedby/me` so the literal path wins. - // Paginated via skip/take (take defaults to 100) and, like `visitedby/me`, a bare - // array — the client's room-source loaders expect a plain list, not a page. + // Another player's visited rooms — what the client shows on their profile. Auth-gated + // (401) but otherwise open: anyone can look at anyone's history, as they can at the + // `ownedby/{accountId}` list beside it. Registered after `visitedby/me` so the + // literal path wins. Paginated via skip/take (take defaults to 100) and, like + // `visitedby/me`, a bare array — the client's room-source loaders expect a plain + // list, not a page. + // + // What is hidden is the ROOM, not the history: everyone else sees only the LISTABLE + // rooms of it (public, not a dorm, not opted out of lists — the same cut as + // `ownedby/{accountId}`). A visit is stamped wherever the player goes, so the raw + // history names their unpublished builds and the private rooms they were invited + // into, and served whole it leaked rooms nobody has released. Reading your OWN + // history by id is the full list, exactly as `visitedby/me` serves it. .get( '/rooms/visitedby/:playerId{[0-9]+}', describeRoute({ tags: ['Rooms'], - summary: 'A friend’s visited rooms', + summary: 'Another player’s visited rooms', description: [ - 'The rooms another player has visited, as a bare array. Friends only: the caller must', - 'be that player or a mutual friend of theirs (403 otherwise) — visit history is not', - 'public.', + 'The rooms another player has visited, as a bare array. Any authed caller may read', + 'anyone’s history, but only its listable rooms (public, not a dorm, not opted out of', + 'lists): the player’s unpublished builds and the private rooms they were invited', + 'into stay off the list. Asking for your own id serves the full history, as', + '`visitedby/me` does.', ].join(' '), security: AUTHED, parameters: [playerIdParam, ...pageParams(100)], responses: { 200: json(RoomDto.array(), 'That player’s visited rooms'), 401: UNAUTHORIZED_RESPONSE, - 403: NOT_FRIENDS_RESPONSE, }, }), async (c) => { const accountId = await authedAccountId(c) if (accountId === null) return unauthorized(c) const playerId = Number.parseInt(c.req.param('playerId'), 10) - // Your own history is always readable (the client sometimes sends the id - // rather than `me`); anyone else's needs a mutual friendship. - if (playerId !== accountId && !(await areFriends(c.env.DB, accountId, playerId))) { - return c.body(null, 403) - } + // Your own history is the full one (the client sometimes sends the id rather + // than `me`); anyone else's is cut to its listable rooms. + const own = playerId === accountId const skip = Number.parseInt(c.req.query('skip') ?? '0', 10) || 0 const take = Number.parseInt(c.req.query('take') ?? '100', 10) || 100 - return c.json(await getVisitedRooms(c.env.DB, playerId, skip, take)) + return c.json( + own + ? await getVisitedRooms(c.env.DB, playerId, skip, take) + : await getPublicVisitedRooms(c.env.DB, playerId, skip, take) + ) } ) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index 3a37a7d..7901cc7 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -134,25 +134,6 @@ beforeAll(async () => { await env.DB.prepare('INSERT OR IGNORE INTO account (data) VALUES (?1)') .bind(JSON.stringify({ accountId: 999, username: 'Dormer' })) .run() - - // Relationship table (owned by the api worker) — `visitedby/:playerId` reads it to - // check the caller is a friend of the player whose history they're asking for. - await env.DB.prepare( - `CREATE TABLE IF NOT EXISTS relationship ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - requester_id INTEGER NOT NULL, - target_id INTEGER NOT NULL, - relationship_type INTEGER NOT NULL DEFAULT 0 - )` - ).run() - const insertRel = env.DB.prepare( - 'INSERT INTO relationship (requester_id, target_id, relationship_type) VALUES (?1, ?2, ?3)' - ) - await env.DB.batch([ - insertRel.bind(791, 790, 3), // friends — the caller (791) is the requester - insertRel.bind(790, 792, 3), // friends — the caller (792) is the target - insertRel.bind(793, 790, 1), // request out, not accepted — 793 is NOT a friend - ]) }) describe('rooms endpoints', () => { @@ -796,7 +777,7 @@ describe('rooms endpoints', () => { expect(other).toEqual([]) }) - it('GET /rooms/visitedby/:playerId serves a friend’s visited rooms and 403s everyone else', async () => { + it('GET /rooms/visitedby/:playerId serves any player’s visited rooms to any authed caller', async () => { // Give 790 a visit history (cheering/favoriting stamps a last-visit). const subject = await bearer('790') await SELF.fetch(`${ORIGIN}/rooms/2/interactionby/me/cheer`, { @@ -808,11 +789,11 @@ describe('rooms endpoints', () => { headers: subject, }) - // A mutual friend reads it — a bare array, regardless of which side of the - // relationship row the caller sits on. - for (const friend of ['791', '792']) { + // Anyone with a token reads it — a bare array. Friendship is not required: a + // player's history is as public as the rooms on their profile. + for (const viewer of ['791', '792', '793', '794']) { const res = await SELF.fetch(`${ORIGIN}/rooms/visitedby/790`, { - headers: await bearer(friend), + headers: await bearer(viewer), }) expect(res.status).toBe(200) const body = (await res.json()) as Array<{ RoomId: number }> @@ -833,14 +814,6 @@ describe('rooms endpoints', () => { ).json()) as unknown[] expect(own.length).toBe(2) - // A pending request is not a friendship, and a stranger is not either → 403. - for (const outsider of ['793', '794']) { - const res = await SELF.fetch(`${ORIGIN}/rooms/visitedby/790`, { - headers: await bearer(outsider), - }) - expect(res.status).toBe(403) - } - // No token at all → 401, never a fallback account. const anon = await SELF.fetch(`${ORIGIN}/rooms/visitedby/790`) expect(anon.status).toBe(401) @@ -852,6 +825,67 @@ describe('rooms endpoints', () => { expect(me.length).toBe(2) }) + it('GET /rooms/visitedby/:playerId hides visits to unpublished rooms', async () => { + // 790 has been to an UNPUBLISHED room (their own build), an unlisted one, and a room + // that opted out of lists — each stamped as the most recent visit, ahead of the two + // public rooms the test above recorded. + const seed = (data: Record) => + env.DB.prepare('INSERT INTO room (data) VALUES (?1)').bind(JSON.stringify(data)).run() + await seed({ + RoomId: 30501, + Name: 'VisitedUnpublished', + CreatorAccountId: 790, + Accessibility: 0, + SubRooms: [], + }) + await seed({ RoomId: 30502, Name: 'VisitedUnlisted', Accessibility: 2, SubRooms: [] }) + await seed({ + RoomId: 30503, + Name: 'VisitedOptedOut', + Accessibility: 1, + ExcludeFromLists: true, + SubRooms: [], + }) + await env.DB.prepare( + `INSERT INTO interaction (player_id, room_id, last_visited_at) + VALUES (790, 30501, '2030-01-03T00:00:00.000Z'), + (790, 30502, '2030-01-02T00:00:00.000Z'), + (790, 30503, '2030-01-01T00:00:00.000Z')` + ).run() + try { + const ids = async (headers: Record, query = '') => + ( + (await ( + await SELF.fetch(`${ORIGIN}/rooms/visitedby/790${query}`, { headers }) + ).json()) as Array<{ RoomId: number }> + ).map((r) => r.RoomId) + + // Anyone else sees only the listable rooms of the history — public, not opted + // out — and a page is a page of THOSE: the first two are the two public rooms, + // not two holes where the hidden ones sat. + const sorted = (list: number[]) => [...list].sort((a, b) => a - b) + const viewer = await bearer('794') + expect(sorted(await ids(viewer))).toEqual([2, 12]) + expect(sorted(await ids(viewer, '?skip=0&take=2'))).toEqual([2, 12]) + + // The player's own history, by id or as `me`, is the whole of it, newest first. + const subject = await bearer('790') + const own = await ids(subject) + expect(own.slice(0, 3)).toEqual([30501, 30502, 30503]) + expect(sorted(own)).toEqual([2, 12, 30501, 30502, 30503]) + expect( + ( + (await ( + await SELF.fetch(`${ORIGIN}/rooms/visitedby/me`, { headers: subject }) + ).json()) as Array<{ RoomId: number }> + ).map((r) => r.RoomId) + ).toEqual(own) + } finally { + await env.DB.prepare('DELETE FROM room WHERE room_id BETWEEN 30501 AND 30503').run() + await env.DB.prepare('DELETE FROM interaction WHERE room_id BETWEEN 30501 AND 30503').run() + } + }) + it('GET /rooms/hot returns a paginated { Results, TotalResults } of public rooms', async () => { const res = await SELF.fetch(`${ORIGIN}/rooms/hot?skip=0&take=100`) expect(res.status).toBe(200) diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index 0a7b893..d5d461f 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -2796,6 +2796,10 @@ export async function getFavoritedRooms( * most recent first. Like favorites, it joins `interaction` to `rooms`, so a * visited room no longer in D1 is simply absent. Paginated via skip/take; returns * a bare array of rooms (the client's room-source loaders expect a plain list). + * + * This is the player's OWN history, so nothing is filtered on accessibility: their + * unpublished builds and the private rooms they were invited into are theirs to see. + * Anyone else reading it goes through {@link getPublicVisitedRooms}. */ export async function getVisitedRooms( db: D1Database, @@ -2803,17 +2807,52 @@ export async function getVisitedRooms( skip: number, take: number ): Promise { + return visitedRooms(db, playerId, skip, take, false) +} + +/** + * {@link getVisitedRooms} as SOMEONE ELSE sees it — a profile, which anyone can look at — + * narrowed to the rooms that are {@link isListable}. A visit is recorded wherever the + * player goes, so the unfiltered history names their unpublished builds and the private + * rooms they were invited into; served whole, it leaked a room its owner hasn't released, + * and `GET /rooms/{id}` is not gated, so the id alone was enough to open it. The same test + * the public `ownedby/{accountId}` profile list applies, so a room that opted out of lists + * stays off this one too. Paginated AFTER the filter, so a page is a page of what the + * viewer may see rather than a page with holes in it. + */ +export async function getPublicVisitedRooms( + db: D1Database, + playerId: number, + skip: number, + take: number +): Promise { + return visitedRooms(db, playerId, skip, take, true) +} + +async function visitedRooms( + db: D1Database, + playerId: number, + skip: number, + take: number, + listableOnly: boolean +): Promise { + // The listable predicate is pushed down (see LISTABLE_WHERE) so the blobs of the + // rooms a friend may not see never leave D1; the in-memory filter remains the + // definition, as everywhere else it is used. const { results } = await db .prepare( `SELECT r.data AS data, r.visits AS visits FROM interaction i JOIN room r ON r.room_id = i.room_id WHERE i.player_id = ?1 AND i.last_visited_at IS NOT NULL + ${listableOnly ? `AND r.${LISTABLE_WHERE.replaceAll(' AND ', ' AND r.')}` : ''} ORDER BY i.last_visited_at DESC` ) .bind(playerId) .all() - return hydrateRooms(db, parseAll(results).slice(skip, skip + take)) + const rooms = parseAll(results) + const visible = listableOnly ? rooms.filter(isListable) : rooms + return hydrateRooms(db, visible.slice(skip, skip + take)) } /** A player's interaction state with a room. */