Skip to content

Commit c10828b

Browse files
committed
fix(security): PermissionHandler — bypass cache when schema has match rules
The per-request permission cache keys verdicts on `(schemaId, action, userId, objectOwner, objectUuid)` and reuses them within the request lifecycle. That is safe for schemas whose authorization is purely group/role based, but unsafe for schemas whose auth block contains a `match: { … }` clause: the rule reads from `$object->getObject()`, which can mutate within a single request via `saveObject()` / TransitionEngine. A write-then-write pattern (e.g. batch update) would otherwise return a stale verdict based on the pre-write field values. Add `schemaHasMatchRule()` helper that walks the schema's authorization block once and reports whether any entry carries a non-empty `match`. `buildPermissionCacheKey()` returns null when it does, forcing each call to re-evaluate the rule chain against fresh object data. Cost: a hot-path per-request cache miss for the specific schemas that opt into conditional rules — acceptable trade versus the correctness regression. Refs: #1419 review (concern 5) — discussion_r3187494474
1 parent bb8cd7f commit c10828b

1 file changed

Lines changed: 54 additions & 2 deletions

File tree

lib/Service/Object/PermissionHandler.php

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ public function hasPermission(
210210
action: $action,
211211
userId: $userId,
212212
objectOwner: $objectOwner,
213-
object: $object
213+
object: $object,
214+
schema: $schema
214215
);
215216
if ($cacheKey !== null && array_key_exists($cacheKey, $this->permissionCache) === true) {
216217
return $this->permissionCache[$cacheKey];
@@ -253,12 +254,24 @@ private function buildPermissionCacheKey(
253254
string $action,
254255
?string $userId,
255256
?string $objectOwner,
256-
?ObjectEntity $object
257+
?ObjectEntity $object,
258+
?Schema $schema=null
257259
): ?string {
258260
if ($schemaId === null) {
259261
return null;
260262
}
261263

264+
// SECURITY: when the schema's authorization block contains any
265+
// `match` rule, the verdict depends on the *current* object data
266+
// — which may change within a single request via saveObject() /
267+
// TransitionEngine. Cache reuse keyed on the (stable) object UUID
268+
// would otherwise serve a pre-mutation verdict to a post-mutation
269+
// re-check. Drop the cache for schemas with match rules so each
270+
// call re-evaluates the rule chain against fresh data.
271+
if ($schema !== null && $this->schemaHasMatchRule(schema: $schema) === true) {
272+
return null;
273+
}
274+
262275
$objectUuid = null;
263276
if ($object !== null) {
264277
$objectUuid = $object->getUuid();
@@ -278,6 +291,45 @@ private function buildPermissionCacheKey(
278291
);
279292
}//end buildPermissionCacheKey()
280293

294+
/**
295+
* Detect whether a schema's authorization block contains any
296+
* conditional `match` rules.
297+
*
298+
* Used to disable the per-request permission cache for schemas
299+
* whose verdict depends on the current object data — see
300+
* {@see buildPermissionCacheKey()}.
301+
*
302+
* @param Schema $schema Schema to inspect.
303+
*
304+
* @return bool True when at least one authorization entry carries
305+
* a non-empty `match` block.
306+
*/
307+
private function schemaHasMatchRule(Schema $schema): bool
308+
{
309+
$authorization = $schema->getAuthorization();
310+
if (is_array($authorization) === false || $authorization === []) {
311+
return false;
312+
}
313+
314+
foreach ($authorization as $action => $entries) {
315+
if (is_array($entries) === false) {
316+
continue;
317+
}
318+
319+
foreach ($entries as $entry) {
320+
if (is_array($entry) === true
321+
&& isset($entry['match']) === true
322+
&& empty($entry['match']) === false
323+
) {
324+
return true;
325+
}
326+
}
327+
}
328+
329+
return false;
330+
331+
}//end schemaHasMatchRule()
332+
281333
/**
282334
* Evaluate the full RBAC rule chain for a permission check.
283335
*

0 commit comments

Comments
 (0)