From 99068a37c4f78c543a67494f9225e5396edd09b4 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 14:23:21 +0300 Subject: [PATCH 1/7] readd log registration exception --- config/permission.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/config/permission.php b/config/permission.php index 7a4eab9..9d7bba7 100644 --- a/config/permission.php +++ b/config/permission.php @@ -52,5 +52,13 @@ * role is updated. Then the cache will be flushed immediately. */ - 'cache_expiration_time' => 60 * 24, + 'cache_expiration_time' => 60 * 24, + + /* + * By default we'll make an entry in the application log when the permissions + * could not be loaded. Normally this only occurs while installing the packages. + * + * If for some reason you want to disable that logging, set this value to false. + */ + 'log_registration_exception' => true, ]; From 6ab4fa2a4de96e550a07df544bee4934322fec88 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 14:58:31 +0300 Subject: [PATCH 2/7] test show permission config --- README.md | 8 +++ config/permission.php | 11 +++- src/Exceptions/UnauthorizedException.php | 11 +++- src/Exceptions/UnauthorizedPermission.php | 11 ++++ src/Exceptions/UnauthorizedRole.php | 11 ++++ src/Helpers.php | 14 ++++- src/Middlewares/PermissionMiddleware.php | 6 ++- src/Middlewares/RoleMiddleware.php | 3 +- tests/HasPermissionsTest.php | 4 +- tests/HasRolesTest.php | 12 ++--- tests/MiddlewareTest.php | 62 ++++++++++++++++------- tests/PermissionTest.php | 2 +- tests/RoleTest.php | 14 ++--- tests/TestCase.php | 26 +++++++--- 14 files changed, 146 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index e9404cb..6c13195 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,14 @@ return [ */ 'log_registration_exception' => true, + + /* + * When set to true, the required permission/role names are added to the exception + * message. This could be considered an information leak in some contexts, so + * the default setting is false here for optimum safety. + */ + + 'display_permission_in_exception' => false, ]; ``` diff --git a/config/permission.php b/config/permission.php index 9d7bba7..6f79028 100644 --- a/config/permission.php +++ b/config/permission.php @@ -52,7 +52,7 @@ * role is updated. Then the cache will be flushed immediately. */ - 'cache_expiration_time' => 60 * 24, + 'cache_expiration_time' => 60 * 24, /* * By default we'll make an entry in the application log when the permissions @@ -60,5 +60,14 @@ * * If for some reason you want to disable that logging, set this value to false. */ + 'log_registration_exception' => true, + + /* + * When set to true, the required permission/role names are added to the exception + * message. This could be considered an information leak in some contexts, so + * the default setting is false here for optimum safety. + */ + + 'display_permission_in_exception' => false, ]; diff --git a/src/Exceptions/UnauthorizedException.php b/src/Exceptions/UnauthorizedException.php index bbf44b3..3f75a3e 100644 --- a/src/Exceptions/UnauthorizedException.php +++ b/src/Exceptions/UnauthorizedException.php @@ -2,7 +2,6 @@ namespace Maklad\Permission\Exceptions; -use http\Exception; use Symfony\Component\HttpKernel\Exception\HttpException; /** @@ -12,13 +11,18 @@ class UnauthorizedException extends HttpException { + private $requiredRoles = []; + private $requiredPermissions = []; + /** * UnauthorizedException constructor. * * @param $statusCode * @param null $message + * @param array $requiredRoles + * @param array $requiredPermissions */ - public function __construct($statusCode, $message = null) + public function __construct($statusCode, $message = null, $requiredRoles = [], $requiredPermissions = []) { parent::__construct($statusCode, $message); @@ -26,5 +30,8 @@ public function __construct($statusCode, $message = null) $logger = \app('log'); $logger->alert($message); } + + $this->requiredRoles = $requiredRoles; + $this->requiredPermissions = $requiredPermissions; } } diff --git a/src/Exceptions/UnauthorizedPermission.php b/src/Exceptions/UnauthorizedPermission.php index a40a94f..9caf033 100644 --- a/src/Exceptions/UnauthorizedPermission.php +++ b/src/Exceptions/UnauthorizedPermission.php @@ -8,4 +8,15 @@ */ class UnauthorizedPermission extends UnauthorizedException { + /** + * UnauthorizedPermission constructor. + * + * @param $statusCode + * @param null $message + * @param array $requiredPermissions + */ + public function __construct($statusCode, $message = null, $requiredPermissions = []) + { + parent::__construct($statusCode, $message, [], $requiredPermissions); + } } diff --git a/src/Exceptions/UnauthorizedRole.php b/src/Exceptions/UnauthorizedRole.php index 4d3aa6e..8de4fb6 100644 --- a/src/Exceptions/UnauthorizedRole.php +++ b/src/Exceptions/UnauthorizedRole.php @@ -8,4 +8,15 @@ */ class UnauthorizedRole extends UnauthorizedException { + /** + * UnauthorizedPermission constructor. + * + * @param $statusCode + * @param null $message + * @param array $requiredRoles + */ + public function __construct($statusCode, $message = null, $requiredRoles = []) + { + parent::__construct($statusCode, $message, $requiredRoles); + } } diff --git a/src/Helpers.php b/src/Helpers.php index ea51f74..282cd57 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -87,7 +87,12 @@ public function getRoleDoesNotExistMessage(string $name, string $guardName): str */ public function getUnauthorizedRoleMessage(string $roles): string { - return "User does not have the right roles `{$roles}`."; + $message = 'User does not have the right roles.'; + if (config('permission.display_permission_in_exception')) { + $message = "User does not have the right roles `{$roles}`."; + } + + return $message; } /** @@ -97,7 +102,12 @@ public function getUnauthorizedRoleMessage(string $roles): string */ public function getUnauthorizedPermissionMessage(string $permissions): string { - return "User does not have the right permissions `{$permissions}`."; + $message = 'User does not have the right permissions.'; + if (config('permission.display_permission_in_exception')) { + $message = "User does not have the right permissions `{$permissions}`."; + } + + return $message; } /** diff --git a/src/Middlewares/PermissionMiddleware.php b/src/Middlewares/PermissionMiddleware.php index 61fadb1..1e32cde 100644 --- a/src/Middlewares/PermissionMiddleware.php +++ b/src/Middlewares/PermissionMiddleware.php @@ -4,6 +4,7 @@ namespace Maklad\Permission\Middlewares; use Closure; +use Maklad\Permission\Exceptions\UnauthorizedPermission; use Maklad\Permission\Exceptions\UserNotLoggedIn; use Maklad\Permission\Helpers; @@ -31,9 +32,10 @@ public function handle($request, Closure $next, $permission) $permissions = \is_array($permission) ? $permission : \explode('|', $permission); - if (! app('auth')->user()->hasAnyPermission($permissions)) { + if ( ! app('auth')->user()->hasAnyPermission($permissions)) { $helpers = new Helpers(); - throw new UserNotLoggedIn(403, $helpers->getUnauthorizedPermissionMessage(implode(', ', $permissions))); + throw new UnauthorizedPermission(403, + $helpers->getUnauthorizedPermissionMessage(implode(', ', $permissions)), $permissions); } return $next($request); diff --git a/src/Middlewares/RoleMiddleware.php b/src/Middlewares/RoleMiddleware.php index a4457ef..92db2d4 100644 --- a/src/Middlewares/RoleMiddleware.php +++ b/src/Middlewares/RoleMiddleware.php @@ -4,6 +4,7 @@ namespace Maklad\Permission\Middlewares; use Closure; +use Maklad\Permission\Exceptions\UnauthorizedRole; use Maklad\Permission\Exceptions\UserNotLoggedIn; use Maklad\Permission\Helpers; @@ -32,7 +33,7 @@ public function handle($request, Closure $next, $role) if (! app('auth')->user()->hasAnyRole($roles)) { $helpers = new Helpers(); - throw new UserNotLoggedIn(403, $helpers->getUnauthorizedRoleMessage(implode(', ', $roles))); + throw new UnauthorizedRole(403, $helpers->getUnauthorizedRoleMessage(implode(', ', $roles)), $roles); } return $next($request); diff --git a/tests/HasPermissionsTest.php b/tests/HasPermissionsTest.php index 613e8b7..e6461de 100644 --- a/tests/HasPermissionsTest.php +++ b/tests/HasPermissionsTest.php @@ -24,7 +24,7 @@ public function it_throws_an_exception_when_assigning_a_permission_that_does_not $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionDoesNotExist::class); @@ -43,7 +43,7 @@ public function it_throws_an_exception_when_assigning_a_permission_to_a_user_fro $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); diff --git a/tests/HasRolesTest.php b/tests/HasRolesTest.php index 63ceb90..9b6a9b0 100644 --- a/tests/HasRolesTest.php +++ b/tests/HasRolesTest.php @@ -66,7 +66,7 @@ public function it_throws_an_exception_when_assigning_a_role_that_does_not_exist $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(RoleDoesNotExist::class); @@ -85,7 +85,7 @@ public function it_can_only_assign_roles_from_the_correct_guard() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(RoleDoesNotExist::class); @@ -104,7 +104,7 @@ public function it_throws_an_exception_when_assigning_a_role_from_a_different_gu $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); @@ -169,7 +169,7 @@ public function it_throws_an_exception_when_syncing_a_role_from_another_guard() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); @@ -323,7 +323,7 @@ public function it_throws_an_exception_when_the_permission_does_not_exist() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionDoesNotExist::class); @@ -342,7 +342,7 @@ public function it_throws_an_exception_when_the_permission_does_not_exist_for_th $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionDoesNotExist::class); diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 1aa9627..9236907 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -30,7 +30,7 @@ public function a_guest_cannot_access_a_route_protected_by_the_role_middleware() $can_logs = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -93,10 +93,10 @@ public function a_user_cannot_access_a_route_protected_by_the_role_middleware_if $this->testUser->assignRole(['testRole']); $can_logs = [false, true]; + $show_permissions = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); - + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( $this->roleMiddleware, @@ -105,8 +105,12 @@ public function a_user_cannot_access_a_route_protected_by_the_role_middleware_if 403 ); - $message = $this->helpers->getUnauthorizedRoleMessage('testRole2'); - $this->assertLogMessage($message, Logger::ALERT); + foreach ($show_permissions as $show_permission) { + config('permission.display_permission_in_exception', $show_permission); + $message = $this->helpers->getUnauthorizedRoleMessage('testRole2'); + $this->assertShowPermission($message, 'testRole2'); + $this->assertLogMessage($message, Logger::ALERT); + } } } @@ -116,9 +120,10 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_hav Auth::login($this->testUser); $can_logs = [false, true]; + $show_permissions = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -128,8 +133,13 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_hav 403 ); - $message = $this->helpers->getUnauthorizedRoleMessage('testRole, testRole2'); - $this->assertLogMessage($message, Logger::ALERT); + foreach ($show_permissions as $show_permission) { + config('permission.display_permission_in_exception', $show_permission); + $message = $this->helpers->getUnauthorizedRoleMessage('testRole, testRole2'); + $this->assertShowPermission($message, 'testRole'); + $this->assertShowPermission($message, 'testRole2'); + $this->assertLogMessage($message, Logger::ALERT); + } } } @@ -139,9 +149,10 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_rol Auth::login($this->testUser); $can_logs = [false, true]; + $show_permissions = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -151,8 +162,12 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_rol 403 ); - $message = $this->helpers->getUnauthorizedRoleMessage(''); - $this->assertLogMessage($message, Logger::ALERT); + foreach ($show_permissions as $show_permission) { + config('permission.display_permission_in_exception', $show_permission); + $message = $this->helpers->getUnauthorizedRoleMessage('test'); + $this->assertShowPermission($message, 'test'); + $this->assertLogMessage($message, Logger::ALERT); + } } } @@ -162,7 +177,7 @@ public function a_guest_cannot_access_a_route_protected_by_the_permission_middle $can_logs = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -225,9 +240,10 @@ public function a_user_cannot_access_a_route_protected_by_the_permission_middlew $this->testUser->givePermissionTo('edit-articles'); $can_logs = [false, true]; + $show_permissions = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -237,8 +253,12 @@ public function a_user_cannot_access_a_route_protected_by_the_permission_middlew 403 ); - $message = $this->helpers->getUnauthorizedPermissionMessage('edit-news'); - $this->assertLogMessage($message, Logger::ALERT); + foreach ($show_permissions as $show_permission) { + config('permission.display_permission_in_exception', $show_permission); + $message = $this->helpers->getUnauthorizedPermissionMessage('edit-news'); + $this->assertShowPermission($message, 'edit-news'); + $this->assertLogMessage($message, Logger::ALERT); + } } } @@ -248,9 +268,10 @@ public function a_user_cannot_access_a_route_protected_by_permission_middleware_ Auth::login($this->testUser); $can_logs = [false, true]; + $show_permissions = [false, true]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); $this->assertEquals( $this->runMiddleware( @@ -260,8 +281,13 @@ public function a_user_cannot_access_a_route_protected_by_permission_middleware_ 403 ); - $message = $this->helpers->getUnauthorizedPermissionMessage('edit-articles, edit-news'); - $this->assertLogMessage($message, Logger::ALERT); + foreach ($show_permissions as $show_permission) { + config('permission.display_permission_in_exception', $show_permission); + $message = $this->helpers->getUnauthorizedPermissionMessage('edit-articles, edit-news'); + $this->assertShowPermission($message, 'edit-articles'); + $this->assertShowPermission($message, 'edit-news'); + $this->assertLogMessage($message, Logger::ALERT); + } } } diff --git a/tests/PermissionTest.php b/tests/PermissionTest.php index ef9957b..024cb1e 100644 --- a/tests/PermissionTest.php +++ b/tests/PermissionTest.php @@ -14,7 +14,7 @@ public function it_throws_an_exception_when_the_permission_already_exists() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionAlreadyExists::class); diff --git a/tests/RoleTest.php b/tests/RoleTest.php index b66c533..49f05ba 100644 --- a/tests/RoleTest.php +++ b/tests/RoleTest.php @@ -41,7 +41,7 @@ public function it_throws_an_exception_when_the_role_already_exists() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(RoleAlreadyExists::class); @@ -69,7 +69,7 @@ public function it_throws_an_exception_when_given_a_permission_that_does_not_exi $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionDoesNotExist::class); @@ -88,7 +88,7 @@ public function it_throws_an_exception_when_given_a_permission_that_belongs_to_a $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); @@ -137,7 +137,7 @@ public function it_throws_an_exception_when_syncing_permissions_that_do_not_exis $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->testUserRole->givePermissionTo('edit-articles'); @@ -158,7 +158,7 @@ public function it_throws_an_exception_when_syncing_permissions_that_belong_to_a $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); @@ -219,7 +219,7 @@ public function it_throws_an_exception_if_the_permission_does_not_exist() $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(PermissionDoesNotExist::class); @@ -246,7 +246,7 @@ public function it_throws_an_exception_when_a_permission_of_the_wrong_guard_is_p $can_logs = [true, false]; foreach ($can_logs as $can_log) { - $this->app['config']->set('permission.log_registration_exception', $can_log); + config('permission.log_registration_exception', $can_log); try { $this->expectException(GuardDoesNotMatch::class); diff --git a/tests/TestCase.php b/tests/TestCase.php index 1254821..623cda2 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -74,7 +74,7 @@ protected function getPackageProviders($app) { return [ PermissionServiceProvider::class, - MongodbServiceProvider::class + MongodbServiceProvider::class, ]; } @@ -91,7 +91,7 @@ protected function getEnvironmentSetUp($app) 'port' => '27017', 'driver' => 'mongodb', 'database' => 'laravel_permission_mongodb_test', - 'prefix' => '' + 'prefix' => '', ]); $app['config']->set('view.paths', [__DIR__ . '/resources/views']); @@ -178,11 +178,11 @@ protected function assertLogged($message, $level) protected function hasLog($message, $level) { return \collect($this->app['log']->getMonolog()->getHandlers())->filter(function ($handler) use ( - $message, - $level - ) { - return $handler instanceof TestHandler && $handler->hasRecordThatContains($message, $level); - })->count() > 0; + $message, + $level + ) { + return $handler instanceof TestHandler && $handler->hasRecordThatContains($message, $level); + })->count() > 0; } /** @@ -196,4 +196,16 @@ protected function assertLogMessage($message, $level) $this->assertNotLogged($message, $level); } } + + /** + * @param $message + */ + protected function assertShowPermission($message, $role_permission) + { + if (\config('permission.display_permission_in_exception')) { + $this->assertContains($role_permission, $message); + } else { + $this->assertNotContains($role_permission, $message); + } + } } From e2eb5f42f7122bb837adb0316399fc6dfc0cb334 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 15:04:47 +0300 Subject: [PATCH 3/7] test if required permissions can be fetched from the exception --- src/Exceptions/UnauthorizedException.php | 20 ++++++++++++++++ tests/MiddlewareTest.php | 29 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/Exceptions/UnauthorizedException.php b/src/Exceptions/UnauthorizedException.php index 3f75a3e..1c4fe4d 100644 --- a/src/Exceptions/UnauthorizedException.php +++ b/src/Exceptions/UnauthorizedException.php @@ -34,4 +34,24 @@ public function __construct($statusCode, $message = null, $requiredRoles = [], $ $this->requiredRoles = $requiredRoles; $this->requiredPermissions = $requiredPermissions; } + + /** + * Return Required Roles + * + * @return array + */ + public function getRequiredRoles(): array + { + return $this->requiredRoles; + } + + /** + * Return Required Permissions + * + * @return array + */ + public function getRequiredPermissions(): array + { + return $this->requiredPermissions; + } } diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 9236907..953b800 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -291,6 +291,35 @@ public function a_user_cannot_access_a_route_protected_by_permission_middleware_ } } + /** @test */ + public function the_required_roles_can_be_fetched_from_the_exception() + { + Auth::login($this->testUser); + $requiredRoles = []; + try { + $this->roleMiddleware->handle(new Request(), function () { + return (new Response())->setContent(''); + }, 'testRole'); + } catch (UnauthorizedException $e) { + $requiredRoles = $e->getRequiredRoles(); + } + $this->assertEquals(['testRole'], $requiredRoles); + } + /** @test */ + public function the_required_permissions_can_be_fetched_from_the_exception() + { + Auth::login($this->testUser); + $requiredPermissions = []; + try { + $this->permissionMiddleware->handle(new Request(), function () { + return (new Response())->setContent(''); + }, 'edit-articles'); + } catch (UnauthorizedException $e) { + $requiredPermissions = $e->getRequiredPermissions(); + } + $this->assertEquals(['edit-articles'], $requiredPermissions); + } + protected function runMiddleware($middleware, $parameter) { try { From 30409e1f62cc5607b99452e4f558932250dc8587 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 15:06:06 +0300 Subject: [PATCH 4/7] fix psr2 styling --- src/Middlewares/PermissionMiddleware.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Middlewares/PermissionMiddleware.php b/src/Middlewares/PermissionMiddleware.php index 1e32cde..a0fb162 100644 --- a/src/Middlewares/PermissionMiddleware.php +++ b/src/Middlewares/PermissionMiddleware.php @@ -32,10 +32,13 @@ public function handle($request, Closure $next, $permission) $permissions = \is_array($permission) ? $permission : \explode('|', $permission); - if ( ! app('auth')->user()->hasAnyPermission($permissions)) { + if (! app('auth')->user()->hasAnyPermission($permissions)) { $helpers = new Helpers(); - throw new UnauthorizedPermission(403, - $helpers->getUnauthorizedPermissionMessage(implode(', ', $permissions)), $permissions); + throw new UnauthorizedPermission( + 403, + $helpers->getUnauthorizedPermissionMessage(implode(', ', $permissions)), + $permissions + ); } return $next($request); From 327b90a0eb69f301e515d5c7d7d53fbaee83917a Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 15:20:21 +0300 Subject: [PATCH 5/7] update phpunit to ^7.0 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b01b5db..ce69bc3 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,7 @@ "codeclimate/php-test-reporter": "^0.4.4", "monolog/monolog": "^1.23", "orchestra/testbench": "^3.2.0", - "phpunit/phpunit": "^5.7|^6.0", + "phpunit/phpunit": "^5.7|^6.0|^7.0", "squizlabs/php_codesniffer": "^3.1" }, "autoload": { From 1241d298a7bc775cb00a65dabd6ca7b11e97c419 Mon Sep 17 00:00:00 2001 From: mostafamaklad Date: Sun, 11 Mar 2018 16:12:42 +0000 Subject: [PATCH 6/7] Apply fixes from StyleCI --- src/Exceptions/UnauthorizedException.php | 1 - tests/TestCase.php | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Exceptions/UnauthorizedException.php b/src/Exceptions/UnauthorizedException.php index 1c4fe4d..5545cc5 100644 --- a/src/Exceptions/UnauthorizedException.php +++ b/src/Exceptions/UnauthorizedException.php @@ -10,7 +10,6 @@ */ class UnauthorizedException extends HttpException { - private $requiredRoles = []; private $requiredPermissions = []; diff --git a/tests/TestCase.php b/tests/TestCase.php index 623cda2..52535a2 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -181,8 +181,8 @@ protected function hasLog($message, $level) $message, $level ) { - return $handler instanceof TestHandler && $handler->hasRecordThatContains($message, $level); - })->count() > 0; + return $handler instanceof TestHandler && $handler->hasRecordThatContains($message, $level); + })->count() > 0; } /** From 19de8846df63b7bdfeefbaa64a3f60a9516c6749 Mon Sep 17 00:00:00 2001 From: Mostafa Maklad Date: Sun, 11 Mar 2018 20:23:28 +0300 Subject: [PATCH 7/7] update code coverage --- src/Helpers.php | 12 ++++++------ tests/MiddlewareTest.php | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Helpers.php b/src/Helpers.php index 282cd57..ad60448 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -87,9 +87,9 @@ public function getRoleDoesNotExistMessage(string $name, string $guardName): str */ public function getUnauthorizedRoleMessage(string $roles): string { - $message = 'User does not have the right roles.'; - if (config('permission.display_permission_in_exception')) { - $message = "User does not have the right roles `{$roles}`."; + $message = "User does not have the right roles `{$roles}`."; + if (! config('permission.display_permission_in_exception')) { + $message = 'User does not have the right roles.'; } return $message; @@ -102,9 +102,9 @@ public function getUnauthorizedRoleMessage(string $roles): string */ public function getUnauthorizedPermissionMessage(string $permissions): string { - $message = 'User does not have the right permissions.'; - if (config('permission.display_permission_in_exception')) { - $message = "User does not have the right permissions `{$permissions}`."; + $message = "User does not have the right permissions `{$permissions}`."; + if (! config('permission.display_permission_in_exception')) { + $message = 'User does not have the right permissions.'; } return $message; diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 953b800..ceaa1fe 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -93,7 +93,7 @@ public function a_user_cannot_access_a_route_protected_by_the_role_middleware_if $this->testUser->assignRole(['testRole']); $can_logs = [false, true]; - $show_permissions = [false, true]; + $show_permissions = [true, false]; foreach ($can_logs as $can_log) { config('permission.log_registration_exception', $can_log); @@ -120,7 +120,7 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_hav Auth::login($this->testUser); $can_logs = [false, true]; - $show_permissions = [false, true]; + $show_permissions = [true, false]; foreach ($can_logs as $can_log) { config('permission.log_registration_exception', $can_log); @@ -149,7 +149,7 @@ public function a_user_cannot_access_a_route_protected_by_role_middleware_if_rol Auth::login($this->testUser); $can_logs = [false, true]; - $show_permissions = [false, true]; + $show_permissions = [true, false]; foreach ($can_logs as $can_log) { config('permission.log_registration_exception', $can_log); @@ -240,7 +240,7 @@ public function a_user_cannot_access_a_route_protected_by_the_permission_middlew $this->testUser->givePermissionTo('edit-articles'); $can_logs = [false, true]; - $show_permissions = [false, true]; + $show_permissions = [true, false]; foreach ($can_logs as $can_log) { config('permission.log_registration_exception', $can_log); @@ -268,7 +268,7 @@ public function a_user_cannot_access_a_route_protected_by_permission_middleware_ Auth::login($this->testUser); $can_logs = [false, true]; - $show_permissions = [false, true]; + $show_permissions = [true, false]; foreach ($can_logs as $can_log) { config('permission.log_registration_exception', $can_log);