diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index bc584d3c5..508efa028 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -35,7 +35,7 @@ class ApiAuthenticate // Return if the user is already found to be signed in via session-based auth. // This is to make it easy to browser the API via browser after just logging into the system. if (signedInUser() || session()->isStarted()) { - if (!user()->can('access-api')) { + if (!$this->sessionUserHasApiAccess()) { throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); } @@ -49,6 +49,15 @@ class ApiAuthenticate auth()->authenticate(); } + /** + * Check if the active session user has API access + */ + protected function sessionUserHasApiAccess(): bool + { + $hasApiPermission = user()->can('access-api'); + return $hasApiPermission && hasAppAccess(); + } + /** * Provide a standard API unauthorised response. */ diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php index c45bd77ee..cc6818e27 100644 --- a/tests/Api/ApiAuthTest.php +++ b/tests/Api/ApiAuthTest.php @@ -3,6 +3,7 @@ namespace Tests\Api; use BookStack\Auth\Permissions\RolePermission; +use BookStack\Auth\Role; use BookStack\Auth\User; use Carbon\Carbon; use Tests\TestCase; @@ -91,6 +92,26 @@ class ApiAuthTest extends TestCase $resp->assertJson($this->errorResponse('The owner of the used API token does not have permission to make API calls', 403)); } + public function test_access_prevented_for_guest_users_with_api_permission_while_public_access_disabled() + { + $this->disableCookieEncryption(); + $publicRole = Role::getSystemRole('public'); + $accessApiPermission = RolePermission::getByName('access-api'); + $publicRole->attachPermission($accessApiPermission); + + $this->withCookie('bookstack_session', 'abc123'); + + // Test API access when not public + setting()->put('app-public', false); + $resp = $this->get($this->endpoint); + $resp->assertStatus(403); + + // Test API access when public + setting()->put('app-public', true); + $resp = $this->get($this->endpoint); + $resp->assertStatus(200); + } + public function test_token_expiry_checked() { $editor = $this->getEditor();