Merge pull request #38273 from nextcloud/backport/38267/stable21

[stable21] fix(lostpassword): Also rate limit the setPassword endpoint

core/Controller/LostController.php

@@ -283,11 +283,13 @@ class LostController extends Controller {
 
 	/**
 	 * @PublicPage
+	 * @BruteForceProtection(action=passwordResetEmail)
+	 * @AnonRateThrottle(limit=10, period=300)
 	 * @param string $token
 	 * @param string $userId
 	 * @param string $password
 	 * @param boolean $proceed
-	 * @return array
+	 * @return JSONResponse
 	 */
 	public function setPassword($token, $userId, $password, $proceed) {
 		if ($this->config->getSystemValue('lost_password_link', '') !== '') {
@@ -301,7 +303,7 @@ class LostController extends Controller {
 				$instance = call_user_func($module['callback']);
 				// this way we can find out whether per-user keys are used or a system wide encryption key
 				if ($instance->needDetailedAccessList()) {
-					return $this->error('', ['encryption' => true]);
+					return new JSONResponse($this->error('', ['encryption' => true]));
 				}
 			}
 		}
@@ -323,12 +325,16 @@ class LostController extends Controller {
 			$this->config->deleteUserValue($userId, 'core', 'lostpassword');
 			@\OC::$server->getUserSession()->unsetMagicInCookie();
 		} catch (HintException $e) {
-			return $this->error($e->getHint());
+			$response = new JSONResponse($this->error($e->getHint()));
+			$response->throttle();
+			return $response;
 		} catch (\Exception $e) {
-			return $this->error($e->getMessage());
+			$response = new JSONResponse($this->error($e->getMessage()));
+			$response->throttle();
+			return $response;
 		}
 
-		return $this->success(['user' => $userId]);
+		return new JSONResponse($this->success(['user' => $userId]));
 	}
 
 	/**

tests/Core/Controller/LostControllerTest.php

@@ -574,7 +574,7 @@ class LostControllerTest extends \Test\TestCase {
 
 		$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
 		$expectedResponse = ['status' => 'error', 'msg' => ''];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordSuccessful() {
@@ -604,7 +604,7 @@ class LostControllerTest extends \Test\TestCase {
 
 		$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true);
 		$expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success'];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordExpiredToken() {
@@ -628,7 +628,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is expired',
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordInvalidDataInDb() {
@@ -651,7 +651,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is invalid',
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordExpiredTokenDueToLogin() {
@@ -679,7 +679,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is expired',
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testIsSetPasswordWithoutTokenFailing() {
@@ -701,7 +701,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is invalid'
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testIsSetPasswordTokenNullFailing() {
@@ -717,7 +717,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is invalid'
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordForDisabledUser() {
@@ -740,7 +740,7 @@ class LostControllerTest extends \Test\TestCase {
 			'status' => 'error',
 			'msg' => 'Couldn\'t reset password because the token is invalid'
 		];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSendEmailNoEmail() {
@@ -776,7 +776,7 @@ class LostControllerTest extends \Test\TestCase {
 			}]]);
 		$response = $this->lostController->setPassword('myToken', 'user', 'newpass', false);
 		$expectedResponse = ['status' => 'error', 'msg' => '', 'encryption' => true];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testSetPasswordDontProceedMasterKey() {
@@ -812,7 +812,7 @@ class LostControllerTest extends \Test\TestCase {
 
 		$response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', false);
 		$expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success'];
-		$this->assertSame($expectedResponse, $response);
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
 	public function testTwoUsersWithSameEmail() {