From 68c44bb6427632e237792bd75d874be4b4562f3f Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Mon, 29 Oct 2018 10:03:52 +0100 Subject: [PATCH 01/10] shares are displayed to users with resharing rights Signed-off-by: Maxence Lange --- .../lib/Controller/ShareAPIController.php | 38 +++++++++++++++++++ lib/private/Share20/DefaultShareProvider.php | 22 +++++------ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 61fad5d2b1..fc03a357f3 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -720,14 +720,23 @@ class ShareAPIController extends OCSController { } $formatted = []; + $resharingRight = false; foreach ($shares as $share) { try { $formatted[] = $this->formatShare($share, $path); + if (!$resharingRight && $this->shareProviderResharingRights($this->currentUser, $share)) { + $resharingRight = true; + } + } catch (NotFoundException $e) { //Ignore share } } + if (!$resharingRight) { + $formatted = []; + } + if ($include_tags) { $formatted = Helper::populateTags($formatted, 'file_source', \OC::$server->getTagManager()); } @@ -1102,4 +1111,33 @@ class ShareAPIController extends OCSController { return $this->serverContainer->query('\OCA\Spreed\Share\Helper\ShareAPIController'); } + + + /** + * Returns if we can find resharing rights in an IShare object for a specific user. + * + * @param string $userId + * @param IShare $share + * @return bool + */ + private function shareProviderResharingRights(string $userId, IShare $share): bool { + if ($share->getShareOwner() === $userId) { + return true; + } + + if ((\OCP\Constants::PERMISSION_SHARE & $share->getPermissions()) === 0) { + return false; + } + + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() === $userId) { + return true; + } + + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && $this->groupManager->isInGroup($userId, $share->getSharedWith())) { + return true; + } + + return false; + } + } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 5011105454..53fd1728b8 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -617,18 +617,18 @@ class DefaultShareProvider implements IShareProvider { /** * Reshares for this user are shares where they are the owner. */ - if ($reshares === false) { - $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); + if ($node === null) { + if ($reshares === false) { + $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); + } else { + $qb->andWhere( + $qb->expr()->orX( + $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), + $qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)) + ) + ); + } } else { - $qb->andWhere( - $qb->expr()->orX( - $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), - $qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)) - ) - ); - } - - if ($node !== null) { $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); } From 72ad2d60b576f182d152735e749aa7e27ff05919 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Tue, 30 Oct 2018 09:58:43 +0100 Subject: [PATCH 02/10] display shares to circles moderator Signed-off-by: Maxence Lange --- .../lib/Controller/ShareAPIController.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index fc03a357f3..0e53863f9b 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -240,6 +240,9 @@ class ShareAPIController extends OCSController { $shareWithStart = ($hasCircleId? strrpos($share->getSharedWith(), '[') + 1: 0); $shareWithLength = ($hasCircleId? -1: strpos($share->getSharedWith(), ' ')); + if (is_bool($shareWithLength)) { + $shareWithLength = -1; + } $result['share_with'] = substr($share->getSharedWith(), $shareWithStart, $shareWithLength); } else if ($share->getShareType() === Share::SHARE_TYPE_ROOM) { $result['share_with'] = $share->getSharedWith(); @@ -1137,6 +1140,25 @@ class ShareAPIController extends OCSController { return true; } + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_CIRCLE && \OC::$server->getAppManager()->isEnabledForUser('circles') && + class_exists('\OCA\Circles\Api\v1\Circles')) { + $hasCircleId = (substr($share->getSharedWith(), -1) === ']'); + $shareWithStart = ($hasCircleId ? strrpos($share->getSharedWith(), '[') + 1 : 0); + $shareWithLength = ($hasCircleId ? -1 : strpos($share->getSharedWith(), ' ')); + if (is_bool($shareWithLength)) { + $shareWithLength = -1; + } + $sharedWith = substr($share->getSharedWith(), $shareWithStart, $shareWithLength); + try { + $member = \OCA\Circles\Api\v1\Circles::getMember($sharedWith, $userId, 1); + if ($member->getLevel() > 0) { + return true; + } + } catch (QueryException $e) { + return false; + } + } + return false; } From 275cea5d9ceee3236b28df8bda0ba520ffe38db5 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Tue, 30 Oct 2018 10:02:38 +0100 Subject: [PATCH 03/10] limit to circles moderator Signed-off-by: Maxence Lange --- apps/files_sharing/lib/Controller/ShareAPIController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 0e53863f9b..8648113133 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -1151,9 +1151,10 @@ class ShareAPIController extends OCSController { $sharedWith = substr($share->getSharedWith(), $shareWithStart, $shareWithLength); try { $member = \OCA\Circles\Api\v1\Circles::getMember($sharedWith, $userId, 1); - if ($member->getLevel() > 0) { + if ($member->getLevel() >= 4) { return true; } + return false; } catch (QueryException $e) { return false; } From b73e03724c402216ccadb0dab1973f1ddaa14494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 30 Oct 2018 14:22:33 +0100 Subject: [PATCH 04/10] Show share settings for owner and reshare owners only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../sharedialogshareelistview.handlebars | 2 ++ core/js/sharedialogshareelistview.js | 5 +++++ core/js/shareitemmodel.js | 13 ++++++++++++ core/js/sharetemplates.js | 20 ++++++++++++------- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/core/js/share/sharedialogshareelistview.handlebars b/core/js/share/sharedialogshareelistview.handlebars index 92c07f8029..672888b5ad 100644 --- a/core/js/share/sharedialogshareelistview.handlebars +++ b/core/js/share/sharedialogshareelistview.handlebars @@ -3,6 +3,7 @@
  • {{shareWithDisplayName}} + {{#if canUpdateShareSettings }} {{#if editPermissionPossible}} @@ -14,6 +15,7 @@ {{{popoverMenu}}} + {{/if}}
  • {{/each}} {{#each linkReshares}} diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 8b709f0d74..dc97099dd5 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -86,6 +86,7 @@ var shareType = this.model.getShareType(shareIndex); var sharedBy = this.model.getSharedBy(shareIndex); var sharedByDisplayName = this.model.getSharedByDisplayName(shareIndex); + var fileOwnerUid = this.model.getFileOwnerUid(shareIndex); var hasPermissionOverride = {}; if (shareType === OC.Share.SHARE_TYPE_GROUP) { @@ -143,6 +144,8 @@ hasCreatePermission: this.model.hasCreatePermission(shareIndex), hasUpdatePermission: this.model.hasUpdatePermission(shareIndex), hasDeletePermission: this.model.hasDeletePermission(shareIndex), + sharedBy: sharedBy, + sharedByDisplayName: sharedByDisplayName, shareWith: shareWith, shareWithDisplayName: shareWithDisplayName, shareWithAvatar: shareWithAvatar, @@ -150,6 +153,8 @@ shareType: shareType, shareId: this.model.get('shares')[shareIndex].id, modSeed: shareWithAvatar || (shareType !== OC.Share.SHARE_TYPE_USER && shareType !== OC.Share.SHARE_TYPE_CIRCLE && shareType !== OC.Share.SHARE_TYPE_ROOM), + owner: fileOwnerUid, + canUpdateShareSettings: (sharedBy === oc_current_user || fileOwnerUid === oc_current_user), isRemoteShare: shareType === OC.Share.SHARE_TYPE_REMOTE, isRemoteGroupShare: shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP, isNoteAvailable: shareType !== OC.Share.SHARE_TYPE_REMOTE && shareType !== OC.Share.SHARE_TYPE_REMOTE_GROUP, diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 1bbdb2448a..f4f8c02370 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -454,6 +454,19 @@ return share.displayname_owner; }, + /** + * @param shareIndex + * @returns {string} + */ + getFileOwnerUid: function(shareIndex) { + /** @type OC.Share.Types.ShareInfo **/ + var share = this.get('shares')[shareIndex]; + if(!_.isObject(share)) { + throw "Unknown Share"; + } + return share.uid_file_owner; + }, + /** * returns the array index of a sharee for a provided shareId * diff --git a/core/js/sharetemplates.js b/core/js/sharetemplates.js index bd9886e6af..6ddf05d328 100644 --- a/core/js/sharetemplates.js +++ b/core/js/sharetemplates.js @@ -300,11 +300,9 @@ templates['sharedialogshareelistview'] = template({"1":function(container,depth0 + alias4(((helper = (helper = helpers.shareWithTitle || (depth0 != null ? depth0.shareWithTitle : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"shareWithTitle","hash":{},"data":data}) : helper))) + "\">" + alias4(((helper = (helper = helpers.shareWithDisplayName || (depth0 != null ? depth0.shareWithDisplayName : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"shareWithDisplayName","hash":{},"data":data}) : helper))) - + "\n \n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.editPermissionPossible : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + "
    \n " - + ((stack1 = ((helper = (helper = helpers.popoverMenu || (depth0 != null ? depth0.popoverMenu : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"popoverMenu","hash":{},"data":data}) : helper))) != null ? stack1 : "") - + "\n
    \n
    \n \n"; + + "\n" + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canUpdateShareSettings : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + " \n"; },"2":function(container,depth0,helpers,partials,data) { return "imageplaceholderseed"; },"4":function(container,depth0,helpers,partials,data) { @@ -316,6 +314,14 @@ templates['sharedialogshareelistview'] = template({"1":function(container,depth0 + alias4(((helper = (helper = helpers.shareType || (depth0 != null ? depth0.shareType : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"shareType","hash":{},"data":data}) : helper))) + "\""; },"6":function(container,depth0,helpers,partials,data) { + var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); + + return " \n" + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.editPermissionPossible : depth0),{"name":"if","hash":{},"fn":container.program(7, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + "
    \n " + + ((stack1 = ((helper = (helper = helpers.popoverMenu || (depth0 != null ? depth0.popoverMenu : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(alias1,{"name":"popoverMenu","hash":{},"data":data}) : helper))) != null ? stack1 : "") + + "\n
    \n
    \n"; +},"7":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return " \n " + alias4(((helper = (helper = helpers.canEditLabel || (depth0 != null ? depth0.canEditLabel : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"canEditLabel","hash":{},"data":data}) : helper))) + "\n \n"; -},"8":function(container,depth0,helpers,partials,data) { +},"9":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return "
  • \n" + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.sharees : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.linkReshares : depth0),{"name":"each","hash":{},"fn":container.program(8, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.linkReshares : depth0),{"name":"each","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "\n"; },"useData":true}); templates['sharedialogshareelistview_popover_menu'] = template({"1":function(container,depth0,helpers,partials,data) { From 59377dbd508c4562994719924d9b6f35885c1642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 30 Oct 2018 14:23:00 +0100 Subject: [PATCH 05/10] Remove reshare owner entry from the share list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/js/sharedialogshareelistview.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index dc97099dd5..140175bc06 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -371,6 +371,9 @@ if(_.isArray(sharees)) { for (var i = 0; i < sharees.length; i++) { data.sharees[i].popoverMenu = this.popoverMenuTemplate(sharees[i]); + if (data.sharees[i].shareType === OC.Share.SHARE_TYPE_USER && data.sharees[i].shareWith === OC.getCurrentUser().uid) { + data.sharees.splice(i, 1); + } } } return OC.Share.Templates['sharedialogshareelistview'](data); From 2804f3a7619ce6ed5cde2c7c4a6ea9b991f56726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 31 Oct 2018 12:11:34 +0100 Subject: [PATCH 06/10] Add uid_owner to enable permission settings on the views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- core/js/tests/specs/sharedialogshareelistview.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index 4f84fa0e08..ff353404bc 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -129,6 +129,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); @@ -144,6 +145,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user _.@-\'', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); @@ -159,6 +161,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); @@ -174,6 +177,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user _.@-\'', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); @@ -189,7 +193,8 @@ describe('OC.Share.ShareDialogShareeListView', function () { permissions: 1, share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', - share_with_displayname: 'User One' + share_with_displayname: 'User One', + uid_owner: oc_current_user, }]); shareModel.set('itemType', 'folder'); listView.render(); @@ -206,6 +211,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); @@ -223,6 +229,7 @@ describe('OC.Share.ShareDialogShareeListView', function () { share_type: OC.Share.SHARE_TYPE_USER, share_with: 'user1', share_with_displayname: 'User One', + uid_owner: oc_current_user, itemType: 'folder' }]); shareModel.set('itemType', 'folder'); From 2a17811caf4710bdacca2639be934f8a90c8dc3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 31 Oct 2018 12:59:32 +0100 Subject: [PATCH 07/10] Use property to check if a share entry is shared with the current user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../sharedialogshareelistview.handlebars | 2 ++ core/js/sharedialogshareelistview.js | 4 +--- core/js/sharetemplates.js | 24 +++++++++++-------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/core/js/share/sharedialogshareelistview.handlebars b/core/js/share/sharedialogshareelistview.handlebars index 672888b5ad..18ff219c12 100644 --- a/core/js/share/sharedialogshareelistview.handlebars +++ b/core/js/share/sharedialogshareelistview.handlebars @@ -1,5 +1,6 @@
      {{#each sharees}} + {{#unless isShareWithCurrentUser}}
    • {{shareWithDisplayName}} @@ -17,6 +18,7 @@ {{/if}}
    • + {{/unless}} {{/each}} {{#each linkReshares}}
    • diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 140175bc06..81326cdb6c 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -154,6 +154,7 @@ shareId: this.model.get('shares')[shareIndex].id, modSeed: shareWithAvatar || (shareType !== OC.Share.SHARE_TYPE_USER && shareType !== OC.Share.SHARE_TYPE_CIRCLE && shareType !== OC.Share.SHARE_TYPE_ROOM), owner: fileOwnerUid, + isShareWithCurrentUser: (shareType === OC.Share.SHARE_TYPE_USER && shareWith === oc_current_user), canUpdateShareSettings: (sharedBy === oc_current_user || fileOwnerUid === oc_current_user), isRemoteShare: shareType === OC.Share.SHARE_TYPE_REMOTE, isRemoteGroupShare: shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP, @@ -371,9 +372,6 @@ if(_.isArray(sharees)) { for (var i = 0; i < sharees.length; i++) { data.sharees[i].popoverMenu = this.popoverMenuTemplate(sharees[i]); - if (data.sharees[i].shareType === OC.Share.SHARE_TYPE_USER && data.sharees[i].shareWith === OC.getCurrentUser().uid) { - data.sharees.splice(i, 1); - } } } return OC.Share.Templates['sharedialogshareelistview'](data); diff --git a/core/js/sharetemplates.js b/core/js/sharetemplates.js index 6ddf05d328..edf7394196 100644 --- a/core/js/sharetemplates.js +++ b/core/js/sharetemplates.js @@ -278,6 +278,10 @@ templates['sharedialogresharerinfoview'] = template({"1":function(container,dept + "\n"; },"useData":true}); templates['sharedialogshareelistview'] = template({"1":function(container,depth0,helpers,partials,data) { + var stack1; + + return ((stack1 = helpers.unless.call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.isShareWithCurrentUser : depth0),{"name":"unless","hash":{},"fn":container.program(2, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : ""); +},"2":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return "
    • \n
      \n " + alias4(((helper = (helper = helpers.shareWithDisplayName || (depth0 != null ? depth0.shareWithDisplayName : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"shareWithDisplayName","hash":{},"data":data}) : helper))) + "\n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canUpdateShareSettings : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canUpdateShareSettings : depth0),{"name":"if","hash":{},"fn":container.program(7, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    • \n"; -},"2":function(container,depth0,helpers,partials,data) { +},"3":function(container,depth0,helpers,partials,data) { return "imageplaceholderseed"; -},"4":function(container,depth0,helpers,partials,data) { +},"5":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return "data-seed=\"" @@ -313,15 +317,15 @@ templates['sharedialogshareelistview'] = template({"1":function(container,depth0 + " " + alias4(((helper = (helper = helpers.shareType || (depth0 != null ? depth0.shareType : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"shareType","hash":{},"data":data}) : helper))) + "\""; -},"6":function(container,depth0,helpers,partials,data) { +},"7":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return " \n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.editPermissionPossible : depth0),{"name":"if","hash":{},"fn":container.program(7, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.editPermissionPossible : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
      \n " + ((stack1 = ((helper = (helper = helpers.popoverMenu || (depth0 != null ? depth0.popoverMenu : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(alias1,{"name":"popoverMenu","hash":{},"data":data}) : helper))) != null ? stack1 : "") + "\n
      \n
      \n"; -},"7":function(container,depth0,helpers,partials,data) { +},"8":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return " \n " + alias4(((helper = (helper = helpers.canEditLabel || (depth0 != null ? depth0.canEditLabel : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"canEditLabel","hash":{},"data":data}) : helper))) + "\n \n"; -},"9":function(container,depth0,helpers,partials,data) { +},"10":function(container,depth0,helpers,partials,data) { var helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression; return "
    • \n" + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.sharees : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.linkReshares : depth0),{"name":"each","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers.each.call(alias1,(depth0 != null ? depth0.linkReshares : depth0),{"name":"each","hash":{},"fn":container.program(10, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n"; },"useData":true}); templates['sharedialogshareelistview_popover_menu'] = template({"1":function(container,depth0,helpers,partials,data) { From 236a293f6a8b983ee832151c592a4e469ed0621e Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 1 Nov 2018 13:41:19 +0100 Subject: [PATCH 08/10] check parents resharing rights Signed-off-by: Maxence Lange --- .../lib/Controller/ShareAPIController.php | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 8648113133..b5c833a6f9 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -39,7 +39,6 @@ use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\AppFramework\QueryException; use OCP\Constants; -use OCP\Files\Folder; use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\IConfig; @@ -727,11 +726,10 @@ class ShareAPIController extends OCSController { foreach ($shares as $share) { try { $formatted[] = $this->formatShare($share, $path); - if (!$resharingRight && $this->shareProviderResharingRights($this->currentUser, $share)) { + if ($path !== null && !$resharingRight && $this->shareProviderResharingRights($this->currentUser, $share, $path)) { $resharingRight = true; } - - } catch (NotFoundException $e) { + } catch (\Exception $e) { //Ignore share } } @@ -1119,15 +1117,25 @@ class ShareAPIController extends OCSController { /** * Returns if we can find resharing rights in an IShare object for a specific user. * + * @suppress PhanUndeclaredClassMethod + * * @param string $userId * @param IShare $share + * @param Node $node * @return bool + * @throws NotFoundException + * @throws \OCP\Files\InvalidPathException */ - private function shareProviderResharingRights(string $userId, IShare $share): bool { + private function shareProviderResharingRights(string $userId, IShare $share, Node $node): bool { if ($share->getShareOwner() === $userId) { return true; } + // we check that current user have parent resharing rights on the current file + if (($node->getPermissions() & \OCP\Constants::PERMISSION_SHARE) !== 0) { + return true; + } + if ((\OCP\Constants::PERMISSION_SHARE & $share->getPermissions()) === 0) { return false; } @@ -1141,7 +1149,7 @@ class ShareAPIController extends OCSController { } if ($share->getShareType() === \OCP\Share::SHARE_TYPE_CIRCLE && \OC::$server->getAppManager()->isEnabledForUser('circles') && - class_exists('\OCA\Circles\Api\v1\Circles')) { + class_exists('\OCA\Circles\Api\v1\Circles')) { $hasCircleId = (substr($share->getSharedWith(), -1) === ']'); $shareWithStart = ($hasCircleId ? strrpos($share->getSharedWith(), '[') + 1 : 0); $shareWithLength = ($hasCircleId ? -1 : strpos($share->getSharedWith(), ' ')); From 0fc8a0f58eebc9bdac5544c114517f397838b38e Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 1 Nov 2018 15:01:01 +0100 Subject: [PATCH 09/10] user can have his resharing rights revoked, yet seeing created shares Signed-off-by: Maxence Lange --- .../lib/Controller/ShareAPIController.php | 19 +++++++++++++------ apps/files_sharing/tests/ApiTest.php | 10 ++++++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index b5c833a6f9..04c72b459b 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -721,12 +721,18 @@ class ShareAPIController extends OCSController { $shares = array_merge($shares, $federatedShares); } - $formatted = []; + $formatted = $miniFormatted = []; $resharingRight = false; foreach ($shares as $share) { + /** @var IShare $share */ try { - $formatted[] = $this->formatShare($share, $path); - if ($path !== null && !$resharingRight && $this->shareProviderResharingRights($this->currentUser, $share, $path)) { + $format = $this->formatShare($share, $path); + $formatted[] = $format; + if ($share->getSharedBy() === $this->currentUser) { + $miniFormatted[] = $format; + } + + if (!$resharingRight && $this->shareProviderResharingRights($this->currentUser, $share, $path)) { $resharingRight = true; } } catch (\Exception $e) { @@ -735,7 +741,7 @@ class ShareAPIController extends OCSController { } if (!$resharingRight) { - $formatted = []; + $formatted = $miniFormatted; } if ($include_tags) { @@ -1126,13 +1132,14 @@ class ShareAPIController extends OCSController { * @throws NotFoundException * @throws \OCP\Files\InvalidPathException */ - private function shareProviderResharingRights(string $userId, IShare $share, Node $node): bool { + private function shareProviderResharingRights(string $userId, IShare $share, $node): bool { + if ($share->getShareOwner() === $userId) { return true; } // we check that current user have parent resharing rights on the current file - if (($node->getPermissions() & \OCP\Constants::PERMISSION_SHARE) !== 0) { + if ($node !== null && ($node->getPermissions() & \OCP\Constants::PERMISSION_SHARE) !== 0) { return true; } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 0616daed62..e3d0b2dbcd 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -811,9 +811,10 @@ class ApiTest extends TestCase { $result1 = $ocs->getShares('false','false','false', $this->subfolder); $ocs->cleanup(); - // test should return one share within $this->folder +// // test should return 2 shares within $this->folder, as the viewer have resharing rights: +// // one from the owner, the second from the reshare $data1 = $result1->getData(); - $this->assertCount(1, $data1); + $this->assertCount(2, $data1); $s1 = reset($data1); //$request = $this->createRequest(['path' => $this->folder.$this->subfolder]); @@ -821,9 +822,10 @@ class ApiTest extends TestCase { $result2 = $ocs->getShares('false', 'false', 'false', $this->folder . $this->subfolder); $ocs->cleanup(); - // test should return one share within $this->folder +// // test should return 2 shares within $this->folder, as the viewer have resharing rights: +// // one from the owner, the second from the reshare $data2 = $result2->getData(); - $this->assertCount(1, $data2); + $this->assertCount(2, $data2); $s2 = reset($data2); $this->assertEquals($this->subfolder, $s1['path']); From 77b95ccd12bb946cba96486d859b8241649868ca Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 1 Nov 2018 16:46:38 +0100 Subject: [PATCH 10/10] revert tests Signed-off-by: Maxence Lange --- apps/files_sharing/tests/ApiTest.php | 10 ++++------ lib/private/Share20/DefaultShareProvider.php | 12 +++++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index e3d0b2dbcd..0616daed62 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -811,10 +811,9 @@ class ApiTest extends TestCase { $result1 = $ocs->getShares('false','false','false', $this->subfolder); $ocs->cleanup(); -// // test should return 2 shares within $this->folder, as the viewer have resharing rights: -// // one from the owner, the second from the reshare + // test should return one share within $this->folder $data1 = $result1->getData(); - $this->assertCount(2, $data1); + $this->assertCount(1, $data1); $s1 = reset($data1); //$request = $this->createRequest(['path' => $this->folder.$this->subfolder]); @@ -822,10 +821,9 @@ class ApiTest extends TestCase { $result2 = $ocs->getShares('false', 'false', 'false', $this->folder . $this->subfolder); $ocs->cleanup(); -// // test should return 2 shares within $this->folder, as the viewer have resharing rights: -// // one from the owner, the second from the reshare + // test should return one share within $this->folder $data2 = $result2->getData(); - $this->assertCount(2, $data2); + $this->assertCount(1, $data2); $s2 = reset($data2); $this->assertEquals($this->subfolder, $s1['path']); diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 53fd1728b8..3bdbc69e89 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -617,10 +617,10 @@ class DefaultShareProvider implements IShareProvider { /** * Reshares for this user are shares where they are the owner. */ - if ($node === null) { - if ($reshares === false) { - $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); - } else { + if ($reshares === false) { + $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); + } else { + if ($node === null) { $qb->andWhere( $qb->expr()->orX( $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), @@ -628,7 +628,9 @@ class DefaultShareProvider implements IShareProvider { ) ); } - } else { + } + + if ($node !== null) { $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId()))); }