Skip to content

Commit 817d0e4

Browse files
committed
fix(symfony): security regression when ResourceAccessChecker is decorated (#7896)
Commit 359a128 introduced a regression when ResourceAccessChecker is decorated, and security/securityPostDenormalize are using object in is_granted expression. The issue arise since AccessCheckerProvider violates the Liskov substitution principle by assuming that if the (previously unknown) interface ObjectVariableCheckerInterface is not defined, then the pre_read optimization can be used without an object instance.
1 parent 7cc01a6 commit 817d0e4

File tree

1 file changed

+11
-4
lines changed

1 file changed

+11
-4
lines changed

src/Symfony/Security/State/AccessCheckerProvider.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
6868
return $this->decorated->provide($operation, $uriVariables, $context);
6969
}
7070

71+
// Skip pre_read optimization when object is used via granted (or usage is not predicatable)
72+
if (
73+
'pre_read' === $this->event
74+
&& (
75+
!$this->resourceAccessChecker instanceof ObjectVariableCheckerInterface
76+
|| !$this->resourceAccessChecker->usesObjectVariable($isGranted, ['object' => null, 'previous_object' => null])
77+
)
78+
) {
79+
return $this->decorated->provide($operation, $uriVariables, $context);
80+
}
81+
7182
$body = 'pre_read' === $this->event ? null : $this->decorated->provide($operation, $uriVariables, $context);
7283

7384
if ($operation instanceof HttpOperation) {
@@ -85,10 +96,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
8596
];
8697
}
8798

88-
if ('pre_read' === $this->event && $this->resourceAccessChecker instanceof ObjectVariableCheckerInterface && $this->resourceAccessChecker->usesObjectVariable($isGranted, $resourceAccessCheckerContext)) {
89-
return $this->decorated->provide($operation, $uriVariables, $context);
90-
}
91-
9299
if (!$this->resourceAccessChecker->isGranted($operation->getClass(), $isGranted, $resourceAccessCheckerContext)) {
93100
$operation instanceof GraphQlOperation ? throw new AccessDeniedHttpException($message ?? 'Access Denied.') : throw new AccessDeniedException($message ?? 'Access Denied.');
94101
}

0 commit comments

Comments
 (0)