Merge pull request #46392 from nextcloud/chore/bruteforce-dont-count-failed-csrf

feat: don't count failed CSRF as failed login attempt
This commit is contained in:
Benjamin Gaussorgues 2024-07-11 11:20:02 +02:00 committed by GitHub
commit d526d7813d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 12 additions and 5 deletions

View file

@ -207,7 +207,7 @@ class LoginController extends Controller {
$this->canResetPassword($passwordLink, $user)
);
}
/**
* Sets the initial state of whether or not a user is allowed to login with their email
* initial state is passed in the array of 1 for email allowed and 0 for not allowed
@ -299,7 +299,8 @@ class LoginController extends Controller {
$user,
$user,
$redirect_url,
self::LOGIN_MSG_CSRFCHECKFAILED
self::LOGIN_MSG_CSRFCHECKFAILED,
false,
);
}
@ -349,7 +350,12 @@ class LoginController extends Controller {
* @return RedirectResponse
*/
private function createLoginFailedResponse(
$user, $originalUser, $redirect_url, string $loginMessage) {
$user,
$originalUser,
$redirect_url,
string $loginMessage,
bool $throttle = true,
) {
// Read current user and append if possible we need to
// return the unmodified user otherwise we will leak the login name
$args = $user !== null ? ['user' => $originalUser, 'direct' => 1] : [];
@ -359,7 +365,9 @@ class LoginController extends Controller {
$response = new RedirectResponse(
$this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)
);
$response->throttle(['user' => substr($user, 0, 64)]);
if ($throttle) {
$response->throttle(['user' => substr($user, 0, 64)]);
}
$this->session->set('loginMessages', [
[$loginMessage], []
]);

View file

@ -520,7 +520,6 @@ class LoginControllerTest extends TestCase {
$response = $this->loginController->tryLogin($loginChain, 'Jane', $password, $originalUrl);
$expected = new RedirectResponse('');
$expected->throttle(['user' => 'Jane']);
$this->assertEquals($expected, $response);
}