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

Merge pull request #36743 from nextcloud/backport/35419/stable24

Browse source 
[stable24] Fix login loop if login CSRF fails and user is not logged in
This commit is contained in:
Arthur Schiwon 2023-03-15 14:40:02 +01:00 committed by GitHub
commit 0a9f73caa8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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() {