Fix default permissions for link shares

apps/files_sharing/lib/API/Share20OCS.php

@@ -343,7 +343,7 @@ public function createShare() {
 			// legacy way, expecting that this won't be used together with "create-only" shares
 			$publicUpload = $this->request->getParam('publicUpload', null);
 			// a few permission checks
-			if ($publicUpload === 'true' || ($permissions & \OCP\Constants::PERMISSION_CREATE) > 0) {
+			if ($publicUpload === 'true' || $permissions === \OCP\Constants::PERMISSION_CREATE) {
 				// Check if public upload is allowed
 				if (!$this->shareManager->shareApiLinkAllowPublicUpload()) {
 					$share->getNode()->unlock(ILockingProvider::LOCK_SHARED);

apps/files_sharing/tests/API/Share20OCSTest.php

@@ -1141,6 +1141,82 @@ public function testCreateShareLinkCreateOnlyFolder() {
 		$this->assertEquals($expected->getData(), $result->getData());
 	}
 
+	public function testCreateShareLinkCreateOnlyFolderPublicUploadDisabled() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', Share::SHARE_TYPE_LINK],
+				['permissions', null, '4'],
+				['expireDate', '', ''],
+				['password', '', ''],
+			]));
+
+		$path = $this->createMock('\OCP\Files\Folder');
+		$storage = $this->createMock('OCP\Files\Storage');
+		$storage->method('instanceOfStorage')
+			->with('OCA\Files_Sharing\External\Storage')
+			->willReturn(false);
+		$path->method('getStorage')->willReturn($storage);
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(false);
+
+		$expected = new \OC\OCS\Result(null, 403, 'Public upload disabled by the administrator');
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function testCreateShareLinkDefaultPerms() {
+		$ocs = $this->mockFormatShare();
+
+		$this->request
+			->method('getParam')
+			->will($this->returnValueMap([
+				['path', null, 'valid-path'],
+				['shareType', '-1', Share::SHARE_TYPE_LINK],
+				['expireDate', '', ''],
+				['password', '', ''],
+			]));
+
+		$path = $this->createMock('\OCP\Files\Folder');
+		$storage = $this->createMock('OCP\Files\Storage');
+		$storage->method('instanceOfStorage')
+			->with('OCA\Files_Sharing\External\Storage')
+			->willReturn(false);
+		$path->method('getStorage')->willReturn($storage);
+		$this->rootFolder->method('getUserFolder')->with($this->currentUser->getUID())->will($this->returnSelf());
+		$this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+
+		$this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
+		$this->shareManager->method('shareApiAllowLinks')->willReturn(true);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(false);
+
+		$this->shareManager->expects($this->once())->method('createShare')->with(
+			$this->callback(function (\OCP\Share\IShare $share) use ($path) {
+				return $share->getNode() === $path &&
+					$share->getShareType() === Share::SHARE_TYPE_LINK &&
+					$share->getPermissions() === (\OCP\Constants::PERMISSION_READ) &&
+					$share->getSharedBy() === 'currentUser' &&
+					$share->getPassword() === null &&
+					$share->getExpirationDate() === null;
+			})
+		)->will($this->returnArgument(0));
+
+		$expected = new \OC\OCS\Result(null);
+		$result = $ocs->createShare();
+
+		$this->assertEquals($expected->getMeta(), $result->getMeta());
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
 	public function testCreateShareLinkPassword() {
 		$ocs = $this->mockFormatShare();
 

tests/integration/features/bootstrap/Sharing.php

@@ -10,6 +10,7 @@
 
 trait Sharing {
 	use Provisioning;
+	use AppConfiguration;
 
 	/** @var int */
 	private $sharingApiVersion = 1;
@@ -672,5 +673,8 @@ private function getLastShareToken() {
 		return $this->lastShareData->data->token;
 	}
 
+	protected function resetAppConfigs() {
+		$this->modifyServerConfig('core', 'shareapi_allow_public_upload', 'yes');
+	}
 }
 

tests/integration/features/sharing-v1.feature

@@ -1158,6 +1158,37 @@ Feature: sharing
 		Then the OCS status code should be "100"
 		And the HTTP status code should be "200"
 
+	Scenario: Creating link share with no specified permissions defaults to read permissions
+		Given As an "admin"
+		And user "user0" exists
+		And user "user0" created a folder "/afolder"
+		And As an "user0"
+		And creating a share with
+			| path | /afolder |
+			| shareType | 3 |
+		Then the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		And Share fields of last share match with
+			| id | A_NUMBER |
+			| share_type | 3 |
+			| permissions | 1 |
+
+	Scenario: Creating link share with no specified permissions defaults to read permissions when public upload disabled globally
+		Given As an "admin"
+		And parameter "shareapi_allow_public_upload" of app "core" is set to "no"
+		And user "user0" exists
+		And user "user0" created a folder "/afolder"
+		And As an "user0"
+		And creating a share with
+			| path | /afolder |
+			| shareType | 3 |
+		Then the OCS status code should be "100"
+		And the HTTP status code should be "200"
+		And Share fields of last share match with
+			| id | A_NUMBER |
+			| share_type | 3 |
+			| permissions | 1 |
+
 	Scenario: resharing using a public link with read only permissions is not allowed
 		Given As an "admin"
 		And user "user0" exists
@@ -1254,3 +1285,4 @@ Feature: sharing
 		And as "user0" the folder "/shared/sub" does not exist
 		And as "user0" the folder "/sub" exists in trash
 		And as "user0" the file "/sub/shared_file.txt" exists in trash
+