From 11b380c893f5883a81420f2ef8a450619b4986e0 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Mon, 7 Nov 2016 22:22:19 +1030 Subject: [PATCH] Fix permission logic regressions Make sure permissions that lie "dormant" in the database don't interfere with the global permissions; actually check each tag's permissions rather than using `hasPermissionLike`. closes flarum/core#1058 closes flarum/core#1062 --- extensions/tags/src/Access/GlobalPolicy.php | 5 +++-- extensions/tags/src/Access/TagPolicy.php | 2 +- extensions/tags/src/Listener/SaveTagsToDatabase.php | 7 +++++-- extensions/tags/src/Tag.php | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/extensions/tags/src/Access/GlobalPolicy.php b/extensions/tags/src/Access/GlobalPolicy.php index 328e30af3..006faff95 100644 --- a/extensions/tags/src/Access/GlobalPolicy.php +++ b/extensions/tags/src/Access/GlobalPolicy.php @@ -11,6 +11,7 @@ namespace Flarum\Tags\Access; use Flarum\Event\GetPermission; +use Flarum\Tags\Tag; use Illuminate\Contracts\Events\Dispatcher; class GlobalPolicy @@ -29,8 +30,8 @@ class GlobalPolicy */ public function grantGlobalDiscussionPermissions(GetPermission $event) { - if (in_array($event->ability, ['viewDiscussions', 'startDiscussion']) && empty($event->arguments)) { - return $event->actor->hasPermissionLike($event->ability); + if (in_array($event->ability, ['viewDiscussions', 'startDiscussion']) && is_null($event->model)) { + return ! empty(Tag::getIdsWhereCan($event->actor, $event->ability)); } } } diff --git a/extensions/tags/src/Access/TagPolicy.php b/extensions/tags/src/Access/TagPolicy.php index 49ad05abd..3591eb5e4 100755 --- a/extensions/tags/src/Access/TagPolicy.php +++ b/extensions/tags/src/Access/TagPolicy.php @@ -39,7 +39,7 @@ class TagPolicy extends AbstractPolicy public function startDiscussion(User $actor, Tag $tag) { if ((! $tag->is_restricted && $actor->hasPermission('startDiscussion')) - || $actor->hasPermission('tag'.$tag->id.'.startDiscussion')) { + || ($tag->is_restricted && $actor->hasPermission('tag'.$tag->id.'.startDiscussion'))) { return true; } } diff --git a/extensions/tags/src/Listener/SaveTagsToDatabase.php b/extensions/tags/src/Listener/SaveTagsToDatabase.php index 8d726a413..0d37c78b5 100755 --- a/extensions/tags/src/Listener/SaveTagsToDatabase.php +++ b/extensions/tags/src/Listener/SaveTagsToDatabase.php @@ -65,10 +65,11 @@ class SaveTagsToDatabase */ public function whenDiscussionWillBeSaved(DiscussionWillBeSaved $event) { + $discussion = $event->discussion; + $actor = $event->actor; + // TODO: clean up, prevent discussion from being created without tags if (isset($event->data['relationships']['tags']['data'])) { - $discussion = $event->discussion; - $actor = $event->actor; $linkage = (array) $event->data['relationships']['tags']['data']; $newTagIds = []; @@ -117,6 +118,8 @@ class SaveTagsToDatabase $discussion->afterSave(function ($discussion) use ($newTagIds) { $discussion->tags()->sync($newTagIds); }); + } elseif (! $discussion->exists && ! $actor->hasPermission('startDiscussion')) { + throw new PermissionDeniedException; } } diff --git a/extensions/tags/src/Tag.php b/extensions/tags/src/Tag.php index 56336689a..d880d2029 100644 --- a/extensions/tags/src/Tag.php +++ b/extensions/tags/src/Tag.php @@ -137,7 +137,7 @@ class Tag extends AbstractModel $hasGlobalPermission = $user->hasPermission($permission); $canForTag = function (Tag $tag) use ($user, $permission, $hasGlobalPermission) { - return ($hasGlobalPermission && ! $tag->is_restricted) || $user->hasPermission('tag'.$tag->id.'.'.$permission); + return ($hasGlobalPermission && ! $tag->is_restricted) || ($tag->is_restricted && $user->hasPermission('tag'.$tag->id.'.'.$permission)); }; foreach ($tags as $tag) {