Merge pull request #2381 from erikn69/patch-17

[v6] fix BadMethodCallException: undefined methods hasAnyRole, hasAnyPermissions
spatie · Mar 30, 2023 · eea8e15 · eea8e15
2 parents d764505 + 9384223
commit eea8e15
Show file tree

Hide file tree

Showing 8 changed files with 129 additions and 8 deletions.
diff --git a/src/Exceptions/UnauthorizedException.php b/src/Exceptions/UnauthorizedException.php
@@ -2,6 +2,7 @@
 
 namespace Spatie\Permission\Exceptions;
 
+use Illuminate\Contracts\Auth\Access\Authorizable;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 
 class UnauthorizedException extends HttpException
@@ -52,6 +53,13 @@ public static function forRolesOrPermissions(array $rolesOrPermissions): self
         return $exception;
     }
 
+    public static function missingTraitHasRoles(Authorizable $user): self
+    {
+        $class = get_class($user);
+
+        return new static(403, "Authorizable class `{$class}` must use Spatie\Permission\Traits\HasRoles trait.", null, []);
+    }
+
     public static function notLoggedIn(): self
     {
         return new static(403, 'User is not logged in.', null, []);

diff --git a/src/Middlewares/PermissionMiddleware.php b/src/Middlewares/PermissionMiddleware.php
@@ -3,28 +3,33 @@
 namespace Spatie\Permission\Middlewares;
 
 use Closure;
+use Illuminate\Support\Facades\Auth;
 use Spatie\Permission\Exceptions\UnauthorizedException;
 
 class PermissionMiddleware
 {
     public function handle($request, Closure $next, $permission, $guard = null)
     {
-        $authGuard = app('auth')->guard($guard);
+        $authGuard = Auth::guard($guard);
 
         if ($authGuard->guest()) {
             throw UnauthorizedException::notLoggedIn();
         }
 
+        $user = $authGuard->user();
+
+        if (! method_exists($user, 'hasAnyPermission')) {
+            throw UnauthorizedException::missingTraitHasRoles($user);
+        }
+
         $permissions = is_array($permission)
             ? $permission
             : explode('|', $permission);
 
-        foreach ($permissions as $permission) {
-            if ($authGuard->user()->can($permission)) {
-                return $next($request);
-            }
+        if (! $user->canAny($permissions)) {
+            throw UnauthorizedException::forPermissions($permissions);
         }
 
-        throw UnauthorizedException::forPermissions($permissions);
+        return $next($request);
     }
 }
diff --git a/src/Middlewares/RoleMiddleware.php b/src/Middlewares/RoleMiddleware.php
@@ -16,11 +16,17 @@ public function handle($request, Closure $next, $role, $guard = null)
             throw UnauthorizedException::notLoggedIn();
         }
 
+        $user = $authGuard->user();
+
+        if (! method_exists($user, 'hasAnyRole')) {
+            throw UnauthorizedException::missingTraitHasRoles($user);
+        }
+
         $roles = is_array($role)
             ? $role
             : explode('|', $role);
 
-        if (! $authGuard->user()->hasAnyRole($roles)) {
+        if (! $user->hasAnyRole($roles) && ! $user->can('')) {
             throw UnauthorizedException::forRoles($roles);
         }
 

diff --git a/src/Middlewares/RoleOrPermissionMiddleware.php b/src/Middlewares/RoleOrPermissionMiddleware.php
@@ -15,11 +15,17 @@ public function handle($request, Closure $next, $roleOrPermission, $guard = null
             throw UnauthorizedException::notLoggedIn();
         }
 
+        $user = $authGuard->user();
+
+        if (! method_exists($user, 'hasAnyRole') || ! method_exists($user, 'hasAnyPermission')) {
+            throw UnauthorizedException::missingTraitHasRoles($user);
+        }
+
         $rolesOrPermissions = is_array($roleOrPermission)
             ? $roleOrPermission
             : explode('|', $roleOrPermission);
 
-        if (! $authGuard->user()->hasAnyRole($rolesOrPermissions) && ! $authGuard->user()->hasAnyPermission($rolesOrPermissions)) {
+        if (! $user->canAny($rolesOrPermissions) && ! $user->hasAnyRole($rolesOrPermissions)) {
             throw UnauthorizedException::forRolesOrPermissions($rolesOrPermissions);
         }
 

diff --git a/tests/PermissionMiddlewareTest.php b/tests/PermissionMiddlewareTest.php
@@ -6,10 +6,12 @@
 use Illuminate\Http\Response;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Gate;
 use InvalidArgumentException;
 use Spatie\Permission\Contracts\Permission;
 use Spatie\Permission\Exceptions\UnauthorizedException;
 use Spatie\Permission\Middlewares\PermissionMiddleware;
+use Spatie\Permission\Tests\TestModels\UserWithoutHasRoles;
 
 class PermissionMiddlewareTest extends TestCase
 {
@@ -69,6 +71,21 @@ public function a_user_cannot_access_a_route_protected_by_the_permission_middlew
         );
     }
 
+    /** @test */
+    public function a_super_admin_user_can_access_a_route_protected_by_permission_middleware()
+    {
+        Auth::login($this->testUser);
+
+        Gate::before(function ($user, $ability) {
+            return $user->getKey() ===  $this->testUser->getKey() ? true : null;
+        });
+
+        $this->assertEquals(
+            200,
+            $this->runMiddleware($this->permissionMiddleware, 'edit-articles')
+        );
+    }
+
     /** @test */
     public function a_user_can_access_a_route_protected_by_permission_middleware_if_have_this_permission()
     {
@@ -100,6 +117,19 @@ public function a_user_can_access_a_route_protected_by_this_permission_middlewar
         );
     }
 
+    /** @test */
+    public function a_user_cannot_access_a_route_protected_by_the_permission_middleware_if_have_not_has_roles_trait()
+    {
+        $userWithoutHasRoles = UserWithoutHasRoles::create(['email' => 'test_not_has_roles@user.com']);
+
+        Auth::login($userWithoutHasRoles);
+
+        $this->assertEquals(
+            403,
+            $this->runMiddleware($this->permissionMiddleware, 'edit-news')
+        );
+    }
+
     /** @test */
     public function a_user_cannot_access_a_route_protected_by_the_permission_middleware_if_have_a_different_permission()
     {

diff --git a/tests/RoleMiddlewareTest.php b/tests/RoleMiddlewareTest.php
@@ -6,9 +6,11 @@
 use Illuminate\Http\Response;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Gate;
 use InvalidArgumentException;
 use Spatie\Permission\Exceptions\UnauthorizedException;
 use Spatie\Permission\Middlewares\RoleMiddleware;
+use Spatie\Permission\Tests\TestModels\UserWithoutHasRoles;
 
 class RoleMiddlewareTest extends TestCase
 {
@@ -74,6 +76,19 @@ public function a_user_can_access_a_route_protected_by_this_role_middleware_if_h
         );
     }
 
+    /** @test */
+    public function a_user_cannot_access_a_route_protected_by_the_role_middleware_if_have_not_has_roles_trait()
+    {
+        $userWithoutHasRoles = UserWithoutHasRoles::create(['email' => 'test_not_has_roles@user.com']);
+
+        Auth::login($userWithoutHasRoles);
+
+        $this->assertEquals(
+            403,
+            $this->runMiddleware($this->roleMiddleware, 'testRole')
+        );
+    }
+
     /** @test */
     public function a_user_cannot_access_a_route_protected_by_the_role_middleware_if_have_a_different_role()
     {

diff --git a/tests/RoleOrPermissionMiddlewareTest.php b/tests/RoleOrPermissionMiddlewareTest.php
@@ -6,9 +6,11 @@
 use Illuminate\Http\Response;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Gate;
 use InvalidArgumentException;
 use Spatie\Permission\Exceptions\UnauthorizedException;
 use Spatie\Permission\Middlewares\RoleOrPermissionMiddleware;
+use Spatie\Permission\Tests\TestModels\UserWithoutHasRoles;
 
 class RoleOrPermissionMiddlewareTest extends TestCase
 {
@@ -64,6 +66,34 @@ public function a_user_can_access_a_route_protected_by_permission_or_role_middle
         );
     }
 
+    /** @test */
+    public function a_super_admin_user_can_access_a_route_protected_by_permission_or_role_middleware()
+    {
+        Auth::login($this->testUser);
+
+        Gate::before(function ($user, $ability) {
+            return $user->getKey() ===  $this->testUser->getKey() ? true : null;
+        });
+
+        $this->assertEquals(
+            200,
+            $this->runMiddleware($this->roleOrPermissionMiddleware, 'testRole|edit-articles')
+        );
+    }
+
+    /** @test */
+    public function a_user_can_not_access_a_route_protected_by_permission_or_role_middleware_if_have_not_has_roles_trait()
+    {
+        $userWithoutHasRoles = UserWithoutHasRoles::create(['email' => 'test_not_has_roles@user.com']);
+
+        Auth::login($userWithoutHasRoles);
+
+        $this->assertEquals(
+            403,
+            $this->runMiddleware($this->roleOrPermissionMiddleware, 'testRole|edit-articles')
+        );
+    }
+
     /** @test */
     public function a_user_can_not_access_a_route_protected_by_permission_or_role_middleware_if_have_not_this_permission_and_role()
     {

diff --git a/tests/TestModels/UserWithoutHasRoles.php b/tests/TestModels/UserWithoutHasRoles.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace Spatie\Permission\Tests\TestModels;
+
+use Illuminate\Auth\Authenticatable;
+use Illuminate\Contracts\Auth\Access\Authorizable as AuthorizableContract;
+use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Foundation\Auth\Access\Authorizable;
+
+class UserWithoutHasRoles extends Model implements AuthorizableContract, AuthenticatableContract
+{
+    use Authorizable;
+    use Authenticatable;
+
+    protected $fillable = ['email'];
+
+    public $timestamps = false;
+
+    protected $table = 'users';
+}