Excludes not writable app roots from the directory permission check

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

settings/Controller/CheckSetupController.php

@@ -542,8 +542,30 @@ Raw output
 		$appDirsWithDifferentOwner = [];
 
 		foreach (OC::$APPSROOTS as $appRoot) {
+			if ($appRoot['writable'] === true) {
+				$appDirsWithDifferentOwner = array_merge(
+					$appDirsWithDifferentOwner,
+					$this->getAppDirsWithDifferentOwnerForAppRoot($currentUser, $appRoot)
+				);
+			}
+		}
+
+		sort($appDirsWithDifferentOwner);
+		return $appDirsWithDifferentOwner;
+	}
+
+	/**
+	 * Tests if the directories for one apps directory are writable by the current user.
+	 *
+	 * @param array $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(array $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();
@@ -553,9 +575,7 @@ Raw output
 				}
 			}
 		}
-		}
 
-		sort($appDirsWithDifferentOwner);
 		return $appDirsWithDifferentOwner;
 	}
 

tests/Settings/Controller/CheckSetupControllerTest.php

@@ -629,6 +629,27 @@ class CheckSetupControllerTest extends TestCase {
 		);
 	}
 
+	/**
+	 * 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->checkSetupController, 'getAppDirsWithDifferentOwner')
+		);
+	}
+
 	public function testIsBuggyNss400() {
 		$this->config->expects($this->any())
 			->method('getSystemValue')