diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php index cd9c3b178..ba0b4b5dd 100644 --- 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 3342ef5a8..df9b1cea9 100644 --- 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 c7fed405c..fffbd9ef6 100644 --- 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 684a7e9bc..df75c2f33 100644 --- 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 deb1aa11c..6d902a196 100644 --- 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 index 000000000..ef975d556 --- /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 index 000000000..2bc751f54 --- /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 460752fc2..7787e34fa 100644 --- 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'); }