Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure "allowed domains" for CORS on DAV #40537

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
'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',
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
176 changes: 176 additions & 0 deletions apps/dav/lib/Connector/Sabre/CorsPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
<?php
/**
* @copyright Copyright (c) 2018, ownCloud GmbH
*
* @author Noveen Sachdeva <[email protected]>
* @author Ferdinand Thiessen <[email protected]>
*
* @license AGPL-3.0
*
* 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.
*
* 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, version 3,
* 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;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;

/**
* Class CorsPlugin is a plugin which adds CORS headers to the responses
*/
class CorsPlugin extends ServerPlugin {
/**
* Reference to main server object
*/
private \Sabre\DAV\Server $server;
susnux marked this conversation as resolved.
Dismissed
Show resolved Hide resolved

private bool $alreadyExecuted = false;

/**
* @param IUserSession $userSession
*/
public function __construct(
private IUserSession $userSession,
private IConfig $config,
) {
}

/**
* This initializes the plugin.
*
* This function is called by \Sabre\DAV\Server, after
* addPlugin is called.
*
* This method should set up the required event subscriptions.
*
* @param \Sabre\DAV\Server $server
* @return void
*/
public function initialize(\Sabre\DAV\Server $server) {
$this->server = $server;

$request = $this->server->httpRequest;
if (!$request->hasHeader('Origin')) {
return;
}
Comment on lines +68 to +70
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can skip this part. The test $originHeader === null on line 72 is equivalent

$originHeader = $request->getHeader('Origin');
if ($originHeader === null || $this->ignoreOriginHeader($originHeader)) {
return;
}

try {
if (Util::isSameDomain($originHeader, $request->getAbsoluteUrl())) {
return;
}
} catch (\InvalidArgumentException $e) {
\OC::$server->getLogger()->debug('Invalid origin header was passed', ['Origin' => $originHeader, 'exception' => $e]);

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OC\Server::getLogger has been marked as deprecated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
\OC::$server->getLogger()->debug('Invalid origin header was passed', ['Origin' => $originHeader, 'exception' => $e]);
\OC::$server->get(LoggerInterface::class)->debug('Invalid origin header was passed', ['origin' => $originHeader, 'exception' => $e]);

Dismissed Show dismissed Hide dismissed
return;
}

$this->server->on('beforeMethod:*', [$this, 'setCorsHeaders']);
$this->server->on('exception', [$this, 'onException']);
$this->server->on('beforeMethod:OPTIONS', [$this, 'setOptionsRequestHeaders'], 5);
}

/**
* @param \Throwable $ex The thrown exception
* @return void
*/
public function onException(\Throwable $ex) {
$this->setCorsHeaders($this->server->httpRequest, $this->server->httpResponse);
}

/**
* This method sets the cors headers for all requests
*
* @param RequestInterface $request
* @param ResponseInterface $response
* @return void
*/
public function setCorsHeaders(RequestInterface $request, ResponseInterface $response) {
$requesterDomain = $request->getHeader('Origin');
if ($requesterDomain === null) {
return;
}

if ($this->alreadyExecuted) {
return;
}

$this->alreadyExecuted = true;

// unauthenticated request shall add cors headers as well
$userId = null;
if ($this->userSession->getUser() !== null) {
$userId = $this->userSession->getUser()->getUID();
Dismissed Show dismissed Hide dismissed
}

\OC_Response::setCorsHeaders(
$response,
$userId,
$requesterDomain,
$this->config,
$this->server->getAllowedMethods($request->getPath())
);
}

/**
* Handles the OPTIONS request
*
* @param RequestInterface $request
* @param ResponseInterface $response
*
* @return false|void
* @throws \InvalidArgumentException
*/
public function setOptionsRequestHeaders(RequestInterface $request, ResponseInterface $response) {
$authorization = $request->getHeader('Authorization');
if ($authorization === null || $authorization === '') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

Suggested change
if ($authorization === null || $authorization === '') {
if (empty($authorization)) {

// Set the proper response
$response->setStatus(200);
\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
// Which we don't want here
// Hence this sendResponse
$this->server->sapi->sendResponse($response);
return false;
}
}

/**
* 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 (Buttercup and Tusk)
*
* @param string $originHeader
* @return bool
*/
public function ignoreOriginHeader($originHeader) {
if (\in_array($originHeader, ['', null, 'null'], true)) {
return true;
}
$schema = \parse_url($originHeader, PHP_URL_SCHEME);
return \in_array(\strtolower($schema), ['moz-extension', 'chrome-extension']);
}
}
1 change: 1 addition & 0 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public function createServer(string $baseUri,

// Load plugins
$server->addPlugin(new \OCA\DAV\Connector\Sabre\MaintenancePlugin($this->config, $this->l10n));
$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);
Expand Down
3 changes: 3 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
use OCA\DAV\Connector\Sabre\ChecksumUpdatePlugin;
use OCA\DAV\Connector\Sabre\CommentPropertiesPlugin;
use OCA\DAV\Connector\Sabre\CopyEtagHeaderPlugin;
use OCA\DAV\Connector\Sabre\CorsPlugin;
use OCA\DAV\Connector\Sabre\DavAclPlugin;
use OCA\DAV\Connector\Sabre\DummyGetResponsePlugin;
use OCA\DAV\Connector\Sabre\FakeLockerPlugin;
Expand Down Expand Up @@ -130,6 +131,8 @@
$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(), \OC::$server->getConfig()));

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OC\Server::getUserSession has been marked as deprecated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can change to:

Suggested change
$this->server->addPlugin(new CorsPlugin(\OC::$server->getUserSession(), \OC::$server->getConfig()));
$this->server->addPlugin(new CorsPlugin(\OC::$server->get(IUserSession), \OC::$server->get(IConfig::class)));

With:

use \OCP\IConfig;
use \OCP\IUserSession

Dismissed Show dismissed Hide dismissed

$authPlugin = new Plugin();
$authPlugin->addBackend(new PublicAuth());
$this->server->addPlugin($authPlugin);
Expand Down
Loading
Loading