diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index af3993df8bddc..fea68e9cfda90 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -451,8 +451,10 @@ protected function decodeContent() { /** - * Checks if the CSRF check was correct - * @return bool true if CSRF check passed + * Checks if the request passes the CSRF checks. + * + * A request must always pass the strict cookie check, unless it has the OCS-APIRequest set or no session (@see cookieCheckRequired). + * If the OCS-APIRequest is set or a valid CSRF token is sent the check will succeed. */ public function passesCSRFCheck(): bool { if ($this->csrfTokenManager === null) { @@ -463,6 +465,10 @@ public function passesCSRFCheck(): bool { return false; } + if ($this->getHeader('OCS-APIRequest') !== '') { + return true; + } + if (isset($this->items['get']['requesttoken'])) { $token = $this->items['get']['requesttoken']; } elseif (isset($this->items['post']['requesttoken'])) { @@ -481,17 +487,12 @@ public function passesCSRFCheck(): bool { /** * Whether the cookie checks are required * + * In case the OCS-APIRequest header is set or the user has no session, we don't need to check the cookies because the client is not a browser and thus doesn't need CSRF checks. + * * @return bool */ private function cookieCheckRequired(): bool { - if ($this->getHeader('OCS-APIREQUEST')) { - return false; - } - if ($this->getCookie(session_name()) === null && $this->getCookie('nc_token') === null) { - return false; - } - - return true; + return $this->getHeader('OCS-APIRequest') === '' && ($this->getCookie(session_name()) !== null || $this->getCookie('nc_token') !== null); } /** @@ -532,11 +533,7 @@ public function passesStrictCookieCheck(): bool { } $cookieName = $this->getProtectedCookieName('nc_sameSiteCookiestrict'); - if ($this->getCookie($cookieName) === 'true' - && $this->passesLaxCookieCheck()) { - return true; - } - return false; + return $this->getCookie($cookieName) === 'true' && $this->passesLaxCookieCheck(); } /** @@ -552,10 +549,7 @@ public function passesLaxCookieCheck(): bool { } $cookieName = $this->getProtectedCookieName('nc_sameSiteCookielax'); - if ($this->getCookie($cookieName) === 'true') { - return true; - } - return false; + return $this->getCookie($cookieName) === 'true'; } diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index 2af5d3ef18af8..f6e75a26db6a1 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -2261,4 +2261,24 @@ public function testPassesCSRFCheckWithoutTokenFail() { $this->assertFalse($request->passesCSRFCheck()); } + + public function testPassesCSRFCheckWithOCSAPIRequestHeader() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_OCS_APIREQUEST' => 'true', + ], + ], + $this->requestId, + $this->config, + $this->csrfTokenManager, + $this->stream + ]) + ->getMock(); + + $this->assertTrue($request->passesCSRFCheck()); + } }