Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 25, 2024
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d5b89c
Fix an issue with the reopen case permission, add integration tests f…
kqualters-elastic Nov 25, 2024
44ea3a3
Merge branch 'main' into fix-reopen-case-auth
cnasikas Nov 25, 2024
4d49fe3
Fix bug with SO error objects
cnasikas Nov 25, 2024
53a06e6
PR feedback
cnasikas Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions x-pack/plugins/cases/server/client/cases/bulk_update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,18 @@ function partitionPatchRequest(
conflictedCases.push(reqCase);
// let's try to authorize the conflicted case even though we'll fail after afterwards just in case
casesToAuthorize.set(foundCase.id, { id: foundCase.id, owner: foundCase.attributes.owner });
} else if (
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also update the tests in x-pack/plugins/cases/server/client/cases/bulk_update.test.ts?

I am thinking specifically about 'checks authorization for both reopenCase and updateCase operations when reopening a case'.

This test only checks that expect(clientArgs.authorization.ensureAuthorized).not.toThrow(); but if we had expect(clientArgs.authorization.ensureAuthorized).toHaveBeenCalledWith(...) we would have caught this bug.

Could you please assert it is called with the proper params?

casesToAuthorize.set(foundCase.id, { id: foundCase.id, owner: foundCase.attributes.owner });
}

if (
reqCase.status != null &&
foundCase != null &&
foundCase.attributes.status !== reqCase.status &&
foundCase.attributes.status === CaseStatuses.closed
) {
// Track cases that are closed and a user is attempting to reopen
reopenedCases.push(reqCase);
} 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
Open in desktop
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,
];
48 changes: 48 additions & 0 deletions x-pack/test/api_integration/apis/cases/common/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ import {
secReadCasesAll,
secReadCasesNone,
secReadCasesRead,
casesV2NoReopenWithCreateComment,
obsCasesV2NoReopenWithCreateComment,
secCasesV2NoReopenWithCreateComment,
secCasesV2NoCreateCommentWithReopen,
casesV2NoCreateCommentWithReopen,
obsCasesV2NoCreateCommentWithReopen,
} from './roles';

/**
Expand Down Expand Up @@ -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_create_comment_no_reopen_user_api_int',
password: 'password',
roles: [secCasesV2NoCreateCommentWithReopen.name],
};

export const secAllSpace1User: User = {
username: 'sec_all_space1_user_api_int',
password: 'password',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -211,6 +253,8 @@ export const users = [
secAllCasesNoDeleteUser,
secAllUser,
secCasesV2AllUser,
secCasesV2NoReopenWithCreateCommentUser,
secCasesV2NoCreateCommentWithReopenUser,
secAllSpace1User,
secAllCasesReadUser,
secAllCasesNoneUser,
Expand All @@ -223,12 +267,16 @@ export const users = [
casesNoDeleteUser,
casesAllUser,
casesV2AllUser,
casesV2NoReopenWithCreateCommentUser,
casesV2NoCreateCommentWithReopenUser,
casesReadUser,
obsCasesOnlyDeleteUser,
obsCasesOnlyReadDeleteUser,
obsCasesNoDeleteUser,
obsCasesAllUser,
obsCasesV2AllUser,
obsCasesV2NoReopenWithCreateCommentUser,
obsCasesV2NoCreateCommentWithReopenUser,
obsCasesReadUser,
obsSecCasesAllUser,
obsSecCasesReadUser,
Expand Down
Loading