Skip to content

Commit

Permalink
[8.17] [Cases] Fix an issue with the reopen case permission, add inte…
Browse files Browse the repository at this point in the history 
…gration tests for failing case (#201517) (#201554)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Cases] Fix an issue with the reopen case permission, add integration
tests for failing case
(#201517)](#201517)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-25T11:12:43Z","message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","v9.0.0","Team:Threat
Hunting:Investigations","backport:version","v8.17.0"],"title":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing
case","number":201517,"url":"https://github.com/elastic/kibana/pull/201517","mergeCommit":{"message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201517","number":201517,"mergeCommit":{"message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kevin Qualters <[email protected]>
  • Loading branch information
@kibanamachine @kqualters-elastic
kibanamachine and kqualters-elastic authored Nov 25, 2024
1 parent b7ddad9 commit ece4c09
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
View file
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
View file
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
View file
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 ece4c09

Please sign in to comment.