mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
# Backport This will backport the following commits from `main` to `8.15`: - [[Security Solution] Fix non-responsive rule details page (#187953)](https://github.com/elastic/kibana/pull/187953) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"maxim.palenov@elastic.co"},"sourceCommit":{"committedDate":"2024-07-15T11:37:26Z","message":"[Security Solution] Fix non-responsive rule details page (#187953)\n\n**Resolves:** https://github.com/elastic/kibana/issues/177734\r\n\r\n## Summary\r\n\r\nThis PR fixes a non-responsive rule details page under non default space.\r\n\r\n## Details\r\n\r\n**[Security Solution] Rule details page is not responsive and leads to page crash for rule in non-default spaces [#177734](https://github.com/elastic/kibana/issues/177734)** resurfaced back. Investigation has show that **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** is the cause.\r\n\r\nThe problem is quite subtle to comprehend it just by looking at the code. In fact it boils down to an unstable `useAsync()` hook dependency. Every re-render `useAsync()` resolves a promise causing an additional re-render to show updated results and the cycle repeats. Such hook is used in `x-pack/plugins/security_solution/public/common/components/visualization_actions/actions.tsx`\r\n\r\n```ts\r\n const panels = useAsync(\r\n () =>\r\n buildContextMenuForActions({\r\n actions: contextMenuActions.map((action) => ({\r\n action,\r\n context: {},\r\n trigger: VISUALIZATION_CONTEXT_MENU_TRIGGER,\r\n })),\r\n }),\r\n [contextMenuActions]\r\n );\r\n```\r\n\r\nwhere `contextMenuActions` is an unstable dependency. This is the case due to refactoring to `useSaveToLibrary()` hook by **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** which started retuning a new object every render. The dependency chain is `contextMenuActions` -> `useActions()` -> `useSaveToLibrary()`.\r\n\r\nThe actual fix is to replace\r\n\r\n```ts\r\nconst { lens, ...startServices } = useKibana().services;\r\n```\r\n\r\nwith\r\n\r\n```ts\r\nconst startServices = useKibana().services;\r\n```\r\n\r\nSince `startServices` is used as a hook dependency it must be stable. A rest property in object destruction expression is always a new object and can't be used as a dependency as is. Using stable `useKibana().services` fixes the problem.","sha":"8a539a8a4ce483d2e5d3aa484d8ec78887f338b8","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","impact:critical","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Rule Details","v8.15.0","v8.16.0"],"title":"[Security Solution] Fix non-responsive rule details page","number":187953,"url":"https://github.com/elastic/kibana/pull/187953","mergeCommit":{"message":"[Security Solution] Fix non-responsive rule details page (#187953)\n\n**Resolves:** https://github.com/elastic/kibana/issues/177734\r\n\r\n## Summary\r\n\r\nThis PR fixes a non-responsive rule details page under non default space.\r\n\r\n## Details\r\n\r\n**[Security Solution] Rule details page is not responsive and leads to page crash for rule in non-default spaces [#177734](https://github.com/elastic/kibana/issues/177734)** resurfaced back. Investigation has show that **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** is the cause.\r\n\r\nThe problem is quite subtle to comprehend it just by looking at the code. In fact it boils down to an unstable `useAsync()` hook dependency. Every re-render `useAsync()` resolves a promise causing an additional re-render to show updated results and the cycle repeats. Such hook is used in `x-pack/plugins/security_solution/public/common/components/visualization_actions/actions.tsx`\r\n\r\n```ts\r\n const panels = useAsync(\r\n () =>\r\n buildContextMenuForActions({\r\n actions: contextMenuActions.map((action) => ({\r\n action,\r\n context: {},\r\n trigger: VISUALIZATION_CONTEXT_MENU_TRIGGER,\r\n })),\r\n }),\r\n [contextMenuActions]\r\n );\r\n```\r\n\r\nwhere `contextMenuActions` is an unstable dependency. This is the case due to refactoring to `useSaveToLibrary()` hook by **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** which started retuning a new object every render. The dependency chain is `contextMenuActions` -> `useActions()` -> `useSaveToLibrary()`.\r\n\r\nThe actual fix is to replace\r\n\r\n```ts\r\nconst { lens, ...startServices } = useKibana().services;\r\n```\r\n\r\nwith\r\n\r\n```ts\r\nconst startServices = useKibana().services;\r\n```\r\n\r\nSince `startServices` is used as a hook dependency it must be stable. A rest property in object destruction expression is always a new object and can't be used as a dependency as is. Using stable `useKibana().services` fixes the problem.","sha":"8a539a8a4ce483d2e5d3aa484d8ec78887f338b8"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187953","number":187953,"mergeCommit":{"message":"[Security Solution] Fix non-responsive rule details page (#187953)\n\n**Resolves:** https://github.com/elastic/kibana/issues/177734\r\n\r\n## Summary\r\n\r\nThis PR fixes a non-responsive rule details page under non default space.\r\n\r\n## Details\r\n\r\n**[Security Solution] Rule details page is not responsive and leads to page crash for rule in non-default spaces [#177734](https://github.com/elastic/kibana/issues/177734)** resurfaced back. Investigation has show that **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** is the cause.\r\n\r\nThe problem is quite subtle to comprehend it just by looking at the code. In fact it boils down to an unstable `useAsync()` hook dependency. Every re-render `useAsync()` resolves a promise causing an additional re-render to show updated results and the cycle repeats. Such hook is used in `x-pack/plugins/security_solution/public/common/components/visualization_actions/actions.tsx`\r\n\r\n```ts\r\n const panels = useAsync(\r\n () =>\r\n buildContextMenuForActions({\r\n actions: contextMenuActions.map((action) => ({\r\n action,\r\n context: {},\r\n trigger: VISUALIZATION_CONTEXT_MENU_TRIGGER,\r\n })),\r\n }),\r\n [contextMenuActions]\r\n );\r\n```\r\n\r\nwhere `contextMenuActions` is an unstable dependency. This is the case due to refactoring to `useSaveToLibrary()` hook by **[Security Solution] Remove usage of deprecated React rendering utilities [#181099](https://github.com/elastic/kibana/pull/181099)** which started retuning a new object every render. The dependency chain is `contextMenuActions` -> `useActions()` -> `useSaveToLibrary()`.\r\n\r\nThe actual fix is to replace\r\n\r\n```ts\r\nconst { lens, ...startServices } = useKibana().services;\r\n```\r\n\r\nwith\r\n\r\n```ts\r\nconst startServices = useKibana().services;\r\n```\r\n\r\nSince `startServices` is used as a hook dependency it must be stable. A rest property in object destruction expression is always a new object and can't be used as a dependency as is. Using stable `useKibana().services` fixes the problem.","sha":"8a539a8a4ce483d2e5d3aa484d8ec78887f338b8"}}]}] BACKPORT--> Co-authored-by: Maxim Palenov <maxim.palenov@elastic.co>
This commit is contained in:
parent
3baf161e18
commit
88524f21df
7 changed files with 192 additions and 40 deletions
|
@ -9,7 +9,6 @@ import React, { useCallback, useMemo } from 'react';
|
|||
import { toMountPoint } from '@kbn/react-kibana-mount';
|
||||
import type { LensEmbeddableInput } from '@kbn/lens-plugin/public';
|
||||
import { unmountComponentAtNode } from 'react-dom';
|
||||
import type { SaveProps } from '@kbn/lens-plugin/public/plugin';
|
||||
import { useKibana } from '../../lib/kibana';
|
||||
import type { LensAttributes } from './types';
|
||||
import { useRedirectToDashboardFromLens } from './use_redirect_to_dashboard_from_lens';
|
||||
|
@ -21,8 +20,8 @@ export const useSaveToLibrary = ({
|
|||
}: {
|
||||
attributes: LensAttributes | undefined | null;
|
||||
}) => {
|
||||
const { lens, ...startServices } = useKibana().services;
|
||||
const { SaveModalComponent, canUseEditor } = lens;
|
||||
const startServices = useKibana().services;
|
||||
const { SaveModalComponent, canUseEditor } = startServices.lens;
|
||||
const getSecuritySolutionUrl = useGetSecuritySolutionUrl();
|
||||
const { redirectTo, getEditOrCreateDashboardPath } = useRedirectToDashboardFromLens({
|
||||
getSecuritySolutionUrl,
|
||||
|
@ -33,12 +32,8 @@ export const useSaveToLibrary = ({
|
|||
const mount = toMountPoint(
|
||||
<SaveModalComponent
|
||||
initialInput={attributes as unknown as LensEmbeddableInput}
|
||||
onSave={(saveProps: SaveProps) => {
|
||||
unmountComponentAtNode(targetDomElement);
|
||||
}}
|
||||
onClose={() => {
|
||||
unmountComponentAtNode(targetDomElement);
|
||||
}}
|
||||
onSave={() => unmountComponentAtNode(targetDomElement)}
|
||||
onClose={() => unmountComponentAtNode(targetDomElement)}
|
||||
originatingApp={APP_UI_ID}
|
||||
getOriginatingPath={(dashboardId) =>
|
||||
`${SecurityPageName.dashboards}/${getEditOrCreateDashboardPath(dashboardId)}`
|
||||
|
|
|
@ -0,0 +1,84 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
|
||||
import { createRule } from '../../../../tasks/api_calls/rules';
|
||||
import { ruleFields } from '../../../../data/detection_engine';
|
||||
import { getExistingRule, getNewRule } from '../../../../objects/rule';
|
||||
|
||||
import { RULE_NAME_HEADER, RULE_SWITCH } from '../../../../screens/rule_details';
|
||||
|
||||
import { createTimeline } from '../../../../tasks/api_calls/timelines';
|
||||
import { deleteAlertsAndRules, deleteConnectors } from '../../../../tasks/api_calls/common';
|
||||
import { login } from '../../../../tasks/login';
|
||||
import { activateSpace, getSpaceUrl } from '../../../../tasks/space';
|
||||
import { visit } from '../../../../tasks/navigation';
|
||||
import { ruleDetailsUrl } from '../../../../urls/rule_details';
|
||||
|
||||
describe('Non-default space rule detail page', { tags: ['@ess'] }, function () {
|
||||
const SPACE_ID = 'test';
|
||||
|
||||
beforeEach(() => {
|
||||
login();
|
||||
activateSpace(SPACE_ID);
|
||||
deleteAlertsAndRules();
|
||||
deleteConnectors();
|
||||
createTimeline().then((response) => {
|
||||
createRule({
|
||||
...getNewRule({
|
||||
rule_id: 'rulez',
|
||||
description: ruleFields.ruleDescription,
|
||||
name: ruleFields.ruleName,
|
||||
severity: ruleFields.ruleSeverity,
|
||||
risk_score: ruleFields.riskScore,
|
||||
tags: ruleFields.ruleTags,
|
||||
false_positives: ruleFields.falsePositives,
|
||||
note: ruleFields.investigationGuide,
|
||||
timeline_id: response.body.data.persistTimeline.timeline.savedObjectId,
|
||||
timeline_title: response.body.data.persistTimeline.timeline.title ?? '',
|
||||
interval: ruleFields.ruleInterval,
|
||||
from: `now-1h`,
|
||||
query: ruleFields.ruleQuery,
|
||||
enabled: false,
|
||||
max_signals: 500,
|
||||
threat: [
|
||||
{
|
||||
...ruleFields.threat,
|
||||
technique: [
|
||||
{
|
||||
...ruleFields.threatTechnique,
|
||||
subtechnique: [ruleFields.threatSubtechnique],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}),
|
||||
}).then((rule) => {
|
||||
cy.wrap(rule.body.id).as('ruleId');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('Check responsiveness by enabling/disabling the rule', function () {
|
||||
visit(getSpaceUrl(SPACE_ID, ruleDetailsUrl(this.ruleId)));
|
||||
cy.get(RULE_NAME_HEADER).should('contain', ruleFields.ruleName);
|
||||
|
||||
cy.intercept(
|
||||
'POST',
|
||||
getSpaceUrl(SPACE_ID, '/api/detection_engine/rules/_bulk_action?dry_run=false')
|
||||
).as('bulk_action');
|
||||
cy.get(RULE_SWITCH).should('be.visible');
|
||||
cy.get(RULE_SWITCH).click();
|
||||
cy.wait('@bulk_action').then(({ response }) => {
|
||||
cy.wrap(response?.statusCode).should('eql', 200);
|
||||
cy.wrap(response?.body.attributes.results.updated[0].max_signals).should(
|
||||
'eql',
|
||||
getExistingRule().max_signals
|
||||
);
|
||||
cy.wrap(response?.body.attributes.results.updated[0].enabled).should('eql', true);
|
||||
});
|
||||
});
|
||||
});
|
|
@ -84,3 +84,8 @@ const waitUntil = (subject, fn, options = {}) => {
|
|||
};
|
||||
|
||||
Cypress.Commands.add('waitUntil', { prevSubject: 'optional' }, waitUntil);
|
||||
|
||||
// Sets non-default space id
|
||||
Cypress.Commands.add('setCurrentSpace', (spaceId) => cy.state('currentSpaceId', spaceId));
|
||||
// Reads non-default space id
|
||||
Cypress.Commands.add('currentSpace', () => cy.state('currentSpaceId'));
|
||||
|
|
|
@ -16,6 +16,14 @@ declare namespace Cypress {
|
|||
timeout: number;
|
||||
}
|
||||
): Chainable<Subject>;
|
||||
/**
|
||||
* Sets a new non-default space id as current
|
||||
*/
|
||||
setCurrentSpace(spaceId: string): void;
|
||||
/**
|
||||
* Reads current space id value. `undefined` is returned for default space.
|
||||
*/
|
||||
currentSpace(): Chainable<string>;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -8,9 +8,11 @@
|
|||
import { DATA_VIEW_PATH, INITIAL_REST_VERSION } from '@kbn/data-views-plugin/server/constants';
|
||||
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
|
||||
import { AllConnectorsResponse } from '@kbn/actions-plugin/common/routes/connector/response';
|
||||
import { DETECTION_ENGINE_RULES_BULK_ACTION } from '@kbn/security-solution-plugin/common/constants';
|
||||
import { ELASTICSEARCH_PASSWORD, ELASTICSEARCH_USERNAME } from '../../env_var_names_constants';
|
||||
import { deleteAllDocuments } from './elasticsearch';
|
||||
import { DEFAULT_ALERTS_INDEX_PATTERN } from './alerts';
|
||||
import { getSpaceUrl } from '../space';
|
||||
|
||||
export const API_AUTH = Object.freeze({
|
||||
user: Cypress.env(ELASTICSEARCH_USERNAME),
|
||||
|
@ -41,18 +43,24 @@ export const rootRequest = <T = unknown>({
|
|||
export const deleteAlertsAndRules = () => {
|
||||
cy.log('Delete all alerts and rules');
|
||||
|
||||
rootRequest({
|
||||
method: 'POST',
|
||||
url: '/api/detection_engine/rules/_bulk_action',
|
||||
body: {
|
||||
query: '',
|
||||
action: 'delete',
|
||||
},
|
||||
failOnStatusCode: false,
|
||||
timeout: 300000,
|
||||
});
|
||||
cy.currentSpace().then((spaceId) => {
|
||||
const url = spaceId
|
||||
? `/s/${spaceId}${DETECTION_ENGINE_RULES_BULK_ACTION}`
|
||||
: DETECTION_ENGINE_RULES_BULK_ACTION;
|
||||
|
||||
deleteAllDocuments(`.lists-*,.items-*,${DEFAULT_ALERTS_INDEX_PATTERN}`);
|
||||
rootRequest({
|
||||
method: 'POST',
|
||||
url,
|
||||
body: {
|
||||
query: '',
|
||||
action: 'delete',
|
||||
},
|
||||
failOnStatusCode: false,
|
||||
timeout: 300000,
|
||||
});
|
||||
|
||||
deleteAllDocuments(`.lists-*,.items-*,${DEFAULT_ALERTS_INDEX_PATTERN}`);
|
||||
});
|
||||
};
|
||||
|
||||
export const getConnectors = () =>
|
||||
|
@ -62,20 +70,24 @@ export const getConnectors = () =>
|
|||
});
|
||||
|
||||
export const deleteConnectors = () => {
|
||||
getConnectors().then(($response) => {
|
||||
if ($response.body.length > 0) {
|
||||
const ids = $response.body.map((connector) => {
|
||||
return connector.id;
|
||||
});
|
||||
ids.forEach((id) => {
|
||||
if (!INTERNAL_CLOUD_CONNECTORS.includes(id)) {
|
||||
rootRequest({
|
||||
method: 'DELETE',
|
||||
url: `api/actions/connector/${id}`,
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
cy.currentSpace().then((spaceId) => {
|
||||
getConnectors().then(($response) => {
|
||||
if ($response.body.length > 0) {
|
||||
const ids = $response.body.map((connector) => {
|
||||
return connector.id;
|
||||
});
|
||||
ids.forEach((id) => {
|
||||
if (!INTERNAL_CLOUD_CONNECTORS.includes(id)) {
|
||||
rootRequest({
|
||||
method: 'DELETE',
|
||||
url: spaceId
|
||||
? getSpaceUrl(spaceId, `api/actions/connector/${id}`)
|
||||
: `api/actions/connector/${id}`,
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
@ -18,6 +18,7 @@ import type {
|
|||
import type { FetchRulesResponse } from '@kbn/security-solution-plugin/public/detection_engine/rule_management/logic/types';
|
||||
import { internalAlertingSnoozeRule } from '../../urls/routes';
|
||||
import { rootRequest } from './common';
|
||||
import { getSpaceUrl } from '../space';
|
||||
|
||||
export const findAllRules = () => {
|
||||
return rootRequest<FetchRulesResponse>({
|
||||
|
@ -28,12 +29,14 @@ export const findAllRules = () => {
|
|||
export const createRule = (
|
||||
rule: RuleCreateProps
|
||||
): Cypress.Chainable<Cypress.Response<RuleResponse>> => {
|
||||
return rootRequest<RuleResponse>({
|
||||
method: 'POST',
|
||||
url: DETECTION_ENGINE_RULES_URL,
|
||||
body: rule,
|
||||
failOnStatusCode: false,
|
||||
});
|
||||
return cy.currentSpace().then((spaceId) =>
|
||||
rootRequest<RuleResponse>({
|
||||
method: 'POST',
|
||||
url: spaceId ? getSpaceUrl(spaceId, DETECTION_ENGINE_RULES_URL) : DETECTION_ENGINE_RULES_URL,
|
||||
body: rule,
|
||||
failOnStatusCode: false,
|
||||
})
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
45
x-pack/test/security_solution_cypress/cypress/tasks/space.ts
Normal file
45
x-pack/test/security_solution_cypress/cypress/tasks/space.ts
Normal file
|
@ -0,0 +1,45 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
|
||||
import { API_HEADERS } from './api_calls/common';
|
||||
|
||||
/**
|
||||
* Creates a space and sets it as the current one
|
||||
*/
|
||||
export function activateSpace(spaceId: string): void {
|
||||
const baseUrl = Cypress.config().baseUrl;
|
||||
if (!baseUrl) {
|
||||
throw Error(`Cypress config baseUrl not set!`);
|
||||
}
|
||||
|
||||
cy.request({
|
||||
url: `${baseUrl}/api/spaces/space`,
|
||||
method: 'POST',
|
||||
body: {
|
||||
id: spaceId,
|
||||
name: spaceId,
|
||||
},
|
||||
headers: API_HEADERS,
|
||||
// For the majority cases the specified space already exists and
|
||||
// this request would fail. To avoid condition logic and an extra
|
||||
// request to check for space existence it fails silently.
|
||||
//
|
||||
// While it will make errors less transparent when a user doesn't
|
||||
// have credentials to create spaces. But it's a trade off for now
|
||||
// choosing simplicity over errors transparency.
|
||||
failOnStatusCode: false,
|
||||
});
|
||||
|
||||
cy.setCurrentSpace(spaceId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Constructs a space aware url
|
||||
*/
|
||||
export function getSpaceUrl(spaceId: string, url: string): string {
|
||||
return spaceId ? `/s/${spaceId}${url}` : url;
|
||||
}
|
Loading…
Add table
Add a link
Reference in a new issue