Skip to content

Commit ea0b5d5

Browse files
committed
fix(security): RealtimeController — add @NoAdminRequired + per-org cursor
Two issues, one commit: 1. Both `events()` and `cursor()` only carried `@NoCSRFRequired`. The docblock contract assumes non-admins can poll, but without `@NoAdminRequired` the framework gate would have blocked them before the body ran. 2. `cursor()` returned `getMaxId()` — the global head pointer across every tenant. Any authed caller could observe the global write rate (and infer other tenants' activity) by polling. Add `@NoAdminRequired` to both methods. Replace `getMaxId()` with a new `getMaxIdForOrganisation()` mapper method scoped to the active organisation. Active org `null` ⇒ cursor `0` (fail-closed). Refs: #1419 review (concern 2) — discussion_r3187494457
1 parent a9759fe commit ea0b5d5

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

lib/Controller/RealtimeController.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public function __construct(
7272
*
7373
* @return JSONResponse JSON response with events, cursor, and hasMore.
7474
*
75+
* @NoAdminRequired
7576
* @NoCSRFRequired
7677
*/
7778
public function events(
@@ -136,13 +137,31 @@ public function events(
136137
*
137138
* @return JSONResponse JSON response containing the current head cursor.
138139
*
140+
* @NoAdminRequired
139141
* @NoCSRFRequired
140142
*/
141143
public function cursor(): JSONResponse
142144
{
145+
// SECURITY: scope the cursor to the caller's active organisation.
146+
// Returning the global head pointer let any authed caller observe
147+
// the global write rate by polling — small but real cross-tenant
148+
// side channel. With no active org, return 0 (fail-closed) so
149+
// there is no head pointer to mine.
150+
$orgUuid = null;
151+
try {
152+
$activeOrg = $this->organisationService->getActiveOrganisation();
153+
$orgUuid = $activeOrg?->getUuid();
154+
} catch (\Throwable $e) {
155+
$orgUuid = null;
156+
}
157+
158+
if ($orgUuid === null) {
159+
return new JSONResponse(['cursor' => 0]);
160+
}
161+
143162
return new JSONResponse(
144163
[
145-
'cursor' => $this->eventMapper->getMaxId(),
164+
'cursor' => $this->eventMapper->getMaxIdForOrganisation($orgUuid),
146165
]
147166
);
148167
}//end cursor()

lib/Db/RealtimeEventMapper.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,33 @@ public function getMaxId(): int
115115
return (int) ($result['max_id'] ?? 0);
116116
}//end getMaxId()
117117

118+
/**
119+
* Get the highest event id scoped to a specific organisation.
120+
*
121+
* SECURITY: the unfiltered `getMaxId()` returns the global head
122+
* pointer, which lets any caller observe the global write rate by
123+
* polling. Per-organisation cursors prevent that side-channel.
124+
*
125+
* @param string $organisationUuid Organisation UUID.
126+
*
127+
* @return int Highest event id for the organisation, or 0 when none.
128+
*/
129+
public function getMaxIdForOrganisation(string $organisationUuid): int
130+
{
131+
$qb = $this->db->getQueryBuilder();
132+
$qb->select($qb->createFunction('MAX(id) AS max_id'))
133+
->from('openregister_realtime_events')
134+
->where(
135+
$qb->expr()->eq(
136+
'organisation',
137+
$qb->createNamedParameter($organisationUuid)
138+
)
139+
);
140+
$result = $qb->executeQuery()->fetch();
141+
return (int) ($result['max_id'] ?? 0);
142+
143+
}//end getMaxIdForOrganisation()
144+
118145
/**
119146
* Prune events older than `$retentionSeconds`. Used by a daily TimedJob
120147
* to keep the event log bounded.

0 commit comments

Comments
 (0)