From 9613159b0c1629a9562a6bdb6ef21b4c70817e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20P=C3=A9tel?= Date: Tue, 15 Nov 2016 19:10:42 +0100 Subject: [PATCH 1/2] Resolve permissions inheritance based on roles --- src/Kodeine/Acl/Traits/HasPermission.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Kodeine/Acl/Traits/HasPermission.php b/src/Kodeine/Acl/Traits/HasPermission.php index c50664f..69bfe4c 100644 --- a/src/Kodeine/Acl/Traits/HasPermission.php +++ b/src/Kodeine/Acl/Traits/HasPermission.php @@ -50,9 +50,7 @@ function () { foreach ($role->getPermissions() as $slug => $array) { if ( array_key_exists($slug, $permissions) ) { foreach ($array as $clearance => $value) { - if( !array_key_exists( $clearance, $permissions[$slug] ) ) { - ! $value ?: $permissions[$slug][$clearance] = true; - } + ! $value ?: $permissions[$slug][$clearance] = true; } } else { $permissions = array_merge($permissions, [$slug => $array]); From 4371b05cb8cbe60ae676728174281111033fe480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20P=C3=A9tel?= Date: Wed, 16 Nov 2016 21:10:27 +0100 Subject: [PATCH 2/2] Resolve permissions issues with roles : user permissions correctly override role ones + cache delete on assign/revoke role/permissions --- src/Kodeine/Acl/Traits/HasPermission.php | 61 +++++++++++++++++------- src/Kodeine/Acl/Traits/HasRole.php | 21 +++++++- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/Kodeine/Acl/Traits/HasPermission.php b/src/Kodeine/Acl/Traits/HasPermission.php index 69bfe4c..f1307e0 100644 --- a/src/Kodeine/Acl/Traits/HasPermission.php +++ b/src/Kodeine/Acl/Traits/HasPermission.php @@ -33,30 +33,41 @@ public function permissions() */ public function getPermissions() { - // user permissions overridden from role. + $permissions = \Cache::remember( 'acl.getPermissionsById_'.$this->id, config('acl.cacheMinutes'), function () { - return $this->getPermissionsInherited(); - } - ); + $permissions = []; + // permissions based on role. + // more permissive permission wins + // if user has multiple roles we keep + // true values. + foreach ($this->roles as $role) { + foreach ($role->getPermissions() as $slug => $array) { + if ( array_key_exists($slug, $permissions) ) { + foreach ($array as $clearance => $value) { + ! $value ?: $permissions[$slug][$clearance] = true; + } + } else { + $permissions = array_merge($permissions, [$slug => $array]); + } + } + } - // permissions based on role. - // more permissive permission wins - // if user has multiple roles we keep - // true values. - foreach ($this->roles as $role) { - foreach ($role->getPermissions() as $slug => $array) { - if ( array_key_exists($slug, $permissions) ) { - foreach ($array as $clearance => $value) { - ! $value ?: $permissions[$slug][$clearance] = true; + // user permissions that override roles ones. + foreach ($this->getPermissionsInherited() as $slug => $array) { + if ( array_key_exists($slug, $permissions) ) { + foreach ($array as $clearance => $value) { + $permissions[$slug][$clearance] = $value; + } + } else { + $permissions = array_merge($permissions, [$slug => $array]); } - } else { - $permissions = array_merge($permissions, [$slug => $array]); } + return $permissions; } - } + ); return $permissions; } @@ -96,6 +107,8 @@ function () { */ public function assignPermission($permission) { + $this->deletePermissionCache(); + return $this->mapArray($permission, function ($permission) { $permissionId = $this->parsePermissionId($permission); @@ -118,6 +131,8 @@ public function assignPermission($permission) */ public function revokePermission($permission) { + $this->deletePermissionCache(); + return $this->mapArray($permission, function ($permission) { $permissionId = $this->parsePermissionId($permission); @@ -141,7 +156,7 @@ public function syncPermissions($permissions) return $sync; }); - + $this->deletePermissionCache(); return $this->permissions()->sync($sync); } @@ -152,6 +167,7 @@ public function syncPermissions($permissions) */ public function revokeAllPermissions() { + $this->deletePermissionCache(); return $this->permissions()->detach(); } @@ -163,6 +179,17 @@ public function revokeAllPermissions() */ + /** + * Delete cache for this traits. + * + * @return null + */ + protected function deletePermissionCache() + { + \Cache::forget('acl.getPermissionsById_'.$this->id); + \Cache::forget('acl.getMergeById_'.$this->id); + } + /** * Parses permission id from object or array. * diff --git a/src/Kodeine/Acl/Traits/HasRole.php b/src/Kodeine/Acl/Traits/HasRole.php index 85f25e3..08778ef 100644 --- a/src/Kodeine/Acl/Traits/HasRole.php +++ b/src/Kodeine/Acl/Traits/HasRole.php @@ -58,7 +58,7 @@ public function scopeRole($query, $role) if (is_null($role)) { return $query; } - + return $query->whereHas('roles', function ($query) use ($role) { $query->where(is_numeric($role) ? 'id' : 'slug', $role); }); @@ -103,11 +103,14 @@ public function hasRole($slug, $operator = null) */ public function assignRole($role) { + $this->deleteRoleCache(); + return $this->mapArray($role, function ($role) { $roleId = $this->parseRoleId($role); if ( ! $this->roles->keyBy('id')->has($roleId) ) { + $this->roles()->attach($roleId); return $role; @@ -125,6 +128,8 @@ public function assignRole($role) */ public function revokeRole($role) { + $this->deleteRoleCache(); + return $this->mapArray($role, function ($role) { $roleId = $this->parseRoleId($role); @@ -149,6 +154,8 @@ public function syncRoles($roles) return $sync; }); + $this->deleteRoleCache(); + return $this->roles()->sync($sync); } @@ -159,6 +166,8 @@ public function syncRoles($roles) */ public function revokeAllRoles() { + $this->deleteRoleCache(); + return $this->roles()->detach(); } @@ -169,6 +178,16 @@ public function revokeAllRoles() | */ + /** + * Delete cache for this traits. + * + * @return null + */ + protected function deleteRoleCache() + { + \Cache::forget('acl.getRolesById_'.$this->id); + } + /** * @param $slug * @param $roles