Skip to content

Commit 1b8dbcb

Browse files
authored
Merge pull request #46456 from nextcloud/fix/check-datadir
2 parents 8c0bece + 0563757 commit 1b8dbcb

File tree

12 files changed

+84
-32
lines changed

12 files changed

+84
-32
lines changed

apps/settings/lib/SetupChecks/DataDirectoryProtected.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,21 @@ public function getName(): string {
4242
public function run(): SetupResult {
4343
$datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
4444

45-
$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ocdata';
45+
$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ncdata';
4646

4747
$noResponse = true;
48-
foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) {
48+
foreach ($this->runRequest('GET', $dataUrl, [ 'httpErrors' => false ]) as $response) {
4949
$noResponse = false;
50-
if ($response->getStatusCode() === 200) {
51-
return SetupResult::error($this->l10n->t('Your data directory and files are probably accessible from the internet. The .htaccess file is not working. It is strongly recommended that you configure your web server so that the data directory is no longer accessible, or move the data directory outside the web server document root.'));
50+
if ($response->getStatusCode() < 400) {
51+
// Read the response body
52+
$body = $response->getBody();
53+
if (is_resource($body)) {
54+
$body = stream_get_contents($body, 64);
55+
}
56+
57+
if (str_contains($body, '# Nextcloud data directory')) {
58+
return SetupResult::error($this->l10n->t('Your data directory and files are probably accessible from the internet. The .htaccess file is not working. It is strongly recommended that you configure your web server so that the data directory is no longer accessible, or move the data directory outside the web server document root.'));
59+
}
5260
} else {
5361
$this->logger->debug('[expected] Could not access data directory from outside.', ['url' => $dataUrl]);
5462
}

apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ protected function setUp(): void {
4545
$this->logger = $this->createMock(LoggerInterface::class);
4646

4747
$this->setupcheck = $this->getMockBuilder(DataDirectoryProtected::class)
48-
->onlyMethods(['runHEAD'])
48+
->onlyMethods(['runRequest'])
4949
->setConstructorArgs([
5050
$this->l10n,
5151
$this->config,
@@ -59,16 +59,17 @@ protected function setUp(): void {
5959
/**
6060
* @dataProvider dataTestStatusCode
6161
*/
62-
public function testStatusCode(array $status, string $expected): void {
63-
$responses = array_map(function ($state) {
62+
public function testStatusCode(array $status, string $expected, bool $hasBody): void {
63+
$responses = array_map(function ($state) use ($hasBody) {
6464
$response = $this->createMock(IResponse::class);
6565
$response->expects($this->any())->method('getStatusCode')->willReturn($state);
66+
$response->expects(($this->atMost(1)))->method('getBody')->willReturn($hasBody ? '# Nextcloud data directory' : 'something else');
6667
return $response;
6768
}, $status);
6869

6970
$this->setupcheck
7071
->expects($this->once())
71-
->method('runHEAD')
72+
->method('runRequest')
7273
->will($this->generate($responses));
7374

7475
$this->config
@@ -82,10 +83,11 @@ public function testStatusCode(array $status, string $expected): void {
8283

8384
public function dataTestStatusCode(): array {
8485
return [
85-
'success: forbidden access' => [[403], SetupResult::SUCCESS],
86-
'error: can access' => [[200], SetupResult::ERROR],
87-
'error: one forbidden one can access' => [[403, 200], SetupResult::ERROR],
88-
'warning: connection issue' => [[], SetupResult::WARNING],
86+
'success: forbidden access' => [[403], SetupResult::SUCCESS, true],
87+
'success: forbidden access with redirect' => [[200], SetupResult::SUCCESS, false],
88+
'error: can access' => [[200], SetupResult::ERROR, true],
89+
'error: one forbidden one can access' => [[403, 200], SetupResult::ERROR, true],
90+
'warning: connection issue' => [[], SetupResult::WARNING, true],
8991
];
9092
}
9193

@@ -95,7 +97,7 @@ public function testNoResponse(): void {
9597

9698
$this->setupcheck
9799
->expects($this->once())
98-
->method('runHEAD')
100+
->method('runRequest')
99101
->will($this->generate([]));
100102

101103
$this->config

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1793,6 +1793,7 @@
17931793
'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
17941794
'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
17951795
'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
1796+
'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
17961797
'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
17971798
'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php',
17981799
'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,6 +1826,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
18261826
'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
18271827
'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
18281828
'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
1829+
'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
18291830
'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
18301831
'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php',
18311832
'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php',

lib/private/Repair.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use OC\Repair\NC22\LookupServerSendCheck;
4242
use OC\Repair\NC24\AddTokenCleanupJob;
4343
use OC\Repair\NC25\AddMissingSecretJob;
44+
use OC\Repair\NC30\RemoveLegacyDatadirFile;
4445
use OC\Repair\OldGroupMembershipShares;
4546
use OC\Repair\Owncloud\CleanPreviews;
4647
use OC\Repair\Owncloud\DropAccountTermsTable;
@@ -187,6 +188,7 @@ public static function getRepairSteps(): array {
187188
\OCP\Server::get(AddMetadataGenerationJob::class),
188189
\OCP\Server::get(AddAppConfigLazyMigration::class),
189190
\OCP\Server::get(RepairLogoDimension::class),
191+
\OCP\Server::get(RemoveLegacyDatadirFile::class),
190192
];
191193
}
192194

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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 OC\Repair\NC30;
10+
11+
use OCP\IConfig;
12+
use OCP\Migration\IOutput;
13+
use OCP\Migration\IRepairStep;
14+
15+
class RemoveLegacyDatadirFile implements IRepairStep {
16+
17+
public function __construct(
18+
private IConfig $config,
19+
) {
20+
}
21+
22+
public function getName(): string {
23+
return 'Remove legacy ".ocdata" file';
24+
}
25+
26+
public function run(IOutput $output): void {
27+
$ocdata = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata';
28+
if (file_exists($ocdata)) {
29+
unlink($ocdata);
30+
}
31+
}
32+
}

lib/private/Setup.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,12 @@ public function install(array $options, ?IOutput $output = null): array {
360360
Installer::installShippedApps(false, $output);
361361

362362
// create empty file in data dir, so we can later find
363-
// out that this is indeed an ownCloud data directory
363+
// out that this is indeed a Nextcloud data directory
364364
$this->outputDebug($output, 'Setup data directory');
365-
file_put_contents($config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
365+
file_put_contents(
366+
$config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
367+
"# Nextcloud data directory\n# Do not change this file",
368+
);
366369

367370
// Update .htaccess files
368371
self::updateHtaccess();

lib/private/Updater.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,12 @@ private function doUpgrade(string $currentVersion, string $installedVersion): vo
208208
}
209209

210210
// create empty file in data dir, so we can later find
211-
// out that this is indeed an ownCloud data directory
211+
// out that this is indeed a Nextcloud data directory
212212
// (in case it didn't exist before)
213-
file_put_contents($this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
213+
file_put_contents(
214+
$this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
215+
"# Nextcloud data directory\n# Do not change this file",
216+
);
214217

215218
// pre-upgrade repairs
216219
$repair = \OCP\Server::get(Repair::class);

lib/private/User/Manager.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ private function verifyUid(string $uid, bool $checkDataDirectory = false): bool
783783
'.htaccess',
784784
'files_external',
785785
'__groupfolders',
786-
'.ocdata',
786+
'.ncdata',
787787
'owncloud.log',
788788
'nextcloud.log',
789789
'updater.log',

lib/private/legacy/OC_Util.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ public static function checkDataDirectoryPermissions($dataDirectory) {
687687

688688
/**
689689
* Check that the data directory exists and is valid by
690-
* checking the existence of the ".ocdata" file.
690+
* checking the existence of the ".ncdata" file.
691691
*
692692
* @param string $dataDirectory data directory path
693693
* @return array errors found
@@ -701,11 +701,11 @@ public static function checkDataDirectoryValidity($dataDirectory) {
701701
'hint' => $l->t('Check the value of "datadirectory" in your configuration.')
702702
];
703703
}
704-
if (!file_exists($dataDirectory . '/.ocdata')) {
704+
705+
if (!file_exists($dataDirectory . '/.ncdata')) {
705706
$errors[] = [
706707
'error' => $l->t('Your data directory is invalid.'),
707-
'hint' => $l->t('Ensure there is a file called ".ocdata"' .
708-
' in the root of the data directory.')
708+
'hint' => $l->t('Ensure there is a file called "%1$s" in the root of the data directory. It should have the content: "%2$s"', ['.ncdata', '# Nextcloud data directory']),
709709
];
710710
}
711711
return $errors;

0 commit comments

Comments
 (0)