From 96d69c8ffb50bfacaba3a78149c6ddf35dece64e Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 17 Jan 2022 10:58:17 -0500 Subject: [PATCH 01/10] [Osquery] Remove agent policy reference from pack on integration deletion (#122082) (#123163) (cherry picked from commit 426ac6a012ef472a3efd468e416bcd1f41e6562b) Co-authored-by: Tomasz Ciecierski --- .../common/types/rest_spec/package_policy.ts | 1 + .../fleet/server/services/package_policy.ts | 1 + .../integration/superuser/packs.spec.ts | 52 +++++++++++++++++- .../osquery/cypress/tasks/integrations.ts | 5 +- .../osquery/server/lib/fleet_integration.ts | 54 +++++++++++++++++++ x-pack/plugins/osquery/server/plugin.ts | 8 +++ 6 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/osquery/server/lib/fleet_integration.ts diff --git a/x-pack/plugins/fleet/common/types/rest_spec/package_policy.ts b/x-pack/plugins/fleet/common/types/rest_spec/package_policy.ts index 9eb20383d57bd..740f0401dca6e 100644 --- a/x-pack/plugins/fleet/common/types/rest_spec/package_policy.ts +++ b/x-pack/plugins/fleet/common/types/rest_spec/package_policy.ts @@ -56,6 +56,7 @@ export type DeletePackagePoliciesResponse = Array<{ name?: string; success: boolean; package?: PackagePolicyPackage; + policy_id?: string; }>; export interface UpgradePackagePolicyBaseResponse { diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 5858ac8900c11..5591c165bd706 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -489,6 +489,7 @@ class PackagePolicyService { title: packagePolicy.package?.title || '', version: packagePolicy.package?.version || '', }, + policy_id: packagePolicy.policy_id, }); } catch (error) { result.push({ diff --git a/x-pack/plugins/osquery/cypress/integration/superuser/packs.spec.ts b/x-pack/plugins/osquery/cypress/integration/superuser/packs.spec.ts index e50524bfdae44..078ea14dcc854 100644 --- a/x-pack/plugins/osquery/cypress/integration/superuser/packs.spec.ts +++ b/x-pack/plugins/osquery/cypress/integration/superuser/packs.spec.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { navigateTo } from '../../tasks/navigation'; +import { FLEET_AGENT_POLICIES, navigateTo } from '../../tasks/navigation'; import { deleteAndConfirm, findAndClickButton, @@ -15,8 +15,10 @@ import { import { login } from '../../tasks/login'; import { ArchiverMethod, runKbnArchiverScript } from '../../tasks/archiver'; import { preparePack } from '../../tasks/packs'; +import { addIntegration, closeModalIfVisible } from '../../tasks/integrations'; describe('SuperUser - Packs', () => { + const integration = 'Osquery Manager'; const SAVED_QUERY_ID = 'Saved-Query-Id'; const PACK_NAME = 'Pack-name'; const NEW_QUERY_NAME = 'new-query-name'; @@ -77,7 +79,7 @@ describe('SuperUser - Packs', () => { findAndClickButton('Save and deploy changes'); }); // THIS TESTS TAKES TOO LONG FOR NOW - LET ME THINK IT THROUGH - it('to click the icon and visit discover', () => { + it.skip('to click the icon and visit discover', () => { preparePack(PACK_NAME, SAVED_QUERY_ID); cy.react('CustomItemAction', { props: { index: 0, item: { id: SAVED_QUERY_ID } }, @@ -149,6 +151,52 @@ describe('SuperUser - Packs', () => { deleteAndConfirm('pack'); }); }); + describe('Validate that agent is getting removed from pack if we remove agent', () => { + beforeEach(() => { + login(); + }); + const AGENT_NAME = 'PackTest'; + const REMOVING_PACK = 'removing-pack'; + it('add integration', () => { + cy.visit(FLEET_AGENT_POLICIES); + cy.contains('Create agent policy').click(); + cy.get('input[placeholder*="Choose a name"]').type(AGENT_NAME); + cy.get('.euiFlyoutFooter').contains('Create agent policy').click(); + cy.contains(`Agent policy '${AGENT_NAME}' created`); + cy.visit(FLEET_AGENT_POLICIES); + cy.contains('Default Fleet Server policy').click(); + cy.contains('Add integration').click(); + cy.contains(integration).click(); + addIntegration(AGENT_NAME); + cy.contains('Add Elastic Agent later').click(); + navigateTo('app/osquery/packs'); + findAndClickButton('Add pack'); + findFormFieldByRowsLabelAndType('Name', REMOVING_PACK); + findFormFieldByRowsLabelAndType('Scheduled agent policies (optional)', AGENT_NAME); + findAndClickButton('Save pack'); + + cy.getBySel('toastCloseButton').click(); + cy.contains(REMOVING_PACK).click(); + cy.contains(`${REMOVING_PACK} details`); + findAndClickButton('Edit'); + cy.react('EuiComboBoxInput', { props: { value: AGENT_NAME } }).should('exist'); + + cy.visit(FLEET_AGENT_POLICIES); + cy.contains(AGENT_NAME).click(); + cy.get('.euiTableCellContent') + .get('.euiPopover__anchor') + .get(`[aria-label="Open"]`) + .first() + .click(); + cy.contains(/^Delete integration$/).click(); + closeModalIfVisible(); + navigateTo('app/osquery/packs'); + cy.contains(REMOVING_PACK).click(); + cy.contains(`${REMOVING_PACK} details`); + findAndClickButton('Edit'); + cy.react('EuiComboBoxInput', { props: { value: '' } }).should('exist'); + }); + }); describe.skip('Remove queries from pack', () => { const TEST_PACK = 'Test-pack'; before(() => { diff --git a/x-pack/plugins/osquery/cypress/tasks/integrations.ts b/x-pack/plugins/osquery/cypress/tasks/integrations.ts index dd57de32628bb..673f2091760a6 100644 --- a/x-pack/plugins/osquery/cypress/tasks/integrations.ts +++ b/x-pack/plugins/osquery/cypress/tasks/integrations.ts @@ -13,14 +13,13 @@ import { DATA_COLLECTION_SETUP_STEP, } from '../screens/integrations'; -export const addIntegration = () => { +export const addIntegration = (agent = 'Default fleet') => { cy.getBySel(ADD_POLICY_BTN).click(); cy.getBySel(DATA_COLLECTION_SETUP_STEP).find('.euiLoadingSpinner').should('not.exist'); - cy.getBySel('comboBoxInput').click().type('Default fleet {downArrow} {enter}'); + cy.getBySel('comboBoxInput').click().type(`${agent} {downArrow} {enter}`); cy.getBySel(CREATE_PACKAGE_POLICY_SAVE_BTN).click(); // sometimes agent is assigned to default policy, sometimes not closeModalIfVisible(); - cy.getBySel(CREATE_PACKAGE_POLICY_SAVE_BTN, { timeout: 60000 }).should('not.exist'); }; export function closeModalIfVisible() { diff --git a/x-pack/plugins/osquery/server/lib/fleet_integration.ts b/x-pack/plugins/osquery/server/lib/fleet_integration.ts new file mode 100644 index 0000000000000..87d48c95648bb --- /dev/null +++ b/x-pack/plugins/osquery/server/lib/fleet_integration.ts @@ -0,0 +1,54 @@ +/* + * 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 { SavedObjectReference, SavedObjectsClient } from 'kibana/server'; +import { filter, map } from 'lodash'; +import { packSavedObjectType } from '../../common/types'; +import { PostPackagePolicyDeleteCallback } from '../../../fleet/server'; +import { AGENT_POLICY_SAVED_OBJECT_TYPE } from '../../../fleet/common'; +import { OSQUERY_INTEGRATION_NAME } from '../../common'; + +export const getPackagePolicyDeleteCallback = + (packsClient: SavedObjectsClient): PostPackagePolicyDeleteCallback => + async (deletedPackagePolicy) => { + const deletedOsqueryManagerPolicies = filter(deletedPackagePolicy, [ + 'package.name', + OSQUERY_INTEGRATION_NAME, + ]); + await Promise.all( + map(deletedOsqueryManagerPolicies, async (deletedOsqueryManagerPolicy) => { + if (deletedOsqueryManagerPolicy.policy_id) { + const foundPacks = await packsClient.find({ + type: packSavedObjectType, + hasReference: { + type: AGENT_POLICY_SAVED_OBJECT_TYPE, + id: deletedOsqueryManagerPolicy.policy_id, + }, + perPage: 1000, + }); + + await Promise.all( + map( + foundPacks.saved_objects, + (pack: { id: string; references: SavedObjectReference[] }) => + packsClient.update( + packSavedObjectType, + pack.id, + {}, + { + references: filter( + pack.references, + (reference) => reference.id !== deletedOsqueryManagerPolicy.policy_id + ), + } + ) + ) + ); + } + }) + ); + }; diff --git a/x-pack/plugins/osquery/server/plugin.ts b/x-pack/plugins/osquery/server/plugin.ts index 77dfde5800c1e..8e887a012dab3 100644 --- a/x-pack/plugins/osquery/server/plugin.ts +++ b/x-pack/plugins/osquery/server/plugin.ts @@ -18,8 +18,10 @@ import { CoreStart, Plugin, Logger, + SavedObjectsClient, DEFAULT_APP_CATEGORIES, } from '../../../../src/core/server'; + import { createConfig } from './create_config'; import { OsqueryPluginSetup, OsqueryPluginStart, SetupPlugins, StartPlugins } from './types'; import { defineRoutes } from './routes'; @@ -30,6 +32,7 @@ import { OsqueryAppContext, OsqueryAppContextService } from './lib/osquery_app_c import { ConfigType } from './config'; import { packSavedObjectType, savedQuerySavedObjectType } from '../common/types'; import { PLUGIN_ID } from '../common'; +import { getPackagePolicyDeleteCallback } from './lib/fleet_integration'; const registerFeatures = (features: SetupPlugins['features']) => { features.registerKibanaFeature({ @@ -257,6 +260,11 @@ export class OsqueryPlugin implements Plugin Date: Mon, 17 Jan 2022 11:19:29 -0500 Subject: [PATCH 02/10] Do not await the incrementCounter usage call (#123152) (#123167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit 74ef3238fc1e3221f6352ceeed8094a65eb8b3ea) Co-authored-by: Alejandro Fernández Haro --- .../server/saved_objects/service/lib/internal_bulk_resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/internal_bulk_resolve.ts b/src/core/server/saved_objects/service/lib/internal_bulk_resolve.ts index 19f774fb068b6..d83cc591b4c25 100644 --- a/src/core/server/saved_objects/service/lib/internal_bulk_resolve.ts +++ b/src/core/server/saved_objects/service/lib/internal_bulk_resolve.ts @@ -201,7 +201,7 @@ export async function internalBulkResolve( } ); - await incrementCounterInternal( + incrementCounterInternal( CORE_USAGE_STATS_TYPE, CORE_USAGE_STATS_ID, resolveCounter.getCounterFields(), From daf372adba697bf2d5318dc91c16aebbd3e0598f Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 17 Jan 2022 13:14:33 -0500 Subject: [PATCH 03/10] [Security Solution] Fix signals index initialization bug (#123087) (#123175) (cherry picked from commit fef96e99e387d419f010d38b69e6d3cc0d0c91a6) Co-authored-by: Steph Milovic --- .../app/home/global_header/index.test.tsx | 4 ++ .../components/sourcerer/index.test.tsx | 63 ++++++++++++++++++- .../common/components/sourcerer/index.tsx | 9 +++ .../timeline/eql_tab_content/index.test.tsx | 3 + .../timeline/query_tab_content/index.test.tsx | 3 + 5 files changed, 81 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/app/home/global_header/index.test.tsx b/x-pack/plugins/security_solution/public/app/home/global_header/index.test.tsx index cfcf5307de8d4..2d6ca63c82bdb 100644 --- a/x-pack/plugins/security_solution/public/app/home/global_header/index.test.tsx +++ b/x-pack/plugins/security_solution/public/app/home/global_header/index.test.tsx @@ -41,6 +41,10 @@ jest.mock('../../../common/containers/source', () => ({ useFetchIndex: () => [false, { indicesExist: true, indexPatterns: mockIndexPattern }], })); +jest.mock('../../../common/containers/sourcerer/use_signal_helpers', () => ({ + useSignalHelpers: () => ({ signalIndexNeedsInit: false }), +})); + jest.mock('react-reverse-portal', () => ({ InPortal: ({ children }: { children: React.ReactNode }) => <>{children}, OutPortal: ({ children }: { children: React.ReactNode }) => <>{children}, diff --git a/x-pack/plugins/security_solution/public/common/components/sourcerer/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/sourcerer/index.test.tsx index 7d3dc9641929a..f4707272d2c24 100644 --- a/x-pack/plugins/security_solution/public/common/components/sourcerer/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/sourcerer/index.test.tsx @@ -21,10 +21,12 @@ import { createStore } from '../../store'; import { EuiSuperSelectOption } from '@elastic/eui/src/components/form/super_select/super_select_control'; import { waitFor } from '@testing-library/dom'; import { useSourcererDataView } from '../../containers/sourcerer'; +import { useSignalHelpers } from '../../containers/sourcerer/use_signal_helpers'; const mockDispatch = jest.fn(); jest.mock('../../containers/sourcerer'); +jest.mock('../../containers/sourcerer/use_signal_helpers'); const mockUseUpdateDataView = jest.fn().mockReturnValue(() => true); jest.mock('./use_update_data_view', () => ({ useUpdateDataView: () => mockUseUpdateDataView, @@ -81,10 +83,12 @@ const sourcererDataView = { describe('Sourcerer component', () => { const { storage } = createSecuritySolutionStorageMock(); - + const pollForSignalIndexMock = jest.fn(); beforeEach(() => { store = createStore(mockGlobalState, SUB_PLUGINS_REDUCER, kibanaObservable, storage); (useSourcererDataView as jest.Mock).mockReturnValue(sourcererDataView); + (useSignalHelpers as jest.Mock).mockReturnValue({ signalIndexNeedsInit: false }); + jest.clearAllMocks(); }); @@ -570,6 +574,63 @@ describe('Sourcerer component', () => { .exists() ).toBeFalsy(); }); + + it('does not poll for signals index if pollForSignalIndex is not defined', () => { + (useSignalHelpers as jest.Mock).mockReturnValue({ + signalIndexNeedsInit: false, + }); + + mount( + + + + ); + + expect(pollForSignalIndexMock).toHaveBeenCalledTimes(0); + }); + + it('does not poll for signals index if it does not exist and scope is default', () => { + (useSignalHelpers as jest.Mock).mockReturnValue({ + pollForSignalIndex: pollForSignalIndexMock, + signalIndexNeedsInit: false, + }); + + mount( + + + + ); + + expect(pollForSignalIndexMock).toHaveBeenCalledTimes(0); + }); + + it('polls for signals index if it does not exist and scope is timeline', () => { + (useSignalHelpers as jest.Mock).mockReturnValue({ + pollForSignalIndex: pollForSignalIndexMock, + signalIndexNeedsInit: false, + }); + + mount( + + + + ); + expect(pollForSignalIndexMock).toHaveBeenCalledTimes(1); + }); + + it('polls for signals index if it does not exist and scope is detections', () => { + (useSignalHelpers as jest.Mock).mockReturnValue({ + pollForSignalIndex: pollForSignalIndexMock, + signalIndexNeedsInit: false, + }); + + mount( + + + + ); + expect(pollForSignalIndexMock).toHaveBeenCalledTimes(1); + }); }); describe('sourcerer on alerts page or rules details page', () => { diff --git a/x-pack/plugins/security_solution/public/common/components/sourcerer/index.tsx b/x-pack/plugins/security_solution/public/common/components/sourcerer/index.tsx index 924601758c730..ad3b11a74e81d 100644 --- a/x-pack/plugins/security_solution/public/common/components/sourcerer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/sourcerer/index.tsx @@ -28,6 +28,7 @@ import { useSourcererDataView } from '../../containers/sourcerer'; import { useUpdateDataView } from './use_update_data_view'; import { Trigger } from './trigger'; import { AlertsCheckbox, SaveButtons, SourcererCallout } from './sub_components'; +import { useSignalHelpers } from '../../containers/sourcerer/use_signal_helpers'; export interface SourcererComponentProps { scope: sourcererModel.SourcererScopeName; @@ -50,6 +51,14 @@ export const Sourcerer = React.memo(({ scope: scopeId } }, } = useDeepEqualSelector((state) => sourcererScopeSelector(state, scopeId)); + const { pollForSignalIndex } = useSignalHelpers(); + + useEffect(() => { + if (pollForSignalIndex != null && (isTimelineSourcerer || isDetectionsSourcerer)) { + pollForSignalIndex(); + } + }, [isDetectionsSourcerer, isTimelineSourcerer, pollForSignalIndex]); + const { activePatterns, indicesExist, loading } = useSourcererDataView(scopeId); const [missingPatterns, setMissingPatterns] = useState( activePatterns && activePatterns.length > 0 diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/eql_tab_content/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/eql_tab_content/index.test.tsx index 0096b152b3236..dba9f99681a0c 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/eql_tab_content/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/eql_tab_content/index.test.tsx @@ -35,6 +35,9 @@ jest.mock('../body/events/index', () => ({ })); jest.mock('../../../../common/containers/sourcerer'); +jest.mock('../../../../common/containers/sourcerer/use_signal_helpers', () => ({ + useSignalHelpers: () => ({ signalIndexNeedsInit: false }), +})); const mockUseResizeObserver: jest.Mock = useResizeObserver as jest.Mock; jest.mock('use-resize-observer/polyfilled'); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/query_tab_content/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/query_tab_content/index.test.tsx index d1741c399dbab..a32e77a107a43 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/query_tab_content/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/query_tab_content/index.test.tsx @@ -38,6 +38,9 @@ jest.mock('../body/events/index', () => ({ })); jest.mock('../../../../common/containers/sourcerer'); +jest.mock('../../../../common/containers/sourcerer/use_signal_helpers', () => ({ + useSignalHelpers: () => ({ signalIndexNeedsInit: false }), +})); const mockUseResizeObserver: jest.Mock = useResizeObserver as jest.Mock; jest.mock('use-resize-observer/polyfilled'); From 3cd5f59ad3fe0eb7e817bb7e790a51be9311198a Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 17 Jan 2022 13:43:49 -0500 Subject: [PATCH 04/10] [ML] Functional tests - stabilize transform wizard field input (#123126) (#123180) This PR stabilizes the transform wizard field input during functional tests by changing to the more stable setValueWithChecks service method that we're already using for many ML wizard fields. (cherry picked from commit 7f15eaad65f8ecac6e01b73321436564f3d1e798) Co-authored-by: Robert Oskamp --- x-pack/test/functional/services/transform/wizard.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/test/functional/services/transform/wizard.ts b/x-pack/test/functional/services/transform/wizard.ts index 434653be49c14..f876e2432931a 100644 --- a/x-pack/test/functional/services/transform/wizard.ts +++ b/x-pack/test/functional/services/transform/wizard.ts @@ -26,6 +26,7 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi const testSubjects = getService('testSubjects'); const comboBox = getService('comboBox'); const retry = getService('retry'); + const ml = getService('ml'); const PageObjects = getPageObjects(['discover', 'timePicker']); return { @@ -679,7 +680,9 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi }, async setTransformId(transformId: string) { - await testSubjects.setValue('transformIdInput', transformId, { clearWithKeyboard: true }); + await ml.commonUI.setValueWithChecks('transformIdInput', transformId, { + clearWithKeyboard: true, + }); await this.assertTransformIdValue(transformId); }, @@ -699,7 +702,7 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi }, async setTransformDescription(transformDescription: string) { - await testSubjects.setValue('transformDescriptionInput', transformDescription, { + await ml.commonUI.setValueWithChecks('transformDescriptionInput', transformDescription, { clearWithKeyboard: true, }); await this.assertTransformDescriptionValue(transformDescription); @@ -721,7 +724,7 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi }, async setDestinationIndex(destinationIndex: string) { - await testSubjects.setValue('transformDestinationIndexInput', destinationIndex, { + await ml.commonUI.setValueWithChecks('transformDestinationIndexInput', destinationIndex, { clearWithKeyboard: true, }); await this.assertDestinationIndexValue(destinationIndex); From ed143741d3287320d7ac2b911c5a9277ee5d8b46 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Tue, 18 Jan 2022 02:55:02 +0000 Subject: [PATCH 05/10] skip flaky suite (#110938) --- x-pack/test/licensing_plugin/server/updates.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/licensing_plugin/server/updates.ts b/x-pack/test/licensing_plugin/server/updates.ts index 87132dd28ddfb..55b2b68ff8f82 100644 --- a/x-pack/test/licensing_plugin/server/updates.ts +++ b/x-pack/test/licensing_plugin/server/updates.ts @@ -17,7 +17,8 @@ export default function (ftrContext: FtrProviderContext) { const scenario = createScenario(ftrContext); - describe('changes in license types', () => { + // FLAKY: https://github.com/elastic/kibana/issues/110938 + describe.skip('changes in license types', () => { after(async () => { await scenario.teardown(); }); From f18a1386a5c38f60be37d4373afe2eabe1446854 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Tue, 18 Jan 2022 02:59:24 +0000 Subject: [PATCH 06/10] skip flaky suite (#118728) --- x-pack/test/functional/apps/security/users.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/security/users.ts b/x-pack/test/functional/apps/security/users.ts index b2bef848b18e2..634c7ace52735 100644 --- a/x-pack/test/functional/apps/security/users.ts +++ b/x-pack/test/functional/apps/security/users.ts @@ -201,7 +201,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); }); - describe('Deactivate/Activate user', () => { + // FLAKY: https://github.com/elastic/kibana/issues/118728 + describe.skip('Deactivate/Activate user', () => { it('deactivates user when confirming', async () => { await PageObjects.security.deactivatesUser(optionalUser); const users = keyBy(await PageObjects.security.getElasticsearchUsers(), 'username'); From 4e149659c4ae30adcc2a5c88d17844aba978b9c5 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 18 Jan 2022 06:02:41 -0500 Subject: [PATCH 07/10] [Infra] Use Kibana time zone setting for all charts (#123139) (#123198) (cherry picked from commit cb3e0ec67a0a20767204be752c31969bd8a3ff61) Co-authored-by: Milton Hultgren --- .../criterion_preview_chart.tsx | 3 +++ .../hooks/use_kibana_time_zone_setting.ts | 19 +++++++++++++++++++ .../single_metric_sparkline.tsx | 3 +++ .../metric_detail/components/series_chart.tsx | 5 +++++ .../components/series_chart.tsx | 5 +++++ 5 files changed, 35 insertions(+) create mode 100644 x-pack/plugins/infra/public/hooks/use_kibana_time_zone_setting.ts diff --git a/x-pack/plugins/infra/public/alerting/log_threshold/components/expression_editor/criterion_preview_chart.tsx b/x-pack/plugins/infra/public/alerting/log_threshold/components/expression_editor/criterion_preview_chart.tsx index 6e26bb52e7921..73d39d82ae03a 100644 --- a/x-pack/plugins/infra/public/alerting/log_threshold/components/expression_editor/criterion_preview_chart.tsx +++ b/x-pack/plugins/infra/public/alerting/log_threshold/components/expression_editor/criterion_preview_chart.tsx @@ -47,6 +47,7 @@ import { } from '../../../../../common/http_api/log_alerts/'; import { useChartPreviewData } from './hooks/use_chart_preview_data'; import { decodeOrThrow } from '../../../../../common/runtime_types'; +import { useKibanaTimeZoneSetting } from '../../../../hooks/use_kibana_time_zone_setting'; const GROUP_LIMIT = 5; @@ -126,6 +127,7 @@ const CriterionPreviewChart: React.FC = ({ }) => { const { uiSettings } = useKibana().services; const isDarkMode = uiSettings?.get('theme:darkMode') || false; + const timezone = useKibanaTimeZoneSetting(); const { getChartPreviewData, @@ -242,6 +244,7 @@ const CriterionPreviewChart: React.FC = ({ }, }} color={!isGrouped ? colorTransformer(Color.color0) : undefined} + timeZone={timezone} /> {showThreshold && threshold ? ( (UI_SETTINGS.DATEFORMAT_TZ); + + if (!kibanaTimeZone || kibanaTimeZone === 'Browser') { + return 'local'; + } + + return kibanaTimeZone; +} diff --git a/x-pack/plugins/infra/public/pages/logs/log_entry_categories/sections/top_categories/single_metric_sparkline.tsx b/x-pack/plugins/infra/public/pages/logs/log_entry_categories/sections/top_categories/single_metric_sparkline.tsx index 289497da0626a..38bb7d224570c 100644 --- a/x-pack/plugins/infra/public/pages/logs/log_entry_categories/sections/top_categories/single_metric_sparkline.tsx +++ b/x-pack/plugins/infra/public/pages/logs/log_entry_categories/sections/top_categories/single_metric_sparkline.tsx @@ -14,6 +14,7 @@ import { } from '@elastic/eui/dist/eui_charts_theme'; import { useKibanaUiSetting } from '../../../../../utils/use_kibana_ui_setting'; +import { useKibanaTimeZoneSetting } from '../../../../../hooks/use_kibana_time_zone_setting'; import { TimeRange } from '../../../../../../common/time'; interface TimeSeriesPoint { @@ -33,6 +34,7 @@ export const SingleMetricSparkline: React.FunctionComponent<{ timeRange: TimeRange; }> = ({ metric, timeRange }) => { const [isDarkMode] = useKibanaUiSetting('theme:darkMode'); + const timeZone = useKibanaTimeZoneSetting(); const theme = useMemo( () => [ @@ -60,6 +62,7 @@ export const SingleMetricSparkline: React.FunctionComponent<{ xAccessor={timestampAccessor} xScaleType={ScaleType.Time} yAccessors={valueAccessor} + timeZone={timeZone} /> ); diff --git a/x-pack/plugins/infra/public/pages/metrics/metric_detail/components/series_chart.tsx b/x-pack/plugins/infra/public/pages/metrics/metric_detail/components/series_chart.tsx index d1c98ed97ce18..22079943efb54 100644 --- a/x-pack/plugins/infra/public/pages/metrics/metric_detail/components/series_chart.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/metric_detail/components/series_chart.tsx @@ -16,6 +16,7 @@ import { } from '@elastic/charts'; import { NodeDetailsDataSeries } from '../../../../../common/http_api/node_details_api'; import { InventoryVisType } from '../../../../../common/inventory_models/types'; +import { useKibanaTimeZoneSetting } from '../../../../hooks/use_kibana_time_zone_setting'; interface Props { id: string; @@ -34,6 +35,7 @@ export const SeriesChart = (props: Props) => { }; export const AreaChart = ({ id, color, series, name, type, stack }: Props) => { + const timezone = useKibanaTimeZoneSetting(); const style: RecursivePartial = { area: { opacity: 1, @@ -56,11 +58,13 @@ export const AreaChart = ({ id, color, series, name, type, stack }: Props) => { areaSeriesStyle={style} color={color ? color : void 0} stackAccessors={stack ? ['timestamp'] : void 0} + timeZone={timezone} /> ); }; export const BarChart = ({ id, color, series, name, stack }: Props) => { + const timezone = useKibanaTimeZoneSetting(); const style: RecursivePartial = { rectBorder: { stroke: color || void 0, @@ -83,6 +87,7 @@ export const BarChart = ({ id, color, series, name, stack }: Props) => { barSeriesStyle={style} color={color ? color : void 0} stackAccessors={stack ? ['timestamp'] : void 0} + timeZone={timezone} /> ); }; diff --git a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx index 47844543a279a..12db775e243f8 100644 --- a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx @@ -21,6 +21,7 @@ import { MetricsExplorerOptionsMetric, MetricsExplorerChartType, } from '../hooks/use_metrics_explorer_options'; +import { useKibanaTimeZoneSetting } from '../../../../hooks/use_kibana_time_zone_setting'; import { getMetricId } from './helpers/get_metric_id'; type NumberOrString = string | number; @@ -42,6 +43,7 @@ export const MetricExplorerSeriesChart = (props: Props) => { }; export const MetricsExplorerAreaChart = ({ metric, id, series, type, stack, opacity }: Props) => { + const timezone = useKibanaTimeZoneSetting(); const color = (metric.color && colorTransformer(metric.color)) || colorTransformer(Color.color0); const yAccessors = Array.isArray(id) @@ -78,11 +80,13 @@ export const MetricsExplorerAreaChart = ({ metric, id, series, type, stack, opac stackAccessors={stack ? ['timestamp'] : void 0} areaSeriesStyle={seriesAreaStyle} color={color} + timeZone={timezone} /> ); }; export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) => { + const timezone = useKibanaTimeZoneSetting(); const color = (metric.color && colorTransformer(metric.color)) || colorTransformer(Color.color0); const yAccessors = Array.isArray(id) @@ -113,6 +117,7 @@ export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) => stackAccessors={stack ? ['timestamp'] : void 0} barSeriesStyle={seriesBarStyle} color={color} + timeZone={timezone} /> ); }; From d67bb3164e5be5b6580eb6fda90345a5c694feb4 Mon Sep 17 00:00:00 2001 From: Thom Heymann <190132+thomheymann@users.noreply.github.com> Date: Tue, 18 Jan 2022 11:43:51 +0000 Subject: [PATCH 08/10] [8.0] Add session cleanup audit logging (#122419) #122898 (#123200) * Add session cleanup audit logging (#122419) * Add session cleanup audit logging * Update snapshots * Added suggestions from code review * Clean up sessions in batches * Added suggestions form code review (cherry picked from commit 39cef8bca958cf828c36909a07b764c4f3ecc3e8) * Fix types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- docs/user/security/audit-logging.asciidoc | 5 +- .../server/audit/audit_events.test.ts | 32 + .../security/server/audit/audit_events.ts | 29 + .../server/audit/audit_service.test.ts | 79 ++ .../security/server/audit/audit_service.ts | 110 +-- .../security/server/audit/index.mock.ts | 3 + x-pack/plugins/security/server/audit/index.ts | 1 + x-pack/plugins/security/server/plugin.test.ts | 3 + x-pack/plugins/security/server/plugin.ts | 5 +- .../session_management/session_index.test.ts | 731 +++++++++++------- .../session_management/session_index.ts | 119 ++- .../session_management_service.test.ts | 44 +- .../session_management_service.ts | 4 + 13 files changed, 806 insertions(+), 359 deletions(-) diff --git a/docs/user/security/audit-logging.asciidoc b/docs/user/security/audit-logging.asciidoc index 33e1c2b523511..1e7eb1971af08 100644 --- a/docs/user/security/audit-logging.asciidoc +++ b/docs/user/security/audit-logging.asciidoc @@ -53,8 +53,11 @@ Refer to the corresponding {es} logs for potential write errors. | `user_logout` | `unknown` | User is logging out. +| `session_cleanup` +| `unknown` | Removing invalid or expired session. + | `access_agreement_acknowledged` -| N/A | User has acknowledged the access agreement. +| n/a | User has acknowledged the access agreement. 3+a| ===== Category: database diff --git a/x-pack/plugins/security/server/audit/audit_events.test.ts b/x-pack/plugins/security/server/audit/audit_events.test.ts index df796b0603176..0a7337e453274 100644 --- a/x-pack/plugins/security/server/audit/audit_events.test.ts +++ b/x-pack/plugins/security/server/audit/audit_events.test.ts @@ -15,6 +15,7 @@ import { httpRequestEvent, SavedObjectAction, savedObjectEvent, + sessionCleanupEvent, SpaceAuditAction, spaceAuditEvent, userLoginEvent, @@ -352,6 +353,37 @@ describe('#userLogoutEvent', () => { }); }); +describe('#sessionCleanupEvent', () => { + test('creates event with `unknown` outcome', () => { + expect( + sessionCleanupEvent({ + usernameHash: 'abcdef', + sessionId: 'sid', + provider: { name: 'basic1', type: 'basic' }, + }) + ).toMatchInlineSnapshot(` + Object { + "event": Object { + "action": "session_cleanup", + "category": Array [ + "authentication", + ], + "outcome": "unknown", + }, + "kibana": Object { + "authentication_provider": "basic1", + "authentication_type": "basic", + "session_id": "sid", + }, + "message": "Removing invalid or expired session for user [hash=abcdef]", + "user": Object { + "hash": "abcdef", + }, + } + `); + }); +}); + describe('#httpRequestEvent', () => { test('creates event with `unknown` outcome', () => { expect( diff --git a/x-pack/plugins/security/server/audit/audit_events.ts b/x-pack/plugins/security/server/audit/audit_events.ts index 96bc85c1be37e..2dfaf8ece004f 100644 --- a/x-pack/plugins/security/server/audit/audit_events.ts +++ b/x-pack/plugins/security/server/audit/audit_events.ts @@ -156,6 +156,35 @@ export function userLogoutEvent({ username, provider }: UserLogoutParams): Audit }; } +export interface SessionCleanupParams { + sessionId: string; + usernameHash?: string; + provider: AuthenticationProvider; +} + +export function sessionCleanupEvent({ + usernameHash, + sessionId, + provider, +}: SessionCleanupParams): AuditEvent { + return { + message: `Removing invalid or expired session for user [hash=${usernameHash}]`, + event: { + action: 'session_cleanup', + category: ['authentication'], + outcome: 'unknown', + }, + user: { + hash: usernameHash, + }, + kibana: { + session_id: sessionId, + authentication_provider: provider.name, + authentication_type: provider.type, + }, + }; +} + export interface AccessAgreementAcknowledgedParams { username: string; provider: AuthenticationProvider; diff --git a/x-pack/plugins/security/server/audit/audit_service.test.ts b/x-pack/plugins/security/server/audit/audit_service.test.ts index 493490a8e8b9f..1815f617dceae 100644 --- a/x-pack/plugins/security/server/audit/audit_service.test.ts +++ b/x-pack/plugins/security/server/audit/audit_service.test.ts @@ -67,6 +67,9 @@ describe('#setup', () => { ).toMatchInlineSnapshot(` Object { "asScoped": [Function], + "withoutRequest": Object { + "log": [Function], + }, } `); audit.stop(); @@ -254,6 +257,82 @@ describe('#asScoped', () => { }); }); +describe('#withoutRequest', () => { + it('logs event without additional meta data', async () => { + const audit = new AuditService(logger); + const auditSetup = audit.setup({ + license, + config, + logging, + http, + getCurrentUser, + getSpaceId, + getSID, + recordAuditLoggingUsage, + }); + + await auditSetup.withoutRequest.log({ message: 'MESSAGE', event: { action: 'ACTION' } }); + expect(logger.info).toHaveBeenCalledWith('MESSAGE', { + event: { action: 'ACTION' }, + }); + audit.stop(); + }); + + it('does not log to audit logger if event matches ignore filter', async () => { + const audit = new AuditService(logger); + const auditSetup = audit.setup({ + license, + config: { + enabled: true, + appender: { + type: 'console', + layout: { + type: 'json', + }, + }, + ignore_filters: [{ actions: ['ACTION'] }], + }, + logging, + http, + getCurrentUser, + getSpaceId, + getSID, + recordAuditLoggingUsage, + }); + + await auditSetup.withoutRequest.log({ message: 'MESSAGE', event: { action: 'ACTION' } }); + expect(logger.info).not.toHaveBeenCalled(); + audit.stop(); + }); + + it('does not log to audit logger if no event was generated', async () => { + const audit = new AuditService(logger); + const auditSetup = audit.setup({ + license, + config: { + enabled: true, + appender: { + type: 'console', + layout: { + type: 'json', + }, + }, + ignore_filters: [{ actions: ['ACTION'] }], + }, + logging, + http, + getCurrentUser, + getSpaceId, + getSID, + recordAuditLoggingUsage, + }); + + await auditSetup.withoutRequest.log(undefined); + expect(logger.info).not.toHaveBeenCalled(); + audit.stop(); + }); +}); + describe('#createLoggingConfig', () => { test('sets log level to `info` when audit logging is enabled and appender is defined', async () => { const features$ = of({ diff --git a/x-pack/plugins/security/server/audit/audit_service.ts b/x-pack/plugins/security/server/audit/audit_service.ts index fb03669ca0fc5..a29ec221b3474 100644 --- a/x-pack/plugins/security/server/audit/audit_service.ts +++ b/x-pack/plugins/security/server/audit/audit_service.ts @@ -26,11 +26,58 @@ export const ECS_VERSION = '1.6.0'; export const RECORD_USAGE_INTERVAL = 60 * 60 * 1000; // 1 hour export interface AuditLogger { + /** + * Logs an {@link AuditEvent} and automatically adds meta data about the + * current user, space and correlation id. + * + * Guidelines around what events should be logged and how they should be + * structured can be found in: `/x-pack/plugins/security/README.md` + * + * @example + * ```typescript + * const auditLogger = securitySetup.audit.asScoped(request); + * auditLogger.log({ + * message: 'User is updating dashboard [id=123]', + * event: { + * action: 'saved_object_update', + * outcome: 'unknown' + * }, + * kibana: { + * saved_object: { type: 'dashboard', id: '123' } + * }, + * }); + * ``` + */ log: (event: AuditEvent | undefined) => void; } export interface AuditServiceSetup { + /** + * Creates an {@link AuditLogger} scoped to the current request. + * + * This audit logger logs events with all required user and session info and should be used for + * all user-initiated actions. + * + * @example + * ```typescript + * const auditLogger = securitySetup.audit.asScoped(request); + * auditLogger.log(event); + * ``` + */ asScoped: (request: KibanaRequest) => AuditLogger; + + /** + * {@link AuditLogger} for background tasks only. + * + * This audit logger logs events without any user or session info and should never be used to log + * user-initiated actions. + * + * @example + * ```typescript + * securitySetup.audit.withoutRequest.log(event); + * ``` + */ + withoutRequest: AuditLogger; } interface AuditServiceSetupParams { @@ -88,46 +135,25 @@ export class AuditService { }); } - /** - * Creates an {@link AuditLogger} scoped to the current request. - * - * @example - * ```typescript - * const auditLogger = securitySetup.audit.asScoped(request); - * auditLogger.log(event); - * ``` - */ - const asScoped = (request: KibanaRequest): AuditLogger => { - /** - * Logs an {@link AuditEvent} and automatically adds meta data about the - * current user, space and correlation id. - * - * Guidelines around what events should be logged and how they should be - * structured can be found in: `/x-pack/plugins/security/README.md` - * - * @example - * ```typescript - * const auditLogger = securitySetup.audit.asScoped(request); - * auditLogger.log({ - * message: 'User is updating dashboard [id=123]', - * event: { - * action: 'saved_object_update', - * outcome: 'unknown' - * }, - * kibana: { - * saved_object: { type: 'dashboard', id: '123' } - * }, - * }); - * ``` - */ - const log: AuditLogger['log'] = async (event) => { + const log = (event: AuditEvent | undefined) => { + if (!event) { + return; + } + if (filterEvent(event, config.ignore_filters)) { + const { message, ...eventMeta } = event; + this.logger.info(message, eventMeta); + } + }; + + const asScoped = (request: KibanaRequest): AuditLogger => ({ + log: async (event) => { if (!event) { return; } const spaceId = getSpaceId(request); const user = getCurrentUser(request); const sessionId = await getSID(request); - const meta: AuditEvent = { + log({ ...event, user: (user && { @@ -141,14 +167,9 @@ export class AuditService { ...event.kibana, }, trace: { id: request.id }, - }; - if (filterEvent(meta, config.ignore_filters)) { - const { message, ...eventMeta } = meta; - this.logger.info(message, eventMeta); - } - }; - return { log }; - }; + }); + }, + }); http.registerOnPostAuth((request, response, t) => { if (request.auth.isAuthenticated) { @@ -157,7 +178,10 @@ export class AuditService { return t.next(); }); - return { asScoped }; + return { + asScoped, + withoutRequest: { log }, + }; } stop() { diff --git a/x-pack/plugins/security/server/audit/index.mock.ts b/x-pack/plugins/security/server/audit/index.mock.ts index ce6885aee50de..c84faacff0147 100644 --- a/x-pack/plugins/security/server/audit/index.mock.ts +++ b/x-pack/plugins/security/server/audit/index.mock.ts @@ -14,6 +14,9 @@ export const auditServiceMock = { asScoped: jest.fn().mockReturnValue({ log: jest.fn(), }), + withoutRequest: { + log: jest.fn(), + }, } as jest.Mocked>; }, }; diff --git a/x-pack/plugins/security/server/audit/index.ts b/x-pack/plugins/security/server/audit/index.ts index f83c7a7f3bd8a..0bd8492b79670 100644 --- a/x-pack/plugins/security/server/audit/index.ts +++ b/x-pack/plugins/security/server/audit/index.ts @@ -11,6 +11,7 @@ export type { AuditEvent } from './audit_events'; export { userLoginEvent, userLogoutEvent, + sessionCleanupEvent, accessAgreementAcknowledgedEvent, httpRequestEvent, savedObjectEvent, diff --git a/x-pack/plugins/security/server/plugin.test.ts b/x-pack/plugins/security/server/plugin.test.ts index 3d43129b63809..85c2fff5a438e 100644 --- a/x-pack/plugins/security/server/plugin.test.ts +++ b/x-pack/plugins/security/server/plugin.test.ts @@ -67,6 +67,9 @@ describe('Security Plugin', () => { Object { "audit": Object { "asScoped": [Function], + "withoutRequest": Object { + "log": [Function], + }, }, "authc": Object { "getCurrentUser": [Function], diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts index e8f7aa2aacfdd..5d56307547558 100644 --- a/x-pack/plugins/security/server/plugin.ts +++ b/x-pack/plugins/security/server/plugin.ts @@ -310,9 +310,7 @@ export class SecurityPlugin }); return Object.freeze({ - audit: { - asScoped: this.auditSetup.asScoped, - }, + audit: this.auditSetup, authc: { getCurrentUser: (request) => this.getAuthentication().getCurrentUser(request) }, authz: { actions: this.authorizationSetup.actions, @@ -345,6 +343,7 @@ export class SecurityPlugin const clusterClient = core.elasticsearch.client; const { watchOnlineStatus$ } = this.elasticsearchService.start(); const { session } = this.sessionManagementService.start({ + auditLogger: this.auditSetup!.withoutRequest, elasticsearchClient: clusterClient.asInternalUser, kibanaIndexName: this.getKibanaIndexName(), online$: watchOnlineStatus$(), diff --git a/x-pack/plugins/security/server/session_management/session_index.test.ts b/x-pack/plugins/security/server/session_management/session_index.test.ts index 251a0a3edb061..45ce865de5635 100644 --- a/x-pack/plugins/security/server/session_management/session_index.test.ts +++ b/x-pack/plugins/security/server/session_management/session_index.test.ts @@ -6,11 +6,19 @@ */ import { errors } from '@elastic/elasticsearch'; +import type { + BulkResponse, + ClosePointInTimeResponse, + OpenPointInTimeResponse, + SearchResponse, +} from '@elastic/elasticsearch/lib/api/types'; import type { DeeplyMockedKeys } from '@kbn/utility-types/jest'; import type { ElasticsearchClient } from 'src/core/server'; import { elasticsearchServiceMock, loggingSystemMock } from 'src/core/server/mocks'; +import type { AuditLogger } from '../audit'; +import { auditServiceMock } from '../audit/index.mock'; import { ConfigSchema, createConfig } from '../config'; import { securityMock } from '../mocks'; import { getSessionIndexTemplate, SessionIndex } from './session_index'; @@ -19,11 +27,13 @@ import { sessionIndexMock } from './session_index.mock'; describe('Session index', () => { let mockElasticsearchClient: DeeplyMockedKeys; let sessionIndex: SessionIndex; + let auditLogger: AuditLogger; const indexName = '.kibana_some_tenant_security_session_1'; const indexTemplateName = '.kibana_some_tenant_security_session_index_template_1'; beforeEach(() => { mockElasticsearchClient = elasticsearchServiceMock.createElasticsearchClient(); - const sessionIndexOptions = { + auditLogger = auditServiceMock.create().withoutRequest; + sessionIndex = new SessionIndex({ logger: loggingSystemMock.createLogger(), kibanaIndexName: '.kibana_some_tenant', config: createConfig( @@ -32,9 +42,8 @@ describe('Session index', () => { { isTLSEnabled: false } ), elasticsearchClient: mockElasticsearchClient, - }; - - sessionIndex = new SessionIndex(sessionIndexOptions); + auditLogger, + }); }); describe('#initialize', () => { @@ -219,74 +228,130 @@ describe('Session index', () => { describe('#cleanUp', () => { const now = 123456; + const sessionValue = { + _id: 'SESSION_ID', + _source: { usernameHash: 'USERNAME_HASH', provider: { name: 'basic1', type: 'basic' } }, + sort: [0], + }; beforeEach(() => { - mockElasticsearchClient.deleteByQuery.mockResolvedValue( - securityMock.createApiResponse({ body: {} as any }) + mockElasticsearchClient.openPointInTime.mockResolvedValue( + securityMock.createApiResponse({ + body: { id: 'PIT_ID' } as OpenPointInTimeResponse, + }) + ); + mockElasticsearchClient.closePointInTime.mockResolvedValue( + securityMock.createApiResponse({ + body: { succeeded: true, num_freed: 1 } as ClosePointInTimeResponse, + }) + ); + mockElasticsearchClient.search.mockResolvedValue( + securityMock.createApiResponse({ + body: { + hits: { hits: [sessionValue] }, + } as SearchResponse, + }) + ); + mockElasticsearchClient.bulk.mockResolvedValue( + securityMock.createApiResponse({ + body: { items: [{}] } as BulkResponse, + }) ); jest.spyOn(Date, 'now').mockImplementation(() => now); }); - it('throws if call to Elasticsearch fails', async () => { + it('throws if search call to Elasticsearch fails', async () => { const failureReason = new errors.ResponseError( securityMock.createApiResponse(securityMock.createApiResponse({ body: { type: 'Uh oh.' } })) ); - mockElasticsearchClient.deleteByQuery.mockRejectedValue(failureReason); + mockElasticsearchClient.search.mockRejectedValue(failureReason); await expect(sessionIndex.cleanUp()).rejects.toBe(failureReason); + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).not.toHaveBeenCalled(); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); + }); + + it('throws if bulk delete call to Elasticsearch fails', async () => { + const failureReason = new errors.ResponseError( + securityMock.createApiResponse(securityMock.createApiResponse({ body: { type: 'Uh oh.' } })) + ); + mockElasticsearchClient.bulk.mockRejectedValue(failureReason); + + await expect(sessionIndex.cleanUp()).rejects.toBe(failureReason); + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); }); it('when neither `lifespan` nor `idleTimeout` is configured', async () => { await sessionIndex.cleanUp(); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledTimes(1); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledWith( - { - index: indexName, - refresh: true, - body: { - query: { - bool: { - should: [ - // All expired sessions based on the lifespan, no matter which provider they belong to. - { range: { lifespanExpiration: { lte: now } } }, - // All sessions that belong to the providers that aren't configured. - { + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledWith({ + _source_includes: 'usernameHash,provider', + sort: '_shard_doc', + track_total_hits: false, + search_after: undefined, + size: 10_000, + pit: { + id: 'PIT_ID', + keep_alive: '5m', + }, + query: { + bool: { + should: [ + // All expired sessions based on the lifespan, no matter which provider they belong to. + { range: { lifespanExpiration: { lte: now } } }, + // All sessions that belong to the providers that aren't configured. + { + bool: { + must_not: { bool: { - must_not: { - bool: { - should: [ - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - }, - }, - ], - minimum_should_match: 1, + should: [ + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + }, }, - }, - }, - }, - // The sessions that belong to a particular provider that are expired based on the idle timeout. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, ], - should: [{ range: { idleTimeoutExpiration: { lte: now } } }], minimum_should_match: 1, }, }, - ], + }, }, - }, + // The sessions that belong to a particular provider that are expired based on the idle timeout. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + should: [{ range: { idleTimeoutExpiration: { lte: now } } }], + minimum_should_match: 1, + }, + }, + ], }, }, - { ignore: [409, 404] } + }); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledWith( + { + index: indexName, + operations: [{ delete: { _id: sessionValue._id } }], + refresh: false, + }, + { + ignore: [409, 404], + } ); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); }); it('when only `lifespan` is configured', async () => { @@ -299,69 +364,85 @@ describe('Session index', () => { { isTLSEnabled: false } ), elasticsearchClient: mockElasticsearchClient, + auditLogger: auditServiceMock.create().withoutRequest, }); await sessionIndex.cleanUp(); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledTimes(1); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledWith( - { - index: indexName, - refresh: true, - body: { - query: { - bool: { - should: [ - // All expired sessions based on the lifespan, no matter which provider they belong to. - { range: { lifespanExpiration: { lte: now } } }, - // All sessions that belong to the providers that aren't configured. - { + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledWith({ + _source_includes: 'usernameHash,provider', + sort: '_shard_doc', + track_total_hits: false, + search_after: undefined, + size: 10_000, + pit: { + id: 'PIT_ID', + keep_alive: '5m', + }, + query: { + bool: { + should: [ + // All expired sessions based on the lifespan, no matter which provider they belong to. + { range: { lifespanExpiration: { lte: now } } }, + // All sessions that belong to the providers that aren't configured. + { + bool: { + must_not: { bool: { - must_not: { - bool: { - should: [ - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - }, - }, - ], - minimum_should_match: 1, + should: [ + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + }, }, - }, - }, - }, - // The sessions that belong to a particular provider but don't have a configured lifespan. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - must_not: { exists: { field: 'lifespanExpiration' } }, - }, - }, - // The sessions that belong to a particular provider that are expired based on the idle timeout. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, ], - should: [{ range: { idleTimeoutExpiration: { lte: now } } }], minimum_should_match: 1, }, }, - ], + }, }, - }, + // The sessions that belong to a particular provider but don't have a configured lifespan. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + must_not: { exists: { field: 'lifespanExpiration' } }, + }, + }, + // The sessions that belong to a particular provider that are expired based on the idle timeout. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + should: [{ range: { idleTimeoutExpiration: { lte: now } } }], + minimum_should_match: 1, + }, + }, + ], }, }, - { ignore: [409, 404] } + }); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledWith( + { + index: indexName, + operations: [{ delete: { _id: sessionValue._id } }], + refresh: false, + }, + { + ignore: [409, 404], + } ); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); }); it('when only `idleTimeout` is configured', async () => { @@ -375,63 +456,79 @@ describe('Session index', () => { { isTLSEnabled: false } ), elasticsearchClient: mockElasticsearchClient, + auditLogger: auditServiceMock.create().withoutRequest, }); await sessionIndex.cleanUp(); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledTimes(1); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledWith( - { - index: indexName, - refresh: true, - body: { - query: { - bool: { - should: [ - // All expired sessions based on the lifespan, no matter which provider they belong to. - { range: { lifespanExpiration: { lte: now } } }, - // All sessions that belong to the providers that aren't configured. - { - bool: { - must_not: { - bool: { - should: [ - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - }, - }, - ], - minimum_should_match: 1, - }, - }, - }, - }, - // The sessions that belong to a particular provider that are either expired based on the idle timeout - // or don't have it configured at all. - { + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledWith({ + _source_includes: 'usernameHash,provider', + sort: '_shard_doc', + track_total_hits: false, + search_after: undefined, + size: 10_000, + pit: { + id: 'PIT_ID', + keep_alive: '5m', + }, + query: { + bool: { + should: [ + // All expired sessions based on the lifespan, no matter which provider they belong to. + { range: { lifespanExpiration: { lte: now } } }, + // All sessions that belong to the providers that aren't configured. + { + bool: { + must_not: { bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], should: [ - { range: { idleTimeoutExpiration: { lte: now - 3 * idleTimeout } } }, - { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + }, + }, ], minimum_should_match: 1, }, }, - ], + }, }, - }, + // The sessions that belong to a particular provider that are either expired based on the idle timeout + // or don't have it configured at all. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + should: [ + { range: { idleTimeoutExpiration: { lte: now - 3 * idleTimeout } } }, + { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + ], + minimum_should_match: 1, + }, + }, + ], }, }, - { ignore: [409, 404] } + }); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledWith( + { + index: indexName, + operations: [{ delete: { _id: sessionValue._id } }], + refresh: false, + }, + { + ignore: [409, 404], + } ); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); }); it('when both `lifespan` and `idleTimeout` are configured', async () => { @@ -445,73 +542,89 @@ describe('Session index', () => { { isTLSEnabled: false } ), elasticsearchClient: mockElasticsearchClient, + auditLogger: auditServiceMock.create().withoutRequest, }); await sessionIndex.cleanUp(); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledTimes(1); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledWith( - { - index: indexName, - refresh: true, - body: { - query: { - bool: { - should: [ - // All expired sessions based on the lifespan, no matter which provider they belong to. - { range: { lifespanExpiration: { lte: now } } }, - // All sessions that belong to the providers that aren't configured. - { - bool: { - must_not: { - bool: { - should: [ - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - }, - }, - ], - minimum_should_match: 1, - }, - }, - }, - }, - // The sessions that belong to a particular provider but don't have a configured lifespan. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], - must_not: { exists: { field: 'lifespanExpiration' } }, - }, - }, - // The sessions that belong to a particular provider that are either expired based on the idle timeout - // or don't have it configured at all. - { + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledWith({ + _source_includes: 'usernameHash,provider', + sort: '_shard_doc', + track_total_hits: false, + search_after: undefined, + size: 10_000, + pit: { + id: 'PIT_ID', + keep_alive: '5m', + }, + query: { + bool: { + should: [ + // All expired sessions based on the lifespan, no matter which provider they belong to. + { range: { lifespanExpiration: { lte: now } } }, + // All sessions that belong to the providers that aren't configured. + { + bool: { + must_not: { bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic' } }, - ], should: [ - { range: { idleTimeoutExpiration: { lte: now - 3 * idleTimeout } } }, - { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + }, + }, ], minimum_should_match: 1, }, }, - ], + }, }, - }, + // The sessions that belong to a particular provider but don't have a configured lifespan. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + must_not: { exists: { field: 'lifespanExpiration' } }, + }, + }, + // The sessions that belong to a particular provider that are either expired based on the idle timeout + // or don't have it configured at all. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic' } }, + ], + should: [ + { range: { idleTimeoutExpiration: { lte: now - 3 * idleTimeout } } }, + { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + ], + minimum_should_match: 1, + }, + }, + ], }, }, - { ignore: [409, 404] } + }); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledWith( + { + index: indexName, + operations: [{ delete: { _id: sessionValue._id } }], + refresh: false, + }, + { + ignore: [409, 404], + } ); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); }); it('when both `lifespan` and `idleTimeout` are configured and multiple providers are enabled', async () => { @@ -540,105 +653,167 @@ describe('Session index', () => { { isTLSEnabled: false } ), elasticsearchClient: mockElasticsearchClient, + auditLogger: auditServiceMock.create().withoutRequest, }); await sessionIndex.cleanUp(); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledTimes(1); - expect(mockElasticsearchClient.deleteByQuery).toHaveBeenCalledWith( - { - index: indexName, - refresh: true, - body: { - query: { - bool: { - should: [ - // All expired sessions based on the lifespan, no matter which provider they belong to. - { range: { lifespanExpiration: { lte: now } } }, - // All sessions that belong to the providers that aren't configured. - { - bool: { - must_not: { - bool: { - should: [ - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic1' } }, - ], - }, - }, - { - bool: { - must: [ - { term: { 'provider.type': 'saml' } }, - { term: { 'provider.name': 'saml1' } }, - ], - }, - }, - ], - minimum_should_match: 1, - }, - }, - }, - }, - // The sessions that belong to a Basic provider but don't have a configured lifespan. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic1' } }, - ], - must_not: { exists: { field: 'lifespanExpiration' } }, - }, - }, - // The sessions that belong to a Basic provider that are either expired based on the idle timeout - // or don't have it configured at all. - { - bool: { - must: [ - { term: { 'provider.type': 'basic' } }, - { term: { 'provider.name': 'basic1' } }, - ], - should: [ - { range: { idleTimeoutExpiration: { lte: now - 3 * globalIdleTimeout } } }, - { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, - ], - minimum_should_match: 1, - }, - }, - // The sessions that belong to a SAML provider but don't have a configured lifespan. - { - bool: { - must: [ - { term: { 'provider.type': 'saml' } }, - { term: { 'provider.name': 'saml1' } }, - ], - must_not: { exists: { field: 'lifespanExpiration' } }, - }, - }, - // The sessions that belong to a SAML provider that are either expired based on the idle timeout - // or don't have it configured at all. - { + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledWith({ + _source_includes: 'usernameHash,provider', + sort: '_shard_doc', + track_total_hits: false, + search_after: undefined, + size: 10_000, + pit: { + id: 'PIT_ID', + keep_alive: '5m', + }, + query: { + bool: { + should: [ + // All expired sessions based on the lifespan, no matter which provider they belong to. + { range: { lifespanExpiration: { lte: now } } }, + // All sessions that belong to the providers that aren't configured. + { + bool: { + must_not: { bool: { - must: [ - { term: { 'provider.type': 'saml' } }, - { term: { 'provider.name': 'saml1' } }, - ], should: [ - { range: { idleTimeoutExpiration: { lte: now - 3 * samlIdleTimeout } } }, - { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic1' } }, + ], + }, + }, + { + bool: { + must: [ + { term: { 'provider.type': 'saml' } }, + { term: { 'provider.name': 'saml1' } }, + ], + }, + }, ], minimum_should_match: 1, }, }, - ], + }, }, - }, + // The sessions that belong to a Basic provider but don't have a configured lifespan. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic1' } }, + ], + must_not: { exists: { field: 'lifespanExpiration' } }, + }, + }, + // The sessions that belong to a Basic provider that are either expired based on the idle timeout + // or don't have it configured at all. + { + bool: { + must: [ + { term: { 'provider.type': 'basic' } }, + { term: { 'provider.name': 'basic1' } }, + ], + should: [ + { range: { idleTimeoutExpiration: { lte: now - 3 * globalIdleTimeout } } }, + { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + ], + minimum_should_match: 1, + }, + }, + // The sessions that belong to a SAML provider but don't have a configured lifespan. + { + bool: { + must: [ + { term: { 'provider.type': 'saml' } }, + { term: { 'provider.name': 'saml1' } }, + ], + must_not: { exists: { field: 'lifespanExpiration' } }, + }, + }, + // The sessions that belong to a SAML provider that are either expired based on the idle timeout + // or don't have it configured at all. + { + bool: { + must: [ + { term: { 'provider.type': 'saml' } }, + { term: { 'provider.name': 'saml1' } }, + ], + should: [ + { range: { idleTimeoutExpiration: { lte: now - 3 * samlIdleTimeout } } }, + { bool: { must_not: { exists: { field: 'idleTimeoutExpiration' } } } }, + ], + minimum_should_match: 1, + }, + }, + ], }, }, - { ignore: [409, 404] } + }); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledWith( + { + index: indexName, + operations: [{ delete: { _id: sessionValue._id } }], + refresh: false, + }, + { + ignore: [409, 404], + } + ); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); + }); + + it('should clean up sessions in batches of 10,000', async () => { + for (const count of [10_000, 1]) { + mockElasticsearchClient.search.mockResolvedValueOnce( + securityMock.createApiResponse({ + body: { + hits: { hits: new Array(count).fill(sessionValue, 0) }, + } as SearchResponse, + }) + ); + } + + await sessionIndex.cleanUp(); + + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(2); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(2); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); + }); + + it('should limit number of batches to 10', async () => { + mockElasticsearchClient.search.mockResolvedValue( + securityMock.createApiResponse({ + body: { + hits: { hits: new Array(10_000).fill(sessionValue, 0) }, + } as SearchResponse, + }) + ); + + await sessionIndex.cleanUp(); + + expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1); + expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(10); + expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(10); + expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); + }); + + it('should log audit event', async () => { + await sessionIndex.cleanUp(); + + expect(auditLogger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: { action: 'session_cleanup', category: ['authentication'], outcome: 'unknown' }, + }) ); }); }); diff --git a/x-pack/plugins/security/server/session_management/session_index.ts b/x-pack/plugins/security/server/session_management/session_index.ts index 801597dad6baf..d05a11c417678 100644 --- a/x-pack/plugins/security/server/session_management/session_index.ts +++ b/x-pack/plugins/security/server/session_management/session_index.ts @@ -5,9 +5,16 @@ * 2.0. */ +import type { + BulkOperationContainer, + SearchSortResults, +} from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; + import type { ElasticsearchClient, Logger } from 'src/core/server'; import type { AuthenticationProvider } from '../../common/model'; +import type { AuditLogger } from '../audit'; +import { sessionCleanupEvent } from '../audit'; import type { ConfigType } from '../config'; export interface SessionIndexOptions { @@ -15,6 +22,7 @@ export interface SessionIndexOptions { readonly kibanaIndexName: string; readonly config: Pick; readonly logger: Logger; + readonly auditLogger: AuditLogger; } /** @@ -34,6 +42,22 @@ export type InvalidateSessionsFilter = */ const SESSION_INDEX_TEMPLATE_VERSION = 1; +/** + * Number of sessions to remove per batch during cleanup. + */ +const SESSION_INDEX_CLEANUP_BATCH_SIZE = 10_000; + +/** + * Maximum number of batches per cleanup. + * If the batch size is 10,000 and this limit is 10, then Kibana will remove up to 100k sessions per cleanup. + */ +const SESSION_INDEX_CLEANUP_BATCH_LIMIT = 10; + +/** + * How long the session cleanup search point-in-time should be kept alive. + */ +const SESSION_INDEX_CLEANUP_KEEP_ALIVE = '5m'; + /** * Returns index template that is used for the current version of the session index. */ @@ -425,6 +449,56 @@ export class SessionIndex { async cleanUp() { this.options.logger.debug(`Running cleanup routine.`); + try { + for await (const sessionValues of this.getSessionValuesInBatches()) { + const operations: Array>> = []; + sessionValues.forEach(({ _id, _source }) => { + const { usernameHash, provider } = _source!; + this.options.auditLogger.log( + sessionCleanupEvent({ sessionId: _id, usernameHash, provider }) + ); + operations.push({ delete: { _id } }); + }); + if (operations.length > 0) { + const { body: bulkResponse } = await this.options.elasticsearchClient.bulk( + { + index: this.indexName, + operations, + refresh: false, + }, + { ignore: [409, 404] } + ); + if (bulkResponse.errors) { + const errorCount = bulkResponse.items.reduce( + (count, item) => (item.delete!.error ? count + 1 : count), + 0 + ); + if (errorCount < bulkResponse.items.length) { + this.options.logger.warn( + `Failed to clean up ${errorCount} of ${bulkResponse.items.length} invalid or expired sessions. The remaining sessions were cleaned up successfully.` + ); + } else { + this.options.logger.error( + `Failed to clean up ${bulkResponse.items.length} invalid or expired sessions.` + ); + } + } else { + this.options.logger.debug( + `Cleaned up ${bulkResponse.items.length} invalid or expired sessions.` + ); + } + } + } + } catch (err) { + this.options.logger.error(`Failed to clean up sessions: ${err.message}`); + throw err; + } + } + + /** + * Fetches session values from session index in batches of 10,000. + */ + private async *getSessionValuesInBatches() { const now = Date.now(); const providersSessionConfig = this.options.config.authc.sortedProviders.map((provider) => { return { @@ -484,24 +558,37 @@ export class SessionIndex { }); } - try { - const { body: response } = await this.options.elasticsearchClient.deleteByQuery( - { - index: this.indexName, - refresh: true, - body: { query: { bool: { should: deleteQueries } } }, - }, - { ignore: [409, 404] } - ); + const { body: openPitResponse } = await this.options.elasticsearchClient.openPointInTime({ + index: this.indexName, + keep_alive: SESSION_INDEX_CLEANUP_KEEP_ALIVE, + }); - if (response.deleted! > 0) { - this.options.logger.debug( - `Cleaned up ${response.deleted} invalid or expired session values.` - ); + try { + let searchAfter: SearchSortResults | undefined; + for (let i = 0; i < SESSION_INDEX_CLEANUP_BATCH_LIMIT; i++) { + const { body: searchResponse } = + await this.options.elasticsearchClient.search({ + pit: { id: openPitResponse.id, keep_alive: SESSION_INDEX_CLEANUP_KEEP_ALIVE }, + _source_includes: 'usernameHash,provider', + query: { bool: { should: deleteQueries } }, + search_after: searchAfter, + size: SESSION_INDEX_CLEANUP_BATCH_SIZE, + sort: '_shard_doc', + track_total_hits: false, // for performance + }); + const { hits } = searchResponse.hits; + if (hits.length > 0) { + yield hits; + searchAfter = hits[hits.length - 1].sort; + } + if (hits.length < SESSION_INDEX_CLEANUP_BATCH_SIZE) { + break; + } } - } catch (err) { - this.options.logger.error(`Failed to clean up sessions: ${err.message}`); - throw err; + } finally { + await this.options.elasticsearchClient.closePointInTime({ + id: openPitResponse.id, + }); } } } diff --git a/x-pack/plugins/security/server/session_management/session_management_service.test.ts b/x-pack/plugins/security/server/session_management/session_management_service.test.ts index 7e99181981e85..100d0b30082c6 100644 --- a/x-pack/plugins/security/server/session_management/session_management_service.test.ts +++ b/x-pack/plugins/security/server/session_management/session_management_service.test.ts @@ -15,6 +15,8 @@ import type { TaskRunCreatorFunction, } from '../../../task_manager/server'; import { taskManagerMock } from '../../../task_manager/server/mocks'; +import type { AuditLogger } from '../audit'; +import { auditServiceMock } from '../audit/index.mock'; import { ConfigSchema, createConfig } from '../config'; import type { OnlineStatusRetryScheduler } from '../elasticsearch'; import { Session } from './session'; @@ -24,10 +26,23 @@ import { SessionManagementService, } from './session_management_service'; +const mockSessionIndexInitialize = jest.spyOn(SessionIndex.prototype, 'initialize'); +mockSessionIndexInitialize.mockResolvedValue(); + +const mockSessionIndexCleanUp = jest.spyOn(SessionIndex.prototype, 'cleanUp'); +mockSessionIndexCleanUp.mockResolvedValue(); + describe('SessionManagementService', () => { let service: SessionManagementService; + let auditLogger: AuditLogger; beforeEach(() => { service = new SessionManagementService(loggingSystemMock.createLogger()); + auditLogger = auditServiceMock.create().withoutRequest; + }); + + afterEach(() => { + mockSessionIndexInitialize.mockReset(); + mockSessionIndexCleanUp.mockReset(); }); describe('setup()', () => { @@ -56,12 +71,9 @@ describe('SessionManagementService', () => { }); describe('start()', () => { - let mockSessionIndexInitialize: jest.SpyInstance; let mockTaskManager: jest.Mocked; let sessionCleanupTaskRunCreator: TaskRunCreatorFunction; beforeEach(() => { - mockSessionIndexInitialize = jest.spyOn(SessionIndex.prototype, 'initialize'); - mockTaskManager = taskManagerMock.createStart(); mockTaskManager.ensureScheduled.mockResolvedValue(undefined as any); @@ -84,14 +96,11 @@ describe('SessionManagementService', () => { sessionCleanupTaskRunCreator = createTaskRunner; }); - afterEach(() => { - mockSessionIndexInitialize.mockReset(); - }); - it('exposes proper contract', () => { const mockStatusSubject = new Subject(); expect( service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -100,10 +109,10 @@ describe('SessionManagementService', () => { ).toEqual({ session: expect.any(Session) }); }); - it('registers proper session index cleanup task runner', () => { - const mockSessionIndexCleanUp = jest.spyOn(SessionIndex.prototype, 'cleanUp'); + it('registers proper session index cleanup task runner', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -113,16 +122,17 @@ describe('SessionManagementService', () => { expect(mockSessionIndexCleanUp).not.toHaveBeenCalled(); const runner = sessionCleanupTaskRunCreator({} as any); - runner.run(); + await runner.run(); expect(mockSessionIndexCleanUp).toHaveBeenCalledTimes(1); - runner.run(); + await runner.run(); expect(mockSessionIndexCleanUp).toHaveBeenCalledTimes(2); }); it('initializes session index and schedules session index cleanup task when Elasticsearch goes online', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -160,6 +170,7 @@ describe('SessionManagementService', () => { it('removes old cleanup task if cleanup interval changes', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -195,6 +206,7 @@ describe('SessionManagementService', () => { it('does not remove old cleanup task if cleanup interval does not change', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -221,6 +233,7 @@ describe('SessionManagementService', () => { it('schedules retry if index initialization fails', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -257,6 +270,7 @@ describe('SessionManagementService', () => { it('schedules retry if cleanup task registration fails', async () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), @@ -291,11 +305,8 @@ describe('SessionManagementService', () => { }); describe('stop()', () => { - let mockSessionIndexInitialize: jest.SpyInstance; let mockTaskManager: jest.Mocked; beforeEach(() => { - mockSessionIndexInitialize = jest.spyOn(SessionIndex.prototype, 'initialize'); - mockTaskManager = taskManagerMock.createStart(); mockTaskManager.ensureScheduled.mockResolvedValue(undefined as any); @@ -309,13 +320,10 @@ describe('SessionManagementService', () => { }); }); - afterEach(() => { - mockSessionIndexInitialize.mockReset(); - }); - it('properly unsubscribes from status updates', () => { const mockStatusSubject = new Subject(); service.start({ + auditLogger, elasticsearchClient: elasticsearchServiceMock.createElasticsearchClient(), kibanaIndexName: '.kibana', online$: mockStatusSubject.asObservable(), diff --git a/x-pack/plugins/security/server/session_management/session_management_service.ts b/x-pack/plugins/security/server/session_management/session_management_service.ts index fcd8e8c53cbe5..03a5d6130c3c1 100644 --- a/x-pack/plugins/security/server/session_management/session_management_service.ts +++ b/x-pack/plugins/security/server/session_management/session_management_service.ts @@ -14,6 +14,7 @@ import type { TaskManagerSetupContract, TaskManagerStartContract, } from '../../../task_manager/server'; +import type { AuditLogger } from '../audit'; import type { ConfigType } from '../config'; import type { OnlineStatusRetryScheduler } from '../elasticsearch'; import { Session } from './session'; @@ -31,6 +32,7 @@ export interface SessionManagementServiceStartParams { readonly kibanaIndexName: string; readonly online$: Observable; readonly taskManager: TaskManagerStartContract; + readonly auditLogger: AuditLogger; } export interface SessionManagementServiceStart { @@ -78,12 +80,14 @@ export class SessionManagementService { kibanaIndexName, online$, taskManager, + auditLogger, }: SessionManagementServiceStartParams): SessionManagementServiceStart { this.sessionIndex = new SessionIndex({ config: this.config, elasticsearchClient, kibanaIndexName, logger: this.logger.get('index'), + auditLogger, }); this.statusSubscription = online$.subscribe(async ({ scheduleRetry }) => { From c2846881e534d74b421c663870a3b4c2cc2b2e6f Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 18 Jan 2022 12:48:08 +0100 Subject: [PATCH 09/10] [Discover] Unskip and improve runtime field test (#123054) (#123161) (cherry picked from commit b6473c9aa3850c12abff4cddf0738b36a555b574) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../apps/discover/_runtime_fields_editor.ts | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/test/functional/apps/discover/_runtime_fields_editor.ts b/test/functional/apps/discover/_runtime_fields_editor.ts index 4757807cb7ac1..2e21b2e1f8ec6 100644 --- a/test/functional/apps/discover/_runtime_fields_editor.ts +++ b/test/functional/apps/discover/_runtime_fields_editor.ts @@ -31,8 +31,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await fieldEditor.save(); }; - // Failing: https://github.com/elastic/kibana/issues/111922 - describe.skip('discover integration with runtime fields editor', function describeIndexTests() { + describe('discover integration with runtime fields editor', function describeIndexTests() { before(async function () { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); @@ -63,7 +62,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('allows creation of a new field', async function () { await createRuntimeField('runtimefield'); await PageObjects.header.waitUntilLoadingHasFinished(); - await retry.waitFor('fieldNames to include runtimefield', async () => { + await retry.waitForWithTimeout('fieldNames to include runtimefield', 5000, async () => { const fieldNames = await PageObjects.discover.getAllFieldNames(); return fieldNames.includes('runtimefield'); }); @@ -76,7 +75,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await fieldEditor.confirmSave(); await PageObjects.header.waitUntilLoadingHasFinished(); - await retry.waitFor('fieldNames to include edits', async () => { + await retry.waitForWithTimeout('fieldNames to include edits', 5000, async () => { const fieldNames = await PageObjects.discover.getAllFieldNames(); return fieldNames.includes('runtimefield edited'); }); @@ -105,7 +104,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.discover.removeField('delete'); await fieldEditor.confirmDelete(); await PageObjects.header.waitUntilLoadingHasFinished(); - await retry.waitFor('fieldNames to include edits', async () => { + await retry.waitForWithTimeout('fieldNames to include edits', 5000, async () => { const fieldNames = await PageObjects.discover.getAllFieldNames(); return !fieldNames.includes('delete'); }); @@ -127,16 +126,22 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await rowActions[idxToClick].click(); }); - await retry.waitFor('doc viewer is displayed with runtime field', async () => { - const hasDocHit = await testSubjects.exists('doc-hit'); - if (!hasDocHit) { - // Maybe loading has not completed - throw new Error('test subject doc-hit is not yet displayed'); + await retry.waitForWithTimeout( + 'doc viewer is displayed with runtime field', + 5000, + async () => { + const hasDocHit = await testSubjects.exists('doc-hit'); + if (!hasDocHit) { + // Maybe loading has not completed + throw new Error('test subject doc-hit is not yet displayed'); + } + const runtimeFieldsRow = await testSubjects.exists( + 'tableDocViewRow-discover runtimefield' + ); + + return hasDocHit && runtimeFieldsRow; } - const runtimeFieldsRow = await testSubjects.exists('tableDocViewRow-discover runtimefield'); - - return hasDocHit && runtimeFieldsRow; - }); + ); }); }); } From 153c4b7fe0870a4bd598918b00ed96a6fafde440 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Tue, 18 Jan 2022 13:48:31 +0100 Subject: [PATCH 10/10] [Cases] Fix form case tags displaying error when all tags are removed (#123172) (#123207) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit d7aacd723c3154c30f8c3c42e39e31132f421b1d) --- .../cases/public/components/create/schema.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/public/components/create/schema.tsx b/x-pack/plugins/cases/public/components/create/schema.tsx index 435ebefd943ca..ae9d03a673842 100644 --- a/x-pack/plugins/cases/public/components/create/schema.tsx +++ b/x-pack/plugins/cases/public/components/create/schema.tsx @@ -18,6 +18,8 @@ import * as i18n from './translations'; import { OptionalFieldLabel } from './optional_field_label'; const { emptyField, maxLengthField } = fieldValidators; +const isEmptyString = (value: string) => value.trim() === ''; + export const schemaTags = { type: FIELD_TYPES.COMBO_BOX, label: i18n.TAGS, @@ -25,7 +27,16 @@ export const schemaTags = { labelAppend: OptionalFieldLabel, validations: [ { - validator: emptyField(i18n.TAGS_EMPTY_ERROR), + validator: ({ value }: { value: string | string[] }) => { + if ( + (!Array.isArray(value) && isEmptyString(value)) || + (Array.isArray(value) && value.length > 0 && value.find(isEmptyString)) + ) { + return { + message: i18n.TAGS_EMPTY_ERROR, + }; + } + }, type: VALIDATION_TYPES.ARRAY_ITEM, isBlocking: false, },