Merge pull request #110 from nextcloud/stable9-soften-policy-if-no-cookie-is-sent

Soften the cookie check if no cookies are sent

lib/private/appframework/http/request.php

@@ -468,12 +468,16 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	}
 
 	/**
-	 * Checks if the strict cookie has been sent with the request
+	 * Checks if the strict cookie has been sent with the request if the request
+	 * is including any cookies.
 	 *
 	 * @return bool
 	 * @since 9.1.0
 	 */
 	public function passesStrictCookieCheck() {
+		if(count($this->cookies) === 0) {
+			return true;
+		}
 		if($this->getCookie('nc_sameSiteCookiestrict') === 'true'
 			&& $this->passesLaxCookieCheck()) {
 			return true;
@@ -483,12 +487,17 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	}
 
 	/**
-	 * Checks if the lax cookie has been sent with the request
+	 * Checks if the lax cookie has been sent with the request if the request
+	 * is including any cookies.
 	 *
 	 * @return bool
 	 * @since 9.1.0
 	 */
 	public function passesLaxCookieCheck() {
+		if(count($this->cookies) === 0) {
+			return true;
+		}
+
 		if($this->getCookie('nc_sameSiteCookielax') === 'true') {
 			return true;
 		}

lib/public/irequest.php

@@ -144,18 +144,20 @@ interface IRequest {
 	public function passesCSRFCheck();
 
 	/**
-	* Checks if the strict cookie has been sent with the request
+	 * Checks if the strict cookie has been sent with the request if the request
-	*
+	 * is including any cookies.
-	* @return bool
+	 *
-	* @since 9.0.0
+	 * @return bool
-	*/
+	 * @since 9.0.0
+	 */
 	public function passesStrictCookieCheck();
 
 	/**
-	 * Checks if the lax cookie has been sent with the request
+	 * Checks if the lax cookie has been sent with the request if the request
+	 * is including any cookies.
 	 *
 	 * @return bool
-	 * @since 9.1.0
+	 * @since 9.0.0
 	 */
 	public function passesLaxCookieCheck();
 

tests/lib/appframework/http/RequestTest.php

@@ -1404,7 +1404,7 @@ class RequestTest extends \Test\TestCase {
 		$this->assertTrue($request->passesCSRFCheck());
 	}
 
-	public function testFailsCSRFCheckWithGetAndWithoutCookies() {
+	public function testPassesCSRFCheckWithGetAndWithoutCookies() {
 		/** @var Request $request */
 		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
 			->setMethods(['getScriptName'])
@@ -1421,13 +1421,14 @@ class RequestTest extends \Test\TestCase {
 			])
 			->getMock();
 		$this->csrfTokenManager
-			->expects($this->never())
+			->expects($this->once())
-			->method('isTokenValid');
+			->method('isTokenValid')
+			->willReturn(true);
 
-		$this->assertFalse($request->passesCSRFCheck());
+		$this->assertTrue($request->passesCSRFCheck());
 	}
 
-	public function testFailsCSRFCheckWithPostAndWithoutCookies() {
+	public function testPassesCSRFCheckWithPostAndWithoutCookies() {
 		/** @var Request $request */
 		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
 			->setMethods(['getScriptName'])
@@ -1444,13 +1445,14 @@ class RequestTest extends \Test\TestCase {
 			])
 			->getMock();
 		$this->csrfTokenManager
-			->expects($this->never())
+			->expects($this->once())
-			->method('isTokenValid');
+			->method('isTokenValid')
+			->willReturn(true);
 
-		$this->assertFalse($request->passesCSRFCheck());
+		$this->assertTrue($request->passesCSRFCheck());
 	}
 
-	public function testFailsCSRFCheckWithHeaderAndWithoutCookies() {
+	public function testPassesCSRFCheckWithHeaderAndWithoutCookies() {
 		/** @var Request $request */
 		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
 			->setMethods(['getScriptName'])
@@ -1467,10 +1469,11 @@ class RequestTest extends \Test\TestCase {
 			])
 			->getMock();
 		$this->csrfTokenManager
-			->expects($this->never())
+			->expects($this->once())
-			->method('isTokenValid');
+			->method('isTokenValid')
+			->willReturn(true);
 
-		$this->assertFalse($request->passesCSRFCheck());
+		$this->assertTrue($request->passesCSRFCheck());
 	}
 
 	public function testFailsCSRFCheckWithHeaderAndNotAllChecksPassing() {
@@ -1523,6 +1526,32 @@ class RequestTest extends \Test\TestCase {
 		$this->assertTrue($request->passesStrictCookieCheck());
 	}
 
+	public function testFailsSRFCheckWithPostAndWithCookies() {
+		/** @var Request $request */
+		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+			->setMethods(['getScriptName'])
+			->setConstructorArgs([
+				[
+					'post' => [
+						'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+					],
+					'cookies' => [
+						'foo' => 'bar',
+					],
+				],
+				$this->secureRandom,
+				$this->config,
+				$this->csrfTokenManager,
+				$this->stream
+			])
+			->getMock();
+		$this->csrfTokenManager
+			->expects($this->never())
+			->method('isTokenValid');
+
+		$this->assertFalse($request->passesCSRFCheck());
+	}
+
 	public function testFailStrictCookieCheckWithOnlyLaxCookie() {
 		/** @var Request $request */
 		$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')