Skip to content

Commit

Permalink
fix: secure view default action
Browse files Browse the repository at this point in the history
Prevents files that have been shared via secure view without having a compatible app to view such (or the file type is not supported) from being clicked.
  • Loading branch information
JammingBen committed Jul 4, 2024
1 parent 129f986 commit 9a099fb
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 20 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-secure-view-default-action
Original file line number Diff line number Diff line change
@@ -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
36 changes: 23 additions & 13 deletions packages/web-pkg/src/components/FilesList/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ import {
useConfigStore,
useClipboardStore,
useResourcesStore,
useRouter
useRouter,
useCanBeOpenedWithSecureView
} from '../../composables'
import ResourceListItem from './ResourceListItem.vue'
import ResourceGhostElement from './ResourceGhostElement.vue'
Expand Down Expand Up @@ -505,6 +506,7 @@ export default defineComponent({
const router = useRouter()
const capabilityStore = useCapabilityStore()
const { getMatchingSpace } = useGetMatchingSpace()
const { canBeOpenedWithSecureView } = useCanBeOpenedWithSecureView()
const {
isLocationPicker,
isFilePicker,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -593,7 +613,8 @@ export default defineComponent({
isEmbedModeEnabled,
toggleSelection,
areFileExtensionsShown,
latestSelectedId
latestSelectedId,
isResourceClickable
}
},
data() {
Expand Down Expand Up @@ -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')
Expand Down
14 changes: 12 additions & 2 deletions packages/web-pkg/src/components/FilesList/ResourceTiles.vue
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ import {
useTileSize,
useResourcesStore,
useViewSizeMax,
useEmbedMode
useEmbedMode,
useCanBeOpenedWithSecureView
} from '../../composables'
type ResourceTileRef = ComponentPublicInstance<typeof ResourceTile>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions packages/web-pkg/src/composables/resources/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './useCanBeOpenedWithSecureView'
export * from './useGetResourceContext'
export * from './useLoadFileInfoById'
export * from './useResourceContents'
Original file line number Diff line number Diff line change
@@ -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 }
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest')
getDomSelector: () => extractDomSelector('forest'),
canDownload: () => true
},
{
id: 'notes',
Expand All @@ -146,7 +147,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('notes')
getDomSelector: () => extractDomSelector('notes'),
canDownload: () => true
},
{
id: 'documents',
Expand All @@ -170,7 +172,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('documents')
getDomSelector: () => extractDomSelector('documents'),
canDownload: () => true
},
{
id: 'another-one==',
Expand All @@ -194,7 +197,8 @@ const resourcesWithAllFields = [
shareTypes: [],
tags: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('another-one==')
getDomSelector: () => extractDomSelector('another-one=='),
canDownload: () => true
}
] as IncomingShareResource[]

Expand Down Expand Up @@ -225,6 +229,7 @@ const processingResourcesWithAllFields = [
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest'),
canDownload: () => true,
processing: true
},
{
Expand All @@ -251,6 +256,7 @@ const processingResourcesWithAllFields = [
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('notes'),
canDownload: () => true,
processing: true
}
] as IncomingShareResource[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ const resources = [
syncEnabled: true,
outgoing: false,
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest')
getDomSelector: () => extractDomSelector('forest'),
canDownload: () => true
}
]

Expand Down
Original file line number Diff line number Diff line change
@@ -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<Resource>({ mimeType: 'text/plain' }))
expect(canBeOpened).toBeTruthy()
},
fileExtensions: [
mock<ApplicationFileExtension>({ 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<Resource>({ mimeType: 'text/plain' }))
expect(canBeOpened).toBeFalsy()
},
fileExtensions: [
mock<ApplicationFileExtension>({ secureView: false, mimeType: 'text/plain' })
]
})
})
it('can not be opened if no file extension exists', () => {
getWrapper({
setup: ({ canBeOpenedWithSecureView }) => {
const canBeOpened = canBeOpenedWithSecureView(mock<Resource>({ 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<Resource>({ mimeType: 'text/plain' }))
expect(canBeOpened).toBeFalsy()
},
fileExtensions: [
mock<ApplicationFileExtension>({ secureView: true, mimeType: 'image/jpg' })
]
})
})
})
})

function getWrapper({
setup,
fileExtensions = [mock<ApplicationFileExtension>()]
}: {
setup: (instance: ReturnType<typeof useCanBeOpenedWithSecureView>) => void
fileExtensions?: ApplicationFileExtension[]
}) {
return {
wrapper: getComposableWrapper(
() => {
const instance = useCanBeOpenedWithSecureView()
setup(instance)
},
{ pluginOptions: { piniaOptions: { appsState: { fileExtensions } } } }
)
}
}

0 comments on commit 9a099fb

Please sign in to comment.