Merge pull request #2089 from erikn69/fix_2022

[V6] Full uuid/guid/ulid support
spatie · Feb 10, 2023 · 3115a43 · 3115a43
2 parents b1ff160 + b2c858b
commit 3115a43
Show file tree

Hide file tree

Showing 10 changed files with 158 additions and 13 deletions.
diff --git a/src/Models/Permission.php b/src/Models/Permission.php
@@ -104,13 +104,13 @@ public static function findByName(string $name, $guardName = null): PermissionCo
     /**
      * Find a permission by its id (and optionally guardName).
      *
-     * @param  int  $id
+     * @param  int|string  $id
      * @param  string|null  $guardName
      * @return \Spatie\Permission\Contracts\Permission
      *
      * @throws \Spatie\Permission\Exceptions\PermissionDoesNotExist
      */
-    public static function findById(int $id, $guardName = null): PermissionContract
+    public static function findById($id, $guardName = null): PermissionContract
     {
         $guardName = $guardName ?? Guard::getDefaultName(static::class);
         $permission = static::getPermission([(new static())->getKeyName() => $id, 'guard_name' => $guardName]);

diff --git a/src/Models/Role.php b/src/Models/Role.php
@@ -110,11 +110,11 @@ public static function findByName(string $name, $guardName = null): RoleContract
     /**
      * Find a role by its id (and optionally guardName).
      *
-     * @param  int  $id
+     * @param  int|string  $id
      * @param  string|null  $guardName
      * @return \Spatie\Permission\Contracts\Role|\Spatie\Permission\Models\Role
      */
-    public static function findById(int $id, $guardName = null): RoleContract
+    public static function findById($id, $guardName = null): RoleContract
     {
         $guardName = $guardName ?? Guard::getDefaultName(static::class);
 

diff --git a/src/PermissionRegistrar.php b/src/PermissionRegistrar.php
@@ -384,4 +384,25 @@ private function hydrateRolesCache()
 
         $this->permissions['roles'] = [];
     }
+
+    public static function isUid($value)
+    {
+        if (! is_string($value) || empty(trim($value))) {
+            return false;
+        }
+
+        // check if is UUID/GUID
+        $uid = preg_match('/^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$/iD', $value) > 0;
+        if ($uid) {
+            return true;
+        }
+
+        // check if is ULID
+        $ulid = 26 == strlen($value) && 26 == strspn($value, '0123456789ABCDEFGHJKMNPQRSTVWXYZabcdefghjkmnpqrstvwxyz') && $value[0] <= '7';
+        if ($ulid) {
+            return true;
+        }
+
+        return false;
+    }
 }
diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php
@@ -134,7 +134,7 @@ protected function convertToPermissionModels($permissions): array
             if ($permission instanceof Permission) {
                 return $permission;
             }
-            $method = is_string($permission) ? 'findByName' : 'findById';
+            $method = is_string($permission) && ! PermissionRegistrar::isUid($permission) ? 'findByName' : 'findById';
 
             return $this->getPermissionClass()->{$method}($permission, $this->getDefaultGuardName());
         }, Arr::wrap($permissions));
@@ -152,14 +152,14 @@ public function filterPermission($permission, $guardName = null)
     {
         $permissionClass = $this->getPermissionClass();
 
-        if (is_string($permission)) {
+        if (is_string($permission) && ! PermissionRegistrar::isUid($permission)) {
             $permission = $permissionClass->findByName(
                 $permission,
                 $guardName ?? $this->getDefaultGuardName()
             );
         }
 
-        if (is_int($permission)) {
+        if (is_int($permission) || is_string($permission)) {
             $permission = $permissionClass->findById(
                 $permission,
                 $guardName ?? $this->getDefaultGuardName()
@@ -204,7 +204,7 @@ protected function hasWildcardPermission($permission, $guardName = null): bool
     {
         $guardName = $guardName ?? $this->getDefaultGuardName();
 
-        if (is_int($permission)) {
+        if (is_int($permission) || PermissionRegistrar::isUid($permission)) {
             $permission = $this->getPermissionClass()->findById($permission, $guardName);
         }
 
@@ -449,7 +449,7 @@ protected function getStoredPermission($permissions)
     {
         $permissionClass = $this->getPermissionClass();
 
-        if (is_numeric($permissions)) {
+        if (is_numeric($permissions) || PermissionRegistrar::isUid($permissions)) {
             return $permissionClass->findById($permissions, $this->getDefaultGuardName());
         }
 

diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php
@@ -83,7 +83,7 @@ public function scopeRole(Builder $query, $roles, $guard = null): Builder
                 return $role;
             }
 
-            $method = is_numeric($role) ? 'findById' : 'findByName';
+            $method = is_numeric($role) || PermissionRegistrar::isUid($role) ? 'findById' : 'findByName';
 
             return $this->getRoleClass()->{$method}($role, $guard ?: $this->getDefaultGuardName());
         }, Arr::wrap($roles));
@@ -195,13 +195,13 @@ public function hasRole($roles, string $guard = null): bool
             $roles = $this->convertPipeToArray($roles);
         }
 
-        if (is_string($roles)) {
+        if (is_string($roles) && ! PermissionRegistrar::isUid($roles)) {
             return $guard
                 ? $this->roles->where('guard_name', $guard)->contains('name', $roles)
                 : $this->roles->contains('name', $roles);
         }
 
-        if (is_int($roles)) {
+        if (is_int($roles) || is_string($roles)) {
             $roleClass = $this->getRoleClass();
             $key = (new $roleClass())->getKeyName();
 
@@ -325,7 +325,7 @@ protected function getStoredRole($role): Role
     {
         $roleClass = $this->getRoleClass();
 
-        if (is_numeric($role)) {
+        if (is_numeric($role) || PermissionRegistrar::isUid($role)) {
             return $roleClass->findById($role, $this->getDefaultGuardName());
         }
 

diff --git a/tests/HasPermissionsWithCustomModelsTest.php b/tests/HasPermissionsWithCustomModelsTest.php
@@ -36,4 +36,31 @@ public function it_can_use_custom_fields_from_cache()
 
         $this->assertSame(0, count(DB::getQueryLog()));
     }
+
+    /** @test */
+    public function it_can_scope_users_using_a_int()
+    {
+        // Skipped because custom model uses uuid,
+        // replacement "it_can_scope_users_using_a_uuid"
+        $this->assertTrue(true);
+    }
+
+    /** @test */
+    public function it_can_scope_users_using_a_uuid()
+    {
+        $uuid1 = $this->testUserPermission->getKey();
+        $uuid2 = app(Permission::class)::where('name', 'edit-news')->first()->getKey();
+
+        $user1 = User::create(['email' => '[email protected]']);
+        $user2 = User::create(['email' => '[email protected]']);
+        $user1->givePermissionTo([$uuid1, $uuid2]);
+        $this->testUserRole->givePermissionTo($uuid1);
+        $user2->assignRole('testRole');
+
+        $scopedUsers1 = User::permission($uuid1)->get();
+        $scopedUsers2 = User::permission([$uuid2])->get();
+
+        $this->assertEquals(2, $scopedUsers1->count());
+        $this->assertEquals(1, $scopedUsers2->count());
+    }
 }
diff --git a/tests/Permission.php b/tests/Permission.php
@@ -10,4 +10,24 @@ class Permission extends \Spatie\Permission\Models\Permission
         'permission_test_id',
         'name',
     ];
+
+    protected static function boot()
+    {
+        parent::boot();
+        static::creating(function ($model) {
+            if (empty($model->{$model->getKeyName()})) {
+                $model->{$model->getKeyName()} = \Str::uuid()->toString();
+            }
+        });
+    }
+
+    public function getIncrementing()
+    {
+        return false;
+    }
+
+    public function getKeyType()
+    {
+        return 'string';
+    }
 }
diff --git a/tests/PermissionRegistarTest.php b/tests/PermissionRegistarTest.php
@@ -0,0 +1,51 @@
+<?php
+
+namespace Spatie\Permission\Test;
+
+use Spatie\Permission\PermissionRegistrar;
+
+class PermissionRegistarTest extends TestCase
+{
+    /** @test */
+    public function it_can_check_uids()
+    {
+        $uids = [
+            // UUIDs
+            '00000000-0000-0000-0000-000000000000',
+            '9be37b52-e1fa-4e86-b65f-cbfcbedde838',
+            'fc458041-fb21-4eea-a04b-b55c87a7224a',
+            '78144b52-a889-11ed-afa1-0242ac120002',
+            '78144f4e-a889-11ed-afa1-0242ac120002',
+            // GUIDs
+            '4b8590bb-90a2-4f38-8dc9-70e663a5b0e5',
+            'A98C5A1E-A742-4808-96FA-6F409E799937',
+            '1f01164a-98e9-4246-93ec-7941aefb1da6',
+            '91b73d20-89e6-46b0-b39b-632706cc3ed7',
+            '0df4a5b8-7c2e-484f-ad1d-787d1b83aacc',
+            // ULIDs
+            '01GRVB3DREB63KNN4G2QVV99DF',
+            '01GRVB3DRECY317SJCJ6DMTFCA',
+            '01GRVB3DREGGPBXNH1M24GX1DS',
+            '01GRVB3DRESRM2K9AVQSW1JCKA',
+            '01GRVB3DRES5CQ31PB24MP4CSV',
+        ];
+
+        $not_uids = [
+            '9be37b52-e1fa',
+            '9be37b52-e1fa-4e86',
+            '9be37b52-e1fa-4e86-b65f',
+            '01GRVB3DREB63KNN4G2',
+            'TEST STRING',
+            '00-00-00-00-00-00',
+            '91GRVB3DRES5CQ31PB24MP4CSV',
+        ];
+
+        foreach ($uids as $uid) {
+            $this->assertTrue(PermissionRegistrar::isUid($uid));
+        }
+
+        foreach ($not_uids as $not_uid) {
+            $this->assertFalse(PermissionRegistrar::isUid($not_uid));
+        }
+    }
+}
diff --git a/tests/Role.php b/tests/Role.php
@@ -10,4 +10,24 @@ class Role extends \Spatie\Permission\Models\Role
         'role_test_id',
         'name',
     ];
+
+    protected static function boot()
+    {
+        parent::boot();
+        static::creating(function ($model) {
+            if (empty($model->{$model->getKeyName()})) {
+                $model->{$model->getKeyName()} = \Str::uuid()->toString();
+            }
+        });
+    }
+
+    public function getIncrementing()
+    {
+        return false;
+    }
+
+    public function getKeyType()
+    {
+        return 'string';
+    }
 }
diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -159,13 +159,19 @@ private function prepareMigration()
                 '(\'id\'); // role id',
                 'references(\'id\') // permission id',
                 'references(\'id\') // role id',
+                'bigIncrements',
+                'unsignedBigInteger(PermissionRegistrar::$pivotRole)',
+                'unsignedBigInteger(PermissionRegistrar::$pivotPermission)',
             ],
             [
                 'CreatePermissionCustomTables',
                 '(\'permission_test_id\');',
                 '(\'role_test_id\');',
                 'references(\'permission_test_id\')',
                 'references(\'role_test_id\')',
+                'uuid',
+                'uuid(PermissionRegistrar::$pivotRole)->nullable(false)',
+                'uuid(PermissionRegistrar::$pivotPermission)->nullable(false)',
             ],
             file_get_contents(__DIR__.'/../database/migrations/create_permission_tables.php.stub')
         );