Skip to content

Commit 2d5a3f4

Browse files
committed
Refactor to a 400 response instead of throwing an exception
If we were to throw an exception it would likely get reported to Sentry
1 parent d022c7f commit 2d5a3f4

File tree

3 files changed

+52
-26
lines changed

3 files changed

+52
-26
lines changed

src/Exceptions/ForbidenRequestParamException.php

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/Http/Middleware/AuthenticateApiKey.php

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
namespace Givebutter\LaravelKeyable\Http\Middleware;
44

55
use Closure;
6-
use Givebutter\LaravelKeyable\Exceptions\ForbidenRequestParamException;
76
use Givebutter\LaravelKeyable\Models\ApiKey;
8-
use Illuminate\Http\Request;
97

108
class AuthenticateApiKey
119
{
@@ -20,7 +18,22 @@ class AuthenticateApiKey
2018
*/
2119
public function handle($request, Closure $next, $guard = null)
2220
{
23-
$this->checkForbidenRequestParams($request);
21+
$forbidenRequestParams = ['apiKey', 'keyable'];
22+
23+
// Check if request has forbidden params
24+
foreach ($forbidenRequestParams as $param) {
25+
if ($request->missing($param)) {
26+
continue;
27+
}
28+
29+
$message = "Request param '{$param}' is not allowed.";
30+
31+
if ($request->wantsJson()) {
32+
return response()->json(['message' => $message], 400);
33+
}
34+
35+
return response($message, 400);
36+
}
2437

2538
//Get API token from request
2639
$token = $this->getKeyFromRequest($request);
@@ -90,17 +103,4 @@ protected function unauthorizedResponse()
90103
],
91104
], 401);
92105
}
93-
94-
private function checkForbidenRequestParams(Request $request): void
95-
{
96-
$forbidenParams = ['apiKey', 'keyable'];
97-
98-
foreach ($forbidenParams as $forbidenParam) {
99-
if ($request->missing($forbidenParam)) {
100-
continue;
101-
}
102-
103-
throw new ForbidenRequestParamException("Request param '{$forbidenParam}' is not allowed.");
104-
}
105-
}
106106
}

tests/Feature/AuthenticateApiKey.php

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ public function throw_exception_if_unauthorized_get_request_has_forbidden_reques
9393
return response('All good', 200);
9494
})->middleware(['api', 'auth.apikey']);
9595

96-
$this->get("/api/posts?{$queryParam}=value")->assertInternalServerError();
96+
$this->get("/api/posts?{$queryParam}=value")
97+
->assertBadRequest()
98+
->assertContent("Request param '{$queryParam}' is not allowed.");
9799
}
98100

99101
/**
@@ -106,7 +108,39 @@ public function throw_exception_if_unauthorized_post_request_has_forbidden_reque
106108
return response('All good', 200);
107109
})->middleware(['api', 'auth.apikey']);
108110

109-
$this->post('/api/posts', [$bodyParam => 'value'])->assertInternalServerError();
111+
$this->post('/api/posts', [$bodyParam => 'value'])
112+
->assertBadRequest()
113+
->assertContent("Request param '{$bodyParam}' is not allowed.");
114+
}
115+
116+
/**
117+
* @test
118+
* @dataProvider forbiddenRequestParams
119+
*/
120+
public function throw_exception_if_unauthorized_json_get_request_has_forbidden_request_query_params(string $queryParam): void
121+
{
122+
Route::get('/api/posts', function () {
123+
return response('All good', 200);
124+
})->middleware(['api', 'auth.apikey']);
125+
126+
$this->getJson("/api/posts?{$queryParam}=value")
127+
->assertBadRequest()
128+
->assertJson(['message' => "Request param '{$queryParam}' is not allowed."]);
129+
}
130+
131+
/**
132+
* @test
133+
* @dataProvider forbiddenRequestParams
134+
*/
135+
public function throw_exception_if_unauthorized_json_post_request_has_forbidden_request_body_params(string $bodyParam): void
136+
{
137+
Route::post('/api/posts', function () {
138+
return response('All good', 200);
139+
})->middleware(['api', 'auth.apikey']);
140+
141+
$this->postJson('/api/posts', [$bodyParam => 'value'])
142+
->assertBadRequest()
143+
->assertJson(['message' => "Request param '{$bodyParam}' is not allowed."]);
110144
}
111145

112146
public function forbiddenRequestParams(): array

0 commit comments

Comments
 (0)