upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-11 01:30:50 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 79

fix: Make CORS handling admin configurable and fix tests

Browse source 
Also make sure to only return allowed methods for DAV responses

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2023-09-18 11:15:43 +02:00
parent 0cb950e4db
commit e608e5d145
11 changed files with 552 additions and 424 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
apps/dav/composer/composer/autoload_classmap.php
View file

@ -156,6 +156,7 @@ return array(
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => $baseDir . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CommentPropertiesPlugin' => $baseDir . '/../lib/Connector/Sabre/CommentPropertiesPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CopyEtagHeaderPlugin' => $baseDir . '/../lib/Connector/Sabre/CopyEtagHeaderPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CorsPlugin' => $baseDir . '/../lib/Connector/Sabre/CorsPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\DavAclPlugin' => $baseDir . '/../lib/Connector/Sabre/DavAclPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\Directory' => $baseDir . '/../lib/Connector/Sabre/Directory.php',
'OCA\\DAV\\Connector\\Sabre\\DummyGetResponsePlugin' => $baseDir . '/../lib/Connector/Sabre/DummyGetResponsePlugin.php',

1
apps/dav/composer/composer/autoload_static.php
View file

@ -171,6 +171,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CommentPropertiesPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CommentPropertiesPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CopyEtagHeaderPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CopyEtagHeaderPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\CorsPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CorsPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\DavAclPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/DavAclPlugin.php',
'OCA\\DAV\\Connector\\Sabre\\Directory' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Directory.php',
'OCA\\DAV\\Connector\\Sabre\\DummyGetResponsePlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/DummyGetResponsePlugin.php',

90
apps/dav/lib/Connector/Sabre/CorsPlugin.php
View file

@ -1,8 +1,10 @@
<?php
/**
* @author Noveen Sachdeva <noveen.sachdeva@research.iiit.ac.in>
*
* @copyright Copyright (c) 2018, ownCloud GmbH
*
* @author Noveen Sachdeva <noveen.sachdeva@research.iiit.ac.in>
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
@ -15,12 +17,13 @@
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\Connector\Sabre;
use OCP\IConfig;
use OCP\IUserSession;
use OCP\Util;
use Sabre\DAV\ServerPlugin;
@ -33,53 +36,18 @@ use Sabre\HTTP\ResponseInterface;
class CorsPlugin extends ServerPlugin {
/**
* Reference to main server object
*
* @var \Sabre\DAV\Server
*/
private $server;
private \Sabre\DAV\Server $server;
/**
* Reference to logged in user's session
*
* @var IUserSession
*/
private $userSession;
/** @var array */
private $extraHeaders;
/**
* @var bool
*/
private $alreadyExecuted = false;
private bool $alreadyExecuted = false;
/**
* @param IUserSession $userSession
*/
public function __construct(IUserSession $userSession) {
$this->userSession = $userSession;
}
private function getExtraHeaders(RequestInterface $request) {
if ($this->extraHeaders === null) {
if ($this->userSession->getUser() === null) {
$this->extraHeaders['Access-Control-Allow-Methods'] = [
'OPTIONS',
'GET',
'HEAD',
'DELETE',
'PROPFIND',
'PUT',
'PROPPATCH',
'COPY',
'MOVE',
'REPORT',
'SEARCH',
];
} else {
$this->extraHeaders['Access-Control-Allow-Methods'] = $this->server->getAllowedMethods($request->getPath());
}
}
return $this->extraHeaders;
public function __construct(
private IUserSession $userSession,
private IConfig $config,
) {
}
/**
@ -104,7 +72,12 @@ class CorsPlugin extends ServerPlugin {
if ($this->ignoreOriginHeader($originHeader)) {
return;
}
if (Util::isSameDomain($originHeader, $request->getAbsoluteUrl())) {
try {
if (Util::isSameDomain($originHeader, $request->getAbsoluteUrl())) {
return;
}
} catch (\InvalidArgumentException $e) {
\OC::$server->getLogger()->debug('Invalid origin header was passed', ['Origin' => $originHeader, 'exception' => $e]);
return;
}
@ -125,24 +98,30 @@ class CorsPlugin extends ServerPlugin {
* @return void
*/
public function setCorsHeaders(RequestInterface $request, ResponseInterface $response) {
if ($request->getHeader('origin') === null) {
$requesterDomain = $request->getHeader('Origin');
if ($requesterDomain === null) {
return;
}
if ($this->alreadyExecuted) {
return;
}
$this->alreadyExecuted = true;
$requesterDomain = $request->getHeader('origin');
// unauthenticated request shall add cors headers as well
$userId = null;
if ($this->userSession->getUser() !== null) {
$userId = $this->userSession->getUser()->getUID();
}
$headers = \OC_Response::setCorsHeaders($userId, $requesterDomain, null, $this->getExtraHeaders($request));
foreach ($headers as $key => $value) {
$response->addHeader($key, \implode(',', $value));
}
\OC_Response::setCorsHeaders(
$response,
$userId,
$requesterDomain,
$this->config,
$this->server->getAllowedMethods($request->getPath())
);
}
/**
@ -159,7 +138,11 @@ class CorsPlugin extends ServerPlugin {
if ($authorization === null || $authorization === '') {
// Set the proper response
$response->setStatus(200);
$response = \OC_Response::setOptionsRequestHeaders($response, $this->getExtraHeaders($request));
\OC_Response::setOptionsRequestHeaders(
$response,
$this->config,
$this->server->getAllowedMethods($request->getPath())
);
// Since All OPTIONS requests are unauthorized, we will have to return false from here
// If we don't return false, due to no authorization, a 401-Unauthorized will be thrown
@ -173,8 +156,7 @@ class CorsPlugin extends ServerPlugin {
/**
* in addition to schemas used by extensions we ignore empty origin header
* values as well as 'null' which is not valid by the specification but used
* by some clients.
* @link https://github.com/owncloud/core/pull/32120#issuecomment-407008243
* by some clients (Buttercup and Tusk)
*
* @param string $originHeader
* @return bool

2
apps/dav/lib/Connector/Sabre/ServerFactory.php
View file

@ -100,7 +100,7 @@ class ServerFactory {
// Load plugins
$server->addPlugin(new \OCA\DAV\Connector\Sabre\MaintenancePlugin($this->config, $this->l10n));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\CorsPlugin($this->userSession));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\CorsPlugin($this->userSession, $this->config));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin($this->config));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin());
$server->addPlugin($authPlugin);

2
apps/dav/lib/Server.php
View file

@ -131,7 +131,7 @@ class Server {
$this->server->addPlugin(new ProfilerPlugin($this->request));
$this->server->addPlugin(new BlockLegacyClientPlugin(\OC::$server->getConfig()));
$this->server->addPlugin(new AnonymousOptionsPlugin());
$this->server->addPlugin(new CorsPlugin(\OC::$server->getUserSession()));
$this->server->addPlugin(new CorsPlugin(\OC::$server->getUserSession(), \OC::$server->getConfig()));
$authPlugin = new Plugin();
$authPlugin->addBackend(new PublicAuth());

585
apps/dav/tests/unit/Connector/Sabre/CorsPluginTest.php
View file

@ -1,13 +1,14 @@
<?php
/**
* @author Vincent Petry <pvince81@owncloud.com>
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @copyright Copyright (c) 2018, ownCloud GmbH
* @license AGPL-3.0
* @copyright Copyright (c) 2023, Nextcloud GmbH
* @license AGPL-3.0-or-later
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
* it under the terms of the GNU Affero General Public License
* as published by the Free Software Foundation,
* either version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
@ -24,352 +25,348 @@ use OCA\DAV\Connector\Sabre\CorsPlugin;
use OCP\IUserSession;
use OCP\IUser;
use OCP\IConfig;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use stdClass;
use Test\TestCase;
class CorsPluginTest extends TestCase {
/**
* @var Server
*/
private $server;
/**
* @var CorsPlugin
*/
private $plugin;
/**
* @var IUserSession | \PHPUnit\Framework\MockObject\MockObject
* @var IUserSession|MockObject
*/
private $userSession;
/**
* @var IConfig | \PHPUnit\Framework\MockObject\MockObject
* @var IConfig|MockObject
*/
private $config;
public function setUp(): void {
parent::setUp();
$this->server = new Server();
$this->server->sapi = $this->getMockBuilder(\stdClass::class)
->setMethods(['sendResponse'])
->getMock();
$this->server->httpRequest->setMethod('OPTIONS');
$this->userSession = $this->createMock(IUserSession::class);
$this->config = $this->createMock(IConfig::class);
$this->overwriteService('AllConfig', $this->config);
$this->plugin = new CorsPlugin($this->userSession);
/** @var ServerPlugin | \PHPUnit\Framework\MockObject\MockObject $extraMethodPlugin */
$extraMethodPlugin = $this->createMock(ServerPlugin::class);
$extraMethodPlugin->method('getHTTPMethods')
->with('owncloud/remote.php/dav/files/user1/target/path')
->willReturn(['EXTRA']);
$extraMethodPlugin->method('getFeatures')->willReturn([]);
$this->server->addPlugin($extraMethodPlugin);
}
public function tearDown(): void {
$this->restoreService('AllConfig');
}
public function optionsCases() {
$allowedDomains = '["https://requesterdomain.tld", "http://anotherdomain.tld"]';
$allowedHeaders = [
'OC-Checksum', 'OC-Total-Length', 'OCS-APIREQUEST', 'X-OC-Mtime',
'OC-RequestAppPassword', 'Accept',
'Authorization', 'Brief', 'Content-Length', 'Content-Range',
'Content-Type', 'Date', 'Depth', 'Destination', 'Host', 'If', 'If-Match',
'If-Modified-Since', 'If-None-Match', 'If-Range', 'If-Unmodified-Since',
'Location', 'Lock-Token', 'Overwrite', 'Prefer', 'Range', 'Schedule-Reply',
'Timeout', 'User-Agent', 'X-Expected-Entity-Length', 'Accept-Language',
'Access-Control-Request-Method', 'Access-Control-Allow-Origin', 'Cache-Control', 'ETag',
'OC-Autorename', 'OC-CalDav-Import', 'OC-Chunked', 'OC-Etag', 'OC-FileId',
'OC-LazyOps', 'OC-Total-File-Length', 'Origin', 'X-Request-ID', 'X-Requested-With'
];
$allowedMethods = [
'GET',
'OPTIONS',
'POST',
'PUT',
'DELETE',
'MKCOL',
'PROPFIND',
'PATCH',
'PROPPATCH',
'REPORT',
'HEAD',
'COPY',
'MOVE',
'EXTRA',
];
$allowedMethodsUnAuthenticated = [
'GET',
'OPTIONS',
'POST',
'PUT',
'DELETE',
'MKCOL',
'PROPFIND',
'PATCH',
'PROPPATCH',
'REPORT',
'HEAD',
'COPY',
'MOVE',
];
public function dataInit() {
return [
'OPTIONS headers' =>
[
$allowedDomains,
false,
[
'Origin' => 'https://requesterdomain.tld',
],
200,
[
'Access-Control-Allow-Headers' => \implode(',', $allowedHeaders),
'Access-Control-Allow-Origin' => '*',
'Access-Control-Allow-Methods' => \implode(',', $allowedMethodsUnAuthenticated),
],
'missing origin will be ignored' => [
[],
'',
'',
false
],
'OPTIONS headers with user' =>
[
$allowedDomains,
true,
[
'Origin' => 'https://requesterdomain.tld',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => \implode(',', $allowedHeaders),
'Access-Control-Allow-Origin' => 'https://requesterdomain.tld',
'Access-Control-Allow-Methods' => \implode(',', $allowedMethods),
],
true
],
'OPTIONS headers no user' =>
[
$allowedDomains,
'invalid null origin will be ignored' => [
['Origin' => null],
'/',
'http://cloud.example.com/',
false,
[
'Origin' => 'https://requesterdomain.tld',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => null,
'Access-Control-Allow-Origin' => null,
'Access-Control-Allow-Methods' => null,
],
true
],
'OPTIONS headers domain not allowed' =>
[
'[]',
true,
[
'Origin' => 'https://requesterdomain.tld',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => null,
'Access-Control-Allow-Origin' => null,
'Access-Control-Allow-Methods' => null,
],
true
'invalid empty origin will be ignored' => [
['Origin' => ''],
'/',
'http://cloud.example.com/',
false,
],
'OPTIONS headers not allowed but no cross-domain' =>
[
'[]',
true,
[
'Origin' => 'https://requesterdomain.tld',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => null,
'Access-Control-Allow-Origin' => null,
'Access-Control-Allow-Methods' => null,
],
true
'invalid null string origin will be ignored' => [
['Origin' => 'null'],
'/',
'http://cloud.example.com/',
false,
],
'OPTIONS headers allowed but no cross-domain' =>
[
'["currentdomain.tld:8080"]',
true,
[
'Origin' => 'https://currentdomain.tld:8080',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => null,
'Access-Control-Allow-Origin' => null,
'Access-Control-Allow-Methods' => null,
],
true
'invalid moz schema origin will be ignored' => [
['Origin' => 'moz-extension://domain.tld'],
'/',
'http://cloud.example.com/',
false,
],
'OPTIONS headers allowed, cross-domain through different port' =>
[
'["https://currentdomain.tld:8443"]',
true,
[
'Origin' => 'https://currentdomain.tld:8443',
'Authorization' => 'abc',
],
200,
[
'Access-Control-Allow-Headers' => \implode(',', $allowedHeaders),
'Access-Control-Allow-Origin' => 'https://currentdomain.tld:8443',
'Access-Control-Allow-Methods' => \implode(',', $allowedMethods),
],
true
'invalid chrome schema origin will be ignored' => [
['Origin' => 'chrome-extension://domain.tld'],
'/',
'http://cloud.example.com/',
false,
],
'no Origin header' =>
[
$allowedDomains,
'same origin will be ignored' => [
['Origin' => 'http://cloud.example.com'],
'remote.php/dav/some/service',
'http://cloud.example.com/remote.php/dav/some/service',
false,
],
'different sub domain will register' => [
['Origin' => 'http://www.example.com'],
'remote.php/dav/some/service',
'http://cloud.example.com/remote.php/dav/some/service',
true,
],
'valid origin will register' => [
['Origin' => 'http://domain.tld'],
'remote.php/dav/some/service',
'http://cloud.example.com/remote.php/dav/some/service',
true,
[
],
200,
[
'Access-Control-Allow-Headers' => null,
'Access-Control-Allow-Origin' => null,
'Access-Control-Allow-Methods' => null,
],
true
],
];
}
/**
* @dataProvider optionsCases
* @param $allowedDomains
* @param $hasUser
* @param $requestHeaders
* @param $expectedStatus
* @param array $expectedHeaders
* @param bool $expectDavHeaders
* @dataProvider dataInit
*/
public function testOptionsHeaders($allowedDomains, $hasUser, $requestHeaders, $expectedStatus, array $expectedHeaders, $expectDavHeaders = false) {
$this->server->sapi->expects($this->once())->method('sendResponse')->with($this->server->httpResponse);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('someuser');
public function testInit($requestHeaders, $requestUrl, $requestAbsolutUrl, $shouldRegister) {
$request = new \Sabre\HTTP\Request('OPTIONS', $requestUrl, $requestHeaders);
$request->setAbsoluteUrl($requestAbsolutUrl);
if ($hasUser) {
$this->userSession->method('getUser')->willReturn($user);
} else {
$this->userSession->method('getUser')->willReturn(null);
}
/**
* @var MockObject|Server
*/
$server = $this->createMock(Server::class);
$server->httpRequest = $request;
$server->expects($this->exactly($shouldRegister ? 3 : 0))->method('on');
$this->config->method('getSystemValue')->willReturn([]);
$this->config->method('getUserValue')
->with('someuser', 'core', 'domains')
->willReturn($allowedDomains);
$plugin = new CorsPlugin($this->userSession, $this->config);
$this->server->httpRequest->setHeaders($requestHeaders);
$this->server->httpRequest->setAbsoluteUrl('https://currentdomain.tld:8080/owncloud/remote.php/dav/files/user1/target/path');
$this->server->httpRequest->setUrl('/owncloud/remote.php/dav/files/user1/target/path');
$this->server->addPlugin($this->plugin);
$this->server->start();
$this->assertEquals($expectedStatus, $this->server->httpResponse->getStatus());
foreach ($expectedHeaders as $headerKey => $headerValue) {
if ($headerValue !== null) {
$this->assertTrue($this->server->httpResponse->hasHeader($headerKey), "Response header \"$headerKey\" exists");
} else {
$this->assertFalse($this->server->httpResponse->hasHeader($headerKey), "Response header \"$headerKey\" does not exist");
}
$this->assertEquals($headerValue, $this->server->httpResponse->getHeader($headerKey));
}
// if it has DAV headers, it means we did not bypass further processing
$this->assertEquals($expectDavHeaders, $this->server->httpResponse->hasHeader('DAV'));
$plugin->initialize($server);
}
/**
* @dataProvider providesOriginUrls
* @param $expectedValue
* @param $url
*/
public function testExtensionRequests($expectedValue, $url) {
$plugin = new CorsPlugin($this->createMock(IUserSession::class));
self::assertEquals($expectedValue, $plugin->ignoreOriginHeader($url));
}
public function providesOriginUrls() {
public function dataOnException() {
return [
'Firefox extension' => [true, 'moz-extension://mgmnhfbjphngabcpbpmapnnaabhnchmi/'],
'Chrome extension' => [true, 'chrome-extension://mgmnhfbjphngabcpbpmapnnaabhnchmi/'],
'Empty Origin' => [true, ''],
'Null string Origin' => [true, 'null'],
'Null Origin' => [true, null],
'plain http' => [false, 'http://example.net/'],
'with allowed domain' => [
['http://some.domain'],
true,
],
'without allowed domain' => [
['http://other.domain'],
false,
],
'without any domain' => [
[],
false,
],
];
}
public function testAuthenticatedAdditionalAllowedHeaders() {
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('someuser');
/**
* @dataProvider dataOnException
*/
public function testOnException(array $allowedDomains, bool $shouldHaveCORS) {
$request = new \Sabre\HTTP\Request('OPTIONS', 'dav/action', ['Origin' => 'http://some.domain']);
$request->setAbsoluteUrl('http://cloud.example.com/dav/action');
$this->userSession->method('getUser')->willReturn($user);
$this->server->httpRequest->setHeader('Origin', 'https://requesterdomain.tld');
$this->server->httpRequest->setUrl('/owncloud/remote.php/dav/files/user1/target/path');
$response = new \Sabre\HTTP\Response(200);
$this->config->method('getSystemValue')->withConsecutive(
['cors.allowed-domains', []],
['cors.allowed-headers', []]
)
->willReturnMap([
['cors.allowed-domains', [], []],
['cors.allowed-headers', [], ['X-Additional-Configured-Header', 'authorization']]
]);
$this->config->method('getUserValue')->willReturn('["https://requesterdomain.tld"]');
$server = $this->createMock(Server::class);
$server->httpRequest = $request;
$server->httpResponse = $response;
$this->server->addPlugin($this->plugin);
$exception = $this->createMock(\Throwable::class);
$this->plugin->setCorsHeaders($this->server->httpRequest, $this->server->httpResponse);
self::assertEquals(
'X-Additional-Configured-Header,authorization,OC-Checksum,OC-Total-Length,OCS-APIREQUEST,X-OC-Mtime,OC-RequestAppPassword,Accept,Authorization,Brief,Content-Length,Content-Range,Content-Type,Date,Depth,Destination,Host,If,If-Match,If-Modified-Since,If-None-Match,If-Range,If-Unmodified-Since,Location,Lock-Token,Overwrite,Prefer,Range,Schedule-Reply,Timeout,User-Agent,X-Expected-Entity-Length,Accept-Language,Access-Control-Request-Method,Access-Control-Allow-Origin,Cache-Control,ETag,OC-Autorename,OC-CalDav-Import,OC-Chunked,OC-Etag,OC-FileId,OC-LazyOps,OC-Total-File-Length,Origin,X-Request-ID,X-Requested-With',
$this->server->httpResponse->getHeader('Access-Control-Allow-Headers')
);
$this->config->method('getSystemValue')->willReturnCallback(fn($name, $default) => match(true) {
$name === 'cors.allowed-domains' => $allowedDomains,
default => $default,
});
$plugin = new CorsPlugin($this->userSession, $this->config);
$plugin->initialize($server);
$plugin->onException($exception);
$this->assertTrue($response->hasHeader('Access-Control-Allow-Origin') === $shouldHaveCORS);
}
public function testUnauthenticatedAdditionalAllowedHeaders() {
$this->userSession->method('getUser')->willReturn(null);
$this->server->httpRequest->setHeader('Origin', 'https://requesterdomain.tld');
public function dataSetOptionsHeaders() {
return [
'skip requests with authorization header for CSRF' => [
// headers of the request
['Authorization' => 'Basic YWxhZGRpbjpvcGVuc2VzYW1l', 'Origin' => 'http://some.domain'],
// expected return value of the function
null,
// should the response have the cors header
false,
// should the response be sent directly (meaning skipped)
false,
//response status
500,
],
'without authorization header the OPTIONS request is answered' => [
// headers of the request
['Origin' => 'http://some.domain'],
// expected return value of the function
false,
// should the response have the cors header
true,
// should the response be sent directly (meaning skipped)
true,
//response status
200,
]
];
}
$this->config->method('getSystemValue')->withConsecutive(
['cors.allowed-domains', []],
['cors.allowed-headers', []]
)
->willReturnMap([
['cors.allowed-domains', [], ['https://requesterdomain.tld']],
['cors.allowed-headers', [], ['X-Additional-Configured-Header', 'authorization']]
]);
/**
* @dataProvider dataSetOptionsHeaders
*/
public function testSetOptionsHeaders($requestHeaders, $expectedReturn, $shouldHaveCORS, $shouldSendResponse, $responseStatus) {
$request = new \Sabre\HTTP\Request('OPTIONS', 'dav/action', $requestHeaders);
$request->setAbsoluteUrl('http://cloud.example.com/dav/action');
$this->server->addPlugin($this->plugin);
$response = new \Sabre\HTTP\Response();
$this->plugin->setCorsHeaders($this->server->httpRequest, $this->server->httpResponse);
self::assertEquals(
'X-Additional-Configured-Header,authorization,OC-Checksum,OC-Total-Length,OCS-APIREQUEST,X-OC-Mtime,OC-RequestAppPassword,Accept,Authorization,Brief,Content-Length,Content-Range,Content-Type,Date,Depth,Destination,Host,If,If-Match,If-Modified-Since,If-None-Match,If-Range,If-Unmodified-Since,Location,Lock-Token,Overwrite,Prefer,Range,Schedule-Reply,Timeout,User-Agent,X-Expected-Entity-Length,Accept-Language,Access-Control-Request-Method,Access-Control-Allow-Origin,Cache-Control,ETag,OC-Autorename,OC-CalDav-Import,OC-Chunked,OC-Etag,OC-FileId,OC-LazyOps,OC-Total-File-Length,Origin,X-Request-ID,X-Requested-With',
$this->server->httpResponse->getHeader('Access-Control-Allow-Headers')
);
$server = $this->createMock(Server::class);
/** @var MockObject */
$server->sapi = $this->getMockBuilder(\stdClass::class)->addMethods(['sendResponse'])->getMock();
$server->httpRequest = $request;
$server->httpResponse = $response;
$server->sapi
->expects($shouldSendResponse ? $this->once() : $this->never())
->method('sendResponse')
->with($response);
$this->config->method('getSystemValue')->willReturnCallback(fn($name, $default) => $default);
$plugin = new CorsPlugin($this->userSession, $this->config);
$plugin->initialize($server);
$this->assertEquals($expectedReturn, $plugin->setOptionsRequestHeaders($request, $response));
$this->assertEquals($responseStatus, $response->getStatus());
$this->assertTrue($response->hasHeader('Access-Control-Allow-Origin') === $shouldHaveCORS);
}
public function dataSetCORSHeaders() {
return [
'no header on missing origin' => [
// method
'PROPFIND',
// request headers
[],
// allow users domain list
false,
// already executed
false,
// user logged in
false,
// extra headers
['PROPFIND'],
// added cors methods
null,
// the resulting cors domain
null,
],
'allowed origin will set header' => [
// method
'PROPFIND',
// request headers
['Origin' => 'http://good.example.com'],
// allow users domain list
false,
// already executed
false,
// user logged in
false,
// allowed methods
['PROPFIND', 'SEARCH'],
// added cors methods
['PROPFIND', 'SEARCH'],
// the resulting cors domain
'http://good.example.com',
],
'no header if already executed' => [
// method
'PROPFIND',
// request headers
['Origin' => 'http://good.example.com'],
// allow users domain list
false,
// already executed
true,
// user logged in
false,
// allowed methods
['PROPFIND'],
// added cors methods
null,
// the resulting cors domain
null,
],
'set header on user domain' => [
// method
'PROPFIND',
// request headers
['Origin' => 'http://users.tld'],
// allow users domain list
true,
// already executed
false,
// user logged in
true,
// allowed methods
['PROPFIND'],
// added cors methods
['PROPFIND'],
// the resulting cors domain
'http://users.tld',
],
'no header is set when user also do not match' => [
// method
'PROPFIND',
// request headers
['Origin' => 'http://some-other.tld'],
// allow users domain list
true,
// already executed
false,
// user logged in
true,
// allowed methods
['PROPFIND'],
// added cors methods
null,
// the resulting cors domain
null,
],
];
}
/**
* @dataProvider dataSetCORSHeaders
*/
public function testSetCORSHeaders(string $requestMethod, array $requestHeaders, bool $allowUserConfig, bool $alreadyExecuted, bool $hasUser, array $allowedHeaders, ?array $corsMethods, string|null $corsDomain) {
$request = new \Sabre\HTTP\Request($requestMethod, 'dav/action', $requestHeaders);
$request->setAbsoluteUrl('http://cloud.example.com/dav/action');
$response = new \Sabre\HTTP\Response();
if ($hasUser === true) {
/** @var MockObject */
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('someuser');
$this->userSession->method('getUser')->willReturn($user);
}
$this->config->method('getSystemValue')->willReturnCallback(fn($name, $default) => match(true) {
$name === 'cors.allowed-domains' => ['http://good.example.com'],
default => $default,
});
$this->config->method('getSystemValueBool')->with('cors.allow-user-domains')->willReturn($allowUserConfig);
$this->config->method('getUserValue')->with('someuser', 'core', 'cors.allowed-domains')->willReturn('["http://users.tld"]');
/** @var Server|MockObject */
$server = $this->createMock(Server::class);
$server->method('getAllowedMethods')->with('dav/action')->willReturn($allowedHeaders);
$server->httpRequest = $request;
$server->httpResponse = $response;
/** @var CorsPlugin|MockObject */
$plugin = new CorsPlugin($this->userSession, $this->config);
$plugin->initialize($server);
$plugin->setCorsHeaders($request, $response);
if ($alreadyExecuted) {
$response = new \Sabre\HTTP\Response();
$plugin->setCorsHeaders($request, $response);
}
$this->assertEquals($corsDomain, $response->getHeader('Access-Control-Allow-Origin'));
$this->assertEquals($corsMethods === null ? null : join(',', $corsMethods), $response->getHeader('Access-Control-Allow-Methods'));
}
}

117
cypress/e2e/dav.cy.ts Normal file
View file

@ -0,0 +1,117 @@
/**
* @copyright Copyright (c) 2022 John Molakvoæ <skjnldsv@protonmail.com>
*
* @author John Molakvoæ <skjnldsv@protonmail.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
describe('check if dav app supports CORS', function() {
describe('OPTIONS requests', () => {
it('set the access control header', function() {
const url = new URL(Cypress.config('baseUrl') as string)
url.pathname = 'remote.php/dav/files/admin/'
cy.request({
method: 'OPTIONS',
url: url.toString(),
headers: {
Origin: 'http://example.com',
'Access-Control-Allow-Methods': 'http://example.com',
},
}).then(response => {
expect(response.status).to.equal(200)
expect(response).to.have.property('headers')
expect(response.headers).to.have.property('access-control-allow-origin')
expect(response.headers['access-control-allow-origin']).to.equal('*')
})
})
it('does not set header if Origin is invalid', function() {
const url = new URL(Cypress.config('baseUrl') as string)
url.pathname = 'remote.php/dav/files/admin/'
cy.request({
method: 'OPTIONS',
url: url.toString(),
failOnStatusCode: false,
headers: {
Origin: 'example.com',
'Access-Control-Allow-Methods': 'http://example.com',
},
}).then(response => {
expect(response.headers).to.not.have.property('access-control-allow-origin')
})
})
})
describe('WebDAV requests', () => {
before(() => {
cy.runOccCommand('config:system:set --value "http://good.example.com" cors.allowed-domains 0')
})
after(() => {
cy.runOccCommand('config:system:delete cors.allowed-domains')
})
it('do not set CORS headers when origin is not allowed', function() {
const url = new URL(Cypress.config('baseUrl') as string)
url.pathname = 'remote.php/dav/files/admin/'
cy.request({
method: 'PROPFIND',
url: url.toString(),
failOnStatusCode: false,
headers: {
Origin: 'http://example.com',
Authorization: `Basic ${btoa('admin:admin')}`,
},
body: `<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns" xmlns:oc="http://owncloud.org/ns" xmlns:ocs="http://open-collaboration-services.org/ns">
<d:prop>
<d:getcontentlength /> <d:getcontenttype /> <d:getetag /> <d:getlastmodified /> <d:quota-available-bytes /> <d:resourcetype /> <nc:has-preview /> <nc:is-encrypted /> <nc:mount-type /> <nc:share-attributes /> <oc:comments-unread /> <oc:favorite /> <oc:fileid /> <oc:owner-display-name /> <oc:owner-id /> <oc:permissions /> <oc:share-types /> <oc:size /> <ocs:share-permissions /> <nc:system-tags />
</d:prop>
</d:propfind>`,
}).then(response => {
expect(response.status).to.equal(207)
expect(response.headers).to.not.have.property('access-control-allow-origin')
})
})
it('set CORS headers when origin is allowed', function() {
const url = new URL(Cypress.config('baseUrl') as string)
url.pathname = 'remote.php/dav/files/admin/'
cy.request({
method: 'PROPFIND',
url: url.toString(),
failOnStatusCode: false,
headers: {
Origin: 'http://good.example.com',
Authorization: `Basic ${btoa('admin:admin')}`,
},
body: `<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns" xmlns:oc="http://owncloud.org/ns" xmlns:ocs="http://open-collaboration-services.org/ns">
<d:prop>
<d:getcontentlength /> <d:getcontenttype /> <d:getetag /> <d:getlastmodified /> <d:quota-available-bytes /> <d:resourcetype /> <nc:has-preview /> <nc:is-encrypted /> <nc:mount-type /> <nc:share-attributes /> <oc:comments-unread /> <oc:favorite /> <oc:fileid /> <oc:owner-display-name /> <oc:owner-id /> <oc:permissions /> <oc:share-types /> <oc:size /> <ocs:share-permissions /> <nc:system-tags />
</d:prop>
</d:propfind>`,
}).then(response => {
expect(response.status).to.equal(207)
expect(response.headers).to.have.property('access-control-allow-origin')
expect(response.headers['access-control-allow-origin']).to.equal('http://good.example.com')
})
})
})
})

16
lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
View file

@ -8,6 +8,7 @@
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Morris Jobke <hey@morrisjobke.de>
* @author Stefan Weil <sw@weilnetz.de>
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0
*
@ -68,22 +69,21 @@ class CORSMiddleware extends Middleware {
*/
public function afterController($controller, $methodName, Response $response) {
$userId = !is_null($this->session->getUser()) ? $this->session->getUser()->getUID() : null;
$requesterDomain = $this->request->getHeader("Origin");
// only react if it's a CORS request and if the request sends origin and
// only react if it's a CORS request and if the request sends origin and user is logged in
$reflectionMethod = new ReflectionMethod($controller, $methodName);
if ($this->request->getHeader("Origin") !== null
&& $this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)
&& !is_null($userId)) {
$requesterDomain = $this->request->getHeader("Origin");
\OC_Response::setCorsHeaders($userId, $requesterDomain, $this->config);
if ($requesterDomain !== ''
&& !is_null($userId)
&& $this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) {
\OC_Response::setCorsHeaders($response, $userId, $requesterDomain, $this->config);
// allow credentials headers must not be true or CSRF is possible
// otherwise
foreach ($response->getHeaders() as $header => $value) {
if (strtolower($header) === 'access-control-allow-credentials' &&
strtolower(trim($value)) === 'true') {
$msg = 'Access-Control-Allow-Credentials must not be '.
'set to true in order to prevent CSRF';
$msg = 'Access-Control-Allow-Credentials must not be set to true in order to prevent CSRF';
throw new SecurityException($msg);
}
}

80
lib/private/legacy/OC_Response.php
View file

@ -11,6 +11,7 @@
* @author Roeland Jago Douma <roeland@famdouma.nl>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Vincent Petry <vincent@nextcloud.com>
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0
*
@ -107,10 +108,11 @@ class OC_Response {
/**
* This function adds the CORS headers if the requester domain is white-listed
*
* @param \OCP\AppFramework\Http\Response|Sabre\HTTP\ResponseInterface $response
* @param string $userId
* @param string $domain
* @param \OCP\IConfig $config
* @param array $headers
* @param \OCP\IConfig|null $config
* @param array $headers Additional CORS headers to merge when setting
*
* Format of $headers:
* Array [
@ -118,44 +120,47 @@ class OC_Response {
* "Access-Control-Allow-Origin": ["a", "b", "c"],
* "Access-Control-Allow-Methods": ["a", "b", "c"]
* ]
*
* @return array
*/
public static function setCorsHeaders($userId, $domain, \OCP\IConfig $config = null, array $headers = []) {
if ($config === null) {
public static function setCorsHeaders($response, ?string $userId, string $domain, $config = null, array|null $methods = null) {
if (is_null($config)) {
$config = \OC::$server->getConfig();
}
// first check if any of the global CORS domains matches
$globalAllowedDomains = $config->getSystemValue('cors.allowed-domains', []);
$isCorsRequest = (\is_array($globalAllowedDomains) && \in_array($domain, $globalAllowedDomains, true));
if (!$isCorsRequest && $userId !== null) {
// check if any of the user specific CORS domains matches
$allowedDomains = \json_decode($config->getUserValue($userId, 'core', 'domains'), true);
// check if user defined CORS domains are enabled
$isUserCorsEnabled = $config->getSystemValueBool('cors.allow-user-domains', false);
// if not a global CORS domain, but user defined ones are enabled, check if one matches
if (!$isCorsRequest && $isUserCorsEnabled && $userId !== null) {
$allowedDomains = \json_decode($config->getUserValue($userId, 'core', 'cors.allowed-domains'), true);
$isCorsRequest = (\is_array($allowedDomains) && \in_array($domain, $allowedDomains, true));
}
// Global or user domain matches so set headers
if ($isCorsRequest) {
// TODO: infer allowed verbs from existing known routes
$allHeaders['Access-Control-Allow-Headers'] = self::getAllowedCorsHeaders($config);
$allHeaders['Access-Control-Expose-Headers'] = self::getExposeCorsHeaders();
$allHeaders['Access-Control-Allow-Origin'] = [$domain];
$allHeaders['Access-Control-Allow-Methods'] = ['GET', 'OPTIONS', 'POST', 'PUT', 'DELETE', 'MKCOL', 'PROPFIND', 'PATCH', 'PROPPATCH', 'REPORT'];
$allHeaders = [
'Access-Control-Allow-Origin' => [$domain],
'Access-Control-Allow-Headers' => self::getAllowedCorsHeaders($config),
'Access-Control-Expose-Headers' => self::getExposeCorsHeaders(),
'Access-Control-Allow-Methods' => $methods ?? self::getAllowedCorsMethods(),
// Indicate that the response might change depending on the origin
'Vary' => ['Origin'],
];
foreach ($headers as $key => $value) {
if (\array_key_exists($key, $allHeaders)) {
$allHeaders[$key] = \array_unique(\array_merge($allHeaders[$key], $value));
}
foreach ($allHeaders as $key => $value) {
$response->addHeader($key, \join(',', $value));
}
return $allHeaders;
}
return [];
}
/**
* This function adds the CORS headers for all domains
*
* @param Sabre\HTTP\ResponseInterface $response
* @param array $headers
* @param \OCP\AppFramework\Http\Response|Sabre\HTTP\ResponseInterface $response
* @param \OCP\IConfig|null $config
* @param array $headers Additional cors headers to merge when setting
*
* Format of $headers:
* Array [
@ -164,26 +169,18 @@ class OC_Response {
* "Access-Control-Allow-Methods": ["a", "b", "c"]
* ]
*
* @param \OCP\IConfig|null $config
* @return Sabre\HTTP\ResponseInterface $response
* @return void
*/
public static function setOptionsRequestHeaders($response, $headers = [], \OCP\IConfig $config = null) {
// TODO: infer allowed verbs from existing known routes
$allHeaders['Access-Control-Allow-Headers'] = self::getAllowedCorsHeaders($config);
$allHeaders['Access-Control-Allow-Origin'] = ['*'];
$allHeaders['Access-Control-Allow-Methods'] = self::getAllowedCorsMethods();
foreach ($headers as $key => $value) {
if (\array_key_exists($key, $allHeaders)) {
$allHeaders[$key] = \array_unique(\array_merge($allHeaders[$key], $value));
}
}
public static function setOptionsRequestHeaders($response, \OCP\IConfig $config = null, ?array $methods = null) {
$allHeaders = [
'Access-Control-Allow-Headers' => self::getAllowedCorsHeaders($config),
'Access-Control-Allow-Origin' => ['*'],
'Access-Control-Allow-Methods' => $methods ?? self::getAllowedCorsMethods(),
];
foreach ($allHeaders as $key => $value) {
$response->addHeader($key, \implode(',', $value));
$response->addHeader($key, \join(',', $value));
}
return $response;
}
/**
@ -203,7 +200,10 @@ class OC_Response {
'PATCH',
'PROPPATCH',
'REPORT',
'SEARCH'
'SEARCH',
'COPY',
'MOVE',
'HEAD',
];
}

21
lib/public/Util.php
View file

@ -635,6 +635,7 @@ class Util {
*
* @param string $url full url
* @return string protocol, domain and port as string
* @throws \InvalidArgumentException On invalid URL
* @since 28.0.0
*/
public static function getFullDomain(string $url): string {
@ -647,17 +648,16 @@ class Util {
}
$protocol = \strtolower($parts['scheme']);
$host = \strtolower($parts['host']);
$port = null;
if ($protocol === 'http') {
$port = 80;
} elseif ($protocol === 'https') {
$port = 443;
} else {
throw new \InvalidArgumentException('Only http based URLs supported');
}
if (isset($parts['port']) && $port !== '') {
$port = $parts['port'];
$port = $parts['port'] ?? null;
if ($port === null || $port === '') {
if ($protocol === 'http') {
$port = 80;
} elseif ($protocol === 'https') {
$port = 443;
} else {
throw new \InvalidArgumentException('Only http based URLs supported');
}
}
return $protocol . '://' . \strtolower($host) . ':' . $port;
@ -673,6 +673,7 @@ class Util {
* @param string $url2
*
* @return bool true if both URLs have the same protocol, domain and port
* @throws \InvalidArgumentException On invalid URL
*
* @since 28.0.0
*/

61
tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
View file

@ -1,12 +1,25 @@
<?php
/**
* ownCloud - App Framework
*
* This file is licensed under the Affero General Public License version 3 or
* later. See the COPYING file.
* @copyright 2014 Bernhard Posselt <dev@bernhard-posselt.com>
*
* @author Bernhard Posselt <dev@bernhard-posselt.com>
* @copyright Bernhard Posselt 2014
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace Test\AppFramework\Middleware\Security;
@ -20,6 +33,7 @@ use OCP\AppFramework\Http\Response;
use OCP\IConfig;
use OCP\IRequest;
use OCP\IRequestId;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Security\Bruteforce\IThrottler;
use PHPUnit\Framework\MockObject\MockObject;
@ -59,17 +73,6 @@ class CORSMiddlewareTest extends \Test\TestCase {
['testSetCORSAPIHeader'],
['testSetCORSAPIHeaderAttribute'],
];
$this->session = $this->getMockBuilder('\OC\User\Session')
->disableOriginalConstructor()
->getMock();
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$userSession = $this->createMock(IUserSession::class);
$userSession->method('getUser')->willReturn($user);
$this->session = $userSession;
}
/**
@ -85,6 +88,19 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
/** @var MockObject */
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$this->session->expects($this->exactly(2))->method('getUser')->willReturn($user);
$this->config
->method('getSystemValue')
->willReturnCallback(fn (string $key, mixed $default) => match (true) {
$key === 'cors.allowed-domains' => ['http://www.test.com'],
default => $default,
});
$this->reflector->reflect($this->controller, $method);
$middleware = new CORSMiddleware(
$request,
@ -163,6 +179,19 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
/** @var MockObject */
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$this->session->expects($this->exactly(2))->method('getUser')->willReturn($user);
$this->config
->method('getSystemValue')
->willReturnCallback(fn (string $key, mixed $default) => match (true) {
$key === 'cors.allowed-domains' => ['http://www.test.com'],
default => $default,
});
$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->config);
$response = new Response();