Qfix: invert delete object permission (#5085)

Signed-off-by: Alexey Zinoviev <alexey.zinoviev@xored.com>

models/board/src/index.ts

@@ -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

models/core/src/permissions.ts

@@ -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
+  )
 }

models/document/src/index.ts

@@ -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
   )

models/lead/src/index.ts

@@ -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

models/recruit/src/index.ts

@@ -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

models/tracker/src/index.ts

@@ -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]
     },

packages/core/lang/en.json

@@ -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"
   }
 }

packages/core/lang/ru.json

@@ -47,6 +47,8 @@
     "CreateObject": "Создавать объект",
     "UpdateObject": "Обновлять объект",
     "DeleteObject": "Удалять объект",
-    "DeleteObjectDescription": "Дает пользователям разрешение удалять объекты в пространстве"
+    "DeleteObjectDescription": "Дает пользователям разрешение удалять объекты в пространстве",
+    "ForbidDeleteObject": "Запретить удалять объект",
+    "ForbidDeleteObjectDescription": "Запрещает пользователям удалять объекты в пространстве"
   }
 }

packages/core/src/component.ts

@@ -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>
   }
 })

plugins/attachment-resources/src/components/AttachmentPresenter.svelte

@@ -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('.')

plugins/view-resources/src/visibilityTester.ts

@@ -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)
 }

server/middleware/src/spacePermissions.ts

@@ -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> {
@@ -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()
+      }
     }
   }
 }

tests/sanity/tests/recruiting/applications.spec.ts

@@ -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()
 
@@ -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 }) => {

tests/sanity/tests/tracker/attachments.spec.ts

@@ -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)
 

tests/sanity/tests/tracker/component.spec.ts

@@ -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'
@@ -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)
   })
 })

tests/sanity/tests/tracker/issues.spec.ts

@@ -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'
@@ -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 }) => {

tests/sanity/tests/tracker/milestone.spec.ts

@@ -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',
@@ -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)
   })
 })

tests/sanity/tests/tracker/subissues.spec.ts

@@ -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'
@@ -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 }) => {

tests/sanity/tests/tracker/template.spec.ts

@@ -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'
@@ -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)
   })
 })