Merge pull request #34632 from nextcloud/fix/rate-limit-recovery-emails

Add rate limiting on lost password emails

3 files changed, 27 insertions, 11 deletions

diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php
index e5dc5218cb1..fadfa242b93 100644
--- a/core/Controller/LostController.php
+++ b/core/Controller/LostController.php
@@ -36,10 +36,6 @@
 namespace OC\Core\Controller;
 
 use Exception;
-use OC\Authentication\TwoFactorAuth\Manager;
-use OC\Core\Events\BeforePasswordResetEvent;
-use OC\Core\Events\PasswordResetEvent;
-use OC\Core\Exception\ResetPasswordException;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -56,8 +52,14 @@ 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\Events\BeforePasswordResetEvent;
+use OC\Core\Events\PasswordResetEvent;
+use OC\Core\Exception\ResetPasswordException;
+use OC\Security\RateLimiting\Exception\RateLimitExceededException;
+use OC\Security\RateLimiting\Limiter;
 use Psr\Log\LoggerInterface;
 use function array_filter;
 use function count;
@@ -84,6 +86,7 @@ class LostController extends Controller {
 	private IInitialState $initialState;
 	private IVerificationToken $verificationToken;
 	private IEventDispatcher $eventDispatcher;
+	private Limiter $limiter;
 
 	public function __construct(
 		string $appName,
@@ -100,7 +103,8 @@ class LostController extends Controller {
 		Manager $twoFactorManager,
 		IInitialState $initialState,
 		IVerificationToken $verificationToken,
-		IEventDispatcher $eventDispatcher
+		IEventDispatcher $eventDispatcher,
+		Limiter $limiter
 	) {
 		parent::__construct($appName, $request);
 		$this->urlGenerator = $urlGenerator;
@@ -116,6 +120,7 @@ class LostController extends Controller {
 		$this->initialState = $initialState;
 		$this->verificationToken = $verificationToken;
 		$this->eventDispatcher = $eventDispatcher;
+		$this->limiter = $limiter;
 	}
 
 	/**
@@ -267,6 +272,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
diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php
index 91657452d99..7848a5b75a7 100644
--- a/lib/private/Security/RateLimiting/Limiter.php
+++ b/lib/private/Security/RateLimiting/Limiter.php
@@ -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
 	 */
diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php
index 31f2767ea4f..3f62c522627 100644
--- a/tests/Core/Controller/LostControllerTest.php
+++ b/tests/Core/Controller/LostControllerTest.php
@@ -26,6 +26,7 @@ use OC\Core\Controller\LostController;
 use OC\Core\Events\BeforePasswordResetEvent;
 use OC\Core\Events\PasswordResetEvent;
 use OC\Mail\Message;
+use OC\Security\RateLimiting\Limiter;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
@@ -43,8 +44,8 @@ use OCP\Mail\IEMailTemplate;
 use OCP\Mail\IMailer;
 use OCP\Security\VerificationToken\InvalidTokenException;
 use OCP\Security\VerificationToken\IVerificationToken;
-use Psr\Log\LoggerInterface;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 /**
@@ -82,6 +83,8 @@ class LostControllerTest extends TestCase {
 	private $verificationToken;
 	/** @var IEventDispatcher|MockObject */
 	private $eventDispatcher;
+	/** @var Limiter|MockObject */
+	private $limiter;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -129,6 +132,7 @@ class LostControllerTest extends TestCase {
 		$this->initialState = $this->createMock(IInitialState::class);
 		$this->verificationToken = $this->createMock(IVerificationToken::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
+		$this->limiter = $this->createMock(Limiter::class);
 		$this->lostController = new LostController(
 			'Core',
 			$this->request,
@@ -144,7 +148,8 @@ class LostControllerTest extends TestCase {
 			$this->twofactorManager,
 			$this->initialState,
 			$this->verificationToken,
-			$this->eventDispatcher
+			$this->eventDispatcher,
+			$this->limiter
 		);
 	}