Merge pull request #34660 from nextcloud/backport/34632/stable23

[stable23] Add rate limiting on lost password emails
This commit is contained in:
Vincent Petry 2022-11-04 17:31:06 +01:00 committed by GitHub
commit ca6eaebe85
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 8 deletions

View file

@ -35,8 +35,6 @@
*/
namespace OC\Core\Controller;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Exception\ResetPasswordException;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\TemplateResponse;
@ -53,8 +51,12 @@ use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Mail\IMailer;
use OCP\Security\VerificationToken\InvalidTokenException;
use OCP\Security\VerificationToken\IVerificationToken;
use OCP\Security\VerificationToken\InvalidTokenException;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Exception\ResetPasswordException;
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
use OC\Security\RateLimiting\Limiter;
use function array_filter;
use function count;
use function reset;
@ -91,6 +93,8 @@ class LostController extends Controller {
private $initialStateService;
/** @var IVerificationToken */
private $verificationToken;
/** @var Limiter */
private $limiter;
public function __construct(
$appName,
@ -106,7 +110,8 @@ class LostController extends Controller {
ILogger $logger,
Manager $twoFactorManager,
IInitialStateService $initialStateService,
IVerificationToken $verificationToken
IVerificationToken $verificationToken,
Limiter $limiter
) {
parent::__construct($appName, $request);
$this->urlGenerator = $urlGenerator;
@ -121,6 +126,7 @@ class LostController extends Controller {
$this->twoFactorManager = $twoFactorManager;
$this->initialStateService = $initialStateService;
$this->verificationToken = $verificationToken;
$this->limiter = $limiter;
}
/**
@ -294,6 +300,12 @@ class LostController extends Controller {
throw new ResetPasswordException('Could not send reset e-mail since there is no email for username ' . $input);
}
try {
$this->limiter->registerUserRequest('lostpasswordemail', 5, 1800, $user);
} catch (RateLimitExceededException $e) {
throw new ResetPasswordException('Could not send reset e-mail, 5 of them were already sent in the last 30 minutes', 0, $e);
}
// Generate the token. It is stored encrypted in the database with the
// secret being the users' email address appended with the system secret.
// This makes the token automatically invalidate once the user changes

View file

@ -45,7 +45,7 @@ class Limiter {
/**
* @param string $methodIdentifier
* @param string $userIdentifier
* @param int $period
* @param int $period in seconds
* @param int $limit
* @throws RateLimitExceededException
*/
@ -66,7 +66,7 @@ class Limiter {
*
* @param string $identifier
* @param int $anonLimit
* @param int $anonPeriod
* @param int $anonPeriod in seconds
* @param string $ip
* @throws RateLimitExceededException
*/
@ -85,7 +85,7 @@ class Limiter {
*
* @param string $identifier
* @param int $userLimit
* @param int $userPeriod
* @param int $userPeriod in seconds
* @param IUser $user
* @throws RateLimitExceededException
*/

View file

@ -24,6 +24,7 @@ namespace Tests\Core\Controller;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Controller\LostController;
use OC\Mail\Message;
use OC\Security\RateLimiting\Limiter;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\Defaults;
@ -41,6 +42,7 @@ use OCP\Mail\IEMailTemplate;
use OCP\Mail\IMailer;
use OCP\Security\VerificationToken\InvalidTokenException;
use OCP\Security\VerificationToken\IVerificationToken;
use PHPUnit\Framework\MockObject\MockObject;
/**
* Class LostControllerTest
@ -77,6 +79,8 @@ class LostControllerTest extends \Test\TestCase {
private $initialStateService;
/** @var IVerificationToken|\PHPUnit\Framework\MockObject\MockObject */
private $verificationToken;
/** @var Limiter|MockObject */
private $limiter;
protected function setUp(): void {
parent::setUp();
@ -129,6 +133,7 @@ class LostControllerTest extends \Test\TestCase {
$this->twofactorManager = $this->createMock(Manager::class);
$this->initialStateService = $this->createMock(IInitialStateService::class);
$this->verificationToken = $this->createMock(IVerificationToken::class);
$this->limiter = $this->createMock(Limiter::class);
$this->lostController = new LostController(
'Core',
$this->request,
@ -143,7 +148,8 @@ class LostControllerTest extends \Test\TestCase {
$this->logger,
$this->twofactorManager,
$this->initialStateService,
$this->verificationToken
$this->verificationToken,
$this->limiter
);
}