Skip to content

Commit

Permalink
[Security Solution] Migrates siem.notifications ruleAlertId to saved …
Browse files Browse the repository at this point in the history 
…object references array (#113205)

## Summary

Fixes #113276

* Migrates the legacy `siem.notifications` "ruleAlertId" to be within the references array
* Adds code to serialize and de-serialize "ruleAlertId" from the saved object references array
* Adds migration code to `kibana-alerting` to migrate on startup
* Adds `legacy_saved_object_references/README.md` which describes how to test and what those files are for.
* Updates earlier similar `signals/saved_object_references/README.md` after reviewing it during my work
* Names these files the format of `legacy_foo` since this is all considered legacy work and will be removed once the legacy notification system is removed after customers have migrated. 
* Adds unit tests
* Adds 2e2 tests

We only migrate if we find these conditions and cases:
* "ruleAlertId" is not `null`, `undefined` or malformed data
* The"ruleAlertId" references do not already have an exceptionItem reference already found within it.

We migrate on the common use case:
* "ruleAlertId" exists and is a string

We do these additional (mis-use) cases and steps as well. These should NOT be common things that happen but we safe guard for them here:
* If the migration is run twice we are idempotent and do NOT add duplicates or remove items.
* If the migration was partially successful but re-run a second time, we only add what is missing. Again no duplicates or removed items should occur.
* If the saved object references already exists and contains a different or foreign value, we will retain the foreign reference(s) and still migrate.

Before migration you should see data structures like this if you query:

```json
# Get the alert type of "siem-notifications" which is part of the legacy system.
GET .kibana/_search
{
  "query": {
    "term": {
      "alert.alertTypeId": "siem.notifications"
    }
  }
}
```

```json
"data..omitted": "data..omitted",
"params" : {
  "ruleAlertId" : "933ca720-1be1-11ec-a722-83da1c22a481" <-- Pre-migration we had this Saved Object ID which is not part of references array below
},
"actions" : [
  {
    "group" : "default",
    "params" : {
      "message" : "Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts"
    },
    "actionTypeId" : ".slack",
    "actionRef" : "action_0" <-- Pre-migration this is correct as this work is already done within the alerting plugin
  },
  "references" : [
    {
      "id" : "879e8ff0-1be1-11ec-a722-83da1c22a481",
      "name" : "action_0", <-- Pre-migration this is correct as this work is already done within the alerting plugin
      "type" : "action"
    }
  ]
],
"data..omitted": "data..omitted",
```

After migration you should see data structures like this:
```json
"data..omitted": "data..omitted",
"params" : {
  "ruleAlertId" : "933ca720-1be1-11ec-a722-83da1c22a481" <-- Post-migration this is not used but rather the serialized version references is used instead.
},
"actions" : [
  {
    "group" : "default",
    "params" : {
      "message" : "Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts"
    },
    "actionTypeId" : ".slack",
    "actionRef" : "action_0"
  },
  "references" : [
    {
      "id" : "879e8ff0-1be1-11ec-a722-83da1c22a481",
      "name" : "action_0",
      "type" : "action"
    },
    {
      "id" : "933ca720-1be1-11ec-a722-83da1c22a481", <-- Our id here is preferred and used during serialization.
      "name" : "param:alert_0", <-- We add the name of our reference which is param:alert_0 similar to action_0 but with "param"
      "type" : "alert" <-- We add the type which is type of alert to the references
    }
  ]
],
"data..omitted": "data..omitted",
```

## Manual testing 
There are e2e and unit tests but for any manual testing or verification you can do the following:

If you have a 7.14.0 system and can migrate it forward that is the most straight forward way to ensure this does migrate correctly and forward. You should see that the legacy notification system still operates as expected.

If you are a developer off of master and want to test different scenarios then this section is for below as it is more involved and harder to do but goes into more depth:

* Create a rule and activate it normally within security_solution
* Do not add actions to the rule at this point as we are exercising the older legacy system. However, you want at least one action configured such as a slack notification.
* Within dev tools do a query for all your actions and grab one of the `_id` of them without their prefix:

```json
# See all your actions
GET .kibana/_search
{
  "query": {
    "term": {
      "type": "action"
    }
  }
}
```

Mine was `"_id" : "action:879e8ff0-1be1-11ec-a722-83da1c22a481"`, so I will be copying the ID of `879e8ff0-1be1-11ec-a722-83da1c22a481`

Go to the file `detection_engine/scripts/legacy_notifications/one_action.json` and add this id to the file. Something like this:

```json
{
  "name": "Legacy notification with one action",
  "interval": "1m",  <--- You can use whatever you want. Real values are "1h", "1d", "1w". I use "1m" for testing purposes.
  "actions": [
    {
      "id": "879e8ff0-1be1-11ec-a722-83da1c22a481", <--- My action id
      "group": "default",
      "params": {
        "message": "Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts"
      },
      "actionTypeId": ".slack" <--- I am a slack action id type.
    }
  ]
}
```

Query for an alert you want to add manually add back a legacy notification to it. Such as:

```json
# See all your siem.signals alert types and choose one
GET .kibana/_search
{
  "query": {
    "term": {
      "alert.alertTypeId": "siem.signals"
    }
  }
}
```

Grab the `_id` without the alert prefix. For mine this was `933ca720-1be1-11ec-a722-83da1c22a481`

Within the directory of detection_engine/scripts execute the script:

```json
./post_legacy_notification.sh 933ca720-1be1-11ec-a722-83da1c22a481
{
  "ok": "acknowledged"
}
```

which is going to do a few things. See the file `detection_engine/routes/rules/legacy_create_legacy_notification.ts` for the definition of the route and what it does in full, but we should notice that we have now:

Created a legacy side car action object of type `siem-detection-engine-rule-actions` you can see in dev tools:

```json
# See the actions "side car" which are part of the legacy notification system.
GET .kibana/_search
{
  "query": {
    "term": {
      "type": {
        "value": "siem-detection-engine-rule-actions"
      }
    }
  }
}
```

But more importantly what the saved object references are which should be this:

```json
# Get the alert type of "siem-notifications" which is part of the legacy system.
GET .kibana/_search
{
  "query": {
    "term": {
      "alert.alertTypeId": "siem.notifications"
    }
  }
}
```

If you need to ad-hoc test what happens when the migration runs you can get the id of an alert and downgrade it, then
restart Kibana. The `ctx._source.references.remove(1)` removes the last element of the references array which is assumed
to have a rule. But it might not, so ensure you check your data structure and adjust accordingly.
```json
POST .kibana/_update/alert:933ca720-1be1-11ec-a722-83da1c22a481
{
  "script" : {
    "source": """
    ctx._source.migrationVersion.alert = "7.15.0";
    ctx._source.references.remove(1);
    """,
    "lang": "painless"
  }
}
```

If you just want to remove your your "param:alert_0" and it is the second array element to test the errors within the console
then you would use
```json
POST .kibana/_update/alert:933ca720-1be1-11ec-a722-83da1c22a481
{
  "script" : {
    "source": """
    ctx._source.references.remove(1);
    """,
    "lang": "painless"
  }
}
```

Check your log files and should see errors about the saved object references missing until you restart Kibana. Once you restart then it will migrate forward and you will no longer see errors.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
@FrankHassanabad
FrankHassanabad authored Oct 4, 2021
1 parent 0d9825d commit ba7bea4
Show file tree
Hide file tree
Showing 17 changed files with 1,117 additions and 13 deletions.
217 changes: 217 additions & 0 deletions x-pack/plugins/alerting/server/saved_objects/migrations.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1696,6 +1696,223 @@ describe('successful migrations', () => {
},
});
});

test('security solution is migrated to saved object references if it has a "ruleAlertId"', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = getMockData({
alertTypeId: 'siem.notifications',
params: {
ruleAlertId: '123',
},
});

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
});
});

test('security solution does not migrate anything if its type is not siem.notifications', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = getMockData({
alertTypeId: 'other-type',
params: {
ruleAlertId: '123',
},
});

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
});
});
test('security solution does not change anything if "ruleAlertId" is missing', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = getMockData({
alertTypeId: 'siem.notifications',
params: {},
});

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
});
});

test('security solution will keep existing references if we do not have a "ruleAlertId" but we do already have references', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = {
...getMockData({
alertTypeId: 'siem.notifications',
params: {},
}),
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
};

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
});
});

test('security solution will keep any foreign references if they exist but still migrate other "ruleAlertId" references', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = {
...getMockData({
alertTypeId: 'siem.notifications',
params: {
ruleAlertId: '123',
},
}),
references: [
{
name: 'foreign-name',
id: '999',
type: 'foreign-name',
},
],
};

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
references: [
{
name: 'foreign-name',
id: '999',
type: 'foreign-name',
},
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
});
});

test('security solution is idempotent and if re-run on the same migrated data will keep the same items "ruleAlertId" references', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = {
...getMockData({
alertTypeId: 'siem.notifications',
params: {
ruleAlertId: '123',
},
}),
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
};

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
});
});

test('security solution will not migrate "ruleAlertId" if it is invalid data', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = {
...getMockData({
alertTypeId: 'siem.notifications',
params: {
ruleAlertId: 55, // This is invalid if it is a number and not a string
},
}),
};

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
});
});

test('security solution will not migrate "ruleAlertId" if it is invalid data but will keep existing references', () => {
const migration7160 = getMigrations(encryptedSavedObjectsSetup, isPreconfigured)['7.16.0'];
const alert = {
...getMockData({
alertTypeId: 'siem.notifications',
params: {
ruleAlertId: 456, // This is invalid if it is a number and not a string
},
}),
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
};

expect(migration7160(alert, migrationContext)).toEqual({
...alert,
attributes: {
...alert.attributes,
legacyId: alert.id,
},
references: [
{
name: 'param:alert_0',
id: '123',
type: 'alert',
},
],
});
});
});

describe('8.0.0', () => {
Expand Down
60 changes: 59 additions & 1 deletion x-pack/plugins/alerting/server/saved_objects/migrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ export const isAnyActionSupportIncidents = (doc: SavedObjectUnsanitizedDoc<RawAl
export const isSecuritySolutionRule = (doc: SavedObjectUnsanitizedDoc<RawAlert>): boolean =>
doc.attributes.alertTypeId === 'siem.signals';

/**
* Returns true if the alert type is that of "siem.notifications" which is a legacy notification system that was deprecated in 7.16.0
* in favor of using the newer alerting notifications system.
* @param doc The saved object alert type document
* @returns true if this is a legacy "siem.notifications" rule, otherwise false
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
*/
export const isSecuritySolutionLegacyNotification = (
doc: SavedObjectUnsanitizedDoc<RawAlert>
): boolean => doc.attributes.alertTypeId === 'siem.notifications';

export function getMigrations(
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup,
isPreconfigured: (connectorId: string) => boolean
Expand Down Expand Up @@ -103,7 +114,11 @@ export function getMigrations(
const migrateRules716 = createEsoMigration(
encryptedSavedObjects,
(doc): doc is SavedObjectUnsanitizedDoc<RawAlert> => true,
pipeMigrations(setLegacyId, getRemovePreconfiguredConnectorsFromReferencesFn(isPreconfigured))
pipeMigrations(
setLegacyId,
getRemovePreconfiguredConnectorsFromReferencesFn(isPreconfigured),
addRuleIdsToLegacyNotificationReferences
)
);

const migrationRules800 = createEsoMigration(
Expand Down Expand Up @@ -574,6 +589,49 @@ function removeMalformedExceptionsList(
}
}

/**
* This migrates rule_id's within the legacy siem.notification to saved object references on an upgrade.
* We only migrate if we find these conditions:
* - ruleAlertId is a string and not null, undefined, or malformed data.
* - The existing references do not already have a ruleAlertId found within it.
* Some of these issues could crop up during either user manual errors of modifying things, earlier migration
* issues, etc... so we are safer to check them as possibilities
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
* @param doc The document that might have "ruleAlertId" to migrate into the references
* @returns The document migrated with saved object references
*/
function addRuleIdsToLegacyNotificationReferences(
doc: SavedObjectUnsanitizedDoc<RawAlert>
): SavedObjectUnsanitizedDoc<RawAlert> {
const {
attributes: {
params: { ruleAlertId },
},
references,
} = doc;
if (!isSecuritySolutionLegacyNotification(doc) || !isString(ruleAlertId)) {
// early return if we are not a string or if we are not a security solution notification saved object.
return doc;
} else {
const existingReferences = references ?? [];
const existingReferenceFound = existingReferences.find((reference) => {
return reference.id === ruleAlertId && reference.type === 'alert';
});
if (existingReferenceFound) {
// skip this if the references already exists for some uncommon reason so we do not add an additional one.
return doc;
} else {
const savedObjectReference: SavedObjectReference = {
id: ruleAlertId,
name: 'param:alert_0',
type: 'alert',
};
const newReferences = [...existingReferences, savedObjectReference];
return { ...doc, references: newReferences };
}
}
}

function setLegacyId(
doc: SavedObjectUnsanitizedDoc<RawAlert>
): SavedObjectUnsanitizedDoc<RawAlert> {
Expand Down
19 changes: 14 additions & 5 deletions ...olution/server/lib/detection_engine/notifications/legacy_rules_notification_alert_type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { Logger } from 'src/core/server';
import { schema } from '@kbn/config-schema';
import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';
import {
DEFAULT_RULE_NOTIFICATION_QUERY_SIZE,
Expand All @@ -15,12 +14,19 @@ import {
} from '../../../../common/constants';

// eslint-disable-next-line no-restricted-imports
import { LegacyNotificationAlertTypeDefinition } from './legacy_types';
import {
LegacyNotificationAlertTypeDefinition,
legacyRulesNotificationParams,
} from './legacy_types';
import { AlertAttributes } from '../signals/types';
import { siemRuleActionGroups } from '../signals/siem_rule_action_groups';
import { scheduleNotificationActions } from './schedule_notification_actions';
import { getNotificationResultsLink } from './utils';
import { getSignals } from './get_signals';
// eslint-disable-next-line no-restricted-imports
import { legacyExtractReferences } from './legacy_saved_object_references/legacy_extract_references';
// eslint-disable-next-line no-restricted-imports
import { legacyInjectReferences } from './legacy_saved_object_references/legacy_inject_references';

/**
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
Expand All @@ -36,9 +42,12 @@ export const legacyRulesNotificationAlertType = ({
defaultActionGroupId: 'default',
producer: SERVER_APP_ID,
validate: {
params: schema.object({
ruleAlertId: schema.string(),
}),
params: legacyRulesNotificationParams,
},
useSavedObjectReferences: {
extractReferences: (params) => legacyExtractReferences({ logger, params }),
injectReferences: (params, savedObjectReferences) =>
legacyInjectReferences({ logger, params, savedObjectReferences }),
},
minimumLicenseRequired: 'basic',
isExportable: false,
Expand Down
Loading

0 comments on commit ba7bea4

Please sign in to comment.