Migrate code integrity to SetupCheck API

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',

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',

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;
@@ -168,6 +169,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);

apps/settings/lib/Controller/CheckSetupController.php

@@ -234,6 +234,7 @@ private function isSettimelimitAvailable() {
 	}
 
 	/**
+	 * @NoCSRFRequired
 	 * @return RedirectResponse
 	 * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
 	 */
@@ -406,8 +407,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(),

apps/settings/lib/SetupChecks/CodeIntegrity.php

@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+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 {
+			return SetupResult::error(
+				$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…',
+						'link' => $this->urlGenerator->linkToRoute('settings.CheckSetup.rescanFailedIntegrityCheck'),
+					],
+				],
+			);
+		}
+	}
+}

apps/settings/tests/Controller/CheckSetupControllerTest.php

@@ -191,10 +191,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())
@@ -254,8 +250,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,

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}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.codeIntegrityCheckerDocumentation + '">')
-								.replace('{linkstart2}', '<a href="' + OC.generateUrl('/settings/integrity/failed') + '">')
-								.replace('{linkstart3}', '<a href="' + OC.generateUrl('/settings/integrity/rescan?requesttoken={requesttoken}', {'requesttoken': OC.requestToken}) + '">')
-								.replace(/{linkend}/g, '</a>'),
-							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.'),
@@ -317,6 +307,15 @@
 			return deferred.promise();
 		},
 
+		escapeHTML: function(text) {
+			return text.toString()
+				.split('&').join('&amp;')
+				.split('<').join('&lt;')
+				.split('>').join('&gt;')
+				.split('"').join('&quot;')
+				.split('\'').join('&#039;')
+		},
+
 		/**
 		* @param message      The message string containing placeholders.
 		* @param parameters   An object with keys as placeholders and values as their replacements.
@@ -327,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 = '<a href="' + encodeURI(parameter.link) + '">' + this.escapeHTML(parameter.name) + '</a>';
 				} else {
-					replacement = parameter.name;
+					replacement = this.escapeHTML(parameter.name);
 				}
 				message = message.replace('{' + placeholder + '}', replacement);
 			}
@@ -350,6 +351,9 @@
 			}
 
 			var message = setupCheck.description;
+			if (message) {
+				message = this.escapeHTML(message)
+			}
 			if (setupCheck.descriptionParameters) {
 				message = this.richToParsed(message, setupCheck.descriptionParameters);
 			}

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: {
@@ -837,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 <a target="_blank" rel="noreferrer noopener" class="external" href="https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#Officially_assigned_code_elements">documentation ↗</a>.',
+					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 &quot;default_phone_region&quot; with the respective ISO 3166-1 code of the region to your config file. For more details see the <a target="_blank" rel="noreferrer noopener" class="external" href="https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#Officially_assigned_code_elements">documentation ↗</a>.',
 					type: OC.SetupChecks.MESSAGE_TYPE_INFO
 				}]);
 				done();
@@ -856,7 +842,6 @@ describe('OC.SetupChecks tests', function() {
 					suggestedOverwriteCliURL: '',
 					isFairUseOfFreePushService: true,
 					isCorrectMemcachedPHPModuleInstalled: true,
-					hasPassedCodeIntegrityCheck: true,
 					isSettimelimitAvailable: true,
 					cronErrors: [],
 					cronInfo: {

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));
 		}
 	}