Added testing coverage to API token auth

diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php
index cd9c3b178c7e0ae01bc9b436fdd24e1561e0c25f..ba0b4b5dd5160581a3e1b975445c643c60c5c46b 100644 (file)
--- a/app/Api/ApiTokenGuard.php
+++ b/app/Api/ApiTokenGuard.php
@@ -150,4 +150,11 @@ class ApiTokenGuard implements Guard
         return Hash::check($credentials['secret'], $token->secret);
     }
 
+    /**
+     * "Log out" the currently authenticated user.
+     */
+    public function logout()
+    {
+        $this->user = null;
+    }
 }
\ No newline at end of file

diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index 3342ef5a8cadfd8ba9701ebf254f8bd4a6cb1f72..df9b1cea93faa7d413800e966248fd6f99eb62b1 100644 (file)
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -72,7 +72,7 @@ class Role extends Model
      */
     public function detachPermission(RolePermission $permission)
     {
-        $this->permissions()->detach($permission->id);
+        $this->permissions()->detach([$permission->id]);
     }
 
     /**

diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php
index c7fed405cc41e77b4176225a386b497e9793bc63..fffbd9ef6cbdd480beb1c89ca2bae9c6b1809b0f 100644 (file)
--- a/app/Http/Middleware/ApiAuthenticate.php
+++ b/app/Http/Middleware/ApiAuthenticate.php
@@ -35,7 +35,7 @@ class ApiAuthenticate
         }
 
         if ($this->awaitingEmailConfirmation()) {
-            return $this->emailConfirmationErrorResponse($request);
+            return $this->emailConfirmationErrorResponse($request, true);
         }
 
         return $next($request);

diff --git a/app/Http/Middleware/ChecksForEmailConfirmation.php b/app/Http/Middleware/ChecksForEmailConfirmation.php
index 684a7e9bc11a5b329f4a92045b99167f825b4629..df75c2f33dfb2bad5bf88f0afb8c98a432f8cc8e 100644 (file)
--- a/app/Http/Middleware/ChecksForEmailConfirmation.php
+++ b/app/Http/Middleware/ChecksForEmailConfirmation.php
@@ -26,9 +26,9 @@ trait ChecksForEmailConfirmation
      * Provide an error response for when the current user's email is not confirmed
      * in a system which requires it.
      */
-    protected function emailConfirmationErrorResponse(Request $request)
+    protected function emailConfirmationErrorResponse(Request $request, bool $forceJson = false)
     {
-        if ($request->wantsJson()) {
+        if ($request->wantsJson() || $forceJson) {
             return response()->json([
                 'error' => [
                     'code' => 401,

diff --git a/database/seeds/DummyContentSeeder.php b/database/seeds/DummyContentSeeder.php
index deb1aa11c3b3b196add089e024404eb7c73555a8..6d902a19632a7e7f7983d6710550854654faaf4e 100644 (file)
--- a/database/seeds/DummyContentSeeder.php
+++ b/database/seeds/DummyContentSeeder.php
@@ -1,6 +1,8 @@
 <?php
 
+use BookStack\Api\ApiToken;
 use BookStack\Auth\Permissions\PermissionService;
+use BookStack\Auth\Permissions\RolePermission;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Bookshelf;
@@ -52,6 +54,18 @@ class DummyContentSeeder extends Seeder
         $shelves = factory(Bookshelf::class, 10)->create($byData);
         $largeBook->shelves()->attach($shelves->pluck('id'));
 
+        // Assign API permission to editor role and create an API key
+        $apiPermission = RolePermission::getByName('access-api');
+        $editorRole->attachPermission($apiPermission);
+        $token = (new ApiToken())->forceFill([
+            'user_id' => $editorUser->id,
+            'name' => 'Testing API key',
+            'expires_at' => ApiToken::defaultExpiry(),
+            'secret' => Hash::make('password'),
+            'token_id' => 'apitoken',
+        ]);
+        $token->save();
+
         app(PermissionService::class)->buildJointPermissions();
         app(SearchService::class)->indexAllEntities();
     }

diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php
new file mode 100644 (file)
index 0000000..ef975d5
--- /dev/null
+++ b/tests/Api/ApiAuthTest.php
@@ -0,0 +1,82 @@
+<?php
+
+namespace Tests;
+
+use BookStack\Auth\Permissions\RolePermission;
+
+class ApiAuthTest extends TestCase
+{
+    use TestsApi;
+
+    protected $endpoint = '/api/books';
+
+    public function test_requests_succeed_with_default_auth()
+    {
+        $viewer = $this->getViewer();
+        $resp = $this->get($this->endpoint);
+        $resp->assertStatus(401);
+
+        $this->actingAs($viewer, 'web');
+
+        $resp = $this->get($this->endpoint);
+        $resp->assertStatus(200);
+    }
+
+    public function test_no_token_throws_error()
+    {
+        $resp = $this->get($this->endpoint);
+        $resp->assertStatus(401);
+        $resp->assertJson($this->errorResponse("No authorization token found on the request", 401));
+    }
+
+    public function test_bad_token_format_throws_error()
+    {
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token abc123"]);
+        $resp->assertStatus(401);
+        $resp->assertJson($this->errorResponse("An authorization token was found on the request but the format appeared incorrect", 401));
+    }
+
+    public function test_token_with_non_existing_id_throws_error()
+    {
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token abc:123"]);
+        $resp->assertStatus(401);
+        $resp->assertJson($this->errorResponse("No matching API token was found for the provided authorization token", 401));
+    }
+
+    public function test_token_with_bad_secret_value_throws_error()
+    {
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:123"]);
+        $resp->assertStatus(401);
+        $resp->assertJson($this->errorResponse("The secret provided for the given used API token is incorrect", 401));
+    }
+
+    public function test_api_access_permission_required_to_access_api()
+    {
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:{$this->apiTokenSecret}"]);
+        $resp->assertStatus(200);
+        auth()->logout();
+
+        $accessApiPermission = RolePermission::getByName('access-api');
+        $editorRole = $this->getEditor()->roles()->first();
+        $editorRole->detachPermission($accessApiPermission);
+
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:{$this->apiTokenSecret}"]);
+        $resp->assertJson($this->errorResponse("The owner of the used API token does not have permission to make API calls", 403));
+    }
+
+
+    public function test_email_confirmation_checked_on_auth_requets()
+    {
+        $editor = $this->getEditor();
+        $editor->email_confirmed = false;
+        $editor->save();
+
+        // Set settings and get user instance
+        $this->setSettings(['registration-enabled' => 'true', 'registration-confirmation' => 'true']);
+
+        $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:{$this->apiTokenSecret}"]);
+        $resp->assertStatus(401);
+        $resp->assertJson($this->errorResponse("The email address for the account in use needs to be confirmed", 401));
+    }
+
+}
\ No newline at end of file

diff --git a/tests/TestsApi.php b/tests/TestsApi.php
new file mode 100644 (file)
index 0000000..2bc751f
--- /dev/null
+++ b/tests/TestsApi.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace Tests;
+
+trait TestsApi
+{
+
+    protected $apiTokenId = 'apitoken';
+    protected $apiTokenSecret = 'password';
+
+    protected function errorResponse(string $messge, int $code)
+    {
+        return ["error" => ["code" => $code, "message" => $messge]];
+    }
+
+}
\ No newline at end of file

diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php
index 460752fc266de5272d5a182bce779168eac7eafe..7787e34fa94991722dbefadb08ed9079c7baa412 100644 (file)
--- a/tests/User/UserApiTokenTest.php
+++ b/tests/User/UserApiTokenTest.php
@@ -14,7 +14,7 @@ class UserApiTokenTest extends TestCase
 
     public function test_tokens_section_not_visible_without_access_api_permission()
     {
-        $user = $this->getEditor();
+        $user = $this->getViewer();
 
         $resp = $this->actingAs($user)->get($user->getEditUrl());
         $resp->assertDontSeeText('API Tokens');
@@ -30,9 +30,9 @@ class UserApiTokenTest extends TestCase
     {
         $viewer = $this->getViewer();
         $editor = $this->getEditor();
-        $this->giveUserPermissions($editor, ['users-manage']);
+        $this->giveUserPermissions($viewer, ['users-manage']);
 
-        $resp = $this->actingAs($editor)->get($viewer->getEditUrl());
+        $resp = $this->actingAs($viewer)->get($editor->getEditUrl());
         $resp->assertSeeText('API Tokens');
         $resp->assertDontSeeText('Create Token');
     }