Skip to content

Commit 498f38b

Browse files
committed
fix(realtime): enforce room access continuously, not only at join
File-doc, table, and workspace-list rooms authorized once at JOIN and never again, so a member whose workspace access was revoked or downgraded kept live collaborative write access — including durable Yjs document writes — for the whole lifetime of an already-open socket. The access-revalidation sweep explicitly skipped every non-workflow room. - sweep every room type, authorizing each against its own resource - share one membership policy (ROOM_MEMBERSHIP_ACTIONS) between the join check and the sweep, so a file-doc room keeps requiring write in both - gate file-doc document frames and table cell selections on the cached permission, evicting on a confirmed loss of access - re-check the cached decision before a join commits, so a join that authorized just before a revocation cannot re-enter the room - never let a join's own cache write clobber a revocation recorded mid-flight - surface room-access-revoked to clients; the file-doc editor falls back to read-only instead of accepting keystrokes that go nowhere
1 parent 03649e9 commit 498f38b

15 files changed

Lines changed: 946 additions & 132 deletions

File tree

apps/realtime/src/access-revalidation.test.ts

Lines changed: 113 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,29 @@
11
/**
22
* @vitest-environment node
33
*
4-
* Tests for the periodic read-access re-validation sweep. The security contract:
5-
* a socket is evicted only when its role resolves to `null` (a confirmed
6-
* revocation), and a transient failure never evicts a still-authorized socket.
4+
* Tests for the periodic access re-validation sweep, which covers EVERY room type
5+
* a socket occupies. The security contract: a socket is evicted only when its
6+
* permission definitively fails the level that room requires (a confirmed
7+
* revocation or downgrade), and a transient failure never evicts a still-authorized
8+
* socket.
79
*/
10+
import { ROOM_TYPES } from '@sim/realtime-protocol/rooms'
811
import { beforeEach, describe, expect, it, vi } from 'vitest'
912

1013
const { mockResolveRole } = vi.hoisted(() => ({
1114
mockResolveRole: vi.fn(),
1215
}))
1316

1417
vi.mock('@/middleware/permissions', () => ({
15-
resolveCurrentWorkflowRole: mockResolveRole,
18+
resolveCurrentRoomPermission: mockResolveRole,
1619
ROLE_REVALIDATION_TTL_MS: 30_000,
1720
}))
1821

1922
import {
2023
ACCESS_REVALIDATION_SWEEP_INTERVAL_MS,
2124
startAccessRevalidationSweep,
2225
} from '@/access-revalidation'
26+
import { registerRoomEvictionHandler } from '@/handlers/room-eviction'
2327
import type { IRoomManager, UserPresence } from '@/rooms'
2428

2529
interface FakeSocket {
@@ -30,9 +34,9 @@ interface FakeSocket {
3034
leave: ReturnType<typeof vi.fn>
3135
}
3236

33-
function makeSocket(id: string, userId: string | undefined, workflowId?: string): FakeSocket {
37+
function makeSocket(id: string, userId: string | undefined, room?: string): FakeSocket {
3438
const rooms = new Set<string>([id])
35-
if (workflowId) rooms.add(workflowId)
39+
if (room) rooms.add(room)
3640
return {
3741
id,
3842
userId,
@@ -131,7 +135,7 @@ describe('access-revalidation sweep', () => {
131135
expect(manager.removeUserFromRoom).not.toHaveBeenCalled()
132136
})
133137

134-
it('resolves with the static safe fallback and no presence reads in the scan', async () => {
138+
it('resolves with the room safe fallback and no presence reads in the scan', async () => {
135139
const socket = makeSocket('sock-1', 'user-1', 'wf-1')
136140
const manager = makeManager([socket], [{ socketId: 'sock-1', role: 'admin' }])
137141
mockResolveRole.mockResolvedValue('admin')
@@ -140,32 +144,123 @@ describe('access-revalidation sweep', () => {
140144
await sweep.runOnce()
141145
sweep.stop()
142146

143-
expect(mockResolveRole).toHaveBeenCalledWith('user-1', 'wf-1', 'read')
147+
expect(mockResolveRole).toHaveBeenCalledWith('user-1', { type: 'workflow', id: 'wf-1' }, 'read')
144148
// The security scan must stay Redis-free — presence is never consulted.
145149
expect(manager.getRoomUsers).not.toHaveBeenCalled()
146150
})
147151

148-
it('never evicts a socket joined only to a non-workflow room (files/tables/file-doc)', async () => {
149-
// The sweep shares one io with the files/tables/file-doc handlers. Those rooms are
150-
// namespaced (`workspace-files:ws-1`, `table:t-1`), so treating every socket.rooms
151-
// entry as a workflow id would resolve a bogus permission → null → evict the socket
152-
// from its files/table room every pass. Non-workflow rooms must be filtered out.
152+
it('sweeps non-workflow rooms against their own resource, not a bogus workflow id', async () => {
153+
// The sweep shares one io with the files/tables/file-doc handlers. Their rooms are
154+
// namespaced (`workspace-files:ws-1`, `table:t-1`), so each name is decoded and
155+
// authorized as its own room type — the whole point of covering them at all.
153156
const filesSocket = makeSocket('sock-1', 'user-1', 'workspace-files:ws-1')
154157
const tableSocket = makeSocket('sock-2', 'user-2', 'table:t-1')
155158
const manager = makeManager([filesSocket, tableSocket])
156-
// Even if the role resolver would say "no access", these must never be swept.
157-
mockResolveRole.mockResolvedValue(null)
159+
mockResolveRole.mockResolvedValue('write')
158160

159161
const sweep = startAccessRevalidationSweep(manager)
160162
await sweep.runOnce()
161163
sweep.stop()
162164

163-
expect(mockResolveRole).not.toHaveBeenCalled()
165+
expect(mockResolveRole).toHaveBeenCalledWith(
166+
'user-1',
167+
{ type: 'workspace-files', id: 'ws-1' },
168+
'read'
169+
)
170+
expect(mockResolveRole).toHaveBeenCalledWith('user-2', { type: 'table', id: 't-1' }, 'read')
171+
// Still authorized: nobody is evicted.
164172
expect(filesSocket.leave).not.toHaveBeenCalled()
165-
expect(filesSocket.emit).not.toHaveBeenCalled()
166173
expect(tableSocket.leave).not.toHaveBeenCalled()
167-
expect(tableSocket.emit).not.toHaveBeenCalled()
174+
})
175+
176+
it('evicts a revoked socket from a presence-free workspace-files room without touching presence', async () => {
177+
const socket = makeSocket('sock-1', 'user-1', 'workspace-files:ws-1')
178+
const manager = makeManager([socket])
179+
mockResolveRole.mockResolvedValue(null)
180+
181+
const sweep = startAccessRevalidationSweep(manager)
182+
await sweep.runOnce()
183+
sweep.stop()
184+
185+
expect(socket.emit).toHaveBeenCalledWith(
186+
'room-access-revoked',
187+
expect.objectContaining({ room: { type: 'workspace-files', id: 'ws-1' } })
188+
)
189+
expect(socket.leave).toHaveBeenCalledWith('workspace-files:ws-1')
190+
// These rooms hold no room-manager presence, so nothing is owed to the cleanup lane.
168191
expect(manager.removeUserFromRoom).not.toHaveBeenCalled()
192+
expect(manager.broadcastPresenceUpdate).not.toHaveBeenCalled()
193+
})
194+
195+
it('evicts a revoked socket from a table room and clears its presence', async () => {
196+
const socket = makeSocket('sock-1', 'user-1', 'table:t-1')
197+
const manager = makeManager([socket], [{ socketId: 'sock-1', role: 'read' }])
198+
mockResolveRole.mockResolvedValue(null)
199+
200+
const sweep = startAccessRevalidationSweep(manager)
201+
await sweep.runOnce()
202+
sweep.stop()
203+
204+
expect(socket.leave).toHaveBeenCalledWith('table:t-1')
205+
expect(manager.removeUserFromRoom).toHaveBeenCalledWith({ type: 'table', id: 't-1' }, 'sock-1')
206+
expect(manager.broadcastPresenceUpdate).toHaveBeenCalledWith({ type: 'table', id: 't-1' })
207+
})
208+
209+
it('evicts a file-doc socket downgraded to read, and keeps its table room', async () => {
210+
// A file-doc room IS the editor and requires `write`; a table room requires only
211+
// `read`. One downgraded user in both rooms must lose exactly the document.
212+
const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
213+
socket.rooms.add('table:t-1')
214+
const manager = makeManager([socket])
215+
mockResolveRole.mockResolvedValue('read')
216+
217+
const sweep = startAccessRevalidationSweep(manager)
218+
await sweep.runOnce()
219+
sweep.stop()
220+
221+
expect(socket.leave).toHaveBeenCalledWith('workspace-file-doc:file-1')
222+
expect(socket.leave).not.toHaveBeenCalledWith('table:t-1')
223+
expect(socket.emit).toHaveBeenCalledWith(
224+
'room-access-revoked',
225+
expect.objectContaining({ room: { type: 'workspace-file-doc', id: 'file-1' } })
226+
)
227+
})
228+
229+
it('falls back to the room type own membership level on a cold-cache failure', async () => {
230+
// A static 'read' fallback would have evicted every file-doc socket (which needs
231+
// `write`) the first time the DB blipped with a cold cache.
232+
const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
233+
const manager = makeManager([socket])
234+
mockResolveRole.mockResolvedValue('write')
235+
236+
const sweep = startAccessRevalidationSweep(manager)
237+
await sweep.runOnce()
238+
sweep.stop()
239+
240+
expect(mockResolveRole).toHaveBeenCalledWith(
241+
'user-1',
242+
{ type: 'workspace-file-doc', id: 'file-1' },
243+
'write'
244+
)
245+
expect(socket.leave).not.toHaveBeenCalled()
246+
})
247+
248+
it('runs the room type registered eviction handler so handler-local state is dropped', async () => {
249+
const evicted = vi.fn()
250+
registerRoomEvictionHandler(ROOM_TYPES.WORKSPACE_FILE_DOC, evicted)
251+
const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
252+
const manager = makeManager([socket])
253+
mockResolveRole.mockResolvedValue(null)
254+
255+
const sweep = startAccessRevalidationSweep(manager)
256+
await sweep.runOnce()
257+
sweep.stop()
258+
259+
expect(evicted).toHaveBeenCalledWith(
260+
'sock-1',
261+
{ type: 'workspace-file-doc', id: 'file-1' },
262+
manager.io
263+
)
169264
})
170265

171266
it('evicts only the revoked socket, not co-members of the room', async () => {

0 commit comments

Comments
 (0)