Merge pull request #11139 from owncloud/fix/secure-view-default-action

fix: secure view default action

changelog/unreleased/bugfix-secure-view-default-action

@@ -0,0 +1,6 @@
+Bugfix: Secure view default action
+
+Clicking files that have been shared via secure view without having a compatible app to view such (or the file type is not supported) is no longer possible. This prevents errors and other file actions from falsely registering themselves as default.
+
+https://github.com/owncloud/web/pull/11139
+https://github.com/owncloud/web/issues/11138

packages/web-pkg/src/components/FilesList/ResourceTable.vue

@@ -258,7 +258,8 @@ import {
   useConfigStore,
   useClipboardStore,
   useResourcesStore,
-  useRouter
+  useRouter,
+  useCanBeOpenedWithSecureView
 } from '../../composables'
 import ResourceListItem from './ResourceListItem.vue'
 import ResourceGhostElement from './ResourceGhostElement.vue'
@@ -505,6 +506,7 @@ export default defineComponent({
     const router = useRouter()
     const capabilityStore = useCapabilityStore()
     const { getMatchingSpace } = useGetMatchingSpace()
+    const { canBeOpenedWithSecureView } = useCanBeOpenedWithSecureView()
     const {
       isLocationPicker,
       isFilePicker,
@@ -559,6 +561,24 @@ export default defineComponent({
       )
     })
 
+    const isResourceClickable = (resource: Resource) => {
+      if (!props.areResourcesClickable) {
+        return false
+      }
+
+      if (!resource.isFolder) {
+        if (!resource.canDownload() && !canBeOpenedWithSecureView(resource)) {
+          return false
+        }
+
+        if (unref(isEmbedModeEnabled) && !unref(isFilePicker)) {
+          return false
+        }
+      }
+
+      return !unref(disabledResources).includes(resource.id)
+    }
+
     return {
       router,
       configOptions,
@@ -593,7 +613,8 @@ export default defineComponent({
       isEmbedModeEnabled,
       toggleSelection,
       areFileExtensionsShown,
-      latestSelectedId
+      latestSelectedId,
+      isResourceClickable
     }
   },
   data() {
@@ -1057,17 +1078,6 @@ export default defineComponent({
        */
       this.$emit('fileClick', { space, resources: [resource] })
     },
-    isResourceClickable(resource: Resource) {
-      if (!this.areResourcesClickable) {
-        return false
-      }
-
-      if (this.isEmbedModeEnabled && !this.isFilePicker && !resource.isFolder) {
-        return false
-      }
-
-      return !this.disabledResources.includes(resource.id)
-    },
     getResourceCheckboxLabel(resource: Resource) {
       if (resource.type === 'folder') {
         return this.$gettext('Select folder')

packages/web-pkg/src/components/FilesList/ResourceTiles.vue

@@ -156,7 +156,8 @@ import {
   useTileSize,
   useResourcesStore,
   useViewSizeMax,
-  useEmbedMode
+  useEmbedMode,
+  useCanBeOpenedWithSecureView
 } from '../../composables'
 
 type ResourceTileRef = ComponentPublicInstance<typeof ResourceTile>
@@ -221,6 +222,7 @@ export default defineComponent({
     const { showMessage } = useMessages()
     const { $gettext } = useGettext()
     const resourcesStore = useResourcesStore()
+    const { canBeOpenedWithSecureView } = useCanBeOpenedWithSecureView()
     const { emit } = context
     const {
       isEnabled: isEmbedModeEnabled,
@@ -311,7 +313,15 @@ export default defineComponent({
     }
 
     const isResourceClickable = (resource: Resource) => {
-      if (unref(isEmbedModeEnabled) && !unref(isFilePicker) && !resource.isFolder) {
+      if (resource.isFolder) {
+        return true
+      }
+
+      if (!resource.canDownload() && !canBeOpenedWithSecureView(resource)) {
+        return false
+      }
+
+      if (unref(isEmbedModeEnabled) && !unref(isFilePicker)) {
         return false
       }
 

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

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

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

@@ -0,0 +1,13 @@
+import { Resource } from '@ownclouders/web-client'
+import { useAppsStore } from '../piniaStores'
+
+export const useCanBeOpenedWithSecureView = () => {
+  const appsStore = useAppsStore()
+
+  const canBeOpenedWithSecureView = (resource: Resource) => {
+    const secureViewExtensions = appsStore.fileExtensions.filter(({ secureView }) => secureView)
+    return secureViewExtensions.some(({ mimeType }) => mimeType === resource.mimeType)
+  }
+
+  return { canBeOpenedWithSecureView }
+}

packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts

@@ -11,6 +11,7 @@ import {
 } from '@ownclouders/web-client'
 import { defaultPlugins, mount, PartialComponentProps } from 'web-test-helpers'
 import { CapabilityStore } from '../../../../src/composables/piniaStores'
+import { useCanBeOpenedWithSecureView } from '../../../../src/composables/resources'
 import { displayPositionedDropdown } from '../../../../src/helpers/contextMenuDropdown'
 import { eventBus } from '../../../../src/services/eventBus'
 import { SideBarEventTopics } from '../../../../src/composables/sideBar'
@@ -30,6 +31,11 @@ vi.mock('../../../../src/composables/embedMode', () => ({
   useEmbedMode: vi.fn().mockImplementation(() => mockUseEmbedMode())
 }))
 
+vi.mock('../../../../src/composables/resources', async (importOriginal) => ({
+  ...(await importOriginal<any>()),
+  useCanBeOpenedWithSecureView: vi.fn()
+}))
+
 const router = {
   push: vi.fn(),
   afterEach: vi.fn(),
@@ -121,7 +127,8 @@ const resourcesWithAllFields = [
     sharePermissions: [],
     shareTypes: [],
     canRename: vi.fn(),
-    getDomSelector: () => extractDomSelector('forest')
+    getDomSelector: () => extractDomSelector('forest'),
+    canDownload: () => true
   },
   {
     id: 'notes',
@@ -146,7 +153,8 @@ const resourcesWithAllFields = [
     sharePermissions: [],
     shareTypes: [],
     canRename: vi.fn(),
-    getDomSelector: () => extractDomSelector('notes')
+    getDomSelector: () => extractDomSelector('notes'),
+    canDownload: () => true
   },
   {
     id: 'documents',
@@ -170,7 +178,8 @@ const resourcesWithAllFields = [
     sharePermissions: [],
     shareTypes: [],
     canRename: vi.fn(),
-    getDomSelector: () => extractDomSelector('documents')
+    getDomSelector: () => extractDomSelector('documents'),
+    canDownload: () => true
   },
   {
     id: 'another-one==',
@@ -194,7 +203,8 @@ const resourcesWithAllFields = [
     shareTypes: [],
     tags: [],
     canRename: vi.fn(),
-    getDomSelector: () => extractDomSelector('another-one==')
+    getDomSelector: () => extractDomSelector('another-one=='),
+    canDownload: () => true
   }
 ] as IncomingShareResource[]
 
@@ -225,6 +235,7 @@ const processingResourcesWithAllFields = [
     shareTypes: [],
     canRename: vi.fn(),
     getDomSelector: () => extractDomSelector('forest'),
+    canDownload: () => true,
     processing: true
   },
   {
@@ -251,6 +262,7 @@ const processingResourcesWithAllFields = [
     shareTypes: [],
     canRename: vi.fn(),
     getDomSelector: () => extractDomSelector('notes'),
+    canDownload: () => true,
     processing: true
   }
 ] as IncomingShareResource[]
@@ -417,6 +429,22 @@ describe('ResourceTable', () => {
       await tr.trigger('click')
       expect(wrapper.emitted().fileClick).toBeUndefined()
     })
+
+    it('does not emit fileClick event if file can not be opened via secure view', async () => {
+      const { wrapper } = getMountedWrapper({
+        canBeOpenedWithSecureView: false,
+        resources: [
+          {
+            ...resourcesWithAllFields[0],
+            isFolder: false,
+            canDownload: () => false
+          }
+        ]
+      })
+      const tr = await wrapper.find('.oc-tbody-tr-forest .oc-resource-name')
+      await tr.trigger('click')
+      expect(wrapper.emitted().fileClick).toBeUndefined()
+    })
   })
 
   describe('resource details', () => {
@@ -579,17 +607,23 @@ function getMountedWrapper({
   props = {},
   userContextReady = true,
   addProcessingResources = false,
+  canBeOpenedWithSecureView = true,
   resources = resourcesWithAllFields
 }: {
   props?: PartialComponentProps<typeof ResourceTable>
   userContextReady?: boolean
   addProcessingResources?: boolean
+  canBeOpenedWithSecureView?: boolean
   resources?: Resource[]
 } = {}) {
   const capabilities = {
     files: { tags: true }
   } satisfies Partial<CapabilityStore['capabilities']>
 
+  vi.mocked(useCanBeOpenedWithSecureView).mockReturnValue({
+    canBeOpenedWithSecureView: () => canBeOpenedWithSecureView
+  })
+
   return {
     wrapper: mount(ResourceTable, {
       props: {