From 55642a33ee7b3221b760ff6ebf122d5f7e8a49c8 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 14 Jan 2023 13:50:41 +0000 Subject: [PATCH] Attempted fix of issues, realised new query system is a failure 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 | 2 ++ app/Auth/Permissions/PermissionApplicator.php | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/Actions/ActivityQueries.php b/app/Actions/ActivityQueries.php index 88e6aa7e7..67bcf7483 100644 --- a/app/Actions/ActivityQueries.php +++ b/app/Actions/ActivityQueries.php @@ -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(); diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php index 3ccccb0ac..af6ca4d67 100644 --- a/app/Auth/Permissions/PermissionApplicator.php +++ b/app/Auth/Permissions/PermissionApplicator.php @@ -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'])); } }); }