Skip to content

Commit

Permalink
Hide actions in space trash bins in case of insufficient permissions (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
JammingBen authored Oct 13, 2022
1 parent c69ba1a commit eea0f23
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 17 deletions.
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
12 changes: 11 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,15 @@ export default {
if (this.capabilities?.files?.permanent_deletion === false) {
return false
}

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

return resources.length > 0
},
componentType: 'button',
Expand Down
10 changes: 10 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,15 @@ export default {
if (this.capabilities?.files?.permanent_deletion === false) {
return false
}

if (
isProjectSpaceResource(this.space) &&
!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
10 changes: 9 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,14 @@ export default {
return false
}

if (
isProjectSpaceResource(this.space) &&
!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

0 comments on commit eea0f23

Please sign in to comment.