fix: a reset link is not built from a header the caller chose

src/Application/User/Services/UserPassRecover.php

@@ -29,6 +29,8 @@
 use SP\Application\Application;
 use SP\Domain\Core\Messages\MailMessage;
 use SP\Domain\Common\Providers\Password;
+use SP\Domain\Config\Ports\ConfigDataInterface;
+use SP\Domain\Core\Bootstrap\UriContextInterface;
 use SP\Domain\Common\Services\Service;
 use SP\Domain\Common\Services\ServiceException;
 use SP\Domain\Core\Exceptions\ConstraintException;
@@ -66,8 +68,33 @@ public function __construct(
         parent::__construct($application);
     }
 
-    public static function getMailMessage(string $hash, string $baseUri): MailMessage
-    {
+    /**
+     * The mail carrying a one-time reset link, and the address that link points at.
+     *
+     * The base URI is decided here rather than taken from the caller, because both callers used to
+     * pass `UriContextInterface::getWebUri()` — which prefers `Forwarded` / `X-Forwarded-Host`,
+     * headers supplied by whoever made the request, with no `setTrustedProxies()` anywhere to
+     * restrict them. Verified against the running instance: `X-Forwarded-Host: evil.example.com`
+     * comes straight back out of the application.
+     *
+     * `saveRequestAction()` needs no session, so an unauthenticated caller who knows a login and
+     * its email address could choose the host in the mail the real user then receives — a
+     * legitimate message, from the real installation, whose link hands the one-time hash to
+     * somebody else.
+     *
+     * Six other link builders in this application already prefer the configured application URL
+     * (`AccountHelper`, `ViewLinkController`, `PublicLinkViewBase`, `AccountSearchItem`,
+     * `Template`, `Account\SaveRequestController`); these two were the exception. The fallback is
+     * the unforwarded host rather than `getWebUri()`, so the header cannot choose it even when no
+     * application URL has been configured.
+     */
+    public static function getMailMessage(
+        string              $hash,
+        ConfigDataInterface $configData,
+        UriContextInterface $uriContext
+    ): MailMessage {
+        $baseUri = $configData->getApplicationUrl() ?: $uriContext->getUnforwardedWebUri();
+
         $mailMessage = new MailMessage();
         $mailMessage->setTitle(__('Password Change'));
         $mailMessage->addDescription(__('A request for changing your user password has been done.'));

src/Domain/Core/Bootstrap/UriContextInterface.php

@@ -37,6 +37,11 @@ interface UriContextInterface
      */
     public function getWebUri(): string;
 
+    /**
+     * The same URI, built without consulting `Forwarded` / `X-Forwarded-*`.
+     */
+    public function getUnforwardedWebUri(): string;
+
     /**
      * The current request path relative to the application root (e.g. files/index.php)
      *

src/Domain/Http/Ports/RequestService.php

@@ -100,6 +100,11 @@ public function verifySignature(string $key, ?string $param = null): void;
      */
     public function getHttpHost(): string;
 
+    /**
+     * The host this request actually arrived at, without consulting the forwarded headers.
+     */
+    public function getHttpHostIgnoringForwarding(): string;
+
     /**
      * Return forward data per RFC 7239
      *

src/Infrastructure/Adapter/In/Web/Controllers/User/UserSaveBase.php

@@ -122,7 +122,7 @@ final protected function checkChangeUserPass(int $userId, User $userData): void
             $this->mailService->send(
                 __('Password Change'),
                 $userData->getEmail() ?? '',
-                UserPassRecover::getMailMessage($hash, $this->uriContext->getWebUri())
+                UserPassRecover::getMailMessage($hash, $this->configData, $this->uriContext)
             );
         }
     }

src/Infrastructure/Adapter/In/Web/Controllers/UserPassReset/SaveRequestController.php

@@ -122,7 +122,7 @@ public function saveRequestAction(): ActionResponse
             $this->mailService->send(
                 __('Password Change'),
                 $email,
-                UserPassRecover::getMailMessage($hash, $this->uriContext->getWebUri())
+                UserPassRecover::getMailMessage($hash, $this->configData, $this->uriContext)
             );
         } catch (Exception $e) {
             // Recorded and counted, not reported. The tracking still runs, so guessing is still

src/Infrastructure/Bootstrap/UriContext.php

@@ -36,12 +36,14 @@
     private string $subUri;
     private string $webRoot;
     private string $webUri;
+    private string $unforwardedWebUri;
 
     public function __construct(RequestService $request)
     {
         $this->subUri = $this->buildSubUri($request);
         $this->webRoot = $this->buildWebRoot($request);
         $this->webUri = $request->getHttpHost() . $this->webRoot;
+        $this->unforwardedWebUri = $request->getHttpHostIgnoringForwarding() . $this->webRoot;
     }
 
     private function buildSubUri(RequestService $request): string
@@ -62,6 +64,21 @@ private function buildWebRoot(RequestService $request): string
         return '';
     }
 
+    /**
+     * The same URI, built without consulting `Forwarded` / `X-Forwarded-*`.
+     *
+     * `getWebUri()` prefers those headers so that an installation behind a reverse proxy reports
+     * the address its users actually type. They are supplied by whoever made the request, though,
+     * and nothing here calls `setTrustedProxies()` — verified against the running instance, where
+     * `X-Forwarded-Host: evil.example.com` comes straight back out. That is the right trade for a
+     * displayed URL and the wrong one for a link that is mailed to somebody else and carries a
+     * one-time token, which is what this exists for.
+     */
+    public function getUnforwardedWebUri(): string
+    {
+        return $this->unforwardedWebUri;
+    }
+
     public function getWebUri(): string
     {
         return $this->webUri;

src/Infrastructure/Http/Services/Request.php

@@ -387,6 +387,19 @@ public function getHttpHost(): string
             return strtolower(sprintf('%s://%s', $forwarded['proto'], $forwarded['host']));
         }
 
+        return $this->getHttpHostIgnoringForwarding();
+    }
+
+    /**
+     * The host this request actually arrived at, without consulting the forwarded headers.
+     *
+     * Those headers are supplied by whoever made the request — nothing here calls
+     * `setTrustedProxies()` — so `getHttpHost()` answers whatever a caller asks it to. That is
+     * acceptable for a URL the same caller is about to be shown, and not for one that is mailed to
+     * somebody else with a one-time token in it.
+     */
+    public function getHttpHostIgnoringForwarding(): string
+    {
         /** @noinspection HttpUrlsUsage */
         $protocol = 'http://';
 

tests/Integration/Infrastructure/Adapter/In/Web/Controllers/UserPassReset/SaveRequestControllerTest.php

@@ -30,6 +30,8 @@
 use PHPUnit\Framework\MockObject\Exception;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
+use SP\Application\Notification\Ports\MailService;
+use SP\Domain\Core\Messages\MailMessage;
 use SP\Domain\User\Models\User as UserModel;
 use SP\Domain\Common\Dtos\QueryResult;
 use SP\Tests\Support\Generators\UserDataGenerator;
@@ -78,4 +80,62 @@ public function testSaveRequestBuildsTheResetMailWithoutFataling(): void
 
         $this->expectOutputRegex('/"status":"OK","description":"Request sent"/');
     }
+
+    /**
+     * The mailed link points where the installation lives, not where the caller said it does.
+     *
+     * `saveRequestAction()` needs no session, and the base URI came from
+     * `UriContextInterface::getWebUri()`, which prefers `Forwarded` / `X-Forwarded-Host`. Nothing
+     * in this application calls `setTrustedProxies()`, so those headers are whatever the caller
+     * sent — verified against the running instance, where `X-Forwarded-Host: evil.example.com`
+     * comes straight back out.
+     *
+     * So an unauthenticated caller who knew a login and its email address could choose the host in
+     * the message the real user then received: a genuine mail, from the real installation, whose
+     * link hands the one-time hash to somebody else's server.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    public function testTheResetLinkIgnoresAForwardedHost(): void
+    {
+        $login = 'resetme';
+        $email = 'resetme@example.com';
+
+        $userData = UserDataGenerator::factory()->buildUserData()->mutate(
+            ['login' => $login, 'email' => $email, 'isDisabled' => false, 'isLdap' => false]
+        );
+
+        $this->addDatabaseMapperResolver(UserModel::class, new QueryResult([$userData]));
+
+        $sent = null;
+        $mailService = $this->createStub(MailService::class);
+        $mailService->method('send')->willReturnCallback(
+            static function (string $subject, string|array $to, MailMessage $mailMessage) use (&$sent): void {
+                $sent = $mailMessage->composeText();
+            }
+        );
+
+        $container = $this->buildContainer(
+            IntegrationTestCase::buildRequest(
+                'post',
+                'index.php',
+                ['r' => 'userPassReset/saveRequest'],
+                ['login' => $login, 'email' => $email],
+                [],
+                self::CSRF_TOKEN,
+                ['HTTP_X_FORWARDED_HOST' => 'evil.example.com', 'HTTP_X_FORWARDED_PROTO' => 'https']
+            ),
+            [MailService::class => $mailService]
+        );
+
+        IntegrationTestCase::runApp($container);
+
+        self::assertNotNull($sent, 'the reset mail has to be sent for this to say anything');
+        self::assertStringNotContainsString('evil.example.com', $sent);
+        self::assertStringContainsString('localhost', $sent);
+
+        $this->expectOutputRegex('/"status":"OK","description":"Request sent"/');
+    }
 }