From 9c47ccd1fde173b67a9ba9cd97301da3d4dabf3c Mon Sep 17 00:00:00 2001 From: Blake Payne Date: Thu, 4 Mar 2021 15:08:12 +0000 Subject: [PATCH] =?UTF-8?q?Updated=20GroupFilterGambit=20to=20prevent=20hi?= =?UTF-8?q?dden=20groups=20being=20visible=20wher=E2=80=A6=20(#2657)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated GroupFilterGambit to prevent hidden groups being visible where they shouldn't be and to ensure that only the selected groups are returned on a search. Fixes #2559 --- .../core/src/User/Query/GroupFilterGambit.php | 10 +- .../integration/api/users/GroupSearchTest.php | 328 ++++++++++++++++++ 2 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 framework/core/tests/integration/api/users/GroupSearchTest.php diff --git a/framework/core/src/User/Query/GroupFilterGambit.php b/framework/core/src/User/Query/GroupFilterGambit.php index 9d419a7b9..ec34912da 100644 --- a/framework/core/src/User/Query/GroupFilterGambit.php +++ b/framework/core/src/User/Query/GroupFilterGambit.php @@ -51,14 +51,20 @@ class GroupFilterGambit extends AbstractRegexGambit implements FilterInterface $groupQuery = Group::whereVisibleTo($actor); + $ids = []; + $names = []; foreach ($groupIdentifiers as $identifier) { if (is_numeric($identifier)) { - $groupQuery->orWhere('id', $identifier); + $ids[] = $identifier; } else { - $groupQuery->orWhere('name_singular', $identifier)->orWhere('name_plural', $identifier); + $names[] = $identifier; } } + $groupQuery->whereIn('id', $ids) + ->orWhereIn('name_singular', $names) + ->orWhereIn('name_plural', $names); + $userIds = $groupQuery->join('group_user', 'groups.id', 'group_user.group_id') ->pluck('group_user.user_id') ->all(); diff --git a/framework/core/tests/integration/api/users/GroupSearchTest.php b/framework/core/tests/integration/api/users/GroupSearchTest.php new file mode 100644 index 000000000..bc6accda5 --- /dev/null +++ b/framework/core/tests/integration/api/users/GroupSearchTest.php @@ -0,0 +1,328 @@ +prepareDatabase([ + 'users' => [ + $this->normalUser(), + ], + ]); + } + + /** + * @test + */ + public function disallows_group_filter_for_user_without_permission() + { + $response = $this->createRequest(['admin']); + + $this->assertEquals(403, $response->getStatusCode()); + } + + /** + * @test + */ + public function allows_group_filter_for_admin() + { + $response = $this->createRequest(['admin'], 1); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function allows_group_filter_for_user_with_permission() + { + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ], + ]); + $response = $this->createRequest(['admin'], 2); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function non_admin_gets_correct_results() + { + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ], + ]); + + $response = $this->createRequest(['admin'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['mod'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + + $response = $this->createRequest(['admins'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['mods'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + + $response = $this->createRequest(['1'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['4'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + } + + /** + * @test + */ + public function non_admin_cannot_see_hidden_groups() + { + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ], + ]); + + $this->createHiddenUser(); + $response = $this->createRequest(['99'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + } + + /** + * @test + */ + public function non_admin_can_select_multiple_groups_but_not_hidden() + { + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ], + ]); + $this->createMultipleUsersAndGroups(); + $response = $this->createRequest(['1', '4', '5', '6', '99'], 2); + $responseBodyContents = json_decode($response->getBody()->getContents()); + $this->assertCount(4, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(4, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + $this->assertEquals(4, $responseBodyContents->included[1]->id); + $this->assertEquals(5, $responseBodyContents->included[2]->id); + $this->assertEquals(6, $responseBodyContents->included[3]->id); + } + + /** + * @test + */ + public function admin_gets_correct_results_group() + { + $response = $this->createRequest(['admin'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['mod'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + + $response = $this->createRequest(['admins'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['mods'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + + $response = $this->createRequest(['1'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + + $response = $this->createRequest(['4'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(0, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertObjectNotHasAttribute('included', $responseBodyContents, json_encode($responseBodyContents)); + } + + /** + * @test + */ + public function admin_can_see_hidden_groups() + { + $this->createHiddenUser(); + $response = $this->createRequest(['99'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + + $this->assertCount(1, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(1, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(99, $responseBodyContents->included[0]->id); + } + + /** + * @test + */ + public function admin_can_select_multiple_groups_and_hidden() + { + $this->createMultipleUsersAndGroups(); + $this->createHiddenUser(); + $response = $this->createRequest(['1', '4', '5', '6', '99'], 1); + $responseBodyContents = json_decode($response->getBody()->getContents()); + $this->assertCount(5, $responseBodyContents->data, json_encode($responseBodyContents)); + $this->assertCount(5, $responseBodyContents->included, json_encode($responseBodyContents)); + $this->assertEquals(1, $responseBodyContents->included[0]->id); + $this->assertEquals(99, $responseBodyContents->included[1]->id); + $this->assertEquals(4, $responseBodyContents->included[2]->id); + $this->assertEquals(5, $responseBodyContents->included[3]->id); + $this->assertEquals(6, $responseBodyContents->included[4]->id); + } + + private function createRequest(array $group, int $userId = null) + { + $auth = $userId ? ['authenticatedAs' => $userId] : []; + + return $this->send( + $this->request('GET', '/api/users', $auth) + ->withQueryParams(['filter' => ['q' => 'group:'.implode(',', $group)]]) + ); + } + + private function createMultipleUsersAndGroups() + { + $this->prepareDatabase([ + 'users' => [ + [ + 'id' => 4, + 'username' => 'normal4', + 'password' => '$2y$10$LO59tiT7uggl6Oe23o/O6.utnF6ipngYjvMvaxo1TciKqBttDNKim', // BCrypt hash for "too-obscure" + 'email' => 'normal4@machine.local', + 'is_email_confirmed' => 1, + ], + [ + 'id' => 5, + 'username' => 'normal5', + 'password' => '$2y$10$LO59tiT7uggl6Oe23o/O6.utnF6ipngYjvMvaxo1TciKqBttDNKim', // BCrypt hash for "too-obscure" + 'email' => 'normal5@machine.local', + 'is_email_confirmed' => 1, + ], + [ + 'id' => 6, + 'username' => 'normal6', + 'password' => '$2y$10$LO59tiT7uggl6Oe23o/O6.utnF6ipngYjvMvaxo1TciKqBttDNKim', // BCrypt hash for "too-obscure" + 'email' => 'normal6@machine.local', + 'is_email_confirmed' => 1, + ], + ], + 'groups' => [ + [ + 'id' => 5, + 'name_singular' => 'test1 user', + 'name_plural' => 'test1 users', + 'is_hidden' => false + ], + [ + 'id' => 6, + 'name_singular' => 'test2 user', + 'name_plural' => 'test2 users', + 'is_hidden' => false + ] + ], + 'group_user' => [ + [ + 'user_id' => 4, + 'group_id' => 4 + ], + [ + 'user_id' => 5, + 'group_id' => 5 + ], + [ + 'user_id' => 6, + 'group_id' => 6 + ], + ], + ]); + } + + private function createHiddenUser() + { + $this->prepareDatabase([ + 'users' => [ + [ + 'id' => 3, + 'username' => 'normal2', + 'password' => '$2y$10$LO59tiT7uggl6Oe23o/O6.utnF6ipngYjvMvaxo1TciKqBttDNKim', // BCrypt hash for "too-obscure" + 'email' => 'normal2@machine.local', + 'is_email_confirmed' => 1, + ], + ], + 'groups' => [ + [ + 'id' => 99, + 'name_singular' => 'hidden user', + 'name_plural' => 'hidden users', + 'is_hidden' => true + ], + ], + 'group_user' => [ + [ + 'user_id' => 3, + 'group_id' => 99 + ] + ], + ]); + } +}