From 7418c3e1f4cc616cf7aaed031ee830d8bfd75ff3 Mon Sep 17 00:00:00 2001 From: Gaspard d'Hautefeuille Date: Tue, 2 Jan 2024 10:52:46 +0200 Subject: [PATCH 1/4] Cancel PR #37405, remove regression code Signed-off-by: Gaspard d'Hautefeuille --- core/Controller/LoginController.php | 4 +-- tests/Core/Controller/LoginControllerTest.php | 29 ++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 72f69241c9d02..fb528839ce786 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -35,6 +35,7 @@ */ namespace OC\Core\Controller; +use OC\AppFramework\Http\Request; use OC\Authentication\Login\Chain; use OC\Authentication\Login\LoginData; use OC\Authentication\WebAuthn\Manager as WebAuthnManager; @@ -105,8 +106,7 @@ public function logout() { $this->session->set('clearingExecutionContexts', '1'); $this->session->close(); - if ($this->request->getServerProtocol() === 'https') { - // This feature is available only in secure contexts + if (!$this->request->isUserAgent([Request::USER_AGENT_CHROME, Request::USER_AGENT_ANDROID_MOBILE_CHROME])) { $response->addHeader('Clear-Site-Data', '"cache", "storage"'); } diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 7d82e256c1751..b427972e1adfd 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -143,8 +143,9 @@ public function testLogoutWithoutToken() { ->with('nc_token') ->willReturn(null); $this->request - ->method('getServerProtocol') - ->willReturn('https'); + ->expects($this->once()) + ->method('isUserAgent') + ->willReturn(false); $this->config ->expects($this->never()) ->method('deleteUserValue'); @@ -159,6 +160,26 @@ public function testLogoutWithoutToken() { $this->assertEquals($expected, $this->loginController->logout()); } + public function testLogoutNoClearSiteData() { + $this->request + ->expects($this->once()) + ->method('getCookie') + ->with('nc_token') + ->willReturn(null); + $this->request + ->expects($this->once()) + ->method('isUserAgent') + ->willReturn(true); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('core.login.showLoginForm') + ->willReturn('/login'); + + $expected = new RedirectResponse('/login'); + $this->assertEquals($expected, $this->loginController->logout()); + } + public function testLogoutWithToken() { $this->request ->expects($this->once()) @@ -167,8 +188,8 @@ public function testLogoutWithToken() { ->willReturn('MyLoginToken'); $this->request ->expects($this->once()) - ->method('getServerProtocol') - ->willReturn('https'); + ->method('isUserAgent') + ->willReturn(false); $user = $this->createMock(IUser::class); $user ->expects($this->once()) From 7e7a4ddd5c0b748a2016efc72718cd9d01cc67db Mon Sep 17 00:00:00 2001 From: Gaspard d'Hautefeuille Date: Thu, 4 Jan 2024 22:35:44 +0200 Subject: [PATCH 2/4] Keep https check https://github.com/nextcloud/server/issues/41196 + keep https check Co-authored-by: Louis Signed-off-by: Gaspard d'Hautefeuille --- core/Controller/LoginController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index fb528839ce786..beeb2034fb7cf 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -106,7 +106,10 @@ public function logout() { $this->session->set('clearingExecutionContexts', '1'); $this->session->close(); - if (!$this->request->isUserAgent([Request::USER_AGENT_CHROME, Request::USER_AGENT_ANDROID_MOBILE_CHROME])) { + if ( + $this->request->getServerProtocol() === 'https' && + !$this->request->isUserAgent([Request::USER_AGENT_CHROME, Request::USER_AGENT_ANDROID_MOBILE_CHROME]) + ) { $response->addHeader('Clear-Site-Data', '"cache", "storage"'); } From c1ef86cbddacaa7aff19914aaf867c0e8b2bf928 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 8 Jan 2024 17:30:29 +0100 Subject: [PATCH 3/4] Fix tests after slow logout fix Signed-off-by: Louis Chemineau --- tests/Core/Controller/LoginControllerTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index b427972e1adfd..88bff12e233da 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -142,10 +142,13 @@ public function testLogoutWithoutToken() { ->method('getCookie') ->with('nc_token') ->willReturn(null); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $this->request ->expects($this->once()) ->method('isUserAgent') - ->willReturn(false); + ->willReturn(true); $this->config ->expects($this->never()) ->method('deleteUserValue'); From 3decdd9b1936c7ce37070c64e4b9193a97873ec0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Jan 2024 15:58:02 +0100 Subject: [PATCH 4/4] fix(tests): Fix remaining tests Signed-off-by: Joas Schilling --- tests/Core/Controller/LoginControllerTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 88bff12e233da..c17f307d4c79d 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -148,7 +148,7 @@ public function testLogoutWithoutToken() { $this->request ->expects($this->once()) ->method('isUserAgent') - ->willReturn(true); + ->willReturn(false); $this->config ->expects($this->never()) ->method('deleteUserValue'); @@ -169,6 +169,9 @@ public function testLogoutNoClearSiteData() { ->method('getCookie') ->with('nc_token') ->willReturn(null); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $this->request ->expects($this->once()) ->method('isUserAgent') @@ -189,6 +192,9 @@ public function testLogoutWithToken() { ->method('getCookie') ->with('nc_token') ->willReturn('MyLoginToken'); + $this->request + ->method('getServerProtocol') + ->willReturn('https'); $this->request ->expects($this->once()) ->method('isUserAgent')