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

Qfix: Invert delete object permission #5085

Merged
merged 1 commit into from
Mar 28, 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
2 changes: 1 addition & 1 deletion models/board/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ export function createModel (builder: Builder): void {
description: board.string.ManageBoardStatuses,
icon: board.icon.Board,
baseClass: board.class.Board,
availablePermissions: [core.permission.DeleteObject],
availablePermissions: [core.permission.ForbidDeleteObject],
allowedTaskTypeDescriptors: [board.descriptors.Card]
},
board.descriptors.BoardType
Expand Down
10 changes: 10 additions & 0 deletions models/core/src/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,14 @@ export function definePermissions (builder: Builder): void {
},
core.permission.DeleteObject
)

builder.createDoc(
core.class.Permission,
core.space.Model,
{
label: core.string.ForbidDeleteObject,
description: core.string.ForbidDeleteObjectDescription
},
core.permission.ForbidDeleteObject
)
}
2 changes: 1 addition & 1 deletion models/document/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ function defineTeamspace (builder: Builder): void {
description: document.string.Description,
icon: document.icon.Document,
baseClass: document.class.Teamspace,
availablePermissions: [core.permission.DeleteObject]
availablePermissions: [core.permission.ForbidDeleteObject]
},
document.descriptor.TeamspaceType
)
Expand Down
2 changes: 1 addition & 1 deletion models/lead/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ export function createModel (builder: Builder): void {
description: lead.string.ManageFunnelStatuses,
icon: lead.icon.LeadApplication,
baseClass: lead.class.Funnel,
availablePermissions: [core.permission.DeleteObject],
availablePermissions: [core.permission.ForbidDeleteObject],
allowedTaskTypeDescriptors: [lead.descriptors.Lead]
},
lead.descriptors.FunnelType
Expand Down
2 changes: 1 addition & 1 deletion models/recruit/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1709,7 +1709,7 @@ export function createModel (builder: Builder): void {
icon: recruit.icon.RecruitApplication,
editor: recruit.component.VacancyTemplateEditor,
baseClass: recruit.class.Vacancy,
availablePermissions: [core.permission.DeleteObject],
availablePermissions: [core.permission.ForbidDeleteObject],
allowedTaskTypeDescriptors: [recruit.descriptors.Application]
},
recruit.descriptors.VacancyType
Expand Down
2 changes: 1 addition & 1 deletion models/tracker/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ export function createModel (builder: Builder): void {
description: tracker.string.ManageWorkflowStatuses,
icon: task.icon.Task,
baseClass: tracker.class.Project,
availablePermissions: [core.permission.DeleteObject],
availablePermissions: [core.permission.ForbidDeleteObject],
allowedClassic: true,
allowedTaskTypeDescriptors: [tracker.descriptors.Issue]
},
Expand Down
4 changes: 3 additions & 1 deletion packages/core/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
"CreateObject": "Create object",
"UpdateObject": "Update object",
"DeleteObject": "Delete object",
"DeleteObjectDescription": "Grants users ability to delete objects in the space"
"DeleteObjectDescription": "Grants users ability to delete objects in the space",
"ForbidDeleteObject": "Forbid delete object",
"ForbidDeleteObjectDescription": "Forbid users deleting objects in the space"
}
}
4 changes: 3 additions & 1 deletion packages/core/lang/ru.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
"CreateObject": "Создавать объект",
"UpdateObject": "Обновлять объект",
"DeleteObject": "Удалять объект",
"DeleteObjectDescription": "Дает пользователям разрешение удалять объекты в пространстве"
"DeleteObjectDescription": "Дает пользователям разрешение удалять объекты в пространстве",
"ForbidDeleteObject": "Запретить удалять объект",
"ForbidDeleteObjectDescription": "Запрещает пользователям удалять объекты в пространстве"
}
}
7 changes: 5 additions & 2 deletions packages/core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,14 @@ export default plugin(coreId, {
CreateObject: '' as IntlString,
UpdateObject: '' as IntlString,
DeleteObject: '' as IntlString,
DeleteObjectDescription: '' as IntlString
DeleteObjectDescription: '' as IntlString,
ForbidDeleteObject: '' as IntlString,
ForbidDeleteObjectDescription: '' as IntlString
},
permission: {
CreateObject: '' as Ref<Permission>,
UpdateObject: '' as Ref<Permission>,
DeleteObject: '' as Ref<Permission>
DeleteObject: '' as Ref<Permission>,
ForbidDeleteObject: '' as Ref<Permission>
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
value !== undefined &&
value.readonly !== true &&
($permissionsStore.whitelist.has(value.space) ||
$permissionsStore.ps[value.space]?.has(core.permission.DeleteObject))
!$permissionsStore.ps[value.space]?.has(core.permission.ForbidDeleteObject))

function iconLabel (name: string): string {
const parts = `${name}`.split('.')
Expand Down
6 changes: 3 additions & 3 deletions plugins/view-resources/src/visibilityTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ export async function canDeleteObject (doc?: Doc | Doc[]): Promise<boolean> {
isTypedSpace
)

return (
return !(
await Promise.all(
Array.from(new Set(targetSpaces.map((t) => t._id))).map(
async (s) => await checkPermission(client, core.permission.DeleteObject, s)
async (s) => await checkPermission(client, core.permission.ForbidDeleteObject, s)
)
)
).every((r) => r)
).some((r) => r)
}
27 changes: 22 additions & 5 deletions server/middleware/src/spacePermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,30 @@ export class SpacePermissionsMiddleware extends BaseMiddleware implements Middle
/**
* @private
*
* Throws if the required permission is missing in the space for the given context
* Checks if the required permission is present in the space for the given context
*/
private async needPermission (ctx: SessionContext, space: Ref<TypedSpace>, id: Ref<Permission>): Promise<void> {
private async checkPermission (ctx: SessionContext, space: Ref<TypedSpace>, id: Ref<Permission>): Promise<boolean> {
const account = await getUser(this.storage, ctx)
const permissions = this.permissionsBySpace[space]?.[account._id] ?? new Set()

if (!permissions.has(id)) {
throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {}))
return permissions.has(id)
}

private throwForbidden (): void {
throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {}))
}

/**
* @private
*
* Throws if the required permission is missing in the space for the given context
*/
private async needPermission (ctx: SessionContext, space: Ref<TypedSpace>, id: Ref<Permission>): Promise<void> {
if (await this.checkPermission(ctx, space, id)) {
return
}

this.throwForbidden()
}

private async handleCreate (tx: TxCUD<Space>): Promise<void> {
Expand Down Expand Up @@ -340,7 +355,9 @@ export class SpacePermissionsMiddleware extends BaseMiddleware implements Middle
// NOTE: move this checking logic later to be defined in some server plugins?
// so they can contribute checks into the middleware for their custom permissions?
if (tx._class === core.class.TxRemoveDoc) {
await this.needPermission(ctx, targetSpaceId as Ref<TypedSpace>, core.permission.DeleteObject)
if (await this.checkPermission(ctx, targetSpaceId as Ref<TypedSpace>, core.permission.ForbidDeleteObject)) {
this.throwForbidden()
}
}
}
}
15 changes: 6 additions & 9 deletions tests/sanity/tests/recruiting/applications.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ test.describe('Application tests', () => {
await applicationsPage.checkApplicationState(talentName, 'Won')
})

test('Cannot delete an Application w/o permissions', async ({ page }) => {
test('Delete an Application', async ({ page }) => {
const navigationMenuPage = new NavigationMenuPage(page)
await navigationMenuPage.buttonApplications.click()

Expand All @@ -132,16 +132,13 @@ test.describe('Application tests', () => {
await applicationsPage.openApplicationByTalentName(talentName)

const applicationsDetailsPage = new ApplicationsDetailsPage(page)
const applicationId = await applicationsDetailsPage.getApplicationId()

await applicationsDetailsPage.checkCannotDelete()

// const applicationId = await applicationsDetailsPage.getApplicationId()

// await applicationsDetailsPage.deleteEntity()
// expect(page.url()).toContain(applicationId)
await applicationsDetailsPage.deleteEntity()
expect(page.url()).toContain(applicationId)

// await navigationMenuPage.buttonApplications.click()
// await applicationsPage.checkApplicationNotExist(applicationId)
await navigationMenuPage.buttonApplications.click()
await applicationsPage.checkApplicationNotExist(applicationId)
})

test('Change & Save all States', async ({ page }) => {
Expand Down
15 changes: 6 additions & 9 deletions tests/sanity/tests/tracker/attachments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,13 @@ test.describe('Attachments tests', () => {
await issuesPage.checkAttachmentsCount(newIssue.title, '3')
})

// Cannot delete attachments in the popup w/o permissions
await issuesPage.checkCannotDeleteAttachmentToIssue(newIssue.title, 'cat2.jpeg')

// await test.step('Delete attachments in the popup', async () => {
// await issuesPage.deleteAttachmentToIssue(newIssue.title, 'cat2.jpeg')
// await issuesPage.checkAddAttachmentPopupContainsFile(newIssue.title, 'cat.jpeg')
// await issuesPage.checkAddAttachmentPopupContainsFile(newIssue.title, 'cat3.jpeg')
await test.step('Delete attachments in the popup', async () => {
await issuesPage.deleteAttachmentToIssue(newIssue.title, 'cat2.jpeg')
await issuesPage.checkAddAttachmentPopupContainsFile(newIssue.title, 'cat.jpeg')
await issuesPage.checkAddAttachmentPopupContainsFile(newIssue.title, 'cat3.jpeg')

// await issuesPage.checkAttachmentsCount(newIssue.title, '2')
// })
await issuesPage.checkAttachmentsCount(newIssue.title, '2')
})

await issuesPage.openIssueByName(newIssue.title)

Expand Down
9 changes: 4 additions & 5 deletions tests/sanity/tests/tracker/component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ test.describe('Tracker component tests', () => {
await componentsDetailsPage.checkComponent(editComponent)
})

test('Cannot delete a component w/o permissions', async ({ page }) => {
test('Delete a component', async ({ page }) => {
const newComponent: NewComponent = {
name: 'Delete component test',
description: 'Delete component test description'
Expand All @@ -90,11 +90,10 @@ test.describe('Tracker component tests', () => {
await componentsPage.openComponentByName(newComponent.name)

const componentsDetailsPage = new ComponentsDetailsPage(page)
await componentsDetailsPage.checkActionMissing('Delete')

// await componentsDetailsPage.checkComponent(newComponent)
// await componentsDetailsPage.deleteComponent()
await componentsDetailsPage.checkComponent(newComponent)
await componentsDetailsPage.deleteComponent()

// await componentsPage.checkComponentNotExist(newComponent.name)
await componentsPage.checkComponentNotExist(newComponent.name)
})
})
12 changes: 5 additions & 7 deletions tests/sanity/tests/tracker/issues.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ test.describe('Tracker issue tests', () => {
await issuesDetailsPage.checkIssue(newIssue)
})

test('Cannot delete an issue w/o permissions', async ({ page }) => {
test('Delete an issue', async ({ page }) => {
const deleteIssue: NewIssue = {
title: 'Issue for deletion',
description: 'Description Issue for deletion'
Expand All @@ -263,13 +263,11 @@ test.describe('Tracker issue tests', () => {
const issuesDetailsPage = new IssuesDetailsPage(page)
await issuesDetailsPage.waitDetailsOpened(deleteIssue.title)

await issuesDetailsPage.checkActionMissing('Delete')
await issuesDetailsPage.moreActionOnIssue('Delete')
await issuesDetailsPage.pressYesForPopup(page)

// await issuesDetailsPage.moreActionOnIssue('Delete')
// await issuesDetailsPage.pressYesForPopup(page)

// await issuesPage.searchIssueByName(deleteIssue.title)
// await issuesPage.checkIssueNotExist(deleteIssue.title)
await issuesPage.searchIssueByName(deleteIssue.title)
await issuesPage.checkIssueNotExist(deleteIssue.title)
})

test('Check the changed description activity', async ({ page }) => {
Expand Down
7 changes: 3 additions & 4 deletions tests/sanity/tests/tracker/milestone.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ test.describe('Tracker milestone tests', () => {
await milestonesDetailsPage.checkActivityExist('changed description at')
})

test('Cannot delete a Milestone w/o permissions', async ({ page }) => {
test('Delete a Milestone', async ({ page }) => {
const deleteMilestone: NewMilestone = {
name: 'Delete Milestone',
description: 'Delete Milestone Description',
Expand All @@ -85,9 +85,8 @@ test.describe('Tracker milestone tests', () => {

const milestonesDetailsPage = new MilestonesDetailsPage(page)
await milestonesDetailsPage.checkIssue(deleteMilestone)
await milestonesDetailsPage.checkActionMissing('Delete')
// await milestonesDetailsPage.deleteMilestone()
await milestonesDetailsPage.deleteMilestone()

// await milestonesPage.checkMilestoneNotExist(deleteMilestone.name)
await milestonesPage.checkMilestoneNotExist(deleteMilestone.name)
})
})
12 changes: 5 additions & 7 deletions tests/sanity/tests/tracker/subissues.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ test.describe('Tracker sub-issues tests', () => {
})
})

test('Cannot delete a sub-issue w/o permissions', async ({ page }) => {
test('Delete a sub-issue', async ({ page }) => {
const deleteIssue: NewIssue = {
title: `Issue for the delete sub-issue-${generateId()}`,
description: 'Description Issue for the delete sub-issue'
Expand Down Expand Up @@ -133,13 +133,11 @@ test.describe('Tracker sub-issues tests', () => {
parentIssue: deleteIssue.title
})

await issuesDetailsPage.checkActionMissing('Delete')
await issuesDetailsPage.moreActionOnIssue('Delete')
await issuesDetailsPage.pressYesForPopup(page)

// await issuesDetailsPage.moreActionOnIssue('Delete')
// await issuesDetailsPage.pressYesForPopup(page)

// await issuesPage.searchIssueByName(deleteSubIssue.title)
// await issuesPage.checkIssueNotExist(deleteSubIssue.title)
await issuesPage.searchIssueByName(deleteSubIssue.title)
await issuesPage.checkIssueNotExist(deleteSubIssue.title)
})

test('Create sub-issue from template', async ({ page }) => {
Expand Down
11 changes: 5 additions & 6 deletions tests/sanity/tests/tracker/template.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ test.describe('Tracker template tests', () => {
}
})

test('Cannot delete a Template w/o permissions', async ({ page }) => {
test('Delete a Template', async ({ page }) => {
const deleteTemplate: NewIssue = {
title: `Template for delete-${generateId()}`,
description: 'Created template for delete'
Expand All @@ -122,16 +122,15 @@ test.describe('Tracker template tests', () => {
const trackerNavigationMenuPage = new TrackerNavigationMenuPage(page)
await trackerNavigationMenuPage.openTemplateForProject('Default')

const templatePage = new TemplatePage(page)
let templatePage = new TemplatePage(page)
await templatePage.createNewTemplate(deleteTemplate)
await templatePage.openTemplate(deleteTemplate.title)

const templateDetailsPage = new TemplateDetailsPage(page)
await templateDetailsPage.checkActionMissing('Delete')

// await templateDetailsPage.deleteTemplate()
await templateDetailsPage.deleteTemplate()

// templatePage = new TemplatePage(page)
// await templatePage.checkTemplateNotExist(deleteTemplate.title)
templatePage = new TemplatePage(page)
await templatePage.checkTemplateNotExist(deleteTemplate.title)
})
})