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 @@ +<?php + +namespace BookStack\Exceptions; + +use Exception; + +class UnauthorizedException extends Exception +{ + + /** + * ApiAuthException constructor. + */ + public function __construct($message, $code = 401) + { + parent::__construct($message, $code); + } +} \ No newline at end of file diff --git a/app/Http/Controllers/Api/ApiController.php b/app/Http/Controllers/Api/ApiController.php index 4971c0cde..b3f1fb747 100644 --- a/app/Http/Controllers/Api/ApiController.php +++ b/app/Http/Controllers/Api/ApiController.php @@ -14,7 +14,7 @@ class ApiController extends Controller */ protected function apiListingResponse(Builder $query, array $fields): JsonResponse { - $listing = new ListingResponseBuilder($query, $fields); + $listing = new ListingResponseBuilder($query, request(), $fields); return $listing->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