Merge pull request #13760 from owncloud/create_enc_key_for_remote_shares

make sure that we always create a public share key for remote shares

3 files changed, 145 insertions, 66 deletions

diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php
index b300999ff24..14d0a0bc4b9 100644
--- a/apps/files_encryption/lib/util.php
+++ b/apps/files_encryption/lib/util.php
@@ -1109,7 +1109,7 @@ class Util {
 			// Find out who, if anyone, is sharing the file
 			$result = \OCP\Share::getUsersSharingFile($ownerPath, $owner);
 			$userIds = \array_merge($userIds, $result['users']);
-			if ($result['public']) {
+			if ($result['public'] || $result['remote']) {
 				$userIds[] = $this->publicShareKeyId;
 			}
 
diff --git a/apps/files_encryption/tests/share.php b/apps/files_encryption/tests/share.php
index 8ecdbabed39..a59838ede1c 100755
--- a/apps/files_encryption/tests/share.php
+++ b/apps/files_encryption/tests/share.php
@@ -89,6 +89,8 @@ class Share extends TestCase {
 
 		// login as first user
 		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
+
+		$this->createMocks();
 	}
 
 	protected function tearDown() {
@@ -99,6 +101,8 @@ class Share extends TestCase {
 			\OC_App::disable('files_trashbin');
 		}
 
+		$this->restoreHttpHelper();
+
 		parent::tearDown();
 	}
 
@@ -115,18 +119,43 @@ class Share extends TestCase {
 		parent::tearDownAfterClass();
 	}
 
-	/**
-	 * @medium
-	 */
-	function testDeclineServer2ServerShare() {
-
+	private function createMocks() {
 		$config = $this->getMockBuilder('\OCP\IConfig')
 				->disableOriginalConstructor()->getMock();
 		$certificateManager = $this->getMock('\OCP\ICertificateManager');
 		$httpHelperMock = $this->getMockBuilder('\OC\HTTPHelper')
 				->setConstructorArgs(array($config, $certificateManager))
 				->getMock();
-		$httpHelperMock->expects($this->once())->method('post')->with($this->anything())->will($this->returnValue(true));
+		$httpHelperMock->expects($this->any())->method('post')->with($this->anything())->will($this->returnValue(array('success' => true, 'result' => "{'ocs' : { 'meta' : { 'statuscode' : 100 }}}")));
+
+		$this->registerHttpHelper($httpHelperMock);
+	}
+
+	/**
+	 * Register an http helper mock for testing purposes.
+	 * @param $httpHelper http helper mock
+	 */
+	private function registerHttpHelper($httpHelper) {
+		$this->oldHttpHelper = \OC::$server->query('HTTPHelper');
+		\OC::$server->registerService('HTTPHelper', function ($c) use ($httpHelper) {
+			return $httpHelper;
+		});
+	}
+
+	/**
+	 * Restore the original http helper
+	 */
+	private function restoreHttpHelper() {
+		$oldHttpHelper = $this->oldHttpHelper;
+		\OC::$server->registerService('HTTPHelper', function ($c) use ($oldHttpHelper) {
+			return $oldHttpHelper;
+		});
+	}
+
+	/**
+	 * @medium
+	 */
+	function testDeclineServer2ServerShare() {
 
 		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
 
@@ -134,7 +163,7 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/'  . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// get the file info from previous created file
 		$fileInfo = $this->view->getFileInfo(
@@ -167,11 +196,9 @@ class Share extends TestCase {
 
 		$share = $query->fetch();
 
-		$this->registerHttpHelper($httpHelperMock);
 		$_POST['token'] = $token;
 		$s2s = new \OCA\Files_Sharing\API\Server2Server();
 		$s2s->declineShare(array('id' => $share['id']));
-		$this->restoreHttpHelper();
 
 		$this->assertFalse($this->view->file_exists(
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/keys/'
@@ -179,28 +206,6 @@ class Share extends TestCase {
 
 	}
 
-
-	/**
-	 * Register an http helper mock for testing purposes.
-	 * @param $httpHelper http helper mock
-	 */
-	private function registerHttpHelper($httpHelper) {
-		$this->oldHttpHelper = \OC::$server->query('HTTPHelper');
-		\OC::$server->registerService('HTTPHelper', function ($c) use ($httpHelper) {
-			return $httpHelper;
-		});
-	}
-
-	/**
-	 * Restore the original http helper
-	 */
-	private function restoreHttpHelper() {
-		$oldHttpHelper = $this->oldHttpHelper;
-		\OC::$server->registerService('HTTPHelper', function ($c) use ($oldHttpHelper) {
-			return $oldHttpHelper;
-		});
-	}
-
 	/**
 	 * @medium
 	 * @param bool $withTeardown
@@ -213,7 +218,7 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// disable encryption proxy to prevent recursive calls
 		$proxyStatus = \OC_FileProxy::$enabled;
@@ -224,7 +229,7 @@ class Share extends TestCase {
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// check if the unencrypted file size is stored
 		$this->assertGreaterThan(0, $fileInfo['unencrypted_size']);
@@ -407,7 +412,7 @@ class Share extends TestCase {
 			. $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// disable encryption proxy to prevent recursive calls
 		$proxyStatus = \OC_FileProxy::$enabled;
@@ -418,7 +423,7 @@ class Share extends TestCase {
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files' . $this->folder1);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// re-enable the file proxy
 		\OC_FileProxy::$enabled = $proxyStatus;
@@ -496,7 +501,7 @@ class Share extends TestCase {
 			. $this->subfolder);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfoSubFolder instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfoSubFolder);
 
 		// re-enable the file proxy
 		\OC_FileProxy::$enabled = $proxyStatus;
@@ -530,7 +535,7 @@ class Share extends TestCase {
 			. $this->subsubfolder . '/' . $this->filename);
 
 		// check if we have fileInfos
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// share the file with user3
 		\OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER4, \OCP\Constants::PERMISSION_ALL);
@@ -607,6 +612,62 @@ class Share extends TestCase {
 	}
 
 
+	function testRemoteShareFile() {
+		// login as admin
+		//self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
+
+		// save file with content
+		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/'  . $this->filename, $this->dataShort);
+
+		// test that data was successfully written
+		$this->assertInternalType('int', $cryptedFile);
+
+		// disable encryption proxy to prevent recursive calls
+		$proxyStatus = \OC_FileProxy::$enabled;
+		\OC_FileProxy::$enabled = false;
+
+		// get the file info from previous created file
+		$fileInfo = $this->view->getFileInfo(
+			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
+
+		// check if we have a valid file info
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
+
+		// check if the unencrypted file size is stored
+		$this->assertGreaterThan(0, $fileInfo['unencrypted_size']);
+
+		// re-enable the file proxy
+		\OC_FileProxy::$enabled = $proxyStatus;
+
+		// share the file
+		\OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_REMOTE, 'user1@server1', \OCP\Constants::PERMISSION_ALL);
+
+		$publicShareKeyId = \OC::$server->getAppConfig()->getValue('files_encryption', 'publicShareKeyId');
+
+		// check if share key for public exists
+		$this->assertTrue($this->view->file_exists(
+			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/keys/'
+			. $this->filename . '/' . $publicShareKeyId . '.shareKey'));
+
+		// unshare the file
+		\OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_REMOTE, 'user1@server1');
+
+		// check if share key not exists
+		$this->assertFalse($this->view->file_exists(
+			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/keys/'
+			. $this->filename . '/' . $publicShareKeyId . '.shareKey'));
+
+		// cleanup
+		$this->view->chroot('/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/');
+		$this->view->unlink($this->filename);
+		$this->view->chroot('/');
+
+		// check if share key not exists
+		$this->assertFalse($this->view->file_exists(
+			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/keys/'
+			. $this->filename . '/' . self::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey'));
+	}
+
 	function testPublicShareFile() {
 		// login as admin
 		self::loginHelper(self::TEST_ENCRYPTION_SHARE_USER1);
@@ -615,7 +676,7 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/'  . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// disable encryption proxy to prevent recursive calls
 		$proxyStatus = \OC_FileProxy::$enabled;
@@ -626,7 +687,7 @@ class Share extends TestCase {
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// check if the unencrypted file size is stored
 		$this->assertGreaterThan(0, $fileInfo['unencrypted_size']);
@@ -693,7 +754,7 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// disable encryption proxy to prevent recursive calls
 		$proxyStatus = \OC_FileProxy::$enabled;
@@ -704,7 +765,7 @@ class Share extends TestCase {
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// check if the unencrypted file size is stored
 		$this->assertGreaterThan(0, $fileInfo['unencrypted_size']);
@@ -799,8 +860,8 @@ class Share extends TestCase {
 			. $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile1));
-		$this->assertTrue(is_int($cryptedFile2));
+		$this->assertInternalType('int', $cryptedFile1);
+		$this->assertInternalType('int', $cryptedFile2);
 
 		// check if share key for admin and recovery exists
 		$this->assertTrue($this->view->file_exists(
@@ -906,8 +967,8 @@ class Share extends TestCase {
 			. $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile1));
-		$this->assertTrue(is_int($cryptedFile2));
+		$this->assertInternalType('int', $cryptedFile1);
+		$this->assertInternalType('int', $cryptedFile2);
 
 		// check if share key for user and recovery exists
 		$this->assertTrue($this->view->file_exists(
@@ -994,7 +1055,7 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// disable encryption proxy to prevent recursive calls
 		$proxyStatus = \OC_FileProxy::$enabled;
@@ -1005,7 +1066,7 @@ class Share extends TestCase {
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// check if the unencrypted file size is stored
 		$this->assertGreaterThan(0, $fileInfo['unencrypted_size']);
@@ -1077,14 +1138,14 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// get the file info from previous created file
 		$fileInfo = $this->view->getFileInfo(
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// share the file
 		\OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER2, \OCP\Constants::PERMISSION_ALL);
@@ -1143,14 +1204,14 @@ class Share extends TestCase {
 		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename, $this->dataShort);
 
 		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// get the file info from previous created file
 		$fileInfo = $this->view->getFileInfo(
 			'/' . self::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename);
 
 		// check if we have a valid file info
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// share the file
 		\OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, self::TEST_ENCRYPTION_SHARE_GROUP1, \OCP\Constants::PERMISSION_ALL);
@@ -1222,7 +1283,7 @@ class Share extends TestCase {
 		$cryptedFile = \OC\Files\Filesystem::file_put_contents($folder . $filename, $this->dataShort);
 
 		// Test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// Get file decrypted contents
 		$decrypt = \OC\Files\Filesystem::file_get_contents($folder . $filename);
@@ -1234,7 +1295,7 @@ class Share extends TestCase {
 
 		// get the file info from previous created file
 		$fileInfo = \OC\Files\Filesystem::getFileInfo('/newfolder');
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// share the folder
 		\OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER2, \OCP\Constants::PERMISSION_ALL);
@@ -1280,7 +1341,7 @@ class Share extends TestCase {
 		$cryptedFile = \OC\Files\Filesystem::file_put_contents($folder . $filename, $this->dataShort);
 
 		// Test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		$this->assertInternalType('int', $cryptedFile);
 
 		// Get file decrypted contents
 		$decrypt = \OC\Files\Filesystem::file_get_contents($folder . $filename);
@@ -1292,7 +1353,7 @@ class Share extends TestCase {
 
 		// get the file info from previous created file
 		$fileInfo = \OC\Files\Filesystem::getFileInfo($folder);
-		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+		$this->assertInstanceOf('\OC\Files\FileInfo', $fileInfo);
 
 		// share the folder
 		\OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_SHARE_USER2, \OCP\Constants::PERMISSION_ALL);
@@ -1323,4 +1384,4 @@ class Share extends TestCase {
 		\OC\Files\Filesystem::unlink($folder);
 	}
 
-}
\ No newline at end of file
+}
diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index b2c84e39044..bd21bdd4b3a 100644
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -103,6 +103,7 @@ class Share extends \OC\Share\Constants {
 
 		$shares = $sharePaths = $fileTargets = array();
 		$publicShare = false;
+		$remoteShare = false;
 		$source = -1;
 		$cache = false;
 
@@ -170,18 +171,16 @@ class Share extends \OC\Share\Constants {
 
 			//check for public link shares
 			if (!$publicShare) {
-				$query = \OC_DB::prepare(
-					'SELECT `share_with`
-					FROM
-					`*PREFIX*share`
-					WHERE
-					`item_source` = ? AND `share_type` = ? AND `item_type` IN (\'file\', \'folder\')'
+				$query = \OC_DB::prepare('
+					SELECT `share_with`
+					FROM `*PREFIX*share`
+					WHERE `item_source` = ? AND `share_type` = ? AND `item_type` IN (\'file\', \'folder\')', 1
 				);
 
 				$result = $query->execute(array($source, self::SHARE_TYPE_LINK));
 
 				if (\OCP\DB::isError($result)) {
-					\OCP\Util::writeLog('OCP\Share', \OC_DB::getErrorMessage($result), \OC_Log::ERROR);
+					\OCP\Util::writeLog('OCP\Share', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR);
 				} else {
 					if ($result->fetchRow()) {
 						$publicShare = true;
@@ -189,6 +188,25 @@ class Share extends \OC\Share\Constants {
 				}
 			}
 
+			//check for remote share
+			if (!$remoteShare) {
+				$query = \OC_DB::prepare('
+					SELECT `share_with`
+					FROM `*PREFIX*share`
+					WHERE `item_source` = ? AND `share_type` = ? AND `item_type` IN (\'file\', \'folder\')', 1
+				);
+
+				$result = $query->execute(array($source, self::SHARE_TYPE_REMOTE));
+
+				if (\OCP\DB::isError($result)) {
+					\OCP\Util::writeLog('OCP\Share', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR);
+				} else {
+					if ($result->fetchRow()) {
+						$remoteShare = true;
+					}
+				}
+			}
+
 			// let's get the parent for the next round
 			$meta = $cache->get((int)$source);
 			if($meta !== false) {
@@ -234,7 +252,7 @@ class Share extends \OC\Share\Constants {
 			return $sharePaths;
 		}
 
-		return array("users" => array_unique($shares), "public" => $publicShare);
+		return array('users' => array_unique($shares), 'public' => $publicShare, 'remote' => $remoteShare);
 	}
 
 	/**
@@ -2281,7 +2299,7 @@ class Share extends \OC\Share\Constants {
 		if ($user && $remote) {
 			$url = $remote . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT;
 
-			$local = \OC::$server->getURLGenerator()->getAbsoluteURL('');
+			$local = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
 
 			$fields = array(
 				'shareWith' => $user,