Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: secure view default action #11139

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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'
Expand All @@ -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(),
Expand Down Expand Up @@ -121,7 +127,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest')
getDomSelector: () => extractDomSelector('forest'),
canDownload: () => true
},
{
id: 'notes',
Expand All @@ -146,7 +153,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('notes')
getDomSelector: () => extractDomSelector('notes'),
canDownload: () => true
},
{
id: 'documents',
Expand All @@ -170,7 +178,8 @@ const resourcesWithAllFields = [
sharePermissions: [],
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('documents')
getDomSelector: () => extractDomSelector('documents'),
canDownload: () => true
},
{
id: 'another-one==',
Expand All @@ -194,7 +203,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 +235,7 @@ const processingResourcesWithAllFields = [
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('forest'),
canDownload: () => true,
processing: true
},
{
Expand All @@ -251,6 +262,7 @@ const processingResourcesWithAllFields = [
shareTypes: [],
canRename: vi.fn(),
getDomSelector: () => extractDomSelector('notes'),
canDownload: () => true,
processing: true
}
] as IncomingShareResource[]
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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: {
Expand Down
Loading