Skip to content

Commit

Permalink
Merge pull request #41312 from nextcloud/feat/migrate-code-integrity-…
Browse files Browse the repository at this point in the history
…check
  • Loading branch information
come-nc authored Jan 16, 2024
2 parents 587057b + a2915d4 commit 80d58f0
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 38 deletions.
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -169,6 +170,7 @@ public function register(IRegistrationContext $context): void {
$context->registerSetupCheck(AppDirsWithDifferentOwner::class);
$context->registerSetupCheck(BruteForceThrottler::class);
$context->registerSetupCheck(CheckUserCertificates::class);
$context->registerSetupCheck(CodeIntegrity::class);
$context->registerSetupCheck(DatabaseHasMissingColumns::class);
$context->registerSetupCheck(DatabaseHasMissingIndices::class);
$context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class);
Expand Down
3 changes: 1 addition & 2 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ private function isSettimelimitAvailable() {
}

/**
* @NoCSRFRequired
* @return RedirectResponse
* @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
*/
Expand Down Expand Up @@ -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(),
Expand Down
76 changes: 76 additions & 0 deletions apps/settings/lib/SetupChecks/CodeIntegrity.php
Original file line number Diff line number Diff line change
@@ -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'),
],
],
);
}
}
}
6 changes: 0 additions & 6 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ public function testCheck() {
'relativeTime' => '2 hours ago',
'backgroundJobsUrl' => 'https://example.org',
]);
$this->checker
->expects($this->once())
->method('hasPassedCheck')
->willReturn(true);

$this->checkSetupController
->expects($this->once())
Expand Down Expand Up @@ -234,8 +230,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,
Expand Down
30 changes: 17 additions & 13 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.'),
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand All @@ -350,6 +351,9 @@
}

var message = setupCheck.description;
if (message) {
message = this.escapeHTML(message)
}
if (setupCheck.descriptionParameters) {
message = this.richToParsed(message, setupCheck.descriptionParameters);
}
Expand Down
17 changes: 1 addition & 16 deletions core/js/tests/specs/setupchecksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -272,7 +271,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -318,7 +316,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -364,7 +361,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: false,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -530,7 +524,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -581,7 +574,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -629,7 +621,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -674,7 +665,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -716,7 +706,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -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();

Expand All @@ -761,7 +749,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -805,7 +792,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down Expand Up @@ -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();
Expand All @@ -856,7 +842,6 @@ describe('OC.SetupChecks tests', function() {
suggestedOverwriteCliURL: '',
isFairUseOfFreePushService: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isSettimelimitAvailable: true,
cronErrors: [],
cronInfo: {
Expand Down
2 changes: 1 addition & 1 deletion lib/private/RichObjectStrings/Validator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down

0 comments on commit 80d58f0

Please sign in to comment.