Skip to content

Commit 9b52f63

Browse files
committed
fix(realtime): leave the prior table room only once the join is certain
A table switch left the previous room before the access re-check ran, so a denial there aborted the join and left the client in no table room at all — silently dropped from one it may still be allowed to occupy. The leave now happens after the re-check, matching the file-doc and workspace-list joins.
1 parent 2dc2d59 commit 9b52f63

2 files changed

Lines changed: 63 additions & 10 deletions

File tree

apps/realtime/src/handlers/tables.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
import { ROOM_TYPES } from '@sim/realtime-protocol/rooms'
55
import { TABLE_PRESENCE_EVENTS } from '@sim/realtime-protocol/table-presence'
6+
import { sleep } from '@sim/utils/helpers'
67
import { beforeEach, describe, expect, it, vi } from 'vitest'
78
import type { IRoomManager } from '@/rooms'
89

@@ -296,6 +297,55 @@ describe('setupTablesHandlers', () => {
296297
}
297298
})
298299

300+
it('keeps the prior table room when a switch is denied at the access re-check', async () => {
301+
// A denied switch must not silently drop the client from a table it may still be
302+
// allowed to occupy, so the prior room is left only once the join is certain.
303+
vi.useFakeTimers()
304+
try {
305+
const prior = { type: ROOM_TYPES.TABLE, id: 'table-prior' }
306+
const { socket, handlers } = createSocket({ id: 'socket-switch', userId: 'user-switch' })
307+
const roomManager = createRoomManager({
308+
getRoomForSocket: vi.fn().mockResolvedValue(prior),
309+
})
310+
setupTablesHandlers(socket as unknown as SetupArg, roomManager)
311+
312+
let call = 0
313+
mockAuthorizeRoom.mockImplementation(async () => {
314+
call += 1
315+
if (call === 1) {
316+
// A later-started read drops this join's own decision, and the join stalls past
317+
// the TTL so that decision is expired by re-check time — forcing the re-resolve
318+
// down its database path below.
319+
commitRoomPermission(
320+
'user-switch',
321+
{ type: ROOM_TYPES.TABLE, id: 'table-target' },
322+
'admin',
323+
beginRoomPermissionRead()
324+
)
325+
await sleep(31_000)
326+
return { allowed: true, status: 200, workspaceId: 'ws-1', workspacePermission: 'admin' }
327+
}
328+
// The authoritative current answer: access is gone.
329+
return { allowed: false, status: 403, workspaceId: 'ws-1', workspacePermission: null }
330+
})
331+
332+
const joining = handlers[TABLE_PRESENCE_EVENTS.JOIN]({ tableId: 'table-target' })
333+
await vi.advanceTimersByTimeAsync(31_000)
334+
await joining
335+
336+
expect(socket.emit).toHaveBeenCalledWith(
337+
TABLE_PRESENCE_EVENTS.JOIN_ERROR,
338+
expect.objectContaining({ code: 'ACCESS_DENIED', retryable: false })
339+
)
340+
// Neither joined the target nor abandoned the prior room.
341+
expect(socket.join).not.toHaveBeenCalled()
342+
expect(socket.leave).not.toHaveBeenCalled()
343+
expect(roomManager.removeUserFromRoom).not.toHaveBeenCalled()
344+
} finally {
345+
vi.useRealTimers()
346+
}
347+
})
348+
299349
it('drops a malformed cell selection without storing or relaying it', async () => {
300350
const { socket, handlers, toEmit } = createSocket()
301351
const roomManager = createRoomManager({

apps/realtime/src/handlers/tables.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -225,16 +225,6 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
225225
// Server-authenticated avatar for the presence roster.
226226
const avatarUrl = await resolveAvatarUrl(socket, userId)
227227

228-
// Leave a previously-joined table room if switching tables. No generation guard is needed
229-
// around this: serialization guarantees no concurrent op committed to a different room
230-
// during the lookup, so `currentRoom` is the socket's genuine prior room, safe to leave.
231-
const currentRoom = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.TABLE)
232-
if (currentRoom && currentRoom.id !== tableId) {
233-
socket.leave(roomName(currentRoom))
234-
await roomManager.removeUserFromRoom(currentRoom, socket.id)
235-
await roomManager.broadcastPresenceUpdate(currentRoom)
236-
}
237-
238228
// Reclaim presence orphaned by an ungraceful disconnect (no `disconnecting`
239229
// event fires on a pod crash; the room hashes have no TTL). Returns the roster it
240230
// read so the same-tab dedup below reuses it instead of issuing a second read.
@@ -273,6 +263,19 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
273263
return
274264
}
275265

266+
// Only now that the join is certain to proceed, leave a previously-joined table room
267+
// if switching. Deliberately AFTER the access re-check: a denial there aborts the
268+
// join, and leaving first would silently drop the client from a prior table it may
269+
// still be allowed to occupy. No generation guard is needed around this —
270+
// serialization guarantees no concurrent op committed to a different room during the
271+
// lookup, so `currentRoom` is the socket's genuine prior room, safe to leave.
272+
const currentRoom = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.TABLE)
273+
if (currentRoom && currentRoom.id !== tableId) {
274+
socket.leave(roomName(currentRoom))
275+
await roomManager.removeUserFromRoom(currentRoom, socket.id)
276+
await roomManager.broadcastPresenceUpdate(currentRoom)
277+
}
278+
276279
// Final re-check before the membership commit: a LEAVE or a newer JOIN enqueued during the
277280
// awaits above — including the access re-resolve — bumped the generation, or the socket
278281
// disconnected. This is the LAST await before registering, so nothing can interleave

0 commit comments

Comments
 (0)