Fix nextcloud#41210 to allow non Same-Site Cookies set on first request

Signed-off-by: Thomas Pointhuber <thomas.pointhuber@gmx.at>
pointhi · Mar 29, 2024 · 37f6382 · 37f6382
1 parent a40838b
commit 37f6382
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 1 deletion.
diff --git a/lib/base.php b/lib/base.php
@@ -563,7 +563,7 @@ private static function performSameSiteCookieProtection(\OCP\IConfig $config): v
 			return;
 		}
 
-		if (count($_COOKIE) > 0) {
+		if (count($_COOKIE) > 0 && (isset($_COOKIE['nc_sameSiteCookielax']) || isset($_COOKIE['nc_sameSiteCookiestrict']))) {
 			$requestUri = $request->getScriptName();
 			$processingScript = explode('/', $requestUri);
 			$processingScript = $processingScript[count($processingScript) - 1];

diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php
@@ -1855,6 +1855,87 @@ public function testPassesCSRFCheckWithHeaderAndWithoutCookies() {
 		$this->assertTrue($request->passesCSRFCheck());
 	}
 
+	public function testPassesCSRFCheckWithGetAndWithoutCSRFCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'get' => [
+						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'some_already_set_cookie' => 'true',
+					],
+				],
+				$this->requestId,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->once())
+			->method('isTokenValid')
+			->willReturn(true);
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithPostAndWithoutCSRFCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'post' => [
+						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'some_already_set_cookie' => 'true',
+					],
+				],
+				$this->requestId,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->once())
+			->method('isTokenValid')
+			->willReturn(true);
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
+	public function testPassesCSRFCheckWithHeaderAndWithoutCSRFCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'server' => [
+						'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'some_already_set_cookie' => 'true',
+					],
+				],
+				$this->requestId,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->once())
+			->method('isTokenValid')
+			->willReturn(true);
+
+		$this->assertTrue($request->passesCSRFCheck());
+	}
+
 	public function testFailsCSRFCheckWithHeaderAndNotAllChecksPassing() {
 		/** @var Request $request */
 		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')