Merge pull request #21038 from owncloud/share-computesharepermissions-notstore

Fix (re)share permission checks in a few code paths

4 files changed, 41 insertions, 22 deletions

diff --git a/apps/dav/tests/unit/connector/sabre/file.php b/apps/dav/tests/unit/connector/sabre/file.php
index 2a6cf46ef16..ad4c1d29ed4 100644
--- a/apps/dav/tests/unit/connector/sabre/file.php
+++ b/apps/dav/tests/unit/connector/sabre/file.php
@@ -48,6 +48,14 @@ class File extends \Test\TestCase {
 		parent::tearDown();
 	}
 
+	private function getMockStorage() {
+		$storage = $this->getMock('\OCP\Files\Storage');
+		$storage->expects($this->any())
+			->method('getId')
+			->will($this->returnValue('home::someuser'));
+		return $storage;
+	}
+
 	/**
 	 * @param string $string
 	 */
@@ -149,7 +157,7 @@ class File extends \Test\TestCase {
 			->method('getRelativePath')
 			->will($this->returnArgument(0));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -209,7 +217,7 @@ class File extends \Test\TestCase {
 
 		$_SERVER['HTTP_OC_CHUNKED'] = true;
 
-		$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', null, null, [
+		$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		], null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -219,7 +227,7 @@ class File extends \Test\TestCase {
 		$this->assertNull($file->put('test data one'));
 		$file->releaseLock(ILockingProvider::LOCK_SHARED);
 
-		$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', null, null, [
+		$info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		], null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -261,7 +269,7 @@ class File extends \Test\TestCase {
 
 		$info = new \OC\Files\FileInfo(
 			$viewRoot . '/' . ltrim($path, '/'),
-			null,
+			$this->getMockStorage(),
 			null,
 			['permissions' => \OCP\Constants::PERMISSION_ALL],
 			null
@@ -450,7 +458,7 @@ class File extends \Test\TestCase {
 		$_SERVER['CONTENT_LENGTH'] = 123456;
 		$_SERVER['REQUEST_METHOD'] = 'PUT';
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -483,7 +491,7 @@ class File extends \Test\TestCase {
 		// simulate situation where the target file is locked
 		$view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE);
 
-		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -518,7 +526,7 @@ class File extends \Test\TestCase {
 
 		$_SERVER['HTTP_OC_CHUNKED'] = true;
 
-		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', null, null, [
+		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		], null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -526,7 +534,7 @@ class File extends \Test\TestCase {
 		$this->assertNull($file->put('test data one'));
 		$file->releaseLock(ILockingProvider::LOCK_SHARED);
 
-		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', null, null, [
+		$info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		], null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -555,7 +563,7 @@ class File extends \Test\TestCase {
 			->method('getRelativePath')
 			->will($this->returnArgument(0));
 
-		$info = new \OC\Files\FileInfo('/*', null, null, array(
+		$info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -591,7 +599,7 @@ class File extends \Test\TestCase {
 			->method('getRelativePath')
 			->will($this->returnArgument(0));
 
-		$info = new \OC\Files\FileInfo('/*', null, null, array(
+		$info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 		$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
@@ -618,7 +626,7 @@ class File extends \Test\TestCase {
 		$_SERVER['CONTENT_LENGTH'] = 12345;
 		$_SERVER['REQUEST_METHOD'] = 'PUT';
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -654,7 +662,7 @@ class File extends \Test\TestCase {
 			->method('unlink')
 			->will($this->returnValue(true));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -672,7 +680,7 @@ class File extends \Test\TestCase {
 		$view = $this->getMock('\OC\Files\View',
 			array());
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => 0
 		), null);
 
@@ -695,7 +703,7 @@ class File extends \Test\TestCase {
 			->method('unlink')
 			->will($this->returnValue(false));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -718,7 +726,7 @@ class File extends \Test\TestCase {
 			->method('unlink')
 			->willThrowException(new ForbiddenException('', true));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -753,7 +761,7 @@ class File extends \Test\TestCase {
 		$path = 'test-locking.txt';
 		$info = new \OC\Files\FileInfo(
 			'/' . $this->user . '/files/' . $path,
-			null,
+			$this->getMockStorage(),
 			null,
 			['permissions' => \OCP\Constants::PERMISSION_ALL],
 			null
@@ -865,7 +873,7 @@ class File extends \Test\TestCase {
 			->method('fopen')
 			->will($this->returnValue(false));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
@@ -883,7 +891,7 @@ class File extends \Test\TestCase {
 			->method('fopen')
 			->willThrowException(new ForbiddenException('', true));
 
-		$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, array(
 			'permissions' => \OCP\Constants::PERMISSION_ALL
 		), null);
 
diff --git a/apps/files/tests/controller/apicontrollertest.php b/apps/files/tests/controller/apicontrollertest.php
index fb728d5eff0..bc66e4641b9 100644
--- a/apps/files/tests/controller/apicontrollertest.php
+++ b/apps/files/tests/controller/apicontrollertest.php
@@ -92,6 +92,7 @@ class ApiControllerTest extends TestCase {
 			[
 				'mtime' => 55,
 				'mimetype' => 'application/pdf',
+				'permissions' => 31,
 				'size' => 1234,
 				'etag' => 'MyEtag',
 			],
@@ -111,7 +112,7 @@ class ApiControllerTest extends TestCase {
 					'parentId' => null,
 					'mtime' => 55000,
 					'name' => 'root.txt',
-					'permissions' => null,
+					'permissions' => 31,
 					'mimetype' => 'application/pdf',
 					'size' => 1234,
 					'type' => 'file',
@@ -139,6 +140,7 @@ class ApiControllerTest extends TestCase {
 			[
 				'mtime' => 55,
 				'mimetype' => 'application/pdf',
+				'permissions' => 31,
 				'size' => 1234,
 				'etag' => 'MyEtag',
 			],
@@ -155,6 +157,7 @@ class ApiControllerTest extends TestCase {
 			[
 				'mtime' => 999,
 				'mimetype' => 'application/binary',
+				'permissions' => 31,
 				'size' => 9876,
 				'etag' => 'SubEtag',
 			],
@@ -174,7 +177,7 @@ class ApiControllerTest extends TestCase {
 					'parentId' => null,
 					'mtime' => 55000,
 					'name' => 'root.txt',
-					'permissions' => null,
+					'permissions' => 31,
 					'mimetype' => 'application/pdf',
 					'size' => 1234,
 					'type' => 'file',
@@ -191,7 +194,7 @@ class ApiControllerTest extends TestCase {
 					'parentId' => null,
 					'mtime' => 999000,
 					'name' => 'root.txt',
-					'permissions' => null,
+					'permissions' => 31,
 					'mimetype' => 'application/binary',
 					'size' => 9876,
 					'type' => 'file',
diff --git a/apps/files_sharing/lib/external/storage.php b/apps/files_sharing/lib/external/storage.php
index 2a0d827e064..36ff4f0c226 100644
--- a/apps/files_sharing/lib/external/storage.php
+++ b/apps/files_sharing/lib/external/storage.php
@@ -265,4 +265,12 @@ class Storage extends DAV implements ISharedStorage {
 		list(, $remote) = explode('://', $this->remote, 2);
 		return $this->remoteUser . '@' . $remote;
 	}
+
+	public function isSharable($path) {
+		if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) {
+			return false;
+		}
+		return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE);
+	}
+
 }
diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
index cda3f564d5f..38f79762dc6 100644
--- a/apps/files_sharing/lib/sharedstorage.php
+++ b/apps/files_sharing/lib/sharedstorage.php
@@ -257,7 +257,7 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 	}
 
 	public function isSharable($path) {
-		if (\OCP\Util::isSharingDisabledForUser()) {
+		if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) {
 			return false;
 		}
 		return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE);