]> source.dussan.org Git - nextcloud-server.git/commitdiff
Migrate app dir owner check to SetupCheck API
authorCôme Chilliet <come.chilliet@nextcloud.com>
Tue, 12 Dec 2023 15:53:13 +0000 (16:53 +0100)
committerCôme Chilliet <come.chilliet@nextcloud.com>
Mon, 8 Jan 2024 16:37:14 +0000 (17:37 +0100)
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
apps/settings/composer/composer/autoload_classmap.php
apps/settings/composer/composer/autoload_static.php
apps/settings/lib/AppInfo/Application.php
apps/settings/lib/Controller/CheckSetupController.php
apps/settings/lib/SetupChecks/AppDirsWithDifferentOwner.php [new file with mode: 0644]
apps/settings/tests/Controller/CheckSetupControllerTest.php
core/js/setupchecks.js
core/js/tests/specs/setupchecksSpec.js

index b32aab9756955a955f435a19d2c05c69dc84db73..60717a472944924fd7afff9ff60d284845a5aa7e 100644 (file)
@@ -76,6 +76,7 @@ return array(
     'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php',
     'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => $baseDir . '/../lib/Settings/Personal/Security/WebAuthn.php',
     'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/../lib/Settings/Personal/ServerDevNotice.php',
+    '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\\DatabaseHasMissingColumns' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
index 09e0d724e4ccb5cb1354daf4229b3429adbdfea0..7cd3892a6ec830da830f16968c2f3db87ad921a8 100644 (file)
@@ -91,6 +91,7 @@ class ComposerStaticInitSettings
         'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php',
         'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/WebAuthn.php',
         'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/..' . '/../lib/Settings/Personal/ServerDevNotice.php',
+        '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\\DatabaseHasMissingColumns' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
index 7e963e355930b70a80e4ab0a8ea9c838a3ff3109..f3e2343e04886d3b76d4151d461f2e676ebd09c7 100644 (file)
@@ -48,6 +48,7 @@ use OCA\Settings\Middleware\SubadminMiddleware;
 use OCA\Settings\Search\AppSearch;
 use OCA\Settings\Search\SectionSearch;
 use OCA\Settings\Search\UserSearch;
+use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner;
 use OCA\Settings\SetupChecks\BruteForceThrottler;
 use OCA\Settings\SetupChecks\CheckUserCertificates;
 use OCA\Settings\SetupChecks\DatabaseHasMissingColumns;
@@ -164,6 +165,7 @@ class Application extends App implements IBootstrap {
                                Util::getDefaultEmailAddress('no-reply')
                        );
                });
+               $context->registerSetupCheck(AppDirsWithDifferentOwner::class);
                $context->registerSetupCheck(BruteForceThrottler::class);
                $context->registerSetupCheck(CheckUserCertificates::class);
                $context->registerSetupCheck(DatabaseHasMissingColumns::class);
index b251adb0d84f8f1ae2ed6c77979e327f075838de..acd73479ea1481395292375795ea790bfd1e1376 100644 (file)
@@ -45,7 +45,6 @@
  */
 namespace OCA\Settings\Controller;
 
-use DirectoryIterator;
 use GuzzleHttp\Exception\ClientException;
 use OC\AppFramework\Http;
 use OC\IntegrityCheck\Checker;
@@ -343,53 +342,6 @@ Raw output
                return false;
        }
 
-       /**
-        * Iterates through the configured app roots and
-        * tests if the subdirectories are owned by the same user than the current user.
-        *
-        * @return array
-        */
-       protected function getAppDirsWithDifferentOwner(): array {
-               $currentUser = posix_getuid();
-               $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;
-       }
-
        protected function isImagickEnabled(): bool {
                if ($this->config->getAppValue('theming', 'enabled', 'no') === 'yes') {
                        if (!extension_loaded('imagick')) {
@@ -470,7 +422,6 @@ Raw output
                                'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
                                'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
                                'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
-                               'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
                                'isImagickEnabled' => $this->isImagickEnabled(),
                                'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(),
                                'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(),
diff --git a/apps/settings/lib/SetupChecks/AppDirsWithDifferentOwner.php b/apps/settings/lib/SetupChecks/AppDirsWithDifferentOwner.php
new file mode 100644 (file)
index 0000000..3404e8d
--- /dev/null
@@ -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'] ?? '']));
+               }
+       }
+}
index b9c2ddc8282ff95debd6e19a6d03bb2b94c504d4..6336ca852d6338e869c7c89500e38b47d83426aa 100644 (file)
@@ -270,7 +270,6 @@ class CheckSetupControllerTest extends TestCase {
                                'hasPassedCodeIntegrityCheck' => true,
                                'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity',
                                'isSettimelimitAvailable' => true,
-                               'appDirsWithDifferentOwner' => [],
                                'isImagickEnabled' => false,
                                'areWebauthnExtensionsEnabled' => false,
                                'isMysqlUsedWithoutUTF8MB4' => false,
index e85f32dae4ac62de91af4e8a6074c1617f6896c1..4331e8e9cc929204487e355665d424ceec97ae63 100644 (file)
                                                })
                                        }
 
-                                       if(data.appDirsWithDifferentOwner && data.appDirsWithDifferentOwner.length > 0) {
-                                               var appDirsWithDifferentOwner = data.appDirsWithDifferentOwner.reduce(
-                                                       function(appDirsWithDifferentOwner, directory) {
-                                                               return appDirsWithDifferentOwner + '<li>' + directory + '</li>';
-                                                       },
-                                                       ''
-                                               );
-                                               messages.push({
-                                                       msg: t('core', '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:')
-                                                                       + '<ul>' + appDirsWithDifferentOwner + '</ul>',
-                                                       type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-                                               });
-                                       }
                                        if (data.isMysqlUsedWithoutUTF8MB4) {
                                                messages.push({
                                                        msg: t('core', 'MySQL is used as database but does not support 4-byte characters. To be able to handle 4-byte characters (like emojis) without issues in filenames or comments for example it is recommended to enable the 4-byte support in MySQL. For further details read {linkstart}the documentation page about this ↗{linkend}.')
index bb7d118fc52c53951a30c2f768c0bca38d5ec3dc..594b9ca39bc6b5f91b88ec7353c28750b437b249 100644 (file)
@@ -232,7 +232,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -280,7 +279,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -328,7 +326,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -376,7 +373,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -404,54 +400,6 @@ describe('OC.SetupChecks tests', function() {
                        });
                });
 
-               it('should return a warning if there are app directories with wrong permissions', function(done) {
-                       var async = OC.SetupChecks.checkSetup();
-
-                       suite.server.requests[0].respond(
-                               200,
-                               {
-                                       'Content-Type': 'application/json',
-                               },
-                               JSON.stringify({
-                                       suggestedOverwriteCliURL: '',
-                                       isFairUseOfFreePushService: true,
-                                       isCorrectMemcachedPHPModuleInstalled: true,
-                                       hasPassedCodeIntegrityCheck: true,
-                                       isSettimelimitAvailable: true,
-                                       cronErrors: [],
-                                       cronInfo: {
-                                               diffInSeconds: 0
-                                       },
-                                       appDirsWithDifferentOwner: [
-                                               '/some/path'
-                                       ],
-                                       isImagickEnabled: true,
-                                       areWebauthnExtensionsEnabled: true,
-                                       isMysqlUsedWithoutUTF8MB4: false,
-                                       isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true,
-                                       reverseProxyGeneratedURL: 'https://server',
-                                       temporaryDirectoryWritable: true,
-                                       generic: {
-                                               network: {
-                                                       "Internet connectivity": {
-                                                               severity: "success",
-                                                               description: null,
-                                                               linkToDoc: null
-                                                       }
-                                               },
-                                       },
-                               })
-                       );
-
-                       async.done(function( data, s, x ){
-                               expect(data).toEqual([{
-                                       msg: '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:<ul><li>/some/path</li></ul>',
-                                       type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-                               }]);
-                               done();
-                       });
-               });
-
                it('should return an error if set_time_limit is unavailable', function(done) {
                        var async = OC.SetupChecks.checkSetup();
 
@@ -471,7 +419,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -518,7 +465,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -596,7 +542,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -649,7 +594,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: true,
@@ -699,7 +643,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -746,7 +689,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -790,7 +732,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -837,7 +778,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: false,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -884,7 +824,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: false,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -930,7 +869,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,
@@ -983,7 +921,6 @@ describe('OC.SetupChecks tests', function() {
                                        cronInfo: {
                                                diffInSeconds: 0
                                        },
-                                       appDirsWithDifferentOwner: [],
                                        isImagickEnabled: true,
                                        areWebauthnExtensionsEnabled: true,
                                        isMysqlUsedWithoutUTF8MB4: false,