diff --git a/src/Auth/UserTags.php b/src/Auth/UserTags.php index 61f34cd36b..82eb76246f 100644 --- a/src/Auth/UserTags.php +++ b/src/Auth/UserTags.php @@ -352,7 +352,7 @@ public function forgotPasswordForm() $params['error_redirect'] = $this->parseRedirect($errorRedirect); } - if ($resetUrl = $this->params->get('reset_url')) { + if ($resetUrl = $this->getPasswordResetUrl($this->params->get('reset_url'))) { $params['reset_url'] = $resetUrl; } @@ -374,6 +374,19 @@ public function forgotPasswordForm() return $html; } + private function getPasswordResetUrl(?string $url = null): ?string + { + if (! $url) { + return null; + } + + if (! URL::isAbsolute($url) && ! str_starts_with($url, '/')) { + $url = '/'.$url; + } + + return encrypt($url); + } + /** * Output a reset password form. * diff --git a/src/Http/Controllers/ForgotPasswordController.php b/src/Http/Controllers/ForgotPasswordController.php index d9e423cbfc..752ca56e58 100644 --- a/src/Http/Controllers/ForgotPasswordController.php +++ b/src/Http/Controllers/ForgotPasswordController.php @@ -2,12 +2,12 @@ namespace Statamic\Http\Controllers; +use Illuminate\Contracts\Encryption\DecryptException; use Illuminate\Http\Request; use Illuminate\Support\Facades\Password; use Inertia\Inertia; use Statamic\Auth\Passwords\PasswordReset; use Statamic\Auth\SendsPasswordResetEmails; -use Statamic\Exceptions\ValidationException; use Statamic\Facades\URL; use Statamic\Http\Middleware\RedirectIfAuthenticated; @@ -32,17 +32,40 @@ public function showLinkRequestForm() public function sendResetLinkEmail(Request $request) { - if ($url = $request->_reset_url) { - throw_if(URL::isExternalToApplication($url), ValidationException::withMessages([ - '_reset_url' => trans('validation.url', ['attribute' => '_reset_url']), - ])); - + if ($url = $this->getResetFormUrl($request)) { PasswordReset::resetFormUrl(URL::makeAbsolute($url)); } return $this->traitSendResetLinkEmail($request); } + private function getResetFormUrl(Request $request): ?string + { + if (! $url = $request->_reset_url) { + return null; + } + + if (strlen($url) > 2048) { + return null; + } + + try { + $url = decrypt($url); + } catch (DecryptException $e) { + if (! str_starts_with($url, '/') || str_starts_with($url, '//')) { + return null; + } + + if (preg_match('/[\x00-\x1F\x7F]/', $url)) { + return null; + } + + return $url; + } + + return URL::isExternalToApplication($url) ? null : $url; + } + public function broker() { $broker = config('statamic.users.passwords.'.PasswordReset::BROKER_RESETS); diff --git a/tests/Auth/ForgotPasswordTest.php b/tests/Auth/ForgotPasswordTest.php index 0a63d5580f..339351f728 100644 --- a/tests/Auth/ForgotPasswordTest.php +++ b/tests/Auth/ForgotPasswordTest.php @@ -5,6 +5,7 @@ use Illuminate\Support\Facades\Password; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use Statamic\Auth\Passwords\PasswordReset; use Statamic\Facades\User; use Tests\Facades\Concerns\ProvidesExternalUrls; use Tests\PreventSavingStacheItemsToDisk; @@ -26,6 +27,8 @@ public function setUp(): void { parent::setUp(); + PasswordReset::resetFormUrl(null); + $this->setSites([ 'a' => ['name' => 'A', 'locale' => 'en_US', 'url' => 'http://this-site.com/'], 'b' => ['name' => 'B', 'locale' => 'en_US', 'url' => 'http://subdomain.this-site.com/'], @@ -34,28 +37,132 @@ public function setUp(): void } #[Test] - #[DataProvider('externalUrlProvider')] - public function it_validates_reset_url_when_sending_reset_link_email($url, $isExternal) + public function it_accepts_encrypted_reset_url_when_sending_reset_link_email() { $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); - User::make() - ->email('san@holo.com') - ->password('chewy') - ->save(); + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => encrypt('http://this-site.com/some-path'), + ])->assertSessionHasNoErrors(); + $this->assertEquals('http://this-site.com/some-path?token=test-token', PasswordReset::url('test-token', 'resets')); + } - $response = $this->post('/!/auth/password/email', [ + #[Test] + public function it_accepts_unencrypted_relative_reset_url_when_sending_reset_link_email() + { + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => '/some-path', + ])->assertSessionHasNoErrors(); + $this->assertEquals('http://absolute-url-resolved-from-request.com/some-path?token=test-token', PasswordReset::url('test-token', 'resets')); + } + + #[Test] + #[DataProvider('externalResetUrlProvider')] + public function it_rejects_unencrypted_external_reset_url_when_sending_reset_link_email($url) + { + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ 'email' => 'san@holo.com', '_reset_url' => $url, - ]); + ])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url. + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } + + #[Test] + public function it_rejects_unencrypted_absolute_internal_reset_url_when_sending_reset_link_email() + { + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => 'http://this-site.com/some-path', + ])->assertSessionHasNoErrors(); + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } - if ($isExternal) { - $response->assertSessionHasErrors(['_reset_url']); + #[Test] + public function it_rejects_unencrypted_relative_reset_url_with_control_characters_when_sending_reset_link_email() + { + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); - return; - } + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => "/some-path\r\nLocation: https://evil.com", + ])->assertSessionHasNoErrors(); + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } - $response->assertSessionHasNoErrors(); + #[Test] + public function it_rejects_reset_url_longer_than_2048_characters_when_sending_reset_link_email() + { + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => '/'.str_repeat('a', 2048), + ])->assertSessionHasNoErrors(); + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } + + #[Test] + public function it_rejects_unencrypted_string_reset_url_when_sending_reset_link_email() + { + // Unencrypted string that doesn't look like a URL is probably a tampered encrypted string. + // It might be a relative url without a leading slash, but we won't treat it as that. + + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => 'not-an-encrypted-string', + ])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url. + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } + + #[Test] + #[DataProvider('externalResetUrlProvider')] + public function it_rejects_encrypted_external_reset_url_when_sending_reset_link_email($url) + { + // It's weird to point to an external URL, even if you encrypt it yourself. + // This is an additional safeguard. + + $this->simulateSuccessfulPasswordResetEmail(); + $this->createUser(); + + $this->post('/!/auth/password/email', [ + 'email' => 'san@holo.com', + '_reset_url' => encrypt($url), + ])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url. + $this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets')); + } + + public static function externalResetUrlProvider() + { + $keyFn = function ($key) { + return is_null($key) ? 'null' : $key; + }; + + return collect(static::externalUrls())->mapWithKeys(fn ($url) => [$keyFn($url) => [$url]])->all(); + } + + private function createUser(): void + { + User::make() + ->email('san@holo.com') + ->password('chewy') + ->save(); } protected function simulateSuccessfulPasswordResetEmail() diff --git a/tests/Facades/Concerns/ProvidesExternalUrls.php b/tests/Facades/Concerns/ProvidesExternalUrls.php index 731b22d637..633a7c4c49 100644 --- a/tests/Facades/Concerns/ProvidesExternalUrls.php +++ b/tests/Facades/Concerns/ProvidesExternalUrls.php @@ -4,84 +4,102 @@ trait ProvidesExternalUrls { - public static function externalUrlProvider() + private static function internalUrls() { return [ - ['http://this-site.com', false], - ['http://this-site.com?foo', false], - ['http://this-site.com#anchor', false], - ['http://this-site.com/', false], - ['http://this-site.com/?foo', false], - ['http://this-site.com/#anchor', false], + 'http://this-site.com', + 'http://this-site.com?foo', + 'http://this-site.com#anchor', + 'http://this-site.com/', + 'http://this-site.com/?foo', + 'http://this-site.com/#anchor', - ['http://that-site.com', true], - ['http://that-site.com/', true], - ['http://that-site.com/?foo', true], - ['http://that-site.com/#anchor', true], - ['http://that-site.com/some-slug', true], - ['http://that-site.com/some-slug?foo', true], - ['http://that-site.com/some-slug#anchor', true], + 'http://subdomain.this-site.com', + 'http://subdomain.this-site.com/', + 'http://subdomain.this-site.com/?foo', + 'http://subdomain.this-site.com/#anchor', + 'http://subdomain.this-site.com/some-slug', + 'http://subdomain.this-site.com/some-slug?foo', + 'http://subdomain.this-site.com/some-slug#anchor', - ['http://subdomain.this-site.com', false], - ['http://subdomain.this-site.com/', false], - ['http://subdomain.this-site.com/?foo', false], - ['http://subdomain.this-site.com/#anchor', false], - ['http://subdomain.this-site.com/some-slug', false], - ['http://subdomain.this-site.com/some-slug?foo', false], - ['http://subdomain.this-site.com/some-slug#anchor', false], + 'http://absolute-url-resolved-from-request.com', + 'http://absolute-url-resolved-from-request.com/', + 'http://absolute-url-resolved-from-request.com/?foo', + 'http://absolute-url-resolved-from-request.com/?anchor', + 'http://absolute-url-resolved-from-request.com/some-slug', + 'http://absolute-url-resolved-from-request.com/some-slug?foo', + 'http://absolute-url-resolved-from-request.com/some-slug#anchor', - ['http://absolute-url-resolved-from-request.com', false], - ['http://absolute-url-resolved-from-request.com/', false], - ['http://absolute-url-resolved-from-request.com/?foo', false], - ['http://absolute-url-resolved-from-request.com/?anchor', false], - ['http://absolute-url-resolved-from-request.com/some-slug', false], - ['http://absolute-url-resolved-from-request.com/some-slug?foo', false], - ['http://absolute-url-resolved-from-request.com/some-slug#anchor', false], - ['/', false], - ['/?foo', false], - ['/#anchor', false], - ['/some-slug', false], - ['?foo', false], - ['#anchor', false], - ['', false], - [null, false], + '/', + '/?foo', + '/#anchor', + '/some-slug', + '?foo', + '#anchor', + '', + null, + ]; + } + + private static function externalUrls() + { + return [ + 'http://that-site.com', + 'http://that-site.com/', + 'http://that-site.com/?foo', + 'http://that-site.com/#anchor', + 'http://that-site.com/some-slug', + 'http://that-site.com/some-slug?foo', + 'http://that-site.com/some-slug#anchor', // Protocol-relative URLs are external - ['//evil.com', true], - ['//evil.com/', true], - ['//evil.com/path', true], - ['//this-site.com', true], + '//evil.com', + '//evil.com/', + '//evil.com/path', + '//this-site.com', // External domain that starts with a valid domain. - ['http://this-site.com.au', true], - ['http://this-site.com.au/', true], - ['http://this-site.com.au/?foo', true], - ['http://this-site.com.au/#anchor', true], - ['http://this-site.com.au/some-slug', true], - ['http://this-site.com.au/some-slug?foo', true], - ['http://this-site.com.au/some-slug#anchor', true], - ['http://subdomain.this-site.com.au', true], - ['http://subdomain.this-site.com.au/', true], - ['http://subdomain.this-site.com.au/?foo', true], - ['http://subdomain.this-site.com.au/#anchor', true], - ['http://subdomain.this-site.com.au/some-slug', true], - ['http://subdomain.this-site.com.au/some-slug?foo', true], - ['http://subdomain.this-site.com.au/some-slug#anchor', true], + 'http://this-site.com.au', + 'http://this-site.com.au/', + 'http://this-site.com.au/?foo', + 'http://this-site.com.au/#anchor', + 'http://this-site.com.au/some-slug', + 'http://this-site.com.au/some-slug?foo', + 'http://this-site.com.au/some-slug#anchor', + 'http://subdomain.this-site.com.au', + 'http://subdomain.this-site.com.au/', + 'http://subdomain.this-site.com.au/?foo', + 'http://subdomain.this-site.com.au/#anchor', + 'http://subdomain.this-site.com.au/some-slug', + 'http://subdomain.this-site.com.au/some-slug?foo', + 'http://subdomain.this-site.com.au/some-slug#anchor', // Credential injection - ['http://this-site.com@evil.com', true], - ['http://this-site.com@evil.com/', true], - ['http://this-site.com@evil.com/path', true], - ['http://this-site.com@evil.com/path?query', true], - ['http://this-site.com:password@evil.com', true], - ['http://user:pass@evil.com', true], - ['http://absolute-url-resolved-from-request.com@evil.com', true], - ['http://absolute-url-resolved-from-request.com@evil.com/path', true], - ['http://subdomain.this-site.com@evil.com', true], - ['http://subdomain.this-site.com@evil.com/path', true], - ['http://this-site.com:8000@evil.com', true], - ['http://this-site.com:8000@evil.com/path', true], - ['http://this-site.com:8000@webhook.site/token', true], + 'http://this-site.com@evil.com', + 'http://this-site.com@evil.com/', + 'http://this-site.com@evil.com/path', + 'http://this-site.com@evil.com/path?query', + 'http://this-site.com:password@evil.com', + 'http://user:pass@evil.com', + 'http://absolute-url-resolved-from-request.com@evil.com', + 'http://absolute-url-resolved-from-request.com@evil.com/path', + 'http://subdomain.this-site.com@evil.com', + 'http://subdomain.this-site.com@evil.com/path', + 'http://this-site.com:8000@evil.com', + 'http://this-site.com:8000@evil.com/path', + 'http://this-site.com:8000@webhook.site/token', + ]; + } + + public static function externalUrlProvider() + { + $keyFn = function ($key) { + return is_null($key) ? 'null' : $key; + }; + + return [ + ...collect(static::internalUrls())->mapWithKeys(fn ($url) => [$keyFn($url) => [$url, false]])->all(), + ...collect(static::externalUrls())->mapWithKeys(fn ($url) => [$keyFn($url) => [$url, true]])->all(), ]; } } diff --git a/tests/Tags/User/ForgotPasswordFormTest.php b/tests/Tags/User/ForgotPasswordFormTest.php index 04cc9eaf92..f2c7e1f282 100644 --- a/tests/Tags/User/ForgotPasswordFormTest.php +++ b/tests/Tags/User/ForgotPasswordFormTest.php @@ -3,6 +3,7 @@ namespace Tests\Tags\User; use Illuminate\Support\Facades\Password; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Statamic\Facades\Parse; use Statamic\Facades\User; @@ -32,12 +33,39 @@ public function it_renders_form() #[Test] public function it_renders_form_with_params() { - $output = $this->tag('{{ user:forgot_password_form redirect="/submitted" error_redirect="/errors" reset_url="/resetting" class="form" id="form" }}{{ /user:forgot_password_form }}'); + $output = $this->tag('{{ user:forgot_password_form redirect="/submitted" error_redirect="/errors" class="form" id="form" }}{{ /user:forgot_password_form }}'); $this->assertStringStartsWith('