Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Detections] Refactor ML calls for newest ML permissions #74582

Merged
merged 25 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7e7e013
Refactor ML Capabilities provider to leverage useAsync
rylnd Aug 5, 2020
12a6077
Add new UseSecurityJobs hook
rylnd Aug 5, 2020
5be0906
Filter out non-security jobs when collecting Detections telemetry
rylnd Aug 5, 2020
d3e0460
Temporarily relax our helper types
rylnd Aug 5, 2020
49ca692
Creates pure function and hook for fetching ML Jobs summary
rylnd Aug 5, 2020
e5a842f
Adds hook to fetch installed Security jobs
rylnd Aug 6, 2020
36d9d4b
Replace remaining uses of useSiemJobs
rylnd Aug 6, 2020
d195945
Delete unused hook
rylnd Aug 6, 2020
a527d5d
Rename siemJob -> securityJob
rylnd Aug 6, 2020
9fc91f3
Rename files to be more accurate
rylnd Aug 6, 2020
bd2c199
Move useMlCapabilities hook to more general folder
rylnd Aug 6, 2020
12f27ac
Fixes type errors
rylnd Aug 6, 2020
23beb1f
Replace our internal JobSummary type with MlSummaryJob from ml
rylnd Aug 6, 2020
f4781c2
Mocks cleanup
rylnd Aug 6, 2020
62b479f
Merge branch 'master' into fix_unauthed_ml_calls
rylnd Aug 6, 2020
66365dc
Add tests for useSecurityJobs hook
rylnd Aug 6, 2020
f376baa
Do not call ML endpoints without valid licensing
rylnd Aug 6, 2020
404974c
Add tests for useInstalledSecurityJobs hook
rylnd Aug 6, 2020
96f3608
Add helper predicate for checking for a valid license
rylnd Aug 7, 2020
f7aa1fd
Revert i18n key change
rylnd Aug 10, 2020
be0a06c
Revert to old functionality on rule creation dropdown
rylnd Aug 11, 2020
7f6aaae
Replace inline mock with an existing one
rylnd Aug 11, 2020
70b7821
Revert to old functionality on Rule Details
rylnd Aug 11, 2020
b41dfd3
Merge branch 'master' into fix_unauthed_ml_calls
elasticmachine Aug 12, 2020
15f26ec
Merge branch 'master' into fix_unauthed_ml_calls
elasticmachine Aug 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export interface MlSummaryJob {
export interface AuditMessage {
job_id: string;
msgTime: number;
level: number;
highestLevel: number;
level: string;
highestLevel: string;
highestLevelText: string;
text: string;
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ml/public/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export * from '../common/constants/anomalies';
export * from '../common/types/data_recognizer';
export * from '../common/types/capabilities';
export * from '../common/types/anomalies';
export * from '../common/types/anomaly_detection_jobs';
export * from '../common/types/modules';
export * from '../common/types/audit_message';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { emptyMlCapabilities } from './empty_ml_capabilities';
import { hasMlLicense } from './has_ml_license';

describe('hasMlLicense', () => {
test('it returns false when license is not platinum or trial', () => {
const capabilities = { ...emptyMlCapabilities, isPlatinumOrTrialLicense: false };
expect(hasMlLicense(capabilities)).toEqual(false);
});

test('it returns true when license is platinum or trial', () => {
const capabilities = { ...emptyMlCapabilities, isPlatinumOrTrialLicense: true };
expect(hasMlLicense(capabilities)).toEqual(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { MlCapabilitiesResponse } from '../../../ml/common/types/capabilities';

export const hasMlLicense = (capabilities: MlCapabilitiesResponse): boolean =>
capabilities.isPlatinumOrTrialLicense;
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { MlSummaryJob } from '../../../ml/common/types/anomaly_detection_jobs';
import { ML_GROUP_IDS } from '../constants';

export const isSecurityJob = (job: MlSummaryJob): boolean =>
export const isSecurityJob = (job: { groups: string[] }): boolean =>
job.groups.some((group) => ML_GROUP_IDS.includes(group));
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import { useState, useEffect, useMemo } from 'react';
import { DEFAULT_ANOMALY_SCORE } from '../../../../../common/constants';
import { anomaliesTableData } from '../api/anomalies_table_data';
import { InfluencerInput, Anomalies, CriteriaFields } from '../types';
import { hasMlUserPermissions } from '../../../../../common/machine_learning/has_ml_user_permissions';
import { useSiemJobs } from '../../ml_popover/hooks/use_siem_jobs';
import { useMlCapabilities } from '../../ml_popover/hooks/use_ml_capabilities';
import { useStateToaster, errorToToaster } from '../../toasters';

import * as i18n from './translations';
import { useTimeZone, useUiSetting$ } from '../../../lib/kibana';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { useInstalledSecurityJobs } from '../hooks/use_installed_security_jobs';

interface Args {
influencers?: InfluencerInput[];
Expand Down Expand Up @@ -58,15 +56,13 @@ export const useAnomaliesTableData = ({
skip = false,
}: Args): Return => {
const [tableData, setTableData] = useState<Anomalies | null>(null);
const [, siemJobs] = useSiemJobs(true);
const { isMlUser, jobs } = useInstalledSecurityJobs();
const [loading, setLoading] = useState(true);
const capabilities = useMlCapabilities();
const userPermissions = hasMlUserPermissions(capabilities);
const [, dispatchToaster] = useStateToaster();
const { addError } = useAppToasts();
const timeZone = useTimeZone();
const [anomalyScore] = useUiSetting$<number>(DEFAULT_ANOMALY_SCORE);

const siemJobIds = siemJobs.filter((job) => job.isInstalled).map((job) => job.id);
const jobIds = jobs.map((job) => job.id);
const startDateMs = useMemo(() => new Date(startDate).getTime(), [startDate]);
const endDateMs = useMemo(() => new Date(endDate).getTime(), [endDate]);

Expand All @@ -81,11 +77,11 @@ export const useAnomaliesTableData = ({
earliestMs: number,
latestMs: number
) {
if (userPermissions && !skip && siemJobIds.length > 0) {
if (isMlUser && !skip && jobIds.length > 0) {
try {
const data = await anomaliesTableData(
{
jobIds: siemJobIds,
jobIds,
criteriaFields: criteriaFieldsInput,
aggregationInterval: 'auto',
threshold: getThreshold(anomalyScore, threshold),
Expand All @@ -104,13 +100,13 @@ export const useAnomaliesTableData = ({
}
} catch (error) {
if (isSubscribed) {
errorToToaster({ title: i18n.SIEM_TABLE_FETCH_FAILURE, error, dispatchToaster });
addError(error, { title: i18n.SIEM_TABLE_FETCH_FAILURE });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ for refactoring to leverage useAppToasts, thanks!

setLoading(false);
}
}
} else if (!userPermissions && isSubscribed) {
} else if (!isMlUser && isSubscribed) {
setLoading(false);
} else if (siemJobIds.length === 0 && isSubscribed) {
} else if (jobIds.length === 0 && isSubscribed) {
setLoading(false);
} else if (isSubscribed) {
setTableData(null);
Expand All @@ -132,9 +128,9 @@ export const useAnomaliesTableData = ({
startDateMs,
endDateMs,
skip,
userPermissions,
isMlUser,
// eslint-disable-next-line react-hooks/exhaustive-deps
siemJobIds.sort().join(),
jobIds.sort().join(),
]);

return [loading, tableData];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { HttpSetup } from '../../../../../../../../src/core/public';
import { MlSummaryJob } from '../../../../../../ml/public';

export interface GetJobsSummaryArgs {
http: HttpSetup;
jobIds?: string[];
signal: AbortSignal;
}

/**
* Fetches a summary of all ML jobs currently installed
*
* @param http HTTP Service
* @param jobIds Array of job IDs to filter against
* @param signal to cancel request
*
* @throws An error if response is not OK
*/
export const getJobsSummary = async ({
http,
jobIds,
signal,
}: GetJobsSummaryArgs): Promise<MlSummaryJob[]> =>
http.fetch<MlSummaryJob[]>('/api/ml/jobs/jobs_summary', {
method: 'POST',
body: JSON.stringify({ jobIds: jobIds ?? [] }),
asSystemRequest: true,
signal,
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { HttpSetup } from '../../../../../../../../src/core/public';
import { MlCapabilitiesResponse } from '../../../../../../ml/public';
import { KibanaServices } from '../../../lib/kibana';
import { InfluencerInput } from '../types';

export interface Body {
Expand All @@ -21,10 +21,15 @@ export interface Body {
maxExamples: number;
}

export const getMlCapabilities = async (signal: AbortSignal): Promise<MlCapabilitiesResponse> => {
return KibanaServices.get().http.fetch<MlCapabilitiesResponse>('/api/ml/ml_capabilities', {
export const getMlCapabilities = async ({
http,
signal,
}: {
http: HttpSetup;
signal: AbortSignal;
}): Promise<MlCapabilitiesResponse> =>
http.fetch<MlCapabilitiesResponse>('/api/ml/ml_capabilities', {
method: 'GET',
asSystemRequest: true,
signal,
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { useAsync, withOptionalSignal } from '../../../../shared_imports';
import { getJobsSummary } from '../api/get_jobs_summary';

const _getJobsSummary = withOptionalSignal(getJobsSummary);

export const useGetJobsSummary = () => useAsync(_getJobsSummary);
Comment on lines +10 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really liking how the new composable hooks are turning out. Keeps the actual API request logic small and easy to grok, while making creating specific hooks like above super simple. Autocomplete is all 👍 too, so that's nice as well. Thanks for all the effort here @rylnd 🙂

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { getMlCapabilities } from '../api/get_ml_capabilities';
import { useAsync, withOptionalSignal } from '../../../../shared_imports';

const _getMlCapabilities = withOptionalSignal(getMlCapabilities);

export const useGetMlCapabilities = () => useAsync(_getMlCapabilities);
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { renderHook } from '@testing-library/react-hooks';

import { hasMlUserPermissions } from '../../../../../common/machine_learning/has_ml_user_permissions';
import { hasMlLicense } from '../../../../../common/machine_learning/has_ml_license';
import { isSecurityJob } from '../../../../../common/machine_learning/is_security_job';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { useAppToastsMock } from '../../../hooks/use_app_toasts.mock';
import { mockJobsSummaryResponse } from '../../ml_popover/api.mock';
import { getJobsSummary } from '../api/get_jobs_summary';
import { useInstalledSecurityJobs } from './use_installed_security_jobs';

jest.mock('../../../../../common/machine_learning/has_ml_user_permissions');
jest.mock('../../../../../common/machine_learning/has_ml_license');
jest.mock('../../../hooks/use_app_toasts');
jest.mock('../api/get_jobs_summary');

describe('useInstalledSecurityJobs', () => {
let appToastsMock: jest.Mocked<ReturnType<typeof useAppToastsMock.create>>;

beforeEach(() => {
appToastsMock = useAppToastsMock.create();
(useAppToasts as jest.Mock).mockReturnValue(appToastsMock);
(getJobsSummary as jest.Mock).mockResolvedValue(mockJobsSummaryResponse);
});

describe('when the user has permissions', () => {
beforeEach(() => {
(hasMlUserPermissions as jest.Mock).mockReturnValue(true);
(hasMlLicense as jest.Mock).mockReturnValue(true);
});

it('returns jobs and permissions', async () => {
const { result, waitForNextUpdate } = renderHook(() => useInstalledSecurityJobs());
await waitForNextUpdate();

expect(result.current.jobs).toHaveLength(3);
expect(result.current.jobs).toEqual(
expect.arrayContaining([
{
datafeedId: 'datafeed-siem-api-rare_process_linux_ecs',
datafeedIndices: ['auditbeat-*'],
datafeedState: 'stopped',
description: 'SIEM Auditbeat: Detect unusually rare processes on Linux (beta)',
earliestTimestampMs: 1557353420495,
groups: ['siem'],
hasDatafeed: true,
id: 'siem-api-rare_process_linux_ecs',
isSingleMetricViewerJob: true,
jobState: 'closed',
latestTimestampMs: 1557434782207,
memory_status: 'hard_limit',
processed_record_count: 582251,
},
])
);
expect(result.current.isMlUser).toEqual(true);
expect(result.current.isLicensed).toEqual(true);
});

it('filters out non-security jobs', async () => {
const { result, waitForNextUpdate } = renderHook(() => useInstalledSecurityJobs());
await waitForNextUpdate();

expect(result.current.jobs.length).toBeGreaterThan(0);
expect(result.current.jobs.every(isSecurityJob)).toEqual(true);
});

it('renders a toast error if the ML call fails', async () => {
(getJobsSummary as jest.Mock).mockRejectedValue('whoops');
const { waitForNextUpdate } = renderHook(() => useInstalledSecurityJobs());
await waitForNextUpdate();

expect(appToastsMock.addError).toHaveBeenCalledWith('whoops', {
title: 'Security job fetch failure',
});
Comment on lines +74 to +81
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had a gap in our coverage around the ML hooks for a long time, so awesome to see these tests! 🚀 🎉

});
});

describe('when the user does not have valid permissions', () => {
beforeEach(() => {
(hasMlUserPermissions as jest.Mock).mockReturnValue(false);
(hasMlLicense as jest.Mock).mockReturnValue(false);
});

it('returns empty jobs and false predicates', () => {
const { result } = renderHook(() => useInstalledSecurityJobs());

expect(result.current.jobs).toEqual([]);
expect(result.current.isMlUser).toEqual(false);
expect(result.current.isLicensed).toEqual(false);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { useEffect, useState } from 'react';

import { MlSummaryJob } from '../../../../../../ml/public';
import { hasMlUserPermissions } from '../../../../../common/machine_learning/has_ml_user_permissions';
import { hasMlLicense } from '../../../../../common/machine_learning/has_ml_license';
import { isSecurityJob } from '../../../../../common/machine_learning/is_security_job';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { useHttp } from '../../../lib/kibana';
import { useMlCapabilities } from './use_ml_capabilities';
import * as i18n from '../translations';
import { useGetJobsSummary } from './use_get_jobs_summary';

export interface UseInstalledSecurityJobsReturn {
loading: boolean;
jobs: MlSummaryJob[];
isMlUser: boolean;
isLicensed: boolean;
}

/**
* Returns a collection of installed ML jobs (MlSummaryJob) relevant to
* Security Solution, i.e. all installed jobs in the `security` ML group.
* Use the corresponding helper functions to filter the job list as
* necessary (running jobs, etc).
*
*/
export const useInstalledSecurityJobs = (): UseInstalledSecurityJobsReturn => {
const [jobs, setJobs] = useState<MlSummaryJob[]>([]);
const { addError } = useAppToasts();
const mlCapabilities = useMlCapabilities();
const http = useHttp();
const { error, loading, result, start } = useGetJobsSummary();

const isMlUser = hasMlUserPermissions(mlCapabilities);
const isLicensed = hasMlLicense(mlCapabilities);

useEffect(() => {
if (isMlUser && isLicensed) {
start({ http });
}
}, [http, isMlUser, isLicensed, start]);

useEffect(() => {
if (result) {
const securityJobs = result.filter(isSecurityJob);
setJobs(securityJobs);
}
}, [result]);

useEffect(() => {
if (error) {
addError(error, { title: i18n.SIEM_JOB_FETCH_FAILURE });
}
}, [addError, error]);

return { isLicensed, isMlUser, jobs, loading };
};
Loading