From e35fa737e4eff2bd2de58b7ac10172df2c77b7c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 26 Oct 2023 15:51:51 +0200 Subject: [PATCH 01/13] Migrate code integrity to SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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/CodeIntegrity.php | 63 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 2 - core/js/setupchecks.js | 10 --- core/js/tests/specs/setupchecksSpec.js | 15 ----- 8 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/CodeIntegrity.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index ff402d11c7441..45640278a97de 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -79,6 +79,7 @@ 'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => $baseDir . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php', 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php', 'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => $baseDir . '/../lib/SetupChecks/CheckUserCertificates.php', + 'OCA\\Settings\\SetupChecks\\CodeIntegrity' => $baseDir . '/../lib/SetupChecks/CodeIntegrity.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingColumns.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index f94bb2e4a1e70..bf48c7ee4507f 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -94,6 +94,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => __DIR__ . '/..' . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php', 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php', 'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckUserCertificates.php', + 'OCA\\Settings\\SetupChecks\\CodeIntegrity' => __DIR__ . '/..' . '/../lib/SetupChecks/CodeIntegrity.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingColumns.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index ce619478bd43a..f8796a90dbe3d 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -51,6 +51,7 @@ use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner; use OCA\Settings\SetupChecks\BruteForceThrottler; use OCA\Settings\SetupChecks\CheckUserCertificates; +use OCA\Settings\SetupChecks\CodeIntegrity; use OCA\Settings\SetupChecks\DatabaseHasMissingColumns; use OCA\Settings\SetupChecks\DatabaseHasMissingIndices; use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys; @@ -169,6 +170,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(AppDirsWithDifferentOwner::class); $context->registerSetupCheck(BruteForceThrottler::class); $context->registerSetupCheck(CheckUserCertificates::class); + $context->registerSetupCheck(CodeIntegrity::class); $context->registerSetupCheck(DatabaseHasMissingColumns::class); $context->registerSetupCheck(DatabaseHasMissingIndices::class); $context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index c648e8af5fc36..7e865785522b8 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -406,8 +406,6 @@ public function check() { 'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), 'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(), - 'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(), - 'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(), 'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(), diff --git a/apps/settings/lib/SetupChecks/CodeIntegrity.php b/apps/settings/lib/SetupChecks/CodeIntegrity.php new file mode 100644 index 0000000000000..234e1fbf05888 --- /dev/null +++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php @@ -0,0 +1,63 @@ + + * + * @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 OC\IntegrityCheck\Checker; +use OCP\IL10N; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class CodeIntegrity implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IURLGenerator $urlGenerator, + private Checker $checker, + ) { + } + + public function getName(): string { + return $this->l10n->t('Code integrity'); + } + + public function getCategory(): string { + return 'security'; + } + + public function run(): SetupResult { + if (!$this->checker->isCodeCheckEnforced()) { + return SetupResult::info($this->l10n->t('Integrity checker has been disabled. Integrity cannot be verified.')); + } elseif ($this->checker->hasPassedCheck()) { + return SetupResult::success($this->l10n->t('No altered files')); + } else { + // FIXME: If setup check can link to settings pages this should link to /settings/integrity/failed and /settings/integrity/rescan?requesttoken=TOKEN + return SetupResult::error( + $this->l10n->t('Some files have not passed the integrity check.'), + $this->urlGenerator->linkToDocs('admin-code-integrity') + ); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 159e0a9358def..4bbef59db85a5 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -234,8 +234,6 @@ public function testCheck() { 'isUsedTlsLibOutdated' => '', 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, - 'hasPassedCodeIntegrityCheck' => true, - 'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity', 'isSettimelimitAvailable' => true, 'areWebauthnExtensionsEnabled' => false, 'isMysqlUsedWithoutUTF8MB4' => false, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 9eacb1b137aa0..646e583ea4539 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -230,16 +230,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }); } - if(!data.hasPassedCodeIntegrityCheck) { - messages.push({ - msg: t('core', 'Some files have not passed the integrity check. Further information on how to resolve this issue can be found in the {linkstart1}documentation ↗{linkend}. ({linkstart2}List of invalid files…{linkend} / {linkstart3}Rescan…{linkend})') - .replace('{linkstart1}', '') - .replace('{linkstart2}', '') - .replace('{linkstart3}', '') - .replace(/{linkend}/g, ''), - type: OC.SetupChecks.MESSAGE_TYPE_ERROR - }); - } if(!data.isSettimelimitAvailable) { messages.push({ msg: t('core', 'The PHP function "set_time_limit" is not available. This could result in scripts being halted mid-execution, breaking your installation. Enabling this function is strongly recommended.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index a407fbb145ad7..69a2e807416ba 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -226,7 +226,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -272,7 +271,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -318,7 +316,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -364,7 +361,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: false, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -409,7 +405,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: false, cronErrors: [], cronInfo: { @@ -454,7 +449,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -530,7 +524,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -581,7 +574,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -629,7 +621,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -674,7 +665,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -716,7 +706,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -748,7 +737,6 @@ describe('OC.SetupChecks tests', function() { }); }); - it('should return an error if gmp or bcmath are not enabled', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -761,7 +749,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -805,7 +792,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { @@ -856,7 +842,6 @@ describe('OC.SetupChecks tests', function() { suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, - hasPassedCodeIntegrityCheck: true, isSettimelimitAvailable: true, cronErrors: [], cronInfo: { From 44b8cf650cd1847d25c497ab2ff8c40c4fa7f33a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 9 Jan 2024 18:03:32 +0100 Subject: [PATCH 02/13] Use highlight rich objecttype to render links in CodeIntegrity setup check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/SetupChecks/CodeIntegrity.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/settings/lib/SetupChecks/CodeIntegrity.php b/apps/settings/lib/SetupChecks/CodeIntegrity.php index 234e1fbf05888..f5882be888f6f 100644 --- a/apps/settings/lib/SetupChecks/CodeIntegrity.php +++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php @@ -55,8 +55,23 @@ public function run(): SetupResult { } else { // FIXME: If setup check can link to settings pages this should link to /settings/integrity/failed and /settings/integrity/rescan?requesttoken=TOKEN return SetupResult::error( - $this->l10n->t('Some files have not passed the integrity check.'), - $this->urlGenerator->linkToDocs('admin-code-integrity') + $this->l10n->t('Some files have not passed the integrity check. {link1} {link2}'), + $this->urlGenerator->linkToDocs('admin-code-integrity'), + [ + 'link1' => [ + 'type' => 'highlight', + 'id' => 'getFailedIntegrityCheckFiles', + 'name' => 'List of invalid files…', + 'link' => $this->urlGenerator->linkToRoute('settings.CheckSetup.getFailedIntegrityCheckFiles'), + ], + 'link2' => [ + 'type' => 'highlight', + 'id' => 'rescanFailedIntegrityCheck', + 'name' => 'Rescan…', + //, ['requesttoken' => '']? + 'link' => $this->urlGenerator->linkToRoute('settings.CheckSetup.rescanFailedIntegrityCheck'), + ], + ], ); } } From 985a91ca437c7af45dc95021561c3a4d3bd9fc48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 11 Jan 2024 12:17:59 +0100 Subject: [PATCH 03/13] Improve validator output in case of invalid RichObject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/RichObjectStrings/Validator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/RichObjectStrings/Validator.php b/lib/private/RichObjectStrings/Validator.php index 4585cbfc8148c..d7329c945e9f5 100644 --- a/lib/private/RichObjectStrings/Validator.php +++ b/lib/private/RichObjectStrings/Validator.php @@ -95,7 +95,7 @@ protected function validateParameter(array $parameter) { $missingKeys = array_diff($requiredParameters, array_keys($parameter)); if (!empty($missingKeys)) { - throw new InvalidObjectExeption('Object is invalid'); + throw new InvalidObjectExeption('Object is invalid, missing keys:'.json_encode($missingKeys)); } } From a59dcd4bd5e16256184db3d32535ccfcc9eecb82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 11 Jan 2024 12:18:25 +0100 Subject: [PATCH 04/13] Properly escape HTML and add support for highlight links in setupchecks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/js/setupchecks.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 646e583ea4539..99e289e5e54e0 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -307,6 +307,15 @@ return deferred.promise(); }, + escapeHTML: function(text) { + return text.toString() + .split('&').join('&') + .split('<').join('<') + .split('>').join('>') + .split('"').join('"') + .split('\'').join(''') + }, + /** * @param message The message string containing placeholders. * @param parameters An object with keys as placeholders and values as their replacements. @@ -317,11 +326,13 @@ for (var [placeholder, parameter] of Object.entries(parameters)) { var replacement; if (parameter.type === 'user') { - replacement = '@' + parameter.name; + replacement = '@' + this.escapeHTML(parameter.name); } else if (parameter.type === 'file') { - replacement = parameter.path || parameter.name; + replacement = this.escapeHTML(parameter.path) || this.escapeHTML(parameter.name); + } else if (parameter.type === 'highlight') { + replacement = '' + this.escapeHTML(parameter.name) + ''; } else { - replacement = parameter.name; + replacement = this.escapeHTML(parameter.name); } message = message.replace('{' + placeholder + '}', replacement); } @@ -340,6 +351,9 @@ } var message = setupCheck.description; + if (message) { + message = this.escapeHTML(message) + } if (setupCheck.descriptionParameters) { message = this.richToParsed(message, setupCheck.descriptionParameters); } From 7305fd7b487a79e05aa1c493dabf2f8fe3af10f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 11 Jan 2024 14:57:25 +0100 Subject: [PATCH 05/13] Remove CSRF check from code integrity rescan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Did not find a way to get a valid token from SetupCheck Signed-off-by: Côme Chilliet --- apps/settings/lib/Controller/CheckSetupController.php | 1 + apps/settings/lib/SetupChecks/CodeIntegrity.php | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 7e865785522b8..eb6664c5e472d 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -234,6 +234,7 @@ private function isSettimelimitAvailable() { } /** + * @NoCSRFRequired * @return RedirectResponse * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview) */ diff --git a/apps/settings/lib/SetupChecks/CodeIntegrity.php b/apps/settings/lib/SetupChecks/CodeIntegrity.php index f5882be888f6f..20ecf5360b7a0 100644 --- a/apps/settings/lib/SetupChecks/CodeIntegrity.php +++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php @@ -53,7 +53,6 @@ public function run(): SetupResult { } elseif ($this->checker->hasPassedCheck()) { return SetupResult::success($this->l10n->t('No altered files')); } else { - // FIXME: If setup check can link to settings pages this should link to /settings/integrity/failed and /settings/integrity/rescan?requesttoken=TOKEN return SetupResult::error( $this->l10n->t('Some files have not passed the integrity check. {link1} {link2}'), $this->urlGenerator->linkToDocs('admin-code-integrity'), @@ -68,7 +67,6 @@ public function run(): SetupResult { 'type' => 'highlight', 'id' => 'rescanFailedIntegrityCheck', 'name' => 'Rescan…', - //, ['requesttoken' => '']? 'link' => $this->urlGenerator->linkToRoute('settings.CheckSetup.rescanFailedIntegrityCheck'), ], ], From 6e86870b6e2549a80cca24f51c1990327590b35f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 11 Jan 2024 15:09:18 +0100 Subject: [PATCH 06/13] Fix tests in CheckSetupControllerTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/tests/Controller/CheckSetupControllerTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 4bbef59db85a5..b8f97b60bd510 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -171,10 +171,6 @@ public function testCheck() { 'relativeTime' => '2 hours ago', 'backgroundJobsUrl' => 'https://example.org', ]); - $this->checker - ->expects($this->once()) - ->method('hasPassedCheck') - ->willReturn(true); $this->checkSetupController ->expects($this->once()) From 956d7ae765473813f672a702e090ea7c425f5808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 13:20:32 +0100 Subject: [PATCH 07/13] Fix tests because we added escaping to setupchecks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/js/tests/specs/setupchecksSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 69a2e807416ba..5e879974fc938 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -823,7 +823,7 @@ describe('OC.SetupChecks tests', function() { async.done(function( data, s, x ){ expect(data).toEqual([{ - msg: 'Your installation has no default phone region set. This is required to validate phone numbers in the profile settings without a country code. To allow numbers without a country code, please add "default_phone_region" with the respective ISO 3166-1 code of the region to your config file. For more details see the documentation ↗.', + msg: 'Your installation has no default phone region set. This is required to validate phone numbers in the profile settings without a country code. To allow numbers without a country code, please add "default_phone_region" with the respective ISO 3166-1 code of the region to your config file. For more details see the documentation ↗.', type: OC.SetupChecks.MESSAGE_TYPE_INFO }]); done(); From 2bc1f78af51c1c8bb664253a603d91c2385dc81c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 13:54:26 +0100 Subject: [PATCH 08/13] Migrate overwrite.cli.url setup check to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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 | 13 --- .../lib/SetupChecks/OverwriteCliUrl.php | 81 +++++++++++++++++++ .../Controller/CheckSetupControllerTest.php | 1 - core/js/setupchecks.js | 6 -- core/js/tests/specs/setupchecksSpec.js | 14 ---- 8 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/OverwriteCliUrl.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 45640278a97de..bfd2901fed54e 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -93,6 +93,7 @@ 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\MaintenanceWindowStart' => $baseDir . '/../lib/SetupChecks/MaintenanceWindowStart.php', 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => $baseDir . '/../lib/SetupChecks/MemcacheConfigured.php', + 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => $baseDir . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php', 'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => $baseDir . '/../lib/SetupChecks/PhpFreetypeSupport.php', 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => $baseDir . '/../lib/SetupChecks/PhpGetEnv.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index bf48c7ee4507f..51c5bb04be9e0 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -108,6 +108,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php', 'OCA\\Settings\\SetupChecks\\MaintenanceWindowStart' => __DIR__ . '/..' . '/../lib/SetupChecks/MaintenanceWindowStart.php', 'OCA\\Settings\\SetupChecks\\MemcacheConfigured' => __DIR__ . '/..' . '/../lib/SetupChecks/MemcacheConfigured.php', + 'OCA\\Settings\\SetupChecks\\OverwriteCliUrl' => __DIR__ . '/..' . '/../lib/SetupChecks/OverwriteCliUrl.php', 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php', 'OCA\\Settings\\SetupChecks\\PhpFreetypeSupport' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpFreetypeSupport.php', 'OCA\\Settings\\SetupChecks\\PhpGetEnv' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpGetEnv.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index f8796a90dbe3d..8b2e9e12a1eed 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -65,6 +65,7 @@ use OCA\Settings\SetupChecks\LegacySSEKeyFormat; use OCA\Settings\SetupChecks\MaintenanceWindowStart; use OCA\Settings\SetupChecks\MemcacheConfigured; +use OCA\Settings\SetupChecks\OverwriteCliUrl; use OCA\Settings\SetupChecks\PhpDefaultCharset; use OCA\Settings\SetupChecks\PhpFreetypeSupport; use OCA\Settings\SetupChecks\PhpGetEnv; @@ -184,6 +185,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(LegacySSEKeyFormat::class); $context->registerSetupCheck(MaintenanceWindowStart::class); $context->registerSetupCheck(MemcacheConfigured::class); + $context->registerSetupCheck(OverwriteCliUrl::class); $context->registerSetupCheck(PhpDefaultCharset::class); $context->registerSetupCheck(PhpFreetypeSupport::class); $context->registerSetupCheck(PhpGetEnv::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index eb6664c5e472d..c7f1f59ba7d88 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -302,18 +302,6 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse { ); } - protected function getSuggestedOverwriteCliURL(): string { - $currentOverwriteCliUrl = $this->config->getSystemValue('overwrite.cli.url', ''); - $suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT; - - // Check correctness by checking if it is a valid URL - if (filter_var($currentOverwriteCliUrl, FILTER_VALIDATE_URL)) { - $suggestedOverwriteCliUrl = ''; - } - - return $suggestedOverwriteCliUrl; - } - protected function getLastCronInfo(): array { $lastCronRun = (int)$this->config->getAppValue('core', 'lastcron', '0'); return [ @@ -400,7 +388,6 @@ protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool { public function check() { return new DataResponse( [ - 'suggestedOverwriteCliURL' => $this->getSuggestedOverwriteCliURL(), 'cronInfo' => $this->getLastCronInfo(), 'cronErrors' => $this->getCronErrors(), 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), diff --git a/apps/settings/lib/SetupChecks/OverwriteCliUrl.php b/apps/settings/lib/SetupChecks/OverwriteCliUrl.php new file mode 100644 index 0000000000000..39c221176ab6a --- /dev/null +++ b/apps/settings/lib/SetupChecks/OverwriteCliUrl.php @@ -0,0 +1,81 @@ + + * + * @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\IConfig; +use OCP\IL10N; +use OCP\IRequest; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class OverwriteCliUrl implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + private IRequest $request, + ) { + } + + public function getCategory(): string { + return 'config'; + } + + public function getName(): string { + return $this->l10n->t('Overwrite cli URL'); + } + + public function run(): SetupResult { + $currentOverwriteCliUrl = $this->config->getSystemValue('overwrite.cli.url', ''); + $suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT; + + // Check correctness by checking if it is a valid URL + if (filter_var($currentOverwriteCliUrl, FILTER_VALIDATE_URL)) { + if ($currentOverwriteCliUrl == $suggestedOverwriteCliUrl) { + return SetupResult::success( + $this->l10n->t( + 'The "overwrite.cli.url" option in your config.php is correctly set to "%s".', + [$currentOverwriteCliUrl] + ) + ); + } else { + return SetupResult::success( + $this->l10n->t( + 'The "overwrite.cli.url" option in your config.php is set to "%s" which is a correct URL. Suggested URL is "%s".', + [$currentOverwriteCliUrl, $suggestedOverwriteCliUrl] + ) + ); + } + } else { + return SetupResult::warning( + $this->l10n->t( + 'Please make sure to set the "overwrite.cli.url" option in your config.php file to the URL that your users mainly use to access this Nextcloud. Suggestion: "%s". Otherwise there might be problems with the URL generation via cron. (It is possible though that the suggested URL is not the URL that your users mainly use to access this Nextcloud. Best is to double check this in any case.)', + [$suggestedOverwriteCliUrl] + ) + ); + } + } +} diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index b8f97b60bd510..022094100cbb0 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -220,7 +220,6 @@ public function testCheck() { $expected = new DataResponse( [ - 'suggestedOverwriteCliURL' => '', 'cronInfo' => [ 'diffInSeconds' => 123, 'relativeTime' => '2 hours ago', diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 99e289e5e54e0..8a70b6b81073e 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -180,12 +180,6 @@ var afterCall = function(data, statusText, xhr) { var messages = []; if (xhr.status === 200 && data) { - if (data.suggestedOverwriteCliURL !== '') { - messages.push({ - msg: t('core', 'Please make sure to set the "overwrite.cli.url" option in your config.php file to the URL that your users mainly use to access this Nextcloud. Suggestion: "{suggestedOverwriteCliURL}". Otherwise there might be problems with the URL generation via cron. (It is possible though that the suggested URL is not the URL that your users mainly use to access this Nextcloud. Best is to double check this in any case.)', {suggestedOverwriteCliURL: data.suggestedOverwriteCliURL}), - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if (data.cronErrors.length > 0) { var listOfCronErrors = ""; data.cronErrors.forEach(function(element){ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 5e879974fc938..7ed100d863b56 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -223,7 +223,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -268,7 +267,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json' }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -313,7 +311,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -358,7 +355,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: false, isSettimelimitAvailable: true, @@ -401,7 +397,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, @@ -445,7 +440,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, @@ -521,7 +515,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -571,7 +564,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -618,7 +610,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -662,7 +653,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -703,7 +693,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -746,7 +735,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -789,7 +777,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, @@ -839,7 +826,6 @@ describe('OC.SetupChecks tests', function() { 'Content-Type': 'application/json', }, JSON.stringify({ - suggestedOverwriteCliURL: '', isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, From 86bfd7339c4db1ccb178a93e7ca0133ad909333e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 15:06:58 +0100 Subject: [PATCH 09/13] Fix CheckSetupControllerTest following overwrite.cli.url migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/settings/tests/Controller/CheckSetupControllerTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 022094100cbb0..cc34197c5bed2 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -128,7 +128,6 @@ protected function setUp(): void { ]) ->setMethods([ 'getLastCronInfo', - 'getSuggestedOverwriteCliURL', 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', @@ -159,10 +158,6 @@ public function testCheck() { ->method('getHeader'); $this->clientService->expects($this->never()) ->method('newClient'); - $this->checkSetupController - ->expects($this->once()) - ->method('getSuggestedOverwriteCliURL') - ->willReturn(''); $this->checkSetupController ->expects($this->once()) ->method('getLastCronInfo') From 27514adef4cdfb51675211644dad6819323b142f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 15:05:14 +0100 Subject: [PATCH 10/13] Migrate Cron checks to new SetupCheck API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 2 + .../composer/composer/autoload_static.php | 2 + apps/settings/lib/AppInfo/Application.php | 4 + .../lib/Controller/CheckSetupController.php | 26 ------ apps/settings/lib/SetupChecks/CronErrors.php | 62 ++++++++++++++ apps/settings/lib/SetupChecks/CronInfo.php | 81 +++++++++++++++++++ core/js/setupchecks.js | 22 ----- 7 files changed, 151 insertions(+), 48 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/CronErrors.php create mode 100644 apps/settings/lib/SetupChecks/CronInfo.php diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index bfd2901fed54e..f00a2aad146d3 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -80,6 +80,8 @@ 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php', 'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => $baseDir . '/../lib/SetupChecks/CheckUserCertificates.php', 'OCA\\Settings\\SetupChecks\\CodeIntegrity' => $baseDir . '/../lib/SetupChecks/CodeIntegrity.php', + 'OCA\\Settings\\SetupChecks\\CronErrors' => $baseDir . '/../lib/SetupChecks/CronErrors.php', + 'OCA\\Settings\\SetupChecks\\CronInfo' => $baseDir . '/../lib/SetupChecks/CronInfo.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingColumns.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 51c5bb04be9e0..d1aa4e3ea9626 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -95,6 +95,8 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php', 'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckUserCertificates.php', 'OCA\\Settings\\SetupChecks\\CodeIntegrity' => __DIR__ . '/..' . '/../lib/SetupChecks/CodeIntegrity.php', + 'OCA\\Settings\\SetupChecks\\CronErrors' => __DIR__ . '/..' . '/../lib/SetupChecks/CronErrors.php', + 'OCA\\Settings\\SetupChecks\\CronInfo' => __DIR__ . '/..' . '/../lib/SetupChecks/CronInfo.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingColumns.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingIndices.php', 'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 8b2e9e12a1eed..820ee4f98ac12 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -52,6 +52,8 @@ use OCA\Settings\SetupChecks\BruteForceThrottler; use OCA\Settings\SetupChecks\CheckUserCertificates; use OCA\Settings\SetupChecks\CodeIntegrity; +use OCA\Settings\SetupChecks\CronErrors; +use OCA\Settings\SetupChecks\CronInfo; use OCA\Settings\SetupChecks\DatabaseHasMissingColumns; use OCA\Settings\SetupChecks\DatabaseHasMissingIndices; use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys; @@ -172,6 +174,8 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(BruteForceThrottler::class); $context->registerSetupCheck(CheckUserCertificates::class); $context->registerSetupCheck(CodeIntegrity::class); + $context->registerSetupCheck(CronErrors::class); + $context->registerSetupCheck(CronInfo::class); $context->registerSetupCheck(DatabaseHasMissingColumns::class); $context->registerSetupCheck(DatabaseHasMissingIndices::class); $context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index c7f1f59ba7d88..00883a0d51eed 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -55,7 +55,6 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\Http\Client\IClientService; use OCP\IConfig; -use OCP\IDateTimeFormatter; use OCP\IL10N; use OCP\IRequest; use OCP\ITempManager; @@ -78,8 +77,6 @@ class CheckSetupController extends Controller { private $checker; /** @var LoggerInterface */ private $logger; - /** @var IDateTimeFormatter */ - private $dateTimeFormatter; /** @var ITempManager */ private $tempManager; /** @var IManager */ @@ -94,7 +91,6 @@ public function __construct($AppName, IL10N $l10n, Checker $checker, LoggerInterface $logger, - IDateTimeFormatter $dateTimeFormatter, ITempManager $tempManager, IManager $manager, ISetupCheckManager $setupCheckManager, @@ -106,7 +102,6 @@ public function __construct($AppName, $this->l10n = $l10n; $this->checker = $checker; $this->logger = $logger; - $this->dateTimeFormatter = $dateTimeFormatter; $this->tempManager = $tempManager; $this->manager = $manager; $this->setupCheckManager = $setupCheckManager; @@ -302,25 +297,6 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse { ); } - protected function getLastCronInfo(): array { - $lastCronRun = (int)$this->config->getAppValue('core', 'lastcron', '0'); - return [ - 'diffInSeconds' => time() - $lastCronRun, - 'relativeTime' => $this->dateTimeFormatter->formatTimeSpan($lastCronRun), - 'backgroundJobsUrl' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => 'server']) . '#backgroundjobs', - ]; - } - - protected function getCronErrors() { - $errors = json_decode($this->config->getAppValue('core', 'cronErrors', ''), true); - - if (is_array($errors)) { - return $errors; - } - - return []; - } - private function isTemporaryDirectoryWritable(): bool { try { if (!empty($this->tempManager->getTempBaseDir())) { @@ -388,8 +364,6 @@ protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool { public function check() { return new DataResponse( [ - 'cronInfo' => $this->getLastCronInfo(), - 'cronErrors' => $this->getCronErrors(), 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), 'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), diff --git a/apps/settings/lib/SetupChecks/CronErrors.php b/apps/settings/lib/SetupChecks/CronErrors.php new file mode 100644 index 0000000000000..a2ebbf5f12cc6 --- /dev/null +++ b/apps/settings/lib/SetupChecks/CronErrors.php @@ -0,0 +1,62 @@ + + * + * @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\IConfig; +use OCP\IL10N; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class CronErrors implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + ) { + } + + public function getCategory(): string { + return 'system'; + } + + public function getName(): string { + return $this->l10n->t('Cron errors'); + } + + public function run(): SetupResult { + $errors = json_decode($this->config->getAppValue('core', 'cronErrors', ''), true); + if (is_array($errors) && count($errors) > 0) { + return SetupResult::error( + $this->l10n->t( + "It was not possible to execute the cron job via CLI. The following technical errors have appeared:\n%s", + implode("\n", array_map(fn (array $error) => '- '.$error['error'].' '.$error['hint'], $errors)) + ) + ); + } else { + return SetupResult::success($this->l10n->t('The last cron job ran without errors.')); + } + } +} diff --git a/apps/settings/lib/SetupChecks/CronInfo.php b/apps/settings/lib/SetupChecks/CronInfo.php new file mode 100644 index 0000000000000..d08bb6852a849 --- /dev/null +++ b/apps/settings/lib/SetupChecks/CronInfo.php @@ -0,0 +1,81 @@ + + * + * @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\IConfig; +use OCP\IDateTimeFormatter; +use OCP\IL10N; +use OCP\IURLGenerator; +use OCP\SetupCheck\ISetupCheck; +use OCP\SetupCheck\SetupResult; + +class CronInfo implements ISetupCheck { + public function __construct( + private IL10N $l10n, + private IConfig $config, + private IURLGenerator $urlGenerator, + private IDateTimeFormatter $dateTimeFormatter, + ) { + } + + public function getCategory(): string { + return 'system'; + } + + public function getName(): string { + return $this->l10n->t('Cron last run'); + } + + public function run(): SetupResult { + $lastCronRun = (int)$this->config->getAppValue('core', 'lastcron', '0'); + $relativeTime = $this->dateTimeFormatter->formatTimeSpan($lastCronRun); + + if ((time() - $lastCronRun) > 3600) { + return SetupResult::error( + $this->l10n->t( + 'Last background job execution ran %s. Something seems wrong. {link}.', + [$relativeTime] + ), + descriptionParameters:[ + 'link' => [ + 'type' => 'highlight', + 'id' => 'backgroundjobs', + 'name' => 'Check the background job settings', + 'link' => $this->urlGenerator->linkToRoute('settings.AdminSettings.index', ['section' => 'server']) . '#backgroundjobs', + ], + ], + ); + } else { + return SetupResult::success( + $this->l10n->t( + 'Last background job execution ran %s.', + [$relativeTime] + ) + ); + } + } +} diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 8a70b6b81073e..653036ba1bcb5 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -180,28 +180,6 @@ var afterCall = function(data, statusText, xhr) { var messages = []; if (xhr.status === 200 && data) { - if (data.cronErrors.length > 0) { - var listOfCronErrors = ""; - data.cronErrors.forEach(function(element){ - listOfCronErrors += '
  • '; - listOfCronErrors += element.error; - listOfCronErrors += ' '; - listOfCronErrors += element.hint; - listOfCronErrors += '
    • '; - }); - messages.push({ - msg: t('core', 'It was not possible to execute the cron job via CLI. The following technical errors have appeared:') + '
      ' + listOfCronErrors + '
    ', - type: OC.SetupChecks.MESSAGE_TYPE_ERROR - }) - } - if (data.cronInfo.diffInSeconds > 3600) { - messages.push({ - msg: t('core', 'Last background job execution ran {relativeTime}. Something seems wrong. {linkstart}Check the background job settings ↗{linkend}.', {relativeTime: data.cronInfo.relativeTime}) - .replace('{linkstart}', '') - .replace('{linkend}', ''), - type: OC.SetupChecks.MESSAGE_TYPE_ERROR - }); - } if (!data.isFairUseOfFreePushService) { messages.push({ msg: t('core', 'This is the unsupported community build of Nextcloud. Given the size of this instance, performance, reliability and scalability cannot be guaranteed. Push notifications are limited to avoid overloading our free service. Learn more about the benefits of Nextcloud Enterprise at {linkstart}https://nextcloud.com/enterprise{linkend}.') From d5c9d6037fcd5badcc547fb112ad13fa149b5114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 15:05:45 +0100 Subject: [PATCH 11/13] Fix Cron setup check tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Controller/CheckSetupControllerTest.php | 15 ----- core/js/tests/specs/setupchecksSpec.js | 56 ------------------- 2 files changed, 71 deletions(-) diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index cc34197c5bed2..d5f5915ddbab9 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -42,7 +42,6 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\Http\Client\IClientService; use OCP\IConfig; -use OCP\IDateTimeFormatter; use OCP\IL10N; use OCP\IRequest; use OCP\ITempManager; @@ -77,8 +76,6 @@ class CheckSetupControllerTest extends TestCase { private $logger; /** @var Checker|\PHPUnit\Framework\MockObject\MockObject */ private $checker; - /** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */ - private $dateTimeFormatter; /** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */ private $tempManager; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ @@ -107,7 +104,6 @@ protected function setUp(): void { $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->disableOriginalConstructor()->getMock(); $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); - $this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock(); $this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock(); $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); $this->setupCheckManager = $this->createMock(ISetupCheckManager::class); @@ -121,7 +117,6 @@ protected function setUp(): void { $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -142,7 +137,6 @@ public function testCheck() { ->method('getAppValue') ->willReturnMap([ ['files_external', 'user_certificate_scan', '', '["a", "b"]'], - ['core', 'cronErrors', '', ''], ['dav', 'needs_system_address_book_sync', 'no', 'no'], ]); $this->config->expects($this->any()) @@ -215,12 +209,6 @@ public function testCheck() { $expected = new DataResponse( [ - 'cronInfo' => [ - 'diffInSeconds' => 123, - 'relativeTime' => '2 hours ago', - 'backgroundJobsUrl' => 'https://example.org', - ], - 'cronErrors' => [], 'isUsedTlsLibOutdated' => '', 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, @@ -248,7 +236,6 @@ public function testGetCurlVersion() { $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -917,7 +904,6 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -963,7 +949,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 7ed100d863b56..0214e589fe7ba 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -226,10 +226,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -270,10 +266,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -314,10 +306,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -358,10 +346,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: false, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -401,10 +385,6 @@ describe('OC.SetupChecks tests', function() { reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: false, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -444,10 +424,6 @@ describe('OC.SetupChecks tests', function() { reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -518,10 +494,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -567,10 +539,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: true, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -613,10 +581,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -656,10 +620,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -696,10 +656,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false, @@ -738,10 +694,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: false, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -780,10 +732,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, @@ -829,10 +777,6 @@ describe('OC.SetupChecks tests', function() { isFairUseOfFreePushService: true, isCorrectMemcachedPHPModuleInstalled: true, isSettimelimitAvailable: true, - cronErrors: [], - cronInfo: { - diffInSeconds: 0 - }, areWebauthnExtensionsEnabled: true, isMysqlUsedWithoutUTF8MB4: false, isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, From 19aaaad8df55690d13baf831f30f02101500f17e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 17:24:25 +0100 Subject: [PATCH 12/13] Remove references to removed functions in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../tests/Controller/CheckSetupControllerTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index d5f5915ddbab9..1b778da661f79 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -122,7 +122,6 @@ protected function setUp(): void { $this->setupCheckManager, ]) ->setMethods([ - 'getLastCronInfo', 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', @@ -152,14 +151,6 @@ public function testCheck() { ->method('getHeader'); $this->clientService->expects($this->never()) ->method('newClient'); - $this->checkSetupController - ->expects($this->once()) - ->method('getLastCronInfo') - ->willReturn([ - 'diffInSeconds' => 123, - 'relativeTime' => '2 hours ago', - 'backgroundJobsUrl' => 'https://example.org', - ]); $this->checkSetupController ->expects($this->once()) From f9eeb285c9896d1ff218f6d82cf241dc9c27f214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 15:40:21 +0100 Subject: [PATCH 13/13] Remove obsolete check of curl SSL version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 74 ------- .../Controller/CheckSetupControllerTest.php | 198 ------------------ core/js/setupchecks.js | 6 - 3 files changed, 278 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 00883a0d51eed..8897548a97713 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -45,7 +45,6 @@ */ namespace OCA\Settings\Controller; -use GuzzleHttp\Exception\ClientException; use OC\AppFramework\Http; use OC\IntegrityCheck\Checker; use OCP\AppFramework\Controller; @@ -53,7 +52,6 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -67,8 +65,6 @@ class CheckSetupController extends Controller { /** @var IConfig */ private $config; - /** @var IClientService */ - private $clientService; /** @var IURLGenerator */ private $urlGenerator; /** @var IL10N */ @@ -86,7 +82,6 @@ class CheckSetupController extends Controller { public function __construct($AppName, IRequest $request, IConfig $config, - IClientService $clientService, IURLGenerator $urlGenerator, IL10N $l10n, Checker $checker, @@ -97,7 +92,6 @@ public function __construct($AppName, ) { parent::__construct($AppName, $request); $this->config = $config; - $this->clientService = $clientService; $this->urlGenerator = $urlGenerator; $this->l10n = $l10n; $this->checker = $checker; @@ -129,73 +123,6 @@ private function isFairUseOfFreePushService(): bool { return $this->manager->isFairUseOfFreePushService(); } - /** - * Public for the sake of unit-testing - * - * @return array - */ - protected function getCurlVersion() { - return curl_version(); - } - - /** - * Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do - * have multiple bugs which likely lead to problems in combination with - * functionality required by ownCloud such as SNI. - * - * @link https://github.com/owncloud/core/issues/17446#issuecomment-122877546 - * @link https://bugzilla.redhat.com/show_bug.cgi?id=1241172 - * @return string - */ - private function isUsedTlsLibOutdated() { - // Don't run check when: - // 1. Server has `has_internet_connection` set to false - // 2. AppStore AND S2S is disabled - if (!$this->config->getSystemValue('has_internet_connection', true)) { - return ''; - } - if (!$this->config->getSystemValue('appstoreenabled', true) - && $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'no' - && $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'no') { - return ''; - } - - $versionString = $this->getCurlVersion(); - if (isset($versionString['ssl_version'])) { - $versionString = $versionString['ssl_version']; - } else { - return ''; - } - - $features = $this->l10n->t('installing and updating apps via the App Store or Federated Cloud Sharing'); - if (!$this->config->getSystemValue('appstoreenabled', true)) { - $features = $this->l10n->t('Federated Cloud Sharing'); - } - - // Check if NSS and perform heuristic check - if (str_starts_with($versionString, 'NSS/')) { - try { - $firstClient = $this->clientService->newClient(); - $firstClient->get('https://nextcloud.com/'); - - $secondClient = $this->clientService->newClient(); - $secondClient->get('https://nextcloud.com/'); - } catch (ClientException $e) { - if ($e->getResponse()->getStatusCode() === 400) { - return $this->l10n->t('cURL is using an outdated %1$s version (%2$s). Please update your operating system or features such as %3$s will not work reliably.', ['NSS', $versionString, $features]); - } - } catch (\Exception $e) { - $this->logger->warning('error checking curl', [ - 'app' => 'settings', - 'exception' => $e, - ]); - return $this->l10n->t('Could not determine if TLS version of cURL is outdated or not because an error happened during the HTTPS request against https://nextcloud.com. Please check the Nextcloud log file for more details.'); - } - } - - return ''; - } - /** * Checks if the correct memcache module for PHP is installed. Only * fails if memcached is configured and the working module is not installed. @@ -365,7 +292,6 @@ public function check() { return new DataResponse( [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), - 'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), 'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 1b778da661f79..3bec435bd6acb 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -40,7 +40,6 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -49,7 +48,6 @@ use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -66,8 +64,6 @@ class CheckSetupControllerTest extends TestCase { private $request; /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ private $config; - /** @var IClientService | \PHPUnit\Framework\MockObject\MockObject*/ - private $clientService; /** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */ private $urlGenerator; /** @var IL10N | \PHPUnit\Framework\MockObject\MockObject */ @@ -90,8 +86,6 @@ protected function setUp(): void { ->disableOriginalConstructor()->getMock(); $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor()->getMock(); - $this->clientService = $this->getMockBuilder(IClientService::class) - ->disableOriginalConstructor()->getMock(); $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class) ->disableOriginalConstructor()->getMock(); $this->l10n = $this->getMockBuilder(IL10N::class) @@ -112,7 +106,6 @@ protected function setUp(): void { 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -149,8 +142,6 @@ public function testCheck() { $this->request->expects($this->never()) ->method('getHeader'); - $this->clientService->expects($this->never()) - ->method('newClient'); $this->checkSetupController ->expects($this->once()) @@ -200,7 +191,6 @@ public function testCheck() { $expected = new DataResponse( [ - 'isUsedTlsLibOutdated' => '', 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, 'isSettimelimitAvailable' => true, @@ -216,192 +206,6 @@ public function testCheck() { $this->assertEquals($expected, $this->checkSetupController->check()); } - public function testGetCurlVersion() { - $checkSetupController = $this->getMockBuilder(CheckSetupController::class) - ->setConstructorArgs([ - 'settings', - $this->request, - $this->config, - $this->clientService, - $this->urlGenerator, - $this->l10n, - $this->checker, - $this->logger, - $this->tempManager, - $this->notificationManager, - $this->setupCheckManager, - ]) - ->setMethods(null)->getMock(); - - $this->assertArrayHasKey('ssl_version', $this->invokePrivate($checkSetupController, 'getCurlVersion')); - } - - public function testIsUsedTlsLibOutdatedWithAnotherLibrary() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'SSLlib']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMisbehavingCurl() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'OpenSSL/1.0.1d']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion1() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'OpenSSL/1.0.2b']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsBuggyNss400() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'NSS/1.0.2b']); - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException') - ->disableOriginalConstructor()->getMock(); - $response = $this->getMockBuilder(ResponseInterface::class) - ->disableOriginalConstructor()->getMock(); - $response->expects($this->once()) - ->method('getStatusCode') - ->willReturn(400); - $exception->expects($this->once()) - ->method('getResponse') - ->willReturn($response); - - $client->expects($this->once()) - ->method('get') - ->with('https://nextcloud.com/', []) - ->will($this->throwException($exception)); - - $this->clientService->expects($this->once()) - ->method('newClient') - ->willReturn($client); - - $this->assertSame('cURL is using an outdated NSS version (NSS/1.0.2b). Please update your operating system or features such as installing and updating apps via the App Store or Federated Cloud Sharing will not work reliably.', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - - public function testIsBuggyNss200() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'NSS/1.0.2b']); - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException') - ->disableOriginalConstructor()->getMock(); - $response = $this->getMockBuilder(ResponseInterface::class) - ->disableOriginalConstructor()->getMock(); - $response->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $exception->expects($this->once()) - ->method('getResponse') - ->willReturn($response); - - $client->expects($this->once()) - ->method('get') - ->with('https://nextcloud.com/', []) - ->will($this->throwException($exception)); - - $this->clientService->expects($this->once()) - ->method('newClient') - ->willReturn($client); - - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithInternetDisabled() { - $this->config - ->expects($this->once()) - ->method('getSystemValue') - ->with('has_internet_connection', true) - ->willReturn(false); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithAppstoreDisabledAndServerToServerSharingEnabled() { - $this->config - ->expects($this->exactly(2)) - ->method('getSystemValue') - ->willReturnMap([ - ['has_internet_connection', true, true], - ['appstoreenabled', true, false], - ]); - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['files_sharing', 'outgoing_server2server_share_enabled', 'yes', 'no'], - ['files_sharing', 'incoming_server2server_share_enabled', 'yes', 'yes'], - ]); - - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithAppstoreDisabledAndServerToServerSharingDisabled() { - $this->config - ->expects($this->exactly(2)) - ->method('getSystemValue') - ->willReturnMap([ - ['has_internet_connection', true, true], - ['appstoreenabled', true, false], - ]); - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['files_sharing', 'outgoing_server2server_share_enabled', 'yes', 'no'], - ['files_sharing', 'incoming_server2server_share_enabled', 'yes', 'no'], - ]); - - $this->checkSetupController - ->expects($this->never()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - public function testRescanFailedIntegrityCheck() { $this->checker ->expects($this->once()) @@ -890,7 +694,6 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -935,7 +738,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 653036ba1bcb5..67ebabe1ae65f 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,12 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(data.isUsedTlsLibOutdated) { - messages.push({ - msg: data.isUsedTlsLibOutdated, - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if(!data.isCorrectMemcachedPHPModuleInstalled) { messages.push({ msg: t('core', 'Memcached is configured as distributed cache, but the wrong PHP module "memcache" is installed. \\OC\\Memcache\\Memcached only supports "memcached" and not "memcache". See the {linkstart}memcached wiki about both modules ↗{linkend}.')