Merge pull request #20 from givebutter/givebutter/security-improvement

Author: jhoff

src/Http/Middleware/AuthenticateApiKey.php

@@ -18,6 +18,23 @@ class AuthenticateApiKey
      */
     public function handle($request, Closure $next, $guard = null)
     {
+        $forbidenRequestParams = ['apiKey', 'keyable'];
+        
+        // Check if request has forbidden params
+        foreach ($forbidenRequestParams as $param) {
+            if ($request->missing($param)) {
+                continue;
+            }
+
+            $message = "Request param '{$param}' is not allowed.";
+
+            if ($request->wantsJson()) {
+                return response()->json(['message' => $message], 400);
+            }
+
+            return response($message, 400);
+        }
+
         //Get API token from request
         $token = $this->getKeyFromRequest($request);
 

tests/Feature/AuthenticateApiKey.php

@@ -2,6 +2,7 @@
 
 namespace Givebutter\Tests\Feature;
 
+use Givebutter\LaravelKeyable\Exceptions\ForbidenRequestParamException;
 use Givebutter\Tests\TestCase;
 use Givebutter\Tests\Support\Account;
 use Illuminate\Support\Facades\Route;
@@ -81,4 +82,72 @@ public function request_without_api_key_responds_unauthorized()
 
         $this->get("/api/posts")->assertUnauthorized();
     }
+
+    /**
+     * @test
+     * @dataProvider forbiddenRequestParams
+     */
+    public function throw_exception_if_unauthorized_get_request_has_forbidden_request_query_params(string $queryParam): void
+    {
+        Route::get('/api/posts', function () {
+            return response('All good', 200);
+        })->middleware(['api', 'auth.apikey']);
+
+        $this->get("/api/posts?{$queryParam}=value")
+            ->assertBadRequest()
+            ->assertContent("Request param '{$queryParam}' is not allowed.");
+    }
+
+    /**
+     * @test
+     * @dataProvider forbiddenRequestParams
+     */
+    public function throw_exception_if_unauthorized_post_request_has_forbidden_request_body_params(string $bodyParam): void
+    {
+        Route::post('/api/posts', function () {
+            return response('All good', 200);
+        })->middleware(['api', 'auth.apikey']);
+
+        $this->post('/api/posts', [$bodyParam => 'value'])
+            ->assertBadRequest()
+            ->assertContent("Request param '{$bodyParam}' is not allowed.");
+    }
+
+    /**
+     * @test
+     * @dataProvider forbiddenRequestParams
+     */
+    public function throw_exception_if_unauthorized_json_get_request_has_forbidden_request_query_params(string $queryParam): void
+    {
+        Route::get('/api/posts', function () {
+            return response('All good', 200);
+        })->middleware(['api', 'auth.apikey']);
+
+        $this->getJson("/api/posts?{$queryParam}=value")
+            ->assertBadRequest()
+            ->assertJson(['message' => "Request param '{$queryParam}' is not allowed."]);
+    }
+
+    /**
+     * @test
+     * @dataProvider forbiddenRequestParams
+     */
+    public function throw_exception_if_unauthorized_json_post_request_has_forbidden_request_body_params(string $bodyParam): void
+    {
+        Route::post('/api/posts', function () {
+            return response('All good', 200);
+        })->middleware(['api', 'auth.apikey']);
+
+        $this->postJson('/api/posts', [$bodyParam => 'value'])
+            ->assertBadRequest()
+            ->assertJson(['message' => "Request param '{$bodyParam}' is not allowed."]);
+    }
+
+    public function forbiddenRequestParams(): array
+    {
+        return [
+            ['keyable'],
+            ['apiKey'],
+        ];
+    }
 }