Merge pull request #33713 from nextcloud/bugfix/noid/viewonlyfix

apps/dav/lib/DAV/ViewOnlyPlugin.php

@@ -57,6 +57,7 @@ class ViewOnlyPlugin extends ServerPlugin {
 		//priority 90 to make sure the plugin is called before
 		//Sabre\DAV\CorePlugin::httpGet
 		$this->server->on('method:GET', [$this, 'checkViewOnly'], 90);
+		$this->server->on('method:COPY', [$this, 'checkViewOnly'], 90);
 	}
 
 	/**

apps/files/js/fileactions.js

@@ -673,6 +673,9 @@
 				displayName: function(context) {
 					var permissions = context.fileInfoModel.attributes.permissions;
 					if (permissions & OC.PERMISSION_UPDATE) {
+						if (!context.fileInfoModel.canDownload()) {
+							return t('files', 'Move');
+						}
 						return t('files', 'Move or copy');
 					}
 					return t('files', 'Copy');
@@ -685,7 +688,11 @@
 					var permissions = context.fileInfoModel.attributes.permissions;
 					var actions = OC.dialogs.FILEPICKER_TYPE_COPY;
 					if (permissions & OC.PERMISSION_UPDATE) {
-						actions = OC.dialogs.FILEPICKER_TYPE_COPY_MOVE;
+						if (!context.fileInfoModel.canDownload()) {
+							actions = OC.dialogs.FILEPICKER_TYPE_MOVE;
+						} else {
+							actions = OC.dialogs.FILEPICKER_TYPE_COPY_MOVE;
+						}
 					}
 					var dialogDir = context.dir;
 					if (typeof context.fileList.dirInfo.dirLastCopiedTo !== 'undefined') {

apps/files/js/fileinfomodel.js

@@ -126,7 +126,18 @@
 				});
 
 			return deferred.promise();
-		}
+		},
+
+		canDownload: function() {
+			for (const i in this.attributes.shareAttributes) {
+				const attr = this.attributes.shareAttributes[i]
+				if (attr.scope === 'permissions' && attr.key === 'download') {
+					return attr.enabled
+				}
+			}
+
+			return true
+		},
 	});
 
 	if (!OCA.Files) {

apps/files_sharing/src/share.js

@@ -94,6 +94,10 @@ import { getCapabilities } from '@nextcloud/capabilities'
 				}
 				if (_.isFunction(fileData.canDownload) && !fileData.canDownload()) {
 					delete fileActions.actions.all.Download
+					if (fileData.permissions & OC.PERMISSION_UPDATE === 0) {
+						// neither move nor copy is allowed, remove the action completely
+						delete fileActions.actions.all.MoveCopy
+					}
 				}
 				tr.attr('data-share-permissions', sharePermissions)
 				tr.attr('data-share-attributes', JSON.stringify(fileData.shareAttributes))

build/integration/features/bootstrap/Sharing.php

@@ -275,7 +275,8 @@ trait Sharing {
 								$shareWith = null,
 								$publicUpload = null,
 								$password = null,
-								$permissions = null) {
+								$permissions = null,
+								$viewOnly = false) {
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares";
 		$client = new Client();
 		$options = [
@@ -309,6 +310,10 @@ trait Sharing {
 			$body['permissions'] = $permissions;
 		}
 
+		if ($viewOnly === true) {
+			$body['attributes'] = json_encode([['scope' => 'permissions', 'key' => 'download', 'enabled' => false]]);
+		}
+
 		$options['form_params'] = $body;
 
 		try {
@@ -402,13 +407,17 @@ trait Sharing {
 	}
 
 	/**
-	 * @Given /^(file|folder|entry) "([^"]*)" of user "([^"]*)" is shared with user "([^"]*)"( with permissions ([\d]*))?$/
+	 * @Given /^(file|folder|entry) "([^"]*)" of user "([^"]*)" is shared with user "([^"]*)"( with permissions ([\d]*))?( view-only)?$/
 	 *
 	 * @param string $filepath
 	 * @param string $user1
 	 * @param string $user2
 	 */
-	public function assureFileIsShared($entry, $filepath, $user1, $user2, $withPerms = null, $permissions = null) {
+	public function assureFileIsShared($entry, $filepath, $user1, $user2, $withPerms = null, $permissions = null, $viewOnly = null) {
+		// when view-only is set, permissions is empty string instead of null...
+		if ($permissions === '') {
+			$permissions = null;
+		}
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares" . "?path=$filepath";
 		$client = new Client();
 		$options = [];
@@ -424,20 +433,24 @@ trait Sharing {
 		if ($this->isUserOrGroupInSharedData($user2, $permissions)) {
 			return;
 		} else {
-			$this->createShare($user1, $filepath, 0, $user2, null, null, $permissions);
+			$this->createShare($user1, $filepath, 0, $user2, null, null, $permissions, $viewOnly !== null);
 		}
 		$this->response = $client->get($fullUrl, $options);
 		Assert::assertEquals(true, $this->isUserOrGroupInSharedData($user2, $permissions));
 	}
 
 	/**
-	 * @Given /^(file|folder|entry) "([^"]*)" of user "([^"]*)" is shared with group "([^"]*)"( with permissions ([\d]*))?$/
+	 * @Given /^(file|folder|entry) "([^"]*)" of user "([^"]*)" is shared with group "([^"]*)"( with permissions ([\d]*))?( view-only)?$/
 	 *
 	 * @param string $filepath
 	 * @param string $user
 	 * @param string $group
 	 */
-	public function assureFileIsSharedWithGroup($entry, $filepath, $user, $group, $withPerms = null, $permissions = null) {
+	public function assureFileIsSharedWithGroup($entry, $filepath, $user, $group, $withPerms = null, $permissions = null, $viewOnly = null) {
+		// when view-only is set, permissions is empty string instead of null...
+		if ($permissions === '') {
+			$permissions = null;
+		}
 		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares" . "?path=$filepath";
 		$client = new Client();
 		$options = [];
@@ -453,7 +466,7 @@ trait Sharing {
 		if ($this->isUserOrGroupInSharedData($group, $permissions)) {
 			return;
 		} else {
-			$this->createShare($user, $filepath, 1, $group, null, null, $permissions);
+			$this->createShare($user, $filepath, 1, $group, null, null, $permissions, $viewOnly !== null);
 		}
 		$this->response = $client->get($fullUrl, $options);
 		Assert::assertEquals(true, $this->isUserOrGroupInSharedData($group, $permissions));

build/integration/sharing_features/sharing-v1-part2.feature

@@ -1167,4 +1167,44 @@ Feature: sharing
       |{http://open-collaboration-services.org/ns}share-permissions |
     Then the single response should contain a property "{http://open-collaboration-services.org/ns}share-permissions" with value "19"
 
+  Scenario: Cannot download a file when it's shared view-only
+    Given user "user0" exists
+    And user "user1" exists
+    And User "user0" moves file "/textfile0.txt" to "/document.odt"
+    And file "document.odt" of user "user0" is shared with user "user1" view-only
+    And user "user1" accepts last share
+    When As an "user1"
+    And Downloading file "/document.odt"
+    Then the HTTP status code should be "403"
+
+  Scenario: Cannot download a file when its parent is shared view-only
+    Given user "user0" exists
+    And user "user1" exists
+    And User "user0" created a folder "/sharedviewonly"
+    And User "user0" moves file "/textfile0.txt" to "/sharedviewonly/document.odt"
+    And folder "sharedviewonly" of user "user0" is shared with user "user1" view-only
+    And user "user1" accepts last share
+    When As an "user1"
+    And Downloading file "/sharedviewonly/document.odt"
+    Then the HTTP status code should be "403"
+
+  Scenario: Cannot copy a file when it's shared view-only
+    Given user "user0" exists
+    And user "user1" exists
+    And User "user0" moves file "/textfile0.txt" to "/document.odt"
+    And file "document.odt" of user "user0" is shared with user "user1" view-only
+    And user "user1" accepts last share
+    When User "user1" copies file "/document.odt" to "/copyforbidden.odt"
+    Then the HTTP status code should be "403"
+
+  Scenario: Cannot copy a file when its parent is shared view-only
+    Given user "user0" exists
+    And user "user1" exists
+    And User "user0" created a folder "/sharedviewonly"
+    And User "user0" moves file "/textfile0.txt" to "/sharedviewonly/document.odt"
+    And folder "sharedviewonly" of user "user0" is shared with user "user1" view-only
+    And user "user1" accepts last share
+    When User "user1" copies file "/sharedviewonly/document.odt" to "/copyforbidden.odt"
+    Then the HTTP status code should be "403"
+
 # See sharing-v1-part3.feature