From ae05f46ea814a03a608dc42d0a73be7eb1fb2327 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 10 Feb 2023 17:01:59 +0200 Subject: [PATCH] PR feedback --- .../server/alert_data_client/alerts_client.ts | 2 +- .../common/lib/alerts.ts | 2 +- .../common/lib/authentication/roles.ts | 19 ++++++++ .../common/lib/authentication/users.ts | 8 ++++ .../tests/common/comments/post_comment.ts | 45 +++++++++++++++++-- .../internal/bulk_create_attachments.ts | 38 +++++++++++++++- 6 files changed, 107 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts index 198fcf75c0c71..2ffe3451ac568 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts @@ -561,7 +561,7 @@ export class AlertsClient { return mgetRes; } catch (exc) { - this.logger.error(`error in ensureAlertsAuthorized ${exc}`); + this.logger.error(`error in ensureAllAlertsAuthorized ${exc}`); throw exc; } } diff --git a/x-pack/test/cases_api_integration/common/lib/alerts.ts b/x-pack/test/cases_api_integration/common/lib/alerts.ts index f5584b61cb934..5aadd7df90949 100644 --- a/x-pack/test/cases_api_integration/common/lib/alerts.ts +++ b/x-pack/test/cases_api_integration/common/lib/alerts.ts @@ -20,8 +20,8 @@ import { getQuerySignalIds, } from '../../../detection_engine_api_integration/utils'; import { superUser } from './authentication/users'; -import { getSpaceUrlPrefix } from './utils'; import { User } from './authentication/types'; +import { getSpaceUrlPrefix } from './api/helpers'; export const createSecuritySolutionAlerts = async ( supertest: SuperTest.SuperTest, diff --git a/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts b/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts index 96262829f90dd..eb11394a0cdf0 100644 --- a/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts +++ b/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts @@ -213,6 +213,24 @@ export const securitySolutionOnlyReadAlerts: Role = { }, }; +export const securitySolutionOnlyReadNoIndexAlerts: Role = { + name: 'sec_only_read_no_index_alerts', + privileges: { + elasticsearch: { + indices: [], + }, + kibana: [ + { + feature: { + securitySolutionFixture: ['all'], + siem: ['read'], + }, + spaces: ['space1'], + }, + ], + }, +}; + export const observabilityOnlyAll: Role = { name: 'obs_only_all', privileges: { @@ -321,4 +339,5 @@ export const roles = [ observabilityOnlyRead, observabilityOnlyReadAlerts, testDisabledPluginAll, + securitySolutionOnlyReadNoIndexAlerts, ]; diff --git a/x-pack/test/cases_api_integration/common/lib/authentication/users.ts b/x-pack/test/cases_api_integration/common/lib/authentication/users.ts index 48e060c926f48..8d997159cdb06 100644 --- a/x-pack/test/cases_api_integration/common/lib/authentication/users.ts +++ b/x-pack/test/cases_api_integration/common/lib/authentication/users.ts @@ -19,6 +19,7 @@ import { securitySolutionOnlyNoDelete, observabilityOnlyReadAlerts, securitySolutionOnlyReadAlerts, + securitySolutionOnlyReadNoIndexAlerts, } from './roles'; import { User } from './types'; @@ -64,6 +65,12 @@ export const secOnlyReadAlerts: User = { roles: [securitySolutionOnlyReadAlerts.name], }; +export const secSolutionOnlyReadNoIndexAlerts: User = { + username: 'sec_only_read_no_index_alerts', + password: 'sec_only_read_no_index_alerts', + roles: [securitySolutionOnlyReadNoIndexAlerts.name], +}; + export const obsOnly: User = { username: 'obs_only', password: 'obs_only', @@ -127,6 +134,7 @@ export const users = [ secOnly, secOnlyRead, secOnlyReadAlerts, + secSolutionOnlyReadNoIndexAlerts, secOnlyDelete, secOnlyNoDelete, obsOnly, diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts index 635b30a383f22..c1512fbc4194c 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts @@ -53,6 +53,7 @@ import { obsOnlyReadAlerts, obsSec, secOnlyReadAlerts, + secSolutionOnlyReadNoIndexAlerts, } from '../../../../common/lib/authentication/users'; import { getSecuritySolutionAlerts, @@ -413,7 +414,7 @@ export default ({ getService }: FtrProviderContext): void => { attachmentAuth?: { user: User; space: string | null }; }) => { const postedCase = await createCase( - supertest, + supertestWithoutAuth, { ...postCaseReq, settings: { syncAlerts }, @@ -423,7 +424,7 @@ export default ({ getService }: FtrProviderContext): void => { ); await updateCase({ - supertest, + supertest: supertestWithoutAuth, params: { cases: [ { @@ -499,7 +500,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should change the status of the alert when the user has read access only', async () => { + it('should change the status of the alert when the user has write access to the indices and only read access to the siem solution', async () => { await bulkCreateAlertsAndVerifyAlertStatus({ syncAlerts: true, expectedAlertStatus: 'acknowledged', @@ -524,6 +525,19 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + it('should NOT change the status of the alert when the user has read access to the kibana feature but no read access to the ES index', async () => { + await bulkCreateAlertsAndVerifyAlertStatus({ + syncAlerts: true, + expectedAlertStatus: 'open', + caseAuth: { + user: superUser, + space: 'space1', + }, + attachmentExpectedHttpCode: 500, + attachmentAuth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + }); + }); + it('should add the case ID to the alert schema', async () => { await bulkCreateAlertsAndVerifyCaseIdsInAlertSchema(1); }); @@ -566,7 +580,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should add the case ID to the alert schema when the user has read access only', async () => { + it('should add the case ID to the alert schema when the user has write access to the indices and only read access to the siem solution', async () => { const postedCase = await createCase( supertest, { @@ -611,6 +625,29 @@ export default ({ getService }: FtrProviderContext): void => { auth: { user: obsSec, space: 'space1' }, }); }); + + it('should add the case ID to the alert schema when the user has read access to the kibana feature but no read access to the ES index', async () => { + const postedCase = await createCase( + supertest, + { + ...postCaseReq, + settings: { syncAlerts: false }, + }, + 200, + { user: superUser, space: 'space1' } + ); + + const signals = await createSecuritySolutionAlerts(supertest, log); + const alert = signals.hits.hits[0]; + + await createCommentAndRefreshIndex({ + caseId: postedCase.id, + alertId: alert._id, + alertIndex: alert._index, + expectedHttpCode: 200, + auth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + }); + }); }); describe('observability', () => { diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts index d346f1351d93b..460c56d08cf27 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts @@ -50,6 +50,7 @@ import { secOnly, secOnlyRead, secOnlyReadAlerts, + secSolutionOnlyReadNoIndexAlerts, superUser, } from '../../../../common/lib/authentication/users'; import { @@ -626,7 +627,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should change the status of the alert when the user has read access only', async () => { + it('should change the status of the alert when the user has write access to the indices and only read access to the siem solution', async () => { await bulkCreateAlertsAndVerifyAlertStatus({ syncAlerts: true, expectedAlertStatus: 'acknowledged', @@ -651,6 +652,19 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + it('should NOT change the status of the alert when the user has read access to the kibana feature but no read access to the ES index', async () => { + await bulkCreateAlertsAndVerifyAlertStatus({ + syncAlerts: true, + expectedAlertStatus: 'open', + caseAuth: { + user: superUser, + space: 'space1', + }, + attachmentExpectedHttpCode: 500, + attachmentAuth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + }); + }); + it('should add the case ID to the alert schema', async () => { await bulkCreateAlertsAndVerifyCaseIdsInAlertSchema(1); }); @@ -734,6 +748,28 @@ export default ({ getService }: FtrProviderContext): void => { auth: { user: obsSec, space: 'space1' }, }); }); + + it('should add the case ID to the alert schema when the user has read access to the kibana feature but no read access to the ES index', async () => { + const postedCase = await createCase( + supertest, + { + ...postCaseReq, + settings: { syncAlerts: false }, + }, + 200, + { user: superUser, space: 'space1' } + ); + + const signals = await createSecuritySolutionAlerts(supertest, log); + const alert = signals.hits.hits[0]; + + await bulkCreateAttachmentsAndRefreshIndex({ + caseId: postedCase.id, + alerts: [{ id: alert._id, index: alert._index }], + expectedHttpCode: 200, + auth: { user: secSolutionOnlyReadNoIndexAlerts, space: 'space1' }, + }); + }); }); describe('observability', () => {