Continued removal of joint permission non-view queries

Cleaned up PermissionApplicator to remove old cache system which was
hardly ever actuall caching anything since it was reset after each
public method run.

Changed the scope of 'userCanOnAny' to just check entity permissions,
and added protections of action scope creep, in case a role permission
action was passed by mistake.

app/Auth/Permissions/PermissionApplicator.php

@@ -11,37 +11,10 @@ use BookStack\Traits\HasCreatorAndUpdater;
 use BookStack\Traits\HasOwner;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Query\Builder as QueryBuilder;
+use InvalidArgumentException;
 
 class PermissionApplicator
 {
-    /**
-     * @var ?array<int>
-     */
-    protected $userRoles = null;
-
-    /**
-     * @var ?User
-     */
-    protected $currentUserModel = null;
-
-    /**
-     * Get the roles for the current logged in user.
-     */
-    protected function getCurrentUserRoles(): array
-    {
-        if (!is_null($this->userRoles)) {
-            return $this->userRoles;
-        }
-
-        if (auth()->guest()) {
-            $this->userRoles = [Role::getSystemRole('public')->id];
-        } else {
-            $this->userRoles = $this->currentUser()->roles->pluck('id')->values()->all();
-        }
-
-        return $this->userRoles;
-    }
-
     /**
      * Checks if an entity has a restriction set upon it.
      *
@@ -74,7 +47,6 @@ class PermissionApplicator
 
         // TODO - Use a non-query based check
         $hasAccess = $this->entityRestrictionQuery($baseQuery, $action)->count() > 0;
-        $this->clean();
 
         return $hasAccess;
     }
@@ -83,25 +55,23 @@ class PermissionApplicator
      * Checks if a user has the given permission for any items in the system.
      * Can be passed an entity instance to filter on a specific type.
      */
-    public function checkUserHasPermissionOnAnything(string $permission, ?string $entityClass = null): bool
+    public function checkUserHasEntityPermissionOnAny(string $action, string $entityClass = ''): bool
     {
-        $userRoleIds = $this->currentUser()->roles()->select('id')->pluck('id')->toArray();
+        if (strpos($action, '-') !== false) {
-        $userId = $this->currentUser()->id;
+            throw new InvalidArgumentException("Action should be a simple entity permission action, not a role permission");
+        }
 
-        $permissionQuery = JointPermission::query()
+        $permissionQuery = EntityPermission::query()
-            ->where('action', '=', $permission)
+            ->where('action', '=', $action)
-            ->whereIn('role_id', $userRoleIds)
+            ->whereIn('role_id', $this->getCurrentUserRoleIds());
-            ->where(function (Builder $query) use ($userId) {
-                $this->addJointHasPermissionCheck($query, $userId);
-            });
 
-        if (!is_null($entityClass)) {
+        if (!empty($entityClass)) {
-            $entityInstance = app($entityClass);
+            /** @var Entity $entityInstance */
-            $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass());
+            $entityInstance = app()->make($entityClass);
+            $permissionQuery = $permissionQuery->where('restrictable_type', '=', $entityInstance->getMorphClass());
         }
 
         $hasPermission = $permissionQuery->count() > 0;
-        $this->clean();
 
         return $hasPermission;
     }
@@ -114,7 +84,8 @@ class PermissionApplicator
     {
         $q = $query->where(function ($parentQuery) use ($action) {
             $parentQuery->whereHas('jointPermissions', function ($permissionQuery) use ($action) {
-                $permissionQuery->whereIn('role_id', $this->getCurrentUserRoles())
+                $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds())
+                    // TODO - Delete line once only views
                     ->where('action', '=', $action)
                     ->where(function (Builder $query) {
                         $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
@@ -122,23 +93,20 @@ class PermissionApplicator
             });
         });
 
-        $this->clean();
-
         return $q;
     }
 
     /**
      * Limited the given entity query so that the query will only
-     * return items that the user has permission for the given ability.
+     * return items that the user has view permission for.
      */
-    public function restrictEntityQuery(Builder $query, string $ability = 'view'): Builder
+    public function restrictEntityQuery(Builder $query): Builder
     {
-        $this->clean();
+        return $query->where(function (Builder $parentQuery) {
-
+            $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) {
-        return $query->where(function (Builder $parentQuery) use ($ability) {
+                $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds())
-            $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) use ($ability) {
+                    // TODO - Delete line once only views
-                $permissionQuery->whereIn('role_id', $this->getCurrentUserRoles())
+                    ->where('action', '=', 'view')
-                    ->where('action', '=', $ability)
                     ->where(function (Builder $query) {
                         $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
                     });
@@ -181,18 +149,18 @@ class PermissionApplicator
      *
      * @param Builder|QueryBuilder $query
      */
-    public function filterRestrictedEntityRelations($query, string $tableName, string $entityIdColumn, string $entityTypeColumn, string $action = 'view')
+    public function filterRestrictedEntityRelations($query, string $tableName, string $entityIdColumn, string $entityTypeColumn)
     {
         $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn];
         $pageMorphClass = (new Page())->getMorphClass();
 
-        $q = $query->whereExists(function ($permissionQuery) use (&$tableDetails, $action) {
+        $q = $query->whereExists(function ($permissionQuery) use (&$tableDetails) {
             /** @var Builder $permissionQuery */
             $permissionQuery->select(['role_id'])->from('joint_permissions')
                 ->whereColumn('joint_permissions.entity_id', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn'])
                 ->whereColumn('joint_permissions.entity_type', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn'])
-                ->where('joint_permissions.action', '=', $action)
+                ->where('joint_permissions.action', '=', 'view')
-                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoles())
+                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
                 ->where(function (QueryBuilder $query) {
                     $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
                 });
@@ -207,8 +175,6 @@ class PermissionApplicator
                 });
         });
 
-        $this->clean();
-
         return $q;
     }
 
@@ -228,7 +194,7 @@ class PermissionApplicator
                 ->whereColumn('joint_permissions.entity_id', '=', $fullEntityIdColumn)
                 ->where('joint_permissions.entity_type', '=', $morphClass)
                 ->where('joint_permissions.action', '=', 'view')
-                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoles())
+                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
                 ->where(function (QueryBuilder $query) {
                     $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
                 });
@@ -251,8 +217,6 @@ class PermissionApplicator
             });
         }
 
-        $this->clean();
-
         return $q;
     }
 
@@ -273,21 +237,22 @@ class PermissionApplicator
     /**
      * Get the current user.
      */
-    private function currentUser(): User
+    protected function currentUser(): User
     {
-        if (is_null($this->currentUserModel)) {
+        return user();
-            $this->currentUserModel = user();
-        }
-
-        return $this->currentUserModel;
     }
 
     /**
-     * Clean the cached user elements.
+     * Get the roles for the current logged-in user.
+     *
+     * @return int[]
      */
-    private function clean(): void
+    protected function getCurrentUserRoleIds(): array
     {
-        $this->currentUserModel = null;
+        if (auth()->guest()) {
-        $this->userRoles = null;
+            return [Role::getSystemRole('public')->id];
+        }
+
+        return $this->currentUser()->roles->pluck('id')->values()->all();
     }
 }

app/Entities/Models/Entity.php

@@ -44,7 +44,6 @@ use Illuminate\Database\Eloquent\SoftDeletes;
  * @property Collection $tags
  *
  * @method static Entity|Builder visible()
- * @method static Entity|Builder hasPermission(string $permission)
  * @method static Builder withLastView()
  * @method static Builder withViewCount()
  */
@@ -69,15 +68,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
      */
     public function scopeVisible(Builder $query): Builder
     {
-        return $this->scopeHasPermission($query, 'view');
+        return app()->make(PermissionApplicator::class)->restrictEntityQuery($query);
-    }
-
-    /**
-     * Scope the query to those entities that the current user has the given permission for.
-     */
-    public function scopeHasPermission(Builder $query, string $permission)
-    {
-        return app()->make(PermissionApplicator::class)->restrictEntityQuery($query, $permission);
     }
 
     /**

app/Entities/Queries/Popular.php

@@ -10,7 +10,7 @@ class Popular extends EntityQuery
     public function run(int $count, int $page, array $filterModels = null)
     {
         $query = $this->permissionService()
-            ->filterRestrictedEntityRelations(View::query(), 'views', 'viewable_id', 'viewable_type', 'view')
+            ->filterRestrictedEntityRelations(View::query(), 'views', 'viewable_id', 'viewable_type')
             ->select('*', 'viewable_id', 'viewable_type', DB::raw('SUM(views) as view_count'))
             ->groupBy('viewable_id', 'viewable_type')
             ->orderBy('view_count', 'desc');

app/Entities/Queries/RecentlyViewed.php

@@ -18,8 +18,7 @@ class RecentlyViewed extends EntityQuery
             View::query(),
             'views',
             'viewable_id',
-            'viewable_type',
+            'viewable_type'
-            'view'
         )
             ->orderBy('views.updated_at', 'desc')
             ->where('user_id', '=', user()->id);

app/Entities/Queries/TopFavourites.php

@@ -15,7 +15,7 @@ class TopFavourites extends EntityQuery
         }
 
         $query = $this->permissionService()
-            ->filterRestrictedEntityRelations(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type', 'view')
+            ->filterRestrictedEntityRelations(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type')
             ->select('favourites.*')
             ->leftJoin('views', function (JoinClause $join) {
                 $join->on('favourites.favouritable_id', '=', 'views.viewable_id');

app/Http/Controllers/BookshelfController.php

@@ -68,7 +68,7 @@ class BookshelfController extends Controller
     public function create()
     {
         $this->checkPermission('bookshelf-create-all');
-        $books = Book::hasPermission('update')->get();
+        $books = Book::visible()->get();
         $this->setPageTitle(trans('entities.shelves_create'));
 
         return view('shelves.create', ['books' => $books]);
@@ -139,7 +139,7 @@ class BookshelfController extends Controller
         $this->checkOwnablePermission('bookshelf-update', $shelf);
 
         $shelfBookIds = $shelf->books()->get(['id'])->pluck('id');
-        $books = Book::hasPermission('update')->whereNotIn('id', $shelfBookIds)->get();
+        $books = Book::visible()->whereNotIn('id', $shelfBookIds)->get();
 
         $this->setPageTitle(trans('entities.shelves_edit_named', ['name' => $shelf->getShortName()]));
 

app/helpers.php

@@ -71,14 +71,14 @@ function userCan(string $permission, Model $ownable = null): bool
 }
 
 /**
- * Check if the current user has the given permission
+ * Check if the current user can perform the given action on any items in the system.
- * on any item in the system.
+ * Can be provided the class name of an entity to filter ability to that specific entity type.
  */
-function userCanOnAny(string $permission, string $entityClass = null): bool
+function userCanOnAny(string $action, string $entityClass = ''): bool
 {
     $permissions = app(PermissionApplicator::class);
 
-    return $permissions->checkUserHasPermissionOnAnything($permission, $entityClass);
+    return $permissions->checkUserHasEntityPermissionOnAny($action, $entityClass);
 }
 
 /**

resources/views/chapters/show.blade.php

@@ -120,7 +120,7 @@
                     <span>{{ trans('common.edit') }}</span>
                 </a>
             @endif
-            @if(userCanOnAny('chapter-create'))
+            @if(userCanOnAny('create', \BookStack\Entities\Models\Book::class) || userCan('chapter-create-all') || userCan('chapter-create-own'))
                 <a href="{{ $chapter->getUrl('/copy') }}" class="icon-list-item">
                     <span>@icon('copy')</span>
                     <span>{{ trans('common.copy') }}</span>

resources/views/pages/show.blade.php

@@ -148,7 +148,7 @@
                     <span>{{ trans('common.edit') }}</span>
                 </a>
             @endif
-            @if(userCanOnAny('page-create'))
+            @if(userCanOnAny('create', \BookStack\Entities\Models\Book::class) || userCanOnAny('create', \BookStack\Entities\Models\Chapter::class) || userCan('page-create-all') || userCan('page-create-own'))
                 <a href="{{ $page->getUrl('/copy') }}" class="icon-list-item">
                     <span>@icon('copy')</span>
                     <span>{{ trans('common.copy') }}</span>