[rooms] better gating for unpublished rooms

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]+`). */

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<App>()
 		}
 	)
 
-	// Another player's visited rooms — what the client shows on a friend's profile.
+	// Another player's visited rooms — what the client shows on their profile. Auth-gated
-	// Auth-gated (401), and FRIENDS-ONLY: a valid token for someone who isn't that
+	// (401) but otherwise open: anyone can look at anyone's history, as they can at the
-	// player and isn't a mutual friend of theirs is a 403, since where a player has
+	// `ownedby/{accountId}` list beside it. Registered after `visitedby/me` so the
-	// been is not public. Registered after `visitedby/me` so the literal path wins.
+	// literal path wins. Paginated via skip/take (take defaults to 100) and, like
-	// Paginated via skip/take (take defaults to 100) and, like `visitedby/me`, a bare
+	// `visitedby/me`, a bare array — the client's room-source loaders expect a plain
-	// array — the client's room-source loaders expect a plain list, not a page.
+	// 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',
+				'The rooms another player has visited, as a bare array. Any authed caller may read',
-				'be that player or a mutual friend of theirs (403 otherwise) — visit history is not',
+				'anyone’s history, but only its listable rooms (public, not a dorm, not opted out of',
-				'public.',
+				'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
+			// Your own history is the full one (the client sometimes sends the id rather
-			// rather than `me`); anyone else's needs a mutual friendship.
+			// than `me`); anyone else's is cut to its listable rooms.
-			if (playerId !== accountId && !(await areFriends(c.env.DB, accountId, playerId))) {
+			const own = playerId === accountId
-				return c.body(null, 403)
-			}
 			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)
+			)
 		}
 	)
 

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
+		// Anyone with a token reads it — a bare array. Friendship is not required: a
-		// relationship row the caller sits on.
+		// player's history is as public as the rooms on their profile.
-		for (const friend of ['791', '792']) {
+		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<string, unknown>) =>
+			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<string, string>, 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)

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<Room[]> {
+	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<Room[]> {
+	return visitedRooms(db, playerId, skip, take, true)
+}
+
+async function visitedRooms(
+	db: D1Database,
+	playerId: number,
+	skip: number,
+	take: number,
+	listableOnly: boolean
+): Promise<Room[]> {
+	// 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<RoomRow>()
-	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. */