From 555d582f35d1704996c3bf72510a8272cc38f833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 17:28:58 +0100 Subject: [PATCH 1/6] Return whether the file is readable or not in the DAV permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now it was safe to assume that every file was readable by its owner, so there was no need to return whether the file was readable or not. However, with the introduction of end to end encryption that is no longer the case, and it is now necessary to explicitly provide that information. Signed-off-by: Daniel Calviño Sánchez --- apps/dav/lib/Connector/Sabre/Node.php | 3 +++ .../tests/unit/Connector/Sabre/NodeTest.php | 20 ++++++++++--------- .../features/sharing-v1-part3.feature | 12 +++++------ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index b6d4090bf8fc2..a046b734661d1 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -299,6 +299,9 @@ public function getDavPermissions() { if ($this->info->isMounted()) { $p .= 'M'; } + if ($this->info->isReadable()) { + $p .= 'G'; + } if ($this->info->isDeletable()) { $p .= 'D'; } diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php index fe6cbd97829eb..a6b9e81d9d822 100644 --- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php @@ -41,15 +41,17 @@ class NodeTest extends \Test\TestCase { public function davPermissionsProvider() { return array( - array(\OCP\Constants::PERMISSION_ALL, 'file', false, false, 'RDNVW'), - array(\OCP\Constants::PERMISSION_ALL, 'dir', false, false, 'RDNVCK'), - array(\OCP\Constants::PERMISSION_ALL, 'file', true, false, 'SRDNVW'), - array(\OCP\Constants::PERMISSION_ALL, 'file', true, true, 'SRMDNVW'), - array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_SHARE, 'file', true, false, 'SDNVW'), - array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_UPDATE, 'file', false, false, 'RD'), - array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_DELETE, 'file', false, false, 'RNVW'), - array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'file', false, false, 'RDNVW'), - array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'dir', false, false, 'RDNV'), + array(\OCP\Constants::PERMISSION_ALL, 'file', false, false, 'RGDNVW'), + array(\OCP\Constants::PERMISSION_ALL, 'dir', false, false, 'RGDNVCK'), + array(\OCP\Constants::PERMISSION_ALL, 'file', true, false, 'SRGDNVW'), + array(\OCP\Constants::PERMISSION_ALL, 'file', true, true, 'SRMGDNVW'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_SHARE, 'file', true, false, 'SGDNVW'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_UPDATE, 'file', false, false, 'RGD'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_DELETE, 'file', false, false, 'RGNVW'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'file', false, false, 'RGDNVW'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'file', false, false, 'RDNVW'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'dir', false, false, 'RGDNV'), + array(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'dir', false, false, 'RDNVCK'), ); } diff --git a/build/integration/features/sharing-v1-part3.feature b/build/integration/features/sharing-v1-part3.feature index b4b1ae9bf94d3..44a41341a028c 100644 --- a/build/integration/features/sharing-v1-part3.feature +++ b/build/integration/features/sharing-v1-part3.feature @@ -167,7 +167,7 @@ Feature: sharing And folder "/merge-test-outside-perms" of user "user0" is shared with user "user1" with permissions 31 Then as "user1" gets properties of folder "/merge-test-outside-perms" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRGDNVCK" And as "user1" the folder "/merge-test-outside-perms (2)" does not exist Scenario: Merging shares for recipient when shared from outside with two groups @@ -197,7 +197,7 @@ Feature: sharing And folder "/merge-test-outside-twogroups-perms" of user "user0" is shared with group "group2" with permissions 31 Then as "user1" gets properties of folder "/merge-test-outside-twogroups-perms" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRGDNVCK" And as "user1" the folder "/merge-test-outside-twogroups-perms (2)" does not exist Scenario: Merging shares for recipient when shared from outside with two groups and member @@ -214,7 +214,7 @@ Feature: sharing And folder "/merge-test-outside-twogroups-member-perms" of user "user0" is shared with user "user1" with permissions 1 Then as "user1" gets properties of folder "/merge-test-outside-twogroups-member-perms" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRGDNVCK" And as "user1" the folder "/merge-test-outside-twogroups-member-perms (2)" does not exist Scenario: Merging shares for recipient when shared from inside with group @@ -253,7 +253,7 @@ Feature: sharing And folder "/merge-test-inside-twogroups-perms" of user "user0" is shared with group "group2" Then as "user0" gets properties of folder "/merge-test-inside-twogroups-perms" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "RDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "RGDNVCK" And as "user0" the folder "/merge-test-inside-twogroups-perms (2)" does not exist And as "user0" the folder "/merge-test-inside-twogroups-perms (3)" does not exist @@ -270,7 +270,7 @@ Feature: sharing And folder "/merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with user "user1" Then as "user1" gets properties of folder "/merge-test-outside-groups-renamebeforesecondshare-renamed" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRGDNVCK" And as "user1" the folder "/merge-test-outside-groups-renamebeforesecondshare" does not exist Scenario: Merging shares for recipient when shared from outside with user then group and recipient renames in between @@ -287,7 +287,7 @@ Feature: sharing And folder "/merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with group "group1" Then as "user1" gets properties of folder "/merge-test-outside-groups-renamebeforesecondshare-renamed" with |{http://owncloud.org/ns}permissions| - And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK" + And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRGDNVCK" And as "user1" the folder "/merge-test-outside-groups-renamebeforesecondshare" does not exist Scenario: Empting trashbin From e6b74fac407b62168633be75312074e8ae0e2175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 14:55:58 +0100 Subject: [PATCH 2/6] Add proper handling of files without permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now a file gets its directory permissions only if it contained no permissions (they were undefined or null), but not if its permissions were set to "NONE". Besides that, now file actions that do not require any permission on the file to be performed can be used on files that have no permissions. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/fileactions.js | 2 +- apps/files/js/filelist.js | 7 ++++++- apps/files/tests/js/filelistSpec.js | 9 +++++++++ core/js/js.js | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 0f320c8b3c7c9..6c031ab06d5ac 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -235,7 +235,7 @@ } var filteredActions = {}; $.each(actions, function (name, action) { - if (action.permissions & permissions) { + if ((action.permissions === OC.PERMISSION_NONE) || (action.permissions & permissions)) { filteredActions[name] = action; } }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 4790afcf4d0a8..c19f517e42245 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -1159,6 +1159,11 @@ } } + var permissions = fileData.permissions; + if (permissions === undefined || permissions === null) { + permissions = this.getDirectoryPermissions(); + } + //containing tr var tr = $('').attr({ "data-id" : fileData.id, @@ -1168,7 +1173,7 @@ "data-mime": mime, "data-mtime": mtime, "data-etag": fileData.etag, - "data-permissions": fileData.permissions || this.getDirectoryPermissions(), + "data-permissions": permissions, "data-has-preview": fileData.hasPreview !== false }); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 8bb188e3602ae..8a3e8cb9d4ba8 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -296,6 +296,15 @@ describe('OCA.Files.FileList tests', function() { expect($tr.find('.filesize').text()).toEqual('Pending'); expect($tr.find('.date').text()).not.toEqual('?'); }); + it('generates file element with no permissions when permissions are explicitly none', function() { + var fileData = { + type: 'dir', + name: 'testFolder', + permissions: OC.PERMISSION_NONE + }; + var $tr = fileList.add(fileData); + expect($tr.attr('data-permissions')).toEqual('0'); + }); it('generates file element with zero size when size is explicitly zero', function() { var fileData = { type: 'dir', diff --git a/core/js/js.js b/core/js/js.js index c02ef5c792036..246aae3ac02aa 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -60,6 +60,7 @@ function fileDownloadPath(dir, file) { /** @namespace */ var OCP = {}, OC = { + PERMISSION_NONE:0, PERMISSION_CREATE:4, PERMISSION_READ:1, PERMISSION_UPDATE:2, From c047f32f25a9c1448bf78ef65d5144f5b882b719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 15:01:17 +0100 Subject: [PATCH 3/6] Remove requeriment for read permission on some actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Details and the Favorite actions do not require any permission on the files to be performed. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/filelist.js | 2 +- apps/files/js/tagsplugin.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index c19f517e42245..926a0b7137a9a 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -407,7 +407,7 @@ mime: 'all', order: -50, iconClass: 'icon-details', - permissions: OC.PERMISSION_READ, + permissions: OC.PERMISSION_NONE, actionHandler: function(fileName, context) { self._updateDetailsView(fileName); } diff --git a/apps/files/js/tagsplugin.js b/apps/files/js/tagsplugin.js index 747a7245a56b3..b174aa7d76620 100644 --- a/apps/files/js/tagsplugin.js +++ b/apps/files/js/tagsplugin.js @@ -104,7 +104,7 @@ }, mime: 'all', order: -100, - permissions: OC.PERMISSION_READ, + permissions: OC.PERMISSION_NONE, iconClass: function(fileName, context) { var $file = context.$file; var isFavorite = $file.data('favorite') === true; From e2c755a4b5638c298c8cae27942681241f92fb7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 16:14:31 +0100 Subject: [PATCH 4/6] Fix asserts silently not executed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first parameter of "apply" must be the object to act as "this", and the Promise callback gets the parameters provided in the "resolve". Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/files/clientSpec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/js/tests/specs/files/clientSpec.js b/core/js/tests/specs/files/clientSpec.js index f75998029a9cd..ec0e15fd7b366 100644 --- a/core/js/tests/specs/files/clientSpec.js +++ b/core/js/tests/specs/files/clientSpec.js @@ -640,14 +640,14 @@ describe('OC.Files.Client tests', function() { function testPermission(permission, isFile, expectedPermissions) { var promise = getFileInfoWithPermission(permission, isFile); - promise.then(function(result) { + promise.then(function(status, result) { expect(result.permissions).toEqual(expectedPermissions); }); } function testMountType(permission, isFile, expectedMountType) { var promise = getFileInfoWithPermission(permission, isFile); - promise.then(function(result) { + promise.then(function(status, result) { expect(result.mountType).toEqual(expectedMountType); }); } @@ -664,7 +664,7 @@ describe('OC.Files.Client tests', function() { ['CKWDR', true, OC.PERMISSION_ALL] ]; _.each(testCases, function(testCase) { - return testPermission.apply(testCase); + return testPermission.apply(this, testCase); }); }); it('properly parses folder permissions', function() { @@ -679,7 +679,7 @@ describe('OC.Files.Client tests', function() { ]; _.each(testCases, function(testCase) { - return testPermission.apply(testCase); + return testPermission.apply(this, testCase); }); }); it('properly parses mount types', function() { @@ -691,7 +691,7 @@ describe('OC.Files.Client tests', function() { ]; _.each(testCases, function(testCase) { - return testMountType.apply(testCase); + return testMountType.apply(this, testCase); }); }); }); From ec375b3d8682e8ce062433314c5a3d035b22b3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 16:31:04 +0100 Subject: [PATCH 5/6] Fix tests for parsing of permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that these tests are executed they are revealed to be partially obsolete; they were fixed to match the current parsing behaviour. Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/files/clientSpec.js | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/core/js/tests/specs/files/clientSpec.js b/core/js/tests/specs/files/clientSpec.js index ec0e15fd7b366..126ba3f4e6545 100644 --- a/core/js/tests/specs/files/clientSpec.js +++ b/core/js/tests/specs/files/clientSpec.js @@ -658,7 +658,7 @@ describe('OC.Files.Client tests', function() { ['', true, OC.PERMISSION_READ], ['C', true, OC.PERMISSION_READ | OC.PERMISSION_CREATE], ['K', true, OC.PERMISSION_READ | OC.PERMISSION_CREATE], - ['W', true, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE], + ['W', true, OC.PERMISSION_READ | OC.PERMISSION_UPDATE], ['D', true, OC.PERMISSION_READ | OC.PERMISSION_DELETE], ['R', true, OC.PERMISSION_READ | OC.PERMISSION_SHARE], ['CKWDR', true, OC.PERMISSION_ALL] @@ -667,21 +667,6 @@ describe('OC.Files.Client tests', function() { return testPermission.apply(this, testCase); }); }); - it('properly parses folder permissions', function() { - var testCases = [ - ['', false, OC.PERMISSION_READ], - ['C', false, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE], - ['K', false, OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE], - ['W', false, OC.PERMISSION_READ | OC.PERMISSION_UPDATE], - ['D', false, OC.PERMISSION_READ | OC.PERMISSION_DELETE], - ['R', false, OC.PERMISSION_READ | OC.PERMISSION_SHARE], - ['CKWDR', false, OC.PERMISSION_ALL] - ]; - - _.each(testCases, function(testCase) { - return testPermission.apply(this, testCase); - }); - }); it('properly parses mount types', function() { var testCases = [ ['CKWDR', false, null], From 3e844d3a59cc571c93b6edcd80dce2cffd89072b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 2 Nov 2017 17:08:00 +0100 Subject: [PATCH 6/6] Set read permission for files based on the data returned by the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the permissions returned by the server specify whether a file is readable or not the frontend no longer needs to assume that every file is readable. Signed-off-by: Daniel Calviño Sánchez --- core/js/files/client.js | 5 ++++- core/js/tests/specs/files/clientSpec.js | 27 +++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/core/js/files/client.js b/core/js/files/client.js index dc9f6ade64178..14d18ecf80cda 100644 --- a/core/js/files/client.js +++ b/core/js/files/client.js @@ -319,7 +319,7 @@ } } - data.permissions = OC.PERMISSION_READ; + data.permissions = OC.PERMISSION_NONE; var permissionProp = props[Client.PROPERTY_PERMISSIONS]; if (!_.isUndefined(permissionProp)) { var permString = permissionProp || ''; @@ -332,6 +332,9 @@ case 'K': data.permissions |= OC.PERMISSION_CREATE; break; + case 'G': + data.permissions |= OC.PERMISSION_READ; + break; case 'W': case 'N': case 'V': diff --git a/core/js/tests/specs/files/clientSpec.js b/core/js/tests/specs/files/clientSpec.js index 126ba3f4e6545..4ab7bbfabf64c 100644 --- a/core/js/tests/specs/files/clientSpec.js +++ b/core/js/tests/specs/files/clientSpec.js @@ -164,7 +164,7 @@ describe('OC.Files.Client tests', function() { 'd:resourcetype': '', 'oc:id': '00000011oc2d13a6a068', 'oc:fileid': '11', - 'oc:permissions': 'RDNVCK', + 'oc:permissions': 'GRDNVCK', 'oc:size': '120' }, [ @@ -196,7 +196,7 @@ describe('OC.Files.Client tests', function() { 'd:resourcetype': '', 'oc:id': '00000015oc2d13a6a068', 'oc:fileid': '15', - 'oc:permissions': 'RDNVCK', + 'oc:permissions': 'GRDNVCK', 'oc:size': '100' }, [ @@ -257,7 +257,7 @@ describe('OC.Files.Client tests', function() { expect(info.id).toEqual(51); expect(info.path).toEqual('/path/to space/文件夹'); expect(info.name).toEqual('One.txt'); - expect(info.permissions).toEqual(27); + expect(info.permissions).toEqual(26); expect(info.size).toEqual(250); expect(info.mtime).toEqual(1436535485000); expect(info.mimetype).toEqual('text/plain'); @@ -482,7 +482,7 @@ describe('OC.Files.Client tests', function() { 'd:resourcetype': '', 'oc:id': '00000011oc2d13a6a068', 'oc:fileid': '11', - 'oc:permissions': 'RDNVCK', + 'oc:permissions': 'GRDNVCK', 'oc:size': '120' }, [ @@ -549,7 +549,7 @@ describe('OC.Files.Client tests', function() { 'd:resourcetype': '', 'oc:id': '00000011oc2d13a6a068', 'oc:fileid': '11', - 'oc:permissions': 'RDNVCK', + 'oc:permissions': 'GRDNVCK', 'oc:size': '120' }, [ @@ -655,13 +655,14 @@ describe('OC.Files.Client tests', function() { it('properly parses file permissions', function() { // permission, isFile, expectedPermissions var testCases = [ - ['', true, OC.PERMISSION_READ], - ['C', true, OC.PERMISSION_READ | OC.PERMISSION_CREATE], - ['K', true, OC.PERMISSION_READ | OC.PERMISSION_CREATE], - ['W', true, OC.PERMISSION_READ | OC.PERMISSION_UPDATE], - ['D', true, OC.PERMISSION_READ | OC.PERMISSION_DELETE], - ['R', true, OC.PERMISSION_READ | OC.PERMISSION_SHARE], - ['CKWDR', true, OC.PERMISSION_ALL] + ['', true, OC.PERMISSION_NONE], + ['C', true, OC.PERMISSION_CREATE], + ['K', true, OC.PERMISSION_CREATE], + ['G', true, OC.PERMISSION_READ], + ['W', true, OC.PERMISSION_UPDATE], + ['D', true, OC.PERMISSION_DELETE], + ['R', true, OC.PERMISSION_SHARE], + ['CKGWDR', true, OC.PERMISSION_ALL] ]; _.each(testCases, function(testCase) { return testPermission.apply(this, testCase); @@ -669,7 +670,7 @@ describe('OC.Files.Client tests', function() { }); it('properly parses mount types', function() { var testCases = [ - ['CKWDR', false, null], + ['CKGWDR', false, null], ['M', false, 'external'], ['S', false, 'shared'], ['SM', false, 'shared']