Adjust handling of invalid API requests (#885)

app/Http/Kernel.php

@@ -43,7 +43,7 @@ class Kernel extends HttpKernel
         'api' => [
             // Throttling is configured in routes/api.php
             \Illuminate\Routing\Middleware\SubstituteBindings::class,
-            \App\Http\Middleware\ContentTypeHeaderValidationMiddleware::class,
+            \App\Http\Middleware\ApiHeaderValidationMiddleware::class,
         ],
     ];
 

app/Http/Middleware/ApiHeaderValidationMiddleware.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+use Illuminate\Http\Request;
+
+class ApiHeaderValidationMiddleware
+{
+    /**
+     * This middleware ensures that the Content-Type header is set to
+     * application/json for POST, PATCH or DELETE, otherwise it will return a 415 Unsupported
+     * Media Type response.
+     *
+     * @param Request $request
+     * @param Closure $next
+     * @return mixed
+     */
+    public function handle(Request $request, Closure $next): mixed
+    {
+        if (in_array($request->method(), ['GET', 'HEAD', 'OPTIONS'])) {
+            return $next($request);
+        }
+
+        if ($request->header('Content-Type') !== 'application/json') {
+            return response()->json([
+                'error' => 'Invalid Content-Type header, LinkAce only supports JSON input'
+            ], 415);
+        }
+
+        if ($request->header('Accept') !== 'application/json') {
+            return response()->json([
+                'error' => 'Invalid Accept header, LinkAce only supports JSON output'
+            ], 415);
+        }
+
+        return $next($request);
+    }
+}

tests/Controller/API/ApiTestCase.php

@@ -74,6 +74,8 @@ public function postJsonAuthorized(
         bool $useSystemToken = false
     ): TestResponse {
         $headers['Authorization'] = 'Bearer ' . ($useSystemToken ? $this->systemAccessToken : $this->accessToken);
+        $headers['Content-Type'] = 'application/json';
+        $headers['Accept'] = 'application/json';
         return $this->postJson($uri, $data, $headers);
     }
 
@@ -92,7 +94,11 @@ public function patchJsonAuthorized(
         array $headers = [],
         bool $useSystemToken = false
     ): TestResponse {
-        $headers['Authorization'] = 'Bearer ' . ($useSystemToken ? $this->systemAccessToken : $this->accessToken);
+        $headers = array_merge([
+            'Authorization' => 'Bearer ' . ($useSystemToken ? $this->systemAccessToken : $this->accessToken),
+            'Content-Type' => 'application/json',
+            'Accept' => 'application/json',
+        ], $headers);
         return $this->patchJson($uri, $data, $headers);
     }
 
@@ -112,6 +118,8 @@ public function deleteJsonAuthorized(
         bool $useSystemToken = false
     ): TestResponse {
         $headers['Authorization'] = 'Bearer ' . ($useSystemToken ? $this->systemAccessToken : $this->accessToken);
+        $headers['Content-Type'] = 'application/json';
+        $headers['Accept'] = 'application/json';
         return $this->deleteJson($uri, $data, $headers);
     }
 }

tests/Controller/API/BulkEditApiTest.php

@@ -38,7 +38,7 @@ public function test_links_edit(): void
         $otherUser = User::factory()->create();
         $otherLink = Link::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/links', [
+        $this->patchJson('api/v2/bulk/links', [
             'models' => [1, 2, 3, 4],
             'tags' => [3],
             'tags_mode' => 'append',
@@ -71,7 +71,7 @@ public function test_alternative_links_edit(): void
         $otherUser = User::factory()->create();
         $otherLink = Link::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/links', [
+        $this->patchJson('api/v2/bulk/links', [
             'models' => [1, 2, 3, 4],
             'tags' => [2, 3],
             'tags_mode' => 'replace',
@@ -104,7 +104,7 @@ public function test_lists_edit(): void
         $otherUser = User::factory()->create();
         $otherList = LinkList::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/lists', [
+        $this->patchJson('api/v2/bulk/lists', [
             'models' => [1, 2, 3, 4],
             'visibility' => null,
         ])->assertJsonCount(4);
@@ -125,7 +125,7 @@ public function test_alternative_lists_edit(): void
         $otherUser = User::factory()->create();
         $otherList = LinkList::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/lists', [
+        $this->patchJson('api/v2/bulk/lists', [
             'models' => [1, 2, 3, 4],
             'visibility' => 2,
         ])->assertJsonCount(4);
@@ -146,7 +146,7 @@ public function test_tags_edit(): void
         $otherUser = User::factory()->create();
         $otherTag = Tag::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/tags', [
+        $this->patchJson('api/v2/bulk/tags', [
             'models' => [1, 2, 3, 4],
             'visibility' => null,
         ])->assertJsonCount(4);
@@ -167,7 +167,7 @@ public function test_alternative_tags_edit(): void
         $otherUser = User::factory()->create();
         $otherTag = Tag::factory()->for($otherUser)->create(['visibility' => ModelAttribute::VISIBILITY_PRIVATE]);
 
-        $this->patch('api/v2/bulk/tags', [
+        $this->patchJson('api/v2/bulk/tags', [
             'models' => [1, 2, 3, 4],
             'visibility' => 2,
         ])->assertJsonCount(4);
@@ -192,7 +192,7 @@ public function test_deletion()
         $tags = $this->createTestTags($this->user);
         $otherTag = Tag::factory()->for($otherUser)->create();
 
-        $this->delete('api/v2/bulk/delete', [
+        $this->deleteJson('api/v2/bulk/delete', [
             'models' => [1, 2, 4],
             'type' => 'links',
         ])->assertJsonCount(3)->assertJson([
@@ -206,7 +206,7 @@ public function test_deletion()
         $this->assertNotNull($links[1]->deleted_at);
         $this->assertNull($otherLink->deleted_at);
 
-        $this->delete('api/v2/bulk/delete', [
+        $this->deleteJson('api/v2/bulk/delete', [
             'models' => [1, 2, 4],
             'type' => 'lists',
         ])->assertJsonCount(3)->assertJson([
@@ -220,7 +220,7 @@ public function test_deletion()
         $this->assertNotNull($lists[1]->deleted_at);
         $this->assertNull($otherList->deleted_at);
 
-        $this->delete('api/v2/bulk/delete', [
+        $this->deleteJson('api/v2/bulk/delete', [
             'models' => [1, 2, 4],
             'type' => 'tags',
         ])->assertJsonCount(3)->assertJson([

tests/Controller/API/LinkApiTest.php

@@ -152,27 +152,6 @@ public function test_full_create_request(): void
             ]);
     }
 
-    public function test_create_invalid_content_type_request(): void
-    {
-        $list = LinkList::factory()->create(['name' => 'Test List 1']);
-        $tag = Tag::factory()->create(['name' => 'a test 1']);
-        $tag2 = Tag::factory()->create(['name' => 'tag #2']);
-
-        $this->postJsonAuthorized('api/v2/links', [
-            'url' => 'https://example.com',
-            'title' => 'Search the Web',
-            'description' => 'There could be a description here',
-            'lists' => [$list->id],
-            'tags' => [$tag->id, $tag2->id],
-            'visibility' => 1,
-            'check_disabled' => false,
-        ], ['Content-Type' => 'application/xml'])
-            ->assertUnsupportedMediaType()
-            ->assertJson([
-                'error' => "Invalid Content-Type"
-            ]);
-    }
-
     public function test_create_request_with_list(): void
     {
         $list = LinkList::factory()->create(['name' => 'Test List 1']);
@@ -469,25 +448,6 @@ public function test_update_request(): void
         ])->assertForbidden();
     }
 
-    public function test_update_invalid_content_type_request(): void
-    {
-        $list = LinkList::factory()->create();
-        $this->createTestLinks();
-
-        $this->patchJsonAuthorized('api/v2/links/1', [
-            'url' => 'https://new-public-link.com',
-            'title' => 'Custom Title',
-            'description' => 'Custom Description',
-            'lists' => [$list->id],
-            'is_private' => false,
-            'check_disabled' => false,
-        ], ['Content-Type' => 'application/xml'])
-            ->assertUnsupportedMediaType()
-            ->assertJson([
-                'error' => 'Invalid Content-Type'
-            ]);
-    }
-
     public function test_invalid_update_request(): void
     {
         Link::factory()->create();

tests/Middleware/ContentTypeHeaderValidationMiddlewareTest.php

@@ -2,21 +2,70 @@
 
 namespace Tests\Middleware;
 
-use App\Http\Middleware\ContentTypeHeaderValidationMiddleware;
-use Illuminate\Http\Request;
+use App\Enums\ApiToken;
+use App\Models\User;
+use Illuminate\Support\Facades\Http;
 use Tests\TestCase;
 
 class ContentTypeHeaderValidationMiddlewareTest extends TestCase
 {
     public function testMissingContentTypeHeader(): void
     {
-        $request = Request::create('/api/v1/links', 'POST');
+        $user = User::factory()->create();
+        $accessToken = $user->createToken('api-test', [ApiToken::ABILITY_USER_ACCESS])->plainTextToken;
 
-        $middleware = new ContentTypeHeaderValidationMiddleware();
+        $testHtml = '<!DOCTYPE html><head>' .
+            '<title>Example Title</title>' .
+            '<meta name="description" content="This an example description">' .
+            '</head></html>';
 
-        $response = $middleware->handle($request, function () {
-        });
+        Http::fake([
+            'example.com' => Http::response($testHtml),
+        ]);
 
-        $this->assertEquals(415, $response->getStatusCode());
+        // content-type and accept are missing
+        $this->post('api/v2/links', ['url' => 'https://example.com'], ['Authorization' => 'Bearer ' . $accessToken])
+            ->assertUnsupportedMediaType()
+            ->assertJson([
+                'error' => 'Invalid Content-Type header, LinkAce only supports JSON input',
+            ]);
+
+        // content-type is present, but not supported
+        $this->post('api/v2/links', ['url' => 'https://example.com'], [
+            'Authorization' => 'Bearer ' . $accessToken,
+            'Content-Type' => 'application/xml',
+        ])
+            ->assertUnsupportedMediaType()
+            ->assertJson([
+                'error' => 'Invalid Content-Type header, LinkAce only supports JSON input',
+            ]);
+
+        // accept header is missing
+        $this->post('api/v2/links', ['url' => 'https://example.com'], [
+            'Authorization' => 'Bearer ' . $accessToken,
+            'Content-Type' => 'application/json',
+        ])
+            ->assertUnsupportedMediaType()
+            ->assertJson([
+                'error' => 'Invalid Accept header, LinkAce only supports JSON output',
+            ]);
+
+        // accept header is present, but not supported
+        $this->post('api/v2/links', ['url' => 'https://example.com'], [
+            'Authorization' => 'Bearer ' . $accessToken,
+            'Content-Type' => 'application/json',
+            'Accept' => 'application/xml',
+        ])
+            ->assertUnsupportedMediaType()
+            ->assertJson([
+                'error' => 'Invalid Accept header, LinkAce only supports JSON output',
+            ]);
+
+        // request headers are correct
+        $this->postJson('api/v2/links', ['url' => 'https://example.com'], [
+            'Authorization' => 'Bearer ' . $accessToken,
+            'Content-Type' => 'application/json',
+            'Accept' => 'application/json',
+        ])->assertOk();
     }
 }