Hide share actions for space viewers/editors

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

packages/web-app-files/src/components/SideBar/Shares/FileShares.vue

@@ -28,7 +28,7 @@
         <li v-for="collaborator in displayCollaborators" :key="collaborator.key">
           <collaborator-list-item
             :share="collaborator"
-            :modifiable="!collaborator.indirect"
+            :modifiable="isShareModifiable(collaborator)"
             :shared-parent-route="getSharedParentRoute(collaborator)"
             @onDelete="$_ocCollaborators_deleteShare_trigger"
           />
@@ -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
+    },
+
+    isShareModifiable(collaborator) {
+      if (
+        this.currentUserIsMemberOfSpace &&
+        !this.currentSpace?.spaceRoles.manager.includes(this.user.uuid)
+      ) {
+        return false
+      }
+
+      return !collaborator.indirect
     }
   }
 }

packages/web-app-files/src/store/actions.ts

@@ -350,18 +350,26 @@ export default {
       })
   },
   async changeShare(
-    { commit, getters, rootGetters },
+    { commit, dispatch, getters, rootGetters },
     { client, graphClient, share, permissions, expirationDate, role }
   ) {
     if (!permissions && !role) {
       throw new Error('Nothing changed')
     }
 
     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

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,

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')