From ca246dc35c76390e26108d3c14e640f3f2d682bf Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=B4me=20Chilliet?= Date: Thu, 29 Feb 2024 12:21:36 +0100 Subject: [PATCH] feat(settings): Migrate SSL access and URL generation check to SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 2 - .../lib/SetupChecks/HttpsUrlGeneration.php | 75 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 2 - core/js/setupchecks.js | 15 ---- core/js/tests/specs/setupchecksSpec.js | 11 --- 8 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/HttpsUrlGeneration.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 067b24592e8..9516c3cff9e 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -92,6 +92,7 @@ return array( 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => $baseDir . '/../lib/SetupChecks/ForwardedForHeaders.php', + 'OCA\\Settings\\SetupChecks\\HttpsUrlGeneration' => $baseDir . '/../lib/SetupChecks/HttpsUrlGeneration.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => $baseDir . '/../lib/SetupChecks/InternetConnectivity.php', 'OCA\\Settings\\SetupChecks\\JavaScriptModules' => $baseDir . '/../lib/SetupChecks/JavaScriptModules.php', 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 44afee35d93..745c63e7b54 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -107,6 +107,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php', 'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php', 'OCA\\Settings\\SetupChecks\\ForwardedForHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/ForwardedForHeaders.php', + 'OCA\\Settings\\SetupChecks\\HttpsUrlGeneration' => __DIR__ . '/..' . '/../lib/SetupChecks/HttpsUrlGeneration.php', 'OCA\\Settings\\SetupChecks\\InternetConnectivity' => __DIR__ . '/..' . '/../lib/SetupChecks/InternetConnectivity.php', 'OCA\\Settings\\SetupChecks\\JavaScriptModules' => __DIR__ . '/..' . '/../lib/SetupChecks/JavaScriptModules.php', 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index cfe8a306365..d8e227a0c01 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -63,6 +63,7 @@ use OCA\Settings\SetupChecks\DefaultPhoneRegionSet; use OCA\Settings\SetupChecks\EmailTestSuccessful; use OCA\Settings\SetupChecks\FileLocking; use OCA\Settings\SetupChecks\ForwardedForHeaders; +use OCA\Settings\SetupChecks\HttpsUrlGeneration; use OCA\Settings\SetupChecks\InternetConnectivity; use OCA\Settings\SetupChecks\JavaScriptModules; use OCA\Settings\SetupChecks\LegacySSEKeyFormat; @@ -188,6 +189,7 @@ class Application extends App implements IBootstrap { $context->registerSetupCheck(EmailTestSuccessful::class); $context->registerSetupCheck(FileLocking::class); $context->registerSetupCheck(ForwardedForHeaders::class); + $context->registerSetupCheck(HttpsUrlGeneration::class); $context->registerSetupCheck(InternetConnectivity::class); $context->registerSetupCheck(JavaScriptModules::class); $context->registerSetupCheck(LegacySSEKeyFormat::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 80083518385..218fa26b998 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -176,8 +176,6 @@ Raw output public function check() { return new DataResponse( [ - 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), - 'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'), 'generic' => $this->setupCheckManager->runAll(), ] ); diff --git a/apps/settings/lib/SetupChecks/HttpsUrlGeneration.php b/apps/settings/lib/SetupChecks/HttpsUrlGeneration.php new file mode 100644 index 00000000000..59f99c801cb --- /dev/null +++ b/apps/settings/lib/SetupChecks/HttpsUrlGeneration.php @@ -0,0 +1,75 @@ + + * + * @author Côme Chilliet + * + * @license GNU AGPL version 3 or any later version + * + * 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 . + * + */ + +namespace OCA\Settings\SetupChecks; + +use OCP\IL10N; +use OCP\IRequest; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class HttpsUrlGeneration implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IURLGenerator $urlGenerator, + private IRequest $request, + ) { + } + + public function getCategory(): string { + return 'security'; + } + + public function getName(): string { + return $this->l10n->t('HTTPS access and URLs'); + } + + public function run(): SetupResult { + if (!\OC::$CLI && $this->request->getServerProtocol() !== 'https') { + if (!preg_match('/(?:^(?:localhost|127\.0\.0\.1|::1)|\.onion)$/', $this->request->getInsecureServerHost())) { + return SetupResult::error( + $this->l10n->t('Accessing site insecurely via HTTP. You are strongly advised to set up your server to require HTTPS instead. Without it some important web functionality like "copy to clipboard" or "service workers" will not work!'), + $this->urlGenerator->linkToDocs('admin-security') + ); + } else { + return SetupResult::info( + $this->l10n->t('Accessing site insecurely via HTTP.'), + $this->urlGenerator->linkToDocs('admin-security') + ); + } + } + $generatedUrl = $this->urlGenerator->getAbsoluteURL('index.php'); + if (!str_starts_with($generatedUrl, 'https://')) { + return SetupResult::warning( + $this->l10n->t('You are accessing your instance over a secure connection, however your instance is generating insecure URLs. This most likely means that you are behind a reverse proxy and the overwrite config variables are not set correctly.'), + $this->urlGenerator->linkToDocs('admin-reverse-proxy') + ); + } + + return SetupResult::success($this->l10n->t('You are accessing your instance over a secure connection, and your instance is generating secure URLs.')); + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 924a76fa4f5..0f3771b27f7 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -163,8 +163,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ - 'reverseProxyDocs' => 'reverse-proxy-doc-link', - 'reverseProxyGeneratedURL' => 'https://server/index.php', 'generic' => [], ] ); diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 598fb541136..de1a788ee95 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -105,14 +105,6 @@ var afterCall = function(data, statusText, xhr) { var messages = []; if (xhr.status === 200 && data) { - if (window.location.protocol === 'https:' && data.reverseProxyGeneratedURL.split('/')[0] !== 'https:') { - messages.push({ - msg: t('core', 'You are accessing your instance over a secure connection, however your instance is generating insecure URLs. This most likely means that you are behind a reverse proxy and the overwrite config variables are not set correctly. Please read {linkstart}the documentation page about this ↗{linkend}.') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }) - } if (Object.keys(data.generic).length > 0) { Object.keys(data.generic).forEach(function(key){ Object.keys(data.generic[key]).forEach(function(title){ @@ -366,13 +358,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }); } - } else if (!/(?:^(?:localhost|127\.0\.0\.1|::1)|\.onion)$/.exec(window.location.hostname)) { - messages.push({ - msg: t('core', 'Accessing site insecurely via HTTP. You are strongly advised to set up your server to require HTTPS instead, as described in the {linkstart}security tips ↗{linkend}. Without it some important web functionality like "copy to clipboard" or "service workers" will not work!') - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_ERROR - }); } } else { messages.push({ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 0f042c19942..f6aa98f9b4a 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -160,7 +160,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { @@ -193,7 +192,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { @@ -226,7 +224,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { @@ -259,8 +256,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { @@ -323,7 +318,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { @@ -365,8 +359,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - reverseProxyGeneratedURL: 'http://server', generic: { network: { "Internet connectivity": { @@ -397,8 +389,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', - reverseProxyGeneratedURL: 'http://server', generic: { network: { "Internet connectivity": { @@ -426,7 +416,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - reverseProxyGeneratedURL: 'https://server', generic: { network: { "Internet connectivity": { -- 2.39.5