From b4aabe983201abfc1b3f7509279bf489a892c4fd Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 17 Aug 2022 13:23:53 +0200 Subject: [PATCH 1/3] Hide share actions for space viewers/editors --- .../unreleased/bugfix-space-share-actions | 6 ++ .../components/SideBar/Shares/FileShares.vue | 17 +++- packages/web-app-files/src/store/actions.ts | 91 +++++++++++-------- 3 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 changelog/unreleased/bugfix-space-share-actions diff --git a/changelog/unreleased/bugfix-space-share-actions b/changelog/unreleased/bugfix-space-share-actions new file mode 100644 index 00000000000..08c654c9249 --- /dev/null +++ b/changelog/unreleased/bugfix-space-share-actions @@ -0,0 +1,6 @@ +Bugfix: Hide share actions for space viewers/editors + +We've fixed a bug where viewers and editors of a space could see the actions to edit and remove shares. We've also improved the error handling when something goes wrong while editing/removing shares. + +https://github.com/owncloud/web/issues/7436 +https://github.com/owncloud/web/pull/7470 diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue index 1eea6ba11c9..5d12dc04f3a 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue @@ -28,7 +28,7 @@
  • @@ -103,7 +103,9 @@ export default { watch( currentStorageId, (storageId) => { - currentSpace.value = store.state.Files.spaces?.find((space) => space.id === storageId) + currentSpace.value = store.state.runtime.spaces?.spaces?.find( + (space) => space.id === storageId + ) }, { immediate: true } ) @@ -418,6 +420,17 @@ export default { } return null + }, + + shareIsModifiable(collaborator) { + if ( + this.currentUserIsMemberOfSpace && + !this.currentSpace?.spaceRoles.manager.includes(this.user.uuid) + ) { + return false + } + + return !collaborator.indirect } } } diff --git a/packages/web-app-files/src/store/actions.ts b/packages/web-app-files/src/store/actions.ts index 7457b847106..ce6fd1e5b0a 100644 --- a/packages/web-app-files/src/store/actions.ts +++ b/packages/web-app-files/src/store/actions.ts @@ -350,7 +350,7 @@ export default { }) }, async changeShare( - { commit, getters, rootGetters }, + { commit, dispatch, getters, rootGetters }, { client, graphClient, share, permissions, expirationDate, role } ) { if (!permissions && !role) { @@ -358,10 +358,18 @@ export default { } if (share.shareType === ShareTypes.space.value) { - await client.shares.shareSpaceWithUser('', share.collaborator.name, share.id, { - permissions, - role: role.name - }) + try { + await client.shares.shareSpaceWithUser('', share.collaborator.name, share.id, { + permissions, + role: role.name + }) + } catch (error) { + dispatch( + 'showMessage', + { title: $gettext('Error while editing the share.'), status: 'danger' }, + { root: true } + ) + } const spaceShare = buildSpaceShare( { @@ -385,20 +393,28 @@ export default { return } - const updatedShare = await client.shares.updateShare(share.id, { - role: role.name, - permissions, - expireDate: expirationDate - }) + try { + const updatedShare = await client.shares.updateShare(share.id, { + role: role.name, + permissions, + expireDate: expirationDate + }) - commit( - 'CURRENT_FILE_OUTGOING_SHARES_UPSERT', - buildCollaboratorShare( - updatedShare.shareInfo, - getters.highlightedFile, - allowSharePermissions(rootGetters) + commit( + 'CURRENT_FILE_OUTGOING_SHARES_UPSERT', + buildCollaboratorShare( + updatedShare.shareInfo, + getters.highlightedFile, + allowSharePermissions(rootGetters) + ) ) - ) + } catch (error) { + dispatch( + 'showMessage', + { title: $gettext('Error while editing the share.'), status: 'danger' }, + { root: true } + ) + } }, addShare( context, @@ -531,31 +547,26 @@ export default { additionalParams.shareWith = share.collaborator.name } - client.shares - .deleteShare(share.id, additionalParams) - .then(() => { - context.commit('CURRENT_FILE_OUTGOING_SHARES_REMOVE', share) + return client.shares.deleteShare(share.id, additionalParams).then(() => { + context.commit('CURRENT_FILE_OUTGOING_SHARES_REMOVE', share) - if (share.shareType !== ShareTypes.space.value) { - context.dispatch('updateCurrentFileShareTypes') - context.dispatch('loadIndicators', { client, currentFolder: path, storageId }) - } else { - context.commit('CURRENT_FILE_OUTGOING_SHARES_LOADING', true) - - return graphClient.drives.getDrive(share.id).then((response) => { - const space = buildSpace(response.data) - context.commit('UPDATE_RESOURCE_FIELD', { - id: share.id, - field: 'spaceRoles', - value: space.spaceRoles - }) - context.commit('CURRENT_FILE_OUTGOING_SHARES_LOADING', false) + if (share.shareType !== ShareTypes.space.value) { + context.dispatch('updateCurrentFileShareTypes') + context.dispatch('loadIndicators', { client, currentFolder: path, storageId }) + } else { + context.commit('CURRENT_FILE_OUTGOING_SHARES_LOADING', true) + + return graphClient.drives.getDrive(share.id).then((response) => { + const space = buildSpace(response.data) + context.commit('UPDATE_RESOURCE_FIELD', { + id: share.id, + field: 'spaceRoles', + value: space.spaceRoles }) - } - }) - .catch((e) => { - console.error(e) - }) + context.commit('CURRENT_FILE_OUTGOING_SHARES_LOADING', false) + }) + } + }) }, /** * Prune all branches of the shares tree that are From 45a0591b4a10821b2487d086807f7434537e03a3 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 17 Aug 2022 13:35:38 +0200 Subject: [PATCH 2/3] Fix unit tests --- .../unit/components/SideBar/Shares/FileShares.spec.js | 8 ++++++-- packages/web-app-files/tests/unit/store/actions.spec.js | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js b/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js index f4ba4915298..fb8c587ae79 100644 --- a/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js +++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js @@ -198,8 +198,7 @@ const storeOptions = (data) => { Files: { state: { incomingShares: incomingCollaborators, - sharesTree: { [Collaborators[0].path]: [Collaborators[0]] }, - spaces + sharesTree: { [Collaborators[0].path]: [Collaborators[0]] } }, namespaced: true, getters: { @@ -225,6 +224,11 @@ const storeOptions = (data) => { }, runtime: { namespaced: true, + state: { + spaces: { + spaces + } + }, modules: { auth: { namespaced: true, diff --git a/packages/web-app-files/tests/unit/store/actions.spec.js b/packages/web-app-files/tests/unit/store/actions.spec.js index b1ce5216436..422b8e9a512 100644 --- a/packages/web-app-files/tests/unit/store/actions.spec.js +++ b/packages/web-app-files/tests/unit/store/actions.spec.js @@ -140,7 +140,7 @@ describe('vuex store actions', () => { describe('deleteShare', () => { it.each([ - { share: spaceShareMock, storageId: spaceMock.id, expectedCommitCalls: 2 }, + { share: spaceShareMock, storageId: spaceMock.id, expectedCommitCalls: 4 }, { share: shareMock, storageId: null, expectedCommitCalls: 1 } ])('succeeds using action %s', async (dataSet) => { const commitSpy = jest.spyOn(stateMock, 'commit') From e056f5671f7ac6bcac26c7c3a0006537df933df9 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 18 Aug 2022 12:22:20 +0200 Subject: [PATCH 3/3] Rename method to isShareModifiable --- .../src/components/SideBar/Shares/FileShares.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue index 5d12dc04f3a..21e69da3fb9 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue @@ -28,7 +28,7 @@
  • @@ -422,7 +422,7 @@ export default { return null }, - shareIsModifiable(collaborator) { + isShareModifiable(collaborator) { if ( this.currentUserIsMemberOfSpace && !this.currentSpace?.spaceRoles.manager.includes(this.user.uuid)