Merge pull request #38268 from nextcloud/backport/38267/stable26

[stable26] fix(lostpassword): Also rate limit the setPassword endpoint
This commit is contained in:
Joas Schilling 2023-05-16 06:35:29 +02:00 committed by GitHub
commit e05da15f5b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 13 deletions

View file

@ -223,8 +223,10 @@ class LostController extends Controller {
/**
* @PublicPage
* @BruteForceProtection(action=passwordResetEmail)
* @AnonRateThrottle(limit=10, period=300)
*/
public function setPassword(string $token, string $userId, string $password, bool $proceed): array {
public function setPassword(string $token, string $userId, string $password, bool $proceed): JSONResponse {
if ($this->encryptionManager->isEnabled() && !$proceed) {
$encryptionModules = $this->encryptionManager->getEncryptionModules();
foreach ($encryptionModules as $module) {
@ -232,7 +234,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]));
}
}
}
@ -260,12 +262,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]));
}
/**

View file

@ -447,7 +447,7 @@ class LostControllerTest extends 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() {
@ -475,7 +475,7 @@ class LostControllerTest extends 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() {
@ -494,7 +494,7 @@ class LostControllerTest extends TestCase {
'status' => 'error',
'msg' => 'Could not reset password because the token is expired',
];
$this->assertSame($expectedResponse, $response);
$this->assertSame($expectedResponse, $response->getData());
}
public function testSetPasswordInvalidDataInDb() {
@ -514,7 +514,7 @@ class LostControllerTest extends TestCase {
'status' => 'error',
'msg' => 'Could not reset password because the token is invalid',
];
$this->assertSame($expectedResponse, $response);
$this->assertSame($expectedResponse, $response->getData());
}
public function testIsSetPasswordWithoutTokenFailing() {
@ -533,7 +533,7 @@ class LostControllerTest extends TestCase {
'status' => 'error',
'msg' => 'Could not reset password because the token is invalid'
];
$this->assertSame($expectedResponse, $response);
$this->assertSame($expectedResponse, $response->getData());
}
public function testSetPasswordForDisabledUser() {
@ -563,7 +563,7 @@ class LostControllerTest extends TestCase {
'status' => 'error',
'msg' => 'Could not reset password because the token is invalid'
];
$this->assertSame($expectedResponse, $response);
$this->assertSame($expectedResponse, $response->getData());
}
public function testSendEmailNoEmail() {
@ -599,7 +599,7 @@ class LostControllerTest extends 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() {
@ -627,7 +627,7 @@ class LostControllerTest extends 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() {