From e381102aa4f52587a23c26850c9920a99340dd59 Mon Sep 17 00:00:00 2001 From: Davi Alexandre Date: Fri, 17 Mar 2017 16:40:15 -0300 Subject: [PATCH] CRM-20210: Improve authorization check for dynamic fks Instead of actually calling the delegated action in order to check if it's authorized or not, we now use Kernel->runAuthorize() to do it. The way the check worked previously was: In order to be able to add an attachment to a entity X, the user must have create permissions to X. DynamicFKAuthorization would then call X.create passing the ID of the entity to which we want to add the attachment to. If the API call fails, it means the authorization failed. Otherwise, the attachment creation would proceed. The problem with this approach is that the API can fail for different reasons other than Authorization. For example, a custom API can fail because it was expecting other mandatory fields (besides the id given by DynamicFKAuthorization). By using Kernel->runAuthorize(), we avoid this problem, since now the actual action will not be executed. Since now we don't need to update the DB in order to check for Authorization, the transaction isn't necessary anymore. --- .../API/Subscriber/DynamicFKAuthorization.php | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/Civi/API/Subscriber/DynamicFKAuthorization.php b/Civi/API/Subscriber/DynamicFKAuthorization.php index ec6c8c01f5bb..e9e9fe1692c7 100644 --- a/Civi/API/Subscriber/DynamicFKAuthorization.php +++ b/Civi/API/Subscriber/DynamicFKAuthorization.php @@ -218,30 +218,8 @@ public function authorizeDelegate($action, $entityTable, $entityId, $apiRequest) return; } - /** - * @var \Exception $exception - */ - $exception = NULL; - $self = $this; - \CRM_Core_Transaction::create(TRUE)->run(function($tx) use ($entity, $action, $entityId, &$exception, $self) { - $tx->rollback(); // Just to be safe. - - $params = array( - 'version' => 3, - 'check_permissions' => 1, - 'id' => $entityId, - ); - - $result = $self->kernel->run($entity, $self->getDelegatedAction($action), $params); - if ($result['is_error'] || empty($result['values'])) { - $exception = new \Civi\API\Exception\UnauthorizedException("Authorization failed on ($entity,$entityId)", array( - 'cause' => $result, - )); - } - }); - - if ($exception) { - throw $exception; + if (!$this->isAuthorized($entity, $action, $entityId)) { + throw new \Civi\API\Exception\UnauthorizedException("Authorization failed on ($entity,$entityId)"); } } @@ -372,4 +350,21 @@ public function getCustomFields() { return $rows; } + /** + * @param string $entity + * @param string $action + * @param int $entityId + * + * @return bool + */ + private function isAuthorized($entity, $action, $entityId) { + $params = array( + 'version' => 3, + 'check_permissions' => 1, + 'id' => $entityId, + ); + + return $this->kernel->runAuthorize($entity, $this->getDelegatedAction($action), $params); + } + }