diff options
| -rw-r--r-- | lib/private/AppFramework/Http/Request.php | 17 | ||||
| -rw-r--r-- | tests/lib/AppFramework/Http/RequestTest.php | 78 |
2 files changed, 92 insertions, 3 deletions
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 8fb19f2d9b2..679ee5bc27c 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -486,6 +486,19 @@ class Request implements \ArrayAccess, \Countable, IRequest { } /** + * Whether the cookie checks are required + * + * @return bool + */ + private function cookieCheckRequired() { + if($this->getCookie(session_name()) === null && $this->getCookie('oc_token') === null) { + return false; + } + + return true; + } + + /** * Checks if the strict cookie has been sent with the request if the request * is including any cookies. * @@ -493,7 +506,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { * @since 9.1.0 */ public function passesStrictCookieCheck() { - if(count($this->cookies) === 0) { + if(!$this->cookieCheckRequired()) { return true; } if($this->getCookie('nc_sameSiteCookiestrict') === 'true' @@ -511,7 +524,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { * @since 9.1.0 */ public function passesLaxCookieCheck() { - if(count($this->cookies) === 0) { + if(!$this->cookieCheckRequired()) { return true; } if($this->getCookie('nc_sameSiteCookielax') === 'true') { diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index 8df81afeb3b..c090892cf7b 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -1485,6 +1485,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookiestrict' => 'true', ], ], @@ -1511,6 +1512,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookiestrict' => 'true', 'nc_sameSiteCookielax' => 'true', ], @@ -1525,7 +1527,76 @@ class RequestTest extends \Test\TestCase { $this->assertTrue($request->passesStrictCookieCheck()); } - public function testFailsSRFCheckWithPostAndWithCookies() { + public function testPassesStrictCookieCheckWithRandomCookies() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', + ], + 'cookies' => [ + 'RandomCookie' => 'asdf', + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ]) + ->getMock(); + + $this->assertTrue($request->passesStrictCookieCheck()); + } + + public function testFailsStrictCookieCheckWithSessionCookie() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', + ], + 'cookies' => [ + session_name() => 'asdf', + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ]) + ->getMock(); + + $this->assertFalse($request->passesStrictCookieCheck()); + } + + public function testFailsStrictCookieCheckWithRememberMeCookie() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', + ], + 'cookies' => [ + 'oc_token' => 'asdf', + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ]) + ->getMock(); + + $this->assertFalse($request->passesStrictCookieCheck()); + } + + public function testFailsCSRFCheckWithPostAndWithCookies() { /** @var Request $request */ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') ->setMethods(['getScriptName']) @@ -1535,6 +1606,7 @@ class RequestTest extends \Test\TestCase { 'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'foo' => 'bar', ], ], @@ -1561,6 +1633,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookielax' => 'true', ], ], @@ -1584,6 +1657,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookiestrict' => 'true', ], ], @@ -1607,6 +1681,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookielax' => 'true', ], ], @@ -1630,6 +1705,7 @@ class RequestTest extends \Test\TestCase { 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'cookies' => [ + session_name() => 'asdf', 'nc_sameSiteCookiestrict' => 'true', ], ], |