fix: password reset leaks user existence (#3616)

framework/core/locale/core.yml

@@ -350,7 +350,7 @@ core:
     forgot_password:
       dismiss_button: => core.ref.okay
       email_placeholder: => core.ref.email
-      email_sent_message: We've sent you an email containing a link to reset your password. Check your spam folder if you don't receive it within the next minute or two.
+      email_sent_message: If the email you entered is registered with this site, we'll send you an email containing a link to reset your password. Check your spam folder if you don't receive it within the next minute or two.
       not_found_message: There is no user registered with that email address.
       submit_button: Recover Password
       text: Enter your email address and we will send you a link to reset your password.

framework/core/src/Api/Controller/ForgotPasswordController.php

@@ -9,10 +9,11 @@
 
 namespace Flarum\Api\Controller;
 
-use Flarum\User\Command\RequestPasswordReset;
+use Flarum\User\Job\RequestPasswordResetJob;
-use Flarum\User\UserRepository;
+use Illuminate\Contracts\Queue\Queue;
-use Illuminate\Contracts\Bus\Dispatcher;
+use Illuminate\Contracts\Validation\Factory;
 use Illuminate\Support\Arr;
+use Illuminate\Validation\ValidationException;
 use Laminas\Diactoros\Response\EmptyResponse;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
@@ -21,23 +22,19 @@ use Psr\Http\Server\RequestHandlerInterface;
 class ForgotPasswordController implements RequestHandlerInterface
 {
     /**
-     * @var \Flarum\User\UserRepository
+     * @var Queue
      */
-    protected $users;
+    protected $queue;
 
     /**
-     * @var Dispatcher
+     * @var Factory
      */
-    protected $bus;
+    protected $validatorFactory;
 
-    /**
+    public function __construct(Queue $queue, Factory $validatorFactory)
-     * @param \Flarum\User\UserRepository $users
-     * @param Dispatcher $bus
-     */
-    public function __construct(UserRepository $users, Dispatcher $bus)
     {
-        $this->users = $users;
+        $this->queue = $queue;
-        $this->bus = $bus;
+        $this->validatorFactory = $validatorFactory;
     }
 
     /**
@@ -47,10 +44,19 @@ class ForgotPasswordController implements RequestHandlerInterface
     {
         $email = Arr::get($request->getParsedBody(), 'email');
 
-        $this->bus->dispatch(
+        $validation = $this->validatorFactory->make(
-            new RequestPasswordReset($email)
+            compact('email'),
+            ['email' => 'required|email']
         );
 
+        if ($validation->fails()) {
+            throw new ValidationException($validation);
+        }
+
+        // Prevents leaking user existence by not throwing an error.
+        // Prevents leaking user existence by duration by using a queued job.
+        $this->queue->push(new RequestPasswordResetJob($email));
+
         return new EmptyResponse;
     }
 }

framework/core/src/User/Command/RequestPasswordReset.php

@@ -1,28 +0,0 @@
-<?php
-
-/*
- * This file is part of Flarum.
- *
- * For detailed copyright and license information, please view the
- * LICENSE file that was distributed with this source code.
- */
-
-namespace Flarum\User\Command;
-
-class RequestPasswordReset
-{
-    /**
-     * The email of the user to request a password reset for.
-     *
-     * @var string
-     */
-    public $email;
-
-    /**
-     * @param string $email The email of the user to request a password reset for.
-     */
-    public function __construct($email)
-    {
-        $this->email = $email;
-    }
-}

framework/core/src/User/Command/RequestPasswordResetHandler.php

@@ -1,119 +0,0 @@
-<?php
-
-/*
- * This file is part of Flarum.
- *
- * For detailed copyright and license information, please view the
- * LICENSE file that was distributed with this source code.
- */
-
-namespace Flarum\User\Command;
-
-use Flarum\Http\UrlGenerator;
-use Flarum\Mail\Job\SendRawEmailJob;
-use Flarum\Settings\SettingsRepositoryInterface;
-use Flarum\User\PasswordToken;
-use Flarum\User\UserRepository;
-use Illuminate\Contracts\Queue\Queue;
-use Illuminate\Contracts\Validation\Factory;
-use Illuminate\Database\Eloquent\ModelNotFoundException;
-use Illuminate\Validation\ValidationException;
-use Symfony\Contracts\Translation\TranslatorInterface;
-
-class RequestPasswordResetHandler
-{
-    /**
-     * @var UserRepository
-     */
-    protected $users;
-
-    /**
-     * @var SettingsRepositoryInterface
-     */
-    protected $settings;
-
-    /**
-     * @var Queue
-     */
-    protected $queue;
-
-    /**
-     * @var UrlGenerator
-     */
-    protected $url;
-
-    /**
-     * @var TranslatorInterface
-     */
-    protected $translator;
-
-    /**
-     * @var Factory
-     */
-    protected $validatorFactory;
-
-    /**
-     * @param UserRepository $users
-     * @param SettingsRepositoryInterface $settings
-     * @param Queue $queue
-     * @param UrlGenerator $url
-     * @param TranslatorInterface $translator
-     * @param Factory $validatorFactory
-     */
-    public function __construct(
-        UserRepository $users,
-        SettingsRepositoryInterface $settings,
-        Queue $queue,
-        UrlGenerator $url,
-        TranslatorInterface $translator,
-        Factory $validatorFactory
-    ) {
-        $this->users = $users;
-        $this->settings = $settings;
-        $this->queue = $queue;
-        $this->url = $url;
-        $this->translator = $translator;
-        $this->validatorFactory = $validatorFactory;
-    }
-
-    /**
-     * @param RequestPasswordReset $command
-     * @return \Flarum\User\User
-     * @throws ModelNotFoundException
-     */
-    public function handle(RequestPasswordReset $command)
-    {
-        $email = $command->email;
-
-        $validation = $this->validatorFactory->make(
-            compact('email'),
-            ['email' => 'required|email']
-        );
-
-        if ($validation->fails()) {
-            throw new ValidationException($validation);
-        }
-
-        $user = $this->users->findByEmail($email);
-
-        if (! $user) {
-            throw new ModelNotFoundException;
-        }
-
-        $token = PasswordToken::generate($user->id);
-        $token->save();
-
-        $data = [
-            'username' => $user->display_name,
-            'url' => $this->url->to('forum')->route('resetPassword', ['token' => $token->token]),
-            'forum' => $this->settings->get('forum_title'),
-        ];
-
-        $body = $this->translator->trans('core.email.reset_password.body', $data);
-        $subject = $this->translator->trans('core.email.reset_password.subject');
-
-        $this->queue->push(new SendRawEmailJob($user->email, $subject, $body));
-
-        return $user;
-    }
-}

framework/core/src/User/Job/RequestPasswordResetJob.php

@@ -0,0 +1,60 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\User\Job;
+
+use Flarum\Http\UrlGenerator;
+use Flarum\Mail\Job\SendRawEmailJob;
+use Flarum\Queue\AbstractJob;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\User\PasswordToken;
+use Flarum\User\UserRepository;
+use Illuminate\Contracts\Queue\Queue;
+use Symfony\Contracts\Translation\TranslatorInterface;
+
+class RequestPasswordResetJob extends AbstractJob
+{
+    /**
+     * @var string
+     */
+    protected $email;
+
+    public function __construct(string $email)
+    {
+        $this->email = $email;
+    }
+
+    public function handle(
+        SettingsRepositoryInterface $settings,
+        UrlGenerator $url,
+        TranslatorInterface $translator,
+        UserRepository $users,
+        Queue $queue
+    ) {
+        $user = $users->findByEmail($this->email);
+
+        if (! $user) {
+            return;
+        }
+
+        $token = PasswordToken::generate($user->id);
+        $token->save();
+
+        $data = [
+            'username' => $user->display_name,
+            'url' => $url->to('forum')->route('resetPassword', ['token' => $token->token]),
+            'forum' => $settings->get('forum_title'),
+        ];
+
+        $body = $translator->trans('core.email.reset_password.body', $data);
+        $subject = $translator->trans('core.email.reset_password.subject');
+
+        $queue->push(new SendRawEmailJob($user->email, $subject, $body));
+    }
+}

framework/core/tests/integration/api/users/SendPasswordResetEmailTest.php

@@ -70,4 +70,19 @@ class SendPasswordResetEmailTest extends TestCase
 
         $this->assertEquals(429, $response->getStatusCode());
     }
+
+    /** @test */
+    public function request_password_reset_does_not_leak_user_existence()
+    {
+        $response = $this->send(
+            $this->request('POST', '/api/forgot', [
+                'authenticatedAs' => 3,
+                'json' => [
+                    'email' => 'missing_user@machine.local'
+                ]
+            ])
+        );
+
+        $this->assertEquals(204, $response->getStatusCode());
+    }
 }