Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: password reset leaks user existence #3616

Merged
merged 11 commits into from
Sep 14, 2022
2 changes: 1 addition & 1 deletion framework/core/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 framework/core/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 framework/core/src/User/Command/RequestPasswordReset.php

This file was deleted.

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

This file was deleted.

60 changes: 60 additions & 0 deletions framework/core/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));
}
}
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' => 'missing_user@machine.local'
]
])
);

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