diff --git a/app/Http/Responses/ConsoleAwareLoginResponse.php b/app/Http/Responses/ConsoleAwareLoginResponse.php new file mode 100644 index 0000000..ee15680 --- /dev/null +++ b/app/Http/Responses/ConsoleAwareLoginResponse.php @@ -0,0 +1,21 @@ +landing($request, new JsonResponse(['two_factor' => false], 200)); + } +} diff --git a/app/Http/Responses/ConsoleAwareTwoFactorLoginResponse.php b/app/Http/Responses/ConsoleAwareTwoFactorLoginResponse.php new file mode 100644 index 0000000..a857dba --- /dev/null +++ b/app/Http/Responses/ConsoleAwareTwoFactorLoginResponse.php @@ -0,0 +1,25 @@ +landing($request, new JsonResponse('', 204)); + } +} diff --git a/app/Http/Responses/LandsWhereSignedIn.php b/app/Http/Responses/LandsWhereSignedIn.php new file mode 100644 index 0000000..1d10e73 --- /dev/null +++ b/app/Http/Responses/LandsWhereSignedIn.php @@ -0,0 +1,62 @@ +session()?->get('url.intended')); + + // Authorization first, before any branch. Answering JSON early let a + // non-operator keep the session on the console host that a browser + // request would have had taken away. + if ($onConsole && ! $request->user()?->can('console.view')) { + // Taken away at the moment it was created: the session exists as + // soon as Fortify authenticates, and a guard that merely refuses + // each page afterwards leaves it sitting in the browser. + Auth::guard('web')->logout(); + $request->session()->invalidate(); + $request->session()->regenerateToken(); + + if ($request->wantsJson()) { + return new JsonResponse(['message' => __('auth.not_an_operator')], 403); + } + + return redirect()->to(AdminArea::home())->withErrors([ + Fortify::username() => __('auth.not_an_operator'), + ]); + } + + // Fortify answers JSON to a JSON client. Replacing that with a redirect + // would break every caller that is not a browser. + if ($request->wantsJson()) { + // The caller supplies it: Fortify answers {"two_factor":false} to an + // ordinary sign-in and an empty 204 after a challenge, and a client + // keying off the status code breaks if the two are merged. + return $success; + } + + return redirect()->intended($onConsole ? AdminArea::home() : Fortify::redirects('login')); + } +} diff --git a/app/Providers/FortifyServiceProvider.php b/app/Providers/FortifyServiceProvider.php index 182bcdf..720117c 100644 --- a/app/Providers/FortifyServiceProvider.php +++ b/app/Providers/FortifyServiceProvider.php @@ -29,6 +29,21 @@ class FortifyServiceProvider extends ServiceProvider */ public function boot(): void { + // Sign-in lands where it was performed: the console on the console's + // hostname, the portal everywhere else. Fortify's own response sends + // everyone to the portal, which on a console-only host is a 404. + // BOTH exits: Fortify never reaches LoginResponse when a two-factor + // challenge was involved, and binding only that one leaves exactly the + // accounts most likely to be operators on the old behaviour. + $this->app->singleton( + \Laravel\Fortify\Contracts\LoginResponse::class, + \App\Http\Responses\ConsoleAwareLoginResponse::class, + ); + $this->app->singleton( + \Laravel\Fortify\Contracts\TwoFactorLoginResponse::class, + \App\Http\Responses\ConsoleAwareTwoFactorLoginResponse::class, + ); + Fortify::createUsersUsing(CreateNewUser::class); Fortify::updateUserProfileInformationUsing(UpdateUserProfileInformation::class); Fortify::updateUserPasswordsUsing(UpdateUserPassword::class); diff --git a/app/Support/AdminArea.php b/app/Support/AdminArea.php index 92bc94d..3f75784 100644 --- a/app/Support/AdminArea.php +++ b/app/Support/AdminArea.php @@ -101,6 +101,32 @@ final class AdminArea : $request->is(self::FALLBACK_PREFIX, self::FALLBACK_PREFIX.'/*'); } + /** + * Does this URL lead into the console? + * + * Needed because the sign-in POST does not go to a console address in + * shared mode — everyone posts to /login — so the request alone cannot say + * whether the console is where this person was heading. The intended URL + * can, and without it a non-operator turned back at /admin keeps the + * session that was just created for them. + */ + public static function pointsAtConsole(?string $url): bool + { + if ($url === null || $url === '') { + return false; + } + + if (self::isExclusive()) { + $host = parse_url($url, PHP_URL_HOST); + + return is_string($host) && self::covers($host); + } + + $path = trim((string) parse_url($url, PHP_URL_PATH), '/'); + + return $path === self::FALLBACK_PREFIX || str_starts_with($path, self::FALLBACK_PREFIX.'/'); + } + /** Is this hostname one the console answers on? */ public static function covers(string $host): bool { diff --git a/lang/de/auth.php b/lang/de/auth.php index 3e0e853..1885447 100644 --- a/lang/de/auth.php +++ b/lang/de/auth.php @@ -55,4 +55,5 @@ return [ 'use_otp' => 'Authenticator-Code verwenden', 'account_suspended' => 'Ihr Konto ist gesperrt. Bitte kontaktieren Sie den Support.', 'account_closed' => 'Dieses Konto wurde geschlossen.', + 'not_an_operator' => 'Dieses Konto hat keinen Zugang zur Konsole.', ]; diff --git a/lang/en/auth.php b/lang/en/auth.php index 44a6839..418e151 100644 --- a/lang/en/auth.php +++ b/lang/en/auth.php @@ -55,4 +55,5 @@ return [ 'use_otp' => 'Use authenticator code', 'account_suspended' => 'Your account is suspended. Please contact support.', 'account_closed' => 'This account has been closed.', + 'not_an_operator' => 'This account has no access to the console.', ]; diff --git a/tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php b/tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php new file mode 100644 index 0000000..6a48186 --- /dev/null +++ b/tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php @@ -0,0 +1,113 @@ +set('admin_access.hosts', ['admin.example.test']); + config()->set('admin_access.exclusive', true); +}); + +it('sends an operator signing in on the console host to the console', function () { + $operator = operator('Owner'); + + $this->post('http://admin.example.test/login', [ + 'email' => $operator->email, + 'password' => 'password', + ])->assertRedirect('/'); + + expect(auth()->check())->toBeTrue(); +}); + +it('sends a customer signing in on the portal host to the portal', function () { + $user = User::factory()->create(); + + $this->post('http://app.example.test/login', [ + 'email' => $user->email, + 'password' => 'password', + ])->assertRedirect('/dashboard'); +}); + +it('refuses to leave a customer holding a session on the console host', function () { + // Fortify authenticates before anything downstream can object, so the + // session already exists. Refusing each page afterwards would leave it in + // the browser; it is taken away at the moment it was created. + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + $user->forceFill(['password' => bcrypt('password')])->save(); + + $this->post('http://admin.example.test/login', [ + 'email' => $user->email, + 'password' => 'password', + ])->assertSessionHasErrors('email'); + + expect(auth()->check())->toBeFalse(); +}); + +it('makes the same decision after a two-factor challenge', function () { + // Fortify never reaches LoginResponse on this path. Binding only that one + // left the accounts most likely to be operators landing in the portal. + $operator = operator('Owner'); + + $response = app(Laravel\Fortify\Contracts\TwoFactorLoginResponse::class) + ->toResponse(tap(request(), function ($request) use ($operator) { + $request->headers->set('HOST', 'admin.example.test'); + $request->setUserResolver(fn () => $operator); + })); + + // The console root on the console host — emphatically not the portal. + expect($response->getTargetUrl())->toContain('admin.example.test') + ->and($response->getTargetUrl())->not->toContain('/dashboard'); +}); + +it('still answers JSON to a JSON client', function () { + // Replacing Fortify's JSON reply with a redirect breaks every caller that + // is not a browser. + $operator = operator('Owner'); + + $this->postJson('http://admin.example.test/login', [ + 'email' => $operator->email, + 'password' => 'password', + ])->assertOk()->assertJson(['two_factor' => false]); +}); + +it('does not let a JSON client keep a session a browser would lose', function () { + // Answering JSON early skipped the permission check entirely. + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + $user->forceFill(['password' => bcrypt('password')])->save(); + + $this->postJson('http://admin.example.test/login', [ + 'email' => $user->email, + 'password' => 'password', + ])->assertForbidden(); + + expect(auth()->check())->toBeFalse(); +}); + +it('removes the session of a non-operator who was heading for /admin on a shared host', function () { + // In shared mode everyone posts to /login, so the request never looks like + // the console. Without reading the intended destination, a non-operator was + // authenticated, shown a 403 — and left holding the session. + config()->set('admin_access.exclusive', false); + config()->set('admin_access.hosts', []); + + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + $user->forceFill(['password' => bcrypt('password')])->save(); + + $this->get('/admin')->assertRedirect('/login'); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertSessionHasErrors('email'); + + expect(auth()->check())->toBeFalse(); +});