Skip to content

Commit 085ca00

Browse files
nfebeAndyScherzinger
authored andcommitted
fix(core): Throttle lost-password reset form when link is disabled
When password reset links were disabled, the reset form did not rate-limit failed token attempts, leaving that endpoint open to unthrottled token guessing. Failed attempts on the form are now always throttled, consistent with the other reset steps. Signed-off-by: nfebe <fenn25.fn@gmail.com>
1 parent 308e3fa commit 085ca00

2 files changed

Lines changed: 41 additions & 20 deletions

File tree

core/Controller/LostController.php

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,24 +89,19 @@ public function resetform(string $token, string $userId): TemplateResponse {
8989
try {
9090
$this->checkPasswordResetToken($token, $userId);
9191
} catch (Exception $e) {
92-
if ($this->config->getSystemValue('lost_password_link', '') !== 'disabled'
93-
|| ($e instanceof InvalidTokenException
94-
&& !in_array($e->getCode(), [InvalidTokenException::TOKEN_NOT_FOUND, InvalidTokenException::USER_UNKNOWN]))
95-
) {
96-
$response = new TemplateResponse(
97-
'core', 'error', [
98-
'errors' => [['error' => $e->getMessage()]]
99-
],
100-
TemplateResponse::RENDER_AS_GUEST
101-
);
102-
$response->throttle();
103-
return $response;
92+
if ($this->config->getSystemValue('lost_password_link', '') === 'disabled') {
93+
$message = $this->l10n->t('Password reset is disabled');
94+
} else {
95+
$message = $e->getMessage();
10496
}
105-
return new TemplateResponse('core', 'error', [
106-
'errors' => [['error' => $this->l10n->t('Password reset is disabled')]]
107-
],
97+
$response = new TemplateResponse(
98+
'core', 'error', [
99+
'errors' => [['error' => $message]]
100+
],
108101
TemplateResponse::RENDER_AS_GUEST
109102
);
103+
$response->throttle();
104+
return $response;
110105
}
111106
$this->initialState->provideInitialState('resetPasswordUser', $userId);
112107
$this->initialState->provideInitialState('resetPasswordTarget',

tests/Core/Controller/LostControllerTest.php

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class LostControllerTest extends TestCase {
5353
private $defaults;
5454
/** @var IConfig | MockObject */
5555
private $config;
56+
private string $lostPasswordLink = '';
5657
/** @var IMailer | MockObject */
5758
private $mailer;
5859
/** @var IManager|MockObject */
@@ -92,11 +93,13 @@ protected function setUp(): void {
9293
$this->config = $this->createMock(IConfig::class);
9394
$this->config->expects($this->any())
9495
->method('getSystemValue')
95-
->willReturnMap([
96-
['secret', null, 'SECRET'],
97-
['secret', '', 'SECRET'],
98-
['lost_password_link', '', ''],
99-
]);
96+
->willReturnCallback(function (string $key, $default = '') {
97+
return match ($key) {
98+
'secret' => 'SECRET',
99+
'lost_password_link' => $this->lostPasswordLink,
100+
default => $default,
101+
};
102+
});
100103
$this->l10n = $this->createMock(IL10N::class);
101104
$this->l10n
102105
->expects($this->any())
@@ -161,6 +164,29 @@ public function testResetFormTokenError(): void {
161164
$this->assertEquals($expectedResponse, $response);
162165
}
163166

167+
public function testResetFormTokenErrorWithDisabledLink(): void {
168+
$this->lostPasswordLink = 'disabled';
169+
$this->userManager->method('get')
170+
->with('ValidTokenUser')
171+
->willReturn($this->existingUser);
172+
$this->verificationToken->expects($this->once())
173+
->method('check')
174+
->with('12345:MySecretToken', $this->existingUser, 'lostpassword', 'test@example.com')
175+
->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_NOT_FOUND));
176+
177+
$response = $this->lostController->resetform('12345:MySecretToken', 'ValidTokenUser');
178+
$expectedResponse = new TemplateResponse('core',
179+
'error',
180+
[
181+
'errors' => [
182+
['error' => 'Password reset is disabled'],
183+
]
184+
],
185+
'guest');
186+
$expectedResponse->throttle();
187+
$this->assertEquals($expectedResponse, $response);
188+
}
189+
164190
public function testResetFormValidToken(): void {
165191
$this->userManager->method('get')
166192
->with('ValidTokenUser')

0 commit comments

Comments
 (0)