diff --git a/changelog/unreleased/bugfix-secure-view-default-action b/changelog/unreleased/bugfix-secure-view-default-action new file mode 100644 index 00000000000..067bdf57334 --- /dev/null +++ b/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 diff --git a/packages/web-pkg/src/components/FilesList/ResourceTable.vue b/packages/web-pkg/src/components/FilesList/ResourceTable.vue index cb3bf127626..5c4224e6373 100644 --- a/packages/web-pkg/src/components/FilesList/ResourceTable.vue +++ b/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') diff --git a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue index a81b9d094b7..e3199da1683 100644 --- a/packages/web-pkg/src/components/FilesList/ResourceTiles.vue +++ b/packages/web-pkg/src/components/FilesList/ResourceTiles.vue @@ -156,7 +156,8 @@ import { useTileSize, useResourcesStore, useViewSizeMax, - useEmbedMode + useEmbedMode, + useCanBeOpenedWithSecureView } from '../../composables' type ResourceTileRef = ComponentPublicInstance @@ -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 } diff --git a/packages/web-pkg/src/composables/resources/index.ts b/packages/web-pkg/src/composables/resources/index.ts index 623db71e764..c54a10d4094 100644 --- a/packages/web-pkg/src/composables/resources/index.ts +++ b/packages/web-pkg/src/composables/resources/index.ts @@ -1,3 +1,4 @@ +export * from './useCanBeOpenedWithSecureView' export * from './useGetResourceContext' export * from './useLoadFileInfoById' export * from './useResourceContents' diff --git a/packages/web-pkg/src/composables/resources/useCanBeOpenedWithSecureView.ts b/packages/web-pkg/src/composables/resources/useCanBeOpenedWithSecureView.ts new file mode 100644 index 00000000000..b220448ab59 --- /dev/null +++ b/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 } +} diff --git a/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts b/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts index 866e5134af2..c9377b90e74 100644 --- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTable.spec.ts +++ b/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()), + 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 userContextReady?: boolean addProcessingResources?: boolean + canBeOpenedWithSecureView?: boolean resources?: Resource[] } = {}) { const capabilities = { files: { tags: true } } satisfies Partial + vi.mocked(useCanBeOpenedWithSecureView).mockReturnValue({ + canBeOpenedWithSecureView: () => canBeOpenedWithSecureView + }) + return { wrapper: mount(ResourceTable, { props: { diff --git a/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts b/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts index 7902f44a868..4b753e53baf 100644 --- a/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts +++ b/packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts @@ -6,6 +6,7 @@ import { mock } from 'vitest-mock-extended' import { ComponentPublicInstance, computed } from 'vue' import { extractDomSelector } from '@ownclouders/web-client' import OcDrop from 'design-system/src/components/OcDrop/OcDrop.vue' +import { useCanBeOpenedWithSecureView } from '../../../../src/composables/resources' vi.mock('../../../../src/composables/viewMode', async (importOriginal) => ({ ...(await importOriginal()), @@ -19,6 +20,11 @@ vi.mock('../../../../src/composables/embedMode', () => ({ useEmbedMode: vi.fn().mockImplementation(() => mockUseEmbedMode()) })) +vi.mock('../../../../src/composables/resources', async (importOriginal) => ({ + ...(await importOriginal()), + useCanBeOpenedWithSecureView: vi.fn() +})) + const spacesResources = [ { id: '1', @@ -57,7 +63,8 @@ const resources = [ syncEnabled: true, outgoing: false, canRename: vi.fn(), - getDomSelector: () => extractDomSelector('forest') + getDomSelector: () => extractDomSelector('forest'), + canDownload: () => true } ] @@ -91,37 +98,52 @@ describe('ResourceTiles component', () => { }) }) it('renders an array of spaces correctly', async () => { - const { wrapper } = getWrapper({ resources: spacesResources }) + const { wrapper } = getWrapper({ props: { resources: spacesResources } }) await wrapper.vm.$nextTick() expect(wrapper.html()).toMatchSnapshot() }) it('renders a footer slot', () => { - const { wrapper } = getWrapper({}, { footer: 'Hello, ResourceTiles footer!' }) + const { wrapper } = getWrapper({ slots: { footer: 'Hello, ResourceTiles footer!' } }) expect(wrapper.html()).toMatchSnapshot() }) - it('emits fileClick event upon click on tile', async () => { - const { wrapper } = getWrapper({ resources: resources }) - await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click') - expect( - wrapper.emitted<{ resources: Resource[] }[]>('fileClick')[0][0].resources[0].name - ).toMatch('forest.jpg') - }) + describe('file click', () => { + it('emits fileClick event upon click on tile', async () => { + const { wrapper } = getWrapper({ props: { resources } }) + await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click') + expect( + wrapper.emitted<{ resources: Resource[] }[]>('fileClick')[0][0].resources[0].name + ).toMatch('forest.jpg') + }) - it('does not emit fileClick event upon click on tile when embed mode is enabled', async () => { - mockUseEmbedMode.mockReturnValue({ - isEnabled: computed(() => true) + it('does not emit fileClick event upon click on tile when embed mode is enabled', async () => { + mockUseEmbedMode.mockReturnValue({ + isEnabled: computed(() => true) + }) + const { wrapper } = getWrapper({ props: { resources } }) + await wrapper.find('.oc-tiles-item .oc-resource-name').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 } = getWrapper({ + canBeOpenedWithSecureView: false, + props: { + resources: [{ ...resources[0], isFolder: false, canDownload: () => false }] + } + }) + await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click') + expect(wrapper.emitted().fileClick).toBeUndefined() }) - const { wrapper } = getWrapper({ resources: resources }) - await wrapper.find('.oc-tiles-item .oc-resource-name').trigger('click') - expect(wrapper.emitted().fileClick).toBeUndefined() }) it('emits update:selectedIds event on resource selection and sets the selection', () => { const { wrapper } = getWrapper({ - resources: spacesResources, - selectedIds: [spacesResources[0].id] + props: { + resources: spacesResources, + selectedIds: [spacesResources[0].id] + } }) wrapper.vm.toggleSelection(spacesResources[0]) expect( @@ -132,21 +154,23 @@ describe('ResourceTiles component', () => { describe('sorting', () => { it('renders the label of the first sort field as default', () => { - const { wrapper } = getWrapper({ sortFields }) + const { wrapper } = getWrapper({ props: { sortFields } }) expect(wrapper.find('#oc-tiles-sort-btn').text()).toEqual(sortFields[0].label) }) it('renders the label of the current sort field as default', () => { const sortField = sortFields[2] const { wrapper } = getWrapper({ - sortFields, - sortBy: sortField.name, - sortDir: sortField.sortDir + props: { + sortFields, + sortBy: sortField.name, + sortDir: sortField.sortDir + } }) expect(wrapper.find('#oc-tiles-sort-btn').text()).toEqual(sortField.label) }) it('emits the "sort"-event', async () => { const index = 2 - const { wrapper } = getWrapper({ sortFields }) + const { wrapper } = getWrapper({ props: { sortFields } }) ;(wrapper.vm.$refs.sortDrop as ComponentPublicInstance).tippy = { hide: vi.fn() } @@ -184,16 +208,22 @@ describe('ResourceTiles component', () => { { viewSize: 5, expected: 'xxxlarge' }, { viewSize: 6, expected: 'xxxlarge' } ])('passes the "viewSize" to the OcTile component', async (data) => { - const { wrapper } = getWrapper({ resources: spacesResources, viewSize: data.viewSize }) + const { wrapper } = getWrapper({ + props: { resources: spacesResources, viewSize: data.viewSize } + }) await wrapper.vm.$nextTick() expect(wrapper.findComponent({ name: 'resource-tile' }).props('resourceIconSize')).toEqual( data.expected ) }) - function getWrapper(props = {}, slots = {}) { + function getWrapper({ props = {}, slots = {}, canBeOpenedWithSecureView = true } = {}) { const mocks = defaultComponentMocks() + vi.mocked(useCanBeOpenedWithSecureView).mockReturnValue({ + canBeOpenedWithSecureView: () => canBeOpenedWithSecureView + }) + return { wrapper: mount(ResourceTiles, { props: { diff --git a/packages/web-pkg/tests/unit/composables/resources/useCanBeOpenedWithSecureView.spec.ts b/packages/web-pkg/tests/unit/composables/resources/useCanBeOpenedWithSecureView.spec.ts new file mode 100644 index 00000000000..850a3be0680 --- /dev/null +++ b/packages/web-pkg/tests/unit/composables/resources/useCanBeOpenedWithSecureView.spec.ts @@ -0,0 +1,70 @@ +import { getComposableWrapper } from 'web-test-helpers' +import { mock } from 'vitest-mock-extended' +import { Resource } from '@ownclouders/web-client' +import { useCanBeOpenedWithSecureView } from '../../../../src/composables/resources' +import { ApplicationFileExtension } from '../../../../src/apps/types' + +describe('canBeOpenedWithSecureView', () => { + describe('resource', () => { + it('can be opened if a matching file extension with secure view exists', () => { + getWrapper({ + setup: ({ canBeOpenedWithSecureView }) => { + const canBeOpened = canBeOpenedWithSecureView(mock({ mimeType: 'text/plain' })) + expect(canBeOpened).toBeTruthy() + }, + fileExtensions: [ + mock({ secureView: true, mimeType: 'text/plain' }) + ] + }) + }) + it('can not be opened if no file extension with secure view exists', () => { + getWrapper({ + setup: ({ canBeOpenedWithSecureView }) => { + const canBeOpened = canBeOpenedWithSecureView(mock({ mimeType: 'text/plain' })) + expect(canBeOpened).toBeFalsy() + }, + fileExtensions: [ + mock({ secureView: false, mimeType: 'text/plain' }) + ] + }) + }) + it('can not be opened if no file extension exists', () => { + getWrapper({ + setup: ({ canBeOpenedWithSecureView }) => { + const canBeOpened = canBeOpenedWithSecureView(mock({ mimeType: 'text/plain' })) + expect(canBeOpened).toBeFalsy() + }, + fileExtensions: [] + }) + }) + it("can not be opened if the file extension's mime type doesn't match the one of the resource", () => { + getWrapper({ + setup: ({ canBeOpenedWithSecureView }) => { + const canBeOpened = canBeOpenedWithSecureView(mock({ mimeType: 'text/plain' })) + expect(canBeOpened).toBeFalsy() + }, + fileExtensions: [ + mock({ secureView: true, mimeType: 'image/jpg' }) + ] + }) + }) + }) +}) + +function getWrapper({ + setup, + fileExtensions = [mock()] +}: { + setup: (instance: ReturnType) => void + fileExtensions?: ApplicationFileExtension[] +}) { + return { + wrapper: getComposableWrapper( + () => { + const instance = useCanBeOpenedWithSecureView() + setup(instance) + }, + { pluginOptions: { piniaOptions: { appsState: { fileExtensions } } } } + ) + } +}