Merge pull request #797 from nextcloud/only-match-for-auth-cookie
Match only for actual session cookie
This commit is contained in:
commit
f9cea0b582
|
@ -484,6 +484,19 @@ class Request implements \ArrayAccess, \Countable, IRequest {
|
|||
return $this->csrfTokenManager->isTokenValid($token);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
|
@ -492,7 +505,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'
|
||||
|
@ -510,7 +523,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') {
|
||||
|
|
|
@ -1469,6 +1469,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookiestrict' => 'true',
|
||||
],
|
||||
],
|
||||
|
@ -1495,6 +1496,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookiestrict' => 'true',
|
||||
'nc_sameSiteCookielax' => 'true',
|
||||
],
|
||||
|
@ -1509,7 +1511,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'])
|
||||
|
@ -1519,6 +1590,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'foo' => 'bar',
|
||||
],
|
||||
],
|
||||
|
@ -1545,6 +1617,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookielax' => 'true',
|
||||
],
|
||||
],
|
||||
|
@ -1568,6 +1641,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookiestrict' => 'true',
|
||||
],
|
||||
],
|
||||
|
@ -1591,6 +1665,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookielax' => 'true',
|
||||
],
|
||||
],
|
||||
|
@ -1614,6 +1689,7 @@ class RequestTest extends \Test\TestCase {
|
|||
'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
|
||||
],
|
||||
'cookies' => [
|
||||
session_name() => 'asdf',
|
||||
'nc_sameSiteCookiestrict' => 'true',
|
||||
],
|
||||
],
|
||||
|
|
Loading…
Reference in New Issue