From 04137e7c98cfe182f3c603e7d1acbc9a0ed524e7 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 28 Dec 2019 14:58:07 +0000 Subject: [PATCH 01/23] Started core API route work --- .env.example.complete | 3 + app/Api/ListingResponseBuilder.php | 82 +++++++++++++++++++ app/Config/api.php | 20 +++++ app/Http/Controllers/Api/ApiController.php | 20 +++++ .../Controllers/Api/BooksApiController.php | 18 ++++ app/Http/Kernel.php | 1 - app/Providers/RouteServiceProvider.php | 4 +- routes/api.php | 12 +++ 8 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 app/Api/ListingResponseBuilder.php create mode 100644 app/Config/api.php create mode 100644 app/Http/Controllers/Api/ApiController.php create mode 100644 app/Http/Controllers/Api/BooksApiController.php create mode 100644 routes/api.php diff --git a/.env.example.complete b/.env.example.complete index a13c8b7d0..716d614a3 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -258,3 +258,6 @@ ALLOW_CONTENT_SCRIPTS=false # Contents of the robots.txt file can be overridden, making this option obsolete. ALLOW_ROBOTS=null +# The default and maximum item-counts for listing API requests. +API_DEFAULT_ITEM_COUNT=100 +API_MAX_ITEM_COUNT=500 \ No newline at end of file diff --git a/app/Api/ListingResponseBuilder.php b/app/Api/ListingResponseBuilder.php new file mode 100644 index 000000000..279fabd5d --- /dev/null +++ b/app/Api/ListingResponseBuilder.php @@ -0,0 +1,82 @@ +query = $query; + $this->fields = $fields; + } + + /** + * Get the response from this builder. + */ + public function toResponse() + { + $total = $this->query->count(); + $data = $this->fetchData(); + + return response()->json([ + 'data' => $data, + 'total' => $total, + ]); + } + + /** + * Fetch the data to return in the response. + */ + protected function fetchData(): Collection + { + $this->applyCountAndOffset($this->query); + $this->applySorting($this->query); + // TODO - Apply filtering + + return $this->query->get($this->fields); + } + + /** + * Apply sorting operations to the query from given parameters + * otherwise falling back to the first given field, ascending. + */ + protected function applySorting(Builder $query) + { + $defaultSortName = $this->fields[0]; + $direction = 'asc'; + + $sort = request()->get('sort', ''); + if (strpos($sort, '-') === 0) { + $direction = 'desc'; + } + + $sortName = ltrim($sort, '+- '); + if (!in_array($sortName, $this->fields)) { + $sortName = $defaultSortName; + } + + $query->orderBy($sortName, $direction); + } + + /** + * Apply count and offset for paging, based on params from the request while falling + * back to system defined default, taking the max limit into account. + */ + protected function applyCountAndOffset(Builder $query) + { + $offset = max(0, request()->get('offset', 0)); + $maxCount = config('api.max_item_count'); + $count = request()->get('count', config('api.default_item_count')); + $count = max(min($maxCount, $count), 1); + + $query->skip($offset)->take($count); + } +} diff --git a/app/Config/api.php b/app/Config/api.php new file mode 100644 index 000000000..dd54b2906 --- /dev/null +++ b/app/Config/api.php @@ -0,0 +1,20 @@ + env('API_DEFAULT_ITEM_COUNT', 100), + + // The maximum number of items that can be returned in a listing API request. + 'max_item_count' => env('API_MAX_ITEM_COUNT', 500), + +]; diff --git a/app/Http/Controllers/Api/ApiController.php b/app/Http/Controllers/Api/ApiController.php new file mode 100644 index 000000000..4971c0cde --- /dev/null +++ b/app/Http/Controllers/Api/ApiController.php @@ -0,0 +1,20 @@ +toResponse(); + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php new file mode 100644 index 000000000..d23aaf025 --- /dev/null +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -0,0 +1,18 @@ +apiListingResponse($books, [ + 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', + 'restricted', 'image_id', + ]); + } +} \ No newline at end of file diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index f9752da09..cd3fc83ec 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -37,7 +37,6 @@ class Kernel extends HttpKernel ], 'api' => [ 'throttle:60,1', - 'bindings', ], ]; diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index c4c39d534..a37780e52 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -34,7 +34,7 @@ class RouteServiceProvider extends ServiceProvider public function map() { $this->mapWebRoutes(); -// $this->mapApiRoutes(); + $this->mapApiRoutes(); } /** * Define the "web" routes for the application. @@ -63,7 +63,7 @@ class RouteServiceProvider extends ServiceProvider { Route::group([ 'middleware' => 'api', - 'namespace' => $this->namespace, + 'namespace' => $this->namespace . '\Api', 'prefix' => 'api', ], function ($router) { require base_path('routes/api.php'); diff --git a/routes/api.php b/routes/api.php new file mode 100644 index 000000000..0604ffd29 --- /dev/null +++ b/routes/api.php @@ -0,0 +1,12 @@ + Date: Sun, 29 Dec 2019 13:02:26 +0000 Subject: [PATCH 02/23] Started work on API token controls - Added access-api permission. - Started user profile UI work. - Created database table and model for tokens. - Fixed incorrect templates down migration :( --- app/Api/ApiToken.php | 9 +++ app/Auth/User.php | 21 +++++-- .../Controllers/UserApiTokenController.php | 20 +++++++ app/Http/Controllers/UserController.php | 14 +++-- ...2019_07_07_112515_add_template_support.php | 4 +- .../2019_12_29_120917_add_api_auth.php | 59 +++++++++++++++++++ resources/lang/en/settings.php | 6 ++ resources/views/settings/roles/form.blade.php | 3 +- resources/views/users/edit.blade.php | 19 ++++++ routes/web.php | 3 + 10 files changed, 143 insertions(+), 15 deletions(-) create mode 100644 app/Api/ApiToken.php create mode 100644 app/Http/Controllers/UserApiTokenController.php create mode 100644 database/migrations/2019_12_29_120917_add_api_auth.php diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php new file mode 100644 index 000000000..838e70abb --- /dev/null +++ b/app/Api/ApiToken.php @@ -0,0 +1,9 @@ +id); + return $this->hasMany(ApiToken::class); + } + + /** + * Get the url for editing this user. + */ + public function getEditUrl(string $path = ''): string + { + $uri = '/settings/users/' . $this->id . '/' . trim($path, '/'); + return url(rtrim($uri, '/')); } /** * Get the url that links to this user's profile. - * @return mixed */ - public function getProfileUrl() + public function getProfileUrl(): string { return url('/user/' . $this->id); } diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php new file mode 100644 index 000000000..385352011 --- /dev/null +++ b/app/Http/Controllers/UserApiTokenController.php @@ -0,0 +1,20 @@ +checkPermission('access-api'); + + // TODO - Form + return 'test'; + } + + +} diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index b55398d2f..207466f38 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -116,22 +116,24 @@ class UserController extends Controller /** * Show the form for editing the specified user. - * @param int $id - * @param \BookStack\Auth\Access\SocialAuthService $socialAuthService - * @return Response */ - public function edit($id, SocialAuthService $socialAuthService) + public function edit(int $id, SocialAuthService $socialAuthService) { $this->checkPermissionOrCurrentUser('users-manage', $id); - $user = $this->user->findOrFail($id); + $user = $this->user->newQuery()->with(['apiTokens'])->findOrFail($id); $authMethod = ($user->system_name) ? 'system' : config('auth.method'); $activeSocialDrivers = $socialAuthService->getActiveDrivers(); $this->setPageTitle(trans('settings.user_profile')); $roles = $this->userRepo->getAllRoles(); - return view('users.edit', ['user' => $user, 'activeSocialDrivers' => $activeSocialDrivers, 'authMethod' => $authMethod, 'roles' => $roles]); + return view('users.edit', [ + 'user' => $user, + 'activeSocialDrivers' => $activeSocialDrivers, + 'authMethod' => $authMethod, + 'roles' => $roles + ]); } /** diff --git a/database/migrations/2019_07_07_112515_add_template_support.php b/database/migrations/2019_07_07_112515_add_template_support.php index a54508198..3fcc68227 100644 --- a/database/migrations/2019_07_07_112515_add_template_support.php +++ b/database/migrations/2019_07_07_112515_add_template_support.php @@ -46,9 +46,9 @@ class AddTemplateSupport extends Migration // Remove templates-manage permission $templatesManagePermission = DB::table('role_permissions') - ->where('name', '=', 'templates_manage')->first(); + ->where('name', '=', 'templates-manage')->first(); DB::table('permission_role')->where('permission_id', '=', $templatesManagePermission->id)->delete(); - DB::table('role_permissions')->where('name', '=', 'templates_manage')->delete(); + DB::table('role_permissions')->where('name', '=', 'templates-manage')->delete(); } } diff --git a/database/migrations/2019_12_29_120917_add_api_auth.php b/database/migrations/2019_12_29_120917_add_api_auth.php new file mode 100644 index 000000000..e80fe3ae4 --- /dev/null +++ b/database/migrations/2019_12_29_120917_add_api_auth.php @@ -0,0 +1,59 @@ +increments('id'); + $table->string('client_id')->index(); + $table->string('client_secret'); + $table->integer('user_id')->unsigned()->index(); + $table->timestamp('expires_at')->index(); + $table->nullableTimestamps(); + }); + + // Add access-api permission + $adminRoleId = DB::table('roles')->where('system_name', '=', 'admin')->first()->id; + $permissionId = DB::table('role_permissions')->insertGetId([ + 'name' => 'access-api', + 'display_name' => 'Access system API', + 'created_at' => Carbon::now()->toDateTimeString(), + 'updated_at' => Carbon::now()->toDateTimeString() + ]); + DB::table('permission_role')->insert([ + 'role_id' => $adminRoleId, + 'permission_id' => $permissionId + ]); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + // Remove API tokens table + Schema::dropIfExists('api_tokens'); + + // Remove access-api permission + $apiAccessPermission = DB::table('role_permissions') + ->where('name', '=', 'access-api')->first(); + + DB::table('permission_role')->where('permission_id', '=', $apiAccessPermission->id)->delete(); + DB::table('role_permissions')->where('name', '=', 'access-api')->delete(); + } +} diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 6be7cc9cb..bb750a780 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -103,6 +103,7 @@ return [ 'role_manage_entity_permissions' => 'Manage all book, chapter & page permissions', 'role_manage_own_entity_permissions' => 'Manage permissions on own book, chapter & pages', 'role_manage_page_templates' => 'Manage page templates', + 'role_access_api' => 'Access system API', 'role_manage_settings' => 'Manage app settings', 'role_asset' => 'Asset Permissions', 'role_asset_desc' => 'These permissions control default access to the assets within the system. Permissions on Books, Chapters and Pages will override these permissions.', @@ -151,6 +152,11 @@ return [ 'users_social_disconnect' => 'Disconnect Account', 'users_social_connected' => ':socialAccount account was successfully attached to your profile.', 'users_social_disconnected' => ':socialAccount account was successfully disconnected from your profile.', + 'users_api_tokens' => 'API Tokens', + 'users_api_tokens_none' => 'No API tokens have been created for this user', + 'users_api_tokens_create' => 'Create Token', + + // API Tokens //! If editing translations files directly please ignore this in all //! languages apart from en. Content will be auto-copied from en. diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 4617b1f52..1fbc80b1f 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -34,12 +34,13 @@ {{ trans('common.toggle_all') }}
+
@include('settings.roles.checkbox', ['permission' => 'settings-manage', 'label' => trans('settings.role_manage_settings')])
@include('settings.roles.checkbox', ['permission' => 'users-manage', 'label' => trans('settings.role_manage_users')])
@include('settings.roles.checkbox', ['permission' => 'user-roles-manage', 'label' => trans('settings.role_manage_roles')])
@include('settings.roles.checkbox', ['permission' => 'restrictions-manage-all', 'label' => trans('settings.role_manage_entity_permissions')])
@include('settings.roles.checkbox', ['permission' => 'restrictions-manage-own', 'label' => trans('settings.role_manage_own_entity_permissions')])
@include('settings.roles.checkbox', ['permission' => 'templates-manage', 'label' => trans('settings.role_manage_page_templates')])
-
@include('settings.roles.checkbox', ['permission' => 'settings-manage', 'label' => trans('settings.role_manage_settings')])
+
@include('settings.roles.checkbox', ['permission' => 'access-api', 'label' => trans('settings.role_access_api')])
diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index ff1e7cbe5..b3f73773b 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -87,6 +87,25 @@ @endif + + {{-- TODO - Review Control--}} + @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('manage-users')) +
+
+

{{ trans('settings.users_api_tokens') }}

+
+ @if(userCan('access-api')) + {{ trans('settings.users_api_tokens_create') }} + @endif +
+
+ @if (count($user->apiTokens) > 0) + + @else +

{{ trans('settings.users_api_tokens_none') }}

+ @endif +
+ @endif @stop diff --git a/routes/web.php b/routes/web.php index 839e5a256..2a0e85dfe 100644 --- a/routes/web.php +++ b/routes/web.php @@ -187,6 +187,9 @@ Route::group(['middleware' => 'auth'], function () { Route::put('/users/{id}', 'UserController@update'); Route::delete('/users/{id}', 'UserController@destroy'); + // User API Tokens + Route::get('/users/{userId}/create-api-token', 'UserApiTokenController@create'); + // Roles Route::get('/roles', 'PermissionController@listRoles'); Route::get('/roles/new', 'PermissionController@createRole'); From dccb279c84a3f523f1c07ffc4ee4e2b526fd5384 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 29 Dec 2019 17:03:52 +0000 Subject: [PATCH 03/23] Built out interfaces & endpoints for API token managment --- app/Api/ApiToken.php | 4 +- app/Auth/UserRepo.php | 1 + .../Controllers/UserApiTokenController.php | 119 +++++++++++++++++- .../2019_12_29_120917_add_api_auth.php | 3 +- resources/lang/en/settings.php | 18 +++ resources/sass/_forms.scss | 3 + resources/views/form/date.blade.php | 9 ++ resources/views/form/text.blade.php | 1 + .../views/users/api-tokens/create.blade.php | 33 +++++ .../views/users/api-tokens/delete.blade.php | 26 ++++ .../views/users/api-tokens/edit.blade.php | 66 ++++++++++ .../views/users/api-tokens/form.blade.php | 21 ++++ resources/views/users/edit.blade.php | 22 +++- routes/web.php | 5 + 14 files changed, 325 insertions(+), 6 deletions(-) create mode 100644 resources/views/form/date.blade.php create mode 100644 resources/views/users/api-tokens/create.blade.php create mode 100644 resources/views/users/api-tokens/delete.blade.php create mode 100644 resources/views/users/api-tokens/edit.blade.php create mode 100644 resources/views/users/api-tokens/form.blade.php diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php index 838e70abb..e7101387f 100644 --- a/app/Api/ApiToken.php +++ b/app/Api/ApiToken.php @@ -5,5 +5,7 @@ use Illuminate\Database\Eloquent\Model; class ApiToken extends Model { protected $fillable = ['name', 'expires_at']; - + protected $casts = [ + 'expires_at' => 'datetime:Y-m-d' + ]; } diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index a903e2c38..e082b2dd5 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -194,6 +194,7 @@ class UserRepo public function destroy(User $user) { $user->socialAccounts()->delete(); + $user->apiTokens()->delete(); $user->delete(); // Delete user profile images diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php index 385352011..3bfb0175e 100644 --- a/app/Http/Controllers/UserApiTokenController.php +++ b/app/Http/Controllers/UserApiTokenController.php @@ -1,6 +1,11 @@ checkPermission('access-api'); + $this->checkPermissionOrCurrentUser('manage-users', $userId); - // TODO - Form - return 'test'; + $user = User::query()->findOrFail($userId); + return view('users.api-tokens.create', [ + 'user' => $user, + ]); } + /** + * Store a new API token in the system. + */ + public function store(Request $request, int $userId) + { + $this->checkPermission('access-api'); + $this->checkPermissionOrCurrentUser('manage-users', $userId); + + $this->validate($request, [ + 'name' => 'required|max:250', + 'expires_at' => 'date_format:Y-m-d', + ]); + + $user = User::query()->findOrFail($userId); + $secret = Str::random(32); + $expiry = $request->get('expires_at', (Carbon::now()->addYears(100))->format('Y-m-d')); + + $token = (new ApiToken())->forceFill([ + 'name' => $request->get('name'), + 'client_id' => Str::random(32), + 'client_secret' => Hash::make($secret), + 'user_id' => $user->id, + 'expires_at' => $expiry + ]); + + while (ApiToken::query()->where('client_id', '=', $token->client_id)->exists()) { + $token->client_id = Str::random(32); + } + + $token->save(); + // TODO - Notification and activity? + session()->flash('api-token-secret:' . $token->id, $secret); + return redirect($user->getEditUrl('/api-tokens/' . $token->id)); + } + + /** + * Show the details for a user API token, with access to edit. + */ + public function edit(int $userId, int $tokenId) + { + [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); + $secret = session()->pull('api-token-secret:' . $token->id, null); + + return view('users.api-tokens.edit', [ + 'user' => $user, + 'token' => $token, + 'model' => $token, + 'secret' => $secret, + ]); + } + + /** + * Update the API token. + */ + public function update(Request $request, int $userId, int $tokenId) + { + $this->validate($request, [ + 'name' => 'required|max:250', + 'expires_at' => 'date_format:Y-m-d', + ]); + + [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); + + $token->fill($request->all())->save(); + // TODO - Notification and activity? + return redirect($user->getEditUrl('/api-tokens/' . $token->id)); + } + + /** + * Show the delete view for this token. + */ + public function delete(int $userId, int $tokenId) + { + [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); + return view('users.api-tokens.delete', [ + 'user' => $user, + 'token' => $token, + ]); + } + + /** + * Destroy a token from the system. + */ + public function destroy(int $userId, int $tokenId) + { + [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); + $token->delete(); + + // TODO - Notification and activity?, Might have text in translations already (user_api_token_delete_success) + return redirect($user->getEditUrl('#api_tokens')); + } + + /** + * Check the permission for the current user and return an array + * where the first item is the user in context and the second item is their + * API token in context. + */ + protected function checkPermissionAndFetchUserToken(int $userId, int $tokenId): array + { + $this->checkPermission('access-api'); + $this->checkPermissionOrCurrentUser('manage-users', $userId); + + $user = User::query()->findOrFail($userId); + $token = ApiToken::query()->where('user_id', '=', $user->id)->where('id', '=', $tokenId)->firstOrFail(); + return [$user, $token]; + } } diff --git a/database/migrations/2019_12_29_120917_add_api_auth.php b/database/migrations/2019_12_29_120917_add_api_auth.php index e80fe3ae4..2af0b292e 100644 --- a/database/migrations/2019_12_29_120917_add_api_auth.php +++ b/database/migrations/2019_12_29_120917_add_api_auth.php @@ -18,7 +18,8 @@ class AddApiAuth extends Migration // Add API tokens table Schema::create('api_tokens', function(Blueprint $table) { $table->increments('id'); - $table->string('client_id')->index(); + $table->string('name'); + $table->string('client_id')->unique(); $table->string('client_secret'); $table->integer('user_id')->unsigned()->index(); $table->timestamp('expires_at')->index(); diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index bb750a780..a2148361a 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -155,8 +155,26 @@ return [ 'users_api_tokens' => 'API Tokens', 'users_api_tokens_none' => 'No API tokens have been created for this user', 'users_api_tokens_create' => 'Create Token', + 'users_api_tokens_expires' => 'Expires', // API Tokens + 'user_api_token_create' => 'Create API Token', + 'user_api_token_name' => 'Name', + 'user_api_token_name_desc' => 'Give your token a readable name as a future reminder of its intended purpose.', + 'user_api_token_expiry' => 'Expiry Date', + 'user_api_token_expiry_desc' => 'Set a date at which this token expires. After this date, requests made using this token will no longer work. Leaving this field blank will set an expiry 100 years into the future.', + 'user_api_token_create_secret_message' => 'Immediately after creating this token a "client id"" & "client secret" will be generated and displayed. The client secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.', + 'user_api_token' => 'API Token', + 'user_api_token_client_id' => 'Client ID', + 'user_api_token_client_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.', + 'user_api_token_client_secret' => 'Client Secret', + 'user_api_token_client_secret_desc' => 'This is a system generated secret for this token which will need to be provided in API requests. This will only be displayed this one time so copy this value to somewhere safe and secure.', + 'user_api_token_created' => 'Token Created :timeAgo', + 'user_api_token_updated' => 'Token Updated :timeAgo', + 'user_api_token_delete' => 'Delete Token', + 'user_api_token_delete_warning' => 'This will fully delete this API token with the name \':tokenName\' from the system.', + 'user_api_token_delete_confirm' => 'Are you sure you want to delete this API token?', + 'user_api_token_delete_success' => 'API token successfully deleted', //! If editing translations files directly please ignore this in all //! languages apart from en. Content will be auto-copied from en. diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss index 3e7ff60f3..da0f7ef4c 100644 --- a/resources/sass/_forms.scss +++ b/resources/sass/_forms.scss @@ -19,6 +19,9 @@ &.disabled, &[disabled] { background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAcAAAAHCAYAAADEUlfTAAAAMUlEQVQIW2NkwAGuXbv2nxGbHEhCS0uLEUMSJgHShCKJLIEiiS4Bl8QmAZbEJQGSBAC62BuJ+tt7zgAAAABJRU5ErkJggg==); } + &[readonly] { + background-color: #f8f8f8; + } &:focus { border-color: var(--color-primary); outline: 1px solid var(--color-primary); diff --git a/resources/views/form/date.blade.php b/resources/views/form/date.blade.php new file mode 100644 index 000000000..c2e70b9e3 --- /dev/null +++ b/resources/views/form/date.blade.php @@ -0,0 +1,9 @@ +has($name)) class="text-neg" @endif + placeholder="{{ $placeholder ?? 'YYYY-MM-DD' }}" + @if($autofocus ?? false) autofocus @endif + @if($disabled ?? false) disabled="disabled" @endif + @if(isset($model) || old($name)) value="{{ old($name) ?? $model->$name->format('Y-m-d') ?? ''}}" @endif> +@if($errors->has($name)) +
{{ $errors->first($name) }}
+@endif diff --git a/resources/views/form/text.blade.php b/resources/views/form/text.blade.php index 4b3631a06..fabfab451 100644 --- a/resources/views/form/text.blade.php +++ b/resources/views/form/text.blade.php @@ -3,6 +3,7 @@ @if(isset($placeholder)) placeholder="{{$placeholder}}" @endif @if($autofocus ?? false) autofocus @endif @if($disabled ?? false) disabled="disabled" @endif + @if($readonly ?? false) readonly="readonly" @endif @if(isset($model) || old($name)) value="{{ old($name) ? old($name) : $model->$name}}" @endif> @if($errors->has($name))
{{ $errors->first($name) }}
diff --git a/resources/views/users/api-tokens/create.blade.php b/resources/views/users/api-tokens/create.blade.php new file mode 100644 index 000000000..46c3e0b8a --- /dev/null +++ b/resources/views/users/api-tokens/create.blade.php @@ -0,0 +1,33 @@ +@extends('simple-layout') + +@section('body') + +
+ +
+

{{ trans('settings.user_api_token_create') }}

+ +
+ {!! csrf_field() !!} + +
+ @include('users.api-tokens.form') + +
+

+ {{ trans('settings.user_api_token_create_secret_message') }} +

+
+
+ +
+ {{ trans('common.cancel') }} + +
+ +
+ +
+
+ +@stop diff --git a/resources/views/users/api-tokens/delete.blade.php b/resources/views/users/api-tokens/delete.blade.php new file mode 100644 index 000000000..8fcfcda95 --- /dev/null +++ b/resources/views/users/api-tokens/delete.blade.php @@ -0,0 +1,26 @@ +@extends('simple-layout') + +@section('body') +
+ +
+

{{ trans('settings.user_api_token_delete') }}

+ +

{{ trans('settings.user_api_token_delete_warning', ['tokenName' => $token->name]) }}

+ +
+

{{ trans('settings.user_api_token_delete_confirm') }}

+
+
+ {!! csrf_field() !!} + {!! method_field('delete') !!} + + {{ trans('common.cancel') }} + +
+
+
+ +
+
+@stop diff --git a/resources/views/users/api-tokens/edit.blade.php b/resources/views/users/api-tokens/edit.blade.php new file mode 100644 index 000000000..0ec9adbe6 --- /dev/null +++ b/resources/views/users/api-tokens/edit.blade.php @@ -0,0 +1,66 @@ +@extends('simple-layout') + +@section('body') + +
+ +
+

{{ trans('settings.user_api_token') }}

+ +
+ {!! method_field('put') !!} + {!! csrf_field() !!} + +
+ +
+
+ +

{{ trans('settings.user_api_token_client_id_desc') }}

+
+
+ @include('form.text', ['name' => 'client_id', 'readonly' => true]) +
+
+ + + @if( $secret ) +
+
+ +

{{ trans('settings.user_api_token_client_secret_desc') }}

+
+
+ +
+
+ @endif + + @include('users.api-tokens.form', ['model' => $token]) +
+ +
+ +
+ + {{ trans('settings.user_api_token_created', ['timeAgo' => $token->created_at->diffForHumans()]) }} + +
+ + {{ trans('settings.user_api_token_updated', ['timeAgo' => $token->created_at->diffForHumans()]) }} + +
+ + +
+ +
+ +
+
+ +@stop diff --git a/resources/views/users/api-tokens/form.blade.php b/resources/views/users/api-tokens/form.blade.php new file mode 100644 index 000000000..d81a330d5 --- /dev/null +++ b/resources/views/users/api-tokens/form.blade.php @@ -0,0 +1,21 @@ + + +
+
+ +

{{ trans('settings.user_api_token_name_desc') }}

+
+
+ @include('form.text', ['name' => 'name']) +
+
+ +
+
+ +

{{ trans('settings.user_api_token_expiry_desc') }}

+
+
+ @include('form.date', ['name' => 'expires_at']) +
+
\ No newline at end of file diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index b3f73773b..54e0ee21a 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -90,7 +90,7 @@ {{-- TODO - Review Control--}} @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('manage-users')) -
+

{{ trans('settings.users_api_tokens') }}

@@ -100,7 +100,25 @@
@if (count($user->apiTokens) > 0) - + + + + + + + @foreach($user->apiTokens as $token) + + + + + + @endforeach +
{{ trans('common.name') }}{{ trans('settings.users_api_tokens_expires') }}
+ {{ $token->name }}
+ {{ $token->client_id }} +
{{ $token->expires_at->format('Y-m-d') ?? '' }} + {{ trans('common.edit') }} +
@else

{{ trans('settings.users_api_tokens_none') }}

@endif diff --git a/routes/web.php b/routes/web.php index 2a0e85dfe..f38575b79 100644 --- a/routes/web.php +++ b/routes/web.php @@ -189,6 +189,11 @@ Route::group(['middleware' => 'auth'], function () { // User API Tokens Route::get('/users/{userId}/create-api-token', 'UserApiTokenController@create'); + Route::post('/users/{userId}/create-api-token', 'UserApiTokenController@store'); + Route::get('/users/{userId}/api-tokens/{tokenId}', 'UserApiTokenController@edit'); + Route::put('/users/{userId}/api-tokens/{tokenId}', 'UserApiTokenController@update'); + Route::get('/users/{userId}/api-tokens/{tokenId}/delete', 'UserApiTokenController@delete'); + Route::delete('/users/{userId}/api-tokens/{tokenId}', 'UserApiTokenController@destroy'); // Roles Route::get('/roles', 'PermissionController@listRoles'); From 832fbd65afcaa8d8f2953fe04de2e479053dbc29 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 29 Dec 2019 19:46:46 +0000 Subject: [PATCH 04/23] Added testing coverage to user API token interfaces --- app/Api/ApiToken.php | 2 +- .../Controllers/UserApiTokenController.php | 17 +- .../2019_12_29_120917_add_api_auth.php | 2 +- resources/lang/en/settings.php | 2 + resources/views/users/edit.blade.php | 3 +- tests/User/UserApiTokenTest.php | 165 ++++++++++++++++++ tests/{ => User}/UserPreferencesTest.php | 0 tests/{ => User}/UserProfileTest.php | 0 8 files changed, 180 insertions(+), 11 deletions(-) create mode 100644 tests/User/UserApiTokenTest.php rename tests/{ => User}/UserPreferencesTest.php (100%) rename tests/{ => User}/UserProfileTest.php (100%) diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php index e7101387f..4ea12888e 100644 --- a/app/Api/ApiToken.php +++ b/app/Api/ApiToken.php @@ -6,6 +6,6 @@ class ApiToken extends Model { protected $fillable = ['name', 'expires_at']; protected $casts = [ - 'expires_at' => 'datetime:Y-m-d' + 'expires_at' => 'date:Y-m-d' ]; } diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php index 3bfb0175e..9f5ebc49e 100644 --- a/app/Http/Controllers/UserApiTokenController.php +++ b/app/Http/Controllers/UserApiTokenController.php @@ -17,7 +17,7 @@ class UserApiTokenController extends Controller { // Ensure user is has access-api permission and is the current user or has permission to manage the current user. $this->checkPermission('access-api'); - $this->checkPermissionOrCurrentUser('manage-users', $userId); + $this->checkPermissionOrCurrentUser('users-manage', $userId); $user = User::query()->findOrFail($userId); return view('users.api-tokens.create', [ @@ -31,7 +31,7 @@ class UserApiTokenController extends Controller public function store(Request $request, int $userId) { $this->checkPermission('access-api'); - $this->checkPermissionOrCurrentUser('manage-users', $userId); + $this->checkPermissionOrCurrentUser('users-manage', $userId); $this->validate($request, [ 'name' => 'required|max:250', @@ -55,8 +55,10 @@ class UserApiTokenController extends Controller } $token->save(); - // TODO - Notification and activity? + $token->refresh(); + session()->flash('api-token-secret:' . $token->id, $secret); + $this->showSuccessNotification(trans('settings.user_api_token_create_success')); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } @@ -89,7 +91,7 @@ class UserApiTokenController extends Controller [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); $token->fill($request->all())->save(); - // TODO - Notification and activity? + $this->showSuccessNotification(trans('settings.user_api_token_update_success')); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } @@ -113,7 +115,7 @@ class UserApiTokenController extends Controller [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); $token->delete(); - // TODO - Notification and activity?, Might have text in translations already (user_api_token_delete_success) + $this->showSuccessNotification(trans('settings.user_api_token_delete_success')); return redirect($user->getEditUrl('#api_tokens')); } @@ -124,8 +126,9 @@ class UserApiTokenController extends Controller */ protected function checkPermissionAndFetchUserToken(int $userId, int $tokenId): array { - $this->checkPermission('access-api'); - $this->checkPermissionOrCurrentUser('manage-users', $userId); + $this->checkPermissionOr('users-manage', function () use ($userId) { + return $userId === user()->id && userCan('access-api'); + }); $user = User::query()->findOrFail($userId); $token = ApiToken::query()->where('user_id', '=', $user->id)->where('id', '=', $tokenId)->firstOrFail(); diff --git a/database/migrations/2019_12_29_120917_add_api_auth.php b/database/migrations/2019_12_29_120917_add_api_auth.php index 2af0b292e..c8a1a7781 100644 --- a/database/migrations/2019_12_29_120917_add_api_auth.php +++ b/database/migrations/2019_12_29_120917_add_api_auth.php @@ -22,7 +22,7 @@ class AddApiAuth extends Migration $table->string('client_id')->unique(); $table->string('client_secret'); $table->integer('user_id')->unsigned()->index(); - $table->timestamp('expires_at')->index(); + $table->date('expires_at')->index(); $table->nullableTimestamps(); }); diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index a2148361a..88eb22aa0 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -164,6 +164,8 @@ return [ 'user_api_token_expiry' => 'Expiry Date', 'user_api_token_expiry_desc' => 'Set a date at which this token expires. After this date, requests made using this token will no longer work. Leaving this field blank will set an expiry 100 years into the future.', 'user_api_token_create_secret_message' => 'Immediately after creating this token a "client id"" & "client secret" will be generated and displayed. The client secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.', + 'user_api_token_create_success' => 'API token successfully created', + 'user_api_token_update_success' => 'API token successfully updated', 'user_api_token' => 'API Token', 'user_api_token_client_id' => 'Client ID', 'user_api_token_client_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.', diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index 54e0ee21a..ba76b022e 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -88,8 +88,7 @@
@endif - {{-- TODO - Review Control--}} - @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('manage-users')) + @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('users-manage'))

{{ trans('settings.users_api_tokens') }}

diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php new file mode 100644 index 000000000..86c2b7bcc --- /dev/null +++ b/tests/User/UserApiTokenTest.php @@ -0,0 +1,165 @@ + 'My test API token', + 'expires_at' => '2099-04-01', + ]; + + public function test_tokens_section_not_visible_without_access_api_permission() + { + $user = $this->getEditor(); + + $resp = $this->actingAs($user)->get($user->getEditUrl()); + $resp->assertDontSeeText('API Tokens'); + + $this->giveUserPermissions($user, ['access-api']); + + $resp = $this->actingAs($user)->get($user->getEditUrl()); + $resp->assertSeeText('API Tokens'); + $resp->assertSeeText('Create Token'); + } + + public function test_those_with_manage_users_can_view_other_user_tokens_but_not_create() + { + $viewer = $this->getViewer(); + $editor = $this->getEditor(); + $this->giveUserPermissions($editor, ['users-manage']); + + $resp = $this->actingAs($editor)->get($viewer->getEditUrl()); + $resp->assertSeeText('API Tokens'); + $resp->assertDontSeeText('Create Token'); + } + + public function test_create_api_token() + { + $editor = $this->getEditor(); + + $resp = $this->asAdmin()->get($editor->getEditUrl('/create-api-token')); + $resp->assertStatus(200); + $resp->assertSee('Create API Token'); + $resp->assertSee('client secret'); + + $resp = $this->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + $resp->assertRedirect($editor->getEditUrl('/api-tokens/' . $token->id)); + $this->assertDatabaseHas('api_tokens', [ + 'user_id' => $editor->id, + 'name' => $this->testTokenData['name'], + 'expires_at' => $this->testTokenData['expires_at'], + ]); + + // Check secret token + $this->assertSessionHas('api-token-secret:' . $token->id); + $secret = session('api-token-secret:' . $token->id); + $this->assertDatabaseMissing('api_tokens', [ + 'client_secret' => $secret, + ]); + $this->assertTrue(\Hash::check($secret, $token->client_secret)); + + $this->assertTrue(strlen($token->client_id) === 32); + $this->assertTrue(strlen($secret) === 32); + + $this->assertSessionHas('success'); + } + + public function test_create_with_no_expiry_sets_expiry_hundred_years_away() + { + $editor = $this->getEditor(); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), ['name' => 'No expiry token']); + $token = ApiToken::query()->latest()->first(); + + $over = Carbon::now()->addYears(101); + $under = Carbon::now()->addYears(99); + $this->assertTrue( + ($token->expires_at < $over && $token->expires_at > $under), + "Token expiry set at 100 years in future" + ); + } + + public function test_created_token_displays_on_profile_page() + { + $editor = $this->getEditor(); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + + $resp = $this->get($editor->getEditUrl()); + $resp->assertElementExists('#api_tokens'); + $resp->assertElementContains('#api_tokens', $token->name); + $resp->assertElementContains('#api_tokens', $token->client_id); + $resp->assertElementContains('#api_tokens', $token->expires_at->format('Y-m-d')); + } + + public function test_client_secret_shown_once_after_creation() + { + $editor = $this->getEditor(); + $resp = $this->asAdmin()->followingRedirects()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $resp->assertSeeText('Client Secret'); + + $token = ApiToken::query()->latest()->first(); + $this->assertNull(session('api-token-secret:' . $token->id)); + + $resp = $this->get($editor->getEditUrl('/api-tokens/' . $token->id)); + $resp->assertDontSeeText('Client Secret'); + } + + public function test_token_update() + { + $editor = $this->getEditor(); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + $updateData = [ + 'name' => 'My updated token', + 'expires_at' => '2011-01-01', + ]; + + $resp = $this->put($editor->getEditUrl('/api-tokens/' . $token->id), $updateData); + $resp->assertRedirect($editor->getEditUrl('/api-tokens/' . $token->id)); + + $this->assertDatabaseHas('api_tokens', array_merge($updateData, ['id' => $token->id])); + $this->assertSessionHas('success'); + } + + public function test_token_delete() + { + $editor = $this->getEditor(); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + + $tokenUrl = $editor->getEditUrl('/api-tokens/' . $token->id); + + $resp = $this->get($tokenUrl . '/delete'); + $resp->assertSeeText('Delete Token'); + $resp->assertSeeText($token->name); + $resp->assertElementExists('form[action="'.$tokenUrl.'"]'); + + $resp = $this->delete($tokenUrl); + $resp->assertRedirect($editor->getEditUrl('#api_tokens')); + $this->assertDatabaseMissing('api_tokens', ['id' => $token->id]); + } + + public function test_user_manage_can_delete_token_without_api_permission_themselves() + { + $viewer = $this->getViewer(); + $editor = $this->getEditor(); + $this->giveUserPermissions($editor, ['users-manage']); + + $this->asAdmin()->post($viewer->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + + $resp = $this->actingAs($editor)->get($viewer->getEditUrl('/api-tokens/' . $token->id)); + $resp->assertStatus(200); + $resp->assertSeeText('Delete Token'); + + $resp = $this->actingAs($editor)->delete($viewer->getEditUrl('/api-tokens/' . $token->id)); + $resp->assertRedirect($viewer->getEditUrl('#api_tokens')); + $this->assertDatabaseMissing('api_tokens', ['id' => $token->id]); + } + +} \ No newline at end of file diff --git a/tests/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php similarity index 100% rename from tests/UserPreferencesTest.php rename to tests/User/UserPreferencesTest.php diff --git a/tests/UserProfileTest.php b/tests/User/UserProfileTest.php similarity index 100% rename from tests/UserProfileTest.php rename to tests/User/UserProfileTest.php From 692fc46c7dfab76f4e9e28a9f1b4c419f60b5ded Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 29 Dec 2019 20:07:28 +0000 Subject: [PATCH 05/23] Removed token 'client' text, avoid confusion w/ oAuth - Instead have a token_id and a secret. - Displayed a 'Token ID' and 'Token Secret'. --- app/Http/Controllers/UserApiTokenController.php | 8 ++++---- .../migrations/2019_12_29_120917_add_api_auth.php | 4 ++-- resources/lang/en/settings.php | 10 +++++----- resources/views/users/api-tokens/edit.blade.php | 10 +++++----- resources/views/users/edit.blade.php | 2 +- tests/User/UserApiTokenTest.php | 14 +++++++------- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php index 9f5ebc49e..c18d52901 100644 --- a/app/Http/Controllers/UserApiTokenController.php +++ b/app/Http/Controllers/UserApiTokenController.php @@ -44,14 +44,14 @@ class UserApiTokenController extends Controller $token = (new ApiToken())->forceFill([ 'name' => $request->get('name'), - 'client_id' => Str::random(32), - 'client_secret' => Hash::make($secret), + 'token_id' => Str::random(32), + 'secret' => Hash::make($secret), 'user_id' => $user->id, 'expires_at' => $expiry ]); - while (ApiToken::query()->where('client_id', '=', $token->client_id)->exists()) { - $token->client_id = Str::random(32); + while (ApiToken::query()->where('token_id', '=', $token->token_id)->exists()) { + $token->token_id = Str::random(32); } $token->save(); diff --git a/database/migrations/2019_12_29_120917_add_api_auth.php b/database/migrations/2019_12_29_120917_add_api_auth.php index c8a1a7781..eff88247f 100644 --- a/database/migrations/2019_12_29_120917_add_api_auth.php +++ b/database/migrations/2019_12_29_120917_add_api_auth.php @@ -19,8 +19,8 @@ class AddApiAuth extends Migration Schema::create('api_tokens', function(Blueprint $table) { $table->increments('id'); $table->string('name'); - $table->string('client_id')->unique(); - $table->string('client_secret'); + $table->string('token_id')->unique(); + $table->string('secret'); $table->integer('user_id')->unsigned()->index(); $table->date('expires_at')->index(); $table->nullableTimestamps(); diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 88eb22aa0..b1da5435f 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -163,14 +163,14 @@ return [ 'user_api_token_name_desc' => 'Give your token a readable name as a future reminder of its intended purpose.', 'user_api_token_expiry' => 'Expiry Date', 'user_api_token_expiry_desc' => 'Set a date at which this token expires. After this date, requests made using this token will no longer work. Leaving this field blank will set an expiry 100 years into the future.', - 'user_api_token_create_secret_message' => 'Immediately after creating this token a "client id"" & "client secret" will be generated and displayed. The client secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.', + 'user_api_token_create_secret_message' => 'Immediately after creating this token a "Token ID"" & "Token Secret" will be generated and displayed. The secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.', 'user_api_token_create_success' => 'API token successfully created', 'user_api_token_update_success' => 'API token successfully updated', 'user_api_token' => 'API Token', - 'user_api_token_client_id' => 'Client ID', - 'user_api_token_client_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.', - 'user_api_token_client_secret' => 'Client Secret', - 'user_api_token_client_secret_desc' => 'This is a system generated secret for this token which will need to be provided in API requests. This will only be displayed this one time so copy this value to somewhere safe and secure.', + 'user_api_token_id' => 'Token ID', + 'user_api_token_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.', + 'user_api_token_secret' => 'Token Secret', + 'user_api_token_secret_desc' => 'This is a system generated secret for this token which will need to be provided in API requests. This will only be displayed this one time so copy this value to somewhere safe and secure.', 'user_api_token_created' => 'Token Created :timeAgo', 'user_api_token_updated' => 'Token Updated :timeAgo', 'user_api_token_delete' => 'Delete Token', diff --git a/resources/views/users/api-tokens/edit.blade.php b/resources/views/users/api-tokens/edit.blade.php index 0ec9adbe6..821a00d93 100644 --- a/resources/views/users/api-tokens/edit.blade.php +++ b/resources/views/users/api-tokens/edit.blade.php @@ -15,11 +15,11 @@
- -

{{ trans('settings.user_api_token_client_id_desc') }}

+ +

{{ trans('settings.user_api_token_id_desc') }}

- @include('form.text', ['name' => 'client_id', 'readonly' => true]) + @include('form.text', ['name' => 'token_id', 'readonly' => true])
@@ -27,8 +27,8 @@ @if( $secret )
- -

{{ trans('settings.user_api_token_client_secret_desc') }}

+ +

{{ trans('settings.user_api_token_secret_desc') }}

diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index ba76b022e..c69d101d4 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -109,7 +109,7 @@ {{ $token->name }}
- {{ $token->client_id }} + {{ $token->token_id }} {{ $token->expires_at->format('Y-m-d') ?? '' }} diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php index 86c2b7bcc..012747296 100644 --- a/tests/User/UserApiTokenTest.php +++ b/tests/User/UserApiTokenTest.php @@ -44,7 +44,7 @@ class UserApiTokenTest extends TestCase $resp = $this->asAdmin()->get($editor->getEditUrl('/create-api-token')); $resp->assertStatus(200); $resp->assertSee('Create API Token'); - $resp->assertSee('client secret'); + $resp->assertSee('Token Secret'); $resp = $this->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); $token = ApiToken::query()->latest()->first(); @@ -59,11 +59,11 @@ class UserApiTokenTest extends TestCase $this->assertSessionHas('api-token-secret:' . $token->id); $secret = session('api-token-secret:' . $token->id); $this->assertDatabaseMissing('api_tokens', [ - 'client_secret' => $secret, + 'secret' => $secret, ]); - $this->assertTrue(\Hash::check($secret, $token->client_secret)); + $this->assertTrue(\Hash::check($secret, $token->secret)); - $this->assertTrue(strlen($token->client_id) === 32); + $this->assertTrue(strlen($token->token_id) === 32); $this->assertTrue(strlen($secret) === 32); $this->assertSessionHas('success'); @@ -92,15 +92,15 @@ class UserApiTokenTest extends TestCase $resp = $this->get($editor->getEditUrl()); $resp->assertElementExists('#api_tokens'); $resp->assertElementContains('#api_tokens', $token->name); - $resp->assertElementContains('#api_tokens', $token->client_id); + $resp->assertElementContains('#api_tokens', $token->token_id); $resp->assertElementContains('#api_tokens', $token->expires_at->format('Y-m-d')); } - public function test_client_secret_shown_once_after_creation() + public function test_secret_shown_once_after_creation() { $editor = $this->getEditor(); $resp = $this->asAdmin()->followingRedirects()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); - $resp->assertSeeText('Client Secret'); + $resp->assertSeeText('Token Secret'); $token = ApiToken::query()->latest()->first(); $this->assertNull(session('api-token-secret:' . $token->id)); From 2cfa37399c6b7f5b743db8223dd90d3dba68d6fa Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 29 Dec 2019 20:18:37 +0000 Subject: [PATCH 06/23] Fixed some empty-expiry conditions of token ui flows --- .../Controllers/UserApiTokenController.php | 14 ++++++++--- tests/User/UserApiTokenTest.php | 24 +++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php index c18d52901..547ec0c2b 100644 --- a/app/Http/Controllers/UserApiTokenController.php +++ b/app/Http/Controllers/UserApiTokenController.php @@ -40,7 +40,11 @@ class UserApiTokenController extends Controller $user = User::query()->findOrFail($userId); $secret = Str::random(32); - $expiry = $request->get('expires_at', (Carbon::now()->addYears(100))->format('Y-m-d')); + + $expiry = $request->get('expires_at', null); + if (empty($expiry)) { + $expiry = Carbon::now()->addYears(100)->format('Y-m-d'); + } $token = (new ApiToken())->forceFill([ 'name' => $request->get('name'), @@ -83,14 +87,18 @@ class UserApiTokenController extends Controller */ public function update(Request $request, int $userId, int $tokenId) { - $this->validate($request, [ + $requestData = $this->validate($request, [ 'name' => 'required|max:250', 'expires_at' => 'date_format:Y-m-d', ]); [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); - $token->fill($request->all())->save(); + if (empty($requestData['expires_at'])) { + $requestData['expires_at'] = Carbon::now()->addYears(100)->format('Y-m-d'); + } + + $token->fill($requestData)->save(); $this->showSuccessNotification(trans('settings.user_api_token_update_success')); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php index 012747296..460752fc2 100644 --- a/tests/User/UserApiTokenTest.php +++ b/tests/User/UserApiTokenTest.php @@ -9,7 +9,7 @@ class UserApiTokenTest extends TestCase protected $testTokenData = [ 'name' => 'My test API token', - 'expires_at' => '2099-04-01', + 'expires_at' => '2050-04-01', ]; public function test_tokens_section_not_visible_without_access_api_permission() @@ -72,7 +72,7 @@ class UserApiTokenTest extends TestCase public function test_create_with_no_expiry_sets_expiry_hundred_years_away() { $editor = $this->getEditor(); - $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), ['name' => 'No expiry token']); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), ['name' => 'No expiry token', 'expires_at' => '']); $token = ApiToken::query()->latest()->first(); $over = Carbon::now()->addYears(101); @@ -126,6 +126,26 @@ class UserApiTokenTest extends TestCase $this->assertSessionHas('success'); } + public function test_token_update_with_blank_expiry_sets_to_hundred_years_away() + { + $editor = $this->getEditor(); + $this->asAdmin()->post($editor->getEditUrl('/create-api-token'), $this->testTokenData); + $token = ApiToken::query()->latest()->first(); + + $resp = $this->put($editor->getEditUrl('/api-tokens/' . $token->id), [ + 'name' => 'My updated token', + 'expires_at' => '', + ]); + $token->refresh(); + + $over = Carbon::now()->addYears(101); + $under = Carbon::now()->addYears(99); + $this->assertTrue( + ($token->expires_at < $over && $token->expires_at > $under), + "Token expiry set at 100 years in future" + ); + } + public function test_token_delete() { $editor = $this->getEditor(); From 3de55ee6454667e2d4b3cb866625a165ccb9aee3 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 02:16:07 +0000 Subject: [PATCH 07/23] Linked new API token system into middleware Base logic in place but needs review and refactor to see if can better fit into Laravel using 'Guard' system. Currently has issues due to cookies in use from active session on API. --- app/Api/ApiToken.php | 10 ++++ app/Http/Kernel.php | 30 ++++++++-- app/Http/Middleware/ApiAuthenticate.php | 78 +++++++++++++++++++++++++ app/Http/Middleware/Authenticate.php | 29 +-------- app/Http/Middleware/ConfirmEmails.php | 60 +++++++++++++++++++ app/helpers.php | 5 -- resources/lang/en/errors.php | 8 +++ 7 files changed, 183 insertions(+), 37 deletions(-) create mode 100644 app/Http/Middleware/ApiAuthenticate.php create mode 100644 app/Http/Middleware/ConfirmEmails.php diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php index 4ea12888e..cdcb33a7b 100644 --- a/app/Api/ApiToken.php +++ b/app/Api/ApiToken.php @@ -1,6 +1,8 @@ 'date:Y-m-d' ]; + + /** + * Get the user that this token belongs to. + */ + public function user(): BelongsTo + { + return $this->belongsTo(User::class); + } } diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index cd3fc83ec..64782fedc 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -1,21 +1,38 @@ [ 'throttle:60,1', + \BookStack\Http\Middleware\EncryptCookies::class, + \Illuminate\Session\Middleware\StartSession::class, + \BookStack\Http\Middleware\ApiAuthenticate::class, + \BookStack\Http\Middleware\ConfirmEmails::class, ], ]; @@ -47,7 +68,6 @@ class Kernel extends HttpKernel */ protected $routeMiddleware = [ 'auth' => \BookStack\Http\Middleware\Authenticate::class, - 'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class, 'can' => \Illuminate\Auth\Middleware\Authorize::class, 'guest' => \BookStack\Http\Middleware\RedirectIfAuthenticated::class, 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php new file mode 100644 index 000000000..3e68cb3ae --- /dev/null +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -0,0 +1,78 @@ +cookies->has($sessionCookieName)) { +// $sessionCookie = $request->cookies->get($sessionCookieName); +// $sessionCookie = decrypt($sessionCookie, false); +// dd($sessionCookie); +// } + + // 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()) { + return $next($request); + } + + $authToken = trim($request->header('Authorization', '')); + if (empty($authToken)) { + return $this->unauthorisedResponse(trans('errors.api_no_authorization_found')); + } + + if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) { + return $this->unauthorisedResponse(trans('errors.api_bad_authorization_format')); + } + + [$id, $secret] = explode(':', str_replace('Token ', '', $authToken)); + $token = ApiToken::query() + ->where('token_id', '=', $id) + ->with(['user'])->first(); + + if ($token === null) { + return $this->unauthorisedResponse(trans('errors.api_user_token_not_found')); + } + + if (!Hash::check($secret, $token->secret)) { + return $this->unauthorisedResponse(trans('errors.api_incorrect_token_secret')); + } + + if (!$token->user->can('access-api')) { + return $this->unauthorisedResponse(trans('errors.api_user_no_api_permission'), 403); + } + + auth()->login($token->user); + + return $next($request); + } + + /** + * Provide a standard API unauthorised response. + */ + protected function unauthorisedResponse(string $message, int $code = 401) + { + return response()->json([ + 'error' => [ + 'code' => $code, + 'message' => $message, + ] + ], 401); + } +} diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index d840a9b2e..40acc254b 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -2,41 +2,16 @@ namespace BookStack\Http\Middleware; +use BookStack\Http\Request; use Closure; -use Illuminate\Contracts\Auth\Guard; class Authenticate { - /** - * The Guard implementation. - * @var Guard - */ - protected $auth; - - /** - * Create a new filter instance. - * @param Guard $auth - */ - public function __construct(Guard $auth) - { - $this->auth = $auth; - } - /** * Handle an incoming request. - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed */ - public function handle($request, Closure $next) + public function handle(Request $request, Closure $next) { - if ($this->auth->check()) { - $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict')); - if ($requireConfirmation && !$this->auth->user()->email_confirmed) { - return redirect('/register/confirm/awaiting'); - } - } - if (!hasAppAccess()) { if ($request->ajax()) { return response('Unauthorized.', 401); diff --git a/app/Http/Middleware/ConfirmEmails.php b/app/Http/Middleware/ConfirmEmails.php new file mode 100644 index 000000000..3700e9973 --- /dev/null +++ b/app/Http/Middleware/ConfirmEmails.php @@ -0,0 +1,60 @@ +auth = $auth; + } + + /** + * Handle an incoming request. + */ + public function handle(Request $request, Closure $next) + { + if ($this->auth->check()) { + $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict')); + if ($requireConfirmation && !$this->auth->user()->email_confirmed) { + return $this->errorResponse($request); + } + } + + return $next($request); + } + + /** + * Provide an error response for when the current user's email is not confirmed + * in a system which requires it. + */ + protected function errorResponse(Request $request) + { + if ($request->wantsJson()) { + return response()->json([ + 'error' => [ + 'code' => 401, + 'message' => trans('errors.email_confirmation_awaiting') + ] + ], 401); + } + + return redirect('/register/confirm/awaiting'); + } +} diff --git a/app/helpers.php b/app/helpers.php index 6211f41be..65da1853b 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -42,7 +42,6 @@ function user(): User /** * Check if current user is a signed in user. - * @return bool */ function signedInUser(): bool { @@ -51,7 +50,6 @@ function signedInUser(): bool /** * Check if the current user has general access. - * @return bool */ function hasAppAccess(): bool { @@ -62,9 +60,6 @@ function hasAppAccess(): bool * Check if the current user has a permission. * If an ownable element is passed in the jointPermissions are checked against * that particular item. - * @param string $permission - * @param Ownable $ownable - * @return bool */ function userCan(string $permission, Ownable $ownable = null): bool { diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index a7c591c5d..85c498f48 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -13,6 +13,7 @@ return [ 'email_already_confirmed' => 'Email has already been confirmed, Try logging in.', 'email_confirmation_invalid' => 'This confirmation token is not valid or has already been used, Please try registering again.', 'email_confirmation_expired' => 'The confirmation token has expired, A new confirmation email has been sent.', + 'email_confirmation_awaiting' => 'The email address for the account in use needs to be confirmed', 'ldap_fail_anonymous' => 'LDAP access failed using anonymous bind', 'ldap_fail_authed' => 'LDAP access failed using given dn & password details', 'ldap_extension_not_installed' => 'LDAP PHP extension not installed', @@ -88,4 +89,11 @@ return [ 'app_down' => ':appName is down right now', 'back_soon' => 'It will be back up soon.', + // API errors + 'api_no_authorization_found' => 'No authorization token found on the request', + 'api_bad_authorization_format' => 'An authorization token was found on the request but the format appeared incorrect', + 'api_user_token_not_found' => 'No matching API token was found for the provided authorization token', + 'api_incorrect_token_secret' => 'The secret provided for the given used API token is incorrect', + 'api_user_no_api_permission' => 'The owner of the used API token does not have permission to make API calls', + ]; From 349b4629bef25e4631c1024749726415507b2cd5 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 14:51:28 +0000 Subject: [PATCH 08/23] Extracted API auth into guard Also implemented more elegant solution to allowing session auth for API routes; A new 'StartSessionIfCookieExists' middleware, which wraps the default 'StartSession' middleware will run for API routes which only sets up the session if a session cookie is found on the request. Also decrypts only the session cookie. Also cleaned some TokenController codeclimate warnings. --- app/Api/ApiToken.php | 10 ++ app/Api/ApiTokenGuard.php | 135 ++++++++++++++++++ app/Config/auth.php | 4 +- app/Exceptions/ApiAuthException.php | 17 +++ .../Controllers/UserApiTokenController.php | 19 +-- app/Http/Kernel.php | 5 +- app/Http/Middleware/ApiAuthenticate.php | 50 ++----- .../Middleware/StartSessionIfCookieExists.php | 39 +++++ app/Providers/AuthServiceProvider.php | 5 +- 9 files changed, 224 insertions(+), 60 deletions(-) create mode 100644 app/Api/ApiTokenGuard.php create mode 100644 app/Exceptions/ApiAuthException.php create mode 100644 app/Http/Middleware/StartSessionIfCookieExists.php diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php index cdcb33a7b..523c3b8b8 100644 --- a/app/Api/ApiToken.php +++ b/app/Api/ApiToken.php @@ -3,6 +3,7 @@ use BookStack\Auth\User; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Support\Carbon; class ApiToken extends Model { @@ -18,4 +19,13 @@ class ApiToken extends Model { return $this->belongsTo(User::class); } + + /** + * Get the default expiry value for an API token. + * Set to 100 years from now. + */ + public static function defaultExpiry(): string + { + return Carbon::now()->addYears(100)->format('Y-m-d'); + } } diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php new file mode 100644 index 000000000..b347e536a --- /dev/null +++ b/app/Api/ApiTokenGuard.php @@ -0,0 +1,135 @@ +request = $request; + } + + + /** + * @inheritDoc + */ + public function user() + { + // Return the user if we've already retrieved them. + // Effectively a request-instance cache for this method. + if (!is_null($this->user)) { + return $this->user; + } + + $user = null; + try { + $user = $this->getAuthorisedUserFromRequest(); + } catch (ApiAuthException $exception) { + $this->lastAuthException = $exception; + } + + $this->user = $user; + return $user; + } + + /** + * Determine if current user is authenticated. If not, throw an exception. + * + * @return \Illuminate\Contracts\Auth\Authenticatable + * + * @throws ApiAuthException + */ + public function authenticate() + { + if (! is_null($user = $this->user())) { + return $user; + } + + if ($this->lastAuthException) { + throw $this->lastAuthException; + } + + throw new ApiAuthException('Unauthorized'); + } + + /** + * Check the API token in the request and fetch a valid authorised user. + * @throws ApiAuthException + */ + protected function getAuthorisedUserFromRequest(): Authenticatable + { + $authToken = trim($this->request->headers->get('Authorization', '')); + if (empty($authToken)) { + throw new ApiAuthException(trans('errors.api_no_authorization_found')); + } + + if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) { + throw new ApiAuthException(trans('errors.api_bad_authorization_format')); + } + + [$id, $secret] = explode(':', str_replace('Token ', '', $authToken)); + $token = ApiToken::query() + ->where('token_id', '=', $id) + ->with(['user'])->first(); + + if ($token === null) { + throw new ApiAuthException(trans('errors.api_user_token_not_found')); + } + + if (!Hash::check($secret, $token->secret)) { + throw new ApiAuthException(trans('errors.api_incorrect_token_secret')); + } + + if (!$token->user->can('access-api')) { + throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); + } + + return $token->user; + } + + /** + * @inheritDoc + */ + public function validate(array $credentials = []) + { + if (empty($credentials['id']) || empty($credentials['secret'])) { + return false; + } + + $token = ApiToken::query() + ->where('token_id', '=', $credentials['id']) + ->with(['user'])->first(); + + if ($token === null) { + return false; + } + + return Hash::check($credentials['secret'], $token->secret); + } + +} \ No newline at end of file diff --git a/app/Config/auth.php b/app/Config/auth.php index 5535a6f9c..b3e22c7e1 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -34,9 +34,7 @@ return [ ], 'api' => [ - 'driver' => 'token', - 'provider' => 'users', - 'hash' => false, + 'driver' => 'api-token', ], ], diff --git a/app/Exceptions/ApiAuthException.php b/app/Exceptions/ApiAuthException.php new file mode 100644 index 000000000..0851dfa4a --- /dev/null +++ b/app/Exceptions/ApiAuthException.php @@ -0,0 +1,17 @@ +findOrFail($userId); $secret = Str::random(32); - $expiry = $request->get('expires_at', null); - if (empty($expiry)) { - $expiry = Carbon::now()->addYears(100)->format('Y-m-d'); - } - $token = (new ApiToken())->forceFill([ 'name' => $request->get('name'), 'token_id' => Str::random(32), 'secret' => Hash::make($secret), 'user_id' => $user->id, - 'expires_at' => $expiry + 'expires_at' => $request->get('expires_at') ?: ApiToken::defaultExpiry(), ]); while (ApiToken::query()->where('token_id', '=', $token->token_id)->exists()) { @@ -59,7 +54,6 @@ class UserApiTokenController extends Controller } $token->save(); - $token->refresh(); session()->flash('api-token-secret:' . $token->id, $secret); $this->showSuccessNotification(trans('settings.user_api_token_create_success')); @@ -87,18 +81,17 @@ class UserApiTokenController extends Controller */ public function update(Request $request, int $userId, int $tokenId) { - $requestData = $this->validate($request, [ + $this->validate($request, [ 'name' => 'required|max:250', 'expires_at' => 'date_format:Y-m-d', ]); [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); + $token->fill([ + 'name' => $request->get('name'), + 'expires_at' => $request->get('expires_at') ?: ApiToken::defaultExpiry(), + ])->save(); - if (empty($requestData['expires_at'])) { - $requestData['expires_at'] = Carbon::now()->addYears(100)->format('Y-m-d'); - } - - $token->fill($requestData)->save(); $this->showSuccessNotification(trans('settings.user_api_token_update_success')); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 64782fedc..6a6e736b9 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -1,6 +1,5 @@ [ 'throttle:60,1', - \BookStack\Http\Middleware\EncryptCookies::class, - \Illuminate\Session\Middleware\StartSession::class, + \BookStack\Http\Middleware\StartSessionIfCookieExists::class, \BookStack\Http\Middleware\ApiAuthenticate::class, \BookStack\Http\Middleware\ConfirmEmails::class, ], diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index 3e68cb3ae..86fb83d58 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -2,10 +2,9 @@ namespace BookStack\Http\Middleware; -use BookStack\Api\ApiToken; +use BookStack\Exceptions\ApiAuthException; use BookStack\Http\Request; use Closure; -use Hash; class ApiAuthenticate { @@ -15,58 +14,29 @@ class ApiAuthenticate */ public function handle(Request $request, Closure $next) { - // TODO - Look to extract a lot of the logic here into a 'Guard' - // Ideally would like to be able to request API via browser without having to boot - // the session middleware (in Kernel). - -// $sessionCookieName = config('session.cookie'); -// if ($request->cookies->has($sessionCookieName)) { -// $sessionCookie = $request->cookies->get($sessionCookieName); -// $sessionCookie = decrypt($sessionCookie, false); -// dd($sessionCookie); -// } - // 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()) { return $next($request); } - $authToken = trim($request->header('Authorization', '')); - if (empty($authToken)) { - return $this->unauthorisedResponse(trans('errors.api_no_authorization_found')); + // Set our api guard to be the default for this request lifecycle. + auth()->shouldUse('api'); + + // Validate the token and it's users API access + try { + auth()->authenticate(); + } catch (ApiAuthException $exception) { + return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode()); } - if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) { - return $this->unauthorisedResponse(trans('errors.api_bad_authorization_format')); - } - - [$id, $secret] = explode(':', str_replace('Token ', '', $authToken)); - $token = ApiToken::query() - ->where('token_id', '=', $id) - ->with(['user'])->first(); - - if ($token === null) { - return $this->unauthorisedResponse(trans('errors.api_user_token_not_found')); - } - - if (!Hash::check($secret, $token->secret)) { - return $this->unauthorisedResponse(trans('errors.api_incorrect_token_secret')); - } - - if (!$token->user->can('access-api')) { - return $this->unauthorisedResponse(trans('errors.api_user_no_api_permission'), 403); - } - - auth()->login($token->user); - return $next($request); } /** * Provide a standard API unauthorised response. */ - protected function unauthorisedResponse(string $message, int $code = 401) + protected function unauthorisedResponse(string $message, int $code) { return response()->json([ 'error' => [ diff --git a/app/Http/Middleware/StartSessionIfCookieExists.php b/app/Http/Middleware/StartSessionIfCookieExists.php new file mode 100644 index 000000000..99553e294 --- /dev/null +++ b/app/Http/Middleware/StartSessionIfCookieExists.php @@ -0,0 +1,39 @@ +cookies->has($sessionCookieName)) { + $this->decryptSessionCookie($request, $sessionCookieName); + return parent::handle($request, $next); + } + + return $next($request); + } + + /** + * Attempt decryption of the session cookie. + */ + protected function decryptSessionCookie(Request $request, string $sessionCookieName) + { + try { + $sessionCookie = $request->cookies->get($sessionCookieName); + $sessionCookie = decrypt($sessionCookie, false); + $request->cookies->set($sessionCookieName, $sessionCookie); + } catch (Exception $e) { + // + } + } +} diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 6e5b6ffde..ab7dd5195 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -3,6 +3,7 @@ namespace BookStack\Providers; use Auth; +use BookStack\Api\ApiTokenGuard; use BookStack\Auth\Access\LdapService; use Illuminate\Support\ServiceProvider; @@ -15,7 +16,9 @@ class AuthServiceProvider extends ServiceProvider */ public function boot() { - // + Auth::extend('api-token', function ($app, $name, array $config) { + return new ApiTokenGuard($app['request']); + }); } /** From 6f1b88a6a6402c7acfdd3e9bef72f50eb5e975c1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 15:46:12 +0000 Subject: [PATCH 09/23] Change email confirmation from own middle to trait Email confirmation middleware caused more mess than good, As caused priority issues and it depended on auth actions. Instead its now a trai used on auth middlewares. Also used 'EncryptCookies' middleware on API instead of custom decryption in custom middleware since we'd need to do replicate all the same actions anyway. Shouldn't have too much effect since it only actions over cookies that exist, of which none should be there for most API requests. Also split out some large guard functions to be a little more readable and appease codeclimate. --- app/Api/ApiTokenGuard.php | 36 ++++++++--- app/Http/Kernel.php | 23 +------ app/Http/Middleware/ApiAuthenticate.php | 10 +++- app/Http/Middleware/Authenticate.php | 8 ++- .../Middleware/ChecksForEmailConfirmation.php | 42 +++++++++++++ app/Http/Middleware/ConfirmEmails.php | 60 ------------------- .../Middleware/StartSessionIfCookieExists.php | 17 ------ 7 files changed, 86 insertions(+), 110 deletions(-) create mode 100644 app/Http/Middleware/ChecksForEmailConfirmation.php delete mode 100644 app/Http/Middleware/ConfirmEmails.php diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php index b347e536a..cd9c3b178 100644 --- a/app/Api/ApiTokenGuard.php +++ b/app/Api/ApiTokenGuard.php @@ -33,8 +33,7 @@ class ApiTokenGuard implements Guard { $this->request = $request; } - - + /** * @inheritDoc */ @@ -84,6 +83,24 @@ class ApiTokenGuard implements Guard protected function getAuthorisedUserFromRequest(): Authenticatable { $authToken = trim($this->request->headers->get('Authorization', '')); + $this->validateTokenHeaderValue($authToken); + + [$id, $secret] = explode(':', str_replace('Token ', '', $authToken)); + $token = ApiToken::query() + ->where('token_id', '=', $id) + ->with(['user'])->first(); + + $this->validateToken($token, $secret); + + return $token->user; + } + + /** + * Validate the format of the token header value string. + * @throws ApiAuthException + */ + protected function validateTokenHeaderValue(string $authToken): void + { if (empty($authToken)) { throw new ApiAuthException(trans('errors.api_no_authorization_found')); } @@ -91,12 +108,15 @@ class ApiTokenGuard implements Guard if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) { throw new ApiAuthException(trans('errors.api_bad_authorization_format')); } + } - [$id, $secret] = explode(':', str_replace('Token ', '', $authToken)); - $token = ApiToken::query() - ->where('token_id', '=', $id) - ->with(['user'])->first(); - + /** + * Validate the given secret against the given token and ensure the token + * currently has access to the instance API. + * @throws ApiAuthException + */ + protected function validateToken(?ApiToken $token, string $secret): void + { if ($token === null) { throw new ApiAuthException(trans('errors.api_user_token_not_found')); } @@ -108,8 +128,6 @@ class ApiTokenGuard implements Guard if (!$token->user->can('access-api')) { throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); } - - return $token->user; } /** diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 6a6e736b9..978583a7f 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -13,26 +13,6 @@ class Kernel extends HttpKernel \Illuminate\Foundation\Http\Middleware\ValidatePostSize::class, \BookStack\Http\Middleware\TrimStrings::class, \BookStack\Http\Middleware\TrustProxies::class, - - ]; - - /** - * The priority ordering of middleware. - */ - protected $middlewarePriority = [ - \BookStack\Http\Middleware\EncryptCookies::class, - \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class, - \Illuminate\Session\Middleware\StartSession::class, - \BookStack\Http\Middleware\StartSessionIfCookieExists::class, - \Illuminate\View\Middleware\ShareErrorsFromSession::class, - \Illuminate\Routing\Middleware\ThrottleRequests::class, - \BookStack\Http\Middleware\VerifyCsrfToken::class, - \Illuminate\Routing\Middleware\SubstituteBindings::class, - \BookStack\Http\Middleware\Localization::class, - \BookStack\Http\Middleware\GlobalViewData::class, - \BookStack\Http\Middleware\Authenticate::class, - \BookStack\Http\Middleware\ApiAuthenticate::class, - \BookStack\Http\Middleware\ConfirmEmails::class, ]; /** @@ -50,13 +30,12 @@ class Kernel extends HttpKernel \BookStack\Http\Middleware\VerifyCsrfToken::class, \BookStack\Http\Middleware\Localization::class, \BookStack\Http\Middleware\GlobalViewData::class, - \BookStack\Http\Middleware\ConfirmEmails::class, ], 'api' => [ 'throttle:60,1', + \BookStack\Http\Middleware\EncryptCookies::class, \BookStack\Http\Middleware\StartSessionIfCookieExists::class, \BookStack\Http\Middleware\ApiAuthenticate::class, - \BookStack\Http\Middleware\ConfirmEmails::class, ], ]; diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index 86fb83d58..c7fed405c 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -3,11 +3,12 @@ namespace BookStack\Http\Middleware; use BookStack\Exceptions\ApiAuthException; -use BookStack\Http\Request; use Closure; +use Illuminate\Http\Request; class ApiAuthenticate { + use ChecksForEmailConfirmation; /** * Handle an incoming request. @@ -17,6 +18,9 @@ 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()) { + if ($this->awaitingEmailConfirmation()) { + return $this->emailConfirmationErrorResponse($request); + } return $next($request); } @@ -30,6 +34,10 @@ class ApiAuthenticate return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode()); } + if ($this->awaitingEmailConfirmation()) { + return $this->emailConfirmationErrorResponse($request); + } + return $next($request); } diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 40acc254b..a171a8a2d 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -2,16 +2,22 @@ namespace BookStack\Http\Middleware; -use BookStack\Http\Request; use Closure; +use Illuminate\Http\Request; class Authenticate { + use ChecksForEmailConfirmation; + /** * Handle an incoming request. */ public function handle(Request $request, Closure $next) { + if ($this->awaitingEmailConfirmation()) { + return $this->emailConfirmationErrorResponse($request); + } + if (!hasAppAccess()) { if ($request->ajax()) { return response('Unauthorized.', 401); diff --git a/app/Http/Middleware/ChecksForEmailConfirmation.php b/app/Http/Middleware/ChecksForEmailConfirmation.php new file mode 100644 index 000000000..684a7e9bc --- /dev/null +++ b/app/Http/Middleware/ChecksForEmailConfirmation.php @@ -0,0 +1,42 @@ +check()) { + $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict')); + if ($requireConfirmation && !auth()->user()->email_confirmed) { + return true; + } + } + + return false; + } + + /** + * 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) + { + if ($request->wantsJson()) { + return response()->json([ + 'error' => [ + 'code' => 401, + 'message' => trans('errors.email_confirmation_awaiting') + ] + ], 401); + } + + return redirect('/register/confirm/awaiting'); + } +} \ No newline at end of file diff --git a/app/Http/Middleware/ConfirmEmails.php b/app/Http/Middleware/ConfirmEmails.php deleted file mode 100644 index 3700e9973..000000000 --- a/app/Http/Middleware/ConfirmEmails.php +++ /dev/null @@ -1,60 +0,0 @@ -auth = $auth; - } - - /** - * Handle an incoming request. - */ - public function handle(Request $request, Closure $next) - { - if ($this->auth->check()) { - $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict')); - if ($requireConfirmation && !$this->auth->user()->email_confirmed) { - return $this->errorResponse($request); - } - } - - return $next($request); - } - - /** - * Provide an error response for when the current user's email is not confirmed - * in a system which requires it. - */ - protected function errorResponse(Request $request) - { - if ($request->wantsJson()) { - return response()->json([ - 'error' => [ - 'code' => 401, - 'message' => trans('errors.email_confirmation_awaiting') - ] - ], 401); - } - - return redirect('/register/confirm/awaiting'); - } -} diff --git a/app/Http/Middleware/StartSessionIfCookieExists.php b/app/Http/Middleware/StartSessionIfCookieExists.php index 99553e294..456508d98 100644 --- a/app/Http/Middleware/StartSessionIfCookieExists.php +++ b/app/Http/Middleware/StartSessionIfCookieExists.php @@ -2,9 +2,7 @@ namespace BookStack\Http\Middleware; -use BookStack\Http\Request; use Closure; -use Exception; use Illuminate\Session\Middleware\StartSession as Middleware; class StartSessionIfCookieExists extends Middleware @@ -16,24 +14,9 @@ class StartSessionIfCookieExists extends Middleware { $sessionCookieName = config('session.cookie'); if ($request->cookies->has($sessionCookieName)) { - $this->decryptSessionCookie($request, $sessionCookieName); return parent::handle($request, $next); } return $next($request); } - - /** - * Attempt decryption of the session cookie. - */ - protected function decryptSessionCookie(Request $request, string $sessionCookieName) - { - try { - $sessionCookie = $request->cookies->get($sessionCookieName); - $sessionCookie = decrypt($sessionCookie, false); - $request->cookies->set($sessionCookieName, $sessionCookie); - } catch (Exception $e) { - // - } - } } From 3d11cba223cad16ad13faee010259e97b05dcee9 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 19:42:46 +0000 Subject: [PATCH 10/23] Added testing coverage to API token auth --- app/Api/ApiTokenGuard.php | 7 ++ app/Auth/Role.php | 2 +- app/Http/Middleware/ApiAuthenticate.php | 2 +- .../Middleware/ChecksForEmailConfirmation.php | 4 +- database/seeds/DummyContentSeeder.php | 14 ++++ tests/Api/ApiAuthTest.php | 82 +++++++++++++++++++ tests/TestsApi.php | 16 ++++ tests/User/UserApiTokenTest.php | 6 +- 8 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 tests/Api/ApiAuthTest.php create mode 100644 tests/TestsApi.php 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 @@ 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 @@ +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 @@ + ["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'); } From 3cacda6762bca67ae2beeb44cdcff39ad6d7ec60 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 19:51:41 +0000 Subject: [PATCH 11/23] Added expiry checking to API token auth - Added test to cover to ensure its checked going forward --- app/Api/ApiTokenGuard.php | 6 ++++++ resources/lang/en/errors.php | 1 + tests/Api/ApiAuthTest.php | 24 ++++++++++++++++++++---- tests/TestsApi.php | 7 +++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php index ba0b4b5dd..e0a50ebe3 100644 --- a/app/Api/ApiTokenGuard.php +++ b/app/Api/ApiTokenGuard.php @@ -6,6 +6,7 @@ use BookStack\Exceptions\ApiAuthException; use Illuminate\Auth\GuardHelpers; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\Guard; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Hash; use Symfony\Component\HttpFoundation\Request; @@ -125,6 +126,11 @@ class ApiTokenGuard implements Guard throw new ApiAuthException(trans('errors.api_incorrect_token_secret')); } + $now = Carbon::now(); + if ($token->expires_at <= $now) { + throw new ApiAuthException(trans('errors.api_user_token_expired'), 403); + } + if (!$token->user->can('access-api')) { throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); } diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 85c498f48..bb7b6148c 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -95,5 +95,6 @@ return [ 'api_user_token_not_found' => 'No matching API token was found for the provided authorization token', 'api_incorrect_token_secret' => 'The secret provided for the given used API token is incorrect', 'api_user_no_api_permission' => 'The owner of the used API token does not have permission to make API calls', + 'api_user_token_expired' => 'The authorization token used has expired', ]; diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php index ef975d556..30d7f4ead 100644 --- a/tests/Api/ApiAuthTest.php +++ b/tests/Api/ApiAuthTest.php @@ -3,6 +3,7 @@ namespace Tests; use BookStack\Auth\Permissions\RolePermission; +use Carbon\Carbon; class ApiAuthTest extends TestCase { @@ -52,7 +53,7 @@ class ApiAuthTest extends TestCase public function test_api_access_permission_required_to_access_api() { - $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:{$this->apiTokenSecret}"]); + $resp = $this->get($this->endpoint, $this->apiAuthHeader()); $resp->assertStatus(200); auth()->logout(); @@ -60,12 +61,27 @@ class ApiAuthTest extends TestCase $editorRole = $this->getEditor()->roles()->first(); $editorRole->detachPermission($accessApiPermission); - $resp = $this->get($this->endpoint, ['Authorization' => "Token {$this->apiTokenId}:{$this->apiTokenSecret}"]); + $resp = $this->get($this->endpoint, $this->apiAuthHeader()); $resp->assertJson($this->errorResponse("The owner of the used API token does not have permission to make API calls", 403)); } + public function test_token_expiry_checked() + { + $editor = $this->getEditor(); + $token = $editor->apiTokens()->first(); - public function test_email_confirmation_checked_on_auth_requets() + $resp = $this->get($this->endpoint, $this->apiAuthHeader()); + $resp->assertStatus(200); + auth()->logout(); + + $token->expires_at = Carbon::now()->subDay()->format('Y-m-d'); + $token->save(); + + $resp = $this->get($this->endpoint, $this->apiAuthHeader()); + $resp->assertJson($this->errorResponse("The authorization token used has expired", 403)); + } + + public function test_email_confirmation_checked_using_api_auth() { $editor = $this->getEditor(); $editor->email_confirmed = false; @@ -74,7 +90,7 @@ class ApiAuthTest extends TestCase // 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 = $this->get($this->endpoint, $this->apiAuthHeader()); $resp->assertStatus(401); $resp->assertJson($this->errorResponse("The email address for the account in use needs to be confirmed", 401)); } diff --git a/tests/TestsApi.php b/tests/TestsApi.php index 2bc751f54..4afcbdf22 100644 --- a/tests/TestsApi.php +++ b/tests/TestsApi.php @@ -13,4 +13,11 @@ trait TestsApi return ["error" => ["code" => $code, "message" => $messge]]; } + protected function apiAuthHeader() + { + return [ + "Authorization" => "Token {$this->apiTokenId}:{$this->apiTokenSecret}" + ]; + } + } \ No newline at end of file From 55abf7be241e0a81f1460da70fcebb7d8530d950 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Dec 2019 20:48:23 +0000 Subject: [PATCH 12/23] Added tests to cover API config and listing code --- tests/Api/ApiConfigTest.php | 47 +++++++++++++++++++++++++++ tests/Api/ApiListingTest.php | 61 ++++++++++++++++++++++++++++++++++++ tests/TestsApi.php | 21 +++++++++++-- 3 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 tests/Api/ApiConfigTest.php create mode 100644 tests/Api/ApiListingTest.php diff --git a/tests/Api/ApiConfigTest.php b/tests/Api/ApiConfigTest.php new file mode 100644 index 000000000..99b063c69 --- /dev/null +++ b/tests/Api/ApiConfigTest.php @@ -0,0 +1,47 @@ +actingAsApiEditor(); + + config()->set(['api.default_item_count' => 5]); + $resp = $this->get($this->endpoint); + $resp->assertJsonCount(5, 'data'); + + config()->set(['api.default_item_count' => 1]); + $resp = $this->get($this->endpoint); + $resp->assertJsonCount(1, 'data'); + } + + public function test_default_item_count_does_not_limit_count_param() + { + $this->actingAsApiEditor(); + config()->set(['api.default_item_count' => 1]); + $resp = $this->get($this->endpoint . '?count=5'); + $resp->assertJsonCount(5, 'data'); + } + + public function test_max_item_count_limits_listing_requests() + { + $this->actingAsApiEditor(); + + config()->set(['api.max_item_count' => 2]); + $resp = $this->get($this->endpoint); + $resp->assertJsonCount(2, 'data'); + + $resp = $this->get($this->endpoint . '?count=5'); + $resp->assertJsonCount(2, 'data'); + } + +} \ No newline at end of file diff --git a/tests/Api/ApiListingTest.php b/tests/Api/ApiListingTest.php new file mode 100644 index 000000000..70d1140d7 --- /dev/null +++ b/tests/Api/ApiListingTest.php @@ -0,0 +1,61 @@ +actingAsApiEditor(); + $bookCount = min(Book::visible()->count(), 100); + + $resp = $this->get($this->endpoint); + $resp->assertJsonCount($bookCount, 'data'); + + $resp = $this->get($this->endpoint . '?count=1'); + $resp->assertJsonCount(1, 'data'); + } + + public function test_offset_parameter() + { + $this->actingAsApiEditor(); + $books = Book::visible()->orderBy('id')->take(3)->get(); + + $resp = $this->get($this->endpoint . '?count=1'); + $resp->assertJsonMissing(['name' => $books[1]->name ]); + + $resp = $this->get($this->endpoint . '?count=1&offset=1000'); + $resp->assertJsonCount(0, 'data'); + } + + public function test_sort_parameter() + { + $this->actingAsApiEditor(); + + $sortChecks = [ + '-id' => Book::visible()->orderBy('id', 'desc')->first(), + '+name' => Book::visible()->orderBy('name', 'asc')->first(), + 'name' => Book::visible()->orderBy('name', 'asc')->first(), + '-name' => Book::visible()->orderBy('name', 'desc')->first() + ]; + + foreach ($sortChecks as $sortOption => $result) { + $resp = $this->get($this->endpoint . '?count=1&sort=' . $sortOption); + $resp->assertJson(['data' => [ + [ + 'id' => $result->id, + 'name' => $result->name, + ] + ]]); + } + } + +} \ No newline at end of file diff --git a/tests/TestsApi.php b/tests/TestsApi.php index 4afcbdf22..0bb10a4cc 100644 --- a/tests/TestsApi.php +++ b/tests/TestsApi.php @@ -8,12 +8,27 @@ trait TestsApi protected $apiTokenId = 'apitoken'; protected $apiTokenSecret = 'password'; - protected function errorResponse(string $messge, int $code) + /** + * Set the API editor role as the current user via the API driver. + */ + protected function actingAsApiEditor() { - return ["error" => ["code" => $code, "message" => $messge]]; + $this->actingAs($this->getEditor(), 'api'); + return $this; } - protected function apiAuthHeader() + /** + * Format the given items into a standardised error format. + */ + protected function errorResponse(string $message, int $code): array + { + return ["error" => ["code" => $code, "message" => $message]]; + } + + /** + * Get an approved API auth header. + */ + protected function apiAuthHeader(): array { return [ "Authorization" => "Token {$this->apiTokenId}:{$this->apiTokenSecret}" From a7a97a53f1d7b9e180d9296cec02c42d0a987a89 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 1 Jan 2020 16:33:47 +0000 Subject: [PATCH 13/23] Added API listing filtering & cleaned ApiAuthenticate returns API listing endpoint filter can be found via &filter[name]=my+book query parameters. There are a range of operators that can be used such as &filter[id:gte]=4 --- app/Api/ListingResponseBuilder.php | 65 +++++++++++++++++-- app/Exceptions/ApiAuthException.php | 12 +--- app/Exceptions/UnauthorizedException.php | 17 +++++ app/Http/Controllers/Api/ApiController.php | 2 +- .../Controllers/Api/BooksApiController.php | 29 +++++++++ app/Http/Middleware/ApiAuthenticate.php | 40 +++++++----- app/Http/Middleware/Authenticate.php | 18 +++++ .../Middleware/ChecksForEmailConfirmation.php | 30 ++++----- tests/Api/ApiListingTest.php | 26 ++++++++ 9 files changed, 186 insertions(+), 53 deletions(-) create mode 100644 app/Exceptions/UnauthorizedException.php diff --git a/app/Api/ListingResponseBuilder.php b/app/Api/ListingResponseBuilder.php index 279fabd5d..2fa5644c3 100644 --- a/app/Api/ListingResponseBuilder.php +++ b/app/Api/ListingResponseBuilder.php @@ -2,19 +2,32 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; +use Illuminate\Http\Request; class ListingResponseBuilder { protected $query; + protected $request; protected $fields; + protected $filterOperators = [ + 'eq' => '=', + 'ne' => '!=', + 'gt' => '>', + 'lt' => '<', + 'gte' => '>=', + 'lte' => '<=', + 'like' => 'like' + ]; + /** * ListingResponseBuilder constructor. */ - public function __construct(Builder $query, array $fields) + public function __construct(Builder $query, Request $request, array $fields) { $this->query = $query; + $this->request = $request; $this->fields = $fields; } @@ -23,8 +36,8 @@ class ListingResponseBuilder */ public function toResponse() { - $total = $this->query->count(); $data = $this->fetchData(); + $total = $this->query->count(); return response()->json([ 'data' => $data, @@ -39,11 +52,51 @@ class ListingResponseBuilder { $this->applyCountAndOffset($this->query); $this->applySorting($this->query); - // TODO - Apply filtering + $this->applyFiltering($this->query); return $this->query->get($this->fields); } + /** + * Apply any filtering operations found in the request. + */ + protected function applyFiltering(Builder $query) + { + $requestFilters = $this->request->get('filter', []); + if (!is_array($requestFilters)) { + return; + } + + $queryFilters = collect($requestFilters)->map(function ($value, $key) { + return $this->requestFilterToQueryFilter($key, $value); + })->filter(function ($value) { + return !is_null($value); + })->values()->toArray(); + + $query->where($queryFilters); + } + + /** + * Convert a request filter query key/value pair into a [field, op, value] where condition. + */ + protected function requestFilterToQueryFilter($fieldKey, $value): ?array + { + $splitKey = explode(':', $fieldKey); + $field = $splitKey[0]; + $filterOperator = $splitKey[1] ?? 'eq'; + + if (!in_array($field, $this->fields)) { + return null; + } + + if (!in_array($filterOperator, array_keys($this->filterOperators))) { + $filterOperator = 'eq'; + } + + $queryOperator = $this->filterOperators[$filterOperator]; + return [$field, $queryOperator, $value]; + } + /** * Apply sorting operations to the query from given parameters * otherwise falling back to the first given field, ascending. @@ -53,7 +106,7 @@ class ListingResponseBuilder $defaultSortName = $this->fields[0]; $direction = 'asc'; - $sort = request()->get('sort', ''); + $sort = $this->request->get('sort', ''); if (strpos($sort, '-') === 0) { $direction = 'desc'; } @@ -72,9 +125,9 @@ class ListingResponseBuilder */ protected function applyCountAndOffset(Builder $query) { - $offset = max(0, request()->get('offset', 0)); + $offset = max(0, $this->request->get('offset', 0)); $maxCount = config('api.max_item_count'); - $count = request()->get('count', config('api.default_item_count')); + $count = $this->request->get('count', config('api.default_item_count')); $count = max(min($maxCount, $count), 1); $query->skip($offset)->take($count); diff --git a/app/Exceptions/ApiAuthException.php b/app/Exceptions/ApiAuthException.php index 0851dfa4a..cc68ba8cf 100644 --- a/app/Exceptions/ApiAuthException.php +++ b/app/Exceptions/ApiAuthException.php @@ -2,16 +2,6 @@ namespace BookStack\Exceptions; -use Exception; +class ApiAuthException extends UnauthorizedException { -class ApiAuthException extends Exception -{ - - /** - * ApiAuthException constructor. - */ - public function __construct($message, $code = 401) - { - parent::__construct($message, $code); - } } \ No newline at end of file diff --git a/app/Exceptions/UnauthorizedException.php b/app/Exceptions/UnauthorizedException.php new file mode 100644 index 000000000..525b431c7 --- /dev/null +++ b/app/Exceptions/UnauthorizedException.php @@ -0,0 +1,17 @@ +toResponse(); } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php index d23aaf025..3943b773a 100644 --- a/app/Http/Controllers/Api/BooksApiController.php +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -4,6 +4,15 @@ use BookStack\Entities\Book; class BooksApiController extends ApiController { + public $validation = [ + 'create' => [ + // TODO + ], + 'update' => [ + // TODO + ], + ]; + /** * Get a listing of books visible to the user. */ @@ -15,4 +24,24 @@ class BooksApiController extends ApiController 'restricted', 'image_id', ]); } + + public function create() + { + // TODO - + } + + public function read() + { + // TODO - + } + + public function update() + { + // TODO - + } + + public function delete() + { + // TODO - + } } \ No newline at end of file diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index fffbd9ef6..655334450 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -2,7 +2,7 @@ namespace BookStack\Http\Middleware; -use BookStack\Exceptions\ApiAuthException; +use BookStack\Exceptions\UnauthorizedException; use Closure; use Illuminate\Http\Request; @@ -14,31 +14,37 @@ class ApiAuthenticate * Handle an incoming request. */ public function handle(Request $request, Closure $next) + { + // Validate the token and it's users API access + try { + $this->ensureAuthorizedBySessionOrToken(); + } catch (UnauthorizedException $exception) { + return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode()); + } + + return $next($request); + } + + /** + * Ensure the current user can access authenticated API routes, either via existing session + * authentication or via API Token authentication. + * @throws UnauthorizedException + */ + protected function ensureAuthorizedBySessionOrToken(): void { // 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()) { - if ($this->awaitingEmailConfirmation()) { - return $this->emailConfirmationErrorResponse($request); - } - return $next($request); + $this->ensureEmailConfirmedIfRequested(); + return; } // Set our api guard to be the default for this request lifecycle. auth()->shouldUse('api'); // Validate the token and it's users API access - try { - auth()->authenticate(); - } catch (ApiAuthException $exception) { - return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode()); - } - - if ($this->awaitingEmailConfirmation()) { - return $this->emailConfirmationErrorResponse($request, true); - } - - return $next($request); + auth()->authenticate(); + $this->ensureEmailConfirmedIfRequested(); } /** @@ -51,6 +57,6 @@ class ApiAuthenticate 'code' => $code, 'message' => $message, ] - ], 401); + ], $code); } } diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index a171a8a2d..9a8affa88 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -28,4 +28,22 @@ class Authenticate return $next($request); } + + /** + * 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) + { + if ($request->wantsJson()) { + return response()->json([ + 'error' => [ + 'code' => 401, + 'message' => trans('errors.email_confirmation_awaiting') + ] + ], 401); + } + + return redirect('/register/confirm/awaiting'); + } } diff --git a/app/Http/Middleware/ChecksForEmailConfirmation.php b/app/Http/Middleware/ChecksForEmailConfirmation.php index df75c2f33..4b1732810 100644 --- a/app/Http/Middleware/ChecksForEmailConfirmation.php +++ b/app/Http/Middleware/ChecksForEmailConfirmation.php @@ -2,10 +2,22 @@ namespace BookStack\Http\Middleware; +use BookStack\Exceptions\UnauthorizedException; use Illuminate\Http\Request; trait ChecksForEmailConfirmation { + /** + * Check if the current user has a confirmed email if the instance deems it as required. + * Throws if confirmation is required by the user. + * @throws UnauthorizedException + */ + protected function ensureEmailConfirmedIfRequested() + { + if ($this->awaitingEmailConfirmation()) { + throw new UnauthorizedException(trans('errors.email_confirmation_awaiting')); + } + } /** * Check if email confirmation is required and the current user is awaiting confirmation. @@ -21,22 +33,4 @@ trait ChecksForEmailConfirmation return false; } - - /** - * 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, bool $forceJson = false) - { - if ($request->wantsJson() || $forceJson) { - return response()->json([ - 'error' => [ - 'code' => 401, - 'message' => trans('errors.email_confirmation_awaiting') - ] - ], 401); - } - - return redirect('/register/confirm/awaiting'); - } } \ No newline at end of file diff --git a/tests/Api/ApiListingTest.php b/tests/Api/ApiListingTest.php index 70d1140d7..26014cdec 100644 --- a/tests/Api/ApiListingTest.php +++ b/tests/Api/ApiListingTest.php @@ -58,4 +58,30 @@ class ApiAuthTest extends TestCase } } + public function test_filter_parameter() + { + $this->actingAsApiEditor(); + $book = Book::visible()->first(); + $nameSubstr = substr($book->name, 0, 4); + $encodedNameSubstr = rawurlencode($nameSubstr); + + $filterChecks = [ + // Test different types of filter + "filter[id]={$book->id}" => 1, + "filter[id:ne]={$book->id}" => Book::visible()->where('id', '!=', $book->id)->count(), + "filter[id:gt]={$book->id}" => Book::visible()->where('id', '>', $book->id)->count(), + "filter[id:gte]={$book->id}" => Book::visible()->where('id', '>=', $book->id)->count(), + "filter[id:lt]={$book->id}" => Book::visible()->where('id', '<', $book->id)->count(), + "filter[name:like]={$encodedNameSubstr}%" => Book::visible()->where('name', 'like', $nameSubstr . '%')->count(), + + // Test mulitple filters 'and' together + "filter[id]={$book->id}&filter[name]=random_non_existing_string" => 0, + ]; + + foreach ($filterChecks as $filterOption => $resultCount) { + $resp = $this->get($this->endpoint . '?count=1&' . $filterOption); + $resp->assertJson(['total' => $resultCount]); + } + } + } \ No newline at end of file From a8595d8aaf96b2070b1173ee2ddb0c3738ba52b6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 1 Jan 2020 17:01:36 +0000 Subject: [PATCH 14/23] Fixed test class names + add perm. check to api session auth --- app/Http/Middleware/ApiAuthenticate.php | 4 ++++ tests/Api/ApiAuthTest.php | 25 +++++++++++++++++++++++++ tests/Api/ApiConfigTest.php | 2 +- tests/Api/ApiListingTest.php | 2 +- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php index 655334450..15962b3b0 100644 --- a/app/Http/Middleware/ApiAuthenticate.php +++ b/app/Http/Middleware/ApiAuthenticate.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Middleware; +use BookStack\Exceptions\ApiAuthException; use BookStack\Exceptions\UnauthorizedException; use Closure; use Illuminate\Http\Request; @@ -36,6 +37,9 @@ class ApiAuthenticate // This is to make it easy to browser the API via browser after just logging into the system. if (signedInUser()) { $this->ensureEmailConfirmedIfRequested(); + if (!auth()->user()->can('access-api')) { + throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403); + } return; } diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php index 30d7f4ead..b6b6b72ac 100644 --- a/tests/Api/ApiAuthTest.php +++ b/tests/Api/ApiAuthTest.php @@ -3,6 +3,7 @@ namespace Tests; use BookStack\Auth\Permissions\RolePermission; +use BookStack\Auth\User; use Carbon\Carbon; class ApiAuthTest extends TestCase @@ -14,6 +15,8 @@ class ApiAuthTest extends TestCase public function test_requests_succeed_with_default_auth() { $viewer = $this->getViewer(); + $this->giveUserPermissions($viewer, ['access-api']); + $resp = $this->get($this->endpoint); $resp->assertStatus(401); @@ -62,6 +65,28 @@ class ApiAuthTest extends TestCase $editorRole->detachPermission($accessApiPermission); $resp = $this->get($this->endpoint, $this->apiAuthHeader()); + $resp->assertStatus(403); + $resp->assertJson($this->errorResponse("The owner of the used API token does not have permission to make API calls", 403)); + } + + public function test_api_access_permission_required_to_access_api_with_session_auth() + { + $editor = $this->getEditor(); + $this->actingAs($editor, 'web'); + + $resp = $this->get($this->endpoint); + $resp->assertStatus(200); + auth('web')->logout(); + + $accessApiPermission = RolePermission::getByName('access-api'); + $editorRole = $this->getEditor()->roles()->first(); + $editorRole->detachPermission($accessApiPermission); + + $editor = User::query()->where('id', '=', $editor->id)->first(); + + $this->actingAs($editor, 'web'); + $resp = $this->get($this->endpoint); + $resp->assertStatus(403); $resp->assertJson($this->errorResponse("The owner of the used API token does not have permission to make API calls", 403)); } diff --git a/tests/Api/ApiConfigTest.php b/tests/Api/ApiConfigTest.php index 99b063c69..d9367741f 100644 --- a/tests/Api/ApiConfigTest.php +++ b/tests/Api/ApiConfigTest.php @@ -5,7 +5,7 @@ namespace Tests; use BookStack\Auth\Permissions\RolePermission; use Carbon\Carbon; -class ApiAuthTest extends TestCase +class ApiConfigTest extends TestCase { use TestsApi; diff --git a/tests/Api/ApiListingTest.php b/tests/Api/ApiListingTest.php index 26014cdec..fa28dfb36 100644 --- a/tests/Api/ApiListingTest.php +++ b/tests/Api/ApiListingTest.php @@ -6,7 +6,7 @@ use BookStack\Auth\Permissions\RolePermission; use BookStack\Entities\Book; use Carbon\Carbon; -class ApiAuthTest extends TestCase +class ApiListingTest extends TestCase { use TestsApi; From 04a86141361e941d2053ca16181e7561bff27650 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 12 Jan 2020 14:45:54 +0000 Subject: [PATCH 15/23] Filled out base Book API endpoints, added example responses --- app/Auth/User.php | 2 +- app/Entities/Book.php | 3 +- app/Http/Controllers/Api/ApiController.php | 10 +++ .../Controllers/Api/BooksApiController.php | 78 ++++++++++++++--- app/Uploads/Image.php | 1 + dev/api/responses/books-create.json | 10 +++ dev/api/responses/books-index.json | 27 ++++++ dev/api/responses/books-read.json | 47 ++++++++++ dev/api/responses/books-update.json | 11 +++ routes/api.php | 11 +-- tests/Api/BooksApiTest.php | 87 +++++++++++++++++++ tests/TestCase.php | 17 ++++ 12 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 dev/api/responses/books-create.json create mode 100644 dev/api/responses/books-index.json create mode 100644 dev/api/responses/books-read.json create mode 100644 dev/api/responses/books-update.json create mode 100644 tests/Api/BooksApiTest.php diff --git a/app/Auth/User.php b/app/Auth/User.php index 69f424cac..35b3cd54f 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -47,7 +47,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon * The attributes excluded from the model's JSON form. * @var array */ - protected $hidden = ['password', 'remember_token']; + protected $hidden = ['password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email']; /** * This holds the user's permissions when loaded. diff --git a/app/Entities/Book.php b/app/Entities/Book.php index 4e54457b8..919f60035 100644 --- a/app/Entities/Book.php +++ b/app/Entities/Book.php @@ -18,7 +18,8 @@ class Book extends Entity implements HasCoverImage { public $searchFactor = 2; - protected $fillable = ['name', 'description', 'image_id']; + protected $fillable = ['name', 'description']; + protected $hidden = ['restricted']; /** * Get the url for this book. diff --git a/app/Http/Controllers/Api/ApiController.php b/app/Http/Controllers/Api/ApiController.php index b3f1fb747..65a5bb99f 100644 --- a/app/Http/Controllers/Api/ApiController.php +++ b/app/Http/Controllers/Api/ApiController.php @@ -8,6 +8,8 @@ use Illuminate\Http\JsonResponse; class ApiController extends Controller { + protected $rules = []; + /** * Provide a paginated listing JSON response in a standard format * taking into account any pagination parameters passed by the user. @@ -17,4 +19,12 @@ class ApiController extends Controller $listing = new ListingResponseBuilder($query, request(), $fields); return $listing->toResponse(); } + + /** + * Get the validation rules for this controller. + */ + public function getValdationRules(): array + { + return $this->rules; + } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php index 3943b773a..e7a0217dc 100644 --- a/app/Http/Controllers/Api/BooksApiController.php +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -1,47 +1,99 @@ [ - // TODO + 'name' => 'required|string|max:255', + 'description' => 'string|max:1000', ], 'update' => [ - // TODO + 'name' => 'string|min:1|max:255', + 'description' => 'string|max:1000', ], ]; + /** + * BooksApiController constructor. + */ + public function __construct(BookRepo $bookRepo) + { + $this->bookRepo = $bookRepo; + } + /** * Get a listing of books visible to the user. + * @api listing */ public function index() { $books = Book::visible(); return $this->apiListingResponse($books, [ - 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', - 'restricted', 'image_id', + 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'image_id', ]); } - public function create() + /** + * Create a new book. + * @throws \Illuminate\Validation\ValidationException + */ + public function create(Request $request) { - // TODO - + $this->checkPermission('book-create-all'); + $requestData = $this->validate($request, $this->rules['create']); + + $book = $this->bookRepo->create($requestData); + Activity::add($book, 'book_create', $book->id); + + return response()->json($book); } - public function read() + /** + * View the details of a single book. + */ + public function read(string $id) { - // TODO - + $book = Book::visible()->with(['tags', 'cover', 'createdBy', 'updatedBy'])->findOrFail($id); + return response()->json($book); } - public function update() + /** + * Update the details of a single book. + * @throws \Illuminate\Validation\ValidationException + */ + public function update(Request $request, string $id) { - // TODO - + $book = Book::visible()->findOrFail($id); + $this->checkOwnablePermission('book-update', $book); + + $requestData = $this->validate($request, $this->rules['update']); + $book = $this->bookRepo->update($book, $requestData); + Activity::add($book, 'book_update', $book->id); + + return response()->json($book); } - public function delete() + /** + * Delete a book from the system. + * @throws \BookStack\Exceptions\NotifyException + * @throws \Illuminate\Contracts\Container\BindingResolutionException + */ + public function delete(string $id) { - // TODO - + $book = Book::visible()->findOrFail($id); + $this->checkOwnablePermission('book-delete', $book); + + $this->bookRepo->destroy($book); + Activity::addMessage('book_delete', $book->name); + + return response('', 204); } } \ No newline at end of file diff --git a/app/Uploads/Image.php b/app/Uploads/Image.php index 6fa5db2a5..c76979d7c 100644 --- a/app/Uploads/Image.php +++ b/app/Uploads/Image.php @@ -8,6 +8,7 @@ class Image extends Ownable { protected $fillable = ['name']; + protected $hidden = []; /** * Get a thumbnail for this image. diff --git a/dev/api/responses/books-create.json b/dev/api/responses/books-create.json new file mode 100644 index 000000000..0b4336ab2 --- /dev/null +++ b/dev/api/responses/books-create.json @@ -0,0 +1,10 @@ +{ + "name": "My new book", + "description": "This is a book created via the API", + "created_by": 1, + "updated_by": 1, + "slug": "my-new-book", + "updated_at": "2020-01-12 14:05:11", + "created_at": "2020-01-12 14:05:11", + "id": 15 +} \ No newline at end of file diff --git a/dev/api/responses/books-index.json b/dev/api/responses/books-index.json new file mode 100644 index 000000000..29e83b1c0 --- /dev/null +++ b/dev/api/responses/books-index.json @@ -0,0 +1,27 @@ +{ + "data": [ + { + "id": 1, + "name": "BookStack User Guide", + "slug": "bookstack-user-guide", + "description": "This is a general guide on using BookStack on a day-to-day basis.", + "created_at": "2019-05-05 21:48:46", + "updated_at": "2019-12-11 20:57:31", + "created_by": 1, + "updated_by": 1, + "image_id": 3 + }, + { + "id": 2, + "name": "Inventore inventore quia voluptatem.", + "slug": "inventore-inventore-quia-voluptatem", + "description": "Veniam nihil voluptas enim laborum corporis quos sint. Ab rerum voluptas ut iste voluptas magni quibusdam ut. Amet omnis enim voluptate neque facilis.", + "created_at": "2019-05-05 22:10:14", + "updated_at": "2019-12-11 20:57:23", + "created_by": 4, + "updated_by": 3, + "image_id": 34 + } + ], + "total": 14 +} \ No newline at end of file diff --git a/dev/api/responses/books-read.json b/dev/api/responses/books-read.json new file mode 100644 index 000000000..e0570444f --- /dev/null +++ b/dev/api/responses/books-read.json @@ -0,0 +1,47 @@ +{ + "id": 16, + "name": "My own book", + "slug": "my-own-book", + "description": "This is my own little book", + "created_at": "2020-01-12 14:09:59", + "updated_at": "2020-01-12 14:11:51", + "created_by": { + "id": 1, + "name": "Admin", + "created_at": "2019-05-05 21:15:13", + "updated_at": "2019-12-16 12:18:37", + "image_id": 48 + }, + "updated_by": { + "id": 1, + "name": "Admin", + "created_at": "2019-05-05 21:15:13", + "updated_at": "2019-12-16 12:18:37", + "image_id": 48 + }, + "image_id": 452, + "tags": [ + { + "id": 13, + "entity_id": 16, + "entity_type": "BookStack\\Book", + "name": "Category", + "value": "Guide", + "order": 0, + "created_at": "2020-01-12 14:11:51", + "updated_at": "2020-01-12 14:11:51" + } + ], + "cover": { + "id": 452, + "name": "sjovall_m117hUWMu40.jpg", + "url": "http:\/\/bookstack.local\/uploads\/images\/cover_book\/2020-01\/sjovall_m117hUWMu40.jpg", + "created_at": "2020-01-12 14:11:51", + "updated_at": "2020-01-12 14:11:51", + "created_by": 1, + "updated_by": 1, + "path": "\/uploads\/images\/cover_book\/2020-01\/sjovall_m117hUWMu40.jpg", + "type": "cover_book", + "uploaded_to": 16 + } +} \ No newline at end of file diff --git a/dev/api/responses/books-update.json b/dev/api/responses/books-update.json new file mode 100644 index 000000000..8f20b5b9f --- /dev/null +++ b/dev/api/responses/books-update.json @@ -0,0 +1,11 @@ +{ + "id": 16, + "name": "My own book", + "slug": "my-own-book", + "description": "This is my own little book - updated", + "created_at": "2020-01-12 14:09:59", + "updated_at": "2020-01-12 14:16:10", + "created_by": 1, + "updated_by": 1, + "image_id": 452 +} \ No newline at end of file diff --git a/routes/api.php b/routes/api.php index 0604ffd29..3348d8907 100644 --- a/routes/api.php +++ b/routes/api.php @@ -2,11 +2,12 @@ /** * Routes for the BookStack API. - * * Routes have a uri prefix of /api/. + * Controllers are all within app/Http/Controllers/Api */ - -// TODO - Authenticate middleware - -Route::get('books', 'BooksApiController@index'); \ No newline at end of file +Route::get('books', 'BooksApiController@index'); +Route::post('books', 'BooksApiController@create'); +Route::get('books/{id}', 'BooksApiController@read'); +Route::put('books/{id}', 'BooksApiController@update'); +Route::delete('books/{id}', 'BooksApiController@delete'); diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php new file mode 100644 index 000000000..f560bfffd --- /dev/null +++ b/tests/Api/BooksApiTest.php @@ -0,0 +1,87 @@ +actingAsApiEditor(); + $firstBook = Book::query()->orderBy('id', 'asc')->first(); + + $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id'); + $resp->assertJson(['data' => [ + [ + 'id' => $firstBook->id, + 'name' => $firstBook->name, + 'slug' => $firstBook->slug, + ] + ]]); + } + + public function test_create_endpoint() + { + $this->actingAsApiEditor(); + $details = [ + 'name' => 'My API book', + 'description' => 'A book created via the API', + ]; + + $resp = $this->postJson($this->baseEndpoint, $details); + $resp->assertStatus(200); + $newItem = Book::query()->orderByDesc('id')->where('name', '=', $details['name'])->first(); + $resp->assertJson(array_merge($details, ['id' => $newItem->id, 'slug' => $newItem->slug])); + $this->assertActivityExists('book_create', $newItem); + } + + public function test_read_endpoint() + { + $this->actingAsApiEditor(); + $book = Book::visible()->first(); + + $resp = $this->getJson($this->baseEndpoint . "/{$book->id}"); + + $resp->assertStatus(200); + $resp->assertJson([ + 'id' => $book->id, + 'slug' => $book->slug, + 'created_by' => [ + 'name' => $book->createdBy->name, + ], + 'updated_by' => [ + 'name' => $book->createdBy->name, + ] + ]); + } + + public function test_update_endpoint() + { + $this->actingAsApiEditor(); + $book = Book::visible()->first(); + $details = [ + 'name' => 'My updated API book', + 'description' => 'A book created via the API', + ]; + + $resp = $this->putJson($this->baseEndpoint . "/{$book->id}", $details); + $book->refresh(); + + $resp->assertStatus(200); + $resp->assertJson(array_merge($details, ['id' => $book->id, 'slug' => $book->slug])); + $this->assertActivityExists('book_update', $book); + } + + public function test_delete_endpoint() + { + $this->actingAsApiEditor(); + $book = Book::visible()->first(); + $resp = $this->deleteJson($this->baseEndpoint . "/{$book->id}"); + + $resp->assertStatus(204); + $this->assertActivityExists('book_delete'); + } +} \ No newline at end of file diff --git a/tests/TestCase.php b/tests/TestCase.php index 939a1a91e..f20b20fd8 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -1,5 +1,6 @@ $key]; + + if ($entity) { + $detailsToCheck['entity_type'] = $entity->getMorphClass(); + $detailsToCheck['entity_id'] = $entity->id; + } + + $this->assertDatabaseHas('activities', $detailsToCheck); + } } \ No newline at end of file From bed24986675de1a42b1e3621d2c5135c6c9fbf14 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 12 Jan 2020 16:25:14 +0000 Subject: [PATCH 16/23] Started work on generating API docs --- app/Api/ApiDocsGenerator.php | 122 ++++++++++++++++++ .../Controllers/Api/ApiDocsController.php | 47 +++++++ .../Controllers/Api/BooksApiController.php | 1 - routes/api.php | 3 + 4 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 app/Api/ApiDocsGenerator.php create mode 100644 app/Http/Controllers/Api/ApiDocsController.php diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php new file mode 100644 index 000000000..b63406696 --- /dev/null +++ b/app/Api/ApiDocsGenerator.php @@ -0,0 +1,122 @@ +getFlatApiRoutes(); + $apiRoutes = $this->loadDetailsFromControllers($apiRoutes); + $apiRoutes = $this->loadDetailsFromFiles($apiRoutes); + $apiRoutes = $apiRoutes->groupBy('base_model'); + return $apiRoutes; + } + + /** + * Load any API details stored in static files. + */ + protected function loadDetailsFromFiles(Collection $routes): Collection + { + return $routes->map(function (array $route) { + $exampleResponseFile = base_path('dev/api/responses/' . $route['name'] . '.json'); + $exampleResponse = file_exists($exampleResponseFile) ? file_get_contents($exampleResponseFile) : null; + $route['example_response'] = $exampleResponse; + return $route; + }); + } + + /** + * Load any details we can fetch from the controller and its methods. + */ + protected function loadDetailsFromControllers(Collection $routes): Collection + { + return $routes->map(function (array $route) { + $method = $this->getReflectionMethod($route['controller'], $route['controller_method']); + $comment = $method->getDocComment(); + $route['description'] = $comment ? $this->parseDescriptionFromMethodComment($comment) : null; + $route['body_params'] = $this->getBodyParamsFromClass($route['controller'], $route['controller_method']); + return $route; + }); + } + + /** + * Load body params and their rules by inspecting the given class and method name. + * @throws \Illuminate\Contracts\Container\BindingResolutionException + */ + protected function getBodyParamsFromClass(string $className, string $methodName): ?array + { + /** @var ApiController $class */ + $class = $this->controllerClasses[$className] ?? null; + if ($class === null) { + $class = app()->make($className); + $this->controllerClasses[$className] = $class; + } + + $rules = $class->getValdationRules()[$methodName] ?? []; + foreach ($rules as $param => $ruleString) { + $rules[$param] = explode('|', $ruleString); + } + return count($rules) > 0 ? $rules : null; + } + + /** + * Parse out the description text from a class method comment. + */ + protected function parseDescriptionFromMethodComment(string $comment) + { + $matches = []; + preg_match_all('/^\s*?\*\s((?![@\s]).*?)$/m', $comment, $matches); + return implode(' ', $matches[1] ?? []); + } + + /** + * Get a reflection method from the given class name and method name. + * @throws ReflectionException + */ + protected function getReflectionMethod(string $className, string $methodName): ReflectionMethod + { + $class = $this->reflectionClasses[$className] ?? null; + if ($class === null) { + $class = new ReflectionClass($className); + $this->reflectionClasses[$className] = $class; + } + + return $class->getMethod($methodName); + } + + /** + * Get the system API routes, formatted into a flat collection. + */ + protected function getFlatApiRoutes(): Collection + { + return collect(Route::getRoutes()->getRoutes())->filter(function ($route) { + return strpos($route->uri, 'api/') === 0; + })->map(function ($route) { + [$controller, $controllerMethod] = explode('@', $route->action['uses']); + $baseModelName = explode('/', $route->uri)[1]; + $shortName = $baseModelName . '-' . $controllerMethod; + return [ + 'name' => $shortName, + 'uri' => $route->uri, + 'method' => $route->methods[0], + 'controller' => $controller, + 'controller_method' => $controllerMethod, + 'base_model' => $baseModelName, + ]; + }); + } + +} \ No newline at end of file diff --git a/app/Http/Controllers/Api/ApiDocsController.php b/app/Http/Controllers/Api/ApiDocsController.php new file mode 100644 index 000000000..bfb0c1834 --- /dev/null +++ b/app/Http/Controllers/Api/ApiDocsController.php @@ -0,0 +1,47 @@ +getDocs(); + dd($docs); + // TODO - Build view for API docs + return view(''); + } + + /** + * Show a JSON view of the API docs data. + */ + public function json() { + $docs = $this->getDocs(); + return response()->json($docs); + } + + /** + * Get the base docs data. + * Checks and uses the system cache for quick re-fetching. + */ + protected function getDocs(): Collection + { + $appVersion = trim(file_get_contents(base_path('version'))); + $cacheKey = 'api-docs::' . $appVersion; + if (Cache::has($cacheKey) && config('app.env') === 'production') { + $docs = Cache::get($cacheKey); + } else { + $docs = (new ApiDocsGenerator())->generate(); + Cache::put($cacheKey, $docs, 60*24); + } + + return $docs; + } + +} diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php index e7a0217dc..8c62b7d7d 100644 --- a/app/Http/Controllers/Api/BooksApiController.php +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -31,7 +31,6 @@ class BooksApiController extends ApiController /** * Get a listing of books visible to the user. - * @api listing */ public function index() { diff --git a/routes/api.php b/routes/api.php index 3348d8907..12b327798 100644 --- a/routes/api.php +++ b/routes/api.php @@ -6,6 +6,9 @@ * Controllers are all within app/Http/Controllers/Api */ +Route::get('docs', 'ApiDocsController@display'); +Route::get('docs.json', 'ApiDocsController@json'); + Route::get('books', 'BooksApiController@index'); Route::post('books', 'BooksApiController@create'); Route::get('books/{id}', 'BooksApiController@read'); From 45b5e631e241e7809d8752279d781ec391245423 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 15 Jan 2020 20:18:02 +0000 Subject: [PATCH 17/23] Added a view for the API docs --- app/Api/ApiDocsGenerator.php | 2 +- .../Controllers/Api/ApiDocsController.php | 6 +-- .../Controllers/Api/BooksApiController.php | 15 +++--- .../js/components/details-highlighter.js | 18 +++++++ resources/js/components/index.js | 2 + resources/js/services/code.js | 18 +++++-- resources/sass/_blocks.scss | 22 ++++++++ resources/sass/_text.scss | 12 +++++ resources/views/api-docs/index.blade.php | 51 +++++++++++++++++++ 9 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 resources/js/components/details-highlighter.js create mode 100644 resources/views/api-docs/index.blade.php diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php index b63406696..41decd23d 100644 --- a/app/Api/ApiDocsGenerator.php +++ b/app/Api/ApiDocsGenerator.php @@ -106,7 +106,7 @@ class ApiDocsGenerator return strpos($route->uri, 'api/') === 0; })->map(function ($route) { [$controller, $controllerMethod] = explode('@', $route->action['uses']); - $baseModelName = explode('/', $route->uri)[1]; + $baseModelName = explode('.', explode('/', $route->uri)[1])[0]; $shortName = $baseModelName . '-' . $controllerMethod; return [ 'name' => $shortName, diff --git a/app/Http/Controllers/Api/ApiDocsController.php b/app/Http/Controllers/Api/ApiDocsController.php index bfb0c1834..84ddd5215 100644 --- a/app/Http/Controllers/Api/ApiDocsController.php +++ b/app/Http/Controllers/Api/ApiDocsController.php @@ -13,9 +13,9 @@ class ApiDocsController extends ApiController public function display() { $docs = $this->getDocs(); - dd($docs); - // TODO - Build view for API docs - return view(''); + return view('api-docs.index', [ + 'docs' => $docs, + ]); } /** diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php index 8c62b7d7d..fa174dfd3 100644 --- a/app/Http/Controllers/Api/BooksApiController.php +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -2,8 +2,11 @@ use BookStack\Entities\Book; use BookStack\Entities\Repos\BookRepo; +use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; +use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; class BooksApiController extends ApiController { @@ -41,8 +44,8 @@ class BooksApiController extends ApiController } /** - * Create a new book. - * @throws \Illuminate\Validation\ValidationException + * Create a new book in the system. + * @throws ValidationException */ public function create(Request $request) { @@ -66,7 +69,7 @@ class BooksApiController extends ApiController /** * Update the details of a single book. - * @throws \Illuminate\Validation\ValidationException + * @throws ValidationException */ public function update(Request $request, string $id) { @@ -81,9 +84,9 @@ class BooksApiController extends ApiController } /** - * Delete a book from the system. - * @throws \BookStack\Exceptions\NotifyException - * @throws \Illuminate\Contracts\Container\BindingResolutionException + * Delete a single book from the system. + * @throws NotifyException + * @throws BindingResolutionException */ public function delete(string $id) { diff --git a/resources/js/components/details-highlighter.js b/resources/js/components/details-highlighter.js new file mode 100644 index 000000000..18c5165fa --- /dev/null +++ b/resources/js/components/details-highlighter.js @@ -0,0 +1,18 @@ +import Code from "../services/code" +class DetailsHighlighter { + + constructor(elem) { + this.elem = elem; + this.dealtWith = false; + elem.addEventListener('toggle', this.onToggle.bind(this)); + } + + onToggle() { + if (this.dealtWith) return; + + Code.highlightWithin(this.elem); + this.dealtWith = true; + } +} + +export default DetailsHighlighter; \ No newline at end of file diff --git a/resources/js/components/index.js b/resources/js/components/index.js index bbe059898..d3ba539dd 100644 --- a/resources/js/components/index.js +++ b/resources/js/components/index.js @@ -30,6 +30,7 @@ import settingColorPicker from "./setting-color-picker"; import entityPermissionsEditor from "./entity-permissions-editor"; import templateManager from "./template-manager"; import newUserPassword from "./new-user-password"; +import detailsHighlighter from "./details-highlighter"; const componentMapping = { 'dropdown': dropdown, @@ -64,6 +65,7 @@ const componentMapping = { 'entity-permissions-editor': entityPermissionsEditor, 'template-manager': templateManager, 'new-user-password': newUserPassword, + 'details-highlighter': detailsHighlighter, }; window.components = {}; diff --git a/resources/js/services/code.js b/resources/js/services/code.js index 26dee5bfb..834a547e3 100644 --- a/resources/js/services/code.js +++ b/resources/js/services/code.js @@ -87,9 +87,20 @@ const modeMap = { * Highlight pre elements on a page */ function highlight() { - let codeBlocks = document.querySelectorAll('.page-content pre, .comment-box .content pre'); - for (let i = 0; i < codeBlocks.length; i++) { - highlightElem(codeBlocks[i]); + const codeBlocks = document.querySelectorAll('.page-content pre, .comment-box .content pre'); + for (const codeBlock of codeBlocks) { + highlightElem(codeBlock); + } +} + +/** + * Highlight all code blocks within the given parent element + * @param {HTMLElement} parent + */ +function highlightWithin(parent) { + const codeBlocks = parent.querySelectorAll('pre'); + for (const codeBlock of codeBlocks) { + highlightElem(codeBlock); } } @@ -308,6 +319,7 @@ function getMetaKey() { export default { highlight: highlight, + highlightWithin: highlightWithin, wysiwygView: wysiwygView, popupEditor: popupEditor, setMode: setMode, diff --git a/resources/sass/_blocks.scss b/resources/sass/_blocks.scss index 2cb17a18d..cc42dc736 100644 --- a/resources/sass/_blocks.scss +++ b/resources/sass/_blocks.scss @@ -236,4 +236,26 @@ .tag-list div:last-child .tag-item { margin-bottom: 0; +} + +/** + * API Docs + */ +.api-method { + font-size: 0.75rem; + background-color: #888; + padding: $-xs; + line-height: 1.3; + opacity: 0.7; + vertical-align: top; + border-radius: 3px; + color: #FFF; + display: inline-block; + min-width: 60px; + text-align: center; + font-weight: bold; + &[data-method="GET"] { background-color: #077b70 } + &[data-method="POST"] { background-color: #cf4d03 } + &[data-method="PUT"] { background-color: #0288D1 } + &[data-method="DELETE"] { background-color: #ab0f0e } } \ No newline at end of file diff --git a/resources/sass/_text.scss b/resources/sass/_text.scss index cf78c162b..77e0773eb 100644 --- a/resources/sass/_text.scss +++ b/resources/sass/_text.scss @@ -213,6 +213,18 @@ blockquote { } } +.text-mono { + font-family: $mono; +} + +.text-uppercase { + text-transform: uppercase; +} + +.text-capitals { + text-transform: capitalize; +} + .code-base { background-color: #F8F8F8; font-size: 0.80em; diff --git a/resources/views/api-docs/index.blade.php b/resources/views/api-docs/index.blade.php new file mode 100644 index 000000000..181bcd746 --- /dev/null +++ b/resources/views/api-docs/index.blade.php @@ -0,0 +1,51 @@ +@extends('simple-layout') + +@section('body') + +
+ +
+ +
+ @foreach($docs as $model => $endpoints) +

{{ $model }}

+ + @foreach($endpoints as $endpoint) + + @endforeach + @endforeach +
+ +
+ @foreach($docs as $model => $endpoints) +
+

{{ $model }}

+ + @foreach($endpoints as $endpoint) +
+ {{ $endpoint['method'] }} + {{ url($endpoint['uri']) }} +
+

{{ $endpoint['description'] ?? '' }}

+ @if($endpoint['example_response'] ?? false) +
+ Example Response +
{{ $endpoint['example_response'] }}
+
+
+ @endif + @endforeach +
+ @endforeach +
+ +
+ + +
+@stop \ No newline at end of file From 8016f1121ef8feef114a5f20c5dba97c8d550bea Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 09:48:30 +0000 Subject: [PATCH 18/23] Refined docs view, Added example requests --- app/Api/ApiDocsGenerator.php | 9 +++-- dev/api/requests/books-create.json | 4 +++ dev/api/requests/books-update.json | 4 +++ resources/views/api-docs/index.blade.php | 42 +++++++++++++++++++++--- 4 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 dev/api/requests/books-create.json create mode 100644 dev/api/requests/books-update.json diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php index 41decd23d..a0c45608a 100644 --- a/app/Api/ApiDocsGenerator.php +++ b/app/Api/ApiDocsGenerator.php @@ -31,9 +31,12 @@ class ApiDocsGenerator protected function loadDetailsFromFiles(Collection $routes): Collection { return $routes->map(function (array $route) { - $exampleResponseFile = base_path('dev/api/responses/' . $route['name'] . '.json'); - $exampleResponse = file_exists($exampleResponseFile) ? file_get_contents($exampleResponseFile) : null; - $route['example_response'] = $exampleResponse; + $exampleTypes = ['request', 'response']; + foreach ($exampleTypes as $exampleType) { + $exampleFile = base_path("dev/api/{$exampleType}s/{$route['name']}.json"); + $exampleContent = file_exists($exampleFile) ? file_get_contents($exampleFile) : null; + $route["example_{$exampleType}"] = $exampleContent; + } return $route; }); } diff --git a/dev/api/requests/books-create.json b/dev/api/requests/books-create.json new file mode 100644 index 000000000..4a6626619 --- /dev/null +++ b/dev/api/requests/books-create.json @@ -0,0 +1,4 @@ +{ + "name": "My own book", + "description": "This is my own little book" +} \ No newline at end of file diff --git a/dev/api/requests/books-update.json b/dev/api/requests/books-update.json new file mode 100644 index 000000000..fc67d5fcc --- /dev/null +++ b/dev/api/requests/books-update.json @@ -0,0 +1,4 @@ +{ + "name": "My updated book", + "description": "This is my book with updated details" +} \ No newline at end of file diff --git a/resources/views/api-docs/index.blade.php b/resources/views/api-docs/index.blade.php index 181bcd746..a20ba04cc 100644 --- a/resources/views/api-docs/index.blade.php +++ b/resources/views/api-docs/index.blade.php @@ -12,32 +12,64 @@ @foreach($endpoints as $endpoint) @endforeach @endforeach
-
+
@foreach($docs as $model => $endpoints)

{{ $model }}

@foreach($endpoints as $endpoint) +
{{ $endpoint['controller_method'] }}
{{ $endpoint['method'] }} {{ url($endpoint['uri']) }}

{{ $endpoint['description'] ?? '' }}

+ @if($endpoint['body_params'] ?? false) +
+ Body Parameters + + + + + + @foreach($endpoint['body_params'] as $paramName => $rules) + + + + + @endforeach +
Param NameValue Rules
{{ $paramName }} + @foreach($rules as $rule) + {{ $rule }} + @endforeach +
+
+ @endif + @if($endpoint['example_request'] ?? false) +
+ Example Request +
{{ $endpoint['example_request'] }}
+
+ @endif @if($endpoint['example_response'] ?? false) -
+
Example Response
{{ $endpoint['example_response'] }}
-
+ @endif + @if(!$loop->last) +
@endif @endforeach
From 8ead596067af45ed3540771ad9ccd37e7143539a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 09:55:02 +0000 Subject: [PATCH 19/23] Updated default codemirror theme - To mdn-like theme, to have better default legibility and contrast --- resources/js/services/code.js | 2 +- resources/sass/_codemirror.scss | 70 ++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/resources/js/services/code.js b/resources/js/services/code.js index 834a547e3..0c5f75db3 100644 --- a/resources/js/services/code.js +++ b/resources/js/services/code.js @@ -185,7 +185,7 @@ function getMode(suggestion, content) { * @returns {*|string} */ function getTheme() { - return window.codeTheme || 'base16-light'; + return window.codeTheme || 'mdn-like'; } /** diff --git a/resources/sass/_codemirror.scss b/resources/sass/_codemirror.scss index 28c777608..92287c484 100644 --- a/resources/sass/_codemirror.scss +++ b/resources/sass/_codemirror.scss @@ -343,44 +343,51 @@ span.CodeMirror-selectedtext { background: none; } /* + MDN-LIKE Theme - Mozilla + Ported to CodeMirror by Peter Kroon + Report bugs/issues here: https://github.com/codemirror/CodeMirror/issues + GitHub: @peterkroon - Name: Base16 Default Light - Author: Chris Kempson (http://chriskempson.com) - - CodeMirror template by Jan T. Sott (https://github.com/idleberg/base16-codemirror) - Original Base16 color scheme by Chris Kempson (https://github.com/chriskempson/base16) + The mdn-like theme is inspired on the displayed code examples at: https://developer.mozilla.org/en-US/docs/Web/CSS/animation */ +.cm-s-mdn-like.CodeMirror { color: #999; background-color: #fff; } +.cm-s-mdn-like div.CodeMirror-selected { background: #cfc; } +.cm-s-mdn-like .CodeMirror-line::selection, .cm-s-mdn-like .CodeMirror-line > span::selection, .cm-s-mdn-like .CodeMirror-line > span > span::selection { background: #cfc; } +.cm-s-mdn-like .CodeMirror-line::-moz-selection, .cm-s-mdn-like .CodeMirror-line > span::-moz-selection, .cm-s-mdn-like .CodeMirror-line > span > span::-moz-selection { background: #cfc; } -.cm-s-base16-light.CodeMirror { background: #f8f8f8; color: #444444; } -.cm-s-base16-light div.CodeMirror-selected { background: #e0e0e0; } -.cm-s-base16-light .CodeMirror-line::selection, .cm-s-base16-light .CodeMirror-line > span::selection, .cm-s-base16-light .CodeMirror-line > span > span::selection { background: #e0e0e0; } -.cm-s-base16-light .CodeMirror-line::-moz-selection, .cm-s-base16-light .CodeMirror-line > span::-moz-selection, .cm-s-base16-light .CodeMirror-line > span > span::-moz-selection { background: #e0e0e0; } -.cm-s-base16-light .CodeMirror-gutters { background: #f5f5f5; border-right: 0px; } -.cm-s-base16-light .CodeMirror-guttermarker { color: #ac4142; } -.cm-s-base16-light .CodeMirror-guttermarker-subtle { color: #b0b0b0; } -.cm-s-base16-light .CodeMirror-linenumber { color: #b0b0b0; } -.cm-s-base16-light .CodeMirror-cursor { border-left: 1px solid #505050; } +.cm-s-mdn-like .CodeMirror-gutters { background: #f8f8f8; border-left: 6px solid rgba(0,83,159,0.65); color: #333; } +.cm-s-mdn-like .CodeMirror-linenumber { color: #aaa; padding-left: 8px; } +.cm-s-mdn-like .CodeMirror-cursor { border-left: 2px solid #222; } -.cm-s-base16-light span.cm-comment { color: #8f5536; } -.cm-s-base16-light span.cm-atom { color: #aa759f; } -.cm-s-base16-light span.cm-number { color: #aa759f; } +.cm-s-mdn-like .cm-keyword { color: #6262FF; } +.cm-s-mdn-like .cm-atom { color: #F90; } +.cm-s-mdn-like .cm-number { color: #ca7841; } +.cm-s-mdn-like .cm-def { color: #8DA6CE; } +.cm-s-mdn-like span.cm-variable-2, .cm-s-mdn-like span.cm-tag { color: #690; } +.cm-s-mdn-like span.cm-variable-3, .cm-s-mdn-like span.cm-def, .cm-s-mdn-like span.cm-type { color: #07a; } -.cm-s-base16-light span.cm-property, .cm-s-base16-light span.cm-attribute { color: #678c30; } -.cm-s-base16-light span.cm-keyword { color: #ac4142; } -.cm-s-base16-light span.cm-string { color: #e09c3c; } +.cm-s-mdn-like .cm-variable { color: #07a; } +.cm-s-mdn-like .cm-property { color: #905; } +.cm-s-mdn-like .cm-qualifier { color: #690; } -.cm-s-base16-light span.cm-builtin { color: #4c7f9e; } -.cm-s-base16-light span.cm-variable { color: #90a959; } -.cm-s-base16-light span.cm-variable-2 { color: #6a9fb5; } -.cm-s-base16-light span.cm-def { color: #d28445; } -.cm-s-base16-light span.cm-bracket { color: #202020; } -.cm-s-base16-light span.cm-tag { color: #ac4142; } -.cm-s-base16-light span.cm-link { color: #aa759f; } -.cm-s-base16-light span.cm-error { background: #ac4142; color: #505050; } +.cm-s-mdn-like .cm-operator { color: #cda869; } +.cm-s-mdn-like .cm-comment { color:#777; font-weight:normal; } +.cm-s-mdn-like .cm-string { color:#07a; font-style:italic; } +.cm-s-mdn-like .cm-string-2 { color:#bd6b18; } /*?*/ +.cm-s-mdn-like .cm-meta { color: #000; } /*?*/ +.cm-s-mdn-like .cm-builtin { color: #9B7536; } /*?*/ +.cm-s-mdn-like .cm-tag { color: #997643; } +.cm-s-mdn-like .cm-attribute { color: #d6bb6d; } /*?*/ +.cm-s-mdn-like .cm-header { color: #FF6400; } +.cm-s-mdn-like .cm-hr { color: #AEAEAE; } +.cm-s-mdn-like .cm-link { color:#ad9361; font-style:italic; text-decoration:none; } +.cm-s-mdn-like .cm-error { border-bottom: 1px solid red; } -.cm-s-base16-light .CodeMirror-activeline-background { background: #DDDCDC; } -.cm-s-base16-light .CodeMirror-matchingbracket { text-decoration: underline; color: white !important; } +div.cm-s-mdn-like .CodeMirror-activeline-background { background: #efefff; } +div.cm-s-mdn-like span.CodeMirror-matchingbracket { outline:1px solid grey; color: inherit; } + +.cm-s-mdn-like.CodeMirror { background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAFcAAAAyCAYAAAAp8UeFAAAHvklEQVR42s2b63bcNgyEQZCSHCdt2vd/0tWF7I+Q6XgMXiTtuvU5Pl57ZQKkKHzEAOtF5KeIJBGJ8uvL599FRFREZhFx8DeXv8trn68RuGaC8TRfo3SNp9dlDDHedyLyTUTeRWStXKPZrjtpZxaRw5hPqozRs1N8/enzIiQRWcCgy4MUA0f+XWliDhyL8Lfyvx7ei/Ae3iQFHyw7U/59pQVIMEEPEz0G7XiwdRjzSfC3UTtz9vchIntxvry5iMgfIhJoEflOz2CQr3F5h/HfeFe+GTdLaKcu9L8LTeQb/R/7GgbsfKedyNdoHsN31uRPWrfZ5wsj/NzzRQHuToIdU3ahwnsKPxXCjJITuOsi7XLc7SG/v5GdALs7wf8JjTFiB5+QvTEfRyGOfX3Lrx8wxyQi3sNq46O7QahQiCsRFgqddjBouVEHOKDgXAQHD9gJCr5sMKkEdjwsarG/ww3BMHBU7OBjXnzdyY7SfCxf5/z6ATccrwlKuwC/jhznnPF4CgVzhhVf4xp2EixcBActO75iZ8/fM9zAs2OMzKdslgXWJ9XG8PQoOAMA5fGcsvORgv0doBXyHrCwfLJAOwo71QLNkb8n2Pl6EWiR7OCibtkPaz4Kc/0NNAze2gju3zOwekALDaCFPI5vjPFmgGY5AZqyGEvH1x7QfIb8YtxMnA/b+QQ0aQDAwc6JMFg8CbQZ4qoYEEHbRwNojuK3EHwd7VALSgq+MNDKzfT58T8qdpADrgW0GmgcAS1lhzztJmkAzcPNOQbsWEALBDSlMKUG0Eq4CLAQWvEVQ9WU57gZJwZtgPO3r9oBTQ9WO8TjqXINx8R0EYpiZEUWOF3FxkbJkgU9B2f41YBrIj5ZfsQa0M5kTgiAAqM3ShXLgu8XMqcrQBvJ0CL5pnTsfMB13oB8athpAq2XOQmcGmoACCLydx7nToa23ATaSIY2ichfOdPTGxlasXMLaL0MLZAOwAKIM+y8CmicobGdCcbbK9DzN+yYGVoNNI5iUKTMyYOjPse4A8SM1MmcXgU0toOq1yO/v8FOxlASyc7TgeYaAMBJHcY1CcCwGI/TK4AmDbDyKYBBtFUkRwto8gygiQEaByFgJ00BH2M8JWwQS1nafDXQCidWyOI8AcjDCSjCLk8ngObuAm3JAHAdubAmOaK06V8MNEsKPJOhobSprwQa6gD7DclRQdqcwL4zxqgBrQcabUiBLclRDKAlWp+etPkBaNMA0AKlrHwTdEByZAA4GM+SNluSY6wAzcMNewxmgig5Ks0nkrSpBvSaQHMdKTBAnLojOdYyGpQ254602ZILPdTD1hdlggdIm74jbTp8vDwF5ZYUeLWGJpWsh6XNyXgcYwVoJQTEhhTYkxzZjiU5npU2TaB979TQehlaAVq4kaGpiPwwwLkYUuBbQwocyQTv1tA0+1UFWoJF3iv1oq+qoSk8EQdJmwHkziIF7oOZk14EGitibAdjLYYK78H5vZOhtWpoI0ATGHs0Q8OMb4Ey+2bU2UYztCtA0wFAs7TplGLRVQCcqaFdGSPCeTI1QNIC52iWNzof6Uib7xjEp07mNNoUYmVosVItHrHzRlLgBn9LFyRHaQCtVUMbtTNhoXWiTOO9k/V8BdAc1Oq0ArSQs6/5SU0hckNy9NnXqQY0PGYo5dWJ7nINaN6o958FWin27aBaWRka1r5myvLOAm0j30eBJqCxHLReVclxhxOEN2JfDWjxBtAC7MIH1fVaGdoOp4qJYDgKtKPSFNID2gSnGldrCqkFZ+5UeQXQBIRrSwocbdZYQT/2LwRahBPBXoHrB8nxaGROST62DKUbQOMMzZIC9abkuELfQzQALWTnDNAm8KHWFOJgJ5+SHIvTPcmx1xQyZRhNL5Qci689aXMEaN/uNIWkEwDAvFpOZmgsBaaGnbs1NPa1Jm32gBZAIh1pCtG7TSH4aE0y1uVY4uqoFPisGlpP2rSA5qTecWn5agK6BzSpgAyD+wFaqhnYoSZ1Vwr8CmlTQbrcO3ZaX0NAEyMbYaAlyquFoLKK3SPby9CeVUPThrSJmkCAE0CrKUQadi4DrdSlWhmah0YL9z9vClH59YGbHx1J8VZTyAjQepJjmXwAKTDQI3omc3p1U4gDUf6RfcdYfrUp5ClAi2J3Ba6UOXGo+K+bQrjjssitG2SJzshaLwMtXgRagUNpYYoVkMSBLM+9GGiJZMvduG6DRZ4qc04DMPtQQxOjEtACmhO7K1AbNbQDEggZyJwscFpAGwENhoBeUwh3bWolhe8BTYVKxQEWrSUn/uhcM5KhvUu/+eQu0Lzhi+VrK0PrZZNDQKs9cpYUuFYgMVpD4/NxenJTiMCNqdUEUf1qZWjppLT5qSkkUZbCwkbZMSuVnu80hfSkzRbQeqCZSAh6huR4VtoM2gHAlLf72smuWgE+VV7XpE25Ab2WFDgyhnSuKbs4GuGzCjR+tIoUuMFg3kgcWKLTwRqanJQ2W00hAsenfaApRC42hbCvK1SlE0HtE9BGgneJO+ELamitD1YjjOYnNYVcraGhtKkW0EqVVeDx733I2NH581k1NNxNLG0i0IJ8/NjVaOZ0tYZ2Vtr0Xv7tPV3hkWp9EFkgS/J0vosngTaSoaG06WHi+xObQkaAdlbanP8B2+2l0f90LmUAAAAASUVORK5CYII=); } /** * Custom BookStack overrides @@ -394,7 +401,8 @@ span.CodeMirror-selectedtext { background: none; } margin-bottom: $-l; border: 1px solid #DDD;; } -.cm-s-base16-light .CodeMirror-gutters { background: #f5f5f5; border-right: 1px solid #DDD; } + +.cm-s-mdn-like .CodeMirror-gutters { background: #f8f8f8; border-left: 0; color: #333; } .code-fill .CodeMirror { position: absolute; From 64455307b13d7eae3d40ab91a75a34ddec3ea686 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 10:04:13 +0000 Subject: [PATCH 20/23] Added a few test to cover api docs pages --- tests/Api/ApiDocsTest.php | 42 ++++++++++++++++++++++++++++++++++++ tests/Api/ApiListingTest.php | 2 -- 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/Api/ApiDocsTest.php diff --git a/tests/Api/ApiDocsTest.php b/tests/Api/ApiDocsTest.php new file mode 100644 index 000000000..b240c1672 --- /dev/null +++ b/tests/Api/ApiDocsTest.php @@ -0,0 +1,42 @@ +getViewer(); + $resp = $this->actingAs($viewer)->get($this->endpoint); + $resp->assertStatus(403); + + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertStatus(200); + } + + public function test_docs_page_returns_view_with_docs_content() + { + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertStatus(200); + $resp->assertSee(url('/api/docs.json')); + $resp->assertSee('Show a JSON view of the API docs data.'); + $resp->assertHeader('Content-Type', 'text/html; charset=UTF-8'); + } + + public function test_docs_json_endpoint_returns_json() + { + $resp = $this->actingAsApiEditor()->get($this->endpoint . '.json'); + $resp->assertStatus(200); + $resp->assertHeader('Content-Type', 'application/json'); + $resp->assertJson([ + 'docs' => [ [ + 'name' => 'docs-display', + 'uri' => 'api/docs' + ] ] + ]); + } +} \ No newline at end of file diff --git a/tests/Api/ApiListingTest.php b/tests/Api/ApiListingTest.php index fa28dfb36..741b9664b 100644 --- a/tests/Api/ApiListingTest.php +++ b/tests/Api/ApiListingTest.php @@ -2,9 +2,7 @@ namespace Tests; -use BookStack\Auth\Permissions\RolePermission; use BookStack\Entities\Book; -use Carbon\Carbon; class ApiListingTest extends TestCase { From b9fb655b60fd8b4ad46699d52a64bd5f29f43ddb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 14:03:11 +0000 Subject: [PATCH 21/23] Added "Getting Started" API docs --- .../Controllers/Api/BooksApiController.php | 2 +- .../{books-index.json => books-list.json} | 0 resources/js/components/code-highlighter.js | 10 ++ resources/js/components/index.js | 2 + resources/lang/en/settings.php | 1 + resources/views/api-docs/index.blade.php | 157 +++++++++++++++++- .../views/users/api-tokens/list.blade.php | 34 ++++ resources/views/users/edit.blade.php | 34 +--- routes/api.php | 2 +- 9 files changed, 206 insertions(+), 36 deletions(-) rename dev/api/responses/{books-index.json => books-list.json} (100%) create mode 100644 resources/js/components/code-highlighter.js create mode 100644 resources/views/users/api-tokens/list.blade.php diff --git a/app/Http/Controllers/Api/BooksApiController.php b/app/Http/Controllers/Api/BooksApiController.php index fa174dfd3..ac4ea171c 100644 --- a/app/Http/Controllers/Api/BooksApiController.php +++ b/app/Http/Controllers/Api/BooksApiController.php @@ -35,7 +35,7 @@ class BooksApiController extends ApiController /** * Get a listing of books visible to the user. */ - public function index() + public function list() { $books = Book::visible(); return $this->apiListingResponse($books, [ diff --git a/dev/api/responses/books-index.json b/dev/api/responses/books-list.json similarity index 100% rename from dev/api/responses/books-index.json rename to dev/api/responses/books-list.json diff --git a/resources/js/components/code-highlighter.js b/resources/js/components/code-highlighter.js new file mode 100644 index 000000000..db6112887 --- /dev/null +++ b/resources/js/components/code-highlighter.js @@ -0,0 +1,10 @@ +import Code from "../services/code" +class CodeHighlighter { + + constructor(elem) { + Code.highlightWithin(elem); + } + +} + +export default CodeHighlighter; \ No newline at end of file diff --git a/resources/js/components/index.js b/resources/js/components/index.js index d3ba539dd..112827330 100644 --- a/resources/js/components/index.js +++ b/resources/js/components/index.js @@ -31,6 +31,7 @@ import entityPermissionsEditor from "./entity-permissions-editor"; import templateManager from "./template-manager"; import newUserPassword from "./new-user-password"; import detailsHighlighter from "./details-highlighter"; +import codeHighlighter from "./code-highlighter"; const componentMapping = { 'dropdown': dropdown, @@ -66,6 +67,7 @@ const componentMapping = { 'template-manager': templateManager, 'new-user-password': newUserPassword, 'details-highlighter': detailsHighlighter, + 'code-highlighter': codeHighlighter, }; window.components = {}; diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index b1da5435f..fb00c3cce 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -156,6 +156,7 @@ return [ 'users_api_tokens_none' => 'No API tokens have been created for this user', 'users_api_tokens_create' => 'Create Token', 'users_api_tokens_expires' => 'Expires', + 'users_api_tokens_docs' => 'API Documentation', // API Tokens 'user_api_token_create' => 'Create API Token', diff --git a/resources/views/api-docs/index.blade.php b/resources/views/api-docs/index.blade.php index a20ba04cc..e9583838c 100644 --- a/resources/views/api-docs/index.blade.php +++ b/resources/views/api-docs/index.blade.php @@ -5,8 +5,17 @@
-
+ +

Getting Started

+ + + @foreach($docs as $model => $endpoints)

{{ $model }}

@@ -24,6 +33,152 @@
+ +
+

Getting Started

+ +
Authentication
+

+ To access the API a user has to have the "Access System API" permission enabled on one of their assigned roles. + Permissions to content accessed via the API is limited by the roles & permissions assigned to the user that's used to access the API. +

+

Authentication to use the API is primarily done using API Tokens. Once the "Access System API" permission has been assigned to a user, a "API Tokens" section should be visible when editing their user profile. Choose "Create Token" and enter an appropriate name and expiry date, relevant for your API usage then press "Save". A "Token ID" and "Token Secret" will be immediately displayed. These values should be used as a header in API HTTP requests in the following format:

+
Authorization: Token <token_id>:<token_secret>
+

Here's an example of an authorized cURL request to list books in the system:

+
curl --request GET \
+  --url https://example.com/api/books \
+  --header 'Authorization: Token C6mdvEQTGnebsmVn3sFNeeuelGEBjyQp:NOvD3VlzuSVuBPNaf1xWHmy7nIRlaj22'
+

If already logged into the system within the browser, via a user account with permission to access the API, the system will also accept an existing session meaning you can browse API endpoints directly in the browser or use the browser devtools to play with the API.

+ +
+ +
Request Format
+

The API is primarily design to be interfaced using JSON so the majority of API endpoints, that accept data, will read JSON request data although application/x-www-form-urlencoded request data is also accepted. Endpoints that receive file data will need data sent in a multipart/form-data format although this will be highlighted in the documentation for such endpoints.

+

For endpoints in this documentation that accept data, a "Body Parameters" table will be available showing the parameters that will accepted in the request. Any rules for the values of such parameters, such as the data-type or if they're required, will be shown alongside the parameter name.

+ +
+ +
Listing Endpoints
+

Some endpoints will return a list of data models. These endpoints will return an array of the model data under a data property along with a numeric total property to indicate the total number of records found for the query within the system. Here's an example of a listing response:

+
{
+  "data": [
+    {
+      "id": 1,
+      "name": "BookStack User Guide",
+      "slug": "bookstack-user-guide",
+      "description": "This is a general guide on using BookStack on a day-to-day basis.",
+      "created_at": "2019-05-05 21:48:46",
+      "updated_at": "2019-12-11 20:57:31",
+      "created_by": 1,
+      "updated_by": 1,
+      "image_id": 3
+    }
+  ],
+  "total": 16
+}
+

+ There are a number of standard URL parameters that can be supplied to manipulate and page through the results returned from a listing endpoint: +

+ + + + + + + + + + + + + + + + + + + + + + + + + + +
ParameterDetailsExamples
count + Specify how many records will be returned in the response.
+ (Default: {{ config('api.default_item_count') }}, Max: {{ config('api.max_item_count') }}) +
Limit the count to 50
?count=50
offset + Specify how many records to skip over in the response.
+ (Default: 0) +
Skip over the first 100 records
?offset=100
sort + Specify what field is used to sort the data and the direction of the sort (Ascending or Descending).
+ Value is the name of a field, A + or - prefix dictates ordering.
+ Direction defaults to ascending.
+ Can use most fields shown in the response. +
+ Sort by name ascending
?sort=+name

+ Sort by "Created At" date descending
?sort=-created_at +
filter[<field>] + Specify a filter to be applied to the query. Can use most fields shown in the response.
+ By default a filter will apply a "where equals" query but the below operations are available using the format filter[<field>:<operation>]
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
eqWhere <field> equals the filter value.
neWhere <field> does not equal the filter value.
gtWhere <field> is greater than the filter value.
ltWhere <field> is less than the filter value.
gteWhere <field> is greater than or equal to the filter value.
lteWhere <field> is less than or equal to the filter value.
like + Where <field> is "like" the filter value.
+ % symbols can be used as wildcards. +
+
+ Filter where id is 5:
?filter[id]=5

+ Filter where id is not 5:
?filter[id:ne]=5

+ Filter where name contains "cat":
?filter[name:like]=%cat%

+ Filter where created after 2020-01-01:
?filter[created_at:gt]=2020-01-01 +
+ +
+ +
Error Handling
+

+ Successful responses will return a 200 or 204 HTTP response code. Errors will return a 4xx or a 5xx HTTP response code depending on the type of error. Errors follow a standard format as shown below. The message provided may be translated depending on the configured language of the system in addition to the API users' language preference. The code provided in the JSON response will match the HTTP response code. +

+ +
{
+	"error": {
+		"code": 401,
+		"message": "No authorization token found on the request"
+	}
+}
+
+ +
+ @foreach($docs as $model => $endpoints)

{{ $model }}

diff --git a/resources/views/users/api-tokens/list.blade.php b/resources/views/users/api-tokens/list.blade.php new file mode 100644 index 000000000..ea1893372 --- /dev/null +++ b/resources/views/users/api-tokens/list.blade.php @@ -0,0 +1,34 @@ +
+
+

{{ trans('settings.users_api_tokens') }}

+ +
+ @if (count($user->apiTokens) > 0) + + + + + + + @foreach($user->apiTokens as $token) + + + + + + @endforeach +
{{ trans('common.name') }}{{ trans('settings.users_api_tokens_expires') }}
+ {{ $token->name }}
+ {{ $token->token_id }} +
{{ $token->expires_at->format('Y-m-d') ?? '' }} + {{ trans('common.edit') }} +
+ @else +

{{ trans('settings.users_api_tokens_none') }}

+ @endif +
\ No newline at end of file diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index c69d101d4..f78c25ceb 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -89,39 +89,7 @@ @endif @if(($currentUser->id === $user->id && userCan('access-api')) || userCan('users-manage')) -
-
-

{{ trans('settings.users_api_tokens') }}

-
- @if(userCan('access-api')) - {{ trans('settings.users_api_tokens_create') }} - @endif -
-
- @if (count($user->apiTokens) > 0) - - - - - - - @foreach($user->apiTokens as $token) - - - - - - @endforeach -
{{ trans('common.name') }}{{ trans('settings.users_api_tokens_expires') }}
- {{ $token->name }}
- {{ $token->token_id }} -
{{ $token->expires_at->format('Y-m-d') ?? '' }} - {{ trans('common.edit') }} -
- @else -

{{ trans('settings.users_api_tokens_none') }}

- @endif -
+ @include('users.api-tokens.list', ['user' => $user]) @endif
diff --git a/routes/api.php b/routes/api.php index 12b327798..73f2faf79 100644 --- a/routes/api.php +++ b/routes/api.php @@ -9,7 +9,7 @@ Route::get('docs', 'ApiDocsController@display'); Route::get('docs.json', 'ApiDocsController@json'); -Route::get('books', 'BooksApiController@index'); +Route::get('books', 'BooksApiController@list'); Route::post('books', 'BooksApiController@create'); Route::get('books/{id}', 'BooksApiController@read'); Route::put('books/{id}', 'BooksApiController@update'); From 1350136ca3029b58cc6f930dbe5e5eed539234ce Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 14:07:43 +0000 Subject: [PATCH 22/23] Fixed bad test class name --- tests/Api/BooksApiTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php index f560bfffd..813e34360 100644 --- a/tests/Api/BooksApiTest.php +++ b/tests/Api/BooksApiTest.php @@ -2,7 +2,7 @@ use BookStack\Entities\Book; -class ApiAuthTest extends TestCase +class BooksApiTest extends TestCase { use TestsApi; From be554b9c797c53a9de301dc58b9e8bc1e8b54ae6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 18 Jan 2020 15:03:28 +0000 Subject: [PATCH 23/23] Added configurable API throttling, Handled API errors standardly --- .env.example.complete | 5 ++- app/Config/api.php | 3 ++ app/Exceptions/Handler.php | 42 +++++++++++++++++++++ app/Http/Kernel.php | 2 +- app/Http/Middleware/ThrottleApiRequests.php | 18 +++++++++ phpunit.xml | 1 + tests/Api/ApiAuthTest.php | 25 ++++++++++++ tests/Api/ApiConfigTest.php | 11 ++++++ tests/Api/BooksApiTest.php | 20 ++++++++++ 9 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 app/Http/Middleware/ThrottleApiRequests.php diff --git a/.env.example.complete b/.env.example.complete index 716d614a3..e44644f08 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -260,4 +260,7 @@ ALLOW_ROBOTS=null # The default and maximum item-counts for listing API requests. API_DEFAULT_ITEM_COUNT=100 -API_MAX_ITEM_COUNT=500 \ No newline at end of file +API_MAX_ITEM_COUNT=500 + +# The number of API requests that can be made per minute by a single user. +API_REQUESTS_PER_MIN=180 \ No newline at end of file diff --git a/app/Config/api.php b/app/Config/api.php index dd54b2906..6afea2dc8 100644 --- a/app/Config/api.php +++ b/app/Config/api.php @@ -17,4 +17,7 @@ return [ // The maximum number of items that can be returned in a listing API request. 'max_item_count' => env('API_MAX_ITEM_COUNT', 500), + // The number of API requests that can be made per minute by a single user. + 'requests_per_minute' => env('API_REQUESTS_PER_MIN', 180) + ]; diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 70a534975..284d731d7 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -7,6 +7,9 @@ use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Auth\AuthenticationException; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; +use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; +use Illuminate\Http\Response; use Illuminate\Validation\ValidationException; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -47,6 +50,10 @@ class Handler extends ExceptionHandler */ public function render($request, Exception $e) { + if ($this->isApiRequest($request)) { + return $this->renderApiException($e); + } + // Handle notify exceptions which will redirect to the // specified location then show a notification message. if ($this->isExceptionType($e, NotifyException::class)) { @@ -70,6 +77,41 @@ class Handler extends ExceptionHandler return parent::render($request, $e); } + /** + * Check if the given request is an API request. + */ + protected function isApiRequest(Request $request): bool + { + return strpos($request->path(), 'api/') === 0; + } + + /** + * Render an exception when the API is in use. + */ + protected function renderApiException(Exception $e): JsonResponse + { + $code = $e->getCode() === 0 ? 500 : $e->getCode(); + $headers = []; + if ($e instanceof HttpException) { + $code = $e->getStatusCode(); + $headers = $e->getHeaders(); + } + + $responseData = [ + 'error' => [ + 'message' => $e->getMessage(), + ] + ]; + + if ($e instanceof ValidationException) { + $responseData['error']['validation'] = $e->errors(); + $code = $e->status; + } + + $responseData['error']['code'] = $code; + return new JsonResponse($responseData, $code, $headers); + } + /** * Check the exception chain to compare against the original exception type. * @param Exception $e diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 978583a7f..c2016281a 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -32,7 +32,7 @@ class Kernel extends HttpKernel \BookStack\Http\Middleware\GlobalViewData::class, ], 'api' => [ - 'throttle:60,1', + \BookStack\Http\Middleware\ThrottleApiRequests::class, \BookStack\Http\Middleware\EncryptCookies::class, \BookStack\Http\Middleware\StartSessionIfCookieExists::class, \BookStack\Http\Middleware\ApiAuthenticate::class, diff --git a/app/Http/Middleware/ThrottleApiRequests.php b/app/Http/Middleware/ThrottleApiRequests.php new file mode 100644 index 000000000..d08840cd1 --- /dev/null +++ b/app/Http/Middleware/ThrottleApiRequests.php @@ -0,0 +1,18 @@ + + diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php index b6b6b72ac..2ab81814b 100644 --- a/tests/Api/ApiAuthTest.php +++ b/tests/Api/ApiAuthTest.php @@ -120,4 +120,29 @@ class ApiAuthTest extends TestCase $resp->assertJson($this->errorResponse("The email address for the account in use needs to be confirmed", 401)); } + public function test_rate_limit_headers_active_on_requests() + { + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertHeader('x-ratelimit-limit', 180); + $resp->assertHeader('x-ratelimit-remaining', 179); + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertHeader('x-ratelimit-remaining', 178); + } + + public function test_rate_limit_hit_gives_json_error() + { + config()->set(['api.requests_per_minute' => 1]); + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertStatus(200); + + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertStatus(429); + $resp->assertHeader('x-ratelimit-remaining', 0); + $resp->assertHeader('retry-after'); + $resp->assertJson([ + 'error' => [ + 'code' => 429, + ] + ]); + } } \ No newline at end of file diff --git a/tests/Api/ApiConfigTest.php b/tests/Api/ApiConfigTest.php index d9367741f..1b3da2f34 100644 --- a/tests/Api/ApiConfigTest.php +++ b/tests/Api/ApiConfigTest.php @@ -44,4 +44,15 @@ class ApiConfigTest extends TestCase $resp->assertJsonCount(2, 'data'); } + public function test_requests_per_min_alters_rate_limit() + { + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertHeader('x-ratelimit-limit', 180); + + config()->set(['api.requests_per_minute' => 10]); + + $resp = $this->actingAsApiEditor()->get($this->endpoint); + $resp->assertHeader('x-ratelimit-limit', 10); + } + } \ No newline at end of file diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php index 813e34360..a40e4c93b 100644 --- a/tests/Api/BooksApiTest.php +++ b/tests/Api/BooksApiTest.php @@ -38,6 +38,26 @@ class BooksApiTest extends TestCase $this->assertActivityExists('book_create', $newItem); } + public function test_book_name_needed_to_create() + { + $this->actingAsApiEditor(); + $details = [ + 'description' => 'A book created via the API', + ]; + + $resp = $this->postJson($this->baseEndpoint, $details); + $resp->assertStatus(422); + $resp->assertJson([ + "error" => [ + "message" => "The given data was invalid.", + "validation" => [ + "name" => ["The name field is required."] + ], + "code" => 422, + ], + ]); + } + public function test_read_endpoint() { $this->actingAsApiEditor();