Merge pull request #17246 from owncloud/share-lock-owner-parent

lock parent folders for the owner when locking a shared file as recipient

3 files changed, 134 insertions, 10 deletions

diff --git a/apps/files_sharing/lib/sharedmount.php b/apps/files_sharing/lib/sharedmount.php
index 2afc0f1f270..fd672d0500c 100644
--- a/apps/files_sharing/lib/sharedmount.php
+++ b/apps/files_sharing/lib/sharedmount.php
@@ -41,34 +41,40 @@ class SharedMount extends MountPoint implements MoveableMount {
 	 */
 	protected $ownerPropagator;
 
+	/**
+	 * @var \OC\Files\View
+	 */
+	private $recipientView;
+
 	public function __construct($storage, $mountpoint, $arguments = null, $loader = null) {
 		// first update the mount point before creating the parent
 		$this->ownerPropagator = $arguments['propagator'];
-		$newMountPoint = $this->verifyMountPoint($arguments['share'], $arguments['user']);
+		$this->recipientView = new View('/' . $arguments['user'] . '/files');
+		$newMountPoint = $this->verifyMountPoint($arguments['share']);
 		$absMountPoint = '/' . $arguments['user'] . '/files' . $newMountPoint;
+		$arguments['ownerView'] = new View('/' . $arguments['share']['uid_owner'] . '/files');
 		parent::__construct($storage, $absMountPoint, $arguments, $loader);
 	}
 
 	/**
 	 * check if the parent folder exists otherwise move the mount point up
 	 */
-	private function verifyMountPoint(&$share, $user) {
+	private function verifyMountPoint(&$share) {
 
 		$mountPoint = basename($share['file_target']);
 		$parent = dirname($share['file_target']);
-		$view = new View('/' . $user . '/files');
 
-		if (!$view->is_dir($parent)) {
+		if (!$this->recipientView->is_dir($parent)) {
 			$parent = Helper::getShareFolder();
 		}
 
 		$newMountPoint = \OCA\Files_Sharing\Helper::generateUniqueTarget(
-				\OC\Files\Filesystem::normalizePath($parent . '/' . $mountPoint),
-				array(),
-				new \OC\Files\View('/' . $user . '/files')
-				);
+			\OC\Files\Filesystem::normalizePath($parent . '/' . $mountPoint),
+			[],
+			$this->recipientView
+		);
 
-		if($newMountPoint !== $share['file_target']) {
+		if ($newMountPoint !== $share['file_target']) {
 			self::updateFileTarget($newMountPoint, $share);
 			$share['file_target'] = $newMountPoint;
 			$share['unique_name'] = true;
@@ -79,6 +85,7 @@ class SharedMount extends MountPoint implements MoveableMount {
 
 	/**
 	 * update fileTarget in the database if the mount point changed
+	 *
 	 * @param string $newPath
 	 * @param array $share reference to the share which should be modified
 	 * @return bool
@@ -99,7 +106,7 @@ class SharedMount extends MountPoint implements MoveableMount {
 					'Update `*PREFIX*share`
 						SET `file_target` = ?
 						WHERE `id` = ?'
-					);
+			);
 			$arguments = array($newPath, $share['id']);
 		}
 
diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
index 50957663b34..ff01489d77b 100644
--- a/apps/files_sharing/lib/sharedstorage.php
+++ b/apps/files_sharing/lib/sharedstorage.php
@@ -45,8 +45,14 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 	private $files = array();
 	private static $isInitialized = array();
 
+	/**
+	 * @var \OC\Files\View
+	 */
+	private $ownerView;
+
 	public function __construct($arguments) {
 		$this->share = $arguments['share'];
+		$this->ownerView = $arguments['ownerView'];
 	}
 
 	/**
@@ -623,6 +629,11 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 		/** @var \OCP\Files\Storage $targetStorage */
 		list($targetStorage, $targetInternalPath) = $this->resolvePath($path);
 		$targetStorage->acquireLock($targetInternalPath, $type, $provider);
+		// lock the parent folders of the owner when locking the share as recipient
+		if ($path === '') {
+			$sourcePath = $this->ownerView->getPath($this->share['file_source']);
+			$this->ownerView->lockFile(dirname($sourcePath), ILockingProvider::LOCK_SHARED, true);
+		}
 	}
 
 	/**
@@ -634,6 +645,11 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 		/** @var \OCP\Files\Storage $targetStorage */
 		list($targetStorage, $targetInternalPath) = $this->resolvePath($path);
 		$targetStorage->releaseLock($targetInternalPath, $type, $provider);
+		// unlock the parent folders of the owner when unlocking the share as recipient
+		if ($path === '') {
+			$sourcePath = $this->ownerView->getPath($this->share['file_source']);
+			$this->ownerView->unlockFile(dirname($sourcePath), ILockingProvider::LOCK_SHARED, true);
+		}
 	}
 
 	/**
diff --git a/apps/files_sharing/tests/locking.php b/apps/files_sharing/tests/locking.php
new file mode 100644
index 00000000000..6d13fc1cda5
--- /dev/null
+++ b/apps/files_sharing/tests/locking.php
@@ -0,0 +1,101 @@
+<?php
+/**
+ * @author Morris Jobke <hey@morrisjobke.de>
+ * @author Robin Appelman <icewind@owncloud.com>
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Files_sharing\Tests;
+
+use OC\Files\Filesystem;
+use OC\Files\View;
+use OC\Lock\MemcacheLockingProvider;
+use OCP\Lock\ILockingProvider;
+
+class Locking extends TestCase {
+	/**
+	 * @var \OC_User_Dummy
+	 */
+	private $userBackend;
+
+	private $ownerUid;
+	private $recipientUid;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->userBackend = new \OC_User_Dummy();
+		\OC::$server->getUserManager()->registerBackend($this->userBackend);
+
+		$this->ownerUid = $this->getUniqueID('owner_');
+		$this->recipientUid = $this->getUniqueID('recipient_');
+		$this->userBackend->createUser($this->ownerUid, '');
+		$this->userBackend->createUser($this->recipientUid, '');
+
+		$this->loginAsUser($this->ownerUid);
+		Filesystem::mkdir('/foo');
+		Filesystem::file_put_contents('/foo/bar.txt', 'asd');
+		$fileId = Filesystem::getFileInfo('/foo/bar.txt')->getId();
+
+		\OCP\Share::shareItem('file', $fileId, \OCP\Share::SHARE_TYPE_USER, $this->recipientUid, 31);
+
+		$this->loginAsUser($this->recipientUid);
+		$this->assertTrue(Filesystem::file_exists('bar.txt'));
+	}
+
+	public function tearDown() {
+		\OC::$server->getUserManager()->removeBackend($this->userBackend);
+		parent::tearDown();
+	}
+
+	/**
+	 * @expectedException \OCP\Lock\LockedException
+	 */
+	public function testLockAsRecipient() {
+		$this->loginAsUser($this->ownerUid);
+
+		Filesystem::initMountPoints($this->recipientUid);
+		$recipientView = new View('/' . $this->recipientUid . '/files');
+		$recipientView->lockFile('bar.txt', ILockingProvider::LOCK_EXCLUSIVE);
+
+		Filesystem::rename('/foo', '/asd');
+	}
+
+	public function testUnLockAsRecipient() {
+		$this->loginAsUser($this->ownerUid);
+
+		Filesystem::initMountPoints($this->recipientUid);
+		$recipientView = new View('/' . $this->recipientUid . '/files');
+		$recipientView->lockFile('bar.txt', ILockingProvider::LOCK_EXCLUSIVE);
+		$recipientView->unlockFile('bar.txt', ILockingProvider::LOCK_EXCLUSIVE);
+
+		$this->assertTrue(Filesystem::rename('/foo', '/asd'));
+	}
+
+	public function testChangeLock() {
+
+		Filesystem::initMountPoints($this->recipientUid);
+		$recipientView = new View('/' . $this->recipientUid . '/files');
+		$recipientView->lockFile('bar.txt', ILockingProvider::LOCK_SHARED);
+		$recipientView->changeLock('bar.txt', ILockingProvider::LOCK_EXCLUSIVE);
+		$recipientView->unlockFile('bar.txt', ILockingProvider::LOCK_EXCLUSIVE);
+
+		$this->assertTrue(true);
+	}
+}