10 files changed, 267 insertions, 35 deletions

diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js
index 71192497143..84e4e62f8a1 100644
--- a/apps/files_sharing/js/sharedfilelist.js
+++ b/apps/files_sharing/js/sharedfilelist.js
@@ -166,24 +166,7 @@
 		},
 
 		updateRow: function($tr, fileInfo, options) {
-			if(!fileInfo instanceof OCA.Sharing.SharedFileInfo) {
-				// recycle SharedFileInfo values if something tries to overwrite it
-				var oldModel = this.getModelForFile($tr);
-
-				if(_.isUndefined(fileInfo.recipientData) && oldModel.recipientData) {
-					fileInfo.recipientData = oldModel.recipientData;
-				}
-				if(_.isUndefined(fileInfo.recipients) && oldModel.recipientData) {
-					fileInfo.recipientData = oldModel.recipientData;
-				}
-				if(_.isUndefined(fileInfo.shares) && oldModel.shares) {
-					fileInfo.shares = oldModel.shares;
-				}
-				if(_.isUndefined(fileInfo.shareOwner) && oldModel.shareOwner) {
-					fileInfo.shareOwner = oldModel.shareOwner;
-				}
-			}
-			OCA.Files.FileList.prototype._createRow.updateRow(this, arguments);
+			// no-op, suppress re-rendering
 		},
 
 		reload: function() {
@@ -328,11 +311,11 @@
 		 * Converts the OCS API share response data to a file info
 		 * list
 		 * @param {Array} data OCS API share array
+		 * @param {bool} sharedWithUser
 		 * @return {Array.<OCA.Sharing.SharedFileInfo>} array of shared file info
 		 */
 		_makeFilesFromShares: function(data, sharedWithUser) {
 			/* jshint camelcase: false */
-			var self = this;
 			var files = data;
 
 			if (this._linksOnly) {
@@ -447,8 +430,8 @@
 					// array of sorted names
 					data.mountType = 'shared';
 					delete data.recipientsCount;
-					if (self._sharedWithUser) {
-						// only for outgoing shres
+					if (sharedWithUser) {
+						// only for outgoing shares
 						delete data.shareTypes;
 					} else {
 						data.shareTypes = _.keys(data.shareTypes);
diff --git a/build/integration/composer.json b/build/integration/composer.json
index 61075d3a828..7061d8dbde9 100644
--- a/build/integration/composer.json
+++ b/build/integration/composer.json
@@ -1,7 +1,7 @@
 {
   "require-dev": {
     "phpunit/phpunit": "~6.5",
-    "behat/behat": "~3.3.0",
+    "behat/behat": "~3.5.0",
     "guzzlehttp/guzzle": "6.3.3",
     "jarnaiz/behat-junit-formatter": "^1.3",
     "sabre/dav": "3.2.2"
diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index 7dabefe9e8f..628606a9e5b 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -325,6 +325,23 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
 						})
 					}
+
+					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
+						});
+					}
+
 				} else {
 					messages.push({
 						msg: t('core', 'Error occurred while checking server setup'),
diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js
index 241b9c19c0d..c28d85efbf0 100644
--- a/core/js/shareitemmodel.js
+++ b/core/js/shareitemmodel.js
@@ -777,7 +777,7 @@
 				return {};
 			}
 
-			var permissions = this.get('possiblePermissions');
+			var permissions = this.fileInfoModel.get('permissions');
 			if(!_.isUndefined(data.reshare) && !_.isUndefined(data.reshare.permissions) && data.reshare.uid_owner !== OC.currentUser) {
 				permissions = permissions & data.reshare.permissions;
 			}
diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js
index 66c84f52bc9..4c0545b7b4f 100644
--- a/core/js/tests/specs/setupchecksSpec.js
+++ b/core/js/tests/specs/setupchecksSpec.js
@@ -171,7 +171,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -219,7 +220,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -268,7 +270,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -315,7 +318,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -360,7 +364,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -373,6 +378,54 @@ 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({
+					hasFileinfoInstalled: true,
+					isGetenvServerWorking: true,
+					isReadOnlyConfig: false,
+					hasWorkingFileLocking: true,
+					hasValidTransactionIsolationLevel: true,
+					suggestedOverwriteCliURL: '',
+					isUrandomAvailable: true,
+					securityDocs: 'https://docs.owncloud.org/myDocs.html',
+					serverHasInternetConnection: true,
+					isMemcacheConfigured: true,
+					forwardedForHeadersWorking: true,
+					isCorrectMemcachedPHPModuleInstalled: true,
+					hasPassedCodeIntegrityCheck: true,
+					isOpcacheProperlySetup: true,
+					hasOpcacheLoaded: true,
+					isSettimelimitAvailable: true,
+					hasFreeTypeSupport: true,
+					missingIndexes: [],
+					outdatedCaches: [],
+					cronErrors: [],
+					cronInfo: {
+						diffInSeconds: 0
+					},
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: [
+						'/some/path'
+					]
+				})
+			);
+
+			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 the forwarded for headers are not working', function(done) {
 			var async = OC.SetupChecks.checkSetup();
 
@@ -405,7 +458,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -450,7 +504,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -495,6 +550,7 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
+					appDirsWithDifferentOwner: [],
 					isMemoryLimitSufficient: false
 				})
 			);
@@ -561,7 +617,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -607,7 +664,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -653,7 +711,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
@@ -699,7 +758,8 @@ describe('OC.SetupChecks tests', function() {
 					cronInfo: {
 						diffInSeconds: 0
 					},
-					isMemoryLimitSufficient: true
+					isMemoryLimitSufficient: true,
+					appDirsWithDifferentOwner: []
 				})
 			);
 
diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js
index 0a345786b73..2e89b2e3cda 100644
--- a/core/js/tests/specs/shareitemmodelSpec.js
+++ b/core/js/tests/specs/shareitemmodelSpec.js
@@ -345,7 +345,7 @@ describe('OC.Share.ShareItemModel', function() {
 			}]));
 			fetchSharesDeferred.resolve(makeOcsResponse([]));
 
-			model.set('possiblePermissions', OC.PERMISSION_READ);
+			model.fileInfoModel.set('permissions', OC.PERMISSION_READ);
 			model.fetch();
 
 			// no resharing allowed
diff --git a/lib/private/MemoryInfo.php b/lib/private/MemoryInfo.php
index 0501d3fd049..4b8d2bce37b 100644
--- a/lib/private/MemoryInfo.php
+++ b/lib/private/MemoryInfo.php
@@ -1,4 +1,25 @@
 <?php
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2018, Michael Weimann (<mail@michael-weimann.eu>)
+ *
+ * @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 OC;
 
diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php
index a64d131648b..747e60c7cb2 100644
--- a/settings/Controller/CheckSetupController.php
+++ b/settings/Controller/CheckSetupController.php
@@ -31,9 +31,11 @@
 namespace OC\Settings\Controller;
 
 use bantu\IniGetWrapper\IniGetWrapper;
+use DirectoryIterator;
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use GuzzleHttp\Exception\ClientException;
+use OC;
 use OC\AppFramework\Http;
 use OC\DB\Connection;
 use OC\DB\MissingIndexInformation;
@@ -535,6 +537,54 @@ Raw output
 	}
 
 	/**
+	 * 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_getpwuid(posix_getuid());
+		$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();
+				$appDirUser = posix_getpwuid(fileowner($absAppPath));
+				if ($appDirUser !== $currentUser) {
+					$appDirsWithDifferentOwner[] = $absAppPath . DIRECTORY_SEPARATOR . $fileInfo->getFilename();
+				}
+			}
+		}
+
+		return $appDirsWithDifferentOwner;
+	}
+
+	/**
 	 * @return DataResponse
 	 */
 	public function check() {
@@ -572,6 +622,7 @@ Raw output
 				'isPhpMailerUsed' => $this->isPhpMailerUsed(),
 				'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin'),
 				'isMemoryLimitSufficient' => $this->memoryInfo->isMemoryLimitSufficient(),
+				'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
 			]
 		);
 	}
diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php
index a7689eed801..cc1ed6555c3 100644
--- a/tests/Settings/Controller/CheckSetupControllerTest.php
+++ b/tests/Settings/Controller/CheckSetupControllerTest.php
@@ -21,6 +21,7 @@
 
 namespace Tests\Settings\Controller;
 
+use OC;
 use OC\DB\Connection;
 use OC\MemoryInfo;
 use OC\Settings\Controller\CheckSetupController;
@@ -46,6 +47,7 @@ use OC\IntegrityCheck\Checker;
 /**
  * Class CheckSetupControllerTest
  *
+ * @backupStaticAttributes
  * @package Tests\Settings\Controller
  */
 class CheckSetupControllerTest extends TestCase {
@@ -78,6 +80,13 @@ class CheckSetupControllerTest extends TestCase {
 	/** @var MemoryInfo|MockObject */
 	private $memoryInfo;
 
+	/**
+	 * Holds a list of directories created during tests.
+	 *
+	 * @var array
+	 */
+	private $dirsToRemove = [];
+
 	public function setUp() {
 		parent::setUp();
 
@@ -143,9 +152,23 @@ class CheckSetupControllerTest extends TestCase {
 				'isSqliteUsed',
 				'isPhpMailerUsed',
 				'hasOpcacheLoaded',
+				'getAppDirsWithDifferentOwner',
 			])->getMock();
 	}
 
+	/**
+	 * Removes directories created during tests.
+	 *
+	 * @after
+	 * @return void
+	 */
+	public function removeTestDirectories() {
+		foreach ($this->dirsToRemove as $dirToRemove) {
+			rmdir($dirToRemove);
+		}
+		$this->dirsToRemove = [];
+	}
+
 	public function testIsInternetConnectionWorkingDisabledViaConfig() {
 		$this->config->expects($this->once())
 			->method('getSystemValue')
@@ -436,6 +459,11 @@ class CheckSetupControllerTest extends TestCase {
 			->method('isMemoryLimitSufficient')
 			->willReturn(true);
 
+		$this->checkSetupController
+			->expects($this->once())
+			->method('getAppDirsWithDifferentOwner')
+			->willReturn([]);
+
 		$expected = new DataResponse(
 			[
 				'isGetenvServerWorking' => true,
@@ -477,6 +505,7 @@ class CheckSetupControllerTest extends TestCase {
 				'isPhpMailerUsed' => false,
 				'mailSettingsDocumentation' => 'https://server/index.php/settings/admin',
 				'isMemoryLimitSufficient' => true,
+				'appDirsWithDifferentOwner' => [],
 			]
 		);
 		$this->assertEquals($expected, $this->checkSetupController->check());
@@ -584,6 +613,56 @@ class CheckSetupControllerTest extends TestCase {
 		$this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated'));
 	}
 
+	/**
+	 * 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->checkSetupController, 'getAppDirsWithDifferentOwner')
+		);
+	}
+
+	/**
+	 * 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')
diff --git a/tests/lib/MemoryInfoTest.php b/tests/lib/MemoryInfoTest.php
index 057d3091b2c..489ef51d373 100644
--- a/tests/lib/MemoryInfoTest.php
+++ b/tests/lib/MemoryInfoTest.php
@@ -1,4 +1,25 @@
 <?php
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2018, Michael Weimann (<mail@michael-weimann.eu>)
+ *
+ * @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 Test;