From b3961d6e9146166247d40febb708a6026bbdf1c4 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 26 Jan 2021 18:55:52 -0500 Subject: [PATCH 1/5] Require viewUserList to get user by ID This makes it more difficult to scape the API for users --- src/Api/Controller/ShowUserController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Api/Controller/ShowUserController.php b/src/Api/Controller/ShowUserController.php index 60412362a4..420821675c 100644 --- a/src/Api/Controller/ShowUserController.php +++ b/src/Api/Controller/ShowUserController.php @@ -61,6 +61,7 @@ protected function data(ServerRequestInterface $request, Document $document) if (Arr::get($request->getQueryParams(), 'bySlug', false)) { $user = $this->slugManager->forResource(User::class)->fromSlug($id, $actor); } else { + $actor->assertCan('viewUserList'); $user = $this->users->findOrFail($id, $actor); } From 14416a9d329b4de9f52652cba62318813340f7b6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 27 Jan 2021 00:14:22 -0500 Subject: [PATCH 2/5] Modify test cases appropriately --- tests/integration/api/users/ShowTest.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/integration/api/users/ShowTest.php b/tests/integration/api/users/ShowTest.php index 8e6cbb2151..d54d7516ce 100644 --- a/tests/integration/api/users/ShowTest.php +++ b/tests/integration/api/users/ShowTest.php @@ -40,6 +40,14 @@ private function forbidMembersFromSearchingUsers() $this->database()->table('group_permission')->where('permission', 'viewUserList')->where('group_id', 3)->delete(); } + private function allowGuestsToSearchUsers() + { + $this->database()->table('group_permission')->insert([ + 'permission' => 'viewUserList', + 'group_id' => 3 + ]); + } + /** * @test */ @@ -73,13 +81,13 @@ public function admin_can_see_user_via_slug() /** * @test */ - public function guest_can_see_user_by_default() + public function guest_cant_see_user_by_id_default() { $response = $this->send( $this->request('GET', '/api/users/2') ); - $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals(403, $response->getStatusCode()); } /** @@ -99,9 +107,9 @@ public function guest_can_see_user_by_slug_by_default() /** * @test */ - public function guest_cant_see_user_if_blocked() + public function guest_can_see_user_by_id_if_allowed() { - $this->forbidGuestsFromSeeingForum(); + $this->allowGuestsToSearchUsers(); $response = $this->send( $this->request('GET', '/api/users/2') From 189b98237b18064275b29dfc8f36207d2146725d Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 27 Jan 2021 00:20:02 -0500 Subject: [PATCH 3/5] Remove redundant insert --- tests/integration/api/users/ShowTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/integration/api/users/ShowTest.php b/tests/integration/api/users/ShowTest.php index d54d7516ce..b816b03cf1 100644 --- a/tests/integration/api/users/ShowTest.php +++ b/tests/integration/api/users/ShowTest.php @@ -40,14 +40,6 @@ private function forbidMembersFromSearchingUsers() $this->database()->table('group_permission')->where('permission', 'viewUserList')->where('group_id', 3)->delete(); } - private function allowGuestsToSearchUsers() - { - $this->database()->table('group_permission')->insert([ - 'permission' => 'viewUserList', - 'group_id' => 3 - ]); - } - /** * @test */ @@ -109,8 +101,6 @@ public function guest_can_see_user_by_slug_by_default() */ public function guest_can_see_user_by_id_if_allowed() { - $this->allowGuestsToSearchUsers(); - $response = $this->send( $this->request('GET', '/api/users/2') ); From ec4fafb5516724cf86d303b7c1f0241a0a6ba32c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 27 Jan 2021 00:24:08 -0500 Subject: [PATCH 4/5] Check against proper error code --- tests/integration/api/users/ShowTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/api/users/ShowTest.php b/tests/integration/api/users/ShowTest.php index b816b03cf1..9ccfe4b00c 100644 --- a/tests/integration/api/users/ShowTest.php +++ b/tests/integration/api/users/ShowTest.php @@ -105,7 +105,7 @@ public function guest_can_see_user_by_id_if_allowed() $this->request('GET', '/api/users/2') ); - $this->assertEquals(404, $response->getStatusCode()); + $this->assertEquals(403, $response->getStatusCode()); } /** From c72d078ef1e0bf4d5ab6101468f7737638f2c90e Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sun, 4 Apr 2021 16:21:28 -0400 Subject: [PATCH 5/5] Fix guest allowed to see by id test --- tests/integration/api/users/ShowTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/integration/api/users/ShowTest.php b/tests/integration/api/users/ShowTest.php index 9ccfe4b00c..58c71b17d5 100644 --- a/tests/integration/api/users/ShowTest.php +++ b/tests/integration/api/users/ShowTest.php @@ -101,11 +101,17 @@ public function guest_can_see_user_by_slug_by_default() */ public function guest_can_see_user_by_id_if_allowed() { + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ] + ]); + $response = $this->send( $this->request('GET', '/api/users/2') ); - $this->assertEquals(403, $response->getStatusCode()); + $this->assertEquals(200, $response->getStatusCode()); } /**