Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permissions Cleanup: Core #2212

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion js/src/admin/AdminApplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default class AdminApplication extends Application {
const required = [];

if (permission === 'startDiscussion' || permission.indexOf('discussion.') === 0) {
required.push('viewDiscussions');
required.push('viewForum');
}
if (permission === 'discussion.delete') {
required.push('discussion.hide');
Expand Down
12 changes: 6 additions & 6 deletions js/src/admin/components/PermissionGrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ export default class PermissionGrid extends Component {
const items = new ItemList();

items.add(
'viewDiscussions',
'viewForum',
{
icon: 'fas fa-eye',
label: app.translator.trans('core.admin.permissions.view_discussions_label'),
permission: 'viewDiscussions',
label: app.translator.trans('core.admin.permissions.view_forum_label'),
permission: 'viewForum',
allowGuest: true,
},
100
Expand All @@ -123,11 +123,11 @@ export default class PermissionGrid extends Component {
);

items.add(
'viewUserList',
'searchUsers',
{
icon: 'fas fa-users',
label: app.translator.trans('core.admin.permissions.view_user_list_label'),
permission: 'viewUserList',
label: app.translator.trans('core.admin.permissions.search_users_label'),
permission: 'searchUsers',
allowGuest: true,
},
100
Expand Down
4 changes: 2 additions & 2 deletions js/src/forum/components/Search.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ export default class Search extends Component {
sourceItems() {
const items = new ItemList();

if (app.forum.attribute('canViewDiscussions')) items.add('discussions', new DiscussionsSearchSource());
if (app.forum.attribute('canViewUserList')) items.add('users', new UsersSearchSource());
if (app.forum.attribute('canViewForum')) items.add('discussions', new DiscussionsSearchSource());
if (app.forum.attribute('canSearchUsers')) items.add('users', new UsersSearchSource());

return items;
}
Expand Down
26 changes: 26 additions & 0 deletions migrations/2020_06_27_000000_rename_permissions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

use Illuminate\Database\Schema\Builder;

return [
'up' => function (Builder $schema) {
$db = $schema->getConnection();

$db->table('group_permission')->where('permission', 'viewDiscussions')->update(['permission' => 'viewForum']);
$db->table('group_permission')->where('permission', 'viewUserList')->update(['permission' => 'searchUsers']);
},

'down' => function (Builder $schema) {
$db = $schema->getConnection();

$db->table('group_permission')->where('permission', 'viewForum')->update(['permission' => 'viewDiscussions']);
$db->table('group_permission')->where('permission', 'searchUsers')->update(['permission' => 'viewUserList']);
}
];
2 changes: 1 addition & 1 deletion src/Api/Controller/ListUsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ protected function data(ServerRequestInterface $request, Document $document)
{
$actor = $request->getAttribute('actor');

$this->assertCan($actor, 'viewUserList');
$this->assertCan($actor, 'searchUsers');

$query = Arr::get($this->extractFilter($request), 'q');
$sort = $this->extractSort($request);
Expand Down
4 changes: 2 additions & 2 deletions src/Api/Serializer/ForumSerializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ protected function getDefaultAttributes($model)
'footerHtml' => $this->settings->get('custom_footer'),
'allowSignUp' => (bool) $this->settings->get('allow_sign_up'),
'defaultRoute' => $this->settings->get('default_route'),
'canViewDiscussions' => $this->actor->can('viewDiscussions'),
'canViewForum' => $this->actor->can('viewForum'),
'canStartDiscussion' => $this->actor->can('startDiscussion'),
'canViewUserList' => $this->actor->can('viewUserList')
'canSearchUsers' => $this->actor->can('searchUsers')
];

if ($this->actor->can('administrate')) {
Expand Down
2 changes: 1 addition & 1 deletion src/Discussion/DiscussionPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function can(User $actor, $ability)
*/
public function find(User $actor, Builder $query)
{
if ($actor->cannot('viewDiscussions')) {
if ($actor->cannot('viewForum')) {
$query->whereRaw('FALSE');

return;
Expand Down
2 changes: 1 addition & 1 deletion src/User/UserPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function can(User $actor, $ability)
*/
public function find(User $actor, Builder $query)
{
if ($actor->cannot('viewUserList')) {
if ($actor->cannot('viewForum')) {
if ($actor->isGuest()) {
$query->whereRaw('FALSE');
} else {
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/api/authentication/WithApiKeyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function cannot_authorize_without_key()
);

$data = json_decode($response->getBody(), true);
$this->assertFalse($data['data']['attributes']['canViewUserList']);
$this->assertFalse($data['data']['attributes']['canSearchUsers']);
}

/**
Expand All @@ -69,7 +69,7 @@ public function master_token_can_authenticate_as_anyone()
);

$data = json_decode($response->getBody(), true);
$this->assertTrue($data['data']['attributes']['canViewUserList']);
$this->assertTrue($data['data']['attributes']['canSearchUsers']);
$this->assertArrayHasKey('adminUrl', $data['data']['attributes']);

$key->refresh();
Expand All @@ -90,7 +90,7 @@ public function personal_api_token_cannot_authenticate_as_anyone()
);

$data = json_decode($response->getBody(), true);
$this->assertTrue($data['data']['attributes']['canViewUserList']);
$this->assertTrue($data['data']['attributes']['canSearchUsers']);
$this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']);

$key->refresh();
Expand All @@ -111,7 +111,7 @@ public function personal_api_token_authenticates_user()
);

$data = json_decode($response->getBody(), true);
$this->assertTrue($data['data']['attributes']['canViewUserList']);
$this->assertTrue($data['data']['attributes']['canSearchUsers']);
$this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']);

$key->refresh();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected function setUp(): void
['user_id' => 1, 'group_id' => 1],
],
'group_permission' => [
['permission' => 'viewUserList', 'group_id' => 3],
['permission' => 'searchUsers', 'group_id' => 3],
],
'api_keys' => [
['user_id' => 1, 'key' => 'superadmin'],
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/api/discussions/ListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected function setUp(): void
$this->guestGroup(),
],
'group_permission' => [
['permission' => 'viewDiscussions', 'group_id' => 2],
['permission' => 'viewForum', 'group_id' => 2],
]
]);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/api/discussions/ShowTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ protected function setUp(): void
['user_id' => 2, 'group_id' => 3],
],
'group_permission' => [
['permission' => 'viewDiscussions', 'group_id' => 2],
['permission' => 'viewDiscussions', 'group_id' => 3],
['permission' => 'viewForum', 'group_id' => 2],
['permission' => 'viewForum', 'group_id' => 3],
]
]);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/api/posts/CreateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected function setUp(): void
['user_id' => 2, 'group_id' => 3],
],
'group_permission' => [
['permission' => 'viewDiscussions', 'group_id' => 3],
['permission' => 'viewForum', 'group_id' => 3],
]
]);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/api/users/ListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function shows_index_for_guest_when_they_have_permission()
{
Permission::unguarded(function () {
Permission::create([
'permission' => 'viewUserList',
'permission' => 'searchUsers',
'group_id' => 2,
]);
});
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/api/users/UpdateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected function setUp(): void
['user_id' => 2, 'group_id' => 3],
],
'group_permission' => [
['permission' => 'viewUserList', 'group_id' => 3],
['permission' => 'searchUsers', 'group_id' => 3],
]
]);
}
Expand Down Expand Up @@ -69,7 +69,7 @@ public function users_can_not_see_other_users_private_information()
);

// Make sure sensitive information is not made public
$this->assertEquals(200, $response->getStatusCode());
$this->assertEquals(404, $response->getStatusCode());
$this->assertNotContains('[email protected]', (string) $response->getBody());
}
}