Skip to content

Commit

Permalink
[Cases] Fix an issue with the reopen case permission, add integration…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
kqualters-elastic authored and CAWilson94 committed Dec 12, 2024
1 parent 9d29d45 commit 92d858f
Show file tree
Hide file tree
Showing 5 changed files with 328 additions and 33 deletions.
91 changes: 58 additions & 33 deletions x-pack/plugins/cases/server/client/cases/bulk_update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -1681,6 +1734,7 @@ describe('update', () => {
status: CaseStatuses.closed,
},
};

clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: [closedCase] });

clientArgs.services.caseService.patchCases.mockResolvedValue({
Expand All @@ -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 () => {
Expand All @@ -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"`
);
});
});
});
});
1 change: 1 addition & 0 deletions x-pack/plugins/cases/server/client/cases/bulk_update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down
156 changes: 156 additions & 0 deletions x-pack/test/api_integration/apis/cases/common/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -613,6 +763,8 @@ export const roles = [
secAllCasesNoDelete,
secAll,
secCasesV2All,
secCasesV2NoReopenWithCreateComment,
secCasesV2NoCreateCommentWithReopen,
secAllSpace1,
secAllCasesRead,
secAllCasesNone,
Expand All @@ -625,11 +777,15 @@ export const roles = [
casesNoDelete,
casesAll,
casesV2All,
casesV2NoReopenWithCreateComment,
casesV2NoCreateCommentWithReopen,
casesRead,
obsCasesOnlyDelete,
obsCasesOnlyReadDelete,
obsCasesNoDelete,
obsCasesAll,
obsCasesV2All,
obsCasesV2NoReopenWithCreateComment,
obsCasesV2NoCreateCommentWithReopen,
obsCasesRead,
];
Loading

0 comments on commit 92d858f

Please sign in to comment.