From 2e10a9f005f50f98434e7ed92ba0e891bc29faee Mon Sep 17 00:00:00 2001 From: Marin Calin Date: Thu, 1 Jun 2023 19:39:30 +0300 Subject: [PATCH 1/4] Refactoring. --- src/Attributes/Authorize.php | 7 +++++-- src/Handlers/HasPermissionRequirementHandler.php | 7 ++++--- src/Handlers/HasRoleRequirementHandler.php | 7 ++++--- src/Middleware/AuthorizationMiddleware.php | 9 +++++---- src/Services/AuthorizationService.php | 16 +++++++++------- src/Services/PolicyService.php | 5 +++-- 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/Attributes/Authorize.php b/src/Attributes/Authorize.php index bfc0267..4e0125d 100644 --- a/src/Attributes/Authorize.php +++ b/src/Attributes/Authorize.php @@ -15,7 +15,10 @@ class Authorize * @param class-string|class-string[]|null $policies * @param array $parameters */ - public function __construct(public readonly string|array|null $policies = null, public readonly array $parameters = []) - { + public function __construct( + public readonly string|array|null $policies = null, + /** @deprecated You should no longer try to pass arguments directly */ + public readonly array $parameters = [] + ) { } } diff --git a/src/Handlers/HasPermissionRequirementHandler.php b/src/Handlers/HasPermissionRequirementHandler.php index 9d785cc..d293f0a 100644 --- a/src/Handlers/HasPermissionRequirementHandler.php +++ b/src/Handlers/HasPermissionRequirementHandler.php @@ -18,8 +18,9 @@ class HasPermissionRequirementHandler implements IRequirementHandler * * @param AuthManager $authManager */ - public function __construct(private readonly AuthManager $authManager) - { + public function __construct( + private readonly AuthManager $_authManager + ) { } /** @@ -31,7 +32,7 @@ public function __construct(private readonly AuthManager $authManager) public function handle(IRequirement $requirement): bool { /** @var HasPermissions $user */ - $user = $this->authManager->user(); + $user = $this->_authManager->user(); // If there is no current user, the requirement fails if (!$user) { diff --git a/src/Handlers/HasRoleRequirementHandler.php b/src/Handlers/HasRoleRequirementHandler.php index b8151e5..8569eed 100644 --- a/src/Handlers/HasRoleRequirementHandler.php +++ b/src/Handlers/HasRoleRequirementHandler.php @@ -18,8 +18,9 @@ class HasRoleRequirementHandler implements IRequirementHandler * * @param AuthManager $authManager */ - public function __construct(private readonly AuthManager $authManager) - { + public function __construct( + private readonly AuthManager $_authManager + ) { } /** @@ -31,7 +32,7 @@ public function __construct(private readonly AuthManager $authManager) public function handle(IRequirement $requirement): bool { /** @var HasPermissions $user */ - $user = $this->authManager->user(); + $user = $this->_authManager->user(); // If there is no current user, the requirement fails if (!$user) { diff --git a/src/Middleware/AuthorizationMiddleware.php b/src/Middleware/AuthorizationMiddleware.php index 690f312..a806a24 100644 --- a/src/Middleware/AuthorizationMiddleware.php +++ b/src/Middleware/AuthorizationMiddleware.php @@ -15,8 +15,9 @@ class AuthorizationMiddleware /** * AuthorizeMiddleware constructor method. */ - public function __construct(private readonly IAuthorizationService $traitService) - { + public function __construct( + private readonly IAuthorizationService $_authorizationService + ) { } /** @@ -42,14 +43,14 @@ public function handle(Request $request, Closure $next): mixed $method = '__invoke'; } - if (!$this->traitService->canAccessControllerMethod($controller, $method)) { + if (!$this->_authorizationService->canAccessControllerMethod($controller, $method)) { throw new AuthorizationException(); } } // If the request is made to an action inside a Closure if ($request->route()->getAction('uses') instanceof Closure) { - if (!$this->traitService->canAccessClosure($request->route()->getAction('uses'))) { + if (!$this->_authorizationService->canAccessClosure($request->route()->getAction('uses'))) { throw new AuthorizationException(); } } diff --git a/src/Services/AuthorizationService.php b/src/Services/AuthorizationService.php index 900a8a3..1991cfc 100644 --- a/src/Services/AuthorizationService.php +++ b/src/Services/AuthorizationService.php @@ -37,11 +37,13 @@ class AuthorizationService implements IAuthorizationService /** * AuthorizationService constructor method. * - * @param AuthManager $authManager - * @param IPolicyService $policyService + * @param AuthManager $_authManager + * @param IPolicyService $_authManager */ - public function __construct(private readonly AuthManager $authManager, private readonly IPolicyService $policyService) - { + public function __construct( + private readonly AuthManager $_authManager, + private readonly IPolicyService $_policyService + ) { } /** @@ -144,14 +146,14 @@ private function canAccessThroughAttributes(Collection|array $attributes): bool return true; } - // Loop through attributes and make sure all of them pass + // Only take into account authorization attributes $authorizationAttributes = $attributes->filter(fn (object $attribute) => $attribute instanceof Authorize); // If Authorize is present, make sure the user is authenticated if ($authorizationAttributes->isNotEmpty()) { // Get the current user /** @var Model|HasPermissions $user */ - $user = $this->authManager->user(); + $user = $this->_authManager->user(); // If the user is not authenticated, no other checks can be performed if (!$user) { @@ -182,7 +184,7 @@ private function checkAttributePasses(Authorize $attribute): bool // If there are policies configured, check if any of them passes. $policies = new Collection($attribute->policies); - return $policies->isEmpty() || $policies->some(fn (string $policyName) => $this->policyService->runPolicy($policyName, $attribute->parameters)); + return $policies->isEmpty() || $policies->some(fn (string $policyName) => $this->_policyService->runPolicy($policyName, $attribute->parameters)); } /** diff --git a/src/Services/PolicyService.php b/src/Services/PolicyService.php index e00376f..31e44b1 100644 --- a/src/Services/PolicyService.php +++ b/src/Services/PolicyService.php @@ -24,8 +24,9 @@ class PolicyService implements IPolicyService * * @param Container $_container */ - public function __construct(private readonly Container $_container) - { + public function __construct( + private readonly Container $_container + ) { } /** From e317fda14579465c458e1b6097e085f961c28329 Mon Sep 17 00:00:00 2001 From: Marin Calin Date: Thu, 1 Jun 2023 20:52:35 +0300 Subject: [PATCH 2/4] Removed parameters from authorization --- src/Attributes/Authorize.php | 8 +- src/Attributes/AuthorizePermission.php | 23 --- src/Attributes/AuthorizeRole.php | 22 --- src/Attributes/ProvidedBy.php | 21 --- src/Contracts/IPolicyProvider.php | 18 --- ...ice.php => IAuthorizationCheckService.php} | 2 +- src/Contracts/Services/IPolicyService.php | 3 +- src/Middleware/AuthorizationMiddleware.php | 4 +- src/Policies/RequirePermissionPolicy.php | 30 ---- src/Policies/RequireRolePolicy.php | 30 ---- .../AuthorizationServiceProvider.php | 6 +- ...vice.php => AuthorizationCheckService.php} | 57 ++++++-- src/Services/DefaultPolicyProvider.php | 134 ------------------ src/Services/PolicyService.php | 36 +---- .../PolicyThatRequiresUserProfile.php | 5 - .../PermissionAuthorizationController1.php | 4 +- .../PermissionAuthorizationController2.php | 4 +- .../PermissionAuthorizationController3.php | 6 +- .../PermissionAuthorizationController4.php | 8 +- .../PolicyAuthorizationTestController2.php | 2 +- .../RoleAuthorizationController1.php | 4 +- .../RoleAuthorizationController2.php | 4 +- .../RoleAuthorizationController3.php | 6 +- .../RoleAuthorizationController4.php | 8 +- tests/Feature/PolicyAuthorizationTest.php | 49 +------ 25 files changed, 88 insertions(+), 406 deletions(-) delete mode 100644 src/Attributes/AuthorizePermission.php delete mode 100644 src/Attributes/AuthorizeRole.php delete mode 100644 src/Attributes/ProvidedBy.php delete mode 100644 src/Contracts/IPolicyProvider.php rename src/Contracts/Services/{IAuthorizationService.php => IAuthorizationCheckService.php} (95%) delete mode 100644 src/Policies/RequirePermissionPolicy.php delete mode 100644 src/Policies/RequireRolePolicy.php rename src/Services/{AuthorizationService.php => AuthorizationCheckService.php} (78%) delete mode 100644 src/Services/DefaultPolicyProvider.php diff --git a/src/Attributes/Authorize.php b/src/Attributes/Authorize.php index 4e0125d..1fc066b 100644 --- a/src/Attributes/Authorize.php +++ b/src/Attributes/Authorize.php @@ -3,6 +3,7 @@ namespace Codestage\Authorization\Attributes; use Attribute; +use Codestage\Authorization\Contracts\IPermissionEnum; #[Attribute( Attribute::IS_REPEATABLE | Attribute::TARGET_CLASS | Attribute::TARGET_FUNCTION | Attribute::TARGET_METHOD @@ -13,12 +14,13 @@ class Authorize * This action requires that the user be authenticated and meet the authorization criteria. * * @param class-string|class-string[]|null $policies - * @param array $parameters + * @param string|string[]|null $roles + * @param IPermissionEnum|IPermissionEnum[]|null $permissions */ public function __construct( public readonly string|array|null $policies = null, - /** @deprecated You should no longer try to pass arguments directly */ - public readonly array $parameters = [] + public readonly string|array|null $roles = null, + public readonly IPermissionEnum|array|null $permissions = null ) { } } diff --git a/src/Attributes/AuthorizePermission.php b/src/Attributes/AuthorizePermission.php deleted file mode 100644 index 6a7a2fb..0000000 --- a/src/Attributes/AuthorizePermission.php +++ /dev/null @@ -1,23 +0,0 @@ - $permissions - ]); - } -} diff --git a/src/Attributes/AuthorizeRole.php b/src/Attributes/AuthorizeRole.php deleted file mode 100644 index 30167e0..0000000 --- a/src/Attributes/AuthorizeRole.php +++ /dev/null @@ -1,22 +0,0 @@ - $roles - ]); - } -} diff --git a/src/Attributes/ProvidedBy.php b/src/Attributes/ProvidedBy.php deleted file mode 100644 index 0c19cb7..0000000 --- a/src/Attributes/ProvidedBy.php +++ /dev/null @@ -1,21 +0,0 @@ - $provider - */ - public function __construct(public string $provider) - { - } -} diff --git a/src/Contracts/IPolicyProvider.php b/src/Contracts/IPolicyProvider.php deleted file mode 100644 index ae66695..0000000 --- a/src/Contracts/IPolicyProvider.php +++ /dev/null @@ -1,18 +0,0 @@ - $policy - * @param array $parameters - * @return TProvides - */ - public function make(string $policy, array $parameters): IPolicy; -} diff --git a/src/Contracts/Services/IAuthorizationService.php b/src/Contracts/Services/IAuthorizationCheckService.php similarity index 95% rename from src/Contracts/Services/IAuthorizationService.php rename to src/Contracts/Services/IAuthorizationCheckService.php index 44bf0c7..2278d7e 100644 --- a/src/Contracts/Services/IAuthorizationService.php +++ b/src/Contracts/Services/IAuthorizationCheckService.php @@ -9,7 +9,7 @@ /** * @internal */ -interface IAuthorizationService +interface IAuthorizationCheckService { /** * Check whether the given controller method can be accessed in the current request context. diff --git a/src/Contracts/Services/IPolicyService.php b/src/Contracts/Services/IPolicyService.php index 6a70fbe..54dd858 100644 --- a/src/Contracts/Services/IPolicyService.php +++ b/src/Contracts/Services/IPolicyService.php @@ -13,9 +13,8 @@ interface IPolicyService * Run the given policy. * * @param class-string $policy - * @param array $parameters * @throws BindingResolutionException * @return bool */ - public function runPolicy(string $policy, array $parameters = []): bool; + public function runPolicy(string $policy): bool; } diff --git a/src/Middleware/AuthorizationMiddleware.php b/src/Middleware/AuthorizationMiddleware.php index a806a24..b36953e 100644 --- a/src/Middleware/AuthorizationMiddleware.php +++ b/src/Middleware/AuthorizationMiddleware.php @@ -3,7 +3,7 @@ namespace Codestage\Authorization\Middleware; use Closure; -use Codestage\Authorization\Contracts\Services\IAuthorizationService; +use Codestage\Authorization\Contracts\Services\IAuthorizationCheckService; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Http\Request; @@ -16,7 +16,7 @@ class AuthorizationMiddleware * AuthorizeMiddleware constructor method. */ public function __construct( - private readonly IAuthorizationService $_authorizationService + private readonly IAuthorizationCheckService $_authorizationService ) { } diff --git a/src/Policies/RequirePermissionPolicy.php b/src/Policies/RequirePermissionPolicy.php deleted file mode 100644 index 38b270f..0000000 --- a/src/Policies/RequirePermissionPolicy.php +++ /dev/null @@ -1,30 +0,0 @@ - - */ - public function requirements(): array - { - return [ - new HasPermissionRequirement($this->permissions) - ]; - } -} diff --git a/src/Policies/RequireRolePolicy.php b/src/Policies/RequireRolePolicy.php deleted file mode 100644 index 4ac5e0f..0000000 --- a/src/Policies/RequireRolePolicy.php +++ /dev/null @@ -1,30 +0,0 @@ - - */ - public function requirements(): array - { - return [ - new HasRoleRequirement($this->roles) - ]; - } -} diff --git a/src/Providers/AuthorizationServiceProvider.php b/src/Providers/AuthorizationServiceProvider.php index ba281c7..3a9b706 100644 --- a/src/Providers/AuthorizationServiceProvider.php +++ b/src/Providers/AuthorizationServiceProvider.php @@ -6,8 +6,8 @@ use Codestage\Authorization\Console\Commands\MakePolicyCommand; use Codestage\Authorization\Console\Commands\MakeRequirementCommand; use Codestage\Authorization\Console\Commands\MakeRequirementHandlerCommand; -use Codestage\Authorization\Contracts\Services\{IAuthorizationService, IPolicyService}; -use Codestage\Authorization\Services\{AuthorizationService, PolicyService}; +use Codestage\Authorization\Contracts\Services\{IAuthorizationCheckService, IPolicyService}; +use Codestage\Authorization\Services\{AuthorizationCheckService, PolicyService}; use Illuminate\Foundation\Console\PolicyMakeCommand as BaseMakePolicyCommand; use Illuminate\Support\ServiceProvider; @@ -19,7 +19,7 @@ class AuthorizationServiceProvider extends ServiceProvider * @var array */ public array $bindings = [ - IAuthorizationService::class => AuthorizationService::class, + IAuthorizationCheckService::class => AuthorizationCheckService::class, IPolicyService::class => PolicyService::class, ]; diff --git a/src/Services/AuthorizationService.php b/src/Services/AuthorizationCheckService.php similarity index 78% rename from src/Services/AuthorizationService.php rename to src/Services/AuthorizationCheckService.php index 1991cfc..5847254 100644 --- a/src/Services/AuthorizationService.php +++ b/src/Services/AuthorizationCheckService.php @@ -4,7 +4,9 @@ use Closure; use Codestage\Authorization\Attributes\{AllowAnonymous, Authorize}; -use Codestage\Authorization\Contracts\{Services\IAuthorizationService, Services\IPolicyService}; +use Codestage\Authorization\Contracts\{IPermissionEnum, IPolicy, Services\IAuthorizationCheckService, Services\IPolicyService}; +use Codestage\Authorization\Requirements\HasPermissionRequirement; +use Codestage\Authorization\Requirements\HasRoleRequirement; use Codestage\Authorization\Traits\HasPermissions; use Illuminate\Auth\AuthenticationException; use Illuminate\Contracts\Auth\Guard as AuthManager; @@ -24,7 +26,7 @@ /** * @internal */ -class AuthorizationService implements IAuthorizationService +class AuthorizationCheckService implements IAuthorizationCheckService { /** * A list of attributes that can be used for authorization. @@ -35,10 +37,10 @@ class AuthorizationService implements IAuthorizationService ]; /** - * AuthorizationService constructor method. + * AuthorizationCheckService constructor method. * * @param AuthManager $_authManager - * @param IPolicyService $_authManager + * @param IPolicyService $_policyService */ public function __construct( private readonly AuthManager $_authManager, @@ -128,8 +130,8 @@ private function computeAttributesForClosure(Closure $closure): Collection * Check whether an action can be accessed when guarded by the given attributes. * * @param Collection|ReflectionAttribute[] $attributes - * @throws BindingResolutionException * @return bool + * @throws AuthenticationException */ private function canAccessThroughAttributes(Collection|array $attributes): bool { @@ -176,15 +178,48 @@ private function canAccessThroughAttributes(Collection|array $attributes): bool */ private function checkAttributePasses(Authorize $attribute): bool { - // If no policies are provided, no checks can fail - if (!$attribute->policies || (is_array($attribute->policies) && count($attribute->policies) === 0)) { - return true; - } - // If there are policies configured, check if any of them passes. $policies = new Collection($attribute->policies); - return $policies->isEmpty() || $policies->some(fn (string $policyName) => $this->_policyService->runPolicy($policyName, $attribute->parameters)); + // Add role policies + if (!!$attribute->roles) { + $roles = new Collection($attribute->roles); + foreach ($roles as $role) { + $policies->push(new class ($role) implements IPolicy { + /** Constructor method. */ + public function __construct(public readonly string $role) + { + } + + /** @inheritDoc */ + public function requirements(): array + { + return [new HasRoleRequirement($this->role)]; + } + }); + } + } + + // Add permission policies + if (!!$attribute->permissions) { + $permissions = new Collection($attribute->permissions); + foreach ($permissions as $permission) { + $policies->push(new class ($permission) implements IPolicy { + /** Constructor method. */ + public function __construct(public readonly IPermissionEnum $permission) + { + } + + /** @inheritDoc */ + public function requirements(): array + { + return [new HasPermissionRequirement($this->permission)]; + } + }); + } + } + + return $policies->isEmpty() || $policies->some(fn (string|IPolicy $policyName) => $this->_policyService->runPolicy($policyName)); } /** diff --git a/src/Services/DefaultPolicyProvider.php b/src/Services/DefaultPolicyProvider.php deleted file mode 100644 index d64c022..0000000 --- a/src/Services/DefaultPolicyProvider.php +++ /dev/null @@ -1,134 +0,0 @@ - $policy - * @param array $parameters - * @throws BindingResolutionException - * @throws AuthorizationException - * @return TProvides - */ - public function make(string $policy, array $parameters): IPolicy - { - // Get the parameters that need to be substituted in routing - $routeParameters = $this->_neededRouteParameters($parameters); - - if (!empty($routeParameters)) { - // Remove route parameters from the parameters array - array_filter($parameters, function (mixed $value, mixed $key) { - return !is_numeric($key) || !is_string($value); - }, ARRAY_FILTER_USE_BOTH); - - // Make sure the current route is accessible - $route = $this->_router->getCurrentRoute(); - - if (!$route) { - throw new AuthorizationException('Routing context could not be determined when providing policy ' . $policy); - } - - // Make sure that all requested parameters can actually be found in the route before trying to resolve them - foreach ($routeParameters as $parameter) { - if (!$route->hasParameter($parameter)) { - throw new BindingResolutionException('Requested route parameter ' . $parameter . ' for policy ' . $policy . ', but it does not exist.'); - } - } - - // Resolve route implicit bindings - try { - ImplicitRouteBinding::resolveForRoute($this->_container, $route); - } catch (ModelNotFoundException) { - throw new AuthorizationException('Requested route parameters for policy ' . $policy . ' could not be resolved.'); - } catch (BackedEnumCaseNotFoundException) { - throw new BindingResolutionException('Requested route parameters for policy ' . $policy . ' could not be resolved.'); - } - - // Add the substituted bindings to the parameters - foreach ($routeParameters as $parameter) { - $parameters[$parameter] = $route->parameter($parameter); - } - } - - // Let the container provide the rest of the parameters - return $this->_container->make($policy, $parameters); - } - - /** - * Get the type hint given in the policy constructor for the requested parameter. - * - * @param class-string $policy - * @param string $parameter - * @throws ReflectionException - * @return class-string - */ - public function _getParameterTypeHint(string $policy, string $parameter): string - { - // Reflect on the policy class - $reflection = new ReflectionClass($policy); - - // Make sure the constructor data is accessible - $constructor = $reflection->getConstructor(); - if (!$constructor) { - throw new ReflectionException('Could not find constructor for policy ' . $policy); - } - - // Find the requested parameter and return its type - $constructorParameters = $constructor->getParameters(); - foreach ($constructorParameters as $constructorParameter) { - if (!strcmp($constructorParameter->getName(), $parameter)) { - return $constructorParameter->getType()->getName(); - } - } - - // Throw exception if parameter could not be found - throw new ReflectionException('Could not find parameter ' . $parameter . ' in constructor for policy ' . $policy); - } - - /** - * Get the parameters that need to be filled in from the router. - * - * @param array $parameters - * @return array - */ - private function _neededRouteParameters(array $parameters): array - { - $needed = []; - foreach ($parameters as $key => $value) { - if (is_numeric($key) && is_string($value)) { - $needed[] = $value; - } - } - return $needed; - } -} diff --git a/src/Services/PolicyService.php b/src/Services/PolicyService.php index 31e44b1..a8d989d 100644 --- a/src/Services/PolicyService.php +++ b/src/Services/PolicyService.php @@ -3,12 +3,10 @@ namespace Codestage\Authorization\Services; use Codestage\Authorization\Attributes\HandledBy; -use Codestage\Authorization\Attributes\ProvidedBy; -use Codestage\Authorization\Contracts\{IPolicyProvider, IRequirement, IRequirementHandler}; +use Codestage\Authorization\Contracts\{IPolicy, IRequirement, IRequirementHandler}; use Codestage\Authorization\Contracts\Services\IPolicyService; use Illuminate\Contracts\Container\{BindingResolutionException, Container}; use Illuminate\Support\Collection; -use ReflectionAttribute; use ReflectionClass; use ReflectionException; use function is_array; @@ -33,18 +31,19 @@ public function __construct( * Run the given policy. * * @param class-string $policy - * @param array $parameters * @throws BindingResolutionException * @throws ReflectionException * @return bool */ - public function runPolicy(string $policy, array $parameters = []): bool + public function runPolicy(string|IPolicy $policy): bool { // Resolve the policy instance from the Container - $policyInstance = $this->getProviderForPolicy($policy)->make($policy, $parameters); + if (is_string($policy)) { + $policy = $this->_container->make($policy); + } // Check that all the requirements defined by this policy pass - $requirements = $policyInstance->requirements(); + $requirements = $policy->requirements(); foreach ($requirements as $requirement) { if (!$this->_checkRequirement($requirement)) { @@ -114,27 +113,4 @@ private function getRequirementHandlers(IRequirement $requirement): Collection // Return a list of unique handlers return $handlers->unique(); } - - /** - * Get the provider for the given policy class. - * - * @param class-string $policy - * @throws BindingResolutionException - * @throws ReflectionException - * @return IPolicyProvider - */ - private function getProviderForPolicy(string $policy): IPolicyProvider { - $reflection = new ReflectionClass($policy); - $configuredProviders = $reflection->getAttributes(ProvidedBy::class); - if (count($configuredProviders)) { - if ($configuredProviders[0] instanceof ReflectionAttribute) { - /** @var ProvidedBy $configuration */ - $configuration = $configuredProviders[0]->newInstance(); - - return $this->_container->make($configuration->provider); - } - } - - return $this->_container->make(DefaultPolicyProvider::class); - } } diff --git a/tests/Fakes/Authorization/Policies/PolicyThatRequiresUserProfile.php b/tests/Fakes/Authorization/Policies/PolicyThatRequiresUserProfile.php index 0e21f7c..e7c7661 100644 --- a/tests/Fakes/Authorization/Policies/PolicyThatRequiresUserProfile.php +++ b/tests/Fakes/Authorization/Policies/PolicyThatRequiresUserProfile.php @@ -9,10 +9,6 @@ class PolicyThatRequiresUserProfile implements IPolicy { - public function __construct(public readonly UserProfile $profile) - { - } - /** * The list of requirements that need to be fulfilled in order to complete this policy. * @@ -21,7 +17,6 @@ public function __construct(public readonly UserProfile $profile) public function requirements(): array { return [ - new NotNullRequirement($this->profile) ]; } } diff --git a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController1.php b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController1.php index 79dbfe7..9b9dba7 100644 --- a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController1.php +++ b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController1.php @@ -2,11 +2,11 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\PermissionsAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizePermission; +use Codestage\Authorization\Attributes\Authorize; use Codestage\Authorization\Tests\Fakes\Enums\FakePermission; use Illuminate\Support\Facades\Response; -#[AuthorizePermission(FakePermission::ExamplePermission1)] +#[Authorize(permissions: FakePermission::ExamplePermission1)] class PermissionAuthorizationController1 { /** diff --git a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController2.php b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController2.php index 0925842..d4c4fc0 100644 --- a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController2.php +++ b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController2.php @@ -2,11 +2,11 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\PermissionsAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizePermission; +use Codestage\Authorization\Attributes\Authorize; use Codestage\Authorization\Tests\Fakes\Enums\FakePermission; use Illuminate\Support\Facades\Response; -#[AuthorizePermission([FakePermission::ExamplePermission1, FakePermission::ExamplePermission3])] +#[Authorize(permissions: [FakePermission::ExamplePermission1, FakePermission::ExamplePermission3])] class PermissionAuthorizationController2 { /** diff --git a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController3.php b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController3.php index e991da8..228d0ac 100644 --- a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController3.php +++ b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController3.php @@ -2,12 +2,12 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\PermissionsAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizePermission; +use Codestage\Authorization\Attributes\Authorize; use Codestage\Authorization\Tests\Fakes\Enums\FakePermission; use Illuminate\Support\Facades\Response; -#[AuthorizePermission(FakePermission::ExamplePermission1)] -#[AuthorizePermission(FakePermission::ExamplePermission3)] +#[Authorize(permissions: FakePermission::ExamplePermission1)] +#[Authorize(permissions: FakePermission::ExamplePermission3)] class PermissionAuthorizationController3 { /** diff --git a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController4.php b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController4.php index 9b34c9f..6235131 100644 --- a/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController4.php +++ b/tests/Fakes/Http/Controllers/PermissionsAuthorizationTest/PermissionAuthorizationController4.php @@ -2,12 +2,12 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\PermissionsAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizePermission; +use Codestage\Authorization\Attributes\Authorize;; use Codestage\Authorization\Tests\Fakes\Enums\FakePermission; use Illuminate\Support\Facades\Response; -#[AuthorizePermission(FakePermission::ExamplePermission1)] -#[AuthorizePermission(FakePermission::ExamplePermission3)] +#[Authorize(permissions: FakePermission::ExamplePermission1)] +#[Authorize(permissions: FakePermission::ExamplePermission3)] class PermissionAuthorizationController4 { /** @@ -21,7 +21,7 @@ public function onlyRequiresClassPermissions(): \Illuminate\Http\Response /** * @return \Illuminate\Http\Response */ - #[AuthorizePermission(FakePermission::ExamplePermission2)] + #[Authorize(permissions: FakePermission::ExamplePermission2)] public function requiresClassAndMethodPermissions(): \Illuminate\Http\Response { return Response::noContent(); diff --git a/tests/Fakes/Http/Controllers/PolicyAuthorizationTest/PolicyAuthorizationTestController2.php b/tests/Fakes/Http/Controllers/PolicyAuthorizationTest/PolicyAuthorizationTestController2.php index abeb7ee..2e2a573 100644 --- a/tests/Fakes/Http/Controllers/PolicyAuthorizationTest/PolicyAuthorizationTestController2.php +++ b/tests/Fakes/Http/Controllers/PolicyAuthorizationTest/PolicyAuthorizationTestController2.php @@ -9,7 +9,7 @@ class PolicyAuthorizationTestController2 { - #[Authorize(PolicyThatRequiresUserProfile::class, ['profile'])] + #[Authorize(PolicyThatRequiresUserProfile::class)] public function __invoke(UserProfile $profile) { return Response::noContent(); diff --git a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController1.php b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController1.php index fc13fa0..c56e416 100644 --- a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController1.php +++ b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController1.php @@ -2,10 +2,10 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\RoleAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizeRole; +use Codestage\Authorization\Attributes\Authorize; use Illuminate\Support\Facades\Response; -#[AuthorizeRole('test-role-1')] +#[Authorize(roles: 'test-role-1')] class RoleAuthorizationController1 { /** diff --git a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController2.php b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController2.php index 8886aab..88d625b 100644 --- a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController2.php +++ b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController2.php @@ -2,10 +2,10 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\RoleAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizeRole; +use Codestage\Authorization\Attributes\Authorize; use Illuminate\Support\Facades\Response; -#[AuthorizeRole(['test-role-1', 'test-role-3'])] +#[Authorize(roles: ['test-role-1', 'test-role-3'])] class RoleAuthorizationController2 { /** diff --git a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController3.php b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController3.php index 1f63edb..6d76c58 100644 --- a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController3.php +++ b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController3.php @@ -2,11 +2,11 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\RoleAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizeRole; +use Codestage\Authorization\Attributes\Authorize; use Illuminate\Support\Facades\Response; -#[AuthorizeRole('test-role-1')] -#[AuthorizeRole('test-role-3')] +#[Authorize(roles: 'test-role-1')] +#[Authorize(roles: 'test-role-3')] class RoleAuthorizationController3 { /** diff --git a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController4.php b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController4.php index c7e5685..54b7dbc 100644 --- a/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController4.php +++ b/tests/Fakes/Http/Controllers/RoleAuthorizationTest/RoleAuthorizationController4.php @@ -2,11 +2,11 @@ namespace Codestage\Authorization\Tests\Fakes\Http\Controllers\RoleAuthorizationTest; -use Codestage\Authorization\Attributes\AuthorizeRole; +use Codestage\Authorization\Attributes\Authorize; use Illuminate\Support\Facades\Response; -#[AuthorizeRole('test-role-1')] -#[AuthorizeRole('test-role-3')] +#[Authorize(roles: 'test-role-1')] +#[Authorize(roles: 'test-role-3')] class RoleAuthorizationController4 { /** @@ -20,7 +20,7 @@ public function onlyRequiresClassRoles(): \Illuminate\Http\Response /** * @return \Illuminate\Http\Response */ - #[AuthorizeRole('test-role-2')] + #[Authorize(roles: 'test-role-2')] public function requiresClassAndMethodRoles(): \Illuminate\Http\Response { return Response::noContent(); diff --git a/tests/Feature/PolicyAuthorizationTest.php b/tests/Feature/PolicyAuthorizationTest.php index 9362521..8012646 100644 --- a/tests/Feature/PolicyAuthorizationTest.php +++ b/tests/Feature/PolicyAuthorizationTest.php @@ -14,7 +14,7 @@ use Illuminate\Testing\TestResponse; class PolicyAuthorizationTest extends TestCase -{ +{ /** * @test * @return void @@ -75,51 +75,4 @@ public function Authorize_WhenClassRequiresPolicyWhichPasses_Success(): void // Assert $responses[0]->assertSuccessful(); } - - /** - * @test - * @return void - */ - public function PolicyWithRouteDependency_WhenDependencyDoesNotExist_Forbidden(): void - { - // Arrange - $this->authenticateUser(); - Route::get('test1/{profile}', PolicyAuthorizationTestController2::class)->middleware([AuthorizationMiddleware::class])->name('test1'); - - // Act - /** @var TestResponse[] $responses */ - $responses = [ - $this->getJson(URL::route('test1', [ - 'profile' => $this->faker->slug() - ])), - ]; - - // Assert - $responses[0]->assertForbidden(); - } - - /** - * @test - * @return void - */ - public function PolicyWithRouteDependency_WhenDependencyFound_Success(): void - { - // Arrange - $this->authenticateUser(); - Route::get('test1/{profile}', PolicyAuthorizationTestController2::class)->middleware([AuthorizationMiddleware::class])->name('test1'); - $profile = UserProfile::query()->create([ - 'user_id' => User::query()->create()->getKey() - ]); - - // Act - /** @var TestResponse[] $responses */ - $responses = [ - $this->getJson(URL::route('test1', [ - 'profile' => $profile->getRouteKey() - ])), - ]; - - // Assert - $responses[0]->assertSuccessful(); - } } From bc889b0168beed52b21c3c748307db884dd00dfa Mon Sep 17 00:00:00 2001 From: Marin Calin Date: Thu, 1 Jun 2023 21:58:54 +0300 Subject: [PATCH 3/4] Implemented resource based authorization. --- src/Contracts/IAuthorizable.php | 7 ++ src/Contracts/IResourceRequirementHandler.php | 19 +++ .../Services/IAuthorizationService.php | 28 +++++ src/Contracts/Services/IPolicyService.php | 20 --- .../HasPermissionRequirementHandler.php | 5 +- src/Handlers/HasRoleRequirementHandler.php | 2 +- .../AuthorizationServiceProvider.php | 6 +- src/Services/AuthorizationCheckService.php | 34 +++-- src/Services/AuthorizationService.php | 112 +++++++++++++++++ src/Services/PolicyService.php | 116 ------------------ src/Traits/HasPermissions.php | 2 +- .../Handlers/SameAuthorRequirementHandler.php | 32 +++++ .../Policies/ActOnDocumentPolicy.php | 19 +++ .../Requirement/SameAuthorRequirement.php | 13 ++ .../DocumentController1.php | 52 ++++++++ tests/Fakes/Models/Document.php | 12 ++ tests/Feature/PolicyAuthorizationTest.php | 2 +- tests/Feature/ResourceAuthorizationTest.php | 93 ++++++++++++++ tests/TestCase.php | 5 + 19 files changed, 424 insertions(+), 155 deletions(-) create mode 100644 src/Contracts/IAuthorizable.php create mode 100644 src/Contracts/IResourceRequirementHandler.php create mode 100644 src/Contracts/Services/IAuthorizationService.php delete mode 100644 src/Contracts/Services/IPolicyService.php create mode 100644 src/Services/AuthorizationService.php delete mode 100644 src/Services/PolicyService.php create mode 100644 tests/Fakes/Authorization/Handlers/SameAuthorRequirementHandler.php create mode 100644 tests/Fakes/Authorization/Policies/ActOnDocumentPolicy.php create mode 100644 tests/Fakes/Authorization/Requirement/SameAuthorRequirement.php create mode 100644 tests/Fakes/Http/Controllers/ResourceAuthorizationTest/DocumentController1.php create mode 100644 tests/Fakes/Models/Document.php create mode 100644 tests/Feature/ResourceAuthorizationTest.php diff --git a/src/Contracts/IAuthorizable.php b/src/Contracts/IAuthorizable.php new file mode 100644 index 0000000..20d469f --- /dev/null +++ b/src/Contracts/IAuthorizable.php @@ -0,0 +1,7 @@ + $requirements + * @return bool + */ + public function authorizeRequirements(mixed $resource, iterable $requirements): bool; +} \ No newline at end of file diff --git a/src/Contracts/Services/IPolicyService.php b/src/Contracts/Services/IPolicyService.php deleted file mode 100644 index 54dd858..0000000 --- a/src/Contracts/Services/IPolicyService.php +++ /dev/null @@ -1,20 +0,0 @@ -permissions))->some(fn (IPermissionEnum $permission) => $user->hasPermission($permission)); + return (new Collection($requirement->permissions)) + ->some(fn (IPermissionEnum $permission) => $user->hasPermission($permission)); } } diff --git a/src/Handlers/HasRoleRequirementHandler.php b/src/Handlers/HasRoleRequirementHandler.php index 8569eed..cf1fe0d 100644 --- a/src/Handlers/HasRoleRequirementHandler.php +++ b/src/Handlers/HasRoleRequirementHandler.php @@ -16,7 +16,7 @@ class HasRoleRequirementHandler implements IRequirementHandler /** * HasRoleRequirementHandler constructor method. * - * @param AuthManager $authManager + * @param AuthManager $_authManager */ public function __construct( private readonly AuthManager $_authManager diff --git a/src/Providers/AuthorizationServiceProvider.php b/src/Providers/AuthorizationServiceProvider.php index 3a9b706..a7a4cca 100644 --- a/src/Providers/AuthorizationServiceProvider.php +++ b/src/Providers/AuthorizationServiceProvider.php @@ -6,8 +6,8 @@ use Codestage\Authorization\Console\Commands\MakePolicyCommand; use Codestage\Authorization\Console\Commands\MakeRequirementCommand; use Codestage\Authorization\Console\Commands\MakeRequirementHandlerCommand; -use Codestage\Authorization\Contracts\Services\{IAuthorizationCheckService, IPolicyService}; -use Codestage\Authorization\Services\{AuthorizationCheckService, PolicyService}; +use Codestage\Authorization\Contracts\Services\{IAuthorizationCheckService, IAuthorizationService}; +use Codestage\Authorization\Services\{AuthorizationCheckService, AuthorizationService}; use Illuminate\Foundation\Console\PolicyMakeCommand as BaseMakePolicyCommand; use Illuminate\Support\ServiceProvider; @@ -20,7 +20,7 @@ class AuthorizationServiceProvider extends ServiceProvider */ public array $bindings = [ IAuthorizationCheckService::class => AuthorizationCheckService::class, - IPolicyService::class => PolicyService::class, + IAuthorizationService::class => AuthorizationService::class, ]; /** diff --git a/src/Services/AuthorizationCheckService.php b/src/Services/AuthorizationCheckService.php index 5847254..dc7edd1 100644 --- a/src/Services/AuthorizationCheckService.php +++ b/src/Services/AuthorizationCheckService.php @@ -4,20 +4,22 @@ use Closure; use Codestage\Authorization\Attributes\{AllowAnonymous, Authorize}; -use Codestage\Authorization\Contracts\{IPermissionEnum, IPolicy, Services\IAuthorizationCheckService, Services\IPolicyService}; +use Illuminate\Contracts\Container\Container; +use Codestage\Authorization\Contracts\{IPermissionEnum, + IPolicy, + Services\IAuthorizationCheckService, + Services\IAuthorizationService}; use Codestage\Authorization\Requirements\HasPermissionRequirement; use Codestage\Authorization\Requirements\HasRoleRequirement; use Codestage\Authorization\Traits\HasPermissions; use Illuminate\Auth\AuthenticationException; use Illuminate\Contracts\Auth\Guard as AuthManager; -use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Collection; use ReflectionAttribute; use ReflectionClass; use ReflectionException; use ReflectionFunction; -use function count; use function get_class; use function in_array; use function is_array; @@ -40,11 +42,13 @@ class AuthorizationCheckService implements IAuthorizationCheckService * AuthorizationCheckService constructor method. * * @param AuthManager $_authManager - * @param IPolicyService $_policyService + * @param Container $_container + * @param IAuthorizationService $_authorizationService */ public function __construct( private readonly AuthManager $_authManager, - private readonly IPolicyService $_policyService + private readonly Container $_container, + private readonly IAuthorizationService $_authorizationService ) { } @@ -173,7 +177,6 @@ private function canAccessThroughAttributes(Collection|array $attributes): bool * Check if requirements defined by the given attribute pass in the current context. * * @param Authorize $attribute - * @throws BindingResolutionException * @return bool */ private function checkAttributePasses(Authorize $attribute): bool @@ -219,7 +222,16 @@ public function requirements(): array } } - return $policies->isEmpty() || $policies->some(fn (string|IPolicy $policyName) => $this->_policyService->runPolicy($policyName)); + // If no policies are present, the attribute cannot fail + if ($policies->isEmpty()) { + return true; + } + + // Instantiate all policies and run them + return $policies->map(fn (string|IPolicy $policy) => is_string($policy) ? $this->_container->make($policy) : $policy) + ->some(function (IPolicy $policy) { + return $this->_authorizationService->authorizePolicy(null, $policy); + }); } /** @@ -227,9 +239,9 @@ public function requirements(): array * * @param class-string $className * @param class-string $methodName - * @throws BindingResolutionException - * @throws ReflectionException * @return bool + * @throws AuthenticationException + * @throws ReflectionException */ public function canAccessControllerMethod(string $className, string $methodName): bool { @@ -242,9 +254,9 @@ public function canAccessControllerMethod(string $className, string $methodName) * Check whether the given controller method can be accessed in the current request context. * * @param Closure $closure - * @throws BindingResolutionException - * @throws ReflectionException * @return bool + * @throws AuthenticationException + * @throws ReflectionException */ public function canAccessClosure(Closure $closure): bool { diff --git a/src/Services/AuthorizationService.php b/src/Services/AuthorizationService.php new file mode 100644 index 0000000..0336089 --- /dev/null +++ b/src/Services/AuthorizationService.php @@ -0,0 +1,112 @@ +_container->make($policy); + } + + // Check that all the requirements defined by this policy pass + $requirements = $policy->requirements(); + return $this->authorizeRequirements($resource, $requirements); + } + + /** + * @inheritDoc + */ + public function authorizeRequirements(mixed $resource, iterable $requirements): bool + { + foreach ($requirements as $requirement) { + // Get all handlers that could handle this requirement + $handlers = new Collection($this->_getRequirementHandlers($requirement)); + + // If all of this requirement's handlers return false, then this requirement fails + $fails = !$handlers->some(function (string $handlerClassName) use ($resource, $requirement) { + $handler = $this->_container->make($handlerClassName); + + if ($handler instanceof IRequirementHandler) { + return $handler->handle($requirement); + } else if ($handler instanceof IResourceRequirementHandler) { + if ($resource === null) { + throw new \Exception("Attempt to use resource handler for authorizing without a resource."); + } + + return $handler->handle($requirement, $resource); + } else { + throw new BindingResolutionException( $handlerClassName . " was not resolved to a valid requirement handler."); + } + }); + + // If this requirement failed, the whole check fails + if ($fails) { + return false; + } + } + + // If all requirements pass, return true + return true; + } + + /** + * Get the handlers that can handle the given requirement. + * + * @param IRequirement $requirement + * @return Collection> + */ + private function _getRequirementHandlers(IRequirement $requirement): Collection + { + // Reflect on the requirement class + $reflection = new ReflectionClass($requirement); + + // Accumulate the handlers that have been found + $handlers = new Collection(); + + // Get the attributes that describe authorization handlers + $handlerAttributes = $reflection->getAttributes(HandledBy::class); + + // Extract handlers from each attribute + foreach ($handlerAttributes as $attribute) { + /** @var HandledBy $instance */ + $instance = $attribute->newInstance(); + + if (is_array($instance->handler)) { + $handlers->push(...$instance->handler); + } else if (is_string($instance->handler)) { + $handlers->push($instance->handler); + } + } + + // Return a list of unique handlers + return $handlers->unique(); + } +} \ No newline at end of file diff --git a/src/Services/PolicyService.php b/src/Services/PolicyService.php deleted file mode 100644 index a8d989d..0000000 --- a/src/Services/PolicyService.php +++ /dev/null @@ -1,116 +0,0 @@ -_container->make($policy); - } - - // Check that all the requirements defined by this policy pass - $requirements = $policy->requirements(); - - foreach ($requirements as $requirement) { - if (!$this->_checkRequirement($requirement)) { - return false; - } - } - - // If no requirement fails, return true - return true; - } - - /** - * Check whether the given requirement passes in the current context. - * - * @param IRequirement $requirement - * @throws BindingResolutionException - * @return bool - */ - private function _checkRequirement(IRequirement $requirement): bool - { - // Get the handlers for this requirement - $handlers = $this->getRequirementHandlers($requirement); - - // Return true if any of the handlers reports success - foreach ($handlers as $handlerName) { - /** @var IRequirementHandler $handler */ - $handler = $this->_container->make($handlerName); - - if ($handler->handle($requirement)) { - return true; - } - } - - // If no handler returned successfully, this requirement fails - return false; - } - - /** - * Get the handlers that can handle the given requirement. - * - * @param IRequirement $requirement - * @return Collection> - */ - private function getRequirementHandlers(IRequirement $requirement): Collection - { - // Reflect on the requirement class - $reflection = new ReflectionClass($requirement); - - // Accumulate the handlers that have been found - $handlers = new Collection(); - - // Get the attributes that describe authorization handlers - $handlerAttributes = $reflection->getAttributes(HandledBy::class); - - // Extract handlers from each attribute - foreach ($handlerAttributes as $attribute) { - /** @var HandledBy $instance */ - $instance = $attribute->newInstance(); - - if (is_array($instance->handler)) { - $handlers->push(...$instance->handler); - } else if (is_string($instance->handler)) { - $handlers->push($instance->handler); - } - } - - // Return a list of unique handlers - return $handlers->unique(); - } -} diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 6b86fa8..c0f7853 100644 --- a/src/Traits/HasPermissions.php +++ b/src/Traits/HasPermissions.php @@ -8,7 +8,7 @@ use Illuminate\Support\Collection; /** - * @template TPermission of \Codestage\Authorization\Contracts\IPermissionEnum + * @template TPermission of IPermissionEnum * @property-read Collection $roles * @method HasManyThrough hasManyThrough(string $related, string $through, string|null $firstKey = null, string|null $secondKey = null, string|null $localKey = null, string|null $secondLocalKey = null) */ diff --git a/tests/Fakes/Authorization/Handlers/SameAuthorRequirementHandler.php b/tests/Fakes/Authorization/Handlers/SameAuthorRequirementHandler.php new file mode 100644 index 0000000..b65afea --- /dev/null +++ b/tests/Fakes/Authorization/Handlers/SameAuthorRequirementHandler.php @@ -0,0 +1,32 @@ + + */ +class SameAuthorRequirementHandler implements IResourceRequirementHandler +{ + /** + * SameAuthorRequirementHandler constructor method. + * + * @param Guard $_authManager + */ + public function __construct( + private readonly Guard $_authManager + ) { + } + + /** + * @inheritDoc + */ + public function handle(mixed $requirement, mixed $resource): bool + { + return !strcmp($this->_authManager->id(), $resource->user_id); + } +} \ No newline at end of file diff --git a/tests/Fakes/Authorization/Policies/ActOnDocumentPolicy.php b/tests/Fakes/Authorization/Policies/ActOnDocumentPolicy.php new file mode 100644 index 0000000..15a672d --- /dev/null +++ b/tests/Fakes/Authorization/Policies/ActOnDocumentPolicy.php @@ -0,0 +1,19 @@ +findOrFail($document); + if ($this->_authorizationService->authorizePolicy($document, ActOnDocumentPolicy::class)) { + return new Response(); + } else if ($this->_authManager->check()) { + throw new AuthorizationException(); + } else { + throw new AuthenticationException(); + } + } +} \ No newline at end of file diff --git a/tests/Fakes/Models/Document.php b/tests/Fakes/Models/Document.php new file mode 100644 index 0000000..cb78df3 --- /dev/null +++ b/tests/Fakes/Models/Document.php @@ -0,0 +1,12 @@ +middleware([AuthorizationMiddleware::class]) + ->name('test1'); + $user = User::query()->create(); + $document = Document::query()->create([ + 'user_id' => $user->getKey() + ]); + + // Act + /** @var TestResponse[] $responses */ + $responses = [ + $this->getJson(URL::route('test1', $document)), + ]; + + // Assert + $responses[0]->assertUnauthorized(); + } + + /** + * @test + * @return void + */ + public function Authorize_WhenFails_Forbidden(): void + { + // Arrange + $this->authenticateUser(); + Route::get('test1/{document}', DocumentController1::class) + ->middleware([AuthorizationMiddleware::class]) + ->name('test1'); + $user = User::query()->create(); + $document = Document::query()->create([ + 'user_id' => $user->getKey() + ]); + + // Act + /** @var TestResponse[] $responses */ + $responses = [ + $this->getJson(URL::route('test1', $document)), + ]; + + // Assert + $responses[0]->assertForbidden(); + } + + /** + * @test + * @return void + */ + public function Authorize_WhenPasses_Success(): void + { + // Arrange + Route::get('test1/{document}', DocumentController1::class) + ->middleware([AuthorizationMiddleware::class]) + ->name('test1'); + $user = $this->authenticateUser(); + $document = Document::query()->create([ + 'user_id' => $user->getKey() + ]); + + // Act + /** @var TestResponse[] $responses */ + $responses = [ + $this->getJson(URL::route('test1', $document)), + ]; + + // Assert + $responses[0]->assertSuccessful(); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 7ff3d35..c0ea0e7 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -56,6 +56,11 @@ protected function setUp(): void $table->foreignIdFor(User::class); $table->timestamps(); }); + Schema::create('documents', function (Blueprint $table): void { + $table->id(); + $table->foreignIdFor(User::class); + $table->timestamps(); + }); } /** From bd7f35c3ce1c5358719169c34f48e95449fa0a6d Mon Sep 17 00:00:00 2001 From: Marin Calin Date: Thu, 1 Jun 2023 22:13:34 +0300 Subject: [PATCH 4/4] Updated documentation. --- src/Contracts/Services/IAuthorizationService.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Contracts/Services/IAuthorizationService.php b/src/Contracts/Services/IAuthorizationService.php index ba08e5e..cfd6795 100644 --- a/src/Contracts/Services/IAuthorizationService.php +++ b/src/Contracts/Services/IAuthorizationService.php @@ -12,6 +12,8 @@ interface IAuthorizationService { /** + * Check if the current user can access an action behind the given {@link $policy}. + * * @param TResource|null $resource * @param IPolicy|class-string $policy * @return bool @@ -20,6 +22,8 @@ interface IAuthorizationService public function authorizePolicy(mixed $resource, IPolicy|string $policy): bool; /** + * Check if the current user can access an action behind the given {@link $requirements}. + * * @param TResource|null $resource * @param iterable $requirements * @return bool