Skip to content

Commit 0fd8b1f

Browse files
committed
test(realtime): assert the role ACL against production, not a fixture
The shared ROLE_ALLOWED_OPERATIONS fixture still listed the two position operations for the read role, and three tests compared that fixture against itself — so they certified whatever it said, including the grants this PR removes. They now assert checkRolePermission over the protocol's complete operation list (the fixture's copy omits subblock/variable/admin-only ops), plus one test that pins the fixture to the production ACL so the two cannot drift apart again.
1 parent 4642715 commit 0fd8b1f

2 files changed

Lines changed: 45 additions & 23 deletions

File tree

apps/realtime/src/middleware/permissions.test.ts

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* - Edge cases and invalid inputs
88
*/
99

10+
import { ALL_SOCKET_OPERATIONS } from '@sim/realtime-protocol/constants'
1011
import {
1112
expectPermissionAllowed,
1213
expectPermissionDenied,
@@ -163,7 +164,7 @@ describe('checkRolePermission', () => {
163164
// Every operation reaching this gate is persisted, so a read-only member must
164165
// hold none of them — including the position updates that used to be granted
165166
// here on the mistaken premise that they were ephemeral cursor sync.
166-
for (const operation of SOCKET_OPERATIONS) {
167+
for (const operation of ALL_SOCKET_OPERATIONS) {
167168
const result = checkRolePermission('read', operation)
168169
expect(result.allowed).toBe(false)
169170
expect(result.reason).toContain('read')
@@ -211,28 +212,42 @@ describe('checkRolePermission', () => {
211212
})
212213

213214
describe('permission hierarchy verification', () => {
214-
it('should verify admin has same permissions as write', () => {
215-
const adminOps = ROLE_ALLOWED_OPERATIONS.admin
216-
const writeOps = ROLE_ALLOWED_OPERATIONS.write
217-
218-
// Admin and write should have same operations
219-
expect(adminOps).toEqual(writeOps)
220-
})
221-
222-
it('should verify read is a subset of write permissions', () => {
223-
const readOps = ROLE_ALLOWED_OPERATIONS.read
224-
const writeOps = ROLE_ALLOWED_OPERATIONS.write
225-
226-
for (const op of readOps) {
227-
expect(writeOps).toContain(op)
215+
// These assert the PRODUCTION ACL over the protocol's complete operation list.
216+
// They used to compare the shared test fixture against itself, which certified
217+
// whatever the fixture said — including, for a while, the read-role grants that
218+
// let a read-only member persist block positions.
219+
220+
it('grants admin everything write has, plus the admin-only operations', () => {
221+
for (const operation of ALL_SOCKET_OPERATIONS) {
222+
if (checkRolePermission('write', operation).allowed) {
223+
expect(checkRolePermission('admin', operation).allowed).toBe(true)
224+
}
225+
}
226+
// Strictly greater: at least one operation admin holds and write does not.
227+
const adminOnly = ALL_SOCKET_OPERATIONS.filter(
228+
(operation) =>
229+
checkRolePermission('admin', operation).allowed &&
230+
!checkRolePermission('write', operation).allowed
231+
)
232+
expect(adminOnly.length).toBeGreaterThan(0)
233+
})
234+
235+
it('grants read nothing, so it is trivially a subset of write', () => {
236+
const readAllowed = ALL_SOCKET_OPERATIONS.filter(
237+
(operation) => checkRolePermission('read', operation).allowed
238+
)
239+
expect(readAllowed).toEqual([])
240+
})
241+
242+
it('keeps the shared fixture in step with the production ACL', () => {
243+
// The fixture is a convenience mirror; drift between it and the real table is
244+
// what made the stale read grants look intentional.
245+
for (const operation of ALL_SOCKET_OPERATIONS) {
246+
const fixtureAllows = ROLE_ALLOWED_OPERATIONS.read.includes(
247+
operation as (typeof ROLE_ALLOWED_OPERATIONS.read)[number]
248+
)
249+
expect(fixtureAllows).toBe(checkRolePermission('read', operation).allowed)
228250
}
229-
})
230-
231-
it('should verify read has minimal permissions', () => {
232-
const readOps = ROLE_ALLOWED_OPERATIONS.read
233-
expect(readOps).toHaveLength(2)
234-
expect(readOps).toContain('update-position')
235-
expect(readOps).toContain('batch-update-positions')
236251
})
237252
})
238253

packages/testing/src/factories/permission.factory.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,18 @@ export type SocketOperation = (typeof SOCKET_OPERATIONS)[number]
307307

308308
/**
309309
* Operations allowed for each role.
310+
*
311+
* A convenience mirror for fixtures — NOT the authority. The real ACL lives in
312+
* `apps/realtime/src/middleware/permissions.ts`; assert against
313+
* `checkRolePermission` rather than this table, or a drift between the two turns
314+
* into a test that certifies whatever the fixture happens to say. (`read` listed
315+
* the two position operations here while production had already granted them for
316+
* real; both are persisted writes and neither role should hold them.)
310317
*/
311318
export const ROLE_ALLOWED_OPERATIONS: Record<PermissionType, readonly SocketOperation[]> = {
312319
admin: SOCKET_OPERATIONS,
313320
write: SOCKET_OPERATIONS,
314-
read: [BLOCK_OPERATIONS.UPDATE_POSITION, BLOCKS_OPERATIONS.BATCH_UPDATE_POSITIONS],
321+
read: [],
315322
}
316323

317324
/**

0 commit comments

Comments
 (0)