upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-09 00:32:29 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

Fix login loop if login CSRF fails and user is not logged in

Browse source 
If CSRF fails but the user is logged in that they probably logged in in
another tab. This is fine. We can just redirect.
If CSRF fails and the user is also not logged in then something is
fishy. E.g. because Nextcloud contantly regenrates the session and the
CSRF token and the user is stuck in an endless login loop.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
Christoph Wurst 2022-11-25 09:22:28 +01:00 committed by Arthur Schiwon
parent 17c118326e
commit b9baa624bc
No known key found for this signature in database
GPG key ID: 7424F1874854DF23
2 changed files with 27 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
core/Controller/LoginController.php
View file

@ -311,11 +311,23 @@ class LoginController extends Controller {
string $redirect_url = null,
string $timezone = '',
string $timezone_offset = ''): RedirectResponse {
// If the user is already logged in and the CSRF check does not pass then
// simply redirect the user to the correct page as required. This is the
// case when an user has already logged-in, in another tab.
if (!$this->request->passesCSRFCheck()) {
return $this->generateRedirect($redirect_url);
if ($this->userSession->isLoggedIn()) {
// If the user is already logged in and the CSRF check does not pass then
// simply redirect the user to the correct page as required. This is the
// case when a user has already logged-in, in another tab.
return $this->generateRedirect($redirect_url);
}
// Clear any auth remnants like cookies to ensure a clean login
// For the next attempt
$this->userSession->logout();
return $this->createLoginFailedResponse(
$user,
$user,
$redirect_url,
$this->l10n->t('Please try again')
);
}
$data = new LoginData(

21
tests/Core/Controller/LoginControllerTest.php
View file

@ -500,7 +500,7 @@ class LoginControllerTest extends TestCase {
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password));
}
public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn() {
public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn(): void {
/** @var IUser|MockObject $user */
$user = $this->createMock(IUser::class);
$user->expects($this->any())
@ -513,7 +513,7 @@ class LoginControllerTest extends TestCase {
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->userSession->expects($this->once())
$this->userSession
->method('isLoggedIn')
->with()
->willReturn(false);
@ -521,13 +521,12 @@ class LoginControllerTest extends TestCase {
->method('deleteUserValue');
$this->userSession->expects($this->never())
->method('createRememberMeToken');
$this->urlGenerator
->expects($this->once())
->method('linkToDefaultPageUrl')
->willReturn('/default/foo');
$expected = new RedirectResponse('/default/foo');
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
$response = $this->loginController->tryLogin('Jane', $password, $originalUrl);
$expected = new RedirectResponse('');
$expected->throttle(['user' => 'Jane']);
$this->assertEquals($expected, $response);
}
public function testLoginWithoutPassedCsrfCheckAndLoggedIn() {
@ -544,7 +543,7 @@ class LoginControllerTest extends TestCase {
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->userSession->expects($this->once())
$this->userSession
->method('isLoggedIn')
->with()
->willReturn(true);
@ -561,8 +560,10 @@ class LoginControllerTest extends TestCase {
->with('remember_login_cookie_lifetime')
->willReturn(1234);
$response = $this->loginController->tryLogin('Jane', $password, $originalUrl);
$expected = new RedirectResponse($redirectUrl);
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
$this->assertEquals($expected, $response);
}
public function testLoginWithValidCredentialsAndRedirectUrl() {