From 6e8884f190395b5a06fb8ebee4f8bb4e3cb8c467 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Tue, 21 Apr 2020 11:49:53 -0400 Subject: [PATCH] Implement hidden permission groups (#2129) Only users that have the new `viewHiddenGroups` permissions will be able to see these groups. You might want this when you want to give certain users special permissions, but don't want to make your authorization scheme public to regular users. Co-authored-by: luceos --- js/src/admin/components/EditGroupModal.js | 15 +++++ js/src/admin/components/PermissionGrid.js | 10 ++++ js/src/common/models/Group.js | 1 + ...change_permission_groups_add_is_hidden.php | 14 +++++ src/Api/Controller/ListGroupsController.php | 4 +- src/Api/Serializer/BasicUserSerializer.php | 6 +- src/Api/Serializer/GroupSerializer.php | 1 + src/Group/Command/CreateGroupHandler.php | 3 +- src/Group/Command/EditGroupHandler.php | 4 ++ src/Group/Group.php | 5 +- src/Group/GroupPolicy.php | 12 ++++ src/User/User.php | 5 ++ tests/integration/api/groups/ListTest.php | 57 ++++++++++++++++++- 13 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 migrations/2020_04_21_130500_change_permission_groups_add_is_hidden.php diff --git a/js/src/admin/components/EditGroupModal.js b/js/src/admin/components/EditGroupModal.js index 12d04c68d..705314fe6 100644 --- a/js/src/admin/components/EditGroupModal.js +++ b/js/src/admin/components/EditGroupModal.js @@ -3,6 +3,7 @@ import Button from '../../common/components/Button'; import Badge from '../../common/components/Badge'; import Group from '../../common/models/Group'; import ItemList from '../../common/utils/ItemList'; +import Switch from '../../common/components/Switch'; /** * The `EditGroupModal` component shows a modal dialog which allows the user @@ -16,6 +17,7 @@ export default class EditGroupModal extends Modal { this.namePlural = m.prop(this.group.namePlural() || ''); this.icon = m.prop(this.group.icon() || ''); this.color = m.prop(this.group.color() || ''); + this.isHidden = m.prop(this.group.isHidden() || false); } className() { @@ -89,6 +91,18 @@ export default class EditGroupModal extends Modal { 10 ); + items.add( + 'hidden', +
+ {Switch.component({ + state: !!Number(this.isHidden()), + children: app.translator.trans('core.admin.edit_group.hide_label'), + onchange: this.isHidden, + })} +
, + 10 + ); + items.add( 'submit',
@@ -118,6 +132,7 @@ export default class EditGroupModal extends Modal { namePlural: this.namePlural(), color: this.color(), icon: this.icon(), + isHidden: this.isHidden(), }; } diff --git a/js/src/admin/components/PermissionGrid.js b/js/src/admin/components/PermissionGrid.js index b81d92faa..b2326d6bd 100644 --- a/js/src/admin/components/PermissionGrid.js +++ b/js/src/admin/components/PermissionGrid.js @@ -112,6 +112,16 @@ export default class PermissionGrid extends Component { 100 ); + items.add( + 'viewHiddenGroups', + { + icon: 'fas fa-users', + label: app.translator.trans('core.admin.permissions.view_hidden_groups_label'), + permission: 'viewHiddenGroups', + }, + 100 + ); + items.add( 'viewUserList', { diff --git a/js/src/common/models/Group.js b/js/src/common/models/Group.js index ad3017ff8..46087032a 100644 --- a/js/src/common/models/Group.js +++ b/js/src/common/models/Group.js @@ -7,6 +7,7 @@ Object.assign(Group.prototype, { namePlural: Model.attribute('namePlural'), color: Model.attribute('color'), icon: Model.attribute('icon'), + isHidden: Model.attribute('isHidden'), }); Group.ADMINISTRATOR_ID = '1'; diff --git a/migrations/2020_04_21_130500_change_permission_groups_add_is_hidden.php b/migrations/2020_04_21_130500_change_permission_groups_add_is_hidden.php new file mode 100644 index 000000000..3b84611b9 --- /dev/null +++ b/migrations/2020_04_21_130500_change_permission_groups_add_is_hidden.php @@ -0,0 +1,14 @@ + ['boolean', 'default' => false] +]); diff --git a/src/Api/Controller/ListGroupsController.php b/src/Api/Controller/ListGroupsController.php index 62c93e27c..098f703d9 100644 --- a/src/Api/Controller/ListGroupsController.php +++ b/src/Api/Controller/ListGroupsController.php @@ -26,6 +26,8 @@ class ListGroupsController extends AbstractListController */ protected function data(ServerRequestInterface $request, Document $document) { - return Group::all(); + $actor = $request->getAttribute('actor'); + + return Group::whereVisibleTo($actor)->get(); } } diff --git a/src/Api/Serializer/BasicUserSerializer.php b/src/Api/Serializer/BasicUserSerializer.php index 851b7833b..6fb6c24d0 100644 --- a/src/Api/Serializer/BasicUserSerializer.php +++ b/src/Api/Serializer/BasicUserSerializer.php @@ -45,6 +45,10 @@ class BasicUserSerializer extends AbstractSerializer */ protected function groups($user) { - return $this->hasMany($user, GroupSerializer::class); + if ($this->getActor()->can('viewHiddenGroups')) { + return $this->hasMany($user, GroupSerializer::class); + } + + return $this->hasMany($user, GroupSerializer::class, 'visibleGroups'); } } diff --git a/src/Api/Serializer/GroupSerializer.php b/src/Api/Serializer/GroupSerializer.php index c0f743dab..7675adc46 100644 --- a/src/Api/Serializer/GroupSerializer.php +++ b/src/Api/Serializer/GroupSerializer.php @@ -52,6 +52,7 @@ class GroupSerializer extends AbstractSerializer 'namePlural' => $this->translateGroupName($group->name_plural), 'color' => $group->color, 'icon' => $group->icon, + 'isHidden' => $group->is_hidden ]; } diff --git a/src/Group/Command/CreateGroupHandler.php b/src/Group/Command/CreateGroupHandler.php index 0efd8fc8a..a55a56349 100644 --- a/src/Group/Command/CreateGroupHandler.php +++ b/src/Group/Command/CreateGroupHandler.php @@ -54,7 +54,8 @@ class CreateGroupHandler Arr::get($data, 'attributes.nameSingular'), Arr::get($data, 'attributes.namePlural'), Arr::get($data, 'attributes.color'), - Arr::get($data, 'attributes.icon') + Arr::get($data, 'attributes.icon'), + Arr::get($data, 'attributes.isHidden', false) ); $this->events->dispatch( diff --git a/src/Group/Command/EditGroupHandler.php b/src/Group/Command/EditGroupHandler.php index ef357c41e..213cafb87 100644 --- a/src/Group/Command/EditGroupHandler.php +++ b/src/Group/Command/EditGroupHandler.php @@ -74,6 +74,10 @@ class EditGroupHandler $group->icon = $attributes['icon']; } + if (isset($attributes['isHidden'])) { + $group->is_hidden = $attributes['isHidden']; + } + $this->events->dispatch( new Saving($group, $actor, $data) ); diff --git a/src/Group/Group.php b/src/Group/Group.php index bc7e8dedf..b64a1ed26 100644 --- a/src/Group/Group.php +++ b/src/Group/Group.php @@ -23,6 +23,7 @@ use Flarum\User\User; * @property string $name_plural * @property string|null $color * @property string|null $icon + * @property bool $is_hidden * @property \Illuminate\Database\Eloquent\Collection $users * @property \Illuminate\Database\Eloquent\Collection $permissions */ @@ -72,9 +73,10 @@ class Group extends AbstractModel * @param string $namePlural * @param string $color * @param string $icon + * @param bool $isHidden * @return static */ - public static function build($nameSingular, $namePlural, $color, $icon) + public static function build($nameSingular, $namePlural, $color = null, $icon = null, bool $isHidden = false): self { $group = new static; @@ -82,6 +84,7 @@ class Group extends AbstractModel $group->name_plural = $namePlural; $group->color = $color; $group->icon = $icon; + $group->is_hidden = $isHidden; $group->raise(new Created($group)); diff --git a/src/Group/GroupPolicy.php b/src/Group/GroupPolicy.php index e3f6d0874..232d8bb0d 100644 --- a/src/Group/GroupPolicy.php +++ b/src/Group/GroupPolicy.php @@ -11,6 +11,7 @@ namespace Flarum\Group; use Flarum\User\AbstractPolicy; use Flarum\User\User; +use Illuminate\Database\Eloquent\Builder; class GroupPolicy extends AbstractPolicy { @@ -30,4 +31,15 @@ class GroupPolicy extends AbstractPolicy return true; } } + + /** + * @param User $actor + * @param Builder $query + */ + public function find(User $actor, Builder $query) + { + if ($actor->cannot('viewHiddenGroups')) { + $query->where('is_hidden', false); + } + } } diff --git a/src/User/User.php b/src/User/User.php index 197657447..8fc28e74c 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -606,6 +606,11 @@ class User extends AbstractModel return $this->belongsToMany(Group::class); } + public function visibleGroups() + { + return $this->belongsToMany(Group::class)->where('is_hidden', false); + } + /** * Define the relationship with the user's notifications. * diff --git a/tests/integration/api/groups/ListTest.php b/tests/integration/api/groups/ListTest.php index 591df53ca..ddcc47e96 100644 --- a/tests/integration/api/groups/ListTest.php +++ b/tests/integration/api/groups/ListTest.php @@ -9,15 +9,37 @@ namespace Flarum\Tests\integration\api\groups; -use Flarum\Group\Group; +use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Illuminate\Support\Arr; class ListTest extends TestCase { + use RetrievesAuthorizedUsers; + + public function setUp() + { + parent::setUp(); + + $this->prepareDatabase([ + 'users' => [ + $this->adminUser(), + $this->normalUser(), + ], + 'groups' => [ + $this->adminGroup(), + $this->hiddenGroup() + ], + 'group_user' => [ + ['user_id' => 1, 'group_id' => 1], + ], + ]); + } + /** * @test */ - public function shows_index_for_guest() + public function shows_limited_index_for_guest() { $response = $this->send( $this->request('GET', '/api/groups') @@ -26,6 +48,35 @@ class ListTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals(Group::count(), count($data['data'])); + $this->assertEquals(['1'], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function shows_index_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/groups', [ + 'authenticatedAs' => 1, + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + $this->assertEquals(['1', '10'], Arr::pluck($data['data'], 'id')); + } + + protected function hiddenGroup(): array + { + return [ + 'id' => 10, + 'name_singular' => 'Hidden', + 'name_plural' => 'Ninjas', + 'color' => null, + 'icon' => 'fas fa-wrench', + 'is_hidden' => 1 + ]; } }