Skip to content

Commit 267bfd2

Browse files
AndrewKostkaAndrewKostka
committed
Refactor routes to use LimitWikiAccess middleware
1 parent 5f9bebe commit 267bfd2

7 files changed

Lines changed: 32 additions & 77 deletions

File tree

‎app/Http/Controllers/WikiController.php‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -200,22 +200,8 @@ public function create(Request $request): \Illuminate\Http\Response
200200

201201
public function delete(Request $request): \Illuminate\Http\JsonResponse
202202
{
203-
$user = $request->user();
204-
205-
$request->validate([
206-
'wiki' => 'required|numeric',
207-
]);
208-
209-
$wikiId = $request->input('wiki');
210-
$userId = $user->id;
211203
$wikiDeletionReason = $request->input('deletionReasons');
212-
213-
// Check that the requesting user manages the wiki
214-
if (WikiManager::where('user_id', $userId)->where('wiki_id', $wikiId)->count() !== 1) {
215-
// The deletion was requested by a user that does not manage the wiki
216-
return response()->json('Unauthorized', 401);
217-
}
218-
$wiki = Wiki::find($wikiId);
204+
$wiki = $request->attributes->get('wiki');
219205

220206
if(isset($wikiDeletionReason)){
221207
//Save the wiki deletion reason
@@ -231,25 +217,11 @@ public function delete(Request $request): \Illuminate\Http\JsonResponse
231217
// TODO should this just be get wiki?
232218
public function getWikiDetailsForIdForOwner(Request $request): \Illuminate\Http\Response
233219
{
234-
$user = $request->user();
235-
236-
$wikiId = $request->input('wiki');
237-
238-
// TODO general check to make sure current user can manage the wiki
239-
// this should probably be middle ware?
240-
// TODO only do 1 query where instead of 2?
241-
$test = WikiManager::where('user_id', $user->id)
242-
->where('wiki_id', $wikiId)
243-
->first();
244-
if (! $test) {
245-
abort(403);
246-
}
247-
248-
$wiki = Wiki::where('id', $wikiId)
249-
->with('wikiManagers')
250-
->with('wikiDbVersion')
251-
->with('wikiLatestProfile')
252-
->with('publicSettings')->first();
220+
$wiki = $request->attributes->get('wiki')
221+
->with('wikiManagers')
222+
->with('wikiDbVersion')
223+
->with('wikiLatestProfile')
224+
->with('publicSettings')->first();
253225

254226
$res = [
255227
'success' => true,

‎app/Http/Controllers/WikiLogoController.php‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,15 @@ class WikiLogoController extends Controller
2020
public function update(Request $request)
2121
{
2222
$request->validate([
23-
'wiki' => 'required|numeric',
2423
'logo' => 'required|mimes:png',
2524
]);
2625

27-
$user = $request->user();
28-
29-
$wikiId = $request->input('wiki');
30-
$userId = $user->id;
31-
32-
// Check that the requesting user manages the wiki
33-
// TODO turn this into a generic guard for all of these types of routes...
34-
if (WikiManager::where('user_id', $userId)->where('wiki_id', $wikiId)->count() !== 1) {
35-
// The logo update was requested by a user that does not manage the wiki
36-
return response()->json('Unauthorized', 401);
37-
}
26+
$wiki = $request->attributes->get('wiki');
3827

3928
// run the job to set the wiki logo
40-
( new SetWikiLogo('id', $wikiId, $request->file('logo')->getRealPath()) )->handle();
29+
( new SetWikiLogo('id', $wiki->id, $request->file('logo')->getRealPath()) )->handle();
4130

4231
// get the logo URL from the settings
43-
$wiki = Wiki::find($wikiId);
4432
$wgLogoSetting = $wiki->settings()->firstWhere(['name' => WikiSetting::wgLogo])->value;
4533

4634
$res['success'] = true;

‎app/Http/Controllers/WikiSettingController.php‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,29 +40,18 @@ public function update($setting, Request $request)
4040
$settingValidations = $this->getSettingValidations();
4141

4242
$request->validate([
43-
'wiki' => 'required|numeric',
4443
// Allow both internal and external setting names, see normalizeSetting
4544
'setting' => 'required|string|in:'.implode(',', array_keys($settingValidations)),
4645
]);
4746
$settingName = $request->input('setting');
4847

4948
$request->validate(['value' => $settingValidations[$settingName]]);
5049
$value = $request->input('value');
51-
52-
$user = $request->user();
53-
$wikiId = $request->input('wiki');
54-
$userId = $user->id;
55-
56-
// Check that the requesting user manages the wiki
57-
// TODO turn this into a generic guard for all of these types of routes...
58-
if (WikiManager::where('user_id', $userId)->where('wiki_id', $wikiId)->count() !== 1) {
59-
// The deletion was requested by a user that does not manage the wiki
60-
return response()->json('Unauthorized', 401);
61-
}
50+
$wiki = $request->attributes->get('wiki');
6251

6352
WikiSetting::updateOrCreate(
6453
[
65-
'wiki_id' => $wikiId,
54+
'wiki_id' => $wiki->id,
6655
'name' => $settingName,
6756
],
6857
[

‎routes/api.php‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,19 @@
3838
});
3939
$router->group(['prefix' => 'wiki', 'middleware' => ['verified']], function () use ($router) {
4040
$router->post('create', ['uses' => 'WikiController@create']);
41-
$router->post('delete', ['uses' => 'WikiController@delete']);
42-
$router->post('details', ['uses' => 'WikiController@getWikiDetailsForIdForOwner']);
43-
$router->get('details', ['uses' => 'WikiController@getWikiDetailsForIdForOwner']);
44-
$router->post('logo/update', ['uses' => 'WikiLogoController@update']);
45-
$router->post('setting/{setting}/update', ['uses' => 'WikiSettingController@update']);
46-
// TODO should wiki managers really be here?
47-
$router->post('managers/list', ['uses' => 'WikiManagersController@getManagersOfWiki']);
48-
$router->get('entityImport', ['middleware' => 'limit_wiki_access', 'uses' => 'WikiEntityImportController@get']);
49-
$router->post('entityImport', ['middleware' => 'limit_wiki_access', 'uses' => 'WikiEntityImportController@create']);
50-
$router->post('profile', ['middleware' => 'limit_wiki_access', 'uses' => 'WikiProfileController@create']);
41+
42+
$router->group(['middleware' => 'limit_wiki_access'], function () use ($router) {
43+
$router->post('delete', ['uses' => 'WikiController@delete']);
44+
$router->post('details', ['uses' => 'WikiController@getWikiDetailsForIdForOwner']);
45+
$router->get('details', ['uses' => 'WikiController@getWikiDetailsForIdForOwner']);
46+
$router->post('logo/update', ['uses' => 'WikiLogoController@update']);
47+
$router->post('setting/{setting}/update', ['uses' => 'WikiSettingController@update']);
48+
// TODO should wiki managers really be here?
49+
$router->post('managers/list', ['uses' => 'WikiManagersController@getManagersOfWiki']);
50+
$router->get('entityImport', ['uses' => 'WikiEntityImportController@get']);
51+
$router->post('entityImport', ['uses' => 'WikiEntityImportController@create']);
52+
$router->post('profile', ['uses' => 'WikiProfileController@create']);
53+
});
5154
});
5255
$router->apiResource('deletedWikiMetrics', 'DeletedWikiMetricsController')->only(['index'])
5356
->middleware(AuthorisedUsersForDeletedWikiMetricsMiddleware::class);

‎tests/Routes/Wiki/DeleteWikiTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ public function testFailOnWrongWikiManager(): void
4343
WikiManager::factory()->create(['wiki_id' => $userWiki->id, 'user_id' => $user->id]);
4444
$this->actingAs($user, 'api')
4545
->post('wiki/delete', ['wiki' => $otherWiki->id])
46-
->assertStatus(401);
46+
->assertStatus(403);
4747
}
4848
}

‎tests/Routes/Wiki/LogoUpdateTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,6 @@ public function testFailOnWrongWikiManager(): void
6565
->createWithContent("logo_200x200.png", file_get_contents(__DIR__ . "/../../data/logo_200x200.png"));
6666
$this->actingAs($user, 'api')
6767
->post('wiki/logo/update', ['wiki' => $otherWiki->id, 'logo' => $file])
68-
->assertStatus(401);
68+
->assertStatus(403);
6969
}
7070
}

‎tests/Routes/Wiki/SettingUpdateTest.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ class SettingUpdateTest extends TestCase
2626
public function testSetInvalidSetting()
2727
{
2828
$settingName = 'iDoNotExistAsASetting';
29-
29+
$wiki = Wiki::factory()->create();
3030
$user = User::factory()->create(['verified' => true]);
31+
WikiManager::factory()->create(['wiki_id' => $wiki->id, 'user_id' => $user->id]);
3132
$this->actingAs($user, 'api')
3233
->json('POST', str_replace('foo', $settingName, $this->route), [
33-
'wiki' => 1,
34+
'wiki' => $wiki->id,
3435
'setting' => $settingName,
3536
'value' => '1',
3637
])
@@ -51,7 +52,7 @@ public function testFailOnWrongWikiManager(): void
5152
'setting' => $settingName,
5253
'value' => '1'
5354
])
54-
->assertStatus(401);
55+
->assertStatus(403);
5556
}
5657

5758
static public function provideValidSettings()
@@ -156,11 +157,13 @@ static public function provideValidSettingsBadValues()
156157
*/
157158
public function testValidSettingBadValues($settingName, $settingValue)
158159
{
160+
$wiki = Wiki::factory()->create();
159161
$user = User::factory()->create(['verified' => true]);
162+
WikiManager::factory()->create(['wiki_id' => $wiki->id, 'user_id' => $user->id]);
160163

161164
$this->actingAs($user, 'api')
162165
->json('POST', str_replace('foo', $settingName, $this->route), [
163-
'wiki' => 1,
166+
'wiki' => $wiki->id,
164167
'setting' => $settingName,
165168
'value' => $settingValue,
166169
])

0 commit comments

Comments
 (0)