From: Dan Brown <redacted>
Date: Fri, 13 Jan 2023 22:13:31 +0000 (+0000)
Subject: Updated additional relation queries to apply permissions correctly
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/a825f27930bd40fd7fbd010a05fea2637406e2d2

Updated additional relation queries to apply permissions correctly
---

diff --git a/app/Actions/ActivityQueries.php b/app/Actions/ActivityQueries.php
index 852913e63..88e6aa7e7 100644
--- a/app/Actions/ActivityQueries.php
+++ b/app/Actions/ActivityQueries.php
@@ -15,6 +15,8 @@ class ActivityQueries
 {
     protected PermissionApplicator $permissions;
 
+    protected array $fieldsForLists = ['id', 'type', 'detail', 'activities.entity_type', 'activities.entity_id', 'user_id', 'created_at'];
+
     public function __construct(PermissionApplicator $permissions)
     {
         $this->permissions = $permissions;
@@ -25,7 +27,7 @@ class ActivityQueries
      */
     public function latest(int $count = 20, int $page = 0): array
     {
-        $query = Activity::query()->select(['id', 'type', 'detail', 'activities.entity_type', 'activities.entity_id', 'user_id', 'created_at']);
+        $query = Activity::query()->select($this->fieldsForLists);
         $activityList = $this->permissions
             ->restrictEntityRelationQuery($query, 'activities', 'entity_id', 'entity_type')
             ->orderBy('created_at', 'desc')
@@ -79,8 +81,9 @@ class ActivityQueries
      */
     public function userActivity(User $user, int $count = 20, int $page = 0): array
     {
+        $query = Activity::query()->select($this->fieldsForLists);
         $activityList = $this->permissions
-            ->restrictEntityRelationQuery(Activity::query(), 'activities', 'entity_id', 'entity_type')
+            ->restrictEntityRelationQuery($query, 'activities', 'entity_id', 'entity_type')
             ->orderBy('created_at', 'desc')
             ->where('user_id', '=', $user->id)
             ->skip($count * $page)
diff --git a/app/Actions/TagRepo.php b/app/Actions/TagRepo.php
index cece30de0..176dbaa61 100644
--- a/app/Actions/TagRepo.php
+++ b/app/Actions/TagRepo.php
@@ -29,15 +29,16 @@ class TagRepo
             $sort = 'value';
         }
 
+        $entityTypeCol = DB::getTablePrefix() . 'tags.entity_type';
         $query = Tag::query()
             ->select([
                 'name',
                 ($searchTerm || $nameFilter) ? 'value' : DB::raw('COUNT(distinct value) as `values`'),
                 DB::raw('COUNT(id) as usages'),
-                DB::raw('SUM(IF(entity_type = \'page\', 1, 0)) as page_count'),
-                DB::raw('SUM(IF(entity_type = \'chapter\', 1, 0)) as chapter_count'),
-                DB::raw('SUM(IF(entity_type = \'book\', 1, 0)) as book_count'),
-                DB::raw('SUM(IF(entity_type = \'bookshelf\', 1, 0)) as shelf_count'),
+                DB::raw("SUM(IF({$entityTypeCol} = 'page', 1, 0)) as page_count"),
+                DB::raw("SUM(IF({$entityTypeCol} = 'chapter', 1, 0)) as chapter_count"),
+                DB::raw("SUM(IF({$entityTypeCol} = 'book', 1, 0)) as book_count"),
+                DB::raw("SUM(IF({$entityTypeCol} = 'bookshelf', 1, 0)) as shelf_count"),
             ])
             ->orderBy($sort, $listOptions->getOrder());
 
diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 64850fd8a..c3ae2dce1 100644
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -319,10 +319,9 @@ class PermissionApplicator
      */
     public function restrictEntityRelationQuery($query, string $tableName, string $entityIdColumn, string $entityTypeColumn)
     {
+        // TODO - Apply admin allow all as per above query thing
         $this->applyPermissionsToQuery($query, $tableName, '', $entityIdColumn, $entityTypeColumn);
         // TODO - Test page draft access (Might allow drafts which should not be seen)
-        // TODO - Test each use of this to check column/relation fetching.
-        //    Original queries might need selects applied to limit field exposure and to get right original table columns.
 
         return $query;
     }
@@ -335,65 +334,12 @@ class PermissionApplicator
      */
     public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder
     {
-        $fullPageIdColumn = $tableName . '.' . $pageIdColumn;
         $morphClass = (new Page())->getMorphClass();
 
-        // TODO
+        $this->applyPermissionsToQuery($query, $tableName, $morphClass, $pageIdColumn, '');
+        // TODO - Admin workaround as above
+        // TODO - Draft display
         return $query;
-
-        $existsQuery = function ($permissionQuery) use ($fullPageIdColumn, $morphClass) {
-            /** @var Builder $permissionQuery */
-            $permissionQuery->select('joint_permissions.role_id')->from('joint_permissions')
-                ->whereColumn('joint_permissions.entity_id', '=', $fullPageIdColumn)
-                ->where('joint_permissions.entity_type', '=', $morphClass)
-                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
-                ->where(function (QueryBuilder $query) {
-                    $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
-                });
-        };
-
-        $userExistsQuery = function ($hasPermission) use ($fullPageIdColumn, $morphClass) {
-            return function ($permissionQuery) use ($fullPageIdColumn, $morphClass) {
-                /** @var Builder $permissionQuery */
-                $permissionQuery->select('joint_user_permissions.user_id')->from('joint_user_permissions')
-                    ->whereColumn('joint_user_permissions.entity_id', '=', $fullPageIdColumn)
-                    ->where('joint_user_permissions.entity_type', '=', $morphClass)
-                    ->where('joint_user_permissions.user_id', $this->currentUser()->id)
-                    ->where('has_permission', '=', true);
-            };
-        };
-
-        $q = $query->where(function ($query) use ($existsQuery, $userExistsQuery, $fullPageIdColumn) {
-            $query->whereExists($existsQuery)
-                ->orWhereExists($userExistsQuery(true))
-                ->orWhere($fullPageIdColumn, '=', 0);
-        })->whereNotExists($userExistsQuery(false));
-
-        // Prevent visibility of non-owned draft pages
-        $q->whereExists(function (QueryBuilder $query) use ($fullPageIdColumn) {
-            $query->select('id')->from('pages')
-                ->whereColumn('pages.id', '=', $fullPageIdColumn)
-                ->where(function (QueryBuilder $query) {
-                    $query->where('pages.draft', '=', false)
-                        ->orWhere('pages.owned_by', '=', $this->currentUser()->id);
-                });
-        });
-
-        return $q;
-    }
-
-    /**
-     * Add the query for checking the given user id has permission
-     * within the join_permissions table.
-     *
-     * @param QueryBuilder|Builder $query
-     */
-    protected function addJointHasPermissionCheck($query, int $userIdToCheck)
-    {
-        $query->where('joint_permissions.has_permission', '=', true)->orWhere(function ($query) use ($userIdToCheck) {
-            $query->where('joint_permissions.has_permission_own', '=', true)
-                ->where('joint_permissions.owned_by', '=', $userIdToCheck);
-        });
     }
 
     /**