Skip to content

Commit

Permalink
fix: password reset leaks user existence (#3616)
Browse files Browse the repository at this point in the history
  • Loading branch information
SychO9 authored Sep 14, 2022
1 parent 5952bf1 commit 2fb79f2
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 164 deletions.
2 changes: 1 addition & 1 deletion locale/core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
38 changes: 22 additions & 16 deletions src/Api/Controller/ForgotPasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@

namespace Flarum\Api\Controller;

use Flarum\User\Command\RequestPasswordReset;
use Flarum\User\UserRepository;
use Illuminate\Contracts\Bus\Dispatcher;
use Flarum\User\Job\RequestPasswordResetJob;
use Illuminate\Contracts\Queue\Queue;
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;
Expand All @@ -21,23 +22,19 @@
class ForgotPasswordController implements RequestHandlerInterface
{
/**
* @var \Flarum\User\UserRepository
* @var Queue
*/
protected $users;
protected $queue;

/**
* @var Dispatcher
* @var Factory
*/
protected $bus;
protected $validatorFactory;

/**
* @param \Flarum\User\UserRepository $users
* @param Dispatcher $bus
*/
public function __construct(UserRepository $users, Dispatcher $bus)
public function __construct(Queue $queue, Factory $validatorFactory)
{
$this->users = $users;
$this->bus = $bus;
$this->queue = $queue;
$this->validatorFactory = $validatorFactory;
}

/**
Expand All @@ -47,10 +44,19 @@ public function handle(ServerRequestInterface $request): ResponseInterface
{
$email = Arr::get($request->getParsedBody(), 'email');

$this->bus->dispatch(
new RequestPasswordReset($email)
$validation = $this->validatorFactory->make(
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;
}
}
28 changes: 0 additions & 28 deletions src/User/Command/RequestPasswordReset.php

This file was deleted.

119 changes: 0 additions & 119 deletions src/User/Command/RequestPasswordResetHandler.php

This file was deleted.

60 changes: 60 additions & 0 deletions src/User/Job/RequestPasswordResetJob.php
Original file line number Diff line number Diff line change
@@ -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));
}
}
15 changes: 15 additions & 0 deletions tests/integration/api/users/SendPasswordResetEmailTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,19 @@ public function users_cant_send_confirmation_emails_too_fast()

$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' => '[email protected]'
]
])
);

$this->assertEquals(204, $response->getStatusCode());
}
}

0 comments on commit 2fb79f2

Please sign in to comment.