Skip to content

Commit 8cb0f1a

Browse files
skjnldsvskjnldsv
authored
Merge pull request #46174 from nextcloud/fix/integrity-check
fix(IntegrityCheck): Ensure the check is run if no results are available
2 parents 0116a63 + 8fc498f commit 8cb0f1a

File tree

4 files changed

+157
-6
lines changed

4 files changed

+157
-6
lines changed

‎apps/settings/lib/Controller/CheckSetupController.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
8585

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

88+
if ($completeResults === null) {
89+
return new DataDisplayResponse('Integrity checker has not been run. Integrity information not available.');
90+
}
91+
8892
if (!empty($completeResults)) {
8993
$formattedTextResponse = 'Technical information
9094
=====================

‎apps/settings/lib/SetupChecks/CodeIntegrity.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,14 @@ public function getCategory(): string {
3333
public function run(): SetupResult {
3434
if (!$this->checker->isCodeCheckEnforced()) {
3535
return SetupResult::info($this->l10n->t('Integrity checker has been disabled. Integrity cannot be verified.'));
36-
} elseif ($this->checker->hasPassedCheck()) {
36+
}
37+
38+
// If there are no results we need to run the verification
39+
if ($this->checker->getResults() === null) {
40+
$this->checker->runInstanceVerification();
41+
}
42+
43+
if ($this->checker->hasPassedCheck()) {
3744
return SetupResult::success($this->l10n->t('No altered files'));
3845
} else {
3946
return SetupResult::error(

‎apps/settings/tests/SetupChecks/CodeIntegrityTest.php‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\Settings\Tests;
10+
11+
use OC\IntegrityCheck\Checker;
12+
use OCA\Settings\SetupChecks\CodeIntegrity;
13+
use OCP\IL10N;
14+
use OCP\IURLGenerator;
15+
use OCP\SetupCheck\SetupResult;
16+
use PHPUnit\Framework\MockObject\MockObject;
17+
use Test\TestCase;
18+
19+
class CodeIntegrityTest extends TestCase {
20+
21+
private IL10N&MockObject $l10n;
22+
private IURLGenerator&MockObject $urlGenerator;
23+
private Checker&MockObject $checker;
24+
25+
protected function setUp(): void {
26+
parent::setUp();
27+
28+
$this->l10n = $this->getMockBuilder(IL10N::class)
29+
->disableOriginalConstructor()->getMock();
30+
$this->l10n->expects($this->any())
31+
->method('t')
32+
->willReturnCallback(function ($message, array $replace) {
33+
return vsprintf($message, $replace);
34+
});
35+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
36+
$this->checker = $this->createMock(Checker::class);
37+
}
38+
39+
public function testSkipOnDisabled(): void {
40+
$this->checker->expects($this->atLeastOnce())
41+
->method('isCodeCheckEnforced')
42+
->willReturn(false);
43+
44+
$check = new CodeIntegrity(
45+
$this->l10n,
46+
$this->urlGenerator,
47+
$this->checker,
48+
);
49+
$this->assertEquals(SetupResult::INFO, $check->run()->getSeverity());
50+
}
51+
52+
public function testSuccessOnEmptyResults(): void {
53+
$this->checker->expects($this->atLeastOnce())
54+
->method('isCodeCheckEnforced')
55+
->willReturn(true);
56+
$this->checker->expects($this->atLeastOnce())
57+
->method('getResults')
58+
->willReturn([]);
59+
$this->checker->expects(($this->atLeastOnce()))
60+
->method('hasPassedCheck')
61+
->willReturn(true);
62+
63+
$check = new CodeIntegrity(
64+
$this->l10n,
65+
$this->urlGenerator,
66+
$this->checker,
67+
);
68+
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
69+
}
70+
71+
public function testCheckerIsReRunWithoutResults(): void {
72+
$this->checker->expects($this->atLeastOnce())
73+
->method('isCodeCheckEnforced')
74+
->willReturn(true);
75+
$this->checker->expects($this->atLeastOnce())
76+
->method('getResults')
77+
->willReturn(null);
78+
$this->checker->expects(($this->atLeastOnce()))
79+
->method('hasPassedCheck')
80+
->willReturn(true);
81+
82+
// This is important and must be called
83+
$this->checker->expects($this->once())
84+
->method('runInstanceVerification');
85+
86+
$check = new CodeIntegrity(
87+
$this->l10n,
88+
$this->urlGenerator,
89+
$this->checker,
90+
);
91+
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
92+
}
93+
94+
public function testCheckerIsNotReReInAdvance(): void {
95+
$this->checker->expects($this->atLeastOnce())
96+
->method('isCodeCheckEnforced')
97+
->willReturn(true);
98+
$this->checker->expects($this->atLeastOnce())
99+
->method('getResults')
100+
->willReturn(['mocked']);
101+
$this->checker->expects(($this->atLeastOnce()))
102+
->method('hasPassedCheck')
103+
->willReturn(true);
104+
105+
// There are results thus this must never be called
106+
$this->checker->expects($this->never())
107+
->method('runInstanceVerification');
108+
109+
$check = new CodeIntegrity(
110+
$this->l10n,
111+
$this->urlGenerator,
112+
$this->checker,
113+
);
114+
$this->assertEquals(SetupResult::SUCCESS, $check->run()->getSeverity());
115+
}
116+
117+
public function testErrorOnMissingIntegrity(): void {
118+
$this->checker->expects($this->atLeastOnce())
119+
->method('isCodeCheckEnforced')
120+
->willReturn(true);
121+
$this->checker->expects($this->atLeastOnce())
122+
->method('getResults')
123+
->willReturn(['mocked']);
124+
$this->checker->expects(($this->atLeastOnce()))
125+
->method('hasPassedCheck')
126+
->willReturn(false);
127+
128+
$check = new CodeIntegrity(
129+
$this->l10n,
130+
$this->urlGenerator,
131+
$this->checker,
132+
);
133+
$this->assertEquals(SetupResult::ERROR, $check->run()->getSeverity());
134+
}
135+
}

‎lib/private/IntegrityCheck/Checker.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -373,23 +373,28 @@ private function verify(string $signaturePath, string $basePath, string $certifi
373373
*/
374374
public function hasPassedCheck(): bool {
375375
$results = $this->getResults();
376-
if (empty($results)) {
376+
if ($results !== null && empty($results)) {
377377
return true;
378378
}
379379

380380
return false;
381381
}
382382

383383
/**
384-
* @return array
384+
* @return array|null Either the results or null if no results available
385385
*/
386-
public function getResults(): array {
386+
public function getResults(): array|null {
387387
$cachedResults = $this->cache->get(self::CACHE_KEY);
388388
if (!\is_null($cachedResults) and $cachedResults !== false) {
389389
return json_decode($cachedResults, true);
390390
}
391391

392-
return $this->appConfig?->getValueArray('core', self::CACHE_KEY, lazy: true) ?? [];
392+
if ($this->appConfig?->hasKey('core', self::CACHE_KEY, lazy: true)) {
393+
return $this->appConfig->getValueArray('core', self::CACHE_KEY, lazy: true);
394+
}
395+
396+
// No results available
397+
return null;
393398
}
394399

395400
/**
@@ -399,7 +404,7 @@ public function getResults(): array {
399404
* @param array $result
400405
*/
401406
private function storeResults(string $scope, array $result) {
402-
$resultArray = $this->getResults();
407+
$resultArray = $this->getResults() ?? [];
403408
unset($resultArray[$scope]);
404409
if (!empty($result)) {
405410
$resultArray[$scope] = $result;

0 commit comments

Comments
 (0)