Skip to content

Commit

Permalink
fix(IntegrityCheck): Ensure the check is run if no results are available
Browse files Browse the repository at this point in the history
If there are no cached results the current implementation was also returning an empty array,

but this was the same as when there was a successful run.

So to distinguish this we return `null` if there are *no* results.

In this case we need to rerun the integrity checker.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

[skip ci]
  • Loading branch information
susnux authored and backportbot[bot] committed Jul 4, 2024
1 parent 23cc6cf commit 2c2a62a
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 5 deletions.
4 changes: 4 additions & 0 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {

$completeResults = $this->checker->getResults();

if ($completeResults === null) {
return new DataDisplayResponse('Integrity checker has not been run. Integrity information not available.');
}

if (!empty($completeResults)) {
$formattedTextResponse = 'Technical information
=====================
Expand Down
9 changes: 8 additions & 1 deletion apps/settings/lib/SetupChecks/CodeIntegrity.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ public function getCategory(): string {
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()) {
}

// If there are no results we need to run the verification
if ($this->checker->getResults() === null) {
$this->checker->runInstanceVerification();
}

if ($this->checker->hasPassedCheck()) {
return SetupResult::success($this->l10n->t('No altered files'));
} else {
return SetupResult::error(
Expand Down
135 changes: 135 additions & 0 deletions apps/settings/tests/SetupChecks/CodeIntegrityTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Settings\Tests;

use OC\IntegrityCheck\Checker;
use OCA\Settings\SetupChecks\CodeIntegrity;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\SetupResult;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

class CodeIntegrityTest extends TestCase {

private IL10N&MockObject $l10n;
private IURLGenerator&MockObject $urlGenerator;
private Checker&MockObject $checker;

protected function setUp(): void {
parent::setUp();

$this->l10n = $this->getMockBuilder(IL10N::class)
->disableOriginalConstructor()->getMock();
$this->l10n->expects($this->any())
->method('t')
->willReturnCallback(function ($message, array $replace) {
return vsprintf($message, $replace);
});
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->checker = $this->createMock(Checker::class);
}

public function testSkipOnDisabled(): void {
$this->checker->expects($this->atLeastOnce())
->method('isCodeCheckEnforced')
->willReturn(false);

$check = new CodeIntegrity(
$this->l10n,
$this->urlGenerator,
$this->checker,
);
$this->assertEquals(SetupResult::INFO, $check->run()->getSeverity());
}

public function testSuccessOnEmptyResults(): void {
$this->checker->expects($this->atLeastOnce())
->method('isCodeCheckEnforced')
->willReturn(true);
$this->checker->expects($this->atLeastOnce())
->method('getResults')
->willReturn([]);
$this->checker->expects(($this->atLeastOnce()))
->method('hasPassedCheck')
->willReturn(true);

$check = new CodeIntegrity(
$this->l10n,
$this->urlGenerator,
$this->checker,
);
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
}

public function testCheckerIsReRunWithoutResults(): void {
$this->checker->expects($this->atLeastOnce())
->method('isCodeCheckEnforced')
->willReturn(true);
$this->checker->expects($this->atLeastOnce())
->method('getResults')
->willReturn(null);
$this->checker->expects(($this->atLeastOnce()))
->method('hasPassedCheck')
->willReturn(true);

// This is important and must be called
$this->checker->expects($this->once())
->method('runInstanceVerification');

$check = new CodeIntegrity(
$this->l10n,
$this->urlGenerator,
$this->checker,
);
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
}

public function testCheckerIsNotReReInAdvance(): void {
$this->checker->expects($this->atLeastOnce())
->method('isCodeCheckEnforced')
->willReturn(true);
$this->checker->expects($this->atLeastOnce())
->method('getResults')
->willReturn(['mocked']);
$this->checker->expects(($this->atLeastOnce()))
->method('hasPassedCheck')
->willReturn(true);

// There are results thus this must never be called
$this->checker->expects($this->never())
->method('runInstanceVerification');

$check = new CodeIntegrity(
$this->l10n,
$this->urlGenerator,
$this->checker,
);
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
}

public function testErrorOnMissingIntegrity(): void {
$this->checker->expects($this->atLeastOnce())
->method('isCodeCheckEnforced')
->willReturn(true);
$this->checker->expects($this->atLeastOnce())
->method('getResults')
->willReturn(['mocked']);
$this->checker->expects(($this->atLeastOnce()))
->method('hasPassedCheck')
->willReturn(false);

$check = new CodeIntegrity(
$this->l10n,
$this->urlGenerator,
$this->checker,
);
$this->assertEquals(SetupResult::ERROR, $check->run()->getSeverity());
}
}
8 changes: 4 additions & 4 deletions lib/private/IntegrityCheck/Checker.php
Original file line number Diff line number Diff line change
Expand Up @@ -427,17 +427,17 @@ private function verify(string $signaturePath, string $basePath, string $certifi
*/
public function hasPassedCheck(): bool {
$results = $this->getResults();
if (empty($results)) {
if ($results !== null && empty($results)) {
return true;
}

return false;
}

/**
* @return array
* @return array|null Either the results or null if no results available
*/
public function getResults(): array {
public function getResults(): array|null {
$cachedResults = $this->cache->get(self::CACHE_KEY);
if (!\is_null($cachedResults) and $cachedResults !== false) {
return json_decode($cachedResults, true);
Expand All @@ -456,7 +456,7 @@ public function getResults(): array {
* @param array $result
*/
private function storeResults(string $scope, array $result) {
$resultArray = $this->getResults();
$resultArray = $this->getResults() ?? [];
unset($resultArray[$scope]);
if (!empty($result)) {
$resultArray[$scope] = $result;
Expand Down

0 comments on commit 2c2a62a

Please sign in to comment.