[Cases] Fix an issue with the reopen case permission, add integration…

… tests for failing case (elastic#201517) ## Summary Currently, the partitionPatchRequest in x-pack/plugins/cases/server/client/cases/bulk_update.ts will not check a case properly if a case is being re-opened, instead of an else if in the loop comparing cases to cases in the request, the status logic should be outside of this set of if statements. If a case is being re-opened, the final else is never run, and casesToAuthorize is 0. This results in ensureAuthorized being called with an empty array of entities, and so the check always succeeds. To fix this, reopenedCases is still populated in the same loop, just not when casesToAuthorize logic is running as well. Also adds some missing tests for this and the create comment permission as requested in elastic#194898. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
paulinashakirova · Nov 26, 2024 · 65bd952 · 65bd952
1 parent 0ffe477
commit 65bd952
Show file tree

Hide file tree

Showing 5 changed files with 328 additions and 33 deletions.
diff --git a/x-pack/plugins/cases/server/client/cases/bulk_update.test.ts b/x-pack/plugins/cases/server/client/cases/bulk_update.test.ts
@@ -1512,6 +1512,59 @@ describe('update', () => {
       );
     });
 
+    it('throws an error if the case is not found', async () => {
+      clientArgsMock.services.caseService.getCases.mockResolvedValue({ saved_objects: [] });
+
+      await expect(
+        bulkUpdate(
+          {
+            cases: [
+              {
+                id: mockCases[0].id,
+                version: mockCases[0].version ?? '',
+                status: CaseStatuses.open,
+              },
+            ],
+          },
+          clientArgsMock,
+          casesClientMock
+        )
+      ).rejects.toThrow(
+        'Failed to update case, ids: [{"id":"mock-id-1","version":"WzAsMV0="}]: Error: These cases mock-id-1 do not exist. Please check you have the correct ids.'
+      );
+    });
+
+    it('throws an error if the case is not found and the SO clients returns an SO object', async () => {
+      clientArgsMock.services.caseService.getCases.mockResolvedValue({
+        saved_objects: [
+          {
+            type: 'cases',
+            id: 'mock-id-1',
+            references: [],
+            error: { error: 'Non found', message: 'Non found', statusCode: 404 },
+          },
+        ],
+      });
+
+      await expect(
+        bulkUpdate(
+          {
+            cases: [
+              {
+                id: mockCases[0].id,
+                version: mockCases[0].version ?? '',
+                status: CaseStatuses.open,
+              },
+            ],
+          },
+          clientArgsMock,
+          casesClientMock
+        )
+      ).rejects.toThrow(
+        'Failed to update case, ids: [{"id":"mock-id-1","version":"WzAsMV0="}]: Error: These cases mock-id-1 do not exist. Please check you have the correct ids.'
+      );
+    });
+
     describe('Validate max user actions per page', () => {
       beforeEach(() => {
         jest.clearAllMocks();
@@ -1681,6 +1734,7 @@ describe('update', () => {
             status: CaseStatuses.closed,
           },
         };
+
         clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: [closedCase] });
 
         clientArgs.services.caseService.patchCases.mockResolvedValue({
@@ -1701,7 +1755,10 @@ describe('update', () => {
           casesClientMock
         );
 
-        expect(clientArgs.authorization.ensureAuthorized).not.toThrow();
+        expect(clientArgs.authorization.ensureAuthorized).toHaveBeenCalledWith({
+          entities: [{ id: closedCase.id, owner: closedCase.attributes.owner }],
+          operation: [Operations.reopenCase, Operations.updateCase],
+        });
       });
 
       it('throws when user is not authorized to update case', async () => {
@@ -1726,38 +1783,6 @@ describe('update', () => {
           `"Failed to update case, ids: [{\\"id\\":\\"mock-id-1\\",\\"version\\":\\"WzAsMV0=\\"}]: Error: Unauthorized"`
         );
       });
-
-      it('throws when user is not authorized to reopen case', async () => {
-        const closedCase = {
-          ...mockCases[0],
-          attributes: {
-            ...mockCases[0].attributes,
-            status: CaseStatuses.closed,
-          },
-        };
-        clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: [closedCase] });
-
-        const error = new Error('Unauthorized to reopen case');
-        clientArgs.authorization.ensureAuthorized.mockRejectedValueOnce(error); // Reject reopenCase
-
-        await expect(
-          bulkUpdate(
-            {
-              cases: [
-                {
-                  id: closedCase.id,
-                  version: closedCase.version ?? '',
-                  status: CaseStatuses.open,
-                },
-              ],
-            },
-            clientArgs,
-            casesClientMock
-          )
-        ).rejects.toThrowErrorMatchingInlineSnapshot(
-          `"Failed to update case, ids: [{\\"id\\":\\"mock-id-1\\",\\"version\\":\\"WzAsMV0=\\"}]: Error: Unauthorized to reopen case"`
-        );
-      });
     });
   });
 });
diff --git a/x-pack/plugins/cases/server/client/cases/bulk_update.ts b/x-pack/plugins/cases/server/client/cases/bulk_update.ts
@@ -295,6 +295,7 @@ function partitionPatchRequest(
     ) {
       // Track cases that are closed and a user is attempting to reopen
       reopenedCases.push(reqCase);
+      casesToAuthorize.set(foundCase.id, { id: foundCase.id, owner: foundCase.attributes.owner });
     } else {
       casesToAuthorize.set(foundCase.id, { id: foundCase.id, owner: foundCase.attributes.owner });
     }

diff --git a/x-pack/test/api_integration/apis/cases/common/roles.ts b/x-pack/test/api_integration/apis/cases/common/roles.ts
@@ -136,6 +136,56 @@ export const secCasesV2All: Role = {
   },
 };
 
+export const secCasesV2NoReopenWithCreateComment: Role = {
+  name: 'sec_cases_v2_no_reopen_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        feature: {
+          siem: ['all'],
+          securitySolutionCasesV2: ['read', 'update', 'create', 'delete', 'create_comment'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+        spaces: ['*'],
+      },
+    ],
+  },
+};
+
+export const secCasesV2NoCreateCommentWithReopen: Role = {
+  name: 'sec_cases_v2_create_comment_no_reopen_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        feature: {
+          siem: ['all'],
+          securitySolutionCasesV2: ['read', 'update', 'create', 'delete', 'case_reopen'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+        spaces: ['*'],
+      },
+    ],
+  },
+};
+
 export const secAllSpace1: Role = {
   name: 'sec_all_role_space1_api_int',
   privileges: {
@@ -434,6 +484,56 @@ export const casesV2All: Role = {
   },
 };
 
+export const casesV2NoReopenWithCreateComment: Role = {
+  name: 'cases_v2_no_reopen_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        spaces: ['*'],
+        base: [],
+        feature: {
+          generalCasesV2: ['read', 'update', 'create', 'delete', 'create_comment'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+      },
+    ],
+  },
+};
+
+export const casesV2NoCreateCommentWithReopen: Role = {
+  name: 'cases_v2_no_create_comment_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        spaces: ['*'],
+        base: [],
+        feature: {
+          generalCasesV2: ['read', 'update', 'create', 'delete', 'case_reopen'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+      },
+    ],
+  },
+};
+
 export const casesRead: Role = {
   name: 'cases_read_role_api_int',
   privileges: {
@@ -583,6 +683,56 @@ export const obsCasesV2All: Role = {
   },
 };
 
+export const obsCasesV2NoReopenWithCreateComment: Role = {
+  name: 'obs_cases_v2_no_reopen_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        spaces: ['*'],
+        base: [],
+        feature: {
+          observabilityCasesV2: ['read', 'update', 'create', 'delete', 'create_comment'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+      },
+    ],
+  },
+};
+
+export const obsCasesV2NoCreateCommentWithReopen: Role = {
+  name: 'obs_cases_v2_no_create_comment_role_api_int',
+  privileges: {
+    elasticsearch: {
+      indices: [
+        {
+          names: ['*'],
+          privileges: ['all'],
+        },
+      ],
+    },
+    kibana: [
+      {
+        spaces: ['*'],
+        base: [],
+        feature: {
+          observabilityCasesV2: ['read', 'update', 'create', 'delete', 'case_reopen'],
+          actions: ['all'],
+          actionsSimulators: ['all'],
+        },
+      },
+    ],
+  },
+};
+
 export const obsCasesRead: Role = {
   name: 'obs_cases_read_role_api_int',
   privileges: {
@@ -613,6 +763,8 @@ export const roles = [
   secAllCasesNoDelete,
   secAll,
   secCasesV2All,
+  secCasesV2NoReopenWithCreateComment,
+  secCasesV2NoCreateCommentWithReopen,
   secAllSpace1,
   secAllCasesRead,
   secAllCasesNone,
@@ -625,11 +777,15 @@ export const roles = [
   casesNoDelete,
   casesAll,
   casesV2All,
+  casesV2NoReopenWithCreateComment,
+  casesV2NoCreateCommentWithReopen,
   casesRead,
   obsCasesOnlyDelete,
   obsCasesOnlyReadDelete,
   obsCasesNoDelete,
   obsCasesAll,
   obsCasesV2All,
+  obsCasesV2NoReopenWithCreateComment,
+  obsCasesV2NoCreateCommentWithReopen,
   obsCasesRead,
 ];