]> BookStack Code Mirror - bookstack/commitdiff
Attempted fix of issues, realised new query system is a failure
authorDan Brown <redacted>
Sat, 14 Jan 2023 13:50:41 +0000 (13:50 +0000)
committerDan Brown <redacted>
Sat, 14 Jan 2023 13:50:41 +0000 (13:50 +0000)
As part of the permission checking we need to check owner user status.
Upon this, we'd also want to check page draft status (and its
creator/owner).
These, for cross-entity/relation queries would need up to another 4 joins.
The performance/index usage is already questionable here.

app/Actions/ActivityQueries.php
app/Auth/Permissions/PermissionApplicator.php

index 88e6aa7e783485a47d67ba3ee91ddc744080e97f..67bcf7483a5844bb482384dd8e9b73984f7e0d89 100644 (file)
@@ -31,6 +31,7 @@ class ActivityQueries
         $activityList = $this->permissions
             ->restrictEntityRelationQuery($query, 'activities', 'entity_id', 'entity_type')
             ->orderBy('created_at', 'desc')
+            ->whereNotNull('activities.entity_id')
             ->with(['user', 'entity'])
             ->skip($count * $page)
             ->take($count)
@@ -86,6 +87,7 @@ class ActivityQueries
             ->restrictEntityRelationQuery($query, 'activities', 'entity_id', 'entity_type')
             ->orderBy('created_at', 'desc')
             ->where('user_id', '=', $user->id)
+            ->whereNotNull('activities.entity_id')
             ->skip($count * $page)
             ->take($count)
             ->get();
index 3ccccb0ace4b03f7fb452d6e0f7306ec806087ad..af6ca4d67f6714a932df500206115d3cc8332cee 100644 (file)
@@ -178,7 +178,7 @@ class PermissionApplicator
         $this->applyFallbackJoin($query, $queryTable, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
         $this->applyRoleJoin($query, $queryTable, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
         $this->applyUserJoin($query, $queryTable, $entityTypeLimiter, $entityIdColumn, $entityTypeColumn);
-        $this->applyPermissionWhereFilter($query, $entityTypeLimiter, $entityTypeColumn);
+        $this->applyPermissionWhereFilter($query, $queryTable, $entityTypeLimiter, $entityTypeColumn);
     }
 
     /**
@@ -188,10 +188,11 @@ class PermissionApplicator
      * Both should not be applied since that would conflict upon intent.
      * @param Builder|QueryBuilder $query
      */
-    protected function applyPermissionWhereFilter($query, string $entityTypeLimiter, string $entityTypeColumn)
+    protected function applyPermissionWhereFilter($query, string $queryTable, string $entityTypeLimiter, string $entityTypeColumn)
     {
         $abilities = ['all' => [], 'own' => []];
         $types = $entityTypeLimiter ? [$entityTypeLimiter] : ['page', 'chapter', 'bookshelf', 'book'];
+        $fullEntityTypeColumn = $queryTable . '.' . $entityTypeColumn;
         foreach ($types as $type) {
             $abilities['all'][$type] = userCan($type . '-view-all');
             $abilities['own'][$type] = userCan($type . '-view-own');
@@ -200,7 +201,7 @@ class PermissionApplicator
         $abilities['all'] = array_filter($abilities['all']);
         $abilities['own'] = array_filter($abilities['own']);
 
-        $query->where(function (Builder $query) use ($abilities, $entityTypeColumn) {
+        $query->where(function (Builder $query) use ($abilities, $fullEntityTypeColumn) {
             $query->where('perms_user', '=', 1)
                 ->orWhere(function (Builder $query) {
                     $query->whereNull('perms_user')->where('perms_role', '=', 1);
@@ -210,20 +211,20 @@ class PermissionApplicator
                 });
 
             if (count($abilities['all']) > 0) {
-                $query->orWhere(function (Builder $query) use ($abilities, $entityTypeColumn) {
+                $query->orWhere(function (Builder $query) use ($abilities, $fullEntityTypeColumn) {
                     $query->whereNull(['perms_user', 'perms_role', 'perms_fallback']);
-                    if ($entityTypeColumn) {
-                        $query->whereIn($entityTypeColumn, array_keys($abilities['all']));
+                    if ($fullEntityTypeColumn) {
+                        $query->whereIn($fullEntityTypeColumn, array_keys($abilities['all']));
                     }
                 });
             }
 
             if (count($abilities['own']) > 0) {
-                $query->orWhere(function (Builder $query) use ($abilities, $entityTypeColumn) {
+                $query->orWhere(function (Builder $query) use ($abilities, $fullEntityTypeColumn) {
                     $query->whereNull(['perms_user', 'perms_role', 'perms_fallback'])
                         ->where('owned_by', '=', $this->currentUser()->id);
-                    if ($entityTypeColumn) {
-                        $query->whereIn($entityTypeColumn, array_keys($abilities['all']));
+                    if ($fullEntityTypeColumn) {
+                        $query->whereIn($fullEntityTypeColumn, array_keys($abilities['all']));
                     }
                 });
             }