mirror of
https://github.com/Part-DB/Part-DB-server.git
synced 2026-07-27 03:31:35 +00:00
Ensure that the Password reset link is generated on a trusted domain, and cannot be manipulated by an attacker by setting the host header
This commit is contained in:
parent
3f7dd59dd1
commit
65631dde4c
4 changed files with 278 additions and 3 deletions
96
src/Services/System/TrustedUrlGenerator.php
Normal file
96
src/Services/System/TrustedUrlGenerator.php
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
<?php
|
||||
/*
|
||||
* This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
|
||||
*
|
||||
* Copyright (C) 2019 - 2024 Jan Böhmer (https://github.com/jbtronics)
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as published
|
||||
* by the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Services\System;
|
||||
|
||||
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
|
||||
|
||||
/**
|
||||
* Generates absolute URLs for routes whose host can be trusted, for use in contexts where the host must not
|
||||
* be attacker-controllable (e.g. links sent out via email).
|
||||
*
|
||||
* If no TRUSTED_HOSTS is configured, Symfony does not validate the Host header of the incoming request in
|
||||
* any way, so it must not be trusted to generate such links (otherwise an attacker could poison them via a
|
||||
* forged Host header). In that case, the host/scheme/base path are forced to the ones configured via
|
||||
* DEFAULT_URI instead of the ones from the current HTTP request.
|
||||
* If TRUSTED_HOSTS is configured, Symfony already rejects requests with a non-matching Host header, so the
|
||||
* request's host can be trusted and is used as usual (which allows the app to be reachable under multiple
|
||||
* trusted hostnames).
|
||||
*/
|
||||
final class TrustedUrlGenerator
|
||||
{
|
||||
public function __construct(
|
||||
private readonly UrlGeneratorInterface $urlGenerator,
|
||||
private readonly TrustedHostsChecker $trustedHostsChecker,
|
||||
#[Autowire('%partdb.default_uri%')]
|
||||
private readonly string $defaultUri,
|
||||
) {
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed> $parameters
|
||||
*/
|
||||
public function generate(string $route, array $parameters = []): string
|
||||
{
|
||||
//If TRUSTED_HOSTS is configured, Symfony already validated the request's Host header, so it can be trusted
|
||||
if (!$this->trustedHostsChecker->isTrustedHostsUnconfigured()) {
|
||||
return $this->urlGenerator->generate($route, $parameters, UrlGeneratorInterface::ABSOLUTE_URL);
|
||||
}
|
||||
|
||||
$parts = parse_url(rtrim($this->defaultUri, '/'));
|
||||
|
||||
$context = $this->urlGenerator->getContext();
|
||||
|
||||
//Backup the original context, so we can restore it after generating the URL
|
||||
$original = [
|
||||
'host' => $context->getHost(),
|
||||
'scheme' => $context->getScheme(),
|
||||
'httpPort' => $context->getHttpPort(),
|
||||
'httpsPort' => $context->getHttpsPort(),
|
||||
'baseUrl' => $context->getBaseUrl(),
|
||||
];
|
||||
|
||||
$scheme = $parts['scheme'] ?? 'https';
|
||||
$context->setScheme($scheme);
|
||||
$context->setHost($parts['host'] ?? '');
|
||||
if (isset($parts['port'])) {
|
||||
if ($scheme === 'https') {
|
||||
$context->setHttpsPort($parts['port']);
|
||||
} else {
|
||||
$context->setHttpPort($parts['port']);
|
||||
}
|
||||
}
|
||||
$context->setBaseUrl($parts['path'] ?? '');
|
||||
|
||||
try {
|
||||
return $this->urlGenerator->generate($route, $parameters, UrlGeneratorInterface::ABSOLUTE_URL);
|
||||
} finally {
|
||||
//Restore the original context, so the rest of the request is not affected
|
||||
$context->setHost($original['host']);
|
||||
$context->setScheme($original['scheme']);
|
||||
$context->setHttpPort($original['httpPort']);
|
||||
$context->setHttpsPort($original['httpsPort']);
|
||||
$context->setBaseUrl($original['baseUrl']);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -23,6 +23,7 @@ declare(strict_types=1);
|
|||
namespace App\Services\UserSystem;
|
||||
|
||||
use App\Entity\UserSystem\User;
|
||||
use App\Services\System\TrustedUrlGenerator;
|
||||
use DateTime;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Bridge\Twig\Mime\TemplatedEmail;
|
||||
|
|
@ -39,7 +40,8 @@ class PasswordResetManager
|
|||
|
||||
public function __construct(protected MailerInterface $mailer, protected EntityManagerInterface $em,
|
||||
protected TranslatorInterface $translator, protected UserPasswordHasherInterface $userPasswordEncoder,
|
||||
PasswordHasherFactoryInterface $encoderFactory)
|
||||
PasswordHasherFactoryInterface $encoderFactory,
|
||||
protected TrustedUrlGenerator $trustedUrlGenerator)
|
||||
{
|
||||
$this->passwordEncoder = $encoderFactory->getPasswordHasher(User::class);
|
||||
}
|
||||
|
|
@ -63,6 +65,9 @@ class PasswordResetManager
|
|||
$user->setPwResetExpires($expiration_date);
|
||||
|
||||
if ($user->getEmail() !== null && $user->getEmail() !== '') {
|
||||
$reset_url = $this->trustedUrlGenerator->generate('pw_reset_new_pw', ['user' => $user->getName(), 'token' => $unencrypted_token]);
|
||||
$reset_url_fallback = $this->trustedUrlGenerator->generate('pw_reset_new_pw');
|
||||
|
||||
$address = new Address($user->getEmail(), $user->getFullName());
|
||||
$mail = new TemplatedEmail();
|
||||
$mail->to($address);
|
||||
|
|
@ -72,6 +77,8 @@ class PasswordResetManager
|
|||
'expiration_date' => $expiration_date,
|
||||
'token' => $unencrypted_token,
|
||||
'user' => $user,
|
||||
'reset_url' => $reset_url,
|
||||
'reset_url_fallback' => $reset_url_fallback,
|
||||
]);
|
||||
|
||||
//Send email
|
||||
|
|
|
|||
|
|
@ -6,9 +6,9 @@
|
|||
<h4>{% trans with {'%name%': user.fullName|escape } %}email.hi %name%{% endtrans %},</h4>
|
||||
{% trans %}email.pw_reset.message{% endtrans %}
|
||||
<br>
|
||||
<button class="large expand" href="{{ url('pw_reset_new_pw', {user: user.name, token: token}) }}">{% trans %}email.pw_reset.button{% endtrans %}</button>
|
||||
<button class="large expand" href="{{ reset_url }}">{% trans %}email.pw_reset.button{% endtrans %}</button>
|
||||
<br>
|
||||
{% trans with {'%url%': url('pw_reset_new_pw') } %}email.pw_reset.fallback{% endtrans %}:
|
||||
{% trans with {'%url%': reset_url_fallback } %}email.pw_reset.fallback{% endtrans %}:
|
||||
<callout class="secondary">
|
||||
<row>
|
||||
<columns>
|
||||
|
|
|
|||
172
tests/Services/System/TrustedUrlGeneratorTest.php
Normal file
172
tests/Services/System/TrustedUrlGeneratorTest.php
Normal file
|
|
@ -0,0 +1,172 @@
|
|||
<?php
|
||||
/*
|
||||
* This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
|
||||
*
|
||||
* Copyright (C) 2019 - 2026 Jan Böhmer (https://github.com/jbtronics)
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as published
|
||||
* by the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Services\System;
|
||||
|
||||
use App\Services\System\TrustedHostsChecker;
|
||||
use App\Services\System\TrustedUrlGenerator;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
|
||||
use Symfony\Component\Routing\RequestContext;
|
||||
|
||||
final class TrustedUrlGeneratorTest extends TestCase
|
||||
{
|
||||
private function createGenerator(UrlGeneratorInterface $urlGenerator, string $trustedHosts, string $defaultUri): TrustedUrlGenerator
|
||||
{
|
||||
return new TrustedUrlGenerator($urlGenerator, new TrustedHostsChecker($trustedHosts), $defaultUri);
|
||||
}
|
||||
|
||||
public function testUsesCurrentRequestContextWhenTrustedHostsIsConfigured(): void
|
||||
{
|
||||
//If TRUSTED_HOSTS is configured, the request's host is already validated by Symfony, so it should be used as-is
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->expects($this->never())->method('getContext');
|
||||
$urlGenerator->expects($this->once())
|
||||
->method('generate')
|
||||
->with('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc'], UrlGeneratorInterface::ABSOLUTE_URL)
|
||||
->willReturn('https://request-host.example.com/pw_reset/new_pw/john/abc');
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, 'example\.com', 'https://trusted.example.com/');
|
||||
|
||||
$url = $generator->generate('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc']);
|
||||
$this->assertSame('https://request-host.example.com/pw_reset/new_pw/john/abc', $url);
|
||||
}
|
||||
|
||||
public function testForcesDefaultUriHostWhenTrustedHostsIsNotConfigured(): void
|
||||
{
|
||||
//If TRUSTED_HOSTS is not configured, the request's Host header is attacker-controllable, so the
|
||||
//host/scheme/base path configured via DEFAULT_URI must be used instead.
|
||||
$context = new RequestContext();
|
||||
$context->setScheme('http');
|
||||
$context->setHost('attacker.evil');
|
||||
$context->setBaseUrl('');
|
||||
|
||||
$capturedHost = null;
|
||||
$capturedScheme = null;
|
||||
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->method('getContext')->willReturn($context);
|
||||
$urlGenerator->expects($this->once())
|
||||
->method('generate')
|
||||
->with('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc'], UrlGeneratorInterface::ABSOLUTE_URL)
|
||||
->willReturnCallback(function () use ($context, &$capturedHost, &$capturedScheme) {
|
||||
//Capture the context state as it is seen by the URL generator during the call
|
||||
$capturedHost = $context->getHost();
|
||||
$capturedScheme = $context->getScheme();
|
||||
|
||||
return $capturedScheme.'://'.$capturedHost.'/pw_reset/new_pw/john/abc';
|
||||
});
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/');
|
||||
|
||||
$url = $generator->generate('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc']);
|
||||
|
||||
$this->assertSame('trusted.example.com', $capturedHost);
|
||||
$this->assertSame('https', $capturedScheme);
|
||||
$this->assertSame('https://trusted.example.com/pw_reset/new_pw/john/abc', $url);
|
||||
}
|
||||
|
||||
public function testRestoresOriginalContextAfterGenerating(): void
|
||||
{
|
||||
//The context is shared with the rest of the application, so it must not be left modified afterward
|
||||
$context = new RequestContext();
|
||||
$context->setScheme('http');
|
||||
$context->setHost('attacker.evil');
|
||||
$context->setHttpPort(8080);
|
||||
$context->setBaseUrl('/original');
|
||||
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->method('getContext')->willReturn($context);
|
||||
$urlGenerator->method('generate')->willReturn('https://trusted.example.com/foo');
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/sub/');
|
||||
$generator->generate('some_route');
|
||||
|
||||
$this->assertSame('attacker.evil', $context->getHost());
|
||||
$this->assertSame('http', $context->getScheme());
|
||||
$this->assertSame(8080, $context->getHttpPort());
|
||||
$this->assertSame('/original', $context->getBaseUrl());
|
||||
}
|
||||
|
||||
public function testRestoresOriginalContextEvenWhenGenerateThrows(): void
|
||||
{
|
||||
$context = new RequestContext();
|
||||
$context->setScheme('http');
|
||||
$context->setHost('attacker.evil');
|
||||
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->method('getContext')->willReturn($context);
|
||||
$urlGenerator->method('generate')->willThrowException(new \RuntimeException('route not found'));
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/');
|
||||
|
||||
try {
|
||||
$generator->generate('unknown_route');
|
||||
$this->fail('Expected exception was not thrown');
|
||||
} catch (\RuntimeException) {
|
||||
//Expected
|
||||
}
|
||||
|
||||
$this->assertSame('attacker.evil', $context->getHost());
|
||||
$this->assertSame('http', $context->getScheme());
|
||||
}
|
||||
|
||||
public function testUsesHttpsPortForHttpsDefaultUri(): void
|
||||
{
|
||||
$context = new RequestContext();
|
||||
|
||||
$capturedHttpsPort = null;
|
||||
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->method('getContext')->willReturn($context);
|
||||
$urlGenerator->method('generate')->willReturnCallback(function () use ($context, &$capturedHttpsPort) {
|
||||
$capturedHttpsPort = $context->getHttpsPort();
|
||||
|
||||
return 'https://trusted.example.com:8443/foo';
|
||||
});
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com:8443/');
|
||||
$generator->generate('some_route');
|
||||
|
||||
$this->assertSame(8443, $capturedHttpsPort);
|
||||
}
|
||||
|
||||
public function testUsesBasePathFromDefaultUri(): void
|
||||
{
|
||||
$context = new RequestContext();
|
||||
|
||||
$capturedBaseUrl = null;
|
||||
|
||||
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
|
||||
$urlGenerator->method('getContext')->willReturn($context);
|
||||
$urlGenerator->method('generate')->willReturnCallback(function () use ($context, &$capturedBaseUrl) {
|
||||
$capturedBaseUrl = $context->getBaseUrl();
|
||||
|
||||
return 'https://trusted.example.com/partdb/foo';
|
||||
});
|
||||
|
||||
$generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/partdb/');
|
||||
$generator->generate('some_route');
|
||||
|
||||
$this->assertSame('/partdb', $capturedBaseUrl);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue