github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[8.17] [Cases] Fix an issue with the reopen case permission, add integration tests for failing case (#201517) (#201554)

Browse source 
# 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)](https://github.com/elastic/kibana/pull/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":"56408403+kqualters-elastic@users.noreply.github.com"},"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/elastic/kibana/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/elastic/kibana/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/elastic/kibana/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 <56408403+kqualters-elastic@users.noreply.github.com>
This commit is contained in:
Kibana Machine 2024-11-26 00:09:49 +11:00 • committed by GitHub
parent b7ddad93a2
commit ece4c0917e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 328 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

91
x-pack/plugins/cases/server/client/cases/bulk_update.test.ts
View file

@ -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"`
);
});
});
});
});

1
x-pack/plugins/cases/server/client/cases/bulk_update.ts
View file

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

156
x-pack/test/api_integration/apis/cases/common/roles.ts
View file

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

48
x-pack/test/api_integration/apis/cases/common/users.ts
View file

@ -31,6 +31,12 @@ import {
secReadCasesAll,
secReadCasesNone,
secReadCasesRead,
casesV2NoReopenWithCreateComment,
obsCasesV2NoReopenWithCreateComment,
secCasesV2NoReopenWithCreateComment,
secCasesV2NoCreateCommentWithReopen,
casesV2NoCreateCommentWithReopen,
obsCasesV2NoCreateCommentWithReopen,
} from './roles';
/**
@ -67,6 +73,18 @@ export const secCasesV2AllUser: User = {
roles: [secCasesV2All.name],
};
export const secCasesV2NoReopenWithCreateCommentUser: User = {
username: 'sec_cases_v2_no_reopen_with_create_comment_user_api_int',
password: 'password',
roles: [secCasesV2NoReopenWithCreateComment.name],
};
export const secCasesV2NoCreateCommentWithReopenUser: User = {
username: 'sec_cases_v2_no_create_comment_with_reopen_user_api_int',
password: 'password',
roles: [secCasesV2NoCreateCommentWithReopen.name],
};
export const secAllSpace1User: User = {
username: 'sec_all_space1_user_api_int',
password: 'password',
@ -143,6 +161,18 @@ export const casesV2AllUser: User = {
roles: [casesV2All.name],
};
export const casesV2NoReopenWithCreateCommentUser: User = {
username: 'cases_v2_no_reopen_with_create_comment_user_api_int',
password: 'password',
roles: [casesV2NoReopenWithCreateComment.name],
};
export const casesV2NoCreateCommentWithReopenUser: User = {
username: 'cases_v2_no_create_comment_with_reopen_user_api_int',
password: 'password',
roles: [casesV2NoCreateCommentWithReopen.name],
};
export const casesReadUser: User = {
username: 'cases_read_user_api_int',
password: 'password',
@ -183,6 +213,18 @@ export const obsCasesV2AllUser: User = {
roles: [obsCasesV2All.name],
};
export const obsCasesV2NoReopenWithCreateCommentUser: User = {
username: 'obs_cases_v2_no_reopen_with_create_comment_user_api_int',
password: 'password',
roles: [obsCasesV2NoReopenWithCreateComment.name],
};
export const obsCasesV2NoCreateCommentWithReopenUser: User = {
username: 'obs_cases_v2_no_create_comment_with_reopen_user_api_int',
password: 'password',
roles: [obsCasesV2NoCreateCommentWithReopen.name],
};
export const obsCasesReadUser: User = {
username: 'obs_cases_read_user_api_int',
password: 'password',
@ -211,6 +253,8 @@ export const users = [
secAllCasesNoDeleteUser,
secAllUser,
secCasesV2AllUser,
secCasesV2NoReopenWithCreateCommentUser,
secCasesV2NoCreateCommentWithReopenUser,
secAllSpace1User,
secAllCasesReadUser,
secAllCasesNoneUser,
@ -223,12 +267,16 @@ export const users = [
casesNoDeleteUser,
casesAllUser,
casesV2AllUser,
casesV2NoReopenWithCreateCommentUser,
casesV2NoCreateCommentWithReopenUser,
casesReadUser,
obsCasesOnlyDeleteUser,
obsCasesOnlyReadDeleteUser,
obsCasesNoDeleteUser,
obsCasesAllUser,
obsCasesV2AllUser,
obsCasesV2NoReopenWithCreateCommentUser,
obsCasesV2NoCreateCommentWithReopenUser,
obsCasesReadUser,
obsSecCasesAllUser,
obsSecCasesReadUser,

65
x-pack/test/api_integration/apis/cases/privileges.ts
View file

@ -40,6 +40,12 @@ import {
secReadCasesNoneUser,
secReadCasesReadUser,
secReadUser,
casesV2NoReopenWithCreateCommentUser,
casesV2NoCreateCommentWithReopenUser,
obsCasesV2NoReopenWithCreateCommentUser,
obsCasesV2NoCreateCommentWithReopenUser,
secCasesV2NoReopenWithCreateCommentUser,
secCasesV2NoCreateCommentWithReopenUser,
} from './common/users';
import { getPostCaseRequest } from '../../../cases_api_integration/common/lib/mock';
@ -183,6 +189,9 @@ export default ({ getService }: FtrProviderContext): void => {
{ user: obsCasesV2AllUser, owner: OBSERVABILITY_APP_ID },
{ user: casesAllUser, owner: CASES_APP_ID },
{ user: casesV2AllUser, owner: CASES_APP_ID },
{ user: casesV2NoCreateCommentWithReopenUser, owner: CASES_APP_ID },
{ user: obsCasesV2NoCreateCommentWithReopenUser, owner: OBSERVABILITY_APP_ID },
{ user: secCasesV2NoCreateCommentWithReopenUser, owner: SECURITY_SOLUTION_APP_ID },
]) {
it(`User ${user.username} with role(s) ${user.roles.join()} can reopen a case`, async () => {
const caseInfo = await createCase(supertest, getPostCaseRequest({ owner }));
@ -206,6 +215,35 @@ export default ({ getService }: FtrProviderContext): void => {
});
}
for (const { user, owner } of [
{ user: casesV2NoReopenWithCreateCommentUser, owner: CASES_APP_ID },
{ user: obsCasesV2NoReopenWithCreateCommentUser, owner: OBSERVABILITY_APP_ID },
{ user: secCasesV2NoReopenWithCreateCommentUser, owner: SECURITY_SOLUTION_APP_ID },
]) {
it(`User ${
user.username
} with role(s) ${user.roles.join()} CANNOT reopen a case`, async () => {
const caseInfo = await createCase(supertest, getPostCaseRequest({ owner }));
await updateCaseStatus({
supertest: supertestWithoutAuth,
caseId: caseInfo.id,
status: 'closed' as CaseStatuses,
version: '2',
expectedHttpCode: 200,
auth: { user, space: null },
});
await updateCaseStatus({
supertest: supertestWithoutAuth,
caseId: caseInfo.id,
status: 'open' as CaseStatuses,
version: '3',
expectedHttpCode: 403,
auth: { user, space: null },
});
});
}
for (const { user, owner } of [
{ user: secAllUser, owner: SECURITY_SOLUTION_APP_ID },
{ user: secCasesV2AllUser, owner: SECURITY_SOLUTION_APP_ID },
@ -213,6 +251,9 @@ export default ({ getService }: FtrProviderContext): void => {
{ user: obsCasesV2AllUser, owner: OBSERVABILITY_APP_ID },
{ user: casesAllUser, owner: CASES_APP_ID },
{ user: casesV2AllUser, owner: CASES_APP_ID },
{ user: casesV2NoReopenWithCreateCommentUser, owner: CASES_APP_ID },
{ user: obsCasesV2NoReopenWithCreateCommentUser, owner: OBSERVABILITY_APP_ID },
{ user: secCasesV2NoReopenWithCreateCommentUser, owner: SECURITY_SOLUTION_APP_ID },
]) {
it(`User ${user.username} with role(s) ${user.roles.join()} can add comments`, async () => {
const caseInfo = await createCase(supertest, getPostCaseRequest({ owner }));
@ -230,5 +271,29 @@ export default ({ getService }: FtrProviderContext): void => {
});
});
}
for (const { user, owner } of [
{ user: casesV2NoCreateCommentWithReopenUser, owner: CASES_APP_ID },
{ user: obsCasesV2NoCreateCommentWithReopenUser, owner: OBSERVABILITY_APP_ID },
{ user: secCasesV2NoCreateCommentWithReopenUser, owner: SECURITY_SOLUTION_APP_ID },
]) {
it(`User ${
user.username
} with role(s) ${user.roles.join()} CANNOT add comments`, async () => {
const caseInfo = await createCase(supertest, getPostCaseRequest({ owner }));
const comment: UserCommentAttachmentPayload = {
comment: 'test',
owner,
type: AttachmentType.user,
};
await createComment({
params: comment,
supertest: supertestWithoutAuth,
caseId: caseInfo.id,
expectedHttpCode: 403,
auth: { user, space: null },
});
});
}
});
};