diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 5319eeee..e7d5117d 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -146,7 +146,13 @@ public function handleProviderCallback(Request $request, $provider = 'sso') public function showLoginForm(Request $request) { if ($request->has('intended')) { - Session::put('url.intended', $request->input('intended')); + $intended = $request->input('intended'); + + if (is_string($intended) && $this->isSafeIntendedUrl($request, $intended)) { + Session::put('url.intended', $intended); + } else { + Session::forget('url.intended'); + } } if (SsoProvider::isForced()) { @@ -157,4 +163,30 @@ public function showLoginForm(Request $request) 'hasSsoLoginAvailable' => SsoProvider::isEnabled(), ]); } + + private function isSafeIntendedUrl(Request $request, string $intended): bool + { + if (str_starts_with($intended, '/')) { + return ! str_starts_with($intended, '//') + && ! str_starts_with($intended, '/\\'); + } + + $parts = parse_url($intended); + + if (! is_array($parts) || ! isset($parts['scheme'], $parts['host'])) { + return false; + } + + $scheme = strtolower($parts['scheme']); + + if (! in_array($scheme, ['http', 'https'], true)) { + return false; + } + + $port = $parts['port'] ?? ($scheme === 'https' ? 443 : 80); + + return $scheme === $request->getScheme() + && strcasecmp($parts['host'], $request->getHost()) === 0 + && $port === $request->getPort(); + } } diff --git a/tests/Feature/Auth/LoginTest.php b/tests/Feature/Auth/LoginTest.php index 2c05b2b1..9534acf8 100644 --- a/tests/Feature/Auth/LoginTest.php +++ b/tests/Feature/Auth/LoginTest.php @@ -31,6 +31,48 @@ $response->assertStatus(302); }); +test('login rejects an external intended URL', function () { + $user = createUser(); + + $this->get(route('login', ['intended' => 'https://attacker.example/landing'])) + ->assertSessionMissing('url.intended'); + + $this->post(route('login'), [ + 'email' => $user->email, + 'password' => 'password', + ])->assertRedirect(route('home')); +}); + +test('login accepts a local intended path', function () { + $user = createUser(); + + $this->get(route('login', ['intended' => '/items/example'])) + ->assertSessionHas('url.intended', '/items/example'); + + $this->post(route('login'), [ + 'email' => $user->email, + 'password' => 'password', + ])->assertRedirect('/items/example'); +}); + +test('login accepts a same-origin intended URL', function () { + $user = createUser(); + $intended = route('items.show', 'example'); + + $this->get(route('login', ['intended' => $intended])) + ->assertSessionHas('url.intended', $intended); + + $this->post(route('login'), [ + 'email' => $user->email, + 'password' => 'password', + ])->assertRedirect($intended); +}); + +test('login rejects a protocol-relative intended URL', function () { + $this->get(route('login', ['intended' => '//attacker.example/landing'])) + ->assertSessionMissing('url.intended'); +}); + test('users cannot authenticate with an incorrect password', function () { $user = createUser();