From 8bf2d4ac52a40323f0797bf8c2ce36277b2bae7b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 15 May 2023 09:21:07 +0200 Subject: [PATCH 1/3] fix(lostpassword): Also rate limit the setPassword endpoint Signed-off-by: Joas Schilling --- core/Controller/LostController.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index 1fda92fb50001..96579b278cf98 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -283,11 +283,13 @@ public function email($user) { /** * @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 @@ public function setPassword($token, $userId, $password, $proceed) { $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 @@ public function setPassword($token, $userId, $password, $proceed) { $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])); } /** From 33f5016998b635f8af5da00dfe631a6254c22227 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 15 May 2023 16:12:14 +0200 Subject: [PATCH 2/3] fix(tests): Adjust unit tests Signed-off-by: Joas Schilling --- tests/Core/Controller/LostControllerTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index 7a28cd864e33a..1ee0d00c169df 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -574,7 +574,7 @@ public function testSetPasswordUnsuccessful() { $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 @@ public function testSetPasswordSuccessful() { $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 @@ public function testSetPasswordExpiredToken() { 'status' => 'error', 'msg' => 'Couldn\'t reset password because the token is expired', ]; - $this->assertSame($expectedResponse, $response); + $this->assertSame($expectedResponse, $response->getData()); } public function testSetPasswordInvalidDataInDb() { @@ -679,7 +679,7 @@ public function testSetPasswordExpiredTokenDueToLogin() { 'status' => 'error', 'msg' => 'Couldn\'t reset password because the token is expired', ]; - $this->assertSame($expectedResponse, $response); + $this->assertSame($expectedResponse, $response->getData()); } public function testIsSetPasswordWithoutTokenFailing() { @@ -717,7 +717,7 @@ public function testIsSetPasswordTokenNullFailing() { '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 @@ public function testSetPasswordForDisabledUser() { '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 @@ public function testSetPasswordEncryptionDontProceedPerUserKey() { }]]); $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 @@ public function testSetPasswordDontProceedMasterKey() { $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() { From 43189698836572c326cf5192329603517e811acd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 16 May 2023 06:39:31 +0200 Subject: [PATCH 3/3] fix(tests): Fix more tests in backport Signed-off-by: Joas Schilling --- tests/Core/Controller/LostControllerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index 1ee0d00c169df..083e69d65f4a5 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -651,7 +651,7 @@ public function testSetPasswordInvalidDataInDb() { 'status' => 'error', 'msg' => 'Couldn\'t reset password because the token is invalid', ]; - $this->assertSame($expectedResponse, $response); + $this->assertSame($expectedResponse, $response->getData()); } public function testSetPasswordExpiredTokenDueToLogin() { @@ -701,7 +701,7 @@ public function testIsSetPasswordWithoutTokenFailing() { 'status' => 'error', 'msg' => 'Couldn\'t reset password because the token is invalid' ]; - $this->assertSame($expectedResponse, $response); + $this->assertSame($expectedResponse, $response->getData()); } public function testIsSetPasswordTokenNullFailing() {