diff --git a/src/Authorization/AuthorizationDataMuxer.php b/src/Authorization/AuthorizationDataMuxer.php index ce6aed51a7859a4348b945903fa7fa48bc4d589b..e6e811af2d9462d93b249b178c6cd5efdee9206f 100644 --- a/src/Authorization/AuthorizationDataMuxer.php +++ b/src/Authorization/AuthorizationDataMuxer.php @@ -129,10 +129,14 @@ class AuthorizationDataMuxer $event = new GetAttributeEvent($this, $attributeName, $value, $userIdentifier); $event->setAttributeValue($value); - // Avoid endless recursions by only emitting an event for each attribute only once - if (!in_array($attributeName, $this->attributeStack, true)) { - array_push($this->attributeStack, $attributeName); + // Prevent endless recursions by only emitting an event for each attribute only once + if (in_array($attributeName, $this->attributeStack, true)) { + throw new AuthorizationException(sprintf('infinite loop caused by a %s subscriber. authorization attribute: %s', GetAttributeEvent::class, $attributeName), AuthorizationException::INFINITE_EVENT_LOOP_DETECTED); + } + array_push($this->attributeStack, $attributeName); + try { $this->eventDispatcher->dispatch($event); + } finally { array_pop($this->attributeStack); } diff --git a/src/Authorization/AuthorizationException.php b/src/Authorization/AuthorizationException.php index 2e5b41c1b96ed6787d79b650e068e27b5ac1db68..eecf525dfc09e8531d8410fdece3a8ac1c4599af 100644 --- a/src/Authorization/AuthorizationException.php +++ b/src/Authorization/AuthorizationException.php @@ -6,7 +6,8 @@ namespace Dbp\Relay\CoreBundle\Authorization; class AuthorizationException extends \RuntimeException { - public const PRIVILEGE_UNDEFINED = 2; - public const ATTRIBUTE_UNDEFINED = 3; - public const INFINITE_LOOP_DETECTED = 4; + public const PRIVILEGE_UNDEFINED = 1; + public const ATTRIBUTE_UNDEFINED = 2; + public const INFINITE_EXRPESSION_LOOP_DETECTED = 3; + public const INFINITE_EVENT_LOOP_DETECTED = 4; } diff --git a/src/Authorization/AuthorizationExpressionChecker.php b/src/Authorization/AuthorizationExpressionChecker.php index 9471bd8af14a74bd25697d21f9a8c87b71bc3057..545edd82523ace570baec86420d7dfd0e3741d04 100644 --- a/src/Authorization/AuthorizationExpressionChecker.php +++ b/src/Authorization/AuthorizationExpressionChecker.php @@ -31,6 +31,12 @@ class AuthorizationExpressionChecker /** @var AuthorizationDataMuxer */ private $dataMux; + /** @var array */ + private $rightExpressionStack; + + /** @var array */ + private $attributeExpressionStack; + public function __construct(AuthorizationDataMuxer $dataMux) { $this->expressionLanguage = new ExpressionLanguage(); @@ -38,6 +44,8 @@ class AuthorizationExpressionChecker $this->rightExpressions = []; $this->attributeExpressions = []; $this->dataMux = $dataMux; + $this->rightExpressionStack = []; + $this->attributeExpressionStack = []; } public function setConfig(array $config) @@ -58,17 +66,24 @@ class AuthorizationExpressionChecker */ public function evalAttributeExpression(AuthorizationUser $currentAuthorizationUser, string $expressionName, $defaultValue = null) { - $this->tryIncreaseRecursionCounter($expressionName); - - if (($expression = $this->attributeExpressions[$expressionName] ?? null) !== null) { - $result = $this->expressionLanguage->evaluate($expression, [ - 'user' => $currentAuthorizationUser, - ]); - } else { - throw new AuthorizationException(sprintf('expression \'%s\' undefined', $expressionName), AuthorizationException::ATTRIBUTE_UNDEFINED); + if (in_array($expressionName, $this->attributeExpressionStack, true)) { + throw new AuthorizationException(sprintf('infinite loop caused by authorization attribute expression %s detected', $expressionName), AuthorizationException::INFINITE_EXRPESSION_LOOP_DETECTED); + } + array_push($this->attributeExpressionStack, $expressionName); + + try { + if (($expression = $this->attributeExpressions[$expressionName] ?? null) !== null) { + $result = $this->expressionLanguage->evaluate($expression, [ + 'user' => $currentAuthorizationUser, + ]); + } else { + throw new AuthorizationException(sprintf('expression \'%s\' undefined', $expressionName), AuthorizationException::ATTRIBUTE_UNDEFINED); + } + + return $result ?? $defaultValue; + } finally { + array_pop($this->attributeExpressionStack); } - - return $result ?? $defaultValue; } /** @@ -91,17 +106,24 @@ class AuthorizationExpressionChecker */ public function isGranted(AuthorizationUser $currentAuthorizationUser, string $rightName, $subject): bool { - $this->tryIncreaseRecursionCounter($rightName); - - $rightExpression = $this->rightExpressions[$rightName] ?? null; - if ($rightExpression === null) { - throw new AuthorizationException(sprintf('right \'%s\' undefined', $rightName), AuthorizationException::PRIVILEGE_UNDEFINED); + if (in_array($rightName, $this->rightExpressionStack, true)) { + throw new AuthorizationException(sprintf('infinite loop caused by authorization right expression %s detected', $rightName), AuthorizationException::INFINITE_EXRPESSION_LOOP_DETECTED); } + array_push($this->rightExpressionStack, $rightName); + + try { + $rightExpression = $this->rightExpressions[$rightName] ?? null; + if ($rightExpression === null) { + throw new AuthorizationException(sprintf('right \'%s\' undefined', $rightName), AuthorizationException::PRIVILEGE_UNDEFINED); + } - return $this->expressionLanguage->evaluate($rightExpression, [ - 'user' => $currentAuthorizationUser, - 'subject' => $subject, - ]); + return $this->expressionLanguage->evaluate($rightExpression, [ + 'user' => $currentAuthorizationUser, + 'subject' => $subject, + ]); + } finally { + array_pop($this->rightExpressionStack); + } } private function loadExpressions(array $expressions, array &$target): void @@ -110,14 +132,4 @@ class AuthorizationExpressionChecker $target[$name] = $expression; } } - - /** - * @throws AuthorizationException - */ - private function tryIncreaseRecursionCounter(string $hint): void - { - if (++$this->callCounter > self::MAX_NUM_CALLS) { - throw new AuthorizationException(sprintf('possible infinite loop in authorization expression detected (%s)', $hint), AuthorizationException::INFINITE_LOOP_DETECTED); - } - } } diff --git a/src/Authorization/EventSubscriber/AbstractGetAttributeSubscriber.php b/src/Authorization/EventSubscriber/AbstractGetAttributeSubscriber.php index bb02caf1270b0198d71a9037ca5716dfd03a3ace..fadd9913c4201ab2230807f5ba9c60781e26b214 100644 --- a/src/Authorization/EventSubscriber/AbstractGetAttributeSubscriber.php +++ b/src/Authorization/EventSubscriber/AbstractGetAttributeSubscriber.php @@ -10,8 +10,8 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterface { - /** @var GetAttributeEvent|null */ - private $event; + /** @var array */ + private $eventStack; public static function getSubscribedEvents(): array { @@ -21,6 +21,11 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac ]; } + public function __construct() + { + $this->eventStack = []; + } + public function onGetAvailableAttributes(GetAvailableAttributesEvent $event) { $event->addAttributes($this->getNewAttributes()); @@ -29,7 +34,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac public function onGetAttributeEvent(GetAttributeEvent $event) { try { - $this->event = $event; + array_push($this->eventStack, $event); $attributeName = $event->getAttributeName(); $event->setAttributeValue(in_array($attributeName, $this->getNewAttributes(), true) ? @@ -37,7 +42,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac $this->updateExistingAttributeValue($event->getUserIdentifier(), $attributeName, $event->getAttributeValue()) ); } finally { - $this->event = null; + array_pop($this->eventStack); } } @@ -48,7 +53,7 @@ abstract class AbstractGetAttributeSubscriber implements EventSubscriberInterfac */ public function getAttribute(string $attributeName, $defaultValue = null) { - return $this->event->getAttribute($attributeName, $defaultValue); + return $this->eventStack[array_key_last($this->eventStack)]->getAttribute($attributeName, $defaultValue); } /**