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] Updates Rule Details to show all historical alerts for a given Rule #120053

Merged
merged 10 commits into from
Dec 8, 2021
Merged
3 changes: 3 additions & 0 deletions packages/kbn-rule-data-utils/src/technical_field_names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const VERSION = `${KIBANA_NAMESPACE}.version` as const;

// Fields pertaining to the alert
const ALERT_ACTION_GROUP = `${ALERT_NAMESPACE}.action_group` as const;
const ALERT_BUILDING_BLOCK_TYPE = `${ALERT_NAMESPACE}.building_block_type` as const;
const ALERT_DURATION = `${ALERT_NAMESPACE}.duration.us` as const;
const ALERT_END = `${ALERT_NAMESPACE}.end` as const;
const ALERT_EVALUATION_THRESHOLD = `${ALERT_NAMESPACE}.evaluation.threshold` as const;
Expand Down Expand Up @@ -91,6 +92,7 @@ const fields = {
TAGS,
TIMESTAMP,
ALERT_ACTION_GROUP,
ALERT_BUILDING_BLOCK_TYPE,
ALERT_DURATION,
ALERT_END,
ALERT_EVALUATION_THRESHOLD,
Expand Down Expand Up @@ -141,6 +143,7 @@ const fields = {

export {
ALERT_ACTION_GROUP,
ALERT_BUILDING_BLOCK_TYPE,
ALERT_DURATION,
ALERT_END,
ALERT_EVALUATION_THRESHOLD,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,16 @@ export const AddExceptionModal = memo(function AddExceptionModal({
const alertIdToClose = shouldCloseAlert && alertData ? alertData._id : undefined;
const bulkCloseIndex =
shouldBulkCloseAlert && signalIndexName != null ? [signalIndexName] : undefined;
addOrUpdateExceptionItems(ruleId, enrichExceptionItems(), alertIdToClose, bulkCloseIndex);
addOrUpdateExceptionItems(
maybeRule?.rule_id ?? '',
enrichExceptionItems(),
alertIdToClose,
bulkCloseIndex
);
}
}, [
addOrUpdateExceptionItems,
ruleId,
maybeRule,
enrichExceptionItems,
shouldCloseAlert,
shouldBulkCloseAlert,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,16 @@ export const EditExceptionModal = memo(function EditExceptionModal({
if (addOrUpdateExceptionItems !== null) {
const bulkCloseIndex =
shouldBulkCloseAlert && signalIndexName !== null ? [signalIndexName] : undefined;
addOrUpdateExceptionItems(ruleId, enrichExceptionItems(), undefined, bulkCloseIndex);
addOrUpdateExceptionItems(
maybeRule?.rule_id ?? '',
enrichExceptionItems(),
undefined,
bulkCloseIndex
);
}
}, [
addOrUpdateExceptionItems,
ruleId,
maybeRule,
enrichExceptionItems,
shouldBulkCloseAlert,
signalIndexName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,14 @@ describe('useAddOrUpdateException', () => {
let updateExceptionListItem: jest.SpyInstance<Promise<ExceptionListItemSchema>>;
let getQueryFilter: jest.SpyInstance<ReturnType<typeof getQueryFilterHelper.getQueryFilter>>;
let buildAlertStatusesFilter: jest.SpyInstance<
ReturnType<typeof buildFilterHelpers.buildAlertStatusesFilterRuleRegistry>
>;
let buildAlertsRuleIdFilter: jest.SpyInstance<
ReturnType<typeof buildFilterHelpers.buildAlertsRuleIdFilter>
ReturnType<typeof buildFilterHelpers.buildAlertStatusesFilter>
>;
let buildAlertsFilter: jest.SpyInstance<ReturnType<typeof buildFilterHelpers.buildAlertsFilter>>;
let addOrUpdateItemsArgs: Parameters<AddOrUpdateExceptionItemsFunc>;
let render: () => RenderHookResult<UseAddOrUpdateExceptionProps, ReturnUseAddOrUpdateException>;
const onError = jest.fn();
const onSuccess = jest.fn();
const ruleId = 'rule-id';
const ruleStaticId = 'rule-id';
const alertIdToClose = 'idToClose';
const bulkCloseIndex = ['.custom'];
const itemsToAdd: CreateExceptionListItemSchema[] = [
Expand Down Expand Up @@ -128,14 +126,11 @@ describe('useAddOrUpdateException', () => {

getQueryFilter = jest.spyOn(getQueryFilterHelper, 'getQueryFilter');

buildAlertStatusesFilter = jest.spyOn(
buildFilterHelpers,
'buildAlertStatusesFilterRuleRegistry'
);
buildAlertStatusesFilter = jest.spyOn(buildFilterHelpers, 'buildAlertStatusesFilter');

buildAlertsRuleIdFilter = jest.spyOn(buildFilterHelpers, 'buildAlertsRuleIdFilter');
buildAlertsFilter = jest.spyOn(buildFilterHelpers, 'buildAlertsFilter');

addOrUpdateItemsArgs = [ruleId, itemsToAddOrUpdate];
addOrUpdateItemsArgs = [ruleStaticId, itemsToAddOrUpdate];
render = () =>
renderHook<UseAddOrUpdateExceptionProps, ReturnUseAddOrUpdateException>(
() =>
Expand Down Expand Up @@ -262,7 +257,7 @@ describe('useAddOrUpdateException', () => {

describe('when alertIdToClose is passed in', () => {
beforeEach(() => {
addOrUpdateItemsArgs = [ruleId, itemsToAddOrUpdate, alertIdToClose];
addOrUpdateItemsArgs = [ruleStaticId, itemsToAddOrUpdate, alertIdToClose];
});
it('should update the alert status', async () => {
await act(async () => {
Expand Down Expand Up @@ -317,7 +312,7 @@ describe('useAddOrUpdateException', () => {

describe('when bulkCloseIndex is passed in', () => {
beforeEach(() => {
addOrUpdateItemsArgs = [ruleId, itemsToAddOrUpdate, undefined, bulkCloseIndex];
addOrUpdateItemsArgs = [ruleStaticId, itemsToAddOrUpdate, undefined, bulkCloseIndex];
});
it('should update the status of only alerts that are open', async () => {
await act(async () => {
Expand Down Expand Up @@ -351,8 +346,8 @@ describe('useAddOrUpdateException', () => {
addOrUpdateItems(...addOrUpdateItemsArgs);
}
await waitForNextUpdate();
expect(buildAlertsRuleIdFilter).toHaveBeenCalledTimes(1);
expect(buildAlertsRuleIdFilter.mock.calls[0][0]).toEqual(ruleId);
expect(buildAlertsFilter).toHaveBeenCalledTimes(1);
expect(buildAlertsFilter.mock.calls[0][0]).toEqual(ruleStaticId);
});
});
it('should generate the query filter using exceptions', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,25 @@ import { HttpStart } from '../../../../../../../src/core/public';
import { updateAlertStatus } from '../../../detections/containers/detection_engine/alerts/api';
import { getUpdateAlertsQuery } from '../../../detections/components/alerts_table/actions';
import {
buildAlertsRuleIdFilter,
buildAlertsFilter,
buildAlertStatusesFilter,
buildAlertStatusesFilterRuleRegistry,
} from '../../../detections/components/alerts_table/default_config';
import { getQueryFilter } from '../../../../common/detection_engine/get_query_filter';
import { Index } from '../../../../common/detection_engine/schemas/common/schemas';
import { useIsExperimentalFeatureEnabled } from '../../hooks/use_experimental_features';
import { formatExceptionItemForUpdate, prepareExceptionItemsForBulkClose } from './helpers';
import { useKibana } from '../../lib/kibana';

/**
* Adds exception items to the list. Also optionally closes alerts.
*
* @param ruleId id of the rule where the exception updates will be applied
* @param ruleStaticId static id of the rule (rule.ruleId, not rule.id) where the exception updates will be applied
* @param exceptionItemsToAddOrUpdate array of ExceptionListItemSchema to add or update
* @param alertIdToClose - optional string representing alert to close
* @param bulkCloseIndex - optional index used to create bulk close query
*
*/
export type AddOrUpdateExceptionItemsFunc = (
ruleId: string,
ruleStaticId: string,
exceptionItemsToAddOrUpdate: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>,
alertIdToClose?: string,
bulkCloseIndex?: Index
Expand Down Expand Up @@ -72,10 +70,10 @@ export const useAddOrUpdateException = ({
const addOrUpdateExceptionRef = useRef<AddOrUpdateExceptionItemsFunc | null>(null);
const { addExceptionListItem, updateExceptionListItem } = useApi(services.http);
const addOrUpdateException = useCallback<AddOrUpdateExceptionItemsFunc>(
async (ruleId, exceptionItemsToAddOrUpdate, alertIdToClose, bulkCloseIndex) => {
async (ruleStaticId, exceptionItemsToAddOrUpdate, alertIdToClose, bulkCloseIndex) => {
if (addOrUpdateExceptionRef.current != null) {
addOrUpdateExceptionRef.current(
ruleId,
ruleStaticId,
exceptionItemsToAddOrUpdate,
alertIdToClose,
bulkCloseIndex
Expand All @@ -84,15 +82,13 @@ export const useAddOrUpdateException = ({
},
[]
);
// TODO: Once we are past experimental phase this code should be removed
const ruleRegistryEnabled = useIsExperimentalFeatureEnabled('ruleRegistryEnabled');

useEffect(() => {
let isSubscribed = true;
const abortCtrl = new AbortController();

const onUpdateExceptionItemsAndAlertStatus: AddOrUpdateExceptionItemsFunc = async (
ruleId,
ruleStaticId,
exceptionItemsToAddOrUpdate,
alertIdToClose,
bulkCloseIndex
Expand Down Expand Up @@ -131,15 +127,16 @@ export const useAddOrUpdateException = ({
}

if (bulkCloseIndex != null) {
// TODO: Once we are past experimental phase this code should be removed
const alertStatusFilter = ruleRegistryEnabled
? buildAlertStatusesFilterRuleRegistry(['open', 'acknowledged', 'in-progress'])
: buildAlertStatusesFilter(['open', 'acknowledged', 'in-progress']);
const alertStatusFilter = buildAlertStatusesFilter([
'open',
'acknowledged',
'in-progress',
]);
Comment on lines +130 to +134
Copy link
Contributor

Choose a reason for hiding this comment

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

Garrett, do you know if a go/no-go decision has been made regarding going live with rule_registry in 8.0? There's still some ongoing discussions regarding AAD schema which is extremely concerning considering where we are in the release cycle. Wondering what are chances of us being asked to revert this stuff back to .siem-signals-* (which means we'd need the ruleRegistryEnabled feature flag in the code):

          // TODO: Once we are past experimental phase this code should be removed
          const alertStatusFilter = ruleRegistryEnabled
            ? buildAlertStatusesFilterRuleRegistry(['open', 'acknowledged', 'in-progress'])
            : buildAlertStatusesFilter(['open', 'acknowledged', 'in-progress']);

Copy link
Member Author

Choose a reason for hiding this comment

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

Verified RuleRegistry is a go for security in 8.0, so all good here 🙂


const filter = getQueryFilter(
'',
'kuery',
[...buildAlertsRuleIdFilter(ruleId), ...alertStatusFilter],
[...buildAlertsFilter(ruleStaticId), ...alertStatusFilter],
bulkCloseIndex,
prepareExceptionItemsForBulkClose(exceptionItemsToAddOrUpdate),
false
Expand Down Expand Up @@ -185,14 +182,7 @@ export const useAddOrUpdateException = ({
isSubscribed = false;
abortCtrl.abort();
};
}, [
addExceptionListItem,
http,
onSuccess,
onError,
ruleRegistryEnabled,
updateExceptionListItem,
]);
}, [addExceptionListItem, http, onSuccess, onError, updateExceptionListItem]);

return [{ isLoading }, addOrUpdateException];
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { ExistsFilter, Filter } from '@kbn/es-query';
import {
buildAlertsRuleIdFilter,
buildAlertsFilter,
buildAlertStatusesFilter,
buildAlertStatusFilter,
buildThreatMatchFilter,
Expand All @@ -18,21 +18,21 @@ jest.mock('./actions');
describe('alerts default_config', () => {
describe('buildAlertsRuleIdFilter', () => {
test('given a rule id this will return an array with a single filter', () => {
const filters: Filter[] = buildAlertsRuleIdFilter('rule-id-1');
const filters: Filter[] = buildAlertsFilter('rule-id-1');
const expectedFilter: Filter = {
meta: {
alias: null,
negate: false,
disabled: false,
type: 'phrase',
key: 'kibana.alert.rule.uuid',
key: 'kibana.alert.rule.rule_id',
params: {
query: 'rule-id-1',
},
},
query: {
match_phrase: {
'kibana.alert.rule.uuid': 'rule-id-1',
'kibana.alert.rule.rule_id': 'rule-id-1',
},
},
};
Expand Down
Loading