Skip to content

Commit 0acb267

Browse files
[8.19] [Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them (#226013) (#226424)
# Backport This will backport the following commits from `main` to `8.19`: - [[Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them (#226013)](#226013) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Ievgen Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-07-03T13:31:27Z","message":"[Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them (#226013)\n\n## Summary\n\nWith these changes we make sure that we do an access control checks\nbefore the user tries to perform operations on attack discovery alerts\nindices:\n\n- `.alerts-security.attack.discovery.alerts`\n- `.internal.alerts-security.attack.discovery.alerts`\n- `.adhoc.alerts-security.attack.discovery.alerts`\n- `.internal.adhoc.alerts-security.attack.discovery.alerts`\n\nTo be able to fetch alerts use has to have `read` elastic search\nprivileges on the above indices. To modify (create, update, delete)\nalerts user has to have `write` and `maintenance` (for `refresh: true`\noperations; we use it to bulk update alerts) privileges for those\nindices.\n\nAlso, as part of this PR, I realized that we would use `internal user`\n(which is a `kibana_system` user behind the scene) to perform some of\nthe operations (`findAttackDiscoveryAlerts`, `getAlertConnectorNames`\nand `bulkUpdateAttackDiscoveryAlerts`) which should be performed with\nthe current user rights instead. Those are fixed and we scope elastic\nsearch operations to current user.\n\n### Testing\n\n#### 1. Create a user and assign a role `with NO` index privileges for\nthe above indices\n\n**Expected**:\n* User should not be able to see any existing attack discovery alerts\n* User should not be able to generate attack discoveries\n\n#### 2. Create a user and assign a role with `read` index privileges for\nthe above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries via \"Generate\"\nbutton\n\n#### 3. Create a user and assign a role with `write` index privileges\nfor the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries because the\n`maintenance` privilege is missing\n\n#### 4. Create a user and assign a role with `write` and `maintenance`\nindex privileges for the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should be able to generate attack discoveries\n\n## NOTES\n\nThe feature is hidden behind the feature flag (in `kibana.dev.yml`):\n\n```\nfeature_flags.overrides:\n securitySolution.attackDiscoveryAlertsEnabled: true\n securitySolution.assistantAttackDiscoverySchedulingEnabled: true\n```","sha":"d6eb939e00d4ee7400574a69f05c5375605d5099","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team: SecuritySolution","Team:Security Generative AI","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them","number":226013,"url":"https://github.com/elastic/kibana/pull/226013","mergeCommit":{"message":"[Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them (#226013)\n\n## Summary\n\nWith these changes we make sure that we do an access control checks\nbefore the user tries to perform operations on attack discovery alerts\nindices:\n\n- `.alerts-security.attack.discovery.alerts`\n- `.internal.alerts-security.attack.discovery.alerts`\n- `.adhoc.alerts-security.attack.discovery.alerts`\n- `.internal.adhoc.alerts-security.attack.discovery.alerts`\n\nTo be able to fetch alerts use has to have `read` elastic search\nprivileges on the above indices. To modify (create, update, delete)\nalerts user has to have `write` and `maintenance` (for `refresh: true`\noperations; we use it to bulk update alerts) privileges for those\nindices.\n\nAlso, as part of this PR, I realized that we would use `internal user`\n(which is a `kibana_system` user behind the scene) to perform some of\nthe operations (`findAttackDiscoveryAlerts`, `getAlertConnectorNames`\nand `bulkUpdateAttackDiscoveryAlerts`) which should be performed with\nthe current user rights instead. Those are fixed and we scope elastic\nsearch operations to current user.\n\n### Testing\n\n#### 1. Create a user and assign a role `with NO` index privileges for\nthe above indices\n\n**Expected**:\n* User should not be able to see any existing attack discovery alerts\n* User should not be able to generate attack discoveries\n\n#### 2. Create a user and assign a role with `read` index privileges for\nthe above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries via \"Generate\"\nbutton\n\n#### 3. Create a user and assign a role with `write` index privileges\nfor the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries because the\n`maintenance` privilege is missing\n\n#### 4. Create a user and assign a role with `write` and `maintenance`\nindex privileges for the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should be able to generate attack discoveries\n\n## NOTES\n\nThe feature is hidden behind the feature flag (in `kibana.dev.yml`):\n\n```\nfeature_flags.overrides:\n securitySolution.attackDiscoveryAlertsEnabled: true\n securitySolution.assistantAttackDiscoverySchedulingEnabled: true\n```","sha":"d6eb939e00d4ee7400574a69f05c5375605d5099"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/226013","number":226013,"mergeCommit":{"message":"[Attack Discovery][Scheduling] RBAC: Make sure we check Index privileges before we allow user to access them (#226013)\n\n## Summary\n\nWith these changes we make sure that we do an access control checks\nbefore the user tries to perform operations on attack discovery alerts\nindices:\n\n- `.alerts-security.attack.discovery.alerts`\n- `.internal.alerts-security.attack.discovery.alerts`\n- `.adhoc.alerts-security.attack.discovery.alerts`\n- `.internal.adhoc.alerts-security.attack.discovery.alerts`\n\nTo be able to fetch alerts use has to have `read` elastic search\nprivileges on the above indices. To modify (create, update, delete)\nalerts user has to have `write` and `maintenance` (for `refresh: true`\noperations; we use it to bulk update alerts) privileges for those\nindices.\n\nAlso, as part of this PR, I realized that we would use `internal user`\n(which is a `kibana_system` user behind the scene) to perform some of\nthe operations (`findAttackDiscoveryAlerts`, `getAlertConnectorNames`\nand `bulkUpdateAttackDiscoveryAlerts`) which should be performed with\nthe current user rights instead. Those are fixed and we scope elastic\nsearch operations to current user.\n\n### Testing\n\n#### 1. Create a user and assign a role `with NO` index privileges for\nthe above indices\n\n**Expected**:\n* User should not be able to see any existing attack discovery alerts\n* User should not be able to generate attack discoveries\n\n#### 2. Create a user and assign a role with `read` index privileges for\nthe above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries via \"Generate\"\nbutton\n\n#### 3. Create a user and assign a role with `write` index privileges\nfor the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should not be able to generate attack discoveries because the\n`maintenance` privilege is missing\n\n#### 4. Create a user and assign a role with `write` and `maintenance`\nindex privileges for the above indices\n\n**Expected**:\n* User should be able to see existing attack discovery alerts\n* User should be able to generate attack discoveries\n\n## NOTES\n\nThe feature is hidden behind the feature flag (in `kibana.dev.yml`):\n\n```\nfeature_flags.overrides:\n securitySolution.attackDiscoveryAlertsEnabled: true\n securitySolution.assistantAttackDiscoverySchedulingEnabled: true\n```","sha":"d6eb939e00d4ee7400574a69f05c5375605d5099"}}]}] BACKPORT--> Co-authored-by: Ievgen Sorokopud <[email protected]>
1 parent 1ec84ca commit 0acb267

File tree

16 files changed

+420
-50
lines changed

16 files changed

+420
-50
lines changed

x-pack/solutions/security/plugins/elastic_assistant/server/__mocks__/request_context.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export const createMockClients = () => {
7777
type MockClients = ReturnType<typeof createMockClients>;
7878

7979
export type ElasticAssistantRequestHandlerContextMock = MockedKeys<
80-
AwaitedProperties<Omit<ElasticAssistantRequestHandlerContext, 'resolve'>>
80+
AwaitedProperties<ElasticAssistantRequestHandlerContext>
8181
> & {
8282
core: MockClients['core'];
8383
};
@@ -94,6 +94,7 @@ const createRequestContextMock = (
9494
core: clients.core,
9595
elasticAssistant: createElasticAssistantRequestContextMock(clients),
9696
licensing: licensingMock.createRequestHandlerContext({ license }),
97+
resolve: jest.fn(),
9798
};
9899
};
99100

@@ -171,6 +172,7 @@ const createElasticAssistantRequestContextMock = (
171172
core: clients.core,
172173
savedObjectsClient: clients.elasticAssistant.savedObjectsClient,
173174
telemetry: clients.elasticAssistant.telemetry,
175+
checkPrivileges: jest.fn(),
174176
};
175177
};
176178

x-pack/solutions/security/plugins/elastic_assistant/server/__mocks__/test_adapters.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ const buildResponses = (method: Method, calls: MockCall[]): ResponseCall[] => {
4545
}));
4646
case 'notFound':
4747
return calls.map(() => ({ status: 404, body: undefined }));
48+
case 'forbidden':
49+
return calls.map(([call]) => ({ status: 403, body: call.body }));
4850
default:
4951
throw new Error(`Encountered unexpected call to response.${method}`);
5052
}

x-pack/solutions/security/plugins/elastic_assistant/server/ai_assistant_data_clients/index.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ export class AIAssistantDataClient {
100100
sortOrder,
101101
filter,
102102
fields,
103-
index,
104103
aggs,
105104
mSearch,
106105
}: {
@@ -110,7 +109,6 @@ export class AIAssistantDataClient {
110109
sortOrder?: string;
111110
filter?: string;
112111
fields?: string[];
113-
index?: string;
114112
aggs?: Record<string, estypes.AggregationsAggregationContainer>;
115113
mSearch?: {
116114
filter: string;
@@ -125,7 +123,7 @@ export class AIAssistantDataClient {
125123
perPage,
126124
filter,
127125
sortField,
128-
index: index ?? this.indexTemplateAndPattern.alias,
126+
index: this.indexTemplateAndPattern.alias,
129127
sortOrder: sortOrder as estypes.SortOrder,
130128
logger: this.options.logger,
131129
aggs,

x-pack/solutions/security/plugins/elastic_assistant/server/lib/attack_discovery/persistence/index.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* 2.0.
66
*/
77

8+
import type { estypes } from '@elastic/elasticsearch';
89
import {
910
type AttackDiscoveryAlert,
1011
type AttackDiscoveryCreateProps,
@@ -17,13 +18,14 @@ import {
1718
type PostAttackDiscoveryGenerationsDismissResponse,
1819
} from '@kbn/elastic-assistant-common';
1920
import { AuthenticatedUser } from '@kbn/core-security-common';
20-
import type { Logger } from '@kbn/core/server';
21+
import type { ElasticsearchClient, Logger } from '@kbn/core/server';
2122
import { IRuleDataClient } from '@kbn/rule-registry-plugin/server';
2223

2324
import {
2425
AIAssistantDataClient,
2526
AIAssistantDataClientParams,
2627
} from '../../../ai_assistant_data_clients';
28+
import { findDocuments } from '../../../ai_assistant_data_clients/find';
2729
import { findAllAttackDiscoveries } from './find_all_attack_discoveries/find_all_attack_discoveries';
2830
import { combineFindAttackDiscoveryFilters } from './combine_find_attack_discovery_filters';
2931
import { findAttackDiscoveryByConnectorId } from './find_attack_discovery_by_connector_id/find_attack_discovery_by_connector_id';
@@ -145,6 +147,7 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
145147
alertIds,
146148
authenticatedUser,
147149
end,
150+
esClient,
148151
ids,
149152
index,
150153
logger,
@@ -157,6 +160,7 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
157160
alertIds: string[] | undefined;
158161
authenticatedUser: AuthenticatedUser;
159162
end: string | undefined;
163+
esClient: ElasticsearchClient;
160164
ids: string[] | undefined;
161165
index: string;
162166
logger: Logger;
@@ -182,14 +186,16 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
182186
filter: connectorsAggsFilter,
183187
});
184188

185-
const aggsResult = await this.findDocuments<AttackDiscoveryAlertDocument>({
189+
const aggsResult = await findDocuments<AttackDiscoveryAlertDocument>({
186190
aggs,
191+
esClient,
187192
filter: combinedConnectorsAggsFilter,
188193
index,
194+
logger,
189195
page,
190196
perPage,
191197
sortField,
192-
sortOrder,
198+
sortOrder: sortOrder as estypes.SortOrder,
193199
});
194200

195201
const { connectorNames } = transformSearchResponseToAlerts({
@@ -202,10 +208,12 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
202208

203209
public findAttackDiscoveryAlerts = async ({
204210
authenticatedUser,
211+
esClient,
205212
findAttackDiscoveryAlertsParams,
206213
logger,
207214
}: {
208215
authenticatedUser: AuthenticatedUser;
216+
esClient: ElasticsearchClient;
209217
findAttackDiscoveryAlertsParams: FindAttackDiscoveryAlertsParams;
210218
logger: Logger;
211219
}): Promise<AttackDiscoveryFindResponse> => {
@@ -243,14 +251,16 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
243251
shared,
244252
});
245253

246-
const result = await this.findDocuments<AttackDiscoveryAlertDocument>({
254+
const result = await findDocuments<AttackDiscoveryAlertDocument>({
247255
aggs,
256+
esClient,
248257
filter: combinedFilter,
249258
index,
259+
logger,
250260
page,
251261
perPage,
252262
sortField,
253-
sortOrder,
263+
sortOrder: sortOrder as estypes.SortOrder,
254264
});
255265

256266
const { data, uniqueAlertIdsCount } = transformSearchResponseToAlerts({
@@ -262,6 +272,7 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
262272
alertIds,
263273
authenticatedUser,
264274
end,
275+
esClient,
265276
ids,
266277
index,
267278
logger,
@@ -334,21 +345,21 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
334345

335346
public bulkUpdateAttackDiscoveryAlerts = async ({
336347
authenticatedUser,
348+
esClient,
337349
ids,
338350
kibanaAlertWorkflowStatus,
339351
logger,
340352
visibility,
341353
}: {
342354
authenticatedUser: AuthenticatedUser;
355+
esClient: ElasticsearchClient;
343356
ids: string[];
344357
kibanaAlertWorkflowStatus?: 'acknowledged' | 'closed' | 'open';
345358
logger: Logger;
346359
visibility?: 'not_shared' | 'shared';
347360
}): Promise<AttackDiscoveryAlert[]> => {
348361
const PER_PAGE = 1000;
349362

350-
const esClient = await this.options.elasticsearchClientPromise;
351-
352363
const indexPattern = this.getScheduledAndAdHocIndexPattern();
353364

354365
if (ids.length === 0) {
@@ -403,6 +414,7 @@ export class AttackDiscoveryDataClient extends AIAssistantDataClient {
403414

404415
const alertsResult = await this.findAttackDiscoveryAlerts({
405416
authenticatedUser,
417+
esClient,
406418
findAttackDiscoveryAlertsParams: {
407419
ids,
408420
page: FIRST_PAGE,

x-pack/solutions/security/plugins/elastic_assistant/server/routes/attack_discovery/get/find_attack_discoveries.test.ts

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,49 +7,45 @@
77

88
import type { KibanaRequest } from '@kbn/core-http-server';
99
import { httpServiceMock, httpServerMock } from '@kbn/core-http-server-mocks';
10-
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
1110

1211
import { findAttackDiscoveriesRoute } from './find_attack_discoveries';
1312
import * as helpers from '../../helpers';
13+
import { hasReadAttackDiscoveryAlertsPrivileges } from '../helpers/index_privileges';
1414
import { getMockAttackDiscoveryFindResponse } from '../../../__mocks__/attack_discovery_find_response';
1515
import { mockAuthenticatedUser } from '../../../__mocks__/mock_authenticated_user';
16+
import { requestContextMock } from '../../../__mocks__/request_context';
17+
import { AttackDiscoveryDataClient } from '../../../lib/attack_discovery/persistence';
1618

1719
const mockAttackDiscoveryFindResponse = getMockAttackDiscoveryFindResponse();
1820

21+
jest.mock('../helpers/index_privileges', () => {
22+
const original = jest.requireActual('../helpers/index_privileges');
23+
24+
return {
25+
...original,
26+
hasReadAttackDiscoveryAlertsPrivileges: jest.fn(),
27+
};
28+
});
29+
30+
const { context: mockContext } = requestContextMock.createTools();
31+
1932
describe('findAttackDiscoveriesRoute', () => {
2033
let router: ReturnType<typeof httpServiceMock.createRouter>;
21-
let mockContext: {
22-
resolve: jest.Mock;
23-
elasticAssistant: Promise<{
24-
logger: ReturnType<typeof loggingSystemMock.createLogger>;
25-
eventLogIndex: string;
26-
getSpaceId: () => string;
27-
getAttackDiscoveryDataClient: jest.Mock;
28-
}>;
29-
};
3034
let mockRequest: Partial<KibanaRequest<unknown, unknown, unknown>>;
3135
let mockResponse: ReturnType<typeof httpServerMock.createResponseFactory>;
3236
let mockDataClient: { findAttackDiscoveryAlerts: jest.Mock };
33-
let mockLogger: ReturnType<typeof loggingSystemMock.createLogger>;
3437
let addVersionMock: jest.Mock;
3538
let getHandler: (ctx: unknown, req: unknown, res: unknown) => Promise<unknown>;
3639

3740
beforeEach(() => {
3841
jest.clearAllMocks();
3942
router = httpServiceMock.createRouter();
40-
mockLogger = loggingSystemMock.createLogger();
4143
mockDataClient = {
4244
findAttackDiscoveryAlerts: jest.fn().mockResolvedValue(mockAttackDiscoveryFindResponse),
4345
};
44-
mockContext = {
45-
resolve: jest.fn().mockResolvedValue({ core: {}, elasticAssistant: {}, licensing: {} }),
46-
elasticAssistant: Promise.resolve({
47-
logger: mockLogger,
48-
eventLogIndex: 'event-log-index',
49-
getSpaceId: () => 'default',
50-
getAttackDiscoveryDataClient: jest.fn().mockResolvedValue(mockDataClient),
51-
}),
52-
};
46+
mockContext.elasticAssistant.getAttackDiscoveryDataClient.mockResolvedValue(
47+
mockDataClient as unknown as AttackDiscoveryDataClient
48+
);
5349
mockRequest = {
5450
query: { page: 1, per_page: 10 },
5551
};
@@ -62,6 +58,9 @@ describe('findAttackDiscoveriesRoute', () => {
6258
(router.versioned.get as jest.Mock).mockReturnValue({ addVersion: addVersionMock });
6359
findAttackDiscoveriesRoute(router);
6460
getHandler = addVersionMock.mock.calls[0][1];
61+
(hasReadAttackDiscoveryAlertsPrivileges as jest.Mock).mockResolvedValue({
62+
isSuccess: true,
63+
});
6564
});
6665

6766
it('returns 200 and the expected discoveries on success', async () => {
@@ -100,6 +99,19 @@ describe('findAttackDiscoveriesRoute', () => {
10099
expect(result).toEqual({ status: 403, payload: { message: 'Forbidden' } });
101100
});
102101

102+
it('returns an error when hasReadAttackDiscoveryAlertsPrivileges fails', async () => {
103+
(hasReadAttackDiscoveryAlertsPrivileges as jest.Mock).mockImplementation(({ response }) => {
104+
return Promise.resolve({
105+
isSuccess: false,
106+
response: { status: 403, payload: { message: 'no privileges' } },
107+
});
108+
});
109+
110+
const result = await getHandler(mockContext, mockRequest, mockResponse);
111+
112+
expect(result).toEqual({ status: 403, payload: { message: 'no privileges' } });
113+
});
114+
103115
describe('when data client throws', () => {
104116
const thrownError = new Error('fail!');
105117

x-pack/solutions/security/plugins/elastic_assistant/server/routes/attack_discovery/get/find_attack_discoveries.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { buildRouteValidationWithZod } from '@kbn/zod-helpers';
1818
import { performChecks } from '../../helpers';
1919
import { buildResponse } from '../../../lib/build_response';
2020
import { ElasticAssistantRequestHandlerContext } from '../../../types';
21+
import { hasReadAttackDiscoveryAlertsPrivileges } from '../helpers/index_privileges';
2122

2223
export const findAttackDiscoveriesRoute = (
2324
router: IRouter<ElasticAssistantRequestHandlerContext>
@@ -65,6 +66,15 @@ export const findAttackDiscoveriesRoute = (
6566
return checkResponse.response;
6667
}
6768

69+
// Perform alerts access check
70+
const privilegesCheckResponse = await hasReadAttackDiscoveryAlertsPrivileges({
71+
context: ctx,
72+
response,
73+
});
74+
if (!privilegesCheckResponse.isSuccess) {
75+
return privilegesCheckResponse.response;
76+
}
77+
6878
try {
6979
const { query } = request;
7080
const dataClient = await assistantContext.getAttackDiscoveryDataClient();
@@ -78,8 +88,12 @@ export const findAttackDiscoveriesRoute = (
7888

7989
const currentUser = await checkResponse.currentUser;
8090

91+
// get an Elasticsearch client for the authenticated user:
92+
const esClient = (await context.core).elasticsearch.client.asCurrentUser;
93+
8194
const result = await dataClient.findAttackDiscoveryAlerts({
8295
authenticatedUser: currentUser,
96+
esClient,
8397
findAttackDiscoveryAlertsParams: {
8498
alertIds: query.alert_ids,
8599
ids: query.ids,

0 commit comments

Comments
 (0)