From d52d8d7970054b909fccd5cdfcb6bd341d0f4b84 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 7 Mar 2018 05:51:51 +0100 Subject: [PATCH] Expand exception code and fix demo user redirect. --- app/Exceptions/FireflyException.php | 3 ++- app/Exceptions/Handler.php | 3 ++- app/Http/Middleware/AuthenticateTwoFactor.php | 4 ++-- app/Http/Middleware/IsAdmin.php | 4 ++-- app/Http/Middleware/IsDemoUser.php | 14 ++++++++------ app/Http/Middleware/IsSandStormUser.php | 2 +- app/Http/Middleware/RedirectIfAuthenticated.php | 2 +- .../RedirectIfTwoFactorAuthenticated.php | 2 +- tests/Unit/Middleware/AuthenticateTest.php | 6 ++---- tests/Unit/Middleware/IsDemoUserTest.php | 4 +--- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/Exceptions/FireflyException.php b/app/Exceptions/FireflyException.php index 538e094b06..b2c9916292 100644 --- a/app/Exceptions/FireflyException.php +++ b/app/Exceptions/FireflyException.php @@ -22,9 +22,10 @@ declare(strict_types=1); namespace FireflyIII\Exceptions; +use Exception; /** * Class FireflyException. */ -class FireflyException extends \Exception +class FireflyException extends Exception { } diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 2dc4e0320c..a024203d97 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -70,14 +70,15 @@ class Handler extends ExceptionHandler return parent::render($request, $exception); } if ($exception instanceof NotFoundHttpException && $request->expectsJson()) { + // JSON error: return response()->json(['message' => 'Resource not found', 'exception' => 'NotFoundHttpException'], 404); } if ($exception instanceof AuthenticationException && $request->expectsJson()) { + // somehow Laravel handler does not catch this: return response()->json(['message' => 'Unauthenticated', 'exception' => 'AuthenticationException'], 401); } - if ($request->expectsJson()) { $isDebug = config('app.debug', false); if ($isDebug) { diff --git a/app/Http/Middleware/AuthenticateTwoFactor.php b/app/Http/Middleware/AuthenticateTwoFactor.php index ca6879d96c..97ed9bd749 100644 --- a/app/Http/Middleware/AuthenticateTwoFactor.php +++ b/app/Http/Middleware/AuthenticateTwoFactor.php @@ -61,7 +61,7 @@ class AuthenticateTwoFactor public function handle($request, Closure $next, ...$guards) { if ($this->auth->guest()) { - return redirect()->guest('login'); + return response()->redirectTo(route('login')); } $is2faEnabled = app('preferences')->get('twoFactorAuthEnabled', false)->data; @@ -71,7 +71,7 @@ class AuthenticateTwoFactor if ($is2faEnabled && $has2faSecret && !$is2faAuthed) { Log::debug('Does not seem to be 2 factor authed, redirect.'); - return redirect(route('two-factor.index')); + return response()->redirectTo(route('two-factor.index')); } return $next($request); diff --git a/app/Http/Middleware/IsAdmin.php b/app/Http/Middleware/IsAdmin.php index e06eecc07b..539cf378fe 100644 --- a/app/Http/Middleware/IsAdmin.php +++ b/app/Http/Middleware/IsAdmin.php @@ -48,12 +48,12 @@ class IsAdmin return response('Unauthorized.', 401); } - return redirect()->guest('login'); + return response()->redirectTo(route('login')); } /** @var User $user */ $user = auth()->user(); if (!$user->hasRole('owner')) { - return redirect(route('home')); + return response()->redirectTo(route('home')); } return $next($request); diff --git a/app/Http/Middleware/IsDemoUser.php b/app/Http/Middleware/IsDemoUser.php index 7f704adc14..10f110315d 100644 --- a/app/Http/Middleware/IsDemoUser.php +++ b/app/Http/Middleware/IsDemoUser.php @@ -23,9 +23,9 @@ declare(strict_types=1); namespace FireflyIII\Http\Middleware; use Closure; +use FireflyIII\Exceptions\IsDemoUserException; use FireflyIII\User; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Auth; use Session; /** @@ -38,7 +38,6 @@ class IsDemoUser * * @param \Illuminate\Http\Request $request * @param \Closure $next - * @param string[] ...$guards * * @return mixed */ @@ -51,11 +50,14 @@ class IsDemoUser } if ($user->hasRole('demo')) { - Session::flash('info', strval(trans('firefly.not_available_demo_user'))); + $request->session()->flash('info', strval(trans('firefly.not_available_demo_user'))); + $current = $request->url(); + $previous = $request->session()->previousUrl(); + if ($current !== $previous) { + return response()->redirectTo($previous); + } - redirect($request->session()->previousUrl()); - - return $next($request); + return response()->redirectTo(route('index')); } return $next($request); diff --git a/app/Http/Middleware/IsSandStormUser.php b/app/Http/Middleware/IsSandStormUser.php index aba1be7020..4847fdef31 100644 --- a/app/Http/Middleware/IsSandStormUser.php +++ b/app/Http/Middleware/IsSandStormUser.php @@ -51,7 +51,7 @@ class IsSandStormUser if (1 === intval(getenv('SANDSTORM'))) { Session::flash('warning', strval(trans('firefly.sandstorm_not_available'))); - return redirect(route('index')); + return response()->redirectTo(route('index')); } return $next($request); diff --git a/app/Http/Middleware/RedirectIfAuthenticated.php b/app/Http/Middleware/RedirectIfAuthenticated.php index f318f14d5d..98d010590c 100644 --- a/app/Http/Middleware/RedirectIfAuthenticated.php +++ b/app/Http/Middleware/RedirectIfAuthenticated.php @@ -43,7 +43,7 @@ class RedirectIfAuthenticated public function handle($request, Closure $next, $guard = null) { if (Auth::guard($guard)->check()) { - return redirect(route('index')); + return response()->redirectTo(route('index')); } return $next($request); diff --git a/app/Http/Middleware/RedirectIfTwoFactorAuthenticated.php b/app/Http/Middleware/RedirectIfTwoFactorAuthenticated.php index 1827455932..f2b4f27e12 100644 --- a/app/Http/Middleware/RedirectIfTwoFactorAuthenticated.php +++ b/app/Http/Middleware/RedirectIfTwoFactorAuthenticated.php @@ -51,7 +51,7 @@ class RedirectIfTwoFactorAuthenticated $is2faAuthed = 'true' === $request->cookie('twoFactorAuthenticated'); if ($is2faEnabled && $has2faSecret && $is2faAuthed) { - return redirect(route('index')); + return response()->redirectTo(route('index')); } } diff --git a/tests/Unit/Middleware/AuthenticateTest.php b/tests/Unit/Middleware/AuthenticateTest.php index 39aff46071..b1022269dc 100644 --- a/tests/Unit/Middleware/AuthenticateTest.php +++ b/tests/Unit/Middleware/AuthenticateTest.php @@ -49,7 +49,6 @@ class AuthenticateTest extends TestCase public function testMiddlewareAjax() { Log::debug('Now at testMiddlewareAjax'); - //$this->withoutExceptionHandling(); $server = ['HTTP_X-Requested-With' => 'XMLHttpRequest']; $response = $this->get('/_test/authenticate', $server); $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode()); @@ -89,15 +88,14 @@ class AuthenticateTest extends TestCase public function testMiddlewareEmail() { Log::debug('Now at testMiddlewareEmail'); - //$this->withoutExceptionHandling(); $user = $this->user(); $user->blocked = 1; $user->blocked_code = 'email_changed'; $this->be($user); $response = $this->get('/_test/authenticate'); - //$this->assertEquals(Response::HTTP_FOUND, $response->getStatusCode()); + $this->assertEquals(Response::HTTP_FOUND, $response->getStatusCode()); $response->assertSessionHas('logoutMessage', strval(trans('firefly.email_changed_logout'))); - //$response->assertRedirect(route('login')); + $response->assertRedirect(route('login')); } /** diff --git a/tests/Unit/Middleware/IsDemoUserTest.php b/tests/Unit/Middleware/IsDemoUserTest.php index 40f36f4bb0..6e695cdfd9 100644 --- a/tests/Unit/Middleware/IsDemoUserTest.php +++ b/tests/Unit/Middleware/IsDemoUserTest.php @@ -39,7 +39,6 @@ class IsDemoUserTest extends TestCase */ public function testMiddlewareAuthenticated() { - $this->withoutExceptionHandling(); $this->be($this->user()); $response = $this->get('/_test/is-demo'); $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); @@ -50,7 +49,6 @@ class IsDemoUserTest extends TestCase */ public function testMiddlewareNotAuthenticated() { - $this->withoutExceptionHandling(); $response = $this->get('/_test/is-demo'); $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); } @@ -62,7 +60,7 @@ class IsDemoUserTest extends TestCase { $this->be($this->demoUser()); $response = $this->get('/_test/is-demo'); - $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); + $this->assertEquals(Response::HTTP_FOUND, $response->getStatusCode()); $response->assertSessionHas('info'); }