Split out 'restrictEntityQuery' function components

Also fixed search query issue with abiguous column

diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 9a2549c0d1fa356887a26388e682104be9b90193..00c957c3bb5777d3b64689b76a598dfc54276dd4 100644 (file)
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -12,6 +12,7 @@ use BookStack\Traits\HasCreatorAndUpdater;
 use BookStack\Traits\HasOwner;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Query\Builder as QueryBuilder;
+use Illuminate\Database\Query\JoinClause;
 use InvalidArgumentException;
 
 class PermissionApplicator
@@ -163,43 +164,29 @@ class PermissionApplicator
         $this->getCurrentUserRoleIds();
         $this->currentUser()->id;
 
-        $userViewAll = userCan($morphClass . '-view-all');
-        $userViewOwn = userCan($morphClass . '-view-own');
-
         // TODO - Leave this as the new admin workaround?
         //   Or auto generate collapsed role permissions for admins?
         if (\user()->hasSystemRole('admin')) {
             return $query;
         }
 
-        // Fallback permission join
-        $query->joinSub(function (QueryBuilder $joinQuery) use ($morphClass) {
-            $joinQuery->select(['entity_id'])->selectRaw('max(view) as perms_fallback')
-                ->from('entity_permissions_collapsed')
-                ->where('entity_type', '=', $morphClass)
-                ->whereNull(['role_id', 'user_id'])
-                ->groupBy('entity_id');
-        }, 'p_f', 'id', '=', 'p_f.entity_id', 'left');
-
-        // Role permission join
-        $query->joinSub(function (QueryBuilder $joinQuery) use ($morphClass) {
-            $joinQuery->select(['entity_id'])->selectRaw('max(view) as perms_role')
-                ->from('entity_permissions_collapsed')
-                ->where('entity_type', '=', $morphClass)
-                ->whereIn('role_id', $this->getCurrentUserRoleIds())
-                ->groupBy('entity_id');
-        }, 'p_r', 'id', '=', 'p_r.entity_id', 'left');
-
-        // User permission join
-        $query->joinSub(function (QueryBuilder $joinQuery) use ($morphClass) {
-            $joinQuery->select(['entity_id'])->selectRaw('max(view) as perms_user')
-                ->from('entity_permissions_collapsed')
-                ->where('entity_type', '=', $morphClass)
-                ->where('user_id', '=', $this->currentUser()->id)
-                ->groupBy('entity_id');
-        }, 'p_u', 'id', '=', 'p_u.entity_id', 'left');
+        // Apply permission level joins
+        $this->applyFallbackJoin($query, $morphClass, 'id', '');
+        $this->applyRoleJoin($query, $morphClass, 'id', '');
+        $this->applyUserJoin($query, $morphClass, 'id', '');
 
         // Where permissions apply
+        $this->applyPermissionWhereFilter($query, $morphClass);
+
+        return $query;
+    }
+
+    protected function applyPermissionWhereFilter($query, string $entityTypeLimiter)
+    {
+        // TODO - Morph for all types
+        $userViewAll = userCan($entityTypeLimiter . '-view-all');
+        $userViewOwn = userCan($entityTypeLimiter . '-view-own');
+
         $query->where(function (Builder $query) use ($userViewOwn, $userViewAll) {
             $query->where('perms_user', '=', 1)
                 ->orWhere(function (Builder $query) {
@@ -220,8 +207,68 @@ class PermissionApplicator
                 });
             }
         });
+    }
 
-        return $query;
+    /**
+     * @param Builder|QueryBuilder $query
+     */
+    protected function applyPermissionJoin(callable $joinCallable, string $subAlias, $query, string $entityTypeLimiter, string $entityIdColumn, string $entityTypeColumn)
+    {
+        $joinCondition = $this->getJoinCondition($subAlias, $entityIdColumn, $entityTypeColumn);
+
+        $query->joinSub(function (QueryBuilder $joinQuery) use ($joinCallable, $entityTypeLimiter) {
+            $joinQuery->select(['entity_id', 'entity_type'])->from('entity_permissions_collapsed')
+                ->groupBy('entity_id', 'entity_type');
+            $joinCallable($joinQuery);
+
+            if ($entityTypeLimiter) {
+                $joinQuery->where('entity_type', '=', $entityTypeLimiter);
+            }
+        }, $subAlias, $joinCondition, null, null, 'left');
+    }
+
+    /**
+     * @param Builder|QueryBuilder $query
+     */
+    protected function applyUserJoin($query, string $entityTypeLimiter, string $entityIdColumn, string $entityTypeColumn)
+    {
+        $this->applyPermissionJoin(function (QueryBuilder $joinQuery) {
+            $joinQuery->selectRaw('max(view) as perms_user')
+                ->where('user_id', '=', $this->currentUser()->id);
+        }, 'p_u', $query, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
+    }
+
+
+    /**
+     * @param Builder|QueryBuilder $query
+     */
+    protected function applyRoleJoin($query, string $entityTypeLimiter, string $entityIdColumn, string $entityTypeColumn)
+    {
+        $this->applyPermissionJoin(function (QueryBuilder $joinQuery) {
+            $joinQuery->selectRaw('max(view) as perms_role')
+                ->whereIn('role_id', $this->getCurrentUserRoleIds());
+        }, 'p_r', $query, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
+    }
+
+    /**
+     * @param Builder|QueryBuilder $query
+     */
+    protected function applyFallbackJoin($query, string $entityTypeLimiter, string $entityIdColumn, string $entityTypeColumn)
+    {
+        $this->applyPermissionJoin(function (QueryBuilder $joinQuery) {
+            $joinQuery->selectRaw('max(view) as perms_fallback')
+                ->whereNull(['role_id', 'user_id']);
+        }, 'p_f', $query, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
+    }
+
+    protected function getJoinCondition(string $joinTableName, string $entityIdColumn, string $entityTypeColumn): callable
+    {
+        return function (JoinClause $join) use ($joinTableName, $entityIdColumn, $entityTypeColumn) {
+            $join->on($entityIdColumn, '=', $joinTableName . '.entity_id');
+            if ($entityTypeColumn) {
+                $join->on($entityTypeColumn, '=', $joinTableName . '.entity_type');
+            }
+        };
     }
 
     /**
@@ -251,7 +298,8 @@ class PermissionApplicator
         $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn];
         $pageMorphClass = (new Page())->getMorphClass();
 
-        // TODO;
+        // TODO - Abstract the permission queries above to make their join columns configurable
+        //   so the query methods can be used on non-entity tables if possible.
         return $query;
 
         $q = $query->where(function ($query) use ($tableDetails) {

diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php
index 013f7b380b82239e8ce475b63d8efa52d856db8f..473fba5c61c23e8baa7f3a46ce12254628762f6f 100644 (file)
--- a/app/Search/SearchRunner.php
+++ b/app/Search/SearchRunner.php
@@ -223,7 +223,7 @@ class SearchRunner
         });
         $subQuery->groupBy('entity_type', 'entity_id');
 
-        $entityQuery->joinSub($subQuery, 's', 'id', '=', 'entity_id');
+        $entityQuery->joinSub($subQuery, 's', 'id', '=', 's.entity_id');
         $entityQuery->addSelect('s.score');
         $entityQuery->orderBy('score', 'desc');
     }

diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php
index 1a4727e1eaac555d232a9846c290f31825aba72f..5c6489281720711c56c3ada7d1458eb634876488 100644 (file)
--- a/tests/Entity/BookShelfTest.php
+++ b/tests/Entity/BookShelfTest.php
@@ -21,7 +21,6 @@ class BookShelfTest extends TestCase
         $this->withHtml($resp)->assertElementContains('header', 'Shelves');
 
         $viewer->roles()->delete();
-        $this->permissions->grantUserRolePermissions($viewer);
         $resp = $this->actingAs($viewer)->get('/');
         $this->withHtml($resp)->assertElementNotContains('header', 'Shelves');
 

diff --git a/tests/Unit/FrameworkAssumptionTest.php b/tests/Unit/FrameworkAssumptionTest.php
index 54d315de94054c2765fa559b3e258a7adb4442c9..d4feff60c9da66973019dec0596cd80a316f2845 100644 (file)
--- a/tests/Unit/FrameworkAssumptionTest.php
+++ b/tests/Unit/FrameworkAssumptionTest.php
@@ -25,7 +25,7 @@ class FrameworkAssumptionTest extends TestCase
         // Page has SoftDeletes trait by default, so we apply our custom scope and ensure
         // it stacks on the global scope to filter out deleted items.
         $query = Page::query()->scopes('visible')->toSql();
-        $this->assertStringContainsString('joint_permissions', $query);
+        $this->assertStringContainsString('entity_permissions_collapsed', $query);
         $this->assertStringContainsString('`deleted_at` is null', $query);
     }
 }