From 775aa764569fea1ee4bbacbd5bbf866b097f228f Mon Sep 17 00:00:00 2001 From: Kevin Pfeifer Date: Mon, 1 May 2023 11:47:20 +0200 Subject: [PATCH 1/3] add ability to add multiple optional parameters to can and canResult --- src/AuthorizationService.php | 19 ++-- src/AuthorizationServiceInterface.php | 11 ++- tests/TestCase/AuthorizationServiceTest.php | 90 ++++++++++++++++++- tests/TestCase/Policy/OrmResolverTest.php | 2 +- tests/bootstrap.php | 3 +- .../test_app/TestApp/Policy/ArticlePolicy.php | 11 +++ 6 files changed, 124 insertions(+), 12 deletions(-) diff --git a/src/AuthorizationService.php b/src/AuthorizationService.php index e009a76e..fdec3125 100644 --- a/src/AuthorizationService.php +++ b/src/AuthorizationService.php @@ -51,9 +51,9 @@ public function __construct(ResolverInterface $resolver) /** * @inheritDoc */ - public function can(?IdentityInterface $user, string $action, $resource): bool + public function can(?IdentityInterface $user, string $action, $resource, ...$optionalArgs): bool { - $result = $this->performCheck($user, $action, $resource); + $result = $this->performCheck($user, $action, $resource, ...$optionalArgs); return is_bool($result) ? $result : $result->getStatus(); } @@ -61,9 +61,9 @@ public function can(?IdentityInterface $user, string $action, $resource): bool /** * @inheritDoc */ - public function canResult(?IdentityInterface $user, string $action, $resource): ResultInterface + public function canResult(?IdentityInterface $user, string $action, $resource, ...$optionalArgs): ResultInterface { - $result = $this->performCheck($user, $action, $resource); + $result = $this->performCheck($user, $action, $resource, ...$optionalArgs); return is_bool($result) ? new Result($result) : $result; } @@ -74,10 +74,15 @@ public function canResult(?IdentityInterface $user, string $action, $resource): * @param \Authorization\IdentityInterface|null $user The user to check permissions for. * @param string $action The action/operation being performed. * @param mixed $resource The resource being operated on. + * @param mixed $optionalArgs Multiple additional arguments which are passed on * @return \Authorization\Policy\ResultInterface|bool */ - protected function performCheck(?IdentityInterface $user, string $action, mixed $resource): bool|ResultInterface - { + protected function performCheck( + ?IdentityInterface $user, + string $action, + mixed $resource, + mixed ...$optionalArgs + ): bool|ResultInterface { $this->authorizationChecked = true; $policy = $this->resolver->getPolicy($resource); @@ -90,7 +95,7 @@ protected function performCheck(?IdentityInterface $user, string $action, mixed } $handler = $this->getCanHandler($policy, $action); - $result = $handler($user, $resource); + $result = $handler($user, $resource, ...$optionalArgs); assert( is_bool($result) || $result instanceof ResultInterface, diff --git a/src/AuthorizationServiceInterface.php b/src/AuthorizationServiceInterface.php index b1bec6f9..4930b7cf 100644 --- a/src/AuthorizationServiceInterface.php +++ b/src/AuthorizationServiceInterface.php @@ -32,9 +32,10 @@ interface AuthorizationServiceInterface * @param \Authorization\IdentityInterface|null $user The user to check permissions for. * @param string $action The action/operation being performed. * @param mixed $resource The resource being operated on. + * @param mixed $optionalArgs Multiple additional arguments which are passed to the scope * @return bool */ - public function can(?IdentityInterface $user, string $action, mixed $resource): bool; + public function can(?IdentityInterface $user, string $action, mixed $resource, mixed ...$optionalArgs): bool; /** * Check whether the provided user can perform an action on a resource. @@ -45,9 +46,15 @@ public function can(?IdentityInterface $user, string $action, mixed $resource): * @param \Authorization\IdentityInterface|null $user The user to check permissions for. * @param string $action The action/operation being performed. * @param mixed $resource The resource being operated on. + * @param mixed $optionalArgs Multiple additional arguments which are passed to the scope * @return \Authorization\Policy\ResultInterface */ - public function canResult(?IdentityInterface $user, string $action, mixed $resource): ResultInterface; + public function canResult( + ?IdentityInterface $user, + string $action, + mixed $resource, + mixed ...$optionalArgs + ): ResultInterface; /** * Apply authorization scope conditions/restrictions. diff --git a/tests/TestCase/AuthorizationServiceTest.php b/tests/TestCase/AuthorizationServiceTest.php index a7a05d81..46d9a0e6 100644 --- a/tests/TestCase/AuthorizationServiceTest.php +++ b/tests/TestCase/AuthorizationServiceTest.php @@ -66,6 +66,50 @@ public function testCan() $this->assertTrue($result); } + public function testCanWithAdditionalParams() + { + $resolver = new MapResolver([ + Article::class => ArticlePolicy::class, + ]); + + $service = new AuthorizationService($resolver); + + $user = new IdentityDecorator($service, [ + 'role' => 'admin', + ]); + + $innerService = function () { + return true; + }; + + $result = $service->can($user, 'withService', new Article(), $innerService); + $this->assertTrue($result); + } + + public function testCanWithAdditionalNamedParams() + { + $resolver = new MapResolver([ + Article::class => ArticlePolicy::class, + ]); + + $service = new AuthorizationService($resolver); + + $user = new IdentityDecorator($service, [ + 'role' => 'admin', + ]); + + $innerService1 = function () { + return true; + }; + + $innerService2 = function () { + return false; + }; + + $result = $service->can(user: $user, action: 'witMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); + $this->assertFalse($result); + } + public function testCanWithResult() { $resolver = new MapResolver([ @@ -82,6 +126,50 @@ public function testCanWithResult() $this->assertInstanceOf(ResultInterface::class, $result); } + public function testCanWithResultAndAdditionalParams() + { + $resolver = new MapResolver([ + Article::class => ArticlePolicy::class, + ]); + + $service = new AuthorizationService($resolver); + + $user = new IdentityDecorator($service, [ + 'role' => 'admin', + ]); + + $innerService = function () { + return true; + }; + + $result = $service->canResult($user, 'withService', new Article(), $innerService); + $this->assertInstanceOf(ResultInterface::class, $result); + } + + public function testCanWithResultAndAdditionalNamedParams() + { + $resolver = new MapResolver([ + Article::class => ArticlePolicy::class, + ]); + + $service = new AuthorizationService($resolver); + + $user = new IdentityDecorator($service, [ + 'role' => 'admin', + ]); + + $innerService1 = function () { + return true; + }; + + $innerService2 = function () { + return false; + }; + + $result = $service->canResult(user: $user, action: 'witMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); + $this->assertInstanceOf(ResultInterface::class, $result); + } + public function testAuthorizationCheckedWithCan() { $resolver = new MapResolver([ @@ -173,7 +261,7 @@ public function testApplyScopeMethodMissing() ]); $article = new Article(); - $result = $service->applyScope($user, 'nope', $article); + $service->applyScope($user, 'nope', $article); } public function testApplyScopeAdditionalArguments() diff --git a/tests/TestCase/Policy/OrmResolverTest.php b/tests/TestCase/Policy/OrmResolverTest.php index 2c215435..f22db452 100644 --- a/tests/TestCase/Policy/OrmResolverTest.php +++ b/tests/TestCase/Policy/OrmResolverTest.php @@ -117,6 +117,6 @@ public function testGetPolicyUnknownTable() $articles = $this->createMock('Cake\Datasource\RepositoryInterface'); $resolver = new OrmResolver('TestApp'); - $policy = $resolver->getPolicy($articles); + $resolver->getPolicy($articles); } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 5ed5cab1..7b5758cf 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -13,11 +13,13 @@ * @link http://cakephp.org CakePHP(tm) Project * @license http://www.opensource.org/licenses/mit-license.php MIT License */ + use Authorization\AuthorizationPlugin; use Cake\Core\Configure; use Cake\Core\Plugin; use Cake\Datasource\ConnectionManager; use Cake\TestSuite\Fixture\SchemaLoader; +use function Cake\Core\env; $findRoot = function ($root) { do { @@ -33,7 +35,6 @@ unset($findRoot); chdir($root); -require_once 'vendor/cakephp/cakephp/src/basics.php'; require_once 'vendor/autoload.php'; define('ROOT', $root . DS . 'tests' . DS . 'test_app' . DS); diff --git a/tests/test_app/TestApp/Policy/ArticlePolicy.php b/tests/test_app/TestApp/Policy/ArticlePolicy.php index a6604814..e915c344 100644 --- a/tests/test_app/TestApp/Policy/ArticlePolicy.php +++ b/tests/test_app/TestApp/Policy/ArticlePolicy.php @@ -4,6 +4,7 @@ namespace TestApp\Policy; use Authorization\Policy\Result; +use Closure; use TestApp\Model\Entity\Article; class ArticlePolicy @@ -116,4 +117,14 @@ public function canPublish($user, Article $article) return new Result($article->get('user_id') === $user['id']); } + + public function canWithService($user, Article $article, Closure $service) + { + return $service(); + } + + public function canWitMultipleServices($user, Article $article, Closure $service1, Closure $service2) + { + return $service1() && $service2(); + } } From d95929e9175eae26eb7b17ded623e00147a0e864 Mon Sep 17 00:00:00 2001 From: Kevin Pfeifer Date: Mon, 1 May 2023 11:52:28 +0200 Subject: [PATCH 2/3] adjust docs --- docs/en/3-0-migration-guide.rst | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/en/3-0-migration-guide.rst b/docs/en/3-0-migration-guide.rst index a3682bc7..3da92c37 100644 --- a/docs/en/3-0-migration-guide.rst +++ b/docs/en/3-0-migration-guide.rst @@ -14,11 +14,14 @@ The following interfaces now have appropriate parameter and return types added: - ``RequestPolicyInterface.php`` - ``ResolverInterface`` -Multiple optional arguments for ``applyScope`` ----------------------------------------------- - -``IdentityInterface::applyScope`` as well as ``AuthorizationServiceInterface::applyScope`` -allow multiple optional arguments to be added. +Multiple optional arguments for ``applyScope``, ``can`` and ``canResult`` +------------------------------------------------------------------------- + +The following interface methods have been adjusted to pass on multiple optional arguments. +- ``IdentityInterface::applyScope`` +- ``AuthorizationServiceInterface::applyScope`` +- ``AuthorizationServiceInterface::can`` +- ``AuthorizationServiceInterface::canResult`` Removed methods --------------- From cab8c55ac981047e9b4cd5c176e609f861dad015 Mon Sep 17 00:00:00 2001 From: Kevin Pfeifer Date: Fri, 5 May 2023 10:21:42 +0200 Subject: [PATCH 3/3] fix typo Co-authored-by: Mark Story --- tests/TestCase/AuthorizationServiceTest.php | 4 ++-- tests/test_app/TestApp/Policy/ArticlePolicy.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/TestCase/AuthorizationServiceTest.php b/tests/TestCase/AuthorizationServiceTest.php index 46d9a0e6..a8c5d57f 100644 --- a/tests/TestCase/AuthorizationServiceTest.php +++ b/tests/TestCase/AuthorizationServiceTest.php @@ -106,7 +106,7 @@ public function testCanWithAdditionalNamedParams() return false; }; - $result = $service->can(user: $user, action: 'witMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); + $result = $service->can(user: $user, action: 'withMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); $this->assertFalse($result); } @@ -166,7 +166,7 @@ public function testCanWithResultAndAdditionalNamedParams() return false; }; - $result = $service->canResult(user: $user, action: 'witMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); + $result = $service->canResult(user: $user, action: 'withMultipleServices', resource: new Article(), service2: $innerService2, service1: $innerService1); $this->assertInstanceOf(ResultInterface::class, $result); } diff --git a/tests/test_app/TestApp/Policy/ArticlePolicy.php b/tests/test_app/TestApp/Policy/ArticlePolicy.php index e915c344..1814c3c6 100644 --- a/tests/test_app/TestApp/Policy/ArticlePolicy.php +++ b/tests/test_app/TestApp/Policy/ArticlePolicy.php @@ -123,7 +123,7 @@ public function canWithService($user, Article $article, Closure $service) return $service(); } - public function canWitMultipleServices($user, Article $article, Closure $service1, Closure $service2) + public function canWithMultipleServices($user, Article $article, Closure $service1, Closure $service2) { return $service1() && $service2(); }