Skip to content

Commit c19575a

Browse files
authored
fix(realtime): enforce the Files capability on workspace document rooms (#9580)
* fix(realtime): enforce the Files capability on workspace document rooms * fix(realtime): pass the route context in the document access integration check
1 parent 868f201 commit c19575a

12 files changed

Lines changed: 513 additions & 75 deletions

File tree

‎apps/realtime/src/handlers/file-doc-app.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import { isPermissionType } from '@sim/platform-authz/predicates'
12
import type { RoomAuthorizationResult } from '@sim/platform-authz/rooms'
23
import { FILE_DOC_INTERNAL_HEADERS, FILE_DOC_TIMEOUTS } from '@sim/realtime-protocol/file-doc'
4+
import { toRecord } from '@sim/utils/object'
35
import { env, getBaseUrl } from '@/env'
46

57
/**
@@ -32,6 +34,41 @@ function postToApp(
3234
})
3335
}
3436

37+
/**
38+
* Resolve a user's access to a workspace file's document room through the app. Only a decision in
39+
* the body denies; any other answer throws, so an outage is never read as a revocation.
40+
*/
41+
export async function fetchFileDocAccess(
42+
fileId: string,
43+
userId: string
44+
): Promise<RoomAuthorizationResult> {
45+
const response = await postToApp(
46+
'/api/internal/file-doc/access',
47+
{ fileId, userId },
48+
FILE_DOC_TIMEOUTS.seedRequestMs
49+
)
50+
if (!response.ok) {
51+
await response.body?.cancel()
52+
throw new Error(`Document authorization failed: ${response.status}`)
53+
}
54+
const body = toRecord(await response.json())
55+
if (body.allowed === false && (body.status === 403 || body.status === 404))
56+
return { allowed: false, status: body.status, workspaceId: null, workspacePermission: null }
57+
if (
58+
body.allowed !== true ||
59+
typeof body.workspaceId !== 'string' ||
60+
!body.workspaceId ||
61+
!isPermissionType(body.workspacePermission)
62+
)
63+
throw new Error('Malformed document authorization')
64+
return {
65+
allowed: true,
66+
status: 200,
67+
workspaceId: body.workspaceId,
68+
workspacePermission: body.workspacePermission,
69+
}
70+
}
71+
3572
/**
3673
* Existing empty files have named, versioned seeds; only missing files return null.
3774
* Transport and malformed-response failures throw so callers retry instead of creating empty rooms.

‎apps/realtime/src/handlers/file-doc-capabilities.integration.ts‎

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
type JoinFileDocSuccess,
1414
} from '@sim/realtime-protocol/file-doc'
1515
import { type FileDocOwner, fileDocRoom } from '@sim/realtime-protocol/file-doc-target'
16-
import { roomName } from '@sim/realtime-protocol/rooms'
16+
import { ROOM_TYPES, roomName } from '@sim/realtime-protocol/rooms'
1717
import { getErrorMessage } from '@sim/utils/errors'
1818
import { generateId } from '@sim/utils/id'
1919
import { eq } from 'drizzle-orm'
@@ -56,7 +56,9 @@ let http: HttpServer
5656
let io: Server
5757
let manager: MemoryRoomManager
5858
let socketUrl: string
59-
let failProjectAccess = false
59+
let failAccessStatus: number | null = null
60+
/** Users whose permission group withholds the Files module, as the app's access route decides. */
61+
const filesWithheld = new Set<string>()
6062

6163
async function listen(server: HttpServer) {
6264
await new Promise<void>((done) => server.listen(0, '127.0.0.1', done))
@@ -113,8 +115,28 @@ beforeAll(async () => {
113115
const fileId = project ? path.split('/')[5] : body.fileId
114116
const actor = project ? request.headers['x-sim-subject-user-id'] : body.userId
115117
if (path.endsWith('/access')) {
116-
if (failProjectAccess) {
117-
response.writeHead(503).end('{}')
118+
if (failAccessStatus !== null) {
119+
response.writeHead(failAccessStatus).end('{}')
120+
return
121+
}
122+
if (!project) {
123+
const authorization = await roomAuthorization.authorizeRoom({
124+
userId: actor,
125+
room: { type: ROOM_TYPES.WORKSPACE_FILE_DOC, id: fileId },
126+
})
127+
if (!authorization.allowed || filesWithheld.has(actor)) {
128+
response.end(
129+
JSON.stringify({ allowed: false, status: authorization.status === 404 ? 404 : 403 })
130+
)
131+
return
132+
}
133+
response.end(
134+
JSON.stringify({
135+
allowed: true,
136+
workspaceId: authorization.workspaceId,
137+
workspacePermission: authorization.workspacePermission,
138+
})
139+
)
118140
return
119141
}
120142
response.end(
@@ -380,13 +402,7 @@ for (const scope of ['workspace', 'project'] as const)
380402
const writer = await join(target, writerId)
381403
const now = Date.now()
382404
const clock = vi.spyOn(Date, 'now').mockReturnValue(now + ROLE_REVALIDATION_TTL_MS + 1)
383-
const authorization =
384-
scope === 'workspace'
385-
? vi
386-
.spyOn(roomAuthorization, 'authorizeRoom')
387-
.mockRejectedValue(new Error('Fixture authorization outage'))
388-
: null
389-
failProjectAccess = true
405+
failAccessStatus = 503
390406
try {
391407
expect(
392408
await writer.socket.timeout(3000).emitWithAck(FILE_DOC_EVENTS.UPDATE, {
@@ -400,14 +416,60 @@ for (const scope of ['workspace', 'project'] as const)
400416
io.sockets.sockets.get(writer.socket.id ?? '')?.rooms.has(roomName(fileDocRoom(target)))
401417
).toBe(true)
402418
} finally {
403-
authorization?.mockRestore()
404419
clock.mockRestore()
405-
failProjectAccess = false
420+
failAccessStatus = null
406421
}
407422
}
408423
)
409424
})
410425

426+
describe('workspace document rooms answer to the Files capability', () => {
427+
check('a member whose permission group withholds Files is refused the room', async () => {
428+
const target = await fixture('workspace')
429+
filesWithheld.add(writerId)
430+
try {
431+
await expect(join(target, writerId)).rejects.toThrow('ACCESS_DENIED')
432+
} finally {
433+
filesWithheld.delete(writerId)
434+
}
435+
})
436+
437+
check('an app that predates the access route holds the room instead of revoking it', async () => {
438+
const target = await fixture('workspace')
439+
const writer = await join(target, writerId)
440+
const room = fileDocRoom(target)
441+
failAccessStatus = 404
442+
const clock = vi.spyOn(Date, 'now').mockReturnValue(Date.now() + ROLE_REVALIDATION_TTL_MS + 1)
443+
const sweep = startAccessRevalidationSweep(manager)
444+
try {
445+
await expect(join(target, readerId)).rejects.toThrow('VERIFY_ACCESS_FAILED')
446+
await sweep.runOnce()
447+
expect(io.sockets.sockets.get(writer.socket.id ?? '')?.rooms.has(roomName(room))).toBe(true)
448+
} finally {
449+
sweep.stop()
450+
clock.mockRestore()
451+
failAccessStatus = null
452+
}
453+
})
454+
455+
check('withdrawing Files mid-session evicts the member on the next revalidation', async () => {
456+
const target = await fixture('workspace')
457+
const writer = await join(target, writerId)
458+
const room = fileDocRoom(target)
459+
filesWithheld.add(writerId)
460+
const clock = vi.spyOn(Date, 'now').mockReturnValue(Date.now() + ROLE_REVALIDATION_TTL_MS + 1)
461+
const sweep = startAccessRevalidationSweep(manager)
462+
try {
463+
await sweep.runOnce()
464+
expect(io.sockets.sockets.get(writer.socket.id ?? '')?.rooms.has(roomName(room))).toBe(false)
465+
} finally {
466+
sweep.stop()
467+
clock.mockRestore()
468+
filesWithheld.delete(writerId)
469+
}
470+
})
471+
})
472+
411473
afterAll(async () => {
412474
for (const socket of clients) socket.disconnect()
413475
if (io) await new Promise<void>((done) => io.close(() => done()))

‎apps/realtime/src/handlers/file-doc.multireplica.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const fakeStore = {
3737
vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: vi.fn() }))
3838

3939
vi.mock('@/handlers/file-doc-app', () => ({
40+
fetchFileDocAccess: vi.fn(),
4041
fetchFileDocSeed: vi.fn(),
4142
fetchFileDocMerge: mockFetchFileDocMerge,
4243
fetchFileDocPersist: vi.fn(),

‎apps/realtime/src/handlers/file-doc.redis-store.test.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@ import * as syncProtocol from 'y-protocols/sync'
2020
import * as Y from 'yjs'
2121
import type { IRoomManager } from '@/rooms'
2222

23-
const { mockAuthorizeRoom, mockFetchFileDocSeed, mockFetchFileDocPersist } = vi.hoisted(() => ({
24-
mockAuthorizeRoom: vi.fn(),
25-
mockFetchFileDocSeed: vi.fn(),
26-
mockFetchFileDocPersist: vi.fn(),
27-
}))
23+
const { mockFetchFileDocAccess, mockFetchFileDocSeed, mockFetchFileDocPersist } = vi.hoisted(
24+
() => ({
25+
mockFetchFileDocAccess: vi.fn(),
26+
mockFetchFileDocSeed: vi.fn(),
27+
mockFetchFileDocPersist: vi.fn(),
28+
})
29+
)
2830

29-
vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: mockAuthorizeRoom }))
31+
vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: vi.fn() }))
3032

3133
vi.mock('@/handlers/file-doc-app', () => ({
34+
fetchFileDocAccess: mockFetchFileDocAccess,
3235
fetchFileDocSeed: mockFetchFileDocSeed,
3336
fetchFileDocMerge: vi.fn(),
3437
fetchFileDocPersist: mockFetchFileDocPersist,
@@ -263,7 +266,7 @@ describe('file-doc join readiness (shared store enabled)', () => {
263266
backing.kv.clear()
264267
backing.seq = 0
265268
backing.readDelayTicks = 0
266-
mockAuthorizeRoom.mockResolvedValue({
269+
mockFetchFileDocAccess.mockResolvedValue({
267270
allowed: true,
268271
status: 200,
269272
workspaceId: 'ws-1',

‎apps/realtime/src/handlers/file-doc.test.ts‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,22 @@ import * as syncProtocol from 'y-protocols/sync'
1616
import * as Y from 'yjs'
1717
import type { IRoomManager } from '@/rooms'
1818

19-
const { mockAuthorizeRoom, mockFetchFileDocSeed, mockFetchFileDocMerge, mockFetchFileDocPersist } =
20-
vi.hoisted(() => ({
21-
mockAuthorizeRoom: vi.fn(),
22-
mockFetchFileDocSeed: vi.fn(),
23-
mockFetchFileDocMerge: vi.fn(),
24-
mockFetchFileDocPersist: vi.fn(),
25-
}))
26-
27-
vi.mock('@sim/platform-authz/rooms', () => ({
28-
authorizeRoom: mockAuthorizeRoom,
19+
const {
20+
mockFetchFileDocAccess,
21+
mockFetchFileDocSeed,
22+
mockFetchFileDocMerge,
23+
mockFetchFileDocPersist,
24+
} = vi.hoisted(() => ({
25+
mockFetchFileDocAccess: vi.fn(),
26+
mockFetchFileDocSeed: vi.fn(),
27+
mockFetchFileDocMerge: vi.fn(),
28+
mockFetchFileDocPersist: vi.fn(),
2929
}))
3030

31+
vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: vi.fn() }))
32+
3133
vi.mock('@/handlers/file-doc-app', () => ({
34+
fetchFileDocAccess: mockFetchFileDocAccess,
3235
fetchFileDocSeed: mockFetchFileDocSeed,
3336
fetchFileDocMerge: mockFetchFileDocMerge,
3437
fetchFileDocPersist: mockFetchFileDocPersist,
@@ -213,7 +216,7 @@ function joinSuccessFileId(socket: { emit: ReturnType<typeof vi.fn> }) {
213216

214217
describe('setupWorkspaceFileDocHandlers', () => {
215218
beforeEach(() => {
216-
mockAuthorizeRoom.mockResolvedValue({
219+
mockFetchFileDocAccess.mockResolvedValue({
217220
allowed: true,
218221
status: 200,
219222
workspaceId: 'ws-1',
@@ -251,7 +254,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
251254
})
252255

253256
it('fails closed when authorization does not resolve a workspace context', async () => {
254-
mockAuthorizeRoom.mockResolvedValueOnce({
257+
mockFetchFileDocAccess.mockResolvedValueOnce({
255258
allowed: true,
256259
status: 200,
257260
workspacePermission: 'write',
@@ -445,7 +448,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
445448
const { io } = createIo()
446449
const { socket, handlers } = setup('socket-race', io, { userId: 'user-race' })
447450

448-
mockAuthorizeRoom.mockImplementation(async () => {
451+
mockFetchFileDocAccess.mockImplementation(async () => {
449452
// Simulate the revocation landing between this join's authorize and its commit,
450453
// exactly as the sweep would record it.
451454
commitRoomPermission(
@@ -479,16 +482,21 @@ describe('setupWorkspaceFileDocHandlers', () => {
479482
const { io } = createIo()
480483
const { socket, handlers } = setup('socket-stale', io, { userId: 'user-stale' })
481484

482-
mockAuthorizeRoom.mockImplementation(async ({ action }: { action: string }) => {
483-
// The authoritative current answer: access is gone.
484-
if (action !== 'write')
485-
return { allowed: false, status: 403, workspaceId: 'ws-1', workspacePermission: null }
485+
mockFetchFileDocAccess
486486
// This join's own authorize saw the pre-revocation state, and the sweep records
487487
// the revocation (later read ticket) while it is still in flight.
488-
commitRoomPermission('user-stale', room, null, beginRoomPermissionRead())
489-
await sleep(31_000)
490-
return { allowed: true, status: 200, workspaceId: 'ws-1', workspacePermission: 'write' }
491-
})
488+
.mockImplementationOnce(async () => {
489+
commitRoomPermission('user-stale', room, null, beginRoomPermissionRead())
490+
await sleep(31_000)
491+
return { allowed: true, status: 200, workspaceId: 'ws-1', workspacePermission: 'write' }
492+
})
493+
// The authoritative current answer: access is gone.
494+
.mockResolvedValue({
495+
allowed: false,
496+
status: 403,
497+
workspaceId: 'ws-1',
498+
workspacePermission: null,
499+
})
492500

493501
const joining = handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-stale', clientId: 1 })
494502
await vi.advanceTimersByTimeAsync(31_000)
@@ -597,7 +605,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
597605
await vi.advanceTimersByTimeAsync(0)
598606

599607
// Access is downgraded to read-only, and the cached join-time decision expires.
600-
mockAuthorizeRoom.mockResolvedValue({
608+
mockFetchFileDocAccess.mockResolvedValue({
601609
allowed: false,
602610
status: 403,
603611
workspaceId: 'ws-1',
@@ -663,7 +671,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
663671
.filter(([event]) => event === FILE_DOC_EVENTS.PERMISSION)
664672
.map(([, payload]) => (payload as { canWrite: boolean }).canWrite)
665673
const changeRole = async (workspacePermission: 'read' | 'write') => {
666-
mockAuthorizeRoom.mockResolvedValue({
674+
mockFetchFileDocAccess.mockResolvedValue({
667675
allowed: true,
668676
status: 200,
669677
workspaceId: 'ws-1',
@@ -1144,7 +1152,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
11441152
it('aborts a join superseded by a newer join during authorization (no cross-binding)', async () => {
11451153
const { io } = createIo()
11461154
let resolveFirst: (v: unknown) => void = () => {}
1147-
mockAuthorizeRoom
1155+
mockFetchFileDocAccess
11481156
.mockReturnValueOnce(new Promise((resolve) => (resolveFirst = resolve)))
11491157
.mockResolvedValueOnce({
11501158
allowed: true,
@@ -1172,7 +1180,7 @@ describe('setupWorkspaceFileDocHandlers', () => {
11721180
it('cancels an in-flight join when the client leaves that same file (no ghost owner)', async () => {
11731181
const { io, sent } = createIo()
11741182
let resolveAuth: (v: unknown) => void = () => {}
1175-
mockAuthorizeRoom.mockReturnValueOnce(new Promise((resolve) => (resolveAuth = resolve)))
1183+
mockFetchFileDocAccess.mockReturnValueOnce(new Promise((resolve) => (resolveAuth = resolve)))
11761184
const s = setup('socket-a', io)
11771185

11781186
// Join file-1 is awaiting authorization when the client leaves file-1 (fast open→close).

‎apps/realtime/src/handlers/room-join-auth.ts‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import type { createLogger } from '@sim/logger'
2-
import { authorizeRoom } from '@sim/platform-authz/rooms'
3-
import { isProjectRoom, type RoomRef } from '@sim/realtime-protocol/rooms'
4-
import { fetchProjectRoomAccess } from '@/handlers/file-list-app'
5-
import { beginRoomPermissionRead, commitRoomPermission } from '@/middleware/permissions'
2+
import type { RoomRef } from '@sim/realtime-protocol/rooms'
3+
import {
4+
authorizeRoomAccess,
5+
beginRoomPermissionRead,
6+
commitRoomPermission,
7+
} from '@/middleware/permissions'
68

7-
type Authorized = Awaited<ReturnType<typeof authorizeRoom>> & { docId?: string | null }
9+
type Authorized = Awaited<ReturnType<typeof authorizeRoomAccess>>
810

911
interface ResolveRoomJoinAuthParams {
1012
userId: string
@@ -27,7 +29,7 @@ interface ResolveRoomJoinAuthParams {
2729
* Deliberately excludes the auth/readiness/id-validation preamble and the join-generation
2830
* capture/recheck — those differ per handler, and for file-doc the generation capture sits
2931
* mid-preamble. This helper is always invoked strictly between a handler's generation capture
30-
* and its post-authorize recheck; it contains exactly the one `await authorizeRoom` that the
32+
* and its post-authorize recheck; it contains exactly the one `await authorizeRoomAccess` that the
3133
* recheck was designed to cover and returns before any state mutation, so it never straddles
3234
* that seam. Pass each handler's own `logger` so the log namespace/request-id context is kept.
3335
*/
@@ -42,15 +44,7 @@ export async function resolveRoomJoinAuth(
4244
// is never overwritten by this older result — see {@link commitRoomPermission}.
4345
const readSeq = beginRoomPermissionRead()
4446
try {
45-
if (isProjectRoom(room)) {
46-
if (!params.connectionId) throw new Error('Project room requires a socket identity')
47-
authorized = await fetchProjectRoomAccess(room, {
48-
userId,
49-
connectionId: params.connectionId,
50-
})
51-
} else {
52-
authorized = await authorizeRoom({ userId, room, action })
53-
}
47+
authorized = await authorizeRoomAccess(userId, room, action, params.connectionId)
5448
} catch (error) {
5549
logger.warn(`Error authorizing ${logLabel}:`, error)
5650
emitError({ error: messages.verifyFailed, code: 'VERIFY_ACCESS_FAILED', retryable: true })

0 commit comments

Comments
 (0)