From 41cd0ae7d3f5141c00c6dfceaf14df0fd2f93d38 Mon Sep 17 00:00:00 2001 From: Pim Date: Thu, 19 Oct 2017 23:08:37 +0200 Subject: [PATCH] added several features & fixed a bug (#65) * added features #46, #58 and fixed some bugs * Rename allPermissions method, add getShinobiTag method --- composer.json | 16 +- ...17_170735_create_permission_user_table.php | 31 ++++ phpcs.xml.dist | 10 + phpunit.xml.dist | 23 +++ src/Models/Role.php | 121 ++----------- src/ShinobiServiceProvider.php | 11 +- src/Traits/PermissionTrait.php | 171 ++++++++++++++++++ src/Traits/ShinobiTrait.php | 62 +++++-- tests/Models/User.php | 29 +++ tests/TestCase.php | 86 +++++++++ tests/UserTest.php | 127 +++++++++++++ 11 files changed, 563 insertions(+), 124 deletions(-) create mode 100644 migrations/2017_10_17_170735_create_permission_user_table.php create mode 100644 phpcs.xml.dist create mode 100644 phpunit.xml.dist create mode 100644 src/Traits/PermissionTrait.php create mode 100644 tests/Models/User.php create mode 100644 tests/TestCase.php create mode 100644 tests/UserTest.php diff --git a/composer.json b/composer.json index 3fc973d..f68ba22 100644 --- a/composer.json +++ b/composer.json @@ -34,5 +34,19 @@ } } }, - "minimum-stability": "dev" + "minimum-stability": "dev", + "require-dev": { + "phpunit/phpunit": "^6.4", + "squizlabs/php_codesniffer": "3.*", + "orchestra/testbench": "~3.0" + }, + "autoload-dev": { + "psr-4": { + "Caffeinated\\Shinobi\\Tests\\": "tests/" + } + }, + "scripts": { + "test": "vendor/bin/phpunit", + "cs": "vendor/bin/phpcs src/*" + } } diff --git a/migrations/2017_10_17_170735_create_permission_user_table.php b/migrations/2017_10_17_170735_create_permission_user_table.php new file mode 100644 index 0000000..5d42624 --- /dev/null +++ b/migrations/2017_10_17_170735_create_permission_user_table.php @@ -0,0 +1,31 @@ +increments('id'); + $table->integer('permission_id')->unsigned()->index(); + $table->foreign('permission_id')->references('id')->on('permissions')->onDelete('cascade'); + $table->integer('user_id')->unsigned()->index(); + $table->foreign('user_id')->references('id')->on('users')->onDelete('cascade'); + $table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::drop('permission_user'); + } + +} diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..61344a7 --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,10 @@ + + + + A basic coding standard. + + vendor/* + + + + diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..3366726 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,23 @@ + + + + + ./tests/ + + + + + ./vendor + + + diff --git a/src/Models/Role.php b/src/Models/Role.php index 3de76c8..bef2e85 100644 --- a/src/Models/Role.php +++ b/src/Models/Role.php @@ -4,9 +4,12 @@ use Config; use Illuminate\Database\Eloquent\Model; +use Caffeinated\Shinobi\Traits\PermissionTrait; class Role extends Model { + use PermissionTrait; + /** * The attributes that are fillable via mass assignment. * @@ -22,11 +25,14 @@ class Role extends Model protected $table = 'roles'; /** - * The cache tag used by the model. + * The shinobi cache tag used by the model. * - * @var string + * @return string */ - protected $tag = 'shinobi.roles'; + protected static function getShinobiTag() + { + return 'shinobi.roles'; + } /** * Roles can belong to many users. @@ -39,31 +45,12 @@ public function users() } /** - * Roles can have many permissions. - * - * @return Model - */ - public function permissions() - { - return $this->belongsToMany('\Caffeinated\Shinobi\Models\Permission')->withTimestamps(); - } - - /** - * Get permission slugs assigned to role. + * Get fresh permission slugs assigned to role from database. * * @return array */ - public function getPermissions() + protected function getFreshPermissions() { - $primaryKey = $this[$this->primaryKey]; - $cacheKey = 'caffeinated.shinobi.permissions.'.$primaryKey; - - if (method_exists(app()->make('cache')->getStore(), 'tags')) { - return app()->make('cache')->tags($this->tag)->remember($cacheKey, 60, function () { - return $this->permissions->pluck('slug')->all(); - }); - } - return $this->permissions->pluck('slug')->all(); } @@ -74,9 +61,10 @@ public function getPermissions() */ public function flushPermissionCache() { - if (method_exists(app()->make('cache')->getStore(), 'tags')) { - app()->make('cache')->tags($this->tag)->flush(); - } + parent::flushPermissionCache([ + static::getShinobiTag(), + \Caffeinated\Shinobi\Traits\ShinobiTrait::getShinobiTag() + ]); } /** @@ -96,17 +84,7 @@ public function can($permission) return true; } - $permissions = $this->getPermissions(); - - if (is_array($permission)) { - $permissionCount = count($permission); - $intersection = array_intersect($permissions, $permission); - $intersectionCount = count($intersection); - - return ($permissionCount == $intersectionCount) ? true : false; - } else { - return in_array($permission, $permissions); - } + return $this->hasAllPermissions($permission, $this->getPermissions()); } /** @@ -126,71 +104,6 @@ public function canAtLeast(array $permission = []) return true; } - $permissions = $this->getPermissions(); - - $intersection = array_intersect($permissions, $permission); - $intersectionCount = count($intersection); - - return ($intersectionCount > 0) ? true : false; - } - - /** - * Assigns the given permission to the role. - * - * @param int $permissionId - * - * @return bool - */ - public function assignPermission($permissionId = null) - { - $permissions = $this->permissions; - - if (!$permissions->contains($permissionId)) { - $this->flushPermissionCache(); - - return $this->permissions()->attach($permissionId); - } - - return false; - } - - /** - * Revokes the given permission from the role. - * - * @param int $permissionId - * - * @return bool - */ - public function revokePermission($permissionId = '') - { - $this->flushPermissionCache(); - - return $this->permissions()->detach($permissionId); - } - - /** - * Syncs the given permission(s) with the role. - * - * @param array $permissionIds - * - * @return bool - */ - public function syncPermissions(array $permissionIds = []) - { - $this->flushPermissionCache(); - - return $this->permissions()->sync($permissionIds); - } - - /** - * Revokes all permissions from the role. - * - * @return bool - */ - public function revokeAllPermissions() - { - $this->flushPermissionCache(); - - return $this->permissions()->detach(); + return $this->hasAnyPermission($permission, $this->getPermissions()); } } diff --git a/src/ShinobiServiceProvider.php b/src/ShinobiServiceProvider.php index 76125f4..77fc779 100644 --- a/src/ShinobiServiceProvider.php +++ b/src/ShinobiServiceProvider.php @@ -3,6 +3,7 @@ namespace Caffeinated\Shinobi; use Blade; +use Illuminate\Foundation\Application; use Illuminate\Support\ServiceProvider; class ShinobiServiceProvider extends ServiceProvider @@ -21,9 +22,13 @@ class ShinobiServiceProvider extends ServiceProvider */ public function boot() { - $this->publishes([ - __DIR__.'/../migrations' => $this->app->databasePath().'/migrations', - ], 'migrations'); + if (version_compare(Application::VERSION, '5.3.0', '<')) { + $this->publishes([ + __DIR__.'/../migrations' => $this->app->databasePath().'/migrations', + ], 'migrations'); + } else { + $this->loadMigrationsFrom(__DIR__.'/../migrations'); + } $this->registerBladeDirectives(); } diff --git a/src/Traits/PermissionTrait.php b/src/Traits/PermissionTrait.php new file mode 100644 index 0000000..e4c7981 --- /dev/null +++ b/src/Traits/PermissionTrait.php @@ -0,0 +1,171 @@ +belongsToMany('\Caffeinated\Shinobi\Models\Permission')->withTimestamps(); + } + + /** + * Get fresh permission slugs assigned to the user or role. + * Internal method, should be implemented by Model using this trait + * + * @return array + */ + protected function getFreshPermissions() + { + } + + /** + * Wrapper for caching the permission slugs + * + * @return array + */ + public function getPermissions() + { + $primaryKey = $this[$this->primaryKey]; + $cacheKey = 'caffeinated.'.substr(static::getShinobiTag(), 0, -1).'.permissions.'.$primaryKey; + + if (method_exists(app()->make('cache')->getStore(), 'tags')) { + return app()->make('cache')->tags(static::getShinobiTag())->remember($cacheKey, 60, function () { + return $this->getFreshPermissions(); + }); + } + + return $this->getFreshPermissions(); + } + + /** + * Assigns the given permission to the user or role. + * + * @param int $permissionId + * + * @return bool + */ + public function assignPermission($permissionId = null) + { + $permissions = $this->permissions; + + if (!$permissions->contains($permissionId)) { + $this->flushPermissionCache(); + + return $this->permissions()->attach($permissionId); + } + + return false; + } + + /** + * Revokes the given permission from the user or role. + * + * @param int $permissionId + * + * @return bool + */ + public function revokePermission($permissionId = '') + { + $this->flushPermissionCache(); + + return $this->permissions()->detach($permissionId); + } + + /** + * Syncs the given permission(s) with the user or role. + * + * @param array $permissionIds + * + * @return bool + */ + public function syncPermissions(array $permissionIds = []) + { + $this->flushPermissionCache(); + + return $this->permissions()->sync($permissionIds); + } + + /** + * Revokes all permissions from the user or role. + * + * @return bool + */ + public function revokeAllPermissions() + { + $this->flushPermissionCache(); + + return $this->permissions()->detach(); + } + + /** + * Flush the permission cache repository. + * + * @return void + */ + public function flushPermissionCache(array $tags = null) + { + if (method_exists(app()->make('cache')->getStore(), 'tags')) { + if ($tags === null) { + $tags = [ static::getShinobiTag() ]; + } + + foreach ($tags as $tag) { + app()->make('cache')->tags($tag)->flush(); + } + } + } + + /** + * Check if the or all requested permissions are satisfied + * + * @param mixed $permission + * @param array $permissions + * + * @return bool + */ + protected function hasAllPermissions($permission, array $permissions) + { + if (is_array($permission)) { + $permissionCount = count($permission); + $intersection = array_intersect($permissions, $permission); + $intersectionCount = count($intersection); + + return ($permissionCount == $intersectionCount) ? true : false; + } else { + return in_array($permission, $permissions); + } + } + + /** + * Check if one of the requested permissions are satisfied + * + * @param array $permission + * @param array $permissions + * + * @return bool + */ + protected function hasAnyPermission(array $permission, array $permissions) + { + $intersection = array_intersect($permissions, $permission); + $intersectionCount = count($intersection); + + return ($intersectionCount > 0) ? true : false; + } +} diff --git a/src/Traits/ShinobiTrait.php b/src/Traits/ShinobiTrait.php index 50d5488..cec08cd 100644 --- a/src/Traits/ShinobiTrait.php +++ b/src/Traits/ShinobiTrait.php @@ -4,6 +4,18 @@ trait ShinobiTrait { + use PermissionTrait; + + /** + * The shinobi cache tag used by the user model. + * + * @return string + */ + protected static function getShinobiTag() + { + return 'shinobi.users'; + } + /* |---------------------------------------------------------------------- | Role Trait Methods @@ -62,6 +74,12 @@ public function isRole($slug) */ public function assignRole($roleId = null) { + $this->flushPermissionCache(); + + if (!is_numeric($roleId)) { + $roleId = \Caffeinated\Shinobi\Models\Role::where('slug', $roleId)->pluck('id')->first(); + } + $roles = $this->roles; if (!$roles->contains($roleId)) { @@ -80,6 +98,12 @@ public function assignRole($roleId = null) */ public function revokeRole($roleId = '') { + $this->flushPermissionCache(); + + if (!is_numeric($roleId)) { + $roleId = \Caffeinated\Shinobi\Models\Role::where('slug', $roleId)->pluck('id')->first(); + } + return $this->roles()->detach($roleId); } @@ -92,6 +116,8 @@ public function revokeRole($roleId = '') */ public function syncRoles(array $roleIds) { + $this->flushPermissionCache(); + return $this->roles()->sync($roleIds); } @@ -102,6 +128,8 @@ public function syncRoles(array $roleIds) */ public function revokeAllRoles() { + $this->flushPermissionCache(); + return $this->roles()->detach(); } @@ -113,13 +141,23 @@ public function revokeAllRoles() */ /** - * Get all user role permissions. + * Get permission slugs assigned to user. + * + * @return array + */ + public function getUserPermissions() + { + return $this->permissions->pluck('slug')->all(); + } + + /** + * Get all user role permissions fresh from database * * @return array|null */ - public function getPermissions() + protected function getFreshPermissions() { - $permissions = [[], []]; + $permissions = [[], $this->getUserPermissions()]; foreach ($this->roles as $role) { $permissions[] = $role->getPermissions(); @@ -138,8 +176,6 @@ public function getPermissions() */ public function can($permission, $arguments = []) { - $can = false; - foreach ($this->roles as $role) { if ($role->special === 'no-access') { return false; @@ -148,13 +184,9 @@ public function can($permission, $arguments = []) if ($role->special === 'all-access') { return true; } - - if ($role->can($permission)) { - $can = true; - } } - return $can; + return $this->hasAllPermissions($permission, $this->getPermissions()); } /** @@ -166,8 +198,6 @@ public function can($permission, $arguments = []) */ public function canAtLeast(array $permissions) { - $can = false; - foreach ($this->roles as $role) { if ($role->special === 'no-access') { return false; @@ -178,11 +208,11 @@ public function canAtLeast(array $permissions) } if ($role->canAtLeast($permissions)) { - $can = true; + return true; } } - return $can; + return false; } /* @@ -204,14 +234,14 @@ public function __call($method, $arguments = []) { // Handle isRoleslug() methods if (starts_with($method, 'is') and $method !== 'is') { - $role = substr($method, 2); + $role = kebab_case(substr($method, 2)); return $this->isRole($role); } // Handle canDoSomething() methods if (starts_with($method, 'can') and $method !== 'can') { - $permission = substr($method, 3); + $permission = kebab_case(substr($method, 3)); return $this->can($permission); } diff --git a/tests/Models/User.php b/tests/Models/User.php new file mode 100644 index 0000000..e4103f1 --- /dev/null +++ b/tests/Models/User.php @@ -0,0 +1,29 @@ +loadLaravelMigrations([]); + $this->artisan('migrate'); + + $this->seedDatabase(); + } + + protected function seedDatabase() + { + $user = [ + 'admin' => User::create(['name' => 'Admin', 'password' => 'password', 'email' => 'admin@example.com']), + 'user' => User::create(['name' => 'User', 'password' => 'password', 'email' => 'user@example.com']), + 'super' => User::create(['name' => 'Super User', 'password' => 'password', 'email' => 'super_user@example.com']), + 'disabled' => User::create([ 'name' => 'Disabled User', 'password' => 'password', 'email' => 'disabled_user@example.com']), + ]; + + $roles = [ + 'admin' => Role::create(['name' => 'Admin', 'slug' => 'admin']), + 'user' => Role::create(['name' => 'User', 'slug' => 'user']), + 'allaccess' => Role::create(['name' => 'Super Access', 'slug' => 'super', 'special' => 'all-access']), + 'noaccess' => Role::create(['name' => 'No access', 'slug' => 'no-access', 'special' => 'no-access']), + ]; + + $permission = [ + 'access' => Permission::create(['name' => 'Admin Access', 'slug' => 'access.admin']), + 'view' => Permission::create(['name' => 'View Users', 'slug' => 'view.users']), + 'edit' => Permission::create(['name' => 'Edit Users', 'slug' => 'edit.users']), + 'email' => Permission::create(['name' => 'Email Users', 'slug' => 'email.users']), + ]; + + $roles['admin']->permissions()->attach($permission['access']); + $roles['admin']->permissions()->attach($permission['edit']); + $roles['user']->permissions()->attach($permission['view']); + $roles['noaccess']->permissions()->attach($permission['view']); + + $user['admin']->roles()->attach(Role::all()); + $user['user']->roles()->attach($roles['user']); + $user['user']->permissions()->attach($permission['email']); + $user['super']->roles()->attach($roles['allaccess']); + $user['disabled']->roles()->attach($roles['noaccess']); + } + + /** + * Set up the environment. + * + * @param \Illuminate\Foundation\Application $app + */ + protected function getEnvironmentSetUp($app) + { + $app['config']->set('app.debug', true); + $app['config']->set('database.default', 'sqlite'); + $app['config']->set('database.connections.sqlite', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + } + + protected function getPackageProviders($app) + { + return [ + \Caffeinated\Shinobi\ShinobiServiceProvider::class, + ]; + } + + protected function getPackageAliases($app) + { + return [ + 'Shinobi' => \Caffeinated\Shinobi\Facades\Shinobi::class, + ]; + } +} \ No newline at end of file diff --git a/tests/UserTest.php b/tests/UserTest.php new file mode 100644 index 0000000..86c293c --- /dev/null +++ b/tests/UserTest.php @@ -0,0 +1,127 @@ +first(); + + $this->assertTrue($user->isRole('admin')); + $this->assertTrue($user->isAdmin()); + } + + public function test_can_access_one_permission() + { + $user = User::where('name', 'Admin')->first(); + + $this->assertTrue($user->can('access.admin')); + } + + public function test_can_access_all_permission_from_same_role() + { + $user = User::where('name', 'Admin')->first(); + + $this->assertTrue($user->can(['access.admin', 'edit.users'])); + } + + public function test_can_access_all_permission_from_different_roles() + { + $user = User::where('name', 'Admin')->first(); + + $this->assertTrue($user->can(['access.admin', 'edit.users', 'view.users'])); + } + + public function test_can_access_user_permission() + { + $user = User::where('name', 'User')->first(); + $this->assertTrue($user->can('email.users')); + } + + public function test_can_access_all_permission_from_user_and_role() + { + $user = User::where('name', 'User')->first(); + + $this->assertTrue($user->can(['view.users', 'email.users'])); + } + + public function test_cant_access_all_permission() + { + $user = User::where('name', 'User')->first(); + + $this->assertFalse($user->can(['access.admin'])); + } + + public function test_can_access_atleast_one_permission() + { + $user = User::where('name', 'User')->first(); + + $this->assertTrue($user->canAtLeast(['edit.users', 'view.users'])); + } + + public function test_cant_access_atleast_one_permission() + { + $user = User::where('name', 'User')->first(); + + $this->assertFalse($user->canAtLeast(['access.admin', 'edit.users'])); + } + + public function test_can_revoke_and_assign_role() + { + $user = User::where('name', 'User')->first(); + + $roleId = $user->roles()->first()->id; + $permission = $user->roles()->first()->permissions()->first()->slug; + + $user->revokeRole($roleId); + $this->assertFalse($user->can($permission), 'User::revokeRoke failed'); + + $user->assignRole($roleId); + // not sure why, but $user->fresh() doesnt work here + $user = User::where('name', 'User')->first(); + $this->assertTrue($user->can($permission), 'User::assignRole failed'); + + $user->revokeAllRoles(); + $user = User::where('name', 'User')->first(); + $this->assertFalse($user->can($permission), 'User::revokeAllRoles failed'); + + $user->syncRoles([$roleId]); + $user = User::where('name', 'User')->first(); + $this->assertTrue($user->can($permission), 'User::syncRoles failed'); + } + + public function test_can_revoke_and_assign_role_by_slug() + { + $user = User::where('name', 'User')->first(); + + $roleSlug = $user->roles()->first()->slug; + $permission = $user->roles()->first()->permissions()->first()->slug; + + $user->revokeRole($roleSlug); + $this->assertFalse($user->can($permission), 'User::revokeRoke failed'); + + $user->assignRole($roleSlug); + $user = User::where('name', 'User')->first(); + $this->assertTrue($user->can($permission), 'User::assignRole failed'); + } + + public function test_special_all_access() + { + $user = User::where('name', 'Super User')->first(); + + $this->assertTrue($user->can('doesnt-even-exist-and-super-still-has-access')); + } + + public function test_special_no_access() + { + $user = User::where('name', 'Disabled User')->first(); + + $this->assertFalse($user->can('view.users')); + } +}