Merge pull request #11484 from owncloud/feat/no-versions-editor-role

feat: hide version panel with insufficient permissions

changelog/unreleased/enhancement-hide-versions-panel

@@ -0,0 +1,6 @@
+Enhancement: Hide versions panel with insufficient permissions
+
+Users that have insufficient permissions to view file versions don't see the versions sidebar panel anymore. This currently affects regular share receivers, space viewers and space editors without the permission to view versions.
+
+https://github.com/owncloud/web/pull/11484
+https://github.com/owncloud/web/issues/11359

packages/web-app-files/src/composables/extensions/useFileSideBars.ts

@@ -19,16 +19,11 @@ import {
   SidebarPanelExtension,
   useIsFilesAppActive,
   useGetMatchingSpace,
-  useUserStore,
   useCapabilityStore,
-  useCanListShares
+  useCanListShares,
+  useCanListVersions
 } from '@ownclouders/web-pkg'
-import {
-  isPersonalSpaceResource,
-  isProjectSpaceResource,
-  isSpaceResource,
-  SpaceResource
-} from '@ownclouders/web-client'
+import { isProjectSpaceResource, SpaceResource } from '@ownclouders/web-client'
 import { Resource } from '@ownclouders/web-client'
 import { useGettext } from 'vue3-gettext'
 import { unref } from 'vue'
@@ -43,7 +38,7 @@ export const useSideBarPanels = (): SidebarPanelExtension<SpaceResource, Resourc
   const isFilesAppActive = useIsFilesAppActive()
   const { isPersonalSpaceRoot } = useGetMatchingSpace()
   const { canListShares } = useCanListShares()
-  const userStore = useUserStore()
+  const { canListVersions } = useCanListVersions()
 
   return [
     {
@@ -251,23 +246,7 @@ export const useSideBarPanels = (): SidebarPanelExtension<SpaceResource, Resourc
           if (items?.length !== 1) {
             return false
           }
-          if (isProjectSpaceResource(items[0])) {
-            // project space roots don't support versions
-            return false
-          }
-
-          const userIsSpaceMember =
-            (isProjectSpaceResource(root) && root.isMember(userStore.user)) ||
-            (isPersonalSpaceResource(root) && root.isOwner(userStore.user))
-
-          if (
-            isLocationTrashActive(router, 'files-trash-generic') ||
-            !userIsSpaceMember ||
-            isSpaceResource(items[0])
-          ) {
-            return false
-          }
-          return items[0].type !== 'folder'
+          return canListVersions({ space: root, resource: items[0] })
         }
       }
     },

packages/web-client/src/helpers/share/types.ts

@@ -12,9 +12,11 @@ export enum GraphSharePermission {
   readChildren = 'libre.graph/driveItem/children/read',
   readDeleted = 'libre.graph/driveItem/deleted/read',
   readPermissions = 'libre.graph/driveItem/permissions/read',
+  readVersions = 'libre.graph/driveItem/versions/read',
   updatePath = 'libre.graph/driveItem/path/update',
   updateDeleted = 'libre.graph/driveItem/deleted/update',
   updatePermissions = 'libre.graph/driveItem/permissions/update',
+  updateVersions = 'libre.graph/driveItem/versions/update',
   deleteStandard = 'libre.graph/driveItem/standard/delete',
   deleteDeleted = 'libre.graph/driveItem/deleted/delete',
   deletePermissions = 'libre.graph/driveItem/permissions/delete'

packages/web-client/src/helpers/space/functions.ts

@@ -8,6 +8,7 @@ import {
 } from '../resource'
 import {
   isPersonalSpaceResource,
+  isPublicSpaceResource,
   PublicSpaceResource,
   ShareSpaceResource,
   SpaceMember,
@@ -296,6 +297,12 @@ export function buildSpace(
         GraphSharePermission.deletePermissions
       )
     },
+    canListVersions: function ({ user }: { user?: User } = {}) {
+      if (isPersonalSpaceResource(this) && this.isOwner(user)) {
+        return true
+      }
+      return getPermissionsForSpaceMember(this, user).includes(GraphSharePermission.readVersions)
+    },
     canCreate: function () {
       return true
     },
@@ -325,6 +332,9 @@ export function buildSpace(
       return urlJoin(webDavTrashUrl, path)
     },
     isMember(user: User): boolean {
+      if (isPublicSpaceResource(this)) {
+        return false
+      }
       if (this.isOwner(user) || !!this.members[user.id]) {
         return true
       }

packages/web-client/src/helpers/space/types.ts

@@ -42,6 +42,7 @@ export interface SpaceResource extends Resource {
   canRestore(args?: { user?: User; ability?: Ability }): boolean
   canDeleteFromTrashBin(args?: { user?: User }): boolean
   canRestoreFromTrashbin(args?: { user?: User }): boolean
+  canListVersions(args?: { user?: User }): boolean
 
   getWebDavUrl({ path }: { path: string }): string
   getWebDavTrashUrl({ path }: { path: string }): string

packages/web-pkg/src/components/SideBar/FileSideBar.vue

@@ -35,8 +35,7 @@ import FileInfo from './Files/FileInfo.vue'
 import {
   isLocationCommonActive,
   isLocationSharesActive,
-  isLocationSpacesActive,
-  isLocationTrashActive
+  isLocationSpacesActive
 } from '../../router'
 import {
   SidebarPanelExtension,
@@ -50,23 +49,22 @@ import {
   useSpacesStore,
   useSharesStore,
   useResourcesStore,
-  useUserStore,
   useConfigStore,
   useAppsStore,
-  useCanListShares
+  useCanListShares,
+  useCanListVersions
 } from '../../composables'
 import {
   isProjectSpaceResource,
   SpaceResource,
   Resource,
   ShareRole,
   call,
-  isSpaceResource,
-  isPersonalSpaceResource,
   isCollaboratorShare,
   isLinkShare,
   isShareSpaceResource,
-  isIncomingShareResource
+  isIncomingShareResource,
+  isPersonalSpaceResource
 } from '@ownclouders/web-client'
 import { storeToRefs } from 'pinia'
 import { useTask } from 'vue-concurrency'
@@ -98,10 +96,10 @@ export default defineComponent({
     const eventBus = useEventBus()
     const spacesStore = useSpacesStore()
     const sharesStore = useSharesStore()
-    const userStore = useUserStore()
     const configStore = useConfigStore()
     const appsStore = useAppsStore()
     const { canListShares } = useCanListShares()
+    const { canListVersions } = useCanListVersions()
 
     const resourcesStore = useResourcesStore()
     const { currentFolder } = storeToRefs(resourcesStore)
@@ -147,7 +145,6 @@ export default defineComponent({
     const isProjectsLocation = isLocationSpacesActive(router, 'files-spaces-projects')
     const isFavoritesLocation = useActiveLocation(isLocationCommonActive, 'files-common-favorites')
     const isSearchLocation = useActiveLocation(isLocationCommonActive, 'files-common-search')
-    const isTrashLocation = useActiveLocation(isLocationTrashActive, 'files-trash-generic')
 
     const closeSideBar = () => {
       eventBus.publish(SideBarEventTopics.close)
@@ -179,12 +176,6 @@ export default defineComponent({
       return unref(isShareLocation) || unref(isSearchLocation) || unref(isFavoritesLocation)
     })
 
-    const userIsSpaceMember = computed(
-      () =>
-        (isProjectSpaceResource(props.space) && props.space.isMember(userStore.user)) ||
-        (isPersonalSpaceResource(props.space) && props.space.isOwner(userStore.user))
-    )
-
     const availablePanels = computed(() =>
       extensionRegistry
         .requestExtensions<SidebarPanelExtension<SpaceResource, Resource, Resource>>({
@@ -325,17 +316,14 @@ export default defineComponent({
           loadVersionsTask.cancelAll()
         }
 
-        if (
-          !resource.isFolder &&
-          !isSpaceResource(resource) &&
-          unref(userIsSpaceMember) &&
-          !unref(isTrashLocation)
-        ) {
-          try {
-            await loadVersionsTask.perform(resource)
-          } catch (e) {
-            console.error(e)
-          }
+        if (!canListVersions({ space: props.space, resource })) {
+          return
+        }
+
+        try {
+          await loadVersionsTask.perform(resource)
+        } catch (e) {
+          console.error(e)
         }
       },
       { immediate: true, deep: true }

packages/web-pkg/src/composables/resources/index.ts

@@ -1,3 +1,4 @@
 export * from './useCanBeOpenedWithSecureView'
+export * from './useCanListVersions'
 export * from './useGetResourceContext'
 export * from './useResourceContents'

packages/web-pkg/src/composables/resources/useCanListVersions.ts

@@ -0,0 +1,23 @@
+import { useUserStore } from '../piniaStores'
+import { isSpaceResource, isTrashResource, Resource, SpaceResource } from '@ownclouders/web-client'
+
+export const useCanListVersions = () => {
+  const userStore = useUserStore()
+
+  const canListVersions = ({ space, resource }: { space: SpaceResource; resource: Resource }) => {
+    if (resource.type === 'folder') {
+      return false
+    }
+    if (isSpaceResource(resource)) {
+      return false
+    }
+    if (isTrashResource(resource)) {
+      return false
+    }
+    return space.canListVersions({ user: userStore.user })
+  }
+
+  return {
+    canListVersions
+  }
+}

packages/web-pkg/tests/unit/components/sidebar/FileSideBar.spec.ts

@@ -23,6 +23,9 @@ const InnerSideBarComponent = defineComponent({
 })
 
 vi.mock('../../../../src/composables/selection', () => ({ useSelectedResources: vi.fn() }))
+vi.mock('../../../../src/composables/resources/useCanListVersions', () => ({
+  useCanListVersions: () => ({ canListVersions: vi.fn() })
+}))
 
 const selectors = {
   sideBar: '.files-side-bar',

packages/web-pkg/tests/unit/composables/resources/useCanListVersions.spec.ts

@@ -0,0 +1,72 @@
+import { getComposableWrapper } from 'web-test-helpers'
+import { mock } from 'vitest-mock-extended'
+import { Resource, SpaceResource, TrashResource } from '@ownclouders/web-client'
+import { useCanListVersions } from '../../../../src/composables/resources'
+
+describe('useCanListVersions', () => {
+  describe('canListVersions', () => {
+    it('returns true for files when user has sufficient permissions in space', () => {
+      getWrapper({
+        setup: ({ canListVersions }) => {
+          const space = mock<SpaceResource>({ canListVersions: () => true })
+          const resource = mock<Resource>({ type: 'file' })
+          const canList = canListVersions({ space, resource })
+          expect(canList).toBeTruthy()
+        }
+      })
+    })
+    it('returns false for folders', () => {
+      getWrapper({
+        setup: ({ canListVersions }) => {
+          const space = mock<SpaceResource>({ canListVersions: () => true })
+          const resource = mock<Resource>({ type: 'folder' })
+          const canList = canListVersions({ space, resource })
+          expect(canList).toBeFalsy()
+        }
+      })
+    })
+    it('returns false for space resources', () => {
+      getWrapper({
+        setup: ({ canListVersions }) => {
+          const space = mock<SpaceResource>({ canListVersions: () => true })
+          const resource = mock<SpaceResource>({ type: 'space' })
+          const canList = canListVersions({ space, resource })
+          expect(canList).toBeFalsy()
+        }
+      })
+    })
+    it('returns false for trash resources', () => {
+      getWrapper({
+        setup: ({ canListVersions }) => {
+          const space = mock<SpaceResource>({ canListVersions: () => true })
+          const resource = mock<TrashResource>({ type: 'file', ddate: '' })
+          const canList = canListVersions({ space, resource })
+          expect(canList).toBeFalsy()
+        }
+      })
+    })
+    it('returns false when user does not have sufficient permissions in space', () => {
+      getWrapper({
+        setup: ({ canListVersions }) => {
+          const space = mock<SpaceResource>({ canListVersions: () => false })
+          const resource = mock<Resource>({ type: 'file' })
+          const canList = canListVersions({ space, resource })
+          expect(canList).toBeFalsy()
+        }
+      })
+    })
+  })
+})
+
+function getWrapper({
+  setup
+}: {
+  setup: (instance: ReturnType<typeof useCanListVersions>) => void
+}) {
+  return {
+    wrapper: getComposableWrapper(() => {
+      const instance = useCanListVersions()
+      setup(instance)
+    })
+  }
+}

tests/e2e/cucumber/features/spaces/project.feature

@@ -152,7 +152,7 @@ Feature: spaces.personal
 
     When "Carol" logs in
     And "Carol" navigates to the project space "team.1"
-    And "Carol" should not see the version of the file
+    And "Carol" should not see the version panel for the file
       | resource     | to     |
       | textfile.txt | parent |
     And "Carol" logs out