Merge pull request #9357 from owncloud/sharing_unshare_on_delete

[sharing] unshare childrens on delete

7 files changed, 52 insertions, 46 deletions

diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php
index 3625d5a09f3..d4a98410a37 100644
--- a/apps/files_encryption/hooks/hooks.php
+++ b/apps/files_encryption/hooks/hooks.php
@@ -340,7 +340,7 @@ class Hooks {
 	}

 

 	/**

-	 * @brief

+	 * unshare file/folder from a user with whom you shared the file before

 	 */

 	public static function postUnshare($params) {

 

@@ -385,8 +385,10 @@ class Hooks {
 				// Unshare every user who no longer has access to the file

 				$delUsers = array_diff($userIds, $sharingUsers);

 

+				list($owner, $ownerPath) = $util->getUidAndFilename($path);

+

 				// delete share key

-				Keymanager::delShareKey($view, $delUsers, $path);

+				Keymanager::delShareKey($view, $delUsers, $ownerPath, $owner);

 			}

 

 		}

@@ -441,7 +443,7 @@ class Hooks {
 			$ownerOld = self::$renamedFiles[$params['oldpath']]['uid'];

 			$pathOld = self::$renamedFiles[$params['oldpath']]['path'];

 		} else {

-			\OCP\Util::writeLog('Encryption library', "can't get path and owner from the file before it was renamed", \OCP\Util::ERROR);

+			\OCP\Util::writeLog('Encryption library', "can't get path and owner from the file before it was renamed", \OCP\Util::DEBUG);

 			return false;

 		}

 

@@ -595,6 +597,7 @@ class Hooks {
 	}

 

 	/**

+	 * unmount file from yourself

 	 * remember files/folders which get unmounted

 	 */

 	public static function preUmount($params) {

@@ -613,6 +616,9 @@ class Hooks {
 			'itemType' => $itemType);

 	}

 

+	/**

+	 * unmount file from yourself

+	 */

 	public static function postUmount($params) {

 

 		if (!isset(self::$umountedFiles[$params[\OC\Files\Filesystem::signal_param_path]])) {

@@ -642,7 +648,7 @@ class Hooks {
 			// check if the user still has access to the file, otherwise delete share key

 			$sharingUsers = \OCP\Share::getUsersSharingFile($path, $user);

 			if (!in_array(\OCP\User::getUser(), $sharingUsers['users'])) {

-				Keymanager::delShareKey($view, array(\OCP\User::getUser()), $path);

+				Keymanager::delShareKey($view, array(\OCP\User::getUser()), $path, $user);

 			}

 		}

 	}

diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php
index 70820a6f940..da84e975a05 100755
--- a/apps/files_encryption/lib/keymanager.php
+++ b/apps/files_encryption/lib/keymanager.php
@@ -444,17 +444,18 @@ class Keymanager {
 
 	/**
 	 * Delete a single user's shareKey for a single file
+	 *
+	 * @param \OC\Files\View $view relative to data/
+	 * @param array $userIds list of users we want to remove
+	 * @param string $filename the owners name of the file for which we want to remove the users relative to data/user/files
+	 * @param string $owner owner of the file
 	 */
-	public static function delShareKey(\OC\Files\View $view, $userIds, $filePath) {
+	public static function delShareKey($view, $userIds, $filename, $owner) {
 
 		$proxyStatus = \OC_FileProxy::$enabled;
 		\OC_FileProxy::$enabled = false;
 
-		$userId = Helper::getUser($filePath);
-
-		$util = new Util($view, $userId);
-
-		list($owner, $filename) = $util->getUidAndFilename($filePath);
+		$util = new Util($view, $owner);
 
 		if ($util->isSystemWideMountPoint($filename)) {
 			$shareKeyPath = \OC\Files\Filesystem::normalizePath('/files_encryption/share-keys/' . $filename);
diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php
index eb7583650a8..e779f8341e6 100644
--- a/apps/files_encryption/tests/keymanager.php
+++ b/apps/files_encryption/tests/keymanager.php
@@ -225,7 +225,7 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file2.user3.shareKey', 'data');
 
 		// recursive delete share keys from user1 and user2
-		Encryption\Keymanager::delShareKey($this->view, array('user1', 'user2', Test_Encryption_Keymanager::TEST_USER), '/folder1/');
+		Encryption\Keymanager::delShareKey($this->view, array('user1', 'user2', Test_Encryption_Keymanager::TEST_USER), '/folder1/', Test_Encryption_Keymanager::TEST_USER);
 
 		// check if share keys from user1 and user2 are deleted
 		$this->assertFalse($this->view->file_exists(
@@ -274,7 +274,7 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data');
 
 		// recursive delete share keys from user1 and user2
-		Encryption\Keymanager::delShareKey($this->view, array('user1', 'user2', Test_Encryption_Keymanager::TEST_USER), '/folder1/existingFile.txt');
+		Encryption\Keymanager::delShareKey($this->view, array('user1', 'user2', Test_Encryption_Keymanager::TEST_USER), '/folder1/existingFile.txt', Test_Encryption_Keymanager::TEST_USER);
 
 		// check if share keys from user1 and user2 are deleted
 		$this->assertFalse($this->view->file_exists(
diff --git a/apps/files_sharing/lib/proxy.php b/apps/files_sharing/lib/proxy.php
index f595328cc63..92303d298b1 100644
--- a/apps/files_sharing/lib/proxy.php
+++ b/apps/files_sharing/lib/proxy.php
@@ -21,47 +21,43 @@
  */
 
 namespace OCA\Files\Share;
+use OCA\Files_Sharing\Helper;
 
 class Proxy extends \OC_FileProxy {
 
 	/**
-	 * check if the deleted folder contains share mount points and move them
-	 * up to the parent
+	 * check if the deleted folder contains share mount points and unshare them
 	 *
 	 * @param string $path
 	 */
 	public function preUnlink($path) {
-		$this->moveMountPointsUp($path);
+		$this->unshareChildren($path);
 	}
 
 	/**
-	 * check if the deleted folder contains share mount points and move them
-	 * up to the parent
+	 * check if the deleted folder contains share mount points and unshare them
 	 *
 	 * @param string $path
 	 */
 	public function preRmdir($path) {
-		$this->moveMountPointsUp($path);
+		$this->unshareChildren($path);
 	}
 
 	/**
-	 * move share mount points up to the parent
+	 * unshare shared items below the deleted folder
 	 *
 	 * @param string $path
 	 */
-	private function moveMountPointsUp($path) {
+	private function unshareChildren($path) {
 		$view = new \OC\Files\View('/');
 
-		// find share mount points within $path and move them up to the parent folder
-		// before we delete $path
+		// find share mount points within $path and unmount them
 		$mountManager = \OC\Files\Filesystem::getMountManager();
 		$mountedShares = $mountManager->findIn($path);
 		foreach ($mountedShares as $mount) {
-			if ($mount->getStorage()->instanceOfStorage('\OC\Files\Storage\Shared')) {
+			if ($mount->getStorage()->instanceOfStorage('OCA\Files_Sharing\ISharedStorage')) {
 				$mountPoint = $mount->getMountPoint();
-				$mountPointName = $mount->getMountPointName();
-				$target = \OCA\Files_Sharing\Helper::generateUniqueTarget(dirname($path) . '/' . $mountPointName, array(), $view);
-				$view->rename($mountPoint, $target);
+				$view->unlink($mountPoint);
 			}
 		}
 	}
diff --git a/apps/files_sharing/lib/sharedmount.php b/apps/files_sharing/lib/sharedmount.php
index f8def2c6a82..564ac43ec74 100644
--- a/apps/files_sharing/lib/sharedmount.php
+++ b/apps/files_sharing/lib/sharedmount.php
@@ -143,8 +143,10 @@ class SharedMount extends Mount implements MoveableMount {
 	 * @return bool
 	 */
 	public function removeMount() {
+		$mountManager = \OC\Files\Filesystem::getMountManager();
 		$storage = $this->getStorage();
 		$result =  \OCP\Share::unshareFromSelf($storage->getItemType(), $storage->getMountPoint());
+		$mountManager->removeMount($this->mountPoint);
 
 		return $result;
 	}
diff --git a/apps/files_sharing/tests/proxy.php b/apps/files_sharing/tests/proxy.php
index 634ed86db54..b6599a1b646 100644
--- a/apps/files_sharing/tests/proxy.php
+++ b/apps/files_sharing/tests/proxy.php
@@ -47,7 +47,6 @@ class Test_Files_Sharing_Proxy extends Test_Files_Sharing_Base {
 		$this->filename = '/share-api-test';
 
 		// save file with content
-		$this->view->file_put_contents($this->filename, $this->data);
 		$this->view->mkdir($this->folder);
 		$this->view->mkdir($this->folder . $this->subfolder);
 		$this->view->mkdir($this->folder . $this->subfolder . $this->subsubfolder);
@@ -56,7 +55,6 @@ class Test_Files_Sharing_Proxy extends Test_Files_Sharing_Base {
 	}
 
 	function tearDown() {
-		$this->view->unlink($this->filename);
 		$this->view->deleteAll($this->folder);
 
 		self::$tempStorage = null;
@@ -69,30 +67,33 @@ class Test_Files_Sharing_Proxy extends Test_Files_Sharing_Base {
 	 */
 	function testpreUnlink() {
 
-		$fileInfo1 = \OC\Files\Filesystem::getFileInfo($this->filename);
 		$fileInfo2 = \OC\Files\Filesystem::getFileInfo($this->folder);
 
-		$result = \OCP\Share::shareItem('file', $fileInfo1->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
-		$this->assertTrue($result);
-
 		$result = \OCP\Share::shareItem('folder', $fileInfo2->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
 		$this->assertTrue($result);
 
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
 
-		// move shared folder to 'localDir' and rename it, so that it uses the same
-		// name as the shared file
+		// one folder should be shared with the user
+		$sharedFolders = \OCP\Share::getItemsSharedWith('folder');
+		$this->assertSame(1, count($sharedFolders));
+
+		// move shared folder to 'localDir'
 		\OC\Files\Filesystem::mkdir('localDir');
-		$result = \OC\Files\Filesystem::rename($this->folder, '/localDir/' . $this->filename);
+		$result = \OC\Files\Filesystem::rename($this->folder, '/localDir/' . $this->folder);
 		$this->assertTrue($result);
 
 		\OC\Files\Filesystem::unlink('localDir');
 
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
 
-		// after we deleted 'localDir' the share should be moved up to the root and be
-		// renamed to "filename (2)"
-		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename));
-		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename . ' (2)' ));
+		// after the parent directory was deleted the share should be unshared
+		$sharedFolders = \OCP\Share::getItemsSharedWith('folder');
+		$this->assertTrue(empty($sharedFolders));
+
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
+
+		// the folder for the owner should still exists
+		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->folder));
 	}
 }
diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php
index 5ec53488702..1f51b9a315c 100644
--- a/apps/files_sharing/tests/updater.php
+++ b/apps/files_sharing/tests/updater.php
@@ -57,7 +57,7 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 
 	/**
 	 * test deletion of a folder which contains share mount points. Share mount
-	 * points should move up to the parent before the folder gets deleted so
+	 * points should be unshared before the folder gets deleted so
 	 * that the mount point doesn't end up at the trash bin
 	 */
 	function testDeleteParentFolder() {
@@ -78,6 +78,9 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 		// check if user2 can see the shared folder
 		$this->assertTrue($view->file_exists($this->folder));
 
+		$foldersShared = \OCP\Share::getItemsSharedWith('folder');
+		$this->assertSame(1, count($foldersShared));
+
 		$view->mkdir("localFolder");
 		$view->file_put_contents("localFolder/localFile.txt", "local file");
 
@@ -91,8 +94,9 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 
 		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
 
-		// mount point should move up again
-		$this->assertTrue($view->file_exists($this->folder));
+		// shared folder should be unshared
+		$foldersShared = \OCP\Share::getItemsSharedWith('folder');
+		$this->assertTrue(empty($foldersShared));
 
 		// trashbin should contain the local file but not the mount point
 		$rootView = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2);
@@ -109,10 +113,6 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 		if ($status === false) {
 			\OC_App::disable('files_trashbin');
 		}
-		// cleanup
-		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
-		$result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2);
-		$this->assertTrue($result);
 	}
 
 	/**