From afe1a042396454e071b4b3bb5bb0043586ba333a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 16 Jul 2022 19:54:25 +0100 Subject: [PATCH] Aligned permission applicator method names Also removed lesser used function, that was mostly a duplicate of an existing function, and only used for search. --- app/Actions/ActivityQueries.php | 4 +- app/Actions/TagRepo.php | 6 +- app/Auth/Permissions/PermissionApplicator.php | 80 ++++++------------- app/Entities/Models/Page.php | 2 +- app/Entities/Queries/Popular.php | 2 +- app/Entities/Queries/RecentlyViewed.php | 2 +- app/Entities/Queries/TopFavourites.php | 2 +- app/Entities/Tools/SearchRunner.php | 4 +- app/Uploads/Attachment.php | 3 +- app/Uploads/ImageRepo.php | 2 +- 10 files changed, 36 insertions(+), 71 deletions(-) diff --git a/app/Actions/ActivityQueries.php b/app/Actions/ActivityQueries.php index 0b8cd3b5e..0e9cbdebb 100644 --- a/app/Actions/ActivityQueries.php +++ b/app/Actions/ActivityQueries.php @@ -26,7 +26,7 @@ class ActivityQueries public function latest(int $count = 20, int $page = 0): array { $activityList = $this->permissions - ->filterRestrictedEntityRelations(Activity::query(), 'activities', 'entity_id', 'entity_type') + ->restrictEntityRelationQuery(Activity::query(), 'activities', 'entity_id', 'entity_type') ->orderBy('created_at', 'desc') ->with(['user', 'entity']) ->skip($count * $page) @@ -79,7 +79,7 @@ class ActivityQueries public function userActivity(User $user, int $count = 20, int $page = 0): array { $activityList = $this->permissions - ->filterRestrictedEntityRelations(Activity::query(), 'activities', 'entity_id', 'entity_type') + ->restrictEntityRelationQuery(Activity::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 c5807108e..172d8ec6e 100644 --- a/app/Actions/TagRepo.php +++ b/app/Actions/TagRepo.php @@ -50,7 +50,7 @@ class TagRepo }); } - return $this->permissions->filterRestrictedEntityRelations($query, 'tags', 'entity_id', 'entity_type'); + return $this->permissions->restrictEntityRelationQuery($query, 'tags', 'entity_id', 'entity_type'); } /** @@ -69,7 +69,7 @@ class TagRepo $query = $query->orderBy('count', 'desc')->take(50); } - $query = $this->permissions->filterRestrictedEntityRelations($query, 'tags', 'entity_id', 'entity_type'); + $query = $this->permissions->restrictEntityRelationQuery($query, 'tags', 'entity_id', 'entity_type'); return $query->get(['name'])->pluck('name'); } @@ -95,7 +95,7 @@ class TagRepo $query = $query->where('name', '=', $tagName); } - $query = $this->permissions->filterRestrictedEntityRelations($query, 'tags', 'entity_id', 'entity_type'); + $query = $this->permissions->restrictEntityRelationQuery($query, 'tags', 'entity_id', 'entity_type'); return $query->get(['value'])->pluck('value'); } diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php index e73db5157..91a7c72ae 100644 --- a/app/Auth/Permissions/PermissionApplicator.php +++ b/app/Auth/Permissions/PermissionApplicator.php @@ -105,7 +105,7 @@ class PermissionApplicator } /** - * Limited the given entity query so that the query will only + * Limit the given entity query so that the query will only * return items that the user has view permission for. */ public function restrictEntityQuery(Builder $query): Builder @@ -126,7 +126,7 @@ class PermissionApplicator * Extend the given page query to ensure draft items are not visible * unless created by the given user. */ - public function enforceDraftVisibilityOnQuery(Builder $query): Builder + public function restrictDraftsOnPageQuery(Builder $query): Builder { return $query->where(function (Builder $query) { $query->where('draft', '=', false) @@ -137,39 +137,6 @@ class PermissionApplicator }); } - /** - * Add restrictions for a generic entity. - */ - public function enforceEntityRestrictions(Entity $entity, Builder $query): Builder - { - if ($entity instanceof Page) { - // Prevent drafts being visible to others. - $this->enforceDraftVisibilityOnQuery($query); - } - - return $this->entityRestrictionQuery($query); - } - - /** - * The general query filter to remove all entities - * that the current user does not have access to. - */ - protected function entityRestrictionQuery(Builder $query): Builder - { - $q = $query->where(function ($parentQuery) { - $parentQuery->whereHas('jointPermissions', function ($permissionQuery) { - $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds()) - // TODO - Delete line once only views - ->where('action', '=', 'view') - ->where(function (Builder $query) { - $this->addJointHasPermissionCheck($query, $this->currentUser()->id); - }); - }); - }); - - return $q; - } - /** * Filter items that have entities set as a polymorphic relation. * For simplicity, this will not return results attached to draft pages. @@ -177,7 +144,7 @@ class PermissionApplicator * * @param Builder|QueryBuilder $query */ - public function filterRestrictedEntityRelations($query, string $tableName, string $entityIdColumn, string $entityTypeColumn) + public function restrictEntityRelationQuery($query, string $tableName, string $entityIdColumn, string $entityTypeColumn) { $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn]; $pageMorphClass = (new Page())->getMorphClass(); @@ -207,19 +174,20 @@ class PermissionApplicator } /** - * Add conditions to a query to filter the selection to related entities - * where view permissions are granted. + * Add conditions to a query for a model that's a relation of a page, so only the model results + * on visible pages are returned by the query. + * Is effectively the same as "restrictEntityRelationQuery" but takes into account page drafts + * while not expecting a polymorphic relation, Just a simpler one-page-to-many-relations set-up. */ - public function filterRelatedEntity(string $entityClass, Builder $query, string $tableName, string $entityIdColumn): Builder + public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder { - $fullEntityIdColumn = $tableName . '.' . $entityIdColumn; - $instance = new $entityClass(); - $morphClass = $instance->getMorphClass(); + $fullPageIdColumn = $tableName . '.' . $pageIdColumn; + $morphClass = (new Page())->getMorphClass(); - $existsQuery = function ($permissionQuery) use ($fullEntityIdColumn, $morphClass) { + $existsQuery = function ($permissionQuery) use ($fullPageIdColumn, $morphClass) { /** @var Builder $permissionQuery */ $permissionQuery->select('joint_permissions.role_id')->from('joint_permissions') - ->whereColumn('joint_permissions.entity_id', '=', $fullEntityIdColumn) + ->whereColumn('joint_permissions.entity_id', '=', $fullPageIdColumn) ->where('joint_permissions.entity_type', '=', $morphClass) ->where('joint_permissions.action', '=', 'view') ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds()) @@ -228,22 +196,20 @@ class PermissionApplicator }); }; - $q = $query->where(function ($query) use ($existsQuery, $fullEntityIdColumn) { + $q = $query->where(function ($query) use ($existsQuery, $fullPageIdColumn) { $query->whereExists($existsQuery) - ->orWhere($fullEntityIdColumn, '=', 0); + ->orWhere($fullPageIdColumn, '=', 0); }); - if ($instance instanceof Page) { - // Prevent visibility of non-owned draft pages - $q->whereExists(function (QueryBuilder $query) use ($fullEntityIdColumn) { - $query->select('id')->from('pages') - ->whereColumn('pages.id', '=', $fullEntityIdColumn) - ->where(function (QueryBuilder $query) { - $query->where('pages.draft', '=', false) - ->orWhere('pages.owned_by', '=', $this->currentUser()->id); - }); - }); - } + // 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; } diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php index a2690b2a0..93729d7f2 100644 --- a/app/Entities/Models/Page.php +++ b/app/Entities/Models/Page.php @@ -51,7 +51,7 @@ class Page extends BookChild */ public function scopeVisible(Builder $query): Builder { - $query = app()->make(PermissionApplicator::class)->enforceDraftVisibilityOnQuery($query); + $query = app()->make(PermissionApplicator::class)->restrictDraftsOnPageQuery($query); return parent::scopeVisible($query); } diff --git a/app/Entities/Queries/Popular.php b/app/Entities/Queries/Popular.php index 82786e3c6..fafd60c59 100644 --- a/app/Entities/Queries/Popular.php +++ b/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') + ->restrictEntityRelationQuery(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'); diff --git a/app/Entities/Queries/RecentlyViewed.php b/app/Entities/Queries/RecentlyViewed.php index 38d1f1be4..0f5c68041 100644 --- a/app/Entities/Queries/RecentlyViewed.php +++ b/app/Entities/Queries/RecentlyViewed.php @@ -14,7 +14,7 @@ class RecentlyViewed extends EntityQuery return collect(); } - $query = $this->permissionService()->filterRestrictedEntityRelations( + $query = $this->permissionService()->restrictEntityRelationQuery( View::query(), 'views', 'viewable_id', diff --git a/app/Entities/Queries/TopFavourites.php b/app/Entities/Queries/TopFavourites.php index 5d138679a..0b9a5ba23 100644 --- a/app/Entities/Queries/TopFavourites.php +++ b/app/Entities/Queries/TopFavourites.php @@ -15,7 +15,7 @@ class TopFavourites extends EntityQuery } $query = $this->permissionService() - ->filterRestrictedEntityRelations(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type') + ->restrictEntityRelationQuery(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type') ->select('favourites.*') ->leftJoin('views', function (JoinClause $join) { $join->on('favourites.favouritable_id', '=', 'views.viewable_id'); diff --git a/app/Entities/Tools/SearchRunner.php b/app/Entities/Tools/SearchRunner.php index 1dcc2eb44..459409084 100644 --- a/app/Entities/Tools/SearchRunner.php +++ b/app/Entities/Tools/SearchRunner.php @@ -161,7 +161,7 @@ class SearchRunner */ protected function buildQuery(SearchOptions $searchOpts, Entity $entityModelInstance): EloquentBuilder { - $entityQuery = $entityModelInstance->newQuery(); + $entityQuery = $entityModelInstance->newQuery()->scopes('visible'); if ($entityModelInstance instanceof Page) { $entityQuery->select($entityModelInstance::$listAttributes); @@ -193,7 +193,7 @@ class SearchRunner } } - return $this->permissions->enforceEntityRestrictions($entityModelInstance, $entityQuery); + return $entityQuery; } /** diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index d9e0ce7e1..6c7066ff9 100644 --- a/app/Uploads/Attachment.php +++ b/app/Uploads/Attachment.php @@ -91,8 +91,7 @@ class Attachment extends Model { $permissions = app()->make(PermissionApplicator::class); - return $permissions->filterRelatedEntity( - Page::class, + return $permissions->restrictPageRelationQuery( self::query(), 'attachments', 'uploaded_to' diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 13d3344dd..8770402ad 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -74,7 +74,7 @@ class ImageRepo } // Filter by page access - $imageQuery = $this->permissions->filterRelatedEntity(Page::class, $imageQuery, 'images', 'uploaded_to'); + $imageQuery = $this->permissions->restrictPageRelationQuery($imageQuery, 'images', 'uploaded_to'); if ($whereClause !== null) { $imageQuery = $imageQuery->where($whereClause);