Skip to content

Commit 53379ff

Browse files
kibanamachinesemd
andauthored
[9.0] [Security Solution] Fix bug with redirect and add test (#210188) (#210589)
# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sergi Massaneda","email":"sergi.massaneda@elastic.co"},"sourceCommit":{"committedDate":"2025-02-11T14:20:50Z","message":"[Security Solution] Fix bug with redirect and add test (#210188)\n\n## Summary\r\n\r\nIt fixes a bug caused by the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT being initialized with `[]`\r\nempty array. The hook considers the state is _loading_ while the hook is\r\n`null`.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/dde1bec88bc7a797c21d65305c23ab6c1dc6bed6/x-pack/solutions/security/plugins/security_solution/public/siem_migrations/rules/service/hooks/use_latest_stats.ts#L23-L26\r\n\r\nThe fix consists of initializing the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT observable to `null`\r\ninstead of `[]`.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"dc3b4c8c3113f71b250e9c4a037f8dee507178f0","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat Hunting","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[Security Solution] Fix bug with redirect and add test","number":210188,"url":"https://github.com/elastic/kibana/pull/210188","mergeCommit":{"message":"[Security Solution] Fix bug with redirect and add test (#210188)\n\n## Summary\r\n\r\nIt fixes a bug caused by the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT being initialized with `[]`\r\nempty array. The hook considers the state is _loading_ while the hook is\r\n`null`.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/dde1bec88bc7a797c21d65305c23ab6c1dc6bed6/x-pack/solutions/security/plugins/security_solution/public/siem_migrations/rules/service/hooks/use_latest_stats.ts#L23-L26\r\n\r\nThe fix consists of initializing the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT observable to `null`\r\ninstead of `[]`.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"dc3b4c8c3113f71b250e9c4a037f8dee507178f0"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/210188","number":210188,"mergeCommit":{"message":"[Security Solution] Fix bug with redirect and add test (#210188)\n\n## Summary\r\n\r\nIt fixes a bug caused by the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT being initialized with `[]`\r\nempty array. The hook considers the state is _loading_ while the hook is\r\n`null`.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/dde1bec88bc7a797c21d65305c23ab6c1dc6bed6/x-pack/solutions/security/plugins/security_solution/public/siem_migrations/rules/service/hooks/use_latest_stats.ts#L23-L26\r\n\r\nThe fix consists of initializing the `latestStats# Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution] Fix bug with redirect and add test (#210188)](#210188) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT observable to `null`\r\ninstead of `[]`.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"dc3b4c8c3113f71b250e9c4a037f8dee507178f0"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Sergi Massaneda <sergi.massaneda@elastic.co>
1 parent 11c09a7 commit 53379ff

2 files changed

Lines changed: 339 additions & 4 deletions

File tree

Lines changed: 335 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,335 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
/**
9+
* SiemRulesMigrationsService.test.ts
10+
*
11+
* This file tests the SiemRulesMigrationsService class.
12+
* We use Jest for assertions and mocking. We also use Jest’s fake timers to simulate the polling loop.
13+
*/
14+
15+
import type { CoreStart } from '@kbn/core/public';
16+
import type { TraceOptions } from '@kbn/elastic-assistant/impl/assistant/types';
17+
import { firstValueFrom } from 'rxjs';
18+
import {
19+
createRuleMigration,
20+
upsertMigrationResources,
21+
startRuleMigration as startRuleMigrationAPI,
22+
getRuleMigrationStats,
23+
getRuleMigrationsStatsAll,
24+
getMissingResources,
25+
getIntegrations,
26+
} from '../api';
27+
import type { CreateRuleMigrationRequestBody } from '../../../../common/siem_migrations/model/api/rules/rule_migration.gen';
28+
import { createTelemetryServiceMock } from '../../../common/lib/telemetry/telemetry_service.mock';
29+
import {
30+
SiemMigrationRetryFilter,
31+
SiemMigrationTaskStatus,
32+
} from '../../../../common/siem_migrations/constants';
33+
import type { StartPluginsDependencies } from '../../../types';
34+
import { getMissingCapabilities } from './capabilities';
35+
import * as i18n from './translations';
36+
import {
37+
REQUEST_POLLING_INTERVAL_SECONDS,
38+
SiemRulesMigrationsService,
39+
} from './rule_migrations_service';
40+
41+
// --- Mocks for external modules ---
42+
43+
jest.mock('../api', () => ({
44+
createRuleMigration: jest.fn(),
45+
upsertMigrationResources: jest.fn(),
46+
startRuleMigration: jest.fn(),
47+
getRuleMigrationStats: jest.fn(),
48+
getRuleMigrationsStatsAll: jest.fn(),
49+
getMissingResources: jest.fn(),
50+
getIntegrations: jest.fn(),
51+
}));
52+
53+
jest.mock('./capabilities', () => ({
54+
getMissingCapabilities: jest.fn(() => []),
55+
}));
56+
57+
jest.mock('../../../common/experimental_features_service', () => ({
58+
ExperimentalFeaturesService: {
59+
get: jest.fn(() => ({ siemMigrationsDisabled: false })),
60+
},
61+
}));
62+
63+
jest.mock('../../../common/hooks/use_license', () => ({
64+
licenseService: {
65+
isEnterprise: jest.fn(() => true),
66+
},
67+
}));
68+
69+
jest.mock('./notifications/success_notification', () => ({
70+
getSuccessToast: jest.fn().mockReturnValue({ title: 'Success' }),
71+
}));
72+
73+
jest.mock('./notifications/no_connector_notification', () => ({
74+
getNoConnectorToast: jest.fn().mockReturnValue({ title: 'No Connector' }),
75+
}));
76+
77+
jest.mock('./notifications/missing_capabilities_notification', () => ({
78+
getMissingCapabilitiesToast: jest.fn().mockReturnValue({ title: 'Missing Capabilities' }),
79+
}));
80+
81+
// --- End of mocks ---
82+
83+
describe('SiemRulesMigrationsService', () => {
84+
let service: SiemRulesMigrationsService;
85+
let mockCore: CoreStart;
86+
let mockPlugins: StartPluginsDependencies;
87+
let mockNotifications: CoreStart['notifications'];
88+
const mockTelemetry = createTelemetryServiceMock();
89+
90+
beforeEach(async () => {
91+
jest.clearAllMocks();
92+
93+
// Create a fake notifications object to spy on toast calls
94+
mockNotifications = {
95+
toasts: { add: jest.fn(), addError: jest.fn(), addSuccess: jest.fn() },
96+
} as unknown as CoreStart['notifications'];
97+
98+
// Minimal core stub
99+
mockCore = {
100+
application: { capabilities: {} },
101+
notifications: mockNotifications,
102+
} as CoreStart;
103+
104+
// Minimal plugins stub with spaces.getActiveSpace returning a fake space
105+
mockPlugins = {
106+
spaces: {
107+
getActiveSpace: jest.fn().mockResolvedValue({ id: 'test-space' }),
108+
},
109+
} as unknown as StartPluginsDependencies;
110+
111+
// Ensure getRuleMigrationsStatsAll returns an empty array by default (so polling exits quickly)
112+
(getRuleMigrationsStatsAll as jest.Mock).mockResolvedValue([]);
113+
114+
// Instantiate the service – note that the constructor calls getActiveSpace and startPolling
115+
service = new SiemRulesMigrationsService(mockCore, mockPlugins, mockTelemetry);
116+
// Wait for any async operations in the constructor to complete
117+
await Promise.resolve();
118+
});
119+
120+
describe('latestStats$', () => {
121+
it('should be initialized to null', async () => {
122+
// Instantiate the service – note that the constructor calls getActiveSpace and startPolling
123+
const testService = new SiemRulesMigrationsService(mockCore, mockPlugins, mockTelemetry);
124+
expect(await firstValueFrom(testService.getLatestStats$())).toBeNull();
125+
await Promise.resolve();
126+
});
127+
});
128+
129+
describe('createRuleMigration', () => {
130+
it('should throw an error when body is empty', async () => {
131+
await expect(service.createRuleMigration([])).rejects.toThrow(i18n.EMPTY_RULES_ERROR);
132+
});
133+
134+
it('should create migration with a single batch', async () => {
135+
const body = [{ id: 'rule1' }] as CreateRuleMigrationRequestBody;
136+
(createRuleMigration as jest.Mock).mockResolvedValue({ migration_id: 'mig-1' });
137+
138+
const migrationId = await service.createRuleMigration(body);
139+
140+
expect(createRuleMigration).toHaveBeenCalledTimes(1);
141+
expect(createRuleMigration).toHaveBeenCalledWith({ migrationId: undefined, body });
142+
expect(migrationId).toBe('mig-1');
143+
});
144+
145+
it('should create migration in batches if body length exceeds the batch size', async () => {
146+
// Create an array of 51 items (the service batches in chunks of 50)
147+
const body = new Array(51).fill({ rule: 'rule' });
148+
(createRuleMigration as jest.Mock)
149+
.mockResolvedValueOnce({ migration_id: 'mig-1' })
150+
.mockResolvedValueOnce({ migration_id: 'mig-2' });
151+
152+
const migrationId = await service.createRuleMigration(body);
153+
154+
expect(createRuleMigration).toHaveBeenCalledTimes(2);
155+
// First call: first 50 items, migrationId undefined
156+
expect((createRuleMigration as jest.Mock).mock.calls[0][0]).toEqual({
157+
migrationId: undefined,
158+
body: body.slice(0, 50),
159+
});
160+
// Second call: remaining 1 item, migrationId passed from previous batch
161+
expect((createRuleMigration as jest.Mock).mock.calls[1][0]).toEqual({
162+
migrationId: 'mig-1',
163+
body: body.slice(50, 51),
164+
});
165+
expect(migrationId).toBe('mig-2');
166+
});
167+
});
168+
169+
describe('upsertMigrationResources', () => {
170+
it('should throw an error when body is empty', async () => {
171+
await expect(service.upsertMigrationResources('mig-1', [])).rejects.toThrow(
172+
i18n.EMPTY_RULES_ERROR
173+
);
174+
});
175+
176+
it('should upsert resources in batches', async () => {
177+
const body = new Array(51).fill({ resource: 'res' });
178+
(upsertMigrationResources as jest.Mock).mockResolvedValue({});
179+
await service.upsertMigrationResources('mig-1', body);
180+
181+
expect(upsertMigrationResources).toHaveBeenCalledTimes(2);
182+
expect((upsertMigrationResources as jest.Mock).mock.calls[0][0]).toEqual({
183+
migrationId: 'mig-1',
184+
body: body.slice(0, 50),
185+
});
186+
expect((upsertMigrationResources as jest.Mock).mock.calls[1][0]).toEqual({
187+
migrationId: 'mig-1',
188+
body: body.slice(50, 51),
189+
});
190+
});
191+
});
192+
193+
describe('startRuleMigration', () => {
194+
it('should notify and not start migration if missing capabilities exist', async () => {
195+
(getMissingCapabilities as jest.Mock).mockReturnValue([{ capability: 'cap' }]);
196+
197+
const result = await service.startRuleMigration('mig-1');
198+
expect(mockNotifications.toasts.add).toHaveBeenCalled();
199+
expect(result).toEqual({ started: false });
200+
});
201+
202+
it('should notify and not start migration if connectorId is missing', async () => {
203+
(getMissingCapabilities as jest.Mock).mockReturnValue([]);
204+
// Force connectorId to be missing
205+
jest.spyOn(service.connectorIdStorage, 'get').mockReturnValue(undefined);
206+
207+
const result = await service.startRuleMigration('mig-1');
208+
expect(mockNotifications.toasts.add).toHaveBeenCalled();
209+
expect(result).toEqual({ started: false });
210+
});
211+
212+
it('should start migration successfully when capabilities and connectorId are present', async () => {
213+
(getMissingCapabilities as jest.Mock).mockReturnValue([]);
214+
// Simulate a valid connector id and trace options
215+
jest.spyOn(service.connectorIdStorage, 'get').mockReturnValue('connector-123');
216+
jest.spyOn(service.traceOptionsStorage, 'get').mockReturnValue({
217+
langSmithProject: 'proj',
218+
langSmithApiKey: 'key',
219+
} as TraceOptions);
220+
(startRuleMigrationAPI as jest.Mock).mockResolvedValue({ started: true });
221+
222+
// Spy on startPolling to ensure it is called after starting the migration
223+
const startPollingSpy = jest.spyOn(service, 'startPolling');
224+
const result = await service.startRuleMigration(
225+
'mig-1',
226+
SiemMigrationRetryFilter.NOT_FULLY_TRANSLATED
227+
);
228+
229+
expect(startRuleMigrationAPI).toHaveBeenCalledWith({
230+
migrationId: 'mig-1',
231+
connectorId: 'connector-123',
232+
retry: SiemMigrationRetryFilter.NOT_FULLY_TRANSLATED,
233+
langSmithOptions: { project_name: 'proj', api_key: 'key' },
234+
});
235+
expect(startPollingSpy).toHaveBeenCalled();
236+
expect(result).toEqual({ started: true });
237+
});
238+
});
239+
240+
describe('getRuleMigrationStats', () => {
241+
it('should return migration stats', async () => {
242+
const stats = { id: 'mig-1', status: SiemMigrationTaskStatus.RUNNING };
243+
(getRuleMigrationStats as jest.Mock).mockResolvedValue(stats);
244+
245+
const result = await service.getRuleMigrationStats('mig-1');
246+
expect(getRuleMigrationStats).toHaveBeenCalledWith({ migrationId: 'mig-1' });
247+
expect(result).toEqual(stats);
248+
});
249+
});
250+
251+
describe('getRuleMigrationsStats', () => {
252+
it('should fetch and update latest stats', async () => {
253+
const statsArray = [
254+
{ id: 'mig-1', status: SiemMigrationTaskStatus.RUNNING },
255+
{ id: 'mig-2', status: SiemMigrationTaskStatus.FINISHED },
256+
];
257+
(getRuleMigrationsStatsAll as jest.Mock).mockResolvedValue(statsArray);
258+
259+
const result = await service.getRuleMigrationsStats();
260+
expect(getRuleMigrationsStatsAll).toHaveBeenCalled();
261+
expect(result).toHaveLength(2);
262+
expect(result[0].number).toBe(1);
263+
expect(result[1].number).toBe(2);
264+
265+
const latestStats = await firstValueFrom(service.getLatestStats$());
266+
expect(latestStats).toEqual(result);
267+
});
268+
});
269+
270+
describe('getMissingResources', () => {
271+
it('should return missing resources', async () => {
272+
const resources = [{ resource: 'res1' }];
273+
(getMissingResources as jest.Mock).mockResolvedValue(resources);
274+
275+
const result = await service.getMissingResources('mig-1');
276+
expect(getMissingResources).toHaveBeenCalledWith({ migrationId: 'mig-1' });
277+
expect(result).toEqual(resources);
278+
});
279+
});
280+
281+
describe('getIntegrations', () => {
282+
it('should return integrations', async () => {
283+
const integrations = { integration1: { id: 'int-1' } };
284+
(getIntegrations as jest.Mock).mockResolvedValue(integrations);
285+
286+
const result = await service.getIntegrations();
287+
expect(getIntegrations).toHaveBeenCalledWith({});
288+
expect(result).toEqual(integrations);
289+
});
290+
});
291+
292+
describe('Polling behavior', () => {
293+
it('should poll and send a success toast when a migration finishes', async () => {
294+
// Use fake timers to simulate delays inside the polling loop.
295+
jest.useFakeTimers();
296+
297+
// Simulate a migration that is first reported as RUNNING and then FINISHED.
298+
const runningMigration = { id: 'mig-1', status: SiemMigrationTaskStatus.RUNNING };
299+
const finishedMigration = { id: 'mig-1', status: SiemMigrationTaskStatus.FINISHED };
300+
301+
// Override getRuleMigrationsStats to return our sequence:
302+
// First call: running, then finished, then empty array.
303+
const getStatsMock = jest
304+
.fn()
305+
.mockResolvedValue([finishedMigration])
306+
.mockResolvedValueOnce([runningMigration]);
307+
308+
service.getRuleMigrationsStats = getStatsMock;
309+
310+
// Ensure a valid connector is present (so that a STOPPED migration would be resumed, if needed)
311+
jest.spyOn(service.connectorIdStorage, 'get').mockReturnValue('connector-123');
312+
313+
// Start polling
314+
service.startPolling();
315+
316+
// Resolve the first getRuleMigrationsStats promise
317+
await Promise.resolve();
318+
319+
// Fast-forward the timer by the polling interval
320+
jest.advanceTimersByTime(REQUEST_POLLING_INTERVAL_SECONDS * 1000);
321+
// Resolve the timeout promise
322+
await Promise.resolve();
323+
// Resolve the second getRuleMigrationsStats promise
324+
await Promise.resolve();
325+
326+
expect(getStatsMock).toHaveBeenCalledTimes(2);
327+
328+
// Expect that a success toast was added when the migration finished.
329+
expect(mockNotifications.toasts.addSuccess).toHaveBeenCalled();
330+
331+
// Restore real timers.
332+
jest.useRealTimers();
333+
});
334+
});
335+
});

x-pack/solutions/security/plugins/security_solution/public/siem_migrations/rules/service/rule_migrations_service.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ import { getMissingCapabilitiesToast } from './notifications/missing_capabilitie
5757
const NAMESPACE_TRACE_OPTIONS_SESSION_STORAGE_KEY =
5858
`${DEFAULT_ASSISTANT_NAMESPACE}.${TRACE_OPTIONS_SESSION_STORAGE_KEY}` as const;
5959

60-
const REQUEST_POLLING_INTERVAL_SECONDS = 10 as const;
60+
export const REQUEST_POLLING_INTERVAL_SECONDS = 10 as const;
6161
const CREATE_MIGRATION_BODY_BATCH_SIZE = 50 as const;
6262

6363
export class SiemRulesMigrationsService {
64-
private readonly latestStats$: BehaviorSubject<RuleMigrationStats[]>;
64+
private readonly latestStats$: BehaviorSubject<RuleMigrationStats[] | null>;
6565
private isPolling = false;
6666
public connectorIdStorage = new RuleMigrationsStorage<string>('connectorId');
6767
public traceOptionsStorage = new RuleMigrationsStorage<TraceOptions>('traceOptions', {
@@ -76,15 +76,15 @@ export class SiemRulesMigrationsService {
7676
telemetryService: TelemetryServiceStart
7777
) {
7878
this.telemetry = new SiemRulesMigrationsTelemetry(telemetryService);
79-
this.latestStats$ = new BehaviorSubject<RuleMigrationStats[]>([]);
79+
this.latestStats$ = new BehaviorSubject<RuleMigrationStats[] | null>(null);
8080

8181
this.plugins.spaces.getActiveSpace().then((space) => {
8282
this.connectorIdStorage.setSpaceId(space.id);
8383
this.startPolling();
8484
});
8585
}
8686

87-
public getLatestStats$(): Observable<RuleMigrationStats[]> {
87+
public getLatestStats$(): Observable<RuleMigrationStats[] | null> {
8888
return this.latestStats$.asObservable();
8989
}
9090

0 commit comments

Comments
 (0)