diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index b68f91f986e92..6df88726afc4d 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -311,11 +311,23 @@ public function tryLogin(string $user, 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( diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 30a625a612b68..9884255a80e69 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -500,7 +500,7 @@ public function testLoginWithValidCredentials() { $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 @@ public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn() { ->expects($this->once()) ->method('passesCSRFCheck') ->willReturn(false); - $this->userSession->expects($this->once()) + $this->userSession ->method('isLoggedIn') ->with() ->willReturn(false); @@ -521,13 +521,12 @@ public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn() { ->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 @@ public function testLoginWithoutPassedCsrfCheckAndLoggedIn() { ->expects($this->once()) ->method('passesCSRFCheck') ->willReturn(false); - $this->userSession->expects($this->once()) + $this->userSession ->method('isLoggedIn') ->with() ->willReturn(true); @@ -561,8 +560,10 @@ public function testLoginWithoutPassedCsrfCheckAndLoggedIn() { ->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() {