Merge pull request #14570 from rullzer/ocs_sane_permissions

Shares should have a least read permission
owncloud · Mar 2, 2015 · 7194952 · 7194952
2 parents 3f91e37 + 4436a9c
commit 7194952
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 1 deletion.
diff --git a/apps/files_sharing/api/local.php b/apps/files_sharing/api/local.php
@@ -276,6 +276,10 @@ public static function createShare($params) {
 				return new \OC_OCS_Result(null, 400, "unknown share type");
 		}
 
+		if (($permissions & \OCP\Constants::PERMISSION_READ) === 0) {
+			return new \OC_OCS_Result(null, 400, 'invalid permissions');
+		}
+
 		try	{
 			$token = \OCP\Share::shareItem(
 					$itemType,
@@ -347,7 +351,6 @@ public static function updateShare($params) {
 		}
 
 		return new \OC_OCS_Result(null, 400, "Wrong or no update parameter given");
-
 	}
 
 	/**
@@ -376,6 +379,10 @@ private static function updatePermissions($share, $params) {
 			}
 		}
 
+		if (($permissions & \OCP\Constants::PERMISSION_READ) === 0) {
+			return new \OC_OCS_Result(null, 400, 'invalid permissions');
+		}
+
 		try {
 			$return = \OCP\Share::setPermissions(
 					$itemType,

diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php
@@ -119,6 +119,32 @@ function testCreateShare() {
 		\OCP\Share::unshare('folder', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_LINK, null);
 	}
 
+	/**
+	 * @medium
+	 */
+	public function testCreateShareInvalidPermissions() {
+
+		// simulate a post request
+		$_POST['path'] = $this->filename;
+		$_POST['shareWith'] = \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2;
+		$_POST['shareType'] = \OCP\Share::SHARE_TYPE_USER;
+		$_POST['permissions'] = \OCP\Constants::PERMISSION_SHARE;
+
+		$result = \OCA\Files_Sharing\API\Local::createShare([]);
+
+		// share was successful?
+		$this->assertFalse($result->succeeded());
+		$this->assertEquals(400, $result->getStatusCode());
+
+		$shares = \OCP\Share::getItemShared('file', null);
+		$this->assertCount(0, $shares);
+
+		$fileinfo = $this->view->getFileInfo($this->filename);
+		\OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER,
+				\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2);
+	}
+
+
 	function testEnfoceLinkPassword() {
 
 		$appConfig = \OC::$server->getAppConfig();
@@ -883,6 +909,51 @@ function testUpdateShare() {
 
 	}
 
+	/**
+	 * @medium
+	 * @depends testCreateShare
+	 */
+	public function testUpdateShareInvalidPermissions() {
+
+		$fileInfo = $this->view->getFileInfo($this->filename);
+
+		$result = \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER,
+				\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_ALL);
+
+		// share was successful?
+		$this->assertTrue($result);
+
+		$share = \OCP\Share::getItemShared('file', null);
+		$this->assertCount(1, $share);
+		$share = reset($share);
+
+		// check if share have expected permissions, single shared files never have
+		// delete permissions
+		$this->assertEquals(\OCP\Constants::PERMISSION_ALL & ~\OCP\Constants::PERMISSION_DELETE, $share['permissions']);
+
+		// update permissions
+		$params = [];
+		$params['id'] = $share['id'];
+		$params['_put'] = [];
+		$params['_put']['permissions'] = \OCP\Constants::PERMISSION_SHARE;
+
+		$result = \OCA\Files_Sharing\API\Local::updateShare($params);
+
+		//Updating should fail with 400
+		$this->assertFalse($result->succeeded());
+		$this->assertEquals(400, $result->getStatusCode());
+
+		$share = \OCP\Share::getItemShared('file', $share['file_source']);
+		$share = reset($share);
+
+		//Permissions should not have changed!
+		$this->assertEquals(\OCP\Constants::PERMISSION_ALL & ~\OCP\Constants::PERMISSION_DELETE, $share['permissions']);
+
+		\OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER,
+				\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2);
+	}
+
+
 	/**
 	 * @medium
 	 */