From bf31f3e8ec4afee68f4ba0634b5d7af3e4ec29c1 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Tue, 8 Dec 2020 19:02:08 -0500 Subject: [PATCH] Refactor for Policies Revamp (#105) --- extensions/tags/extend.php | 32 ++++++-- .../tags/src/Access/DiscussionPolicy.php | 75 ++----------------- extensions/tags/src/Access/GlobalPolicy.php | 33 +++++--- .../src/Access/ScopeDiscussionVisibility.php | 43 +++++++++++ .../ScopeDiscussionVisibilityForAbility.php | 38 ++++++++++ ...FlagPolicy.php => ScopeFlagVisibility.php} | 11 +-- .../tags/src/Access/ScopeTagVisibility.php | 26 +++++++ extensions/tags/src/Access/TagPolicy.php | 22 +----- .../Listener/CreatePostWhenTagsAreChanged.php | 22 +----- extensions/tags/src/Tag.php | 14 ++-- 10 files changed, 176 insertions(+), 140 deletions(-) create mode 100644 extensions/tags/src/Access/ScopeDiscussionVisibility.php create mode 100644 extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php rename extensions/tags/src/Access/{FlagPolicy.php => ScopeFlagVisibility.php} (80%) mode change 100755 => 100644 create mode 100644 extensions/tags/src/Access/ScopeTagVisibility.php diff --git a/extensions/tags/extend.php b/extensions/tags/extend.php index c0dd41862..c547a578a 100644 --- a/extensions/tags/extend.php +++ b/extensions/tags/extend.php @@ -13,12 +13,15 @@ use Flarum\Api\Serializer\ForumSerializer; use Flarum\Discussion\Discussion; use Flarum\Discussion\Event\Saving; use Flarum\Extend; +use Flarum\Flags\Flag; use Flarum\Tags\Access; use Flarum\Tags\Api\Controller; use Flarum\Tags\Api\Serializer\TagSerializer; use Flarum\Tags\Content; +use Flarum\Tags\Event\DiscussionWasTagged; use Flarum\Tags\Listener; use Flarum\Tags\LoadForumTagsRelationship; +use Flarum\Tags\Post\DiscussionTaggedPost; use Flarum\Tags\Tag; use Illuminate\Contracts\Events\Dispatcher; @@ -71,21 +74,36 @@ return [ ->serializeToForum('minSecondaryTags', 'flarum-tags.min_secondary_tags') ->serializeToForum('maxSecondaryTags', 'flarum-tags.max_secondary_tags'), + (new Extend\Policy()) + ->modelPolicy(Discussion::class, Access\DiscussionPolicy::class) + ->modelPolicy(Tag::class, Access\TagPolicy::class) + ->globalPolicy(Access\GlobalPolicy::class), + + (new Extend\ModelVisibility(Discussion::class)) + ->scope(Access\ScopeDiscussionVisibility::class) + ->scopeAll(Access\ScopeDiscussionVisibilityForAbility::class), + + (new Extend\ModelVisibility(Flag::class)) + ->scope(Access\ScopeFlagVisibility::class), + + (new Extend\ModelVisibility(Tag::class)) + ->scope(Access\ScopeTagVisibility::class), + new Extend\Locales(__DIR__.'/locale'), (new Extend\View) ->namespace('tags', __DIR__.'/views'), + (new Extend\Post) + ->type(DiscussionTaggedPost::class), + + (new Extend\Event()) + ->listen(Saving::class, Listener\SaveTagsToDatabase::class) + ->listen(DiscussionWasTagged::class, Listener\CreatePostWhenTagsAreChanged::class), + function (Dispatcher $events) { - $events->subscribe(Listener\CreatePostWhenTagsAreChanged::class); $events->subscribe(Listener\FilterDiscussionListByTags::class); $events->subscribe(Listener\FilterPostsQueryByTag::class); - $events->listen(Saving::class, Listener\SaveTagsToDatabase::class); $events->subscribe(Listener\UpdateTagMetadata::class); - - $events->subscribe(Access\GlobalPolicy::class); - $events->subscribe(Access\DiscussionPolicy::class); - $events->subscribe(Access\TagPolicy::class); - $events->subscribe(Access\FlagPolicy::class); }, ]; diff --git a/extensions/tags/src/Access/DiscussionPolicy.php b/extensions/tags/src/Access/DiscussionPolicy.php index 4171d606d..bd93b3809 100755 --- a/extensions/tags/src/Access/DiscussionPolicy.php +++ b/extensions/tags/src/Access/DiscussionPolicy.php @@ -11,38 +11,22 @@ namespace Flarum\Tags\Access; use Carbon\Carbon; use Flarum\Discussion\Discussion; -use Flarum\Event\ScopeModelVisibility; use Flarum\Settings\SettingsRepositoryInterface; -use Flarum\Tags\Tag; -use Flarum\User\AbstractPolicy; +use Flarum\User\Access\AbstractPolicy; use Flarum\User\User; -use Illuminate\Contracts\Events\Dispatcher; -use Illuminate\Database\Eloquent\Builder; class DiscussionPolicy extends AbstractPolicy { - /** - * {@inheritdoc} - */ - protected $model = Discussion::class; - - /** - * @var Dispatcher - */ - protected $events; - /** * @var SettingsRepositoryInterface */ protected $settings; /** - * @param Dispatcher $events * @param SettingsRepositoryInterface $settings */ - public function __construct(Dispatcher $events, SettingsRepositoryInterface $settings) + public function __construct(SettingsRepositoryInterface $settings) { - $this->events = $events; $this->settings = $settings; } @@ -60,69 +44,24 @@ class DiscussionPolicy extends AbstractPolicy $tags = $discussion->tags; if (count($tags)) { - $restricted = false; + $restrictedButHasAccess = false; foreach ($tags as $tag) { if ($tag->is_restricted) { if (! $actor->hasPermission('tag'.$tag->id.'.discussion.'.$ability)) { - return false; + return $this->deny(); } - $restricted = true; + $restrictedButHasAccess = true; } } - if ($restricted) { - return true; + if ($restrictedButHasAccess) { + return $this->allow(); } } } - /** - * @param User $actor - * @param Builder $query - */ - public function find(User $actor, Builder $query) - { - // Hide discussions which have tags that the user is not allowed to see, unless an extension overrides this. - $query->where(function ($query) use ($actor) { - $query - ->whereNotIn('discussions.id', function ($query) use ($actor) { - return $query->select('discussion_id') - ->from('discussion_tag') - ->whereIn('tag_id', Tag::getIdsWhereCannot($actor, 'viewDiscussions')); - }) - ->orWhere(function ($query) use ($actor) { - $this->events->dispatch( - new ScopeModelVisibility($query, $actor, 'viewDiscussionsInRestrictedTags') - ); - }); - }); - - // Hide discussions with no tags if the user doesn't have that global - // permission. - if (! $actor->hasPermission('viewDiscussions')) { - $query->has('tags'); - } - } - - /** - * @param User $actor - * @param Builder $query - * @param string $ability - */ - public function findWithPermission(User $actor, Builder $query, $ability) - { - // If a discussion requires a certain permission in order for it to be - // visible, then we can check if the user has been granted that - // permission for any of the discussion's tags. - $query->whereIn('discussions.id', function ($query) use ($actor, $ability) { - return $query->select('discussion_id') - ->from('discussion_tag') - ->whereIn('tag_id', Tag::getIdsWhereCan($actor, 'discussion.'.$ability)); - }); - } - /** * This method checks, if the user is still allowed to edit the tags * based on the configuration item. diff --git a/extensions/tags/src/Access/GlobalPolicy.php b/extensions/tags/src/Access/GlobalPolicy.php index 20d4715f5..a119d35d5 100644 --- a/extensions/tags/src/Access/GlobalPolicy.php +++ b/extensions/tags/src/Access/GlobalPolicy.php @@ -9,28 +9,39 @@ namespace Flarum\Tags\Access; -use Flarum\Event\GetPermission; +use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Tags\Tag; -use Illuminate\Contracts\Events\Dispatcher; +use Flarum\User\Access\AbstractPolicy; +use Flarum\User\User; -class GlobalPolicy +class GlobalPolicy extends AbstractPolicy { /** - * @param Dispatcher $events + * @var SettingsRepositoryInterface */ - public function subscribe(Dispatcher $events) + protected $settings; + + public function __construct(SettingsRepositoryInterface $settings) { - $events->listen(GetPermission::class, [$this, 'grantGlobalDiscussionPermissions']); + $this->settings = $settings; } /** - * @param GetPermission $event - * @return bool + * @param Flarum\User\User $actor + * @param string $ability + * @return bool|void */ - public function grantGlobalDiscussionPermissions(GetPermission $event) + public function can(User $actor, string $ability) { - if (in_array($event->ability, ['viewDiscussions', 'startDiscussion']) && is_null($event->model)) { - return ! empty(Tag::getIdsWhereCan($event->actor, $event->ability)); + if (in_array($ability, ['viewDiscussions', 'startDiscussion'])) { + $enoughPrimary = count(Tag::getIdsWhereCan($actor, $ability, true, false)) >= $this->settings->get('min_primary_tags'); + $enoughSecondary = count(Tag::getIdsWhereCan($actor, $ability, false, true)) >= $this->settings->get('min_secondary_tags'); + + if ($enoughPrimary && $enoughSecondary) { + return $this->allow(); + } else { + return $this->deny(); + } } } } diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibility.php b/extensions/tags/src/Access/ScopeDiscussionVisibility.php new file mode 100644 index 000000000..d3bbc0765 --- /dev/null +++ b/extensions/tags/src/Access/ScopeDiscussionVisibility.php @@ -0,0 +1,43 @@ +where(function ($query) use ($actor) { + $query + ->whereNotIn('discussions.id', function ($query) use ($actor) { + return $query->select('discussion_id') + ->from('discussion_tag') + ->whereIn('tag_id', Tag::getIdsWhereCannot($actor, 'viewDiscussions')); + }) + ->orWhere(function ($query) use ($actor) { + $query->whereVisibleTo($actor, 'viewDiscussionsInRestrictedTags'); + }); + }); + + // Hide discussions with no tags if the user doesn't have that global + // permission. + if (! $actor->hasPermission('viewDiscussions')) { + $query->has('tags'); + } + } +} diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php new file mode 100644 index 000000000..5bf2b6ba0 --- /dev/null +++ b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php @@ -0,0 +1,38 @@ +whereIn('discussions.id', function ($query) use ($actor, $ability) { + return $query->select('discussion_id') + ->from('discussion_tag') + ->whereIn('tag_id', Tag::getIdsWhereCan($actor, 'discussion.'.$ability)); + }); + } +} diff --git a/extensions/tags/src/Access/FlagPolicy.php b/extensions/tags/src/Access/ScopeFlagVisibility.php old mode 100755 new mode 100644 similarity index 80% rename from extensions/tags/src/Access/FlagPolicy.php rename to extensions/tags/src/Access/ScopeFlagVisibility.php index 659963e6a..bcb459b6f --- a/extensions/tags/src/Access/FlagPolicy.php +++ b/extensions/tags/src/Access/ScopeFlagVisibility.php @@ -9,24 +9,17 @@ namespace Flarum\Tags\Access; -use Flarum\Flags\Flag; use Flarum\Tags\Tag; -use Flarum\User\AbstractPolicy; use Flarum\User\User; use Illuminate\Database\Eloquent\Builder; -class FlagPolicy extends AbstractPolicy +class ScopeFlagVisibility { - /** - * {@inheritdoc} - */ - protected $model = Flag::class; - /** * @param User $actor * @param Builder $query */ - public function find(User $actor, Builder $query) + public function __invoke(User $actor, Builder $query) { $query ->select('flags.*') diff --git a/extensions/tags/src/Access/ScopeTagVisibility.php b/extensions/tags/src/Access/ScopeTagVisibility.php new file mode 100644 index 000000000..f159dc467 --- /dev/null +++ b/extensions/tags/src/Access/ScopeTagVisibility.php @@ -0,0 +1,26 @@ +whereNotIn('id', Tag::getIdsWhereCannot($actor, 'viewDiscussions')); + } +} diff --git a/extensions/tags/src/Access/TagPolicy.php b/extensions/tags/src/Access/TagPolicy.php index 5e69f1b02..a9121af6c 100755 --- a/extensions/tags/src/Access/TagPolicy.php +++ b/extensions/tags/src/Access/TagPolicy.php @@ -10,26 +10,11 @@ namespace Flarum\Tags\Access; use Flarum\Tags\Tag; -use Flarum\User\AbstractPolicy; +use Flarum\User\Access\AbstractPolicy; use Flarum\User\User; -use Illuminate\Database\Eloquent\Builder; class TagPolicy extends AbstractPolicy { - /** - * {@inheritdoc} - */ - protected $model = Tag::class; - - /** - * @param User $actor - * @param Builder $query - */ - public function find(User $actor, Builder $query) - { - $query->whereNotIn('id', Tag::getIdsWhereCannot($actor, 'viewDiscussions')); - } - /** * @param User $actor * @param Tag $tag @@ -37,9 +22,8 @@ class TagPolicy extends AbstractPolicy */ public function startDiscussion(User $actor, Tag $tag) { - if ((! $tag->is_restricted && $actor->hasPermission('startDiscussion')) - || ($tag->is_restricted && $actor->hasPermission('tag'.$tag->id.'.startDiscussion'))) { - return true; + if ($tag->is_restricted) { + return $actor->hasPermission('tag'.$tag->id.'.startDiscussion') ? $this->allow() : $this->deny(); } } diff --git a/extensions/tags/src/Listener/CreatePostWhenTagsAreChanged.php b/extensions/tags/src/Listener/CreatePostWhenTagsAreChanged.php index 151d2bd3f..2ad235431 100755 --- a/extensions/tags/src/Listener/CreatePostWhenTagsAreChanged.php +++ b/extensions/tags/src/Listener/CreatePostWhenTagsAreChanged.php @@ -9,35 +9,17 @@ namespace Flarum\Tags\Listener; -use Flarum\Event\ConfigurePostTypes; use Flarum\Tags\Event\DiscussionWasTagged; use Flarum\Tags\Post\DiscussionTaggedPost; -use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Support\Arr; class CreatePostWhenTagsAreChanged { - /** - * @param Dispatcher $events - */ - public function subscribe(Dispatcher $events) - { - $events->listen(ConfigurePostTypes::class, [$this, 'addPostType']); - $events->listen(DiscussionWasTagged::class, [$this, 'whenDiscussionWasTagged']); - } - - /** - * @param ConfigurePostTypes $event - */ - public function addPostType(ConfigurePostTypes $event) - { - $event->add(DiscussionTaggedPost::class); - } - /** * @param DiscussionWasTagged $event + * @return void */ - public function whenDiscussionWasTagged(DiscussionWasTagged $event) + public function handle(DiscussionWasTagged $event) { $post = DiscussionTaggedPost::reply( $event->discussion->id, diff --git a/extensions/tags/src/Tag.php b/extensions/tags/src/Tag.php index e8865ca22..4bc474255 100644 --- a/extensions/tags/src/Tag.php +++ b/extensions/tags/src/Tag.php @@ -190,7 +190,7 @@ class Tag extends AbstractModel Permission::where('permission', 'like', "tag{$this->id}.%")->delete(); } - protected static function getIdsWherePermission(User $user, string $permission, bool $condition = true): array + protected static function getIdsWherePermission(User $user, string $permission, bool $condition = true, bool $includePrimary, bool $includeSecondary): array { static $tags; @@ -212,7 +212,9 @@ class Tag extends AbstractModel $can = $canForTag($tag->parent); } - if ($can === $condition) { + $isPrimary = $tag->position === null && ! $tag->parent; + + if ($can === $condition && ($includePrimary && $isPrimary || $includeSecondary && ! $isPrimary)) { $ids[] = $tag->id; } } @@ -220,13 +222,13 @@ class Tag extends AbstractModel return $ids; } - public static function getIdsWhereCan(User $user, string $permission): array + public static function getIdsWhereCan(User $user, string $permission, bool $includePrimary = true, bool $includeSecondary = true): array { - return static::getIdsWherePermission($user, $permission, true); + return static::getIdsWherePermission($user, $permission, true, $includePrimary, $includeSecondary); } - public static function getIdsWhereCannot(User $user, string $permission): array + public static function getIdsWhereCannot(User $user, string $permission, bool $includePrimary = true, bool $includeSecondary = true): array { - return static::getIdsWherePermission($user, $permission, false); + return static::getIdsWherePermission($user, $permission, false, $includePrimary, $includeSecondary); } }