From 4c069b6d7e15ffd2b4e9b1a75aa08a228bd0b5a4 Mon Sep 17 00:00:00 2001 From: mmattel Date: Fri, 15 Nov 2019 10:44:23 +0100 Subject: [PATCH 1/3] New mount setting: read only mount --- apps/files_external/js/settings.js | 6 ++++++ apps/files_external/lib/Command/ListCommand.php | 1 + apps/files_external/templates/settings.php | 1 + apps/files_external/tests/js/settingsSpec.js | 1 + changelog/unreleased/36397 | 7 +++++++ lib/private/legacy/util.php | 10 ++++++++++ tests/lib/Files/External/StorageConfigTest.php | 4 ++-- 7 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/36397 diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 2a1a444b3ab7..4d92bc8d3759 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -19,6 +19,10 @@ var MOUNT_OPTIONS_DROPDOWN_TEMPLATE = ' ' + ' ' + '
' + + ' ' + + ' ' + + '
' + + '
' + ' ' + ' ' + '
' + @@ -960,6 +964,7 @@ MountConfigListView.prototype = _.extend({ // FIXME default backend mount options $tr.find('input.mountOptions').val(JSON.stringify({ 'encrypt': true, + 'read_only': false, 'previews': true, 'enable_sharing': false, 'filesystem_check_changes': 1, @@ -1342,6 +1347,7 @@ MountConfigListView.prototype = _.extend({ var $toggle = $tr.find('.mountOptionsToggle'); var dropDown = new MountOptionsDropdown(); var visibleOptions = [ + 'read_only', 'previews', 'filesystem_check_changes', 'encoding_compatibility' diff --git a/apps/files_external/lib/Command/ListCommand.php b/apps/files_external/lib/Command/ListCommand.php index 8e56053b17b5..80d653bbaca6 100644 --- a/apps/files_external/lib/Command/ListCommand.php +++ b/apps/files_external/lib/Command/ListCommand.php @@ -184,6 +184,7 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output 'encrypt' => true, 'previews' => true, 'filesystem_check_changes' => 1, + 'read_only' => false, 'enable_sharing' => false, 'encoding_compatibility' => false ]; diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 83e3e0ef6081..d507274c55e5 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -5,6 +5,7 @@ use OCP\Files\External\IStoragesBackendService; $l->t("Enable encryption"); + $l->t("Set read-only"); $l->t("Enable previews"); $l->t("Enable sharing"); $l->t("Check for changes"); diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js index 98ff4455d8a1..5a7611fa7c15 100644 --- a/apps/files_external/tests/js/settingsSpec.js +++ b/apps/files_external/tests/js/settingsSpec.js @@ -371,6 +371,7 @@ describe('OCA.External.Settings tests', function() { expect(JSON.parse($tr.find('input.mountOptions').val())).toEqual({ encrypt: true, previews: true, + read_only: false, enable_sharing: false, filesystem_check_changes: 0, encoding_compatibility: false diff --git a/changelog/unreleased/36397 b/changelog/unreleased/36397 new file mode 100644 index 000000000000..db8cba8b2f67 --- /dev/null +++ b/changelog/unreleased/36397 @@ -0,0 +1,7 @@ +Enhancement: Adding a option in the mount settings to provide a mount in read only mode + +Adds a new option in the mount settings to provide a mount in read only mode. +This enables users or admins to provide a write protected mount independent of any backend settings. +The sync client automatically respects this mount setting without any additional intervention. + +https://github.com/owncloud/core/pull/36397 diff --git a/lib/private/legacy/util.php b/lib/private/legacy/util.php index 8cdfbc577ecf..e3d2647739a0 100644 --- a/lib/private/legacy/util.php +++ b/lib/private/legacy/util.php @@ -175,6 +175,16 @@ public static function setupFS($user = '') { return $storage; }); + \OC\Files\Filesystem::addStorageWrapper('read_only', function ($mountPoint, \OCP\Files\Storage $storage, \OCP\Files\Mount\IMountPoint $mount) { + if ($mount->getOption('read_only', false)) { + return new \OC\Files\Storage\Wrapper\PermissionsMask([ + 'storage' => $storage, + 'mask' => \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE + ]); + } + return $storage; + }); + // install storage availability wrapper, before most other wrappers \OC\Files\Filesystem::addStorageWrapper('oc_availability', function ($mountPoint, $storage) { if (!$storage->instanceOfStorage('\OCA\Files_Sharing\SharedStorage') && !$storage->isLocal()) { diff --git a/tests/lib/Files/External/StorageConfigTest.php b/tests/lib/Files/External/StorageConfigTest.php index 6af361adb650..fa0521122cda 100644 --- a/tests/lib/Files/External/StorageConfigTest.php +++ b/tests/lib/Files/External/StorageConfigTest.php @@ -59,7 +59,7 @@ public function testJsonSerialization() { $storageConfig->setPriority(128); $storageConfig->setApplicableUsers(['user1', 'user2']); $storageConfig->setApplicableGroups(['group1', 'group2']); - $storageConfig->setMountOptions(['preview' => false]); + $storageConfig->setMountOptions(['preview' => false, 'read_only' => true]); $json = $storageConfig->jsonSerialize(); @@ -74,6 +74,6 @@ public function testJsonSerialization() { $this->assertSame(128, $json['priority']); $this->assertSame(['user1', 'user2'], $json['applicableUsers']); $this->assertSame(['group1', 'group2'], $json['applicableGroups']); - $this->assertSame(['preview' => false], $json['mountOptions']); + $this->assertSame(['preview' => false, 'read_only' => true], $json['mountOptions']); } } From b94f11929f69e0df5be0f621311981fb1a401ef3 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Sat, 28 Dec 2019 17:10:09 +0545 Subject: [PATCH 2/3] Minor review changes --- apps/files_external/js/settings.js | 4 ++-- changelog/unreleased/36397 | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 4d92bc8d3759..3e565590d289 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -19,8 +19,8 @@ var MOUNT_OPTIONS_DROPDOWN_TEMPLATE = ' ' + ' ' + '
' + - ' ' + - ' ' + + ' ' + + ' ' + '
' + '
' + ' ' + diff --git a/changelog/unreleased/36397 b/changelog/unreleased/36397 index db8cba8b2f67..209e801874e1 100644 --- a/changelog/unreleased/36397 +++ b/changelog/unreleased/36397 @@ -1,4 +1,4 @@ -Enhancement: Adding a option in the mount settings to provide a mount in read only mode +Enhancement: Add an option to provide a mount in read only mode Adds a new option in the mount settings to provide a mount in read only mode. This enables users or admins to provide a write protected mount independent of any backend settings. From 2a19481fd5aa71453b7be2dab9c23f87c867524b Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Mon, 30 Dec 2019 17:04:36 +0545 Subject: [PATCH 3/3] Add acceptance test for read-only storage mount --- .../acceptance/features/bootstrap/WebDav.php | 68 +++++++++++++++++-- .../WebUIAdminStorageSettingsContext.php | 41 +++++++++-- .../features/lib/AdminStorageSettingsPage.php | 42 ++++++++++++ .../adminStorageSettings.feature | 27 +++++++- 4 files changed, 169 insertions(+), 9 deletions(-) diff --git a/tests/acceptance/features/bootstrap/WebDav.php b/tests/acceptance/features/bootstrap/WebDav.php index 298679551e97..ce6fe454e863 100644 --- a/tests/acceptance/features/bootstrap/WebDav.php +++ b/tests/acceptance/features/bootstrap/WebDav.php @@ -545,6 +545,39 @@ public function userMovesFileUsingTheAPI( } } + /** + * @Then /^user "([^"]*)" should be able to rename (file|folder|entry) "([^"]*)" to "([^"]*)"$/ + * + * @param string $user + * @param string $entry + * @param string $source + * @param string $destination + * + * @return void + */ + public function theUserShouldBeAbleToRenameEntryTo($user, $entry, $source, $destination) { + $this->asFileOrFolderShouldExist($user, $entry, $source); + $this->userMovesFileUsingTheAPI($user, $source, "", $destination); + $this->asFileOrFolderShouldNotExist($user, $entry, $source); + $this->asFileOrFolderShouldExist($user, $entry, $destination); + } + + /** + * @Then /^user "([^"]*)" should not be able to rename (file|folder|entry) "([^"]*)" to "([^"]*)"$/ + * + * @param string $user + * @param string $entry + * @param string $source + * @param string $destination + * + * @return void + */ + public function theUserShouldNotBeAbleToRenameEntryTo($user, $entry, $source, $destination) { + $this->asFileOrFolderShouldExist($user, $entry, $source); + $this->userMovesFileUsingTheAPI($user, $source, "", $destination); + $this->asFileOrFolderShouldExist($user, $entry, $source); + } + /** * @When /^user "([^"]*)" on "(LOCAL|REMOTE)" moves (?:file|folder|entry) "([^"]*)" to "([^"]*)" using the WebDAV API$/ * @@ -1739,15 +1772,13 @@ public function userUploadsAFileToEndingWithOfSizeBytes($user, $destination, $te } /** - * @When user :user uploads file with content :content to :destination using the WebDAV API - * * @param string $user * @param string $content * @param string $destination * * @return string */ - public function userUploadsAFileWithContentTo( + public function uploadFileWithContent( $user, $content, $destination ) { $file = \GuzzleHttp\Stream\Stream::factory($content); @@ -1759,6 +1790,35 @@ public function userUploadsAFileWithContentTo( return $this->response->getHeader('oc-fileid'); } + /** + * @When the administrator uploads file with content :content to :destination using the WebDAV API + * + * @param string $content + * @param string $destination + * + * @return string + */ + public function adminUploadsAFileWithContentTo( + $content, $destination + ) { + return $this->uploadFileWithContent($this->getAdminUsername(), $content, $destination); + } + + /** + * @When user :user uploads file with content :content to :destination using the WebDAV API + * + * @param string $user + * @param string $content + * @param string $destination + * + * @return string + */ + public function userUploadsAFileWithContentTo( + $user, $content, $destination + ) { + return $this->uploadFileWithContent($user, $content, $destination); + } + /** * @Given user :user has uploaded file with content :content to :destination * @@ -1771,7 +1831,7 @@ public function userUploadsAFileWithContentTo( public function userHasUploadedAFileWithContentTo( $user, $content, $destination ) { - $fileId = $this->userUploadsAFileWithContentTo($user, $content, $destination); + $fileId = $this->uploadFileWithContent($user, $content, $destination); $this->theHTTPStatusCodeShouldBeOr("201", "204"); return $fileId; } diff --git a/tests/acceptance/features/bootstrap/WebUIAdminStorageSettingsContext.php b/tests/acceptance/features/bootstrap/WebUIAdminStorageSettingsContext.php index e1b5d7ea17ae..a83545c86907 100644 --- a/tests/acceptance/features/bootstrap/WebUIAdminStorageSettingsContext.php +++ b/tests/acceptance/features/bootstrap/WebUIAdminStorageSettingsContext.php @@ -96,14 +96,12 @@ public function theAdministratorDisablesTheExternalStorageUsingTheWebui() { } /** - * @When the administrator creates the local storage mount :mount using the webUI - * @Given the administrator has created the local storage mount :mount from the admin storage settings page - * * @param string $mount * * @return void + * @throws Exception */ - public function theAdministratorCreatesTheLocalStorageMountUsingTheWebui($mount) { + public function createLocalStorageMountUsingTheWebui($mount) { $serverRoot = SetupHelper::getServerRoot( $this->featureContext->getBaseUrl(), $this->featureContext->getAdminUsername(), @@ -122,6 +120,31 @@ public function theAdministratorCreatesTheLocalStorageMountUsingTheWebui($mount) $this->featureContext->addStorageId($mount, $lastMount + 1); } + /** + * @When the administrator creates the local storage mount :mount using the webUI + * + * @param string $mount + * + * @return void + * @throws Exception + */ + public function theAdministratorCreatesTheLocalStorageMountUsingTheWebui($mount) { + $this->createLocalStorageMountUsingTheWebui($mount); + } + + /** + * @Given the administrator has created the local storage mount :mount from the admin storage settings page + * + * @param string $mount + * + * @return void + * @throws Exception + */ + public function theAdministratorHasCreatedTheLocalStorageMountUsingTheWebui($mount) { + $this->createLocalStorageMountUsingTheWebui($mount); + $this->featureContext->asFileOrFolderShouldExist($this->featureContext->getAdminUsername(), "folder", $mount); + } + /** * @When /^the administrator (adds|removes) (user|group) "([^"]*)" (?:as|from) the applicable (?:user|group) for the last local storage mount using the webUI$/ * @Given /^the administrator has (added|removed) (user|group) "([^"]*)" (?:as|from) the applicable (?:user|group) for the last local storage mount from the admin storage settings page$/ @@ -161,6 +184,16 @@ public function theAdministratorDeletesTheLastCreatedLocalStorageMountUsingTheWe $this->adminStorageSettingsPage->deleteLastCreatedLocalMount($this->getSession()); } + /** + * @When the administrator enables read-only for the last created local storage mount using the webUI + * + * @return void + */ + public function theAdministratorEnablesReadonlyForTheLastCreatedLocalStorageMountUsingTheWebui() { + $this->adminStorageSettingsPage->openMountOptions($this->getSession()); + $this->adminStorageSettingsPage->enableReadonlyMountOption($this->getSession()); + } + /** * @Then /^the last created local storage mount should (not|)\s?be listed on the webUI$/ * diff --git a/tests/acceptance/features/lib/AdminStorageSettingsPage.php b/tests/acceptance/features/lib/AdminStorageSettingsPage.php index 0b8ec817a633..64bc00d5d7e7 100644 --- a/tests/acceptance/features/lib/AdminStorageSettingsPage.php +++ b/tests/acceptance/features/lib/AdminStorageSettingsPage.php @@ -56,6 +56,8 @@ class AdminStorageSettingsPage extends OwncloudPage { protected $applicableUserXpath = "//tr[@id='addMountPoint']/preceding-sibling::tr[1]//li[@class='select2-search-choice'][%s]//span"; protected $applicableUserDeleteXpath = "//tr[@id='addMountPoint']/preceding-sibling::tr[1]//li[@class='select2-search-choice'][%s]//a"; protected $lastCreatedMountDeleteButtonXpath = "//tr[@id='addMountPoint']/preceding-sibling::tr[1]/td[@class='remove']"; + protected $lastCreatedMountOptionsButtonXpath = "//tr[@id='addMountPoint']/preceding-sibling::tr[1]/td[@class='mountOptionsToggle']"; + protected $setReadonlyId = "mountOptionsReadonly"; protected $mountPointNameXpath = "//tr[@class='local'][%s]//input[@placeholder='Folder name']"; /** @@ -282,6 +284,46 @@ public function waitUntilSuccessOrFailureSymbolAppears( } } + /** + * open the mount options/advanced settings + * + * @param Session $session + * + * @return void + */ + public function openMountOptions($session) { + $lastCreatedMountOptionsButton = $this->find( + "xpath", $this->lastCreatedMountOptionsButtonXpath + ); + $this->assertElementNotNull( + $lastCreatedMountOptionsButton, + __METHOD__ . + " xpath $this->lastCreatedMountOptionsButtonXpath " . + "could not find mount options advanced settings button for last created mount" + ); + $lastCreatedMountOptionsButton->click(); + } + + /** + * enable read-only mount option + * + * @param Session $session + * + * @return void + */ + public function enableReadonlyMountOption(Session $session) { + $checkCheckbox = $this->findById($this->setReadonlyId); + $this->assertElementNotNull( + $checkCheckbox, + __METHOD__ . + " id " . $this->setReadonlyId . + " could not find checkbox for read-only mount option" + ); + if ((!($checkCheckbox->isChecked()))) { + $checkCheckbox->click(); + } + } + /** * toggle checkbox * diff --git a/tests/acceptance/features/webUIAdminSettings/adminStorageSettings.feature b/tests/acceptance/features/webUIAdminSettings/adminStorageSettings.feature index 765401240b83..de7aa2d3f1c8 100644 --- a/tests/acceptance/features/webUIAdminSettings/adminStorageSettings.feature +++ b/tests/acceptance/features/webUIAdminSettings/adminStorageSettings.feature @@ -19,8 +19,33 @@ Feature: admin storage settings And the administrator has browsed to the admin storage settings page And the administrator has enabled the external storage When the administrator creates the local storage mount "local_storage1" using the webUI + And the administrator uploads file with content "this is a file in local storage" to "/local_storage1/file-in-local-storage.txt" using the WebDAV API And the user re-logs in as "user0" using the webUI - Then folder "local_storage1" should be listed on the webUI + And user "user0" uploads file "filesForUpload/textfile.txt" to filenames based on "/local_storage1/textfile.txt" with all mechanisms using the WebDAV API + Then the HTTP status code of all upload responses should be "201" + And as "user0" the files uploaded to "/local_storage1/textfile.txt" with all mechanisms should exist + And as "user0" file "local_storage1/file-in-local-storage.txt" should exist + And the content of file "/local_storage1/file-in-local-storage.txt" for user "user0" should be "this is a file in local storage" + And user "user0" should be able to rename file "/local_storage1/file-in-local-storage.txt" to "/local_storage1/another-name.txt" + And user "user0" should be able to delete file "/local_storage1/another-name.txt" + And folder "local_storage1" should be listed on the webUI + + Scenario: administrator creates a read-only local storage mount + Given user "user0" has been created with default attributes and without skeleton files + And the administrator has browsed to the admin storage settings page + And the administrator has enabled the external storage + When the administrator creates the local storage mount "local_storage1" using the webUI + And the administrator uploads file with content "this is a file in local storage" to "/local_storage1/file-in-local-storage.txt" using the WebDAV API + And the administrator enables read-only for the last created local storage mount using the webUI + And the user re-logs in as "user0" using the webUI + And user "user0" uploads file "filesForUpload/textfile.txt" to filenames based on "/local_storage1/textfile.txt" with all mechanisms using the WebDAV API + Then the HTTP status code of all upload responses should be "403" + And as "user0" the files uploaded to "/local_storage1/textfile.txt" with all mechanisms should not exist + And as "user0" file "local_storage1/file-in-local-storage.txt" should exist + And user "user0" should not be able to rename file "/local_storage1/file-in-local-storage.txt" to "/local_storage1/another-name.txt" + And user "user0" should not be able to delete file "/local_storage1/file-in-local-storage.txt" + And the content of file "/local_storage1/file-in-local-storage.txt" for user "user0" should be "this is a file in local storage" + And folder "local_storage1" should be listed on the webUI Scenario: administrator assigns an applicable user to a local storage mount Given these users have been created with default attributes and without skeleton files: