From 492b1925fb1ef9c63d9aa759bbd11c0a29d50831 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 06:15:16 +0200 Subject: [PATCH] Land a sign-in where it was performed, not always in the customer portal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fortify sends every successful sign-in to config('fortify.home') — /dashboard. That is why an operator signing in ended up in the customer portal, and once the console has a hostname to itself it stops being merely confusing: /dashboard does not exist on that host, so signing in would land on a 404. Three outcomes now, decided by where the sign-in happened and who signed in: the console for an operator on the console, the portal everywhere else, and for a non-operator on the console the session is taken away again at the moment it was created — a guard that merely refuses each page afterwards leaves that session in the browser. Four things the review caught, each of which would have left a hole: - Fortify never reaches LoginResponse when a two-factor challenge was involved. Binding only that one left exactly the accounts most likely to be operators on the old behaviour, so both exits are bound and share one decision. - The JSON branch ran before the authorization check, so a JSON client kept a session a browser would have lost. - The two exits do not share a JSON success contract — an ordinary sign-in answers {"two_factor":false}, a completed challenge answers an empty 204 — and merging them breaks a client keying off the status code. - In shared mode everyone posts to /login, so the request never looks like the console even when /admin is the destination. The intended URL is read too. Co-Authored-By: Claude Opus 5 --- .../Responses/ConsoleAwareLoginResponse.php | 21 ++++ .../ConsoleAwareTwoFactorLoginResponse.php | 25 ++++ app/Http/Responses/LandsWhereSignedIn.php | 62 ++++++++++ app/Providers/FortifyServiceProvider.php | 15 +++ app/Support/AdminArea.php | 26 ++++ lang/de/auth.php | 1 + lang/en/auth.php | 1 + .../ConsoleLoginLandsInTheConsoleTest.php | 113 ++++++++++++++++++ 8 files changed, 264 insertions(+) create mode 100644 app/Http/Responses/ConsoleAwareLoginResponse.php create mode 100644 app/Http/Responses/ConsoleAwareTwoFactorLoginResponse.php create mode 100644 app/Http/Responses/LandsWhereSignedIn.php create mode 100644 tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php 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(); +});