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 ff402d11c74..45640278a97 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -79,6 +79,7 @@ return array( '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 f94bb2e4a1e..bf48c7ee450 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 ce619478bd4..f8796a90dbe 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -51,6 +51,7 @@ use OCA\Settings\Search\UserSearch; 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 @@ class Application extends App implements IBootstrap { $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 c648e8af5fc..7e865785522 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -406,8 +406,6 @@ Raw output '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 00000000000..234e1fbf058 --- /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 159e0a9358d..4bbef59db85 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -234,8 +234,6 @@ class CheckSetupControllerTest extends TestCase { '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 9eacb1b137a..646e583ea45 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 a407fbb145a..69a2e807416 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 234e1fbf058..f5882be888f 100644 --- a/apps/settings/lib/SetupChecks/CodeIntegrity.php +++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php @@ -55,8 +55,23 @@ class CodeIntegrity implements ISetupCheck { } 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 4585cbfc814..d7329c945e9 100644 --- a/lib/private/RichObjectStrings/Validator.php +++ b/lib/private/RichObjectStrings/Validator.php @@ -95,7 +95,7 @@ class Validator implements IValidator { $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 646e583ea45..99e289e5e54 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 7e865785522..eb6664c5e47 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -234,6 +234,7 @@ class CheckSetupController extends Controller { } /** + * @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 f5882be888f..20ecf5360b7 100644 --- a/apps/settings/lib/SetupChecks/CodeIntegrity.php +++ b/apps/settings/lib/SetupChecks/CodeIntegrity.php @@ -53,7 +53,6 @@ class CodeIntegrity implements ISetupCheck { } 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 @@ class CodeIntegrity implements ISetupCheck { '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 4bbef59db85..b8f97b60bd5 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -171,10 +171,6 @@ class CheckSetupControllerTest extends TestCase { '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 69a2e807416..5e879974fc9 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 45640278a97..bfd2901fed5 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -93,6 +93,7 @@ return array( '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 bf48c7ee450..51c5bb04be9 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 f8796a90dbe..8b2e9e12a1e 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -65,6 +65,7 @@ use OCA\Settings\SetupChecks\JavaScriptModules; 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 @@ class Application extends App implements IBootstrap { $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 eb6664c5e47..c7f1f59ba7d 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -302,18 +302,6 @@ Raw output ); } - 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 @@ Raw output 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 00000000000..39c221176ab --- /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 b8f97b60bd5..022094100cb 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -220,7 +220,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ - 'suggestedOverwriteCliURL' => '', 'cronInfo' => [ 'diffInSeconds' => 123, 'relativeTime' => '2 hours ago', diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 99e289e5e54..8a70b6b8107 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 5e879974fc9..7ed100d863b 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 022094100cb..cc34197c5be 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -128,7 +128,6 @@ class CheckSetupControllerTest extends TestCase { ]) ->setMethods([ 'getLastCronInfo', - 'getSuggestedOverwriteCliURL', 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', @@ -159,10 +158,6 @@ class CheckSetupControllerTest extends TestCase { ->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 bfd2901fed5..f00a2aad146 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -80,6 +80,8 @@ return array( '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 51c5bb04be9..d1aa4e3ea96 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 8b2e9e12a1e..820ee4f98ac 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -52,6 +52,8 @@ use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner; 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 @@ class Application extends App implements IBootstrap { $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 c7f1f59ba7d..00883a0d51e 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -55,7 +55,6 @@ use OCP\AppFramework\Http\DataResponse; 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 @@ class CheckSetupController extends Controller { IL10N $l10n, Checker $checker, LoggerInterface $logger, - IDateTimeFormatter $dateTimeFormatter, ITempManager $tempManager, IManager $manager, ISetupCheckManager $setupCheckManager, @@ -106,7 +102,6 @@ class CheckSetupController extends Controller { $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 @@ Raw output ); } - 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 @@ Raw output 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 00000000000..a2ebbf5f12c --- /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 00000000000..d08bb6852a8 --- /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 8a70b6b8107..653036ba1bc 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 cc34197c5be..d5f5915ddba 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -42,7 +42,6 @@ use OCP\AppFramework\Http\DataResponse; 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 @@ class CheckSetupControllerTest extends TestCase { $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 @@ class CheckSetupControllerTest extends TestCase { $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -142,7 +137,6 @@ class CheckSetupControllerTest extends TestCase { ->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 @@ class CheckSetupControllerTest extends TestCase { $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 @@ class CheckSetupControllerTest extends TestCase { $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -917,7 +904,6 @@ Array $this->l10n, $this->checker, $this->logger, - $this->dateTimeFormatter, $this->tempManager, $this->notificationManager, $this->setupCheckManager, @@ -963,7 +949,6 @@ Array $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 7ed100d863b..0214e589fe7 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 d5f5915ddba..1b778da661f 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -122,7 +122,6 @@ class CheckSetupControllerTest extends TestCase { $this->setupCheckManager, ]) ->setMethods([ - 'getLastCronInfo', 'getCurlVersion', 'isPhpOutdated', 'isPHPMailerUsed', @@ -152,14 +151,6 @@ class CheckSetupControllerTest extends TestCase { ->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 00883a0d51e..8897548a977 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\Attribute\IgnoreOpenAPI; 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 @@ use Psr\Log\LoggerInterface; 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 @@ class CheckSetupController extends Controller { ) { parent::__construct($AppName, $request); $this->config = $config; - $this->clientService = $clientService; $this->urlGenerator = $urlGenerator; $this->l10n = $l10n; $this->checker = $checker; @@ -129,73 +123,6 @@ class CheckSetupController extends Controller { 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 @@ Raw output 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 1b778da661f..3bec435bd6a 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -40,7 +40,6 @@ use OCP\AppFramework\Http; 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\IURLGenerator; 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 @@ class CheckSetupControllerTest extends TestCase { ->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 @@ class CheckSetupControllerTest extends TestCase { 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -149,8 +142,6 @@ class CheckSetupControllerTest extends TestCase { $this->request->expects($this->never()) ->method('getHeader'); - $this->clientService->expects($this->never()) - ->method('newClient'); $this->checkSetupController ->expects($this->once()) @@ -200,7 +191,6 @@ class CheckSetupControllerTest extends TestCase { $expected = new DataResponse( [ - 'isUsedTlsLibOutdated' => '', 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, 'isSettimelimitAvailable' => true, @@ -216,192 +206,6 @@ class CheckSetupControllerTest extends TestCase { $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 @@ Array 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -935,7 +738,6 @@ Array '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 653036ba1bc..67ebabe1ae6 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}.')