From cc8610e104a278144c475db9bee8037a33c298c5 Mon Sep 17 00:00:00 2001 From: erikn69 Date: Mon, 6 Feb 2023 12:49:29 -0500 Subject: [PATCH 1/2] Full uuid support --- src/Models/Permission.php | 4 +-- src/Models/Role.php | 4 +-- src/Traits/HasPermissions.php | 10 ++++---- src/Traits/HasRoles.php | 8 +++--- tests/HasPermissionsWithCustomModelsTest.php | 27 ++++++++++++++++++++ tests/Permission.php | 20 +++++++++++++++ tests/Role.php | 20 +++++++++++++++ tests/TestCase.php | 6 +++++ 8 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/Models/Permission.php b/src/Models/Permission.php index 9bbf367e4..d0ea85122 100644 --- 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 index 104916dd2..aa1639e1d 100644 --- a/src/Models/Role.php +++ b/src/Models/Role.php @@ -108,11 +108,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/Traits/HasPermissions.php b/src/Traits/HasPermissions.php index 0d85b998f..217ea526e 100644 --- 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) && ! \Str::isUuid($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) && ! \Str::isUuid($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) || \Str::isUuid($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) || \Str::isUuid($permissions)) { return $permissionClass->findById($permissions, $this->getDefaultGuardName()); } diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php index 8ab3732ad..35d008aa4 100644 --- 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) || \Str::isUuid($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) && ! \Str::isUuid($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) || \Str::isUuid($role)) { return $roleClass->findById($role, $this->getDefaultGuardName()); } diff --git a/tests/HasPermissionsWithCustomModelsTest.php b/tests/HasPermissionsWithCustomModelsTest.php index 6b9fa34ca..b491c8e05 100644 --- 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' => 'user1@test.com']); + $user2 = User::create(['email' => 'user2@test.com']); + $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 index 24d4938b8..9d8f2cd91 100644 --- 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/Role.php b/tests/Role.php index ee2862f82..79f8cdbfd 100644 --- 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 index 7fd2ebb57..9481feb84 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -159,6 +159,9 @@ private function prepareMigration() '(\'id\'); // role id', 'references(\'id\') // permission id', 'references(\'id\') // role id', + 'bigIncrements', + 'unsignedBigInteger(PermissionRegistrar::$pivotRole)', + 'unsignedBigInteger(PermissionRegistrar::$pivotPermission)', ], [ 'CreatePermissionCustomTables', @@ -166,6 +169,9 @@ private function prepareMigration() '(\'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') ); From b2c858bc3cce4a7dfa13fd262a57ce258ceb4193 Mon Sep 17 00:00:00 2001 From: erikn69 Date: Thu, 9 Feb 2023 10:12:19 -0500 Subject: [PATCH 2/2] Support ULIDs --- src/PermissionRegistrar.php | 21 +++++++++++++ src/Traits/HasPermissions.php | 8 ++--- src/Traits/HasRoles.php | 6 ++-- tests/PermissionRegistarTest.php | 51 ++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 tests/PermissionRegistarTest.php diff --git a/src/PermissionRegistrar.php b/src/PermissionRegistrar.php index 38f71e4bf..d771b2205 100644 --- 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 index 217ea526e..a29717d71 100644 --- 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) && ! \Str::isUuid($permission) ? 'findByName' : 'findById'; + $method = is_string($permission) && ! PermissionRegistrar::isUid($permission) ? 'findByName' : 'findById'; return $this->getPermissionClass()->{$method}($permission, $this->getDefaultGuardName()); }, Arr::wrap($permissions)); @@ -152,7 +152,7 @@ public function filterPermission($permission, $guardName = null) { $permissionClass = $this->getPermissionClass(); - if (is_string($permission) && ! \Str::isUuid($permission)) { + if (is_string($permission) && ! PermissionRegistrar::isUid($permission)) { $permission = $permissionClass->findByName( $permission, $guardName ?? $this->getDefaultGuardName() @@ -204,7 +204,7 @@ protected function hasWildcardPermission($permission, $guardName = null): bool { $guardName = $guardName ?? $this->getDefaultGuardName(); - if (is_int($permission) || \Str::isUuid($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) || \Str::isUuid($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 index 35d008aa4..42cfb4ac0 100644 --- 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) || \Str::isUuid($role) ? 'findById' : 'findByName'; + $method = is_numeric($role) || PermissionRegistrar::isUid($role) ? 'findById' : 'findByName'; return $this->getRoleClass()->{$method}($role, $guard ?: $this->getDefaultGuardName()); }, Arr::wrap($roles)); @@ -195,7 +195,7 @@ public function hasRole($roles, string $guard = null): bool $roles = $this->convertPipeToArray($roles); } - if (is_string($roles) && ! \Str::isUuid($roles)) { + if (is_string($roles) && ! PermissionRegistrar::isUid($roles)) { return $guard ? $this->roles->where('guard_name', $guard)->contains('name', $roles) : $this->roles->contains('name', $roles); @@ -325,7 +325,7 @@ protected function getStoredRole($role): Role { $roleClass = $this->getRoleClass(); - if (is_numeric($role) || \Str::isUuid($role)) { + if (is_numeric($role) || PermissionRegistrar::isUid($role)) { return $roleClass->findById($role, $this->getDefaultGuardName()); } diff --git a/tests/PermissionRegistarTest.php b/tests/PermissionRegistarTest.php new file mode 100644 index 000000000..b864b3e90 --- /dev/null +++ b/tests/PermissionRegistarTest.php @@ -0,0 +1,51 @@ +assertTrue(PermissionRegistrar::isUid($uid)); + } + + foreach ($not_uids as $not_uid) { + $this->assertFalse(PermissionRegistrar::isUid($not_uid)); + } + } +}