mirror of https://github.com/nextcloud/server.git
Merge pull request #42176 from nextcloud/feat/migrate-appdirowner-setupcheck
Migrate app dir owner check to new SetupCheck APIpull/42643/head
Côme Chilliet
5 months ago
committed by
GitHub
commit
012dcbbccc
@ -0,0 +1,104 @@
|
||||
<?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 OCP\IL10N;
|
||||
use OCP\SetupCheck\ISetupCheck;
|
||||
use OCP\SetupCheck\SetupResult;
|
||||
|
||||
class AppDirsWithDifferentOwner implements ISetupCheck {
|
||||
public function __construct(
|
||||
private IL10N $l10n,
|
||||
) {
|
||||
}
|
||||
|
||||
public function getName(): string {
|
||||
return $this->l10n->t('App directories owner');
|
||||
}
|
||||
|
||||
public function getCategory(): string {
|
||||
return 'security';
|
||||
}
|
||||
|
||||
/**
|
||||
* Iterates through the configured app roots and
|
||||
* tests if the subdirectories are owned by the same user than the current user.
|
||||
*
|
||||
* @return string[]
|
||||
*/
|
||||
private function getAppDirsWithDifferentOwner(int $currentUser): array {
|
||||
$appDirsWithDifferentOwner = [[]];
|
||||
|
||||
foreach (\OC::$APPSROOTS as $appRoot) {
|
||||
if ($appRoot['writable'] === true) {
|
||||
$appDirsWithDifferentOwner[] = $this->getAppDirsWithDifferentOwnerForAppRoot($currentUser, $appRoot);
|
||||
}
|
||||
}
|
||||
|
||||
$appDirsWithDifferentOwner = array_merge(...$appDirsWithDifferentOwner);
|
||||
sort($appDirsWithDifferentOwner);
|
||||
|
||||
return $appDirsWithDifferentOwner;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests if the directories for one apps directory are writable by the current user.
|
||||
*
|
||||
* @param int $currentUser The current user
|
||||
* @param array $appRoot The app root config
|
||||
* @return string[] The none writable directory paths inside the app root
|
||||
*/
|
||||
private function getAppDirsWithDifferentOwnerForAppRoot(int $currentUser, array $appRoot): array {
|
||||
$appDirsWithDifferentOwner = [];
|
||||
$appsPath = $appRoot['path'];
|
||||
$appsDir = new \DirectoryIterator($appRoot['path']);
|
||||
|
||||
foreach ($appsDir as $fileInfo) {
|
||||
if ($fileInfo->isDir() && !$fileInfo->isDot()) {
|
||||
$absAppPath = $appsPath . DIRECTORY_SEPARATOR . $fileInfo->getFilename();
|
||||
$appDirUser = fileowner($absAppPath);
|
||||
if ($appDirUser !== $currentUser) {
|
||||
$appDirsWithDifferentOwner[] = $absAppPath;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $appDirsWithDifferentOwner;
|
||||
}
|
||||
|
||||
public function run(): SetupResult {
|
||||
$currentUser = posix_getuid();
|
||||
$currentUserInfos = posix_getpwuid($currentUser) ?: [];
|
||||
$appDirsWithDifferentOwner = $this->getAppDirsWithDifferentOwner($currentUser);
|
||||
if (count($appDirsWithDifferentOwner) > 0) {
|
||||
return SetupResult::warning(
|
||||
$this->l10n->t("Some app directories are owned by a different user than the web server one. This may be the case if apps have been installed manually. Check the permissions of the following app directories:\n%s", implode("\n", $appDirsWithDifferentOwner))
|
||||
);
|
||||
} else {
|
||||
return SetupResult::success($this->l10n->t('App directories have the correct owner "%s"', [$currentUserInfos['name'] ?? '']));
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,100 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* @copyright Copyright (c) 2024 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\Tests;
|
||||
|
||||
use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner;
|
||||
use OCP\IL10N;
|
||||
use Test\TestCase;
|
||||
|
||||
class AppDirsWithDifferentOwnerTest extends TestCase {
|
||||
private IL10N $l10n;
|
||||
private AppDirsWithDifferentOwner $check;
|
||||
|
||||
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->check = new AppDirsWithDifferentOwner(
|
||||
$this->l10n,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Setups a temp directory and some subdirectories.
|
||||
* Then calls the 'getAppDirsWithDifferentOwner' method.
|
||||
* The result is expected to be empty since
|
||||
* there are no directories with different owners than the current user.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testAppDirectoryOwnersOk() {
|
||||
$tempDir = tempnam(sys_get_temp_dir(), 'apps') . 'dir';
|
||||
mkdir($tempDir);
|
||||
mkdir($tempDir . DIRECTORY_SEPARATOR . 'app1');
|
||||
mkdir($tempDir . DIRECTORY_SEPARATOR . 'app2');
|
||||
$this->dirsToRemove[] = $tempDir . DIRECTORY_SEPARATOR . 'app1';
|
||||
$this->dirsToRemove[] = $tempDir . DIRECTORY_SEPARATOR . 'app2';
|
||||
$this->dirsToRemove[] = $tempDir;
|
||||
\OC::$APPSROOTS = [
|
||||
[
|
||||
'path' => $tempDir,
|
||||
'url' => '/apps',
|
||||
'writable' => true,
|
||||
],
|
||||
];
|
||||
$this->assertSame(
|
||||
[],
|
||||
$this->invokePrivate($this->check, 'getAppDirsWithDifferentOwner', [posix_getuid()])
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Calls the check for a none existing app root that is marked as not writable.
|
||||
* It's expected that no error happens since the check shouldn't apply.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function testAppDirectoryOwnersNotWritable() {
|
||||
$tempDir = tempnam(sys_get_temp_dir(), 'apps') . 'dir';
|
||||
\OC::$APPSROOTS = [
|
||||
[
|
||||
'path' => $tempDir,
|
||||
'url' => '/apps',
|
||||
'writable' => false,
|
||||
],
|
||||
];
|
||||
$this->assertSame(
|
||||
[],
|
||||
$this->invokePrivate($this->check, 'getAppDirsWithDifferentOwner', [posix_getuid()])
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue