Make sure that permissions stay in sync for share_type 2

When a file/folder is shared with a group and one of the group members
moves this file/folder an extra entry is created in the share table.

When the permission of the group share is updated we used to only
sometimes update the shares for individual users.

* Added intergration tests

2 files changed, 208 insertions, 44 deletions

diff --git a/apps/files_sharing/tests/sharedmount.php b/apps/files_sharing/tests/sharedmount.php
index 7b256588f93..5b625585ebb 100644
--- a/apps/files_sharing/tests/sharedmount.php
+++ b/apps/files_sharing/tests/sharedmount.php
@@ -237,6 +237,141 @@ class Test_Files_Sharing_Mount extends OCA\Files_sharing\Tests\TestCase {
 		);
 	}
 
+	function dataPermissionMovedGroupShare() {
+		$data = [];
+
+		$powerset = function($permissions) {
+			$results = [\OCP\Constants::PERMISSION_READ];
+
+			foreach ($permissions as $permission) {
+				foreach ($results as $combination) {
+					$results[] = $permission | $combination;
+				}
+			}
+			return $results;
+		};
+
+		//Generate file permissions
+		$permissions = [
+			\OCP\Constants::PERMISSION_UPDATE,
+			\OCP\Constants::PERMISSION_CREATE,
+			\OCP\Constants::PERMISSION_SHARE,
+		];
+
+		$allPermissions = $powerset($permissions);
+
+		foreach ($allPermissions as $before) {
+			foreach ($allPermissions as $after) {
+				if ($before === $after) { continue; }
+
+				$data[] = [
+					'file', 
+					$before,
+					$after,
+				];
+			}
+		}
+
+		//Generate folder permissions
+		$permissions = [
+			\OCP\Constants::PERMISSION_UPDATE,
+			\OCP\Constants::PERMISSION_CREATE,
+			\OCP\Constants::PERMISSION_SHARE,
+			\OCP\Constants::PERMISSION_DELETE
+		];
+
+		$allPermissions = $powerset($permissions);
+
+		foreach ($allPermissions as $before) {
+			foreach ($allPermissions as $after) {
+				if ($before === $after) { continue; }
+
+				$data[] = [
+					'folder',
+					$before,
+					$after,
+				];
+			}
+		}
+
+		return $data;
+	}
+
+
+
+	/**
+	 * moved mountpoints of a group share should keep the same permission as their parent group share.
+	 * See #15253
+	 *
+	 * @dataProvider dataPermissionMovedGroupShare
+	 */
+	function testPermissionMovedGroupShare($type, $beforePerm, $afterPerm) {
+
+		if ($type === 'file') {
+			$path = $this->filename;
+		} else if ($type === 'folder') {
+			$path = $this->folder;
+		}
+
+		\OC_Group::createGroup('testGroup');
+		\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER1, 'testGroup');
+		\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup');
+		\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER3, 'testGroup');
+
+		// Share item with group
+		$fileinfo = $this->view->getFileInfo($path);
+		$this->assertTrue(
+			\OCP\Share::shareItem($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP,	"testGroup", $beforePerm)
+		);
+
+		// Login as user 2 and verify the item exists
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$this->assertTrue(\OC\Files\Filesystem::file_exists($path));
+		$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
+		$this->assertNotEmpty($result);
+		$this->assertEquals($beforePerm, $result['permissions']);
+
+		// Now move the item forcing a new entry in the share table
+		\OC\Files\Filesystem::rename($path, "newPath");
+		$this->assertTrue(\OC\Files\Filesystem::file_exists('newPath'));
+		$this->assertFalse(\OC\Files\Filesystem::file_exists($path));
+
+		// Login as user 1 again and change permissions
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$this->assertTrue(
+			\OCP\Share::setPermissions($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, "testGroup", $afterPerm)
+		);
+
+		// Login as user 3 and verify that the permissions are changed
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER3);
+		$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
+		$this->assertNotEmpty($result);
+		$this->assertEquals($afterPerm, $result['permissions']);
+		$groupShareId = $result['id'];
+
+		// Login as user 2 and verify that the permissions are changed
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
+		$this->assertNotEmpty($result);
+		$this->assertEquals($afterPerm, $result['permissions']);
+		$this->assertNotEquals($groupShareId, $result['id']);
+
+		// Also verify in the DB
+		$statement = "SELECT `permissions` FROM `*PREFIX*share` WHERE `id`=?";
+		$query = \OCP\DB::prepare($statement);
+		$result = $query->execute([$result['id']]);
+		$shares = $result->fetchAll();
+		$this->assertCount(1, $shares);
+		$this->assertEquals($afterPerm, $shares[0]['permissions']);
+
+		//cleanup
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		\OCP\Share::unshare($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, 'testGroup');
+		\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER1, 'testGroup');
+		\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup');
+		\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER3, 'testGroup');
+	}
+
 }
 
 class DummyTestClassSharedMount extends \OCA\Files_Sharing\SharedMount {
diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index db27fa6a891..7e6b06ade93 100644
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -1100,13 +1100,13 @@ class Share extends Constants {
 	 */
 	public static function setPermissions($itemType, $itemSource, $shareType, $shareWith, $permissions) {
 		$l = \OC::$server->getL10N('lib');
-		if ($item = self::getItems($itemType, $itemSource, $shareType, $shareWith,
+		if ($rootItem = self::getItems($itemType, $itemSource, $shareType, $shareWith,
 			\OC_User::getUser(), self::FORMAT_NONE, null, 1, false)) {
 			// Check if this item is a reshare and verify that the permissions
 			// granted don't exceed the parent shared item
-			if (isset($item['parent'])) {
+			if (isset($rootItem['parent'])) {
 				$query = \OC_DB::prepare('SELECT `permissions` FROM `*PREFIX*share` WHERE `id` = ?', 1);
-				$result = $query->execute(array($item['parent']))->fetchRow();
+				$result = $query->execute([$rootItem['parent']])->fetchRow();
 				if (~(int)$result['permissions'] & $permissions) {
 					$message = 'Setting permissions for %s failed,'
 						.' because the permissions exceed permissions granted to %s';
@@ -1116,7 +1116,7 @@ class Share extends Constants {
 				}
 			}
 			$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` = ?');
-			$query->execute(array($permissions, $item['id']));
+			$query->execute([$permissions, $rootItem['id']]);
 			if ($itemType === 'file' || $itemType === 'folder') {
 				\OC_Hook::emit('OCP\Share', 'post_update_permissions', array(
 					'itemType' => $itemType,
@@ -1125,56 +1125,85 @@ class Share extends Constants {
 					'shareWith' => $shareWith,
 					'uidOwner' => \OC_User::getUser(),
 					'permissions' => $permissions,
-					'path' => $item['path'],
-					'share' => $item
+					'path' => $rootItem['path'],
+					'share' => $rootItem
 				));
 			}
+
+			// Share id's to update with the new permissions
+			$ids = [];
+			$items = [];
+
 			// Check if permissions were removed
-			if ($item['permissions'] & ~$permissions) {
+			if ((int)$rootItem['permissions'] & ~$permissions) {
 				// If share permission is removed all reshares must be deleted
-				if (($item['permissions'] & \OCP\Constants::PERMISSION_SHARE) && (~$permissions & \OCP\Constants::PERMISSION_SHARE)) {
+				if (($rootItem['permissions'] & \OCP\Constants::PERMISSION_SHARE) && (~$permissions & \OCP\Constants::PERMISSION_SHARE)) {
 					// delete all shares, keep parent and group children
-					Helper::delete($item['id'], true, null, null, true);
-				} else {
-					$ids = array();
-					$items = [];
-					$parents = array($item['id']);
-					while (!empty($parents)) {
-						$parents = "'".implode("','", $parents)."'";
-						$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
-							.' WHERE `parent` IN ('.$parents.')');
-						$result = $query->execute();
-						// Reset parents array, only go through loop again if
-						// items are found that need permissions removed
-						$parents = array();
-						while ($item = $result->fetchRow()) {
-							$items[] = $item;
-							// Check if permissions need to be removed
-							if ($item['permissions'] & ~$permissions) {
-								// Add to list of items that need permissions removed
-								$ids[] = $item['id'];
-								$parents[] = $item['id'];
-							}
-						}
-					}
-					// Remove the permissions for all reshares of this item
-					if (!empty($ids)) {
-						$ids = "'".implode("','", $ids)."'";
-						// TODO this should be done with Doctrine platform objects
-						if (\OC::$server->getConfig()->getSystemValue("dbtype") === 'oci') {
-							$andOp = 'BITAND(`permissions`, ?)';
-						} else {
-							$andOp = '`permissions` & ?';
+					Helper::delete($rootItem['id'], true, null, null, true);
+				}
+
+				// Remove permission from all children
+				$parents = [$rootItem['id']];
+				while (!empty($parents)) {
+					$parents = "'".implode("','", $parents)."'";
+					$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
+						.' WHERE `parent` IN ('.$parents.')');
+					$result = $query->execute();
+					// Reset parents array, only go through loop again if
+					// items are found that need permissions removed
+					$parents = array();
+					while ($item = $result->fetchRow()) {
+						$items[] = $item;
+						// Check if permissions need to be removed
+						if ($item['permissions'] & ~$permissions) {
+							// Add to list of items that need permissions removed
+							$ids[] = $item['id'];
+							$parents[] = $item['id'];
 						}
-						$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = '.$andOp
-							.' WHERE `id` IN ('.$ids.')');
-						$query->execute(array($permissions));
 					}
+				}
 
-					foreach ($items as $item) {
-						\OC_Hook::emit('OCP\Share', 'post_update_permissions', ['share' => $item]);
+				// Remove the permissions for all reshares of this item
+				if (!empty($ids)) {
+					$ids = "'".implode("','", $ids)."'";
+					// TODO this should be done with Doctrine platform objects
+					if (\OC::$server->getConfig()->getSystemValue("dbtype") === 'oci') {
+						$andOp = 'BITAND(`permissions`, ?)';
+					} else {
+						$andOp = '`permissions` & ?';
 					}
+					$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = '.$andOp
+						.' WHERE `id` IN ('.$ids.')');
+					$query->execute(array($permissions));
 				}
+
+			}
+
+			/*
+			 * Permissions were added
+			 * Update all USERGROUP shares. (So group shares where the user moved his mountpoint).
+			 */
+			if ($permissions & ~(int)$rootItem['permissions']) {
+				$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
+						.' WHERE `parent` = ? AND `share_type` = ?');
+				$result = $query->execute([$rootItem['id'], 2]);
+
+				$ids = [];
+				while ($item = $result->fetchRow()) {
+					$items[] = $item;
+					$ids[] = $item['id'];
+				}
+
+				// Add permssions for all USERGROUP shares of this item
+				if (!empty($ids)) {
+					$ids = "'".implode("','", $ids)."'";
+					$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` IN ('.$ids.')');
+					$query->execute(array($permissions));
+				}
+			}
+
+			foreach ($items as $item) {
+				\OC_Hook::emit('OCP\Share', 'post_update_permissions', ['share' => $item]);
 			}
 
 			return true;