From 16c142c11d6022143cc2b6a4b0d09e96990671f5 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Jun 2021 10:20:49 +0200 Subject: [PATCH 1/5] Apply CSP rules for onlyoffice and richdocuments --- changelog/unreleased/enhancement-oc10-web-csp | 5 + .../lib/Controller/FilesController.php | 101 ++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/enhancement-oc10-web-csp diff --git a/changelog/unreleased/enhancement-oc10-web-csp b/changelog/unreleased/enhancement-oc10-web-csp new file mode 100644 index 00000000000..854e404bb7e --- /dev/null +++ b/changelog/unreleased/enhancement-oc10-web-csp @@ -0,0 +1,5 @@ +Enhancement: Content Security Policy for known iframe integrations + +We added CSP rules for allowing iframe integrations of the onlyoffice and richdocuments documentservers. + +https://github.com/owncloud/web/pull/5420 diff --git a/packages/web-integration-oc10/lib/Controller/FilesController.php b/packages/web-integration-oc10/lib/Controller/FilesController.php index 31a1dae49f3..963baa156d8 100644 --- a/packages/web-integration-oc10/lib/Controller/FilesController.php +++ b/packages/web-integration-oc10/lib/Controller/FilesController.php @@ -28,7 +28,7 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\Response; -use OCP\ILogger; +use OCP\IConfig; use OCP\IRequest; /** @@ -39,20 +39,20 @@ class FilesController extends Controller { /** - * @var ILogger + * @var IConfig */ - private $logger; + private $config; /** * FilesController constructor. * * @param string $appName * @param IRequest $request - * @param ILogger $logger + * @param IConfig $config */ - public function __construct(string $appName, IRequest $request, ILogger $logger) { + public function __construct(string $appName, IRequest $request, IConfig $config) { parent::__construct($appName, $request); - $this->logger = $logger; + $this->config = $config; } /** @@ -104,11 +104,21 @@ public function getFile(string $path): Response { 'Expires' => 'Wed, 11 Jan 1984 05:00:00 GMT', 'X-Frame-Options' => 'DENY' ]); - if (\strpos($path, "index.html") === 0 || \strpos($path, "oidc-callback.html") === 0 || \strpos($path, "oidc-silent-redirect.html") === 0) { + if (\strpos($path, "oidc-callback.html") === 0 || \strpos($path, "oidc-silent-redirect.html") === 0) { $csp = new ContentSecurityPolicy(); $csp->allowInlineScript(true); $response->setContentSecurityPolicy($csp); } + if (\strpos($path, "index.html") === 0) { + $csp = new ContentSecurityPolicy(); + $csp->allowInlineScript(true); + + // for now we set CSP rules manually, until we have sufficient requirements for a generic solution. + $csp = $this->applyCSPOnlyOffice($csp); + $csp = $this->applyCSPRichDocuments($csp); + + $response->setContentSecurityPolicy($csp); + } return $response; } @@ -117,4 +127,81 @@ private function getMimeType(string $filename): string { $mimeTypes = Mimetypes::getInstance(); return $mimeTypes->fromFilename($filename); } + + private function applyCSPOnlyOffice(ContentSecurityPolicy $csp): ContentSecurityPolicy { + $documentServerUrl = $this->extractDomain($this->getOnlyOfficeDocumentServerUrl()); + if (!empty($documentServerUrl)) { + $csp->addAllowedScriptDomain($documentServerUrl); + $csp->addAllowedFrameDomain($documentServerUrl); + } + return $csp; + } + + /** + * Extracts the onlyoffice document server URL from the app-config or system-config, in the same manner + * like the onlyoffice connector app: + * - https://github.com/ONLYOFFICE/onlyoffice-owncloud/blob/34f69c833ee4b00880d538aed1ecc48025ac8791/lib/appconfig.php#L379 + * - https://github.com/ONLYOFFICE/onlyoffice-owncloud/blob/34f69c833ee4b00880d538aed1ecc48025ac8791/lib/appconfig.php#L278 + * + * @return string + */ + private function getOnlyOfficeDocumentServerUrl(): string { + $appName = 'onlyoffice'; + $documentServerKey = 'DocumentServerUrl'; + $documentServerUrl = $this->config->getAppValue($appName, $documentServerKey); + if (!empty($documentServerUrl)) { + return $documentServerUrl; + } + $documentServerUrl = $this->config->getSystemValue($documentServerKey); + if (!empty($documentServerUrl)) { + return $documentServerUrl; + } + $onlyOfficeConfig = $this->config->getSystemValue($appName); + if (\is_array($onlyOfficeConfig) && \array_key_exists($documentServerKey, $onlyOfficeConfig)) { + return $onlyOfficeConfig[$documentServerKey]; + } + return ""; + } + + private function applyCSPRichDocuments(ContentSecurityPolicy $csp): ContentSecurityPolicy { + $documentServerUrl = $this->extractDomain($this->getRichDocumentsServerUrl()); + if (!empty($documentServerUrl)) { + $csp->addAllowedFrameDomain($documentServerUrl); + } + return $csp; + } + + /** + * Extracts the richdocuments document server URL from the app-config, in the same manner like + * the richdocuments app: + * - https://github.com/owncloud/richdocuments/blob/9a23f426048c540793fc16119f71a44c26077f16/lib/Controller/DocumentController.php#L122 + * - https://github.com/owncloud/richdocuments/blob/9a23f426048c540793fc16119f71a44c26077f16/lib/Controller/DocumentController.php#L393 + * + * @return string + */ + private function getRichDocumentsServerUrl(): string { + $appName = 'richdocuments'; + $documentServerKey = 'wopi_url'; + $documentServerUrl = $this->config->getAppValue($appName, $documentServerKey); + if (!empty($documentServerUrl)) { + return $documentServerUrl; + } + return ""; + } + + /** + * Extracts the domain part from a url. + * + * @param string $url + * @return string + */ + private function extractDomain(string $url): string { + $parsedUrl = \parse_url($url); + if (empty($parsedUrl['host'])) { + return ""; + } + return (isset($parsedUrl['scheme']) ? $parsedUrl['scheme'] . '://' : '') + . $parsedUrl['host'] + . (isset($parsedUrl['port']) ? ':' . $parsedUrl['port'] : ''); + } } From b87c3a4ec7e45a85f8b2a5a74e92a6f0cb8163ef Mon Sep 17 00:00:00 2001 From: Sergey Linnik Date: Wed, 30 Jun 2021 18:13:50 +0300 Subject: [PATCH 2/5] Allow onlyoffice on the same domain --- .../lib/Controller/FilesController.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/web-integration-oc10/lib/Controller/FilesController.php b/packages/web-integration-oc10/lib/Controller/FilesController.php index 963baa156d8..f041ec9ad73 100644 --- a/packages/web-integration-oc10/lib/Controller/FilesController.php +++ b/packages/web-integration-oc10/lib/Controller/FilesController.php @@ -129,11 +129,14 @@ private function getMimeType(string $filename): string { } private function applyCSPOnlyOffice(ContentSecurityPolicy $csp): ContentSecurityPolicy { - $documentServerUrl = $this->extractDomain($this->getOnlyOfficeDocumentServerUrl()); + $ooUrl = $this->getOnlyOfficeDocumentServerUrl(); + $documentServerUrl = $this->extractDomain($ooUrl); if (!empty($documentServerUrl)) { $csp->addAllowedScriptDomain($documentServerUrl); $csp->addAllowedFrameDomain($documentServerUrl); - } + } else if (!empty($ooUrl)) { + $csp->addAllowedFrameDomain("'self'"); + } return $csp; } From 0ccf8c508dc7170792447e9ef17dcfec952bfe0c Mon Sep 17 00:00:00 2001 From: Sergey Linnik Date: Thu, 1 Jul 2021 17:46:15 +0300 Subject: [PATCH 3/5] getting onlyoffice address from the app --- .../lib/Controller/FilesController.php | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/packages/web-integration-oc10/lib/Controller/FilesController.php b/packages/web-integration-oc10/lib/Controller/FilesController.php index f041ec9ad73..5b022d9a23a 100644 --- a/packages/web-integration-oc10/lib/Controller/FilesController.php +++ b/packages/web-integration-oc10/lib/Controller/FilesController.php @@ -141,29 +141,16 @@ private function applyCSPOnlyOffice(ContentSecurityPolicy $csp): ContentSecurity } /** - * Extracts the onlyoffice document server URL from the app-config or system-config, in the same manner - * like the onlyoffice connector app: - * - https://github.com/ONLYOFFICE/onlyoffice-owncloud/blob/34f69c833ee4b00880d538aed1ecc48025ac8791/lib/appconfig.php#L379 - * - https://github.com/ONLYOFFICE/onlyoffice-owncloud/blob/34f69c833ee4b00880d538aed1ecc48025ac8791/lib/appconfig.php#L278 + * Extracts the onlyoffice document server URL from the app * * @return string */ private function getOnlyOfficeDocumentServerUrl(): string { - $appName = 'onlyoffice'; - $documentServerKey = 'DocumentServerUrl'; - $documentServerUrl = $this->config->getAppValue($appName, $documentServerKey); - if (!empty($documentServerUrl)) { - return $documentServerUrl; - } - $documentServerUrl = $this->config->getSystemValue($documentServerKey); - if (!empty($documentServerUrl)) { - return $documentServerUrl; - } - $onlyOfficeConfig = $this->config->getSystemValue($appName); - if (\is_array($onlyOfficeConfig) && \array_key_exists($documentServerKey, $onlyOfficeConfig)) { - return $onlyOfficeConfig[$documentServerKey]; + if (!class_exists("\OCA\Onlyoffice\AppConfig")) { + return ""; } - return ""; + $onlyofficeConfig = new \OCA\Onlyoffice\AppConfig("onlyoffice"); + return $onlyofficeConfig->GetDocumentServerUrl(); } private function applyCSPRichDocuments(ContentSecurityPolicy $csp): ContentSecurityPolicy { From 0ca7858c43ed7eb83d367951ee5e707c03a0e0b9 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 8 Jul 2021 16:01:18 +0200 Subject: [PATCH 4/5] Use richdocuments app config for querying wopi url --- .../lib/Controller/FilesController.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/web-integration-oc10/lib/Controller/FilesController.php b/packages/web-integration-oc10/lib/Controller/FilesController.php index 5b022d9a23a..a35a8b66ca6 100644 --- a/packages/web-integration-oc10/lib/Controller/FilesController.php +++ b/packages/web-integration-oc10/lib/Controller/FilesController.php @@ -149,7 +149,7 @@ private function getOnlyOfficeDocumentServerUrl(): string { if (!class_exists("\OCA\Onlyoffice\AppConfig")) { return ""; } - $onlyofficeConfig = new \OCA\Onlyoffice\AppConfig("onlyoffice"); + $onlyofficeConfig = \OC::$server->query(\OCA\Onlyoffice\AppConfig::class); return $onlyofficeConfig->GetDocumentServerUrl(); } @@ -168,15 +168,14 @@ private function applyCSPRichDocuments(ContentSecurityPolicy $csp): ContentSecur * - https://github.com/owncloud/richdocuments/blob/9a23f426048c540793fc16119f71a44c26077f16/lib/Controller/DocumentController.php#L393 * * @return string + * @throws \OCP\AppFramework\QueryException */ private function getRichDocumentsServerUrl(): string { - $appName = 'richdocuments'; - $documentServerKey = 'wopi_url'; - $documentServerUrl = $this->config->getAppValue($appName, $documentServerKey); - if (!empty($documentServerUrl)) { - return $documentServerUrl; + if (!class_exists("\OCA\Richdocuments\AppConfig")) { + return ""; } - return ""; + $richdocumentsConfig = \OC::$server->query(\OCA\Richdocuments\AppConfig::class); + return $richdocumentsConfig->getAppValue('wopi_url'); } /** From 47bd0e94ec1610731ffaadfc795af77ceb6f032e Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 8 Jul 2021 17:18:44 +0200 Subject: [PATCH 5/5] Fix document server loading for richdocuments and onlyoffice --- .../lib/Controller/FilesController.php | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/packages/web-integration-oc10/lib/Controller/FilesController.php b/packages/web-integration-oc10/lib/Controller/FilesController.php index a35a8b66ca6..b5024917266 100644 --- a/packages/web-integration-oc10/lib/Controller/FilesController.php +++ b/packages/web-integration-oc10/lib/Controller/FilesController.php @@ -23,6 +23,7 @@ use GuzzleHttp\Mimetypes; use OC\AppFramework\Http; +use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\DataDisplayResponse; @@ -42,17 +43,23 @@ class FilesController extends Controller { * @var IConfig */ private $config; + /** + * @var IAppManager + */ + private $appManager; - /** - * FilesController constructor. - * - * @param string $appName - * @param IRequest $request - * @param IConfig $config - */ - public function __construct(string $appName, IRequest $request, IConfig $config) { + /** + * FilesController constructor. + * + * @param string $appName + * @param IRequest $request + * @param IConfig $config + * @param IAppManager $appManager + */ + public function __construct(string $appName, IRequest $request, IConfig $config, IAppManager $appManager) { parent::__construct($appName, $request); $this->config = $config; + $this->appManager = $appManager; } /** @@ -144,8 +151,12 @@ private function applyCSPOnlyOffice(ContentSecurityPolicy $csp): ContentSecurity * Extracts the onlyoffice document server URL from the app * * @return string + * @throws \OCP\AppFramework\QueryException */ private function getOnlyOfficeDocumentServerUrl(): string { + if (!$this->isAppEnabled("onlyoffice")) { + return ""; + } if (!class_exists("\OCA\Onlyoffice\AppConfig")) { return ""; } @@ -171,10 +182,16 @@ private function applyCSPRichDocuments(ContentSecurityPolicy $csp): ContentSecur * @throws \OCP\AppFramework\QueryException */ private function getRichDocumentsServerUrl(): string { + if (!$this->isAppEnabled("richdocuments")) { + return ""; + } if (!class_exists("\OCA\Richdocuments\AppConfig")) { return ""; } $richdocumentsConfig = \OC::$server->query(\OCA\Richdocuments\AppConfig::class); + if (empty($richdocumentsConfig)) { + return ""; + } return $richdocumentsConfig->getAppValue('wopi_url'); } @@ -193,4 +210,17 @@ private function extractDomain(string $url): string { . $parsedUrl['host'] . (isset($parsedUrl['port']) ? ':' . $parsedUrl['port'] : ''); } + + /** + * Checks whether the given app is installed and enabled. + * + * @param string $appName + * @return bool + */ + private function isAppEnabled(string $appName): bool { + if (!$this->appManager->isInstalled($appName)) { + return false; + } + return $this->appManager->isEnabledForUser($appName); + } }