Skip to content

Commit 28192ff

Browse files
committed
fix(realtime): re-check access before workspace-list room joins too
The workspace-files / workspace-tables joins committed straight from their authorize result, so a join that authorized just before a revocation could put the socket back in a room the sweep had already evicted it from. Mirrors the guard the file-doc and table joins already had.
1 parent 553d1aa commit 28192ff

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

apps/realtime/src/handlers/workspace-invalidation-room.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ vi.mock('@sim/platform-authz/rooms', () => ({
1919
}))
2020

2121
import { setupWorkspaceInvalidationRoom } from '@/handlers/workspace-invalidation-room'
22+
import { recordRoomPermission } from '@/middleware/permissions'
2223

2324
type Payload = { workspaceId?: string }
2425

@@ -157,6 +158,30 @@ describe.each([ROOM_TYPES.WORKSPACE_FILES, ROOM_TYPES.WORKSPACE_TABLES] as const
157158
expect(roomManager.broadcastPresenceUpdate).not.toHaveBeenCalled()
158159
})
159160

161+
it('does not join when access was revoked while the join was in flight', async () => {
162+
// The sweep records a revocation before it evicts, so a join whose authorize
163+
// completed just before that must not put the socket back in the room.
164+
const { handlers, socket } = createSocket({ id: 'socket-race', userId: 'user-race' })
165+
setupWorkspaceInvalidationRoom(
166+
socket as unknown as Parameters<typeof setupWorkspaceInvalidationRoom>[0],
167+
createRoomManager(),
168+
roomType
169+
)
170+
171+
mockAuthorizeRoom.mockImplementation(async () => {
172+
recordRoomPermission('user-race', { type: roomType, id: 'ws-race' }, null)
173+
return { allowed: true, status: 200, workspaceId: 'ws-race', workspacePermission: 'admin' }
174+
})
175+
176+
await handlers[joinEvent]({ workspaceId: 'ws-race' })
177+
178+
expect(socket.emit).toHaveBeenCalledWith(
179+
errorEvent,
180+
expect.objectContaining({ code: 'ACCESS_DENIED', retryable: false })
181+
)
182+
expect(socket.join).not.toHaveBeenCalled()
183+
})
184+
160185
it('leaves a previously-joined room when switching workspaces', async () => {
161186
const { socket, handlers, rooms } = createSocket()
162187
rooms.add(roomOf('ws-old'))

apps/realtime/src/handlers/workspace-invalidation-room.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import { ROOM_MEMBERSHIP_ACTIONS } from '@sim/platform-authz/room-policy'
2+
import { ROOM_MEMBERSHIP_ACTIONS, satisfiesRoomMembership } from '@sim/platform-authz/room-policy'
33
import { type RoomRef, type RoomType, roomName } from '@sim/realtime-protocol/rooms'
44
import { resolveRoomJoinAuth } from '@/handlers/room-join-auth'
55
import type { AuthenticatedSocket } from '@/middleware/auth'
6+
import { peekRoomPermission } from '@/middleware/permissions'
67
import type { IRoomManager } from '@/rooms'
78

89
const logger = createLogger('WorkspaceInvalidationRoom')
@@ -107,6 +108,22 @@ export function setupWorkspaceInvalidationRoom(
107108
// stale join can't leave the room the client has since switched to.
108109
if (joinGeneration !== joinAttempt || socket.disconnected) return
109110

111+
// Re-check the cached decision before committing: the access re-validation sweep
112+
// records a revocation BEFORE it evicts, so a join that authorized just before the
113+
// revocation must not complete afterwards and put the socket back in the room.
114+
// `undefined` (nothing cached) is "unknown", never a denial — the authorize above
115+
// is then the freshest word we have. Mirrors the file-doc and table joins.
116+
const recheck = peekRoomPermission(socket.userId, ref)
117+
if (recheck !== undefined && !satisfiesRoomMembership(recheck, roomType)) {
118+
socket.emit(errorEvent, {
119+
workspaceId,
120+
error: 'Access denied to workspace',
121+
code: 'ACCESS_DENIED',
122+
retryable: false,
123+
})
124+
return
125+
}
126+
110127
// Leave any previously-joined room of this type (workspace switch), read straight from the
111128
// socket's native room membership so there's no presence store to keep in sync.
112129
const target = roomName(ref)

0 commit comments

Comments
 (0)