From d393c693dedc46a3233418d7e4597a117b3e1578 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 9 Feb 2018 14:47:37 +0100 Subject: [PATCH] Fix issues with API authentication. --- app/Api/V1/Controllers/BillController.php | 60 +++++++++++-- app/Http/Middleware/Authenticate.php | 99 ++++++++++++++++------ app/Http/Middleware/Binder.php | 21 ++++- app/Models/Bill.php | 2 +- app/Transformers/AttachmentTransformer.php | 7 ++ app/Transformers/BillTransformer.php | 17 ++-- app/Transformers/NoteTransformer.php | 7 ++ routes/api.php | 2 +- 8 files changed, 168 insertions(+), 47 deletions(-) diff --git a/app/Api/V1/Controllers/BillController.php b/app/Api/V1/Controllers/BillController.php index 25b4055e9b..ad9b711a13 100644 --- a/app/Api/V1/Controllers/BillController.php +++ b/app/Api/V1/Controllers/BillController.php @@ -25,12 +25,14 @@ namespace FireflyIII\Api\V1\Controllers; use Auth; use Carbon\Carbon; use FireflyIII\Models\Bill; +use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\Transformers\BillTransformer; use Illuminate\Http\Request; use Illuminate\Support\Collection; use League\Fractal\Manager; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; +use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; use Preferences; use Response; @@ -40,6 +42,28 @@ use Response; */ class BillController extends Controller { + /** @var BillRepositoryInterface */ + private $repository; + + /** + * BillController constructor. + * + * @throws \FireflyIII\Exceptions\FireflyException + */ + public function __construct() + { + parent::__construct(); + $this->middleware( + function ($request, $next) { + /** @var BillRepositoryInterface repository */ + $this->repository = app(BillRepositoryInterface::class); + $this->repository->setUser(auth()->user()); + + return $next($request); + } + ); + } + /** * Remove the specified resource from storage. * @@ -49,7 +73,9 @@ class BillController extends Controller */ public function destroy(Bill $bill) { - // + $this->repository->destroy($bill); + return response()->json(null, 204); + } /** @@ -69,7 +95,7 @@ class BillController extends Controller if (null !== $request->get('end')) { $end = new Carbon($request->get('end')); } - $paginator = $user->bills()->paginate($pageSize); + $paginator = $this->repository->getPaginator($pageSize); /** @var Collection $bills */ $bills = $paginator->getCollection(); @@ -84,16 +110,34 @@ class BillController extends Controller return Response::json($manager->createData($resource)->toArray()); } + /** - * Display the specified resource. + * @param Request $request + * @param Bill $bill * - * @param \FireflyIII\Models\Bill $bill - * - * @return \Illuminate\Http\Response + * @return \Illuminate\Http\JsonResponse */ - public function show(Bill $bill) + public function show(Request $request, Bill $bill) { - // + $start = null; + $end = null; + if (null !== $request->get('start')) { + $start = new Carbon($request->get('start')); + } + if (null !== $request->get('end')) { + $end = new Carbon($request->get('end')); + } + + + $manager = new Manager(); + $manager->parseIncludes(['attachments']); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); + + $resource = new Item($bill, new BillTransformer($start, $end), 'bill'); + + return Response::json($manager->createData($resource)->toArray()); + } /** diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 3e6263c80f..66373305ca 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -1,7 +1,7 @@ . */ -declare(strict_types=1); namespace FireflyIII\Http\Middleware; use Closure; -use Illuminate\Http\Request; -use Illuminate\Support\Facades\Auth; -use Session; +use Illuminate\Auth\AuthenticationException; +use Illuminate\Contracts\Auth\Factory as Auth; /** - * Class Authenticate. + * Class Authenticate */ class Authenticate { + /** + * The authentication factory instance. + * + * @var \Illuminate\Contracts\Auth\Factory + */ + protected $auth; + + /** + * Create a new middleware instance. + * + * @param \Illuminate\Contracts\Auth\Factory $auth + * + * @return void + */ + public function __construct(Auth $auth) + { + $this->auth = $auth; + } + /** * Handle an incoming request. * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @param string|null $guard + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @param string[] ...$guards * * @return mixed + * + * @throws \Illuminate\Auth\AuthenticationException */ - public function handle(Request $request, Closure $next, $guard = null) + public function handle($request, Closure $next, ...$guards) { - if (Auth::guard($guard)->guest()) { - if ($request->ajax()) { - return response('Unauthorized.', 401); - } - - return redirect()->guest('login'); - } - if (1 === intval(Auth::guard($guard)->user()->blocked)) { - $message = strval(trans('firefly.block_account_logout')); - if ('email_changed' === Auth::guard($guard)->user()->blocked_code) { - $message = strval(trans('firefly.email_changed_logout')); - } - - Session::flash('logoutMessage', $message); - Auth::guard($guard)->logout(); - - return redirect()->guest('login'); - } + $this->authenticate($guards); return $next($request); } + + /** + * Determine if the user is logged in to any of the given guards. + * + * @param array $guards + * + * @return void + * + * @throws \Illuminate\Auth\AuthenticationException + */ + protected function authenticate(array $guards) + { + if (empty($guards)) { + // go for default guard: + if ($this->auth->check()) { + // do an extra check on user object. + $user = $this->auth->authenticate(); + if (1 === intval($user->blocked)) { + $message = strval(trans('firefly.block_account_logout')); + if ('email_changed' === $user->blocked_code) { + $message = strval(trans('firefly.email_changed_logout')); + } + + app('session')->flash('logoutMessage', $message); + $this->auth->logout(); + + return redirect()->guest('login'); + } + } + + return $this->auth->authenticate(); + } + + foreach ($guards as $guard) { + if ($this->auth->guard($guard)->check()) { + return $this->auth->shouldUse($guard); + } + } + + throw new AuthenticationException('Unauthenticated.', $guards); + } } diff --git a/app/Http/Middleware/Binder.php b/app/Http/Middleware/Binder.php index 0a7bfc1bda..48e04243c9 100644 --- a/app/Http/Middleware/Binder.php +++ b/app/Http/Middleware/Binder.php @@ -24,6 +24,7 @@ namespace FireflyIII\Http\Middleware; use Closure; use FireflyIII\Support\Domain; +use Illuminate\Contracts\Auth\Factory as Auth; use Illuminate\Http\Request; /** @@ -31,6 +32,12 @@ use Illuminate\Http\Request; */ class Binder { + /** + * The authentication factory instance. + * + * @var \Illuminate\Contracts\Auth\Factory + */ + protected $auth; /** * @var array */ @@ -38,21 +45,27 @@ class Binder /** * Binder constructor. + * + * @param \Illuminate\Contracts\Auth\Factory $auth */ - public function __construct() + public function __construct(Auth $auth) { $this->binders = Domain::getBindables(); + $this->auth = $auth; } /** * Handle an incoming request. * - * @param \Illuminate\Http\Request $request - * @param \Closure $next + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @param string[] ...$guards * * @return mixed + * + * @throws \Illuminate\Auth\AuthenticationException */ - public function handle(Request $request, Closure $next) + public function handle($request, Closure $next, ...$guards) { foreach ($request->route()->parameters() as $key => $value) { if (isset($this->binders[$key])) { diff --git a/app/Models/Bill.php b/app/Models/Bill.php index ec09d0fcc7..06878b225a 100644 --- a/app/Models/Bill.php +++ b/app/Models/Bill.php @@ -29,7 +29,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\SoftDeletes; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Watson\Validating\ValidatingTrait; - +use Illuminate\Contracts\Auth\Factory as Auth; /** * Class Bill. */ diff --git a/app/Transformers/AttachmentTransformer.php b/app/Transformers/AttachmentTransformer.php index 81c03f4b7e..5495a8ae2c 100644 --- a/app/Transformers/AttachmentTransformer.php +++ b/app/Transformers/AttachmentTransformer.php @@ -49,6 +49,13 @@ class AttachmentTransformer extends TransformerAbstract 'notes' => $attachment->notes, 'mime' => $attachment->mime, 'size' => $attachment->size, + 'links' => [ + [ + 'rel' => 'self', + 'uri' => '/attachment/' . $attachment->id, + ], + ] + ]; } diff --git a/app/Transformers/BillTransformer.php b/app/Transformers/BillTransformer.php index a7aac08745..0394ad0037 100644 --- a/app/Transformers/BillTransformer.php +++ b/app/Transformers/BillTransformer.php @@ -72,7 +72,8 @@ class BillTransformer extends TransformerAbstract { $attachments = $bill->attachments()->get(); - return $this->collection($attachments, new AttachmentTransformer); + return $this->collection($attachments, new AttachmentTransformer,'attachment'); + } /** @@ -84,7 +85,8 @@ class BillTransformer extends TransformerAbstract { $notes = $bill->notes()->get(); - return $this->collection($notes, new NoteTransformer); + return $this->collection($notes, new NoteTransformer,'note'); + } /** @@ -95,7 +97,9 @@ class BillTransformer extends TransformerAbstract public function transform(Bill $bill): array { $paidData = $this->paidData($bill); - $data = [ + $payDates = $this->payDates($bill); + + $data = [ 'id' => (int)$bill->id, 'name' => $bill->name, 'match' => explode(',', $bill->match), @@ -107,13 +111,13 @@ class BillTransformer extends TransformerAbstract 'automatch' => intval($bill->automatch) === 1, 'active' => intval($bill->active) === 1, 'attachments_count' => $bill->attachments()->count(), - 'pay_dates' => $this->payDates($bill), + 'pay_dates' => $payDates, 'paid_dates' => $paidData['paid_dates'], 'next_expected_match' => $paidData['next_expected_match'], 'links' => [ [ 'rel' => 'self', - 'uri' => '/bills/' . $bill->id, + 'uri' => '/bill/' . $bill->id, ], ], ]; @@ -217,6 +221,9 @@ class BillTransformer extends TransformerAbstract */ protected function payDates(Bill $bill): array { + if (is_null($this->start) || is_null($this->end)) { + return []; + } $set = new Collection; $currentStart = clone $this->start; while ($currentStart <= $this->end) { diff --git a/app/Transformers/NoteTransformer.php b/app/Transformers/NoteTransformer.php index e46a13d710..c4d9694bc8 100644 --- a/app/Transformers/NoteTransformer.php +++ b/app/Transformers/NoteTransformer.php @@ -48,6 +48,13 @@ class NoteTransformer extends TransformerAbstract 'title' => $note->title, 'text' => $note->text, 'markdown' => $converter->convertToHtml($note->text), + 'links' => [ + [ + 'rel' => 'self', + 'uri' => '/note/' . $note->id, + ], + ] + ]; } diff --git a/routes/api.php b/routes/api.php index eda5bd4676..fa9436aa93 100644 --- a/routes/api.php +++ b/routes/api.php @@ -20,7 +20,7 @@ */ Route::group( - ['middleware' => 'auth:api', 'namespace' => 'FireflyIII\Api\V1\Controllers', 'prefix' => 'bill', 'as' => 'api.v1.bills.'], function () { + ['middleware' => ['auth:api','bindings'], 'namespace' => 'FireflyIII\Api\V1\Controllers', 'prefix' => 'bill', 'as' => 'api.v1.bills.'], function () { // Bills API routes: Route::get('', ['uses' => 'BillController@index', 'as' => 'index']);