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

Hide actions in space trash bins in case of insufficient permissions #7768

Merged
merged 5 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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-space-trashbin-actions
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Hide actions in space trash bins

Actions in space trash bins are now hidden if the current user has insufficient permissions.

https://github.com/owncloud/web/pull/7768
https://github.com/owncloud/web/issues/7702
13 changes: 12 additions & 1 deletion packages/web-app-files/src/mixins/actions/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import {
isLocationTrashActive,
isLocationCommonActive
} from '../../router'
import { isProjectSpaceResource } from 'web-client/src/helpers'

export default {
mixins: [MixinDeleteResources],
computed: {
...mapState('Files', ['currentFolder']),
...mapGetters(['capabilities']),
...mapGetters(['capabilities', 'user']),
$_delete_items() {
return [
{
Expand Down Expand Up @@ -60,6 +61,16 @@ export default {
if (this.capabilities?.files?.permanent_deletion === false) {
return false
}

const isProjectSpace = isProjectSpaceResource(this.space)
if (
isProjectSpace &&
JammingBen marked this conversation as resolved.
Show resolved Hide resolved
!this.space.isEditor(this.user.uuid) &&
!this.space.isManager(this.user.uuid)
) {
return false
}

return resources.length > 0
},
componentType: 'button',
Expand Down
11 changes: 11 additions & 0 deletions packages/web-app-files/src/mixins/actions/emptyTrashBin.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mapActions, mapGetters, mapState } from 'vuex'
import { isLocationTrashActive } from '../../router'
import { buildWebDavFilesTrashPath, buildWebDavSpacesTrashPath } from '../../helpers/resources'
import { isProjectSpaceResource } from 'web-client/src/helpers'

export default {
computed: {
Expand All @@ -21,6 +22,16 @@ export default {
if (this.capabilities?.files?.permanent_deletion === false) {
return false
}

const isProjectSpace = isProjectSpaceResource(this.space)
if (
isProjectSpace &&
!this.space.isEditor(this.user.uuid) &&
!this.space.isManager(this.user.uuid)
) {
return false
}

// empty trash bin is not available for individual resources, but only for the trash bin as a whole
return resources.length === 0
},
Expand Down
11 changes: 10 additions & 1 deletion packages/web-app-files/src/mixins/actions/restore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
buildWebDavSpacesTrashPath
} from '../../helpers/resources'
import { clientService } from 'web-pkg/src/services'
import { buildWebDavSpacesPath } from 'web-client/src/helpers'
import { buildWebDavSpacesPath, isProjectSpaceResource } from 'web-client/src/helpers'

export default {
computed: {
Expand All @@ -29,6 +29,15 @@ export default {
return false
}

const isProjectSpace = isProjectSpaceResource(this.space)
if (
isProjectSpace &&
!this.space.isEditor(this.user.uuid) &&
!this.space.isManager(this.user.uuid)
) {
return false
}

return resources.length > 0
},
componentType: 'button',
Expand Down
12 changes: 11 additions & 1 deletion packages/web-app-files/src/views/spaces/GenericTrash.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
:sort-by="sortBy"
:sort-dir="sortDir"
:space="space"
:has-actions="showActions"
@sort="handleSort"
>
<template #contextMenu="{ resource }">
Expand Down Expand Up @@ -78,7 +79,7 @@ import { computed, defineComponent, PropType } from '@vue/composition-api'
import { Resource } from 'web-client'
import { useCapabilityShareJailEnabled, useTranslations } from 'web-pkg/src/composables'
import { createLocationTrash } from '../../router'
import { SpaceResource } from 'web-client/src/helpers'
import { isProjectSpaceResource, SpaceResource } from 'web-client/src/helpers'

export default defineComponent({
name: 'GenericTrash',
Expand Down Expand Up @@ -125,6 +126,7 @@ export default defineComponent({
computed: {
...mapState('Files', ['files']),
...mapGetters('Files', ['highlightedFile', 'totalFilesCount']),
...mapGetters(['user']),

isEmpty() {
return this.paginatedResources.length < 1
Expand All @@ -148,6 +150,14 @@ export default defineComponent({
onClick: () => bus.publish('app.files.list.load')
}
]
},

showActions() {
return (
!isProjectSpaceResource(this.space) ||
this.space.isEditor(this.user.uuid) ||
this.space.isManager(this.user.uuid)
)
}
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function getWrapper({ deletePermanent = false, invalidLocation = false } = {}) {
localVue,
store,
mocks: {
space: { driveType: 'personal', spaceRoles: { viewer: [], editor: [], manager: [] } },
$router: {
currentRoute: invalidLocation
? createLocationShares('files-shares-via-link')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ describe('emptyTrashBin', () => {
const wrapper = getWrapper({ invalidLocation: true })
expect(wrapper.vm.$_emptyTrashBin_items[0].isEnabled({ resources: [] })).toBe(false)
})
it('should be false in a space trash bin with insufficient permissions', () => {
const wrapper = getWrapper({ driveType: 'project' })
expect(
wrapper.vm.$_emptyTrashBin_items[0].isEnabled({
resources: [{ canBeRestored: () => true }]
})
).toBe(false)
})
})

describe('method "$_emptyTrashBin_trigger"', () => {
Expand Down Expand Up @@ -65,7 +73,11 @@ describe('emptyTrashBin', () => {
})
})

function getWrapper({ invalidLocation = false, resolveClearTrashBin = true } = {}) {
function getWrapper({
invalidLocation = false,
resolveClearTrashBin = true,
driveType = 'personal'
} = {}) {
return mount(Component, {
localVue,
mocks: {
Expand All @@ -79,6 +91,7 @@ function getWrapper({ invalidLocation = false, resolveClearTrashBin = true } = {
},
$gettext: jest.fn(),
$pgettext: jest.fn(),
space: { driveType, isEditor: () => false, isManager: () => false },
$client: {
...sdkMock,
fileTrash: {
Expand Down
13 changes: 12 additions & 1 deletion packages/web-app-files/tests/unit/mixins/actions/restore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ describe('restore', () => {
const wrapper = getWrapper({ invalidLocation: true })
expect(wrapper.vm.$_restore_items[0].isEnabled({ resources: [{}] })).toBe(false)
})
it('should be false in a space trash bin with insufficient permissions', () => {
const wrapper = getWrapper({ driveType: 'project' })
expect(
wrapper.vm.$_restore_items[0].isEnabled({ resources: [{ canBeRestored: () => true }] })
).toBe(false)
})
})

describe('method "$_restore_trigger"', () => {
Expand Down Expand Up @@ -65,7 +71,11 @@ describe('restore', () => {
})
})

function getWrapper({ invalidLocation = false, resolveClearTrashBin: resolveRestore = true } = {}) {
function getWrapper({
invalidLocation = false,
resolveClearTrashBin: resolveRestore = true,
driveType = 'personal'
} = {}) {
return mount(Component, {
localVue,
mocks: {
Expand All @@ -79,6 +89,7 @@ function getWrapper({ invalidLocation = false, resolveClearTrashBin: resolveRest
},
$gettext: jest.fn(),
$gettextInterpolate: jest.fn(),
space: { driveType, isEditor: () => false, isManager: () => false },
$client: {
...sdkMock,
fileTrash: {
Expand Down
11 changes: 10 additions & 1 deletion packages/web-client/src/helpers/space/functions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { User } from '../user'
import { buildWebDavSpacesPath, extractDomSelector, Resource } from '../resource'
import { SpacePeopleShareRoles, spaceRoleEditor, spaceRoleManager } from '../share'
import { SpacePeopleShareRoles, spaceRoleEditor, spaceRoleManager, spaceRoleViewer } from '../share'
import { PublicSpaceResource, ShareSpaceResource, SpaceResource, SHARE_JAIL_ID } from './types'
import { DavProperty } from 'web-pkg/src/constants'
import { buildWebDavPublicPath } from 'files/src/helpers/resources'
Expand Down Expand Up @@ -197,6 +197,15 @@ export function buildSpace(data): SpaceResource {
},
getWebDavUrl(resource: Resource): string {
return urlJoin(this.webDavUrl, resource.path)
},
isViewer(uuid: string): boolean {
return this.spaceRoles[spaceRoleViewer.name].includes(uuid)
},
isEditor(uuid: string): boolean {
return this.spaceRoles[spaceRoleEditor.name].includes(uuid)
},
isManager(uuid: string): boolean {
return this.spaceRoles[spaceRoleManager.name].includes(uuid)
}
}
Object.defineProperty(s, 'nodeId', {
Expand Down
3 changes: 3 additions & 0 deletions packages/web-client/src/helpers/space/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export const isPersonalSpaceResource = (resource: Resource): resource is Persona

export interface ProjectSpaceResource extends SpaceResource {
__projectSpaceResource?: any
isViewer(uuid: string): boolean
isEditor(uuid: string): boolean
isManager(uuid: string): boolean
}
export const isProjectSpaceResource = (resource: Resource): resource is ProjectSpaceResource => {
return resource.driveType === 'project'
Expand Down
23 changes: 14 additions & 9 deletions tests/e2e/cucumber/steps/app-files/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,14 @@ Then(
const { page } = this.actorsEnvironment.getActor({ key: stepUser })
const resourceObject = new objects.applicationFiles.Resource({ page })
for (const info of stepTable.hashes()) {
const message = await resourceObject.deleteTrashBin({ resource: info.resource })
const paths = info.resource.split('/')
if (actionType === 'should not') {
expect(message).toBe(`failed to delete "${paths[paths.length - 1]}"`)
const isVisible = await resourceObject.isDeleteTrashBinButtonVisible({
resource: info.resource
})
expect(isVisible).toBe(false)
} else {
const message = await resourceObject.deleteTrashBin({ resource: info.resource })
const paths = info.resource.split('/')
expect(message).toBe(`"${paths[paths.length - 1]}" was deleted successfully`)
}
}
Expand All @@ -190,14 +193,16 @@ Then(
const { page } = this.actorsEnvironment.getActor({ key: stepUser })
const resourceObject = new objects.applicationFiles.Resource({ page })
for (const info of stepTable.hashes()) {
const message = await resourceObject.restoreTrashBin({
resource: info.resource,
actionType
})
const paths = info.resource.split('/')
if (actionType === 'should not') {
expect(message).toBe(`failed to restore "${paths[paths.length - 1]}"`)
const isVisible = await resourceObject.isRestoreTrashBinButtonVisible({
resource: info.resource
})
expect(isVisible).toBe(false)
} else {
const message = await resourceObject.restoreTrashBin({
resource: info.resource
})
const paths = info.resource.split('/')
expect(message).toBe(`${paths[paths.length - 1]} was restored successfully`)
}
}
Expand Down
27 changes: 26 additions & 1 deletion tests/e2e/support/objects/app-files/resource/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,21 @@ export const deleteResourceTrashbin = async (args: deleteResourceArgs): Promise<
return message.trim().toLowerCase()
}

export const getDeleteResourceButtonVisibility = async (
args: deleteResourceArgs
): Promise<boolean> => {
const { page, resource } = args
const resourceCheckbox = page.locator(
util.format(checkBoxForTrashbin, `/${resource.replace(/^\/+/, '')}`)
)
if (!(await resourceCheckbox.isChecked())) {
await resourceCheckbox.check()
}
return await page.locator(permanentDeleteButton).isVisible()
}

export interface restoreResourceTrashbinArgs {
resource: string
actionType: string
page: Page
}

Expand All @@ -499,6 +511,19 @@ export const restoreResourceTrashbin = async (
return message.trim().toLowerCase()
}

export const getRestoreResourceButtonVisibility = async (
args: restoreResourceTrashbinArgs
): Promise<boolean> => {
const { page, resource } = args
const resourceCheckbox = page.locator(
util.format(checkBoxForTrashbin, `/${resource.replace(/^\/+/, '')}`)
)
if (!(await resourceCheckbox.isChecked())) {
await resourceCheckbox.check()
}
return await page.locator(restoreResourceButton).isVisible()
}

export interface searchResourceGlobalSearchArgs {
keyword: string
page: Page
Expand Down
14 changes: 13 additions & 1 deletion tests/e2e/support/objects/app-files/resource/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import {
editResources,
editResourcesArgs,
openFileInViewer,
openFileInViewerArgs
openFileInViewerArgs,
getDeleteResourceButtonVisibility,
getRestoreResourceButtonVisibility
} from './actions'

export class Resource {
Expand Down Expand Up @@ -115,13 +117,23 @@ export class Resource {
return message
}

async isDeleteTrashBinButtonVisible(args: Omit<deleteResourceArgs, 'page'>): Promise<boolean> {
return await getDeleteResourceButtonVisibility({ ...args, page: this.#page })
}

async restoreTrashBin(args: Omit<restoreResourceTrashbinArgs, 'page'>): Promise<string> {
const startUrl = this.#page.url()
const message = await restoreResourceTrashbin({ ...args, page: this.#page })
await this.#page.goto(startUrl)
return message
}

async isRestoreTrashBinButtonVisible(
args: Omit<restoreResourceTrashbinArgs, 'page'>
): Promise<boolean> {
return await getRestoreResourceButtonVisibility({ ...args, page: this.#page })
}

async searchResource(args: Omit<searchResourceGlobalSearchArgs, 'page'>): Promise<void> {
await searchResourceGlobalSearch({ ...args, page: this.#page })
}
Expand Down