Merge pull request #38270 from nextcloud/backport/38267/stable24

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

View file

@ -247,11 +247,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->encryptionManager->isEnabled() && !$proceed) {
@ -261,7 +263,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]));
}
}
}
@ -283,12 +285,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

@ -429,7 +429,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() {
@ -451,7 +451,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() {
@ -470,7 +470,7 @@ class LostControllerTest extends \Test\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() {
@ -490,7 +490,7 @@ class LostControllerTest extends \Test\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() {
@ -509,7 +509,7 @@ class LostControllerTest extends \Test\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() {
@ -539,7 +539,7 @@ class LostControllerTest extends \Test\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() {
@ -575,7 +575,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() {
@ -603,7 +603,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() {