From 6f29df4de1d5f1b86b837b5f67211043288f48be Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 8 Dec 2021 12:59:43 -0500 Subject: [PATCH 01/19] WIP --- .../plugins/actions/server/actions_client.ts | 1 + .../routes/rules/import_rules_route.ts | 6 + .../detection_engine/routes/rules/utils.ts | 9 +- .../security_and_spaces/tests/import_rules.ts | 95 +++++ .../security_and_spaces/tests/index.ts | 92 ++-- .../tests/resolve_read_rules.ts | 25 +- .../import_rule_connector/data.json | 48 +++ .../import_rule_connector/mappings.json | 400 ++++++++++++++++++ .../resolve_read_rules/7_14/data.json | 26 ++ .../resolve_read_rules/7_14/mappings.json | 3 + 10 files changed, 645 insertions(+), 60 deletions(-) create mode 100644 x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json create mode 100644 x-pack/test/functional/es_archives/security_solution/import_rule_connector/mappings.json diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index deaa1a79d1640..3d0c1045e2d8b 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -547,6 +547,7 @@ function actionFromSavedObject(savedObject: SavedObject): ActionResul return { id: savedObject.id, ...savedObject.attributes, + originId: savedObject.originId, isPreconfigured: false, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index b056fd3ed80f0..865115de3b08c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -37,6 +37,7 @@ import { isBulkError, isImportRegular, buildSiemResponse, + migrateLegacyActionsIds, } from '../utils'; import { patchRules } from '../../rules/patch_rules'; @@ -120,6 +121,11 @@ export const importRulesRoute = ( const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = getTupleDuplicateErrorsAndUniqueRules(parsedObjects, request.query.overwrite); + const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( + parsedObjectsWithoutDuplicateErrors, + actionsClient + ); + const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( parsedObjectsWithoutDuplicateErrors, actionsClient diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index e706a3c914974..931fb7c8ba864 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -193,6 +193,13 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; +export const migrateLegacyActionsIds = async ( + rules: PromiseFromStreams[], + actionsClient: ActionsClient +) => { + const actionsFind = await actionsClient.getAll(); +}; + /** * Given a set of rules and an actions client this will return connectors that are invalid * such as missing connectors and filter out the rules that have invalid connectors. @@ -205,7 +212,7 @@ export const getInvalidConnectors = async ( actionsClient: ActionsClient ): Promise<[BulkError[], PromiseFromStreams[]]> => { const actionsFind = await actionsClient.getAll(); - const actionIds = new Set(actionsFind.map((action) => action.id)); + const actionIds = new Set(actionsFind.flatMap((action) => [action.id, action?.originId])); const { errors, rulesAcc } = rules.reduce( (acc, parsedRule) => { if (parsedRule instanceof Error) { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 9a9d4f7758d07..9dd294a9e1623 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -25,15 +25,22 @@ import { export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); const log = getService('log'); + const esArchiver = getService('esArchiver'); describe('import_rules', () => { describe('importing rules with an index', () => { beforeEach(async () => { await createSignalsIndex(supertest, log); + await esArchiver.load( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); }); afterEach(async () => { await deleteSignalsIndex(supertest, log); + await esArchiver.unload( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); await deleteAllAlerts(supertest, log); }); @@ -476,6 +483,94 @@ export default ({ getService }: FtrProviderContext): void => { ], }); }); + + it.only('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + const spaceId = '714-space'; + const connectorId = 'cf92c3c5-203e-54be-bb57-9348afc6651e'; + + // const rule1: ReturnType = { + // ...getSimpleRule('rule-1'), + // actions: [ + // { + // group: 'default', + // id: '51b17790-544e-11ec-a349-11361cc441c4', + // action_type_id: '.slack', + // params: {}, + // }, + // ], + // }; + + const connectorRes = await supertest + .get(`/s/${spaceId}/api/actions/connectors`) //${connectorId}`) + .set('kbn-xsrf', 'true') + .send(); + console.error('DOES THE CONNECTOR ID EXIST?', JSON.stringify(connectorRes, null, 2)); + + const rule1 = { + id: '53aad690-544e-11ec-a349-11361cc441c4', + updated_at: '2021-12-03T15:33:13.271Z', + updated_by: 'elastic', + created_at: '2021-12-03T15:33:13.271Z', + created_by: 'elastic', + name: '7.16 test with action', + tags: [], + interval: '5m', + enabled: true, + description: 'test', + risk_score: 21, + severity: 'low', + license: '', + output_index: '.siem-signals-devin-hurley-7', + meta: { from: '1m', kibana_siem_app_url: 'http://0.0.0.0:5601/s/7/app/security' }, + author: [], + false_positives: [], + from: 'now-360s', + rule_id: 'aa525d7c-8948-439f-b32d-27e00c750246', + max_signals: 100, + risk_score_mapping: [], + severity_mapping: [], + threat: [], + to: 'now', + references: [], + version: 1, + exceptions_list: [], + immutable: false, + type: 'query', + language: 'kuery', + index: [ + 'apm-*-transaction*', + 'traces-apm*', + 'auditbeat-*', + 'endgame-*', + 'filebeat-*', + 'logs-*', + 'packetbeat-*', + 'winlogbeat-*', + ], + query: '*:*', + filters: [], + throttle: '1h', + actions: [ + { + group: 'default', + id: '51b17790-544e-11ec-a349-11361cc441c4', // 'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + }, + ], + }; + + const rule1String = JSON.stringify(rule1); + const buffer = Buffer.from(`${rule1String}\n`); + + const { body } = await supertest + .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson'); + console.error('BODY', JSON.stringify(body, null, 2)); + }); }); }); }; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index db616baa0678a..9505dbf0e6adb 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -13,60 +13,60 @@ export default ({ loadTestFile }: FtrProviderContext): void => { describe('', function () { this.tags('ciGroup11'); - loadTestFile(require.resolve('./aliases')); - loadTestFile(require.resolve('./create_endpoint_exceptions')); - loadTestFile(require.resolve('./add_actions')); - loadTestFile(require.resolve('./update_actions')); - loadTestFile(require.resolve('./add_prepackaged_rules')); - loadTestFile(require.resolve('./check_privileges')); - loadTestFile(require.resolve('./create_index')); - loadTestFile(require.resolve('./create_rules')); - loadTestFile(require.resolve('./create_rules_bulk')); - loadTestFile(require.resolve('./create_ml')); - loadTestFile(require.resolve('./create_threat_matching')); - loadTestFile(require.resolve('./create_exceptions')); - loadTestFile(require.resolve('./delete_rules')); - loadTestFile(require.resolve('./delete_rules_bulk')); - loadTestFile(require.resolve('./export_rules')); - loadTestFile(require.resolve('./find_rules')); - loadTestFile(require.resolve('./find_statuses')); - loadTestFile(require.resolve('./generating_signals')); - loadTestFile(require.resolve('./get_prepackaged_rules_status')); + // loadTestFile(require.resolve('./aliases')); + // loadTestFile(require.resolve('./create_endpoint_exceptions')); + // loadTestFile(require.resolve('./add_actions')); + // loadTestFile(require.resolve('./update_actions')); + // loadTestFile(require.resolve('./add_prepackaged_rules')); + // loadTestFile(require.resolve('./check_privileges')); + // loadTestFile(require.resolve('./create_index')); + // loadTestFile(require.resolve('./create_rules')); + // loadTestFile(require.resolve('./create_rules_bulk')); + // loadTestFile(require.resolve('./create_ml')); + // loadTestFile(require.resolve('./create_threat_matching')); + // loadTestFile(require.resolve('./create_exceptions')); + // loadTestFile(require.resolve('./delete_rules')); + // loadTestFile(require.resolve('./delete_rules_bulk')); + // loadTestFile(require.resolve('./export_rules')); + // loadTestFile(require.resolve('./find_rules')); + // loadTestFile(require.resolve('./find_statuses')); + // loadTestFile(require.resolve('./generating_signals')); + // loadTestFile(require.resolve('./get_prepackaged_rules_status')); loadTestFile(require.resolve('./import_rules')); - loadTestFile(require.resolve('./read_rules')); - loadTestFile(require.resolve('./resolve_read_rules')); - loadTestFile(require.resolve('./update_rules')); - loadTestFile(require.resolve('./update_rules_bulk')); - loadTestFile(require.resolve('./patch_rules_bulk')); - loadTestFile(require.resolve('./perform_bulk_action')); - loadTestFile(require.resolve('./patch_rules')); - loadTestFile(require.resolve('./read_privileges')); - loadTestFile(require.resolve('./open_close_signals')); - loadTestFile(require.resolve('./get_signals_migration_status')); - loadTestFile(require.resolve('./create_signals_migrations')); - loadTestFile(require.resolve('./finalize_signals_migrations')); - loadTestFile(require.resolve('./delete_signals_migrations')); - loadTestFile(require.resolve('./timestamps')); - loadTestFile(require.resolve('./runtime')); - loadTestFile(require.resolve('./throttle')); - loadTestFile(require.resolve('./ignore_fields')); - loadTestFile(require.resolve('./migrations')); + // loadTestFile(require.resolve('./read_rules')); + // loadTestFile(require.resolve('./resolve_read_rules')); + // loadTestFile(require.resolve('./update_rules')); + // loadTestFile(require.resolve('./update_rules_bulk')); + // loadTestFile(require.resolve('./patch_rules_bulk')); + // loadTestFile(require.resolve('./perform_bulk_action')); + // loadTestFile(require.resolve('./patch_rules')); + // loadTestFile(require.resolve('./read_privileges')); + // loadTestFile(require.resolve('./open_close_signals')); + // loadTestFile(require.resolve('./get_signals_migration_status')); + // loadTestFile(require.resolve('./create_signals_migrations')); + // loadTestFile(require.resolve('./finalize_signals_migrations')); + // loadTestFile(require.resolve('./delete_signals_migrations')); + // loadTestFile(require.resolve('./timestamps')); + // loadTestFile(require.resolve('./runtime')); + // loadTestFile(require.resolve('./throttle')); + // loadTestFile(require.resolve('./ignore_fields')); + // loadTestFile(require.resolve('./migrations')); }); // That split here enable us on using a different ciGroup to run the tests // listed on ./exception_operators_data_types/index - describe('', function () { - loadTestFile(require.resolve('./exception_operators_data_types/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./exception_operators_data_types/index')); + // }); // That split here enable us on using a different ciGroup to run the tests // listed on ./keyword_family/index - describe('', function () { - loadTestFile(require.resolve('./keyword_family/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./keyword_family/index')); + // }); - describe('', function () { - loadTestFile(require.resolve('./alerts/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./alerts/index')); + // }); }); }; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts index a7333ef8716f2..09ed0b78e4e27 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts @@ -37,20 +37,19 @@ export default ({ getService }: FtrProviderContext) => { ); }); - it('should create a "migrated" rule where querying for the new SO _id will resolve the new object and not return the outcome field when outcome === exactMatch', async () => { + it.only('should create a "migrated" rule where querying for the new SO _id will resolve the new object and not return the outcome field when outcome === exactMatch', async () => { // link to the new URL with migrated SO id 74f3e6d7-b7bb-477d-ac28-92ee22728e6e - const URL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=90e3ca0e-71f7-513a-b60a-ac678efd8887`; - const readRulesAliasMatchRes = await supertest.get(URL).set('kbn-xsrf', 'true').send(); - expect(readRulesAliasMatchRes.body.outcome).to.eql('aliasMatch'); - - // now that we have the migrated alias_target_id, let's attempt an 'exactMatch' query - // the result of which should have the outcome as undefined when querying the read rules api. - const exactMatchURL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=${readRulesAliasMatchRes.body.alias_target_id}`; - const readRulesExactMatchRes = await supertest - .get(exactMatchURL) - .set('kbn-xsrf', 'true') - .send(); - expect(readRulesExactMatchRes.body.outcome).to.eql(undefined); + // const URL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=90e3ca0e-71f7-513a-b60a-ac678efd8887`; + // const readRulesAliasMatchRes = await supertest.get(URL).set('kbn-xsrf', 'true').send(); + // expect(readRulesAliasMatchRes.body.outcome).to.eql('aliasMatch'); + // // now that we have the migrated alias_target_id, let's attempt an 'exactMatch' query + // // the result of which should have the outcome as undefined when querying the read rules api. + // const exactMatchURL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=${readRulesAliasMatchRes.body.alias_target_id}`; + // const readRulesExactMatchRes = await supertest + // .get(exactMatchURL) + // .set('kbn-xsrf', 'true') + // .send(); + // expect(readRulesExactMatchRes.body.outcome).to.eql(undefined); }); it('should create a rule and a "conflicting rule" where the SO _id matches the sourceId (see legacy-url-alias SO) of a migrated rule', async () => { diff --git a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json new file mode 100644 index 0000000000000..945408b6d0067 --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json @@ -0,0 +1,48 @@ +{ + "type" : "doc", + "value": { + "index" : ".kibana_1", + "id" : "space:714-space", + "source" : { + "space" : { + "name" : "714-space", + "initials" : "t", + "color" : "#B9A888", + "disabledFeatures" : [ ], + "imageUrl" : "" + }, + "type" : "space", + "references" : [ ], + "migrationVersion" : { + "space" : "6.6.0" + }, + "updated_at" : "2021-10-11T14:49:07.012Z" + } + } +} + +{ + "type": "doc", + "value": { + "index": ".kibana_1", + "id": "action:cf92c3c5-203e-54be-bb57-9348afc6651e", + "source": { + "action": { + "actionTypeId": ".slack", + "name": "7.16 test connector", + "isMissingSecrets": false, + "config": {}, + "secrets": "fEO5Lk2AxFWtXNpTyg2DZKaZCjeCfMh/DGch02neTGJ/Hzu+w4DXigUVuUtgynNyRbhe7TbnTzi44jVg39WB3VR3yoWSFtvV/W7NHa3B1Kr3za1S3V4XCIu/CMIk0k8vnQMiNGiMuolwws6UjvQk8fiVJygjznhEGc66TMuAmKdz7fM=" + }, + "type": "action", + "references": [], + "namespaces": ["714-space"], + "originId": "51b17790-544e-11ec-a349-11361cc441c4", + "migrationVersion": { + "action": "8.0.0" + }, + "coreMigrationVersion": "8.0.0", + "updated_at": "2021-12-03T15:33:09.651Z" + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/mappings.json b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/mappings.json new file mode 100644 index 0000000000000..9ff4456560aa7 --- /dev/null +++ b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/mappings.json @@ -0,0 +1,400 @@ +{ + "type": "index", + "value": { + "aliases": { + ".kibana": {} + }, + "index": ".kibana_1", + "mappings": { + "_meta": { + "migrationMappingPropertyHashes": { + "action": "6e96ac5e648f57523879661ea72525b7", + "action_task_params": "a9d49f184ee89641044be0ca2950fa3a", + "alert": "7b44fba6773e37c806ce290ea9b7024e", + "apm-indices": "9bb9b2bf1fa636ed8619cbab5ce6a1dd", + "apm-telemetry": "3525d7c22c42bc80f5e6e9cb3f2b26a2", + "application_usage_totals": "c897e4310c5f24b07caaff3db53ae2c1", + "application_usage_transactional": "965839e75f809fefe04f92dc4d99722a", + "canvas-element": "7390014e1091044523666d97247392fc", + "canvas-workpad": "b0a1706d356228dbdcb4a17e6b9eb231", + "cases": "32aa96a6d3855ddda53010ae2048ac22", + "cases-comments": "c2061fb929f585df57425102fa928b4b", + "cases-configure": "42711cbb311976c0687853f4c1354572", + "cases-user-actions": "32277330ec6b721abe3b846cfd939a71", + "config": "ae24d22d5986d04124cc6568f771066f", + "dashboard": "d00f614b29a80360e1190193fd333bab", + "file-upload-telemetry": "0ed4d3e1983d1217a30982630897092e", + "graph-workspace": "cd7ba1330e6682e9cc00b78850874be1", + "index-pattern": "66eccb05066c5a89924f48a9e9736499", + "infrastructure-ui-source": "ddc0ecb18383f6b26101a2fadb2dab0c", + "inventory-view": "88fc7e12fd1b45b6f0787323ce4f18d2", + "kql-telemetry": "d12a98a6f19a2d273696597547e064ee", + "lens": "d33c68a69ff1e78c9888dedd2164ac22", + "lens-ui-telemetry": "509bfa5978586998e05f9e303c07a327", + "map": "23d7aa4a720d4938ccde3983f87bd58d", + "maps-telemetry": "bfd39d88aadadb4be597ea984d433dbe", + "metrics-explorer-view": "428e319af3e822c80a84cf87123ca35c", + "migrationVersion": "4a1746014a75ade3a714e1db5763276f", + "ml-telemetry": "257fd1d4b4fdbb9cb4b8a3b27da201e9", + "namespace": "2f4316de49999235636386fe51dc06c1", + "namespaces": "2f4316de49999235636386fe51dc06c1", + "query": "11aaeb7f5f7fa5bb43f25e18ce26e7d9", + "references": "7997cf5a56cc02bdc9c93361bde732b0", + "sample-data-telemetry": "7d3cfeb915303c9641c59681967ffeb4", + "search": "181661168bbadd1eff5902361e2a0d5c", + "space": "c5ca8acafa0beaa4d08d014a97b6bc6b", + "telemetry": "36a616f7026dfa617d6655df850fe16d", + "todo": "082a2cc96a590268344d5cd74c159ac4", + "tsvb-validation-telemetry": "3a37ef6c8700ae6fc97d5c7da00e9215", + "type": "2f4316de49999235636386fe51dc06c1", + "ui-metric": "0d409297dc5ebe1e3a1da691c6ee32e3", + "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "upgrade-assistant-reindex-operation": "296a89039fc4260292be36b1b005d8f2", + "upgrade-assistant-telemetry": "56702cec857e0a9dacfb696655b4ff7b", + "uptime-dynamic-settings": "fcdb453a30092f022f2642db29523d80", + "url": "b675c3be8d76ecf029294d51dc7ec65d", + "visualization": "52d7a13ad68a150c4525b292d23e12cc" + } + }, + "dynamic": "strict", + "properties": { + "action": { + "properties": { + "actionTypeId": { + "type": "keyword" + }, + "isMissingSecrets": { + "type": "boolean" + }, + "config": { + "enabled": false, + "type": "object" + }, + "name": { + "fields": { + "keyword": { + "type": "keyword" + } + }, + "type": "text" + }, + "secrets": { + "type": "binary" + } + } + }, + "action_task_params": { + "properties": { + "actionId": { + "type": "keyword" + }, + "apiKey": { + "type": "binary" + }, + "params": { + "enabled": false, + "type": "object" + } + } + }, + "alert": { + "properties": { + "actions": { + "properties": { + "actionRef": { + "type": "keyword" + }, + "actionTypeId": { + "type": "keyword" + }, + "group": { + "type": "keyword" + }, + "params": { + "enabled": false, + "type": "object" + } + }, + "type": "nested" + }, + "alertTypeId": { + "type": "keyword" + }, + "apiKey": { + "type": "binary" + }, + "apiKeyOwner": { + "type": "keyword" + }, + "consumer": { + "type": "keyword" + }, + "createdAt": { + "type": "date" + }, + "legacyId": { + "type": "keyword" + }, + "createdBy": { + "type": "keyword" + }, + "updatedAt": { + "type": "date" + }, + "executionStatus": { + "properties": { + "error": { + "properties": { + "message": { + "type": "keyword" + }, + "reason": { + "type": "keyword" + } + } + }, + "lastExecutionDate": { + "type": "date" + }, + "status": { + "type": "keyword" + } + } + }, + "enabled": { + "type": "boolean" + }, + "muteAll": { + "type": "boolean" + }, + "mutedInstanceIds": { + "type": "keyword" + }, + "name": { + "fields": { + "keyword": { + "type": "keyword" + } + }, + "type": "text" + }, + "params": { + "enabled": false, + "type": "object" + }, + "schedule": { + "properties": { + "interval": { + "type": "keyword" + } + } + }, + "scheduledTaskId": { + "type": "keyword" + }, + "tags": { + "type": "keyword" + }, + "throttle": { + "type": "keyword" + }, + "updatedBy": { + "type": "keyword" + } + } + }, + "legacy-url-alias": { + "properties": { + "sourceId": { + "type": "text" + }, + "targetNamespace": { + "type": "keyword" + }, + "targetType": { + "type": "keyword" + }, + "targetId": { + "type": "keyword" + }, + "resolveCounter": { + "type": "integer" + }, + "lastResolved": { + "type": "date" + } + } + }, + "config": { + "dynamic": "true", + "properties": { + "buildNum": { + "type": "keyword" + } + } + }, + "migrationVersion": { + "dynamic": "true", + "properties": { + "alert": { + "fields": { + "keyword": { + "ignore_above": 256, + "type": "keyword" + } + }, + "type": "text" + }, + "config": { + "fields": { + "keyword": { + "ignore_above": 256, + "type": "keyword" + } + }, + "type": "text" + }, + "space": { + "fields": { + "keyword": { + "ignore_above": 256, + "type": "keyword" + } + }, + "type": "text" + } + } + }, + "namespace": { + "type": "keyword" + }, + "namespaces": { + "type": "keyword" + }, + "originId": { + "type": "keyword" + }, + "coreMigrationVersion": { + "type": "keyword" + }, + "query": { + "properties": { + "description": { + "type": "text" + }, + "filters": { + "enabled": false, + "type": "object" + }, + "query": { + "properties": { + "language": { + "type": "keyword" + }, + "query": { + "index": false, + "type": "keyword" + } + } + }, + "timefilter": { + "enabled": false, + "type": "object" + }, + "title": { + "type": "text" + } + } + }, + "references": { + "properties": { + "id": { + "type": "keyword" + }, + "name": { + "type": "keyword" + }, + "type": { + "type": "keyword" + } + }, + "type": "nested" + }, + "search": { + "properties": { + "columns": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "sort": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "space": { + "properties": { + "_reserved": { + "type": "boolean" + }, + "color": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "disabledFeatures": { + "type": "keyword" + }, + "imageUrl": { + "index": false, + "type": "text" + }, + "initials": { + "type": "keyword" + }, + "name": { + "fields": { + "keyword": { + "ignore_above": 2048, + "type": "keyword" + } + }, + "type": "text" + } + } + }, + "type": { + "type": "keyword" + }, + "updated_at": { + "type": "date" + } + } + }, + "settings": { + "index": { + "auto_expand_replicas": "0-1", + "number_of_replicas": "1", + "number_of_shards": "1" + } + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json index fc078b6164b2e..89247b1d094a5 100644 --- a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json +++ b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json @@ -99,3 +99,29 @@ } } } + +{ + "type": "doc", + "value": { + "index": ".kibana_1", + "id": "action:cf92c3c5-203e-54be-bb57-9348afc6651e", + "source": { + "action": { + "actionTypeId": ".slack", + "name": "7.16 test connector", + "isMissingSecrets": false, + "config": {}, + "secrets": "fEO5Lk2AxFWtXNpTyg2DZKaZCjeCfMh/DGch02neTGJ/Hzu+w4DXigUVuUtgynNyRbhe7TbnTzi44jVg39WB3VR3yoWSFtvV/W7NHa3B1Kr3za1S3V4XCIu/CMIk0k8vnQMiNGiMuolwws6UjvQk8fiVJygjznhEGc66TMuAmKdz7fM=" + }, + "type": "action", + "references": [], + "namespaces": ["714-space"], + "originId": "51b17790-544e-11ec-a349-11361cc441c4", + "migrationVersion": { + "action": "8.0.0" + }, + "coreMigrationVersion": "8.0.0", + "updated_at": "2021-12-03T15:33:09.651Z" + } + } +} diff --git a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json index 069f70badce4e..9ff4456560aa7 100644 --- a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json +++ b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json @@ -63,6 +63,9 @@ "actionTypeId": { "type": "keyword" }, + "isMissingSecrets": { + "type": "boolean" + }, "config": { "enabled": false, "type": "object" From ee3e7d1f6a18d622be5481eb15092db76601ddd1 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 8 Dec 2021 16:50:03 -0500 Subject: [PATCH 02/19] WIP --- .../src/actions/index.ts | 2 + .../plugins/actions/server/actions_client.ts | 50 +++++++++++++++++++ .../routes/rules/import_rules_route.ts | 9 ++-- .../detection_engine/routes/rules/utils.ts | 32 ++++++++++-- .../security_and_spaces/tests/import_rules.ts | 2 +- 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts index 7503bcc330e0e..f984484dd4091 100644 --- a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts +++ b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts @@ -29,6 +29,8 @@ export const action = t.exact( }) ); +export type Action = t.TypeOf; + export const actions = t.array(action); export type Actions = t.TypeOf; diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index 3d0c1045e2d8b..d81eb52823d22 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -323,6 +323,56 @@ export class ActionsClient { }; } + /** + * resolve an action + * @returns actionResult + */ + public async resolve({ id }: { id: string }): Promise { + try { + await this.authorization.ensureAuthorized('get'); + } catch (error) { + this.auditLogger?.log( + connectorAuditEvent({ + action: ConnectorAuditAction.GET, + savedObject: { type: 'action', id }, + error, + }) + ); + throw error; + } + + const preconfiguredActionsList = this.preconfiguredActions.find( + (preconfiguredAction) => preconfiguredAction.id === id + ); + if (preconfiguredActionsList !== undefined) { + this.auditLogger?.log( + connectorAuditEvent({ + action: ConnectorAuditAction.GET, + savedObject: { type: 'action', id }, + }) + ); + + return { + id, + actionTypeId: preconfiguredActionsList.actionTypeId, + name: preconfiguredActionsList.name, + isPreconfigured: true, + }; + } + + const result = await this.unsecuredSavedObjectsClient.resolve('action', id); + + this.auditLogger?.log( + connectorAuditEvent({ + action: ConnectorAuditAction.GET, + savedObject: { type: 'action', id }, + }) + ); + + return { + ...result, + }; + } /** * Get all actions with preconfigured list */ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index 865115de3b08c..48f8285c33ea2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -37,12 +37,15 @@ import { isBulkError, isImportRegular, buildSiemResponse, - migrateLegacyActionsIds, } from '../utils'; import { patchRules } from '../../rules/patch_rules'; import { legacyMigrate } from '../../rules/utils'; -import { getTupleDuplicateErrorsAndUniqueRules, getInvalidConnectors } from './utils'; +import { + getTupleDuplicateErrorsAndUniqueRules, + getInvalidConnectors, + migrateLegacyActionsIds, +} from './utils'; import { createRulesStreamFromNdJson } from '../../rules/create_rules_stream_from_ndjson'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { HapiReadableStream } from '../../rules/types'; @@ -127,7 +130,7 @@ export const importRulesRoute = ( ); const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( - parsedObjectsWithoutDuplicateErrors, + migratedParsedObjectsWithoutDuplicateErrors, actionsClient ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 931fb7c8ba864..6a625be2ac08e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -5,8 +5,9 @@ * 2.0. */ -import { countBy } from 'lodash/fp'; +import { countBy, partition } from 'lodash/fp'; import uuid from 'uuid'; +import { Action, Actions } from '@kbn/securitysolution-io-ts-alerting-types'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; @@ -193,11 +194,34 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; +const swapActionIds = async (action: Action, actionsClient: ActionsClient): Promise => { + const resolveResponse = await actionsClient.resolve({ id: action.id }); + console.error('RESOLVE RESPONSE', JSON.stringify(resolveResponse, null, 2)); + if (resolveResponse.outcome === 'aliasMatch') { + const tempAction: Action = { ...action, id: resolveResponse.id }; + return tempAction; + } + return action; +}; + export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], actionsClient: ActionsClient -) => { - const actionsFind = await actionsClient.getAll(); +): Promise => { + console.error('\n\n\n\n\n********\nDID WE GET IN HERE?\n********\n'); + const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); + const rulesToReturn = rules.map(async (rule) => { + if (isImportRule(rule)) { + const newActions = await Promise.all( + rule.actions.map((action) => swapActionIds(action, actionsClient)) + ).catch((err) => console.error('INTERIOR ERROR', err)); + return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; + } + return rule; + }); + await Promise.all(rulesToReturn).catch((err) => console.error('THERE WAS AN ERROR', err)); + console.error('RULES TO RETURN', JSON.stringify(rulesToReturn, null, 2)); + return rulesToReturn; }; /** @@ -212,7 +236,7 @@ export const getInvalidConnectors = async ( actionsClient: ActionsClient ): Promise<[BulkError[], PromiseFromStreams[]]> => { const actionsFind = await actionsClient.getAll(); - const actionIds = new Set(actionsFind.flatMap((action) => [action.id, action?.originId])); + const actionIds = new Set(actionsFind.flatMap((action) => [action.id])); const { errors, rulesAcc } = rules.reduce( (acc, parsedRule) => { if (parsedRule instanceof Error) { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 9dd294a9e1623..4f10dd5a661f9 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -553,7 +553,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: '51b17790-544e-11ec-a349-11361cc441c4', // 'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' + id: 'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' params: { message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', }, From fa8b09f3092f2b262c6708db7cd35ea1dc74e0ea Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 8 Dec 2021 17:15:56 -0500 Subject: [PATCH 03/19] working with actions client resolve --- .../lib/detection_engine/routes/rules/utils.ts | 12 +++++++----- .../security_and_spaces/tests/import_rules.ts | 2 +- .../import_rule_connector/data.json | 9 ++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 6a625be2ac08e..9c1cb125deea2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -196,9 +196,9 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( const swapActionIds = async (action: Action, actionsClient: ActionsClient): Promise => { const resolveResponse = await actionsClient.resolve({ id: action.id }); - console.error('RESOLVE RESPONSE', JSON.stringify(resolveResponse, null, 2)); + console.error('RESOLVE RESPONSE', resolveResponse); if (resolveResponse.outcome === 'aliasMatch') { - const tempAction: Action = { ...action, id: resolveResponse.id }; + const tempAction: Action = { ...action, id: resolveResponse.alias_target_id }; return tempAction; } return action; @@ -219,9 +219,11 @@ export const migrateLegacyActionsIds = async ( } return rule; }); - await Promise.all(rulesToReturn).catch((err) => console.error('THERE WAS AN ERROR', err)); - console.error('RULES TO RETURN', JSON.stringify(rulesToReturn, null, 2)); - return rulesToReturn; + const resolvedRulesToReturn = await Promise.all(rulesToReturn).catch((err) => + console.error('THERE WAS AN ERROR', err) + ); + console.error('RULES TO RETURN', JSON.stringify(resolvedRulesToReturn, null, 2)); + return resolvedRulesToReturn; }; /** diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 4f10dd5a661f9..f160c81dc3fd6 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -553,7 +553,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: 'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' + id: '51b17790-544e-11ec-a349-11361cc441c4', //'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' params: { message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', }, diff --git a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json index 945408b6d0067..a97034c3e3cab 100644 --- a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json +++ b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json @@ -21,11 +21,12 @@ } } + { "type": "doc", "value": { "index": ".kibana_1", - "id": "action:cf92c3c5-203e-54be-bb57-9348afc6651e", + "id": "714-space:action:51b17790-544e-11ec-a349-11361cc441c4", "source": { "action": { "actionTypeId": ".slack", @@ -36,12 +37,10 @@ }, "type": "action", "references": [], - "namespaces": ["714-space"], - "originId": "51b17790-544e-11ec-a349-11361cc441c4", + "namespace": "714-space", "migrationVersion": { - "action": "8.0.0" + "action": "7.14.0" }, - "coreMigrationVersion": "8.0.0", "updated_at": "2021-12-03T15:33:09.651Z" } } From 4b43eb56407ac542da4b4ea40909f2a41f91e95b Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 8 Dec 2021 18:38:53 -0500 Subject: [PATCH 04/19] use elasticsearch instead of saved objects client resolve api --- .../routes/rules/import_rules_route.ts | 2 +- .../detection_engine/routes/rules/utils.ts | 40 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index 48f8285c33ea2..9773de809c5a5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -126,7 +126,7 @@ export const importRulesRoute = ( const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( parsedObjectsWithoutDuplicateErrors, - actionsClient + esClient.asInternalUser ); const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 9c1cb125deea2..d545b3514dc14 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -27,6 +27,7 @@ import { RuleParams } from '../../schemas/rule_schemas'; import { SanitizedAlert } from '../../../../../../alerting/common'; // eslint-disable-next-line no-restricted-imports import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rule_actions_saved_object'; +import { SavedObjectsClientContract } from 'kibana/server'; type PromiseFromStreams = ImportRulesSchemaDecoded | Error; @@ -194,26 +195,49 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; -const swapActionIds = async (action: Action, actionsClient: ActionsClient): Promise => { - const resolveResponse = await actionsClient.resolve({ id: action.id }); - console.error('RESOLVE RESPONSE', resolveResponse); - if (resolveResponse.outcome === 'aliasMatch') { - const tempAction: Action = { ...action, id: resolveResponse.alias_target_id }; - return tempAction; +const swapActionIds = async ( + action: Action, + savedObjectsClient: SavedObjectsClientContract +): Promise => { + try { + // const resolveResponse = await savedObjectsClient.find({ + // type: 'space', + // }); + const resolveResponse = await savedObjectsClient.search( + { + index: '.kibana', + body: { + query: { + term: { + originId: { + value: action.id, + }, + }, + }, + }, + }, + { meta: true } + ); + console.error('RESOLVE RESPONSE', JSON.stringify(resolveResponse.body, null, 2)); + if (resolveResponse.body.hits.hits.length === 1) { + return { ...action, id: resolveResponse.body.hits.hits[0]._id.split(':')[1] }; + } + } catch (exc) { + console.error('SWAP ACTION IDS EXCEPTION', JSON.stringify(exc, null, 2)); } return action; }; export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], - actionsClient: ActionsClient + savedObjectsClient: SavedObjectsClientContract ): Promise => { console.error('\n\n\n\n\n********\nDID WE GET IN HERE?\n********\n'); const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); const rulesToReturn = rules.map(async (rule) => { if (isImportRule(rule)) { const newActions = await Promise.all( - rule.actions.map((action) => swapActionIds(action, actionsClient)) + rule.actions.map((action) => swapActionIds(action, savedObjectsClient)) ).catch((err) => console.error('INTERIOR ERROR', err)); return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; } From 8c40aad509f044ec8b5901d4ad83034a08bb0b3d Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 9 Dec 2021 12:10:48 -0500 Subject: [PATCH 05/19] cleanup --- .../plugins/actions/server/actions_client.ts | 51 ------------------- .../detection_engine/routes/rules/utils.ts | 32 ++++-------- .../security_and_spaces/tests/import_rules.ts | 46 +++++++---------- 3 files changed, 27 insertions(+), 102 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index d81eb52823d22..deaa1a79d1640 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/x-pack/plugins/actions/server/actions_client.ts @@ -323,56 +323,6 @@ export class ActionsClient { }; } - /** - * resolve an action - * @returns actionResult - */ - public async resolve({ id }: { id: string }): Promise { - try { - await this.authorization.ensureAuthorized('get'); - } catch (error) { - this.auditLogger?.log( - connectorAuditEvent({ - action: ConnectorAuditAction.GET, - savedObject: { type: 'action', id }, - error, - }) - ); - throw error; - } - - const preconfiguredActionsList = this.preconfiguredActions.find( - (preconfiguredAction) => preconfiguredAction.id === id - ); - if (preconfiguredActionsList !== undefined) { - this.auditLogger?.log( - connectorAuditEvent({ - action: ConnectorAuditAction.GET, - savedObject: { type: 'action', id }, - }) - ); - - return { - id, - actionTypeId: preconfiguredActionsList.actionTypeId, - name: preconfiguredActionsList.name, - isPreconfigured: true, - }; - } - - const result = await this.unsecuredSavedObjectsClient.resolve('action', id); - - this.auditLogger?.log( - connectorAuditEvent({ - action: ConnectorAuditAction.GET, - savedObject: { type: 'action', id }, - }) - ); - - return { - ...result, - }; - } /** * Get all actions with preconfigured list */ @@ -597,7 +547,6 @@ function actionFromSavedObject(savedObject: SavedObject): ActionResul return { id: savedObject.id, ...savedObject.attributes, - originId: savedObject.originId, isPreconfigured: false, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index d545b3514dc14..6363b1249d668 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -5,9 +5,10 @@ * 2.0. */ -import { countBy, partition } from 'lodash/fp'; +import { countBy } from 'lodash/fp'; import uuid from 'uuid'; -import { Action, Actions } from '@kbn/securitysolution-io-ts-alerting-types'; +import { Action } from '@kbn/securitysolution-io-ts-alerting-types'; +import { ElasticsearchClient } from 'kibana/server'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; @@ -27,7 +28,6 @@ import { RuleParams } from '../../schemas/rule_schemas'; import { SanitizedAlert } from '../../../../../../alerting/common'; // eslint-disable-next-line no-restricted-imports import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rule_actions_saved_object'; -import { SavedObjectsClientContract } from 'kibana/server'; type PromiseFromStreams = ImportRulesSchemaDecoded | Error; @@ -195,15 +195,9 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; -const swapActionIds = async ( - action: Action, - savedObjectsClient: SavedObjectsClientContract -): Promise => { +const swapActionIds = async (action: Action, esClient: ElasticsearchClient): Promise => { try { - // const resolveResponse = await savedObjectsClient.find({ - // type: 'space', - // }); - const resolveResponse = await savedObjectsClient.search( + const resolveResponse = await esClient.search( { index: '.kibana', body: { @@ -218,35 +212,29 @@ const swapActionIds = async ( }, { meta: true } ); - console.error('RESOLVE RESPONSE', JSON.stringify(resolveResponse.body, null, 2)); if (resolveResponse.body.hits.hits.length === 1) { return { ...action, id: resolveResponse.body.hits.hits[0]._id.split(':')[1] }; } } catch (exc) { - console.error('SWAP ACTION IDS EXCEPTION', JSON.stringify(exc, null, 2)); + return action; } return action; }; export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], - savedObjectsClient: SavedObjectsClientContract + esClient: ElasticsearchClient ): Promise => { - console.error('\n\n\n\n\n********\nDID WE GET IN HERE?\n********\n'); const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); const rulesToReturn = rules.map(async (rule) => { if (isImportRule(rule)) { - const newActions = await Promise.all( - rule.actions.map((action) => swapActionIds(action, savedObjectsClient)) - ).catch((err) => console.error('INTERIOR ERROR', err)); + const actionsMap = rule.actions.map((action: Action) => swapActionIds(action, esClient)); + const newActions = await Promise.all(actionsMap); return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; } return rule; }); - const resolvedRulesToReturn = await Promise.all(rulesToReturn).catch((err) => - console.error('THERE WAS AN ERROR', err) - ); - console.error('RULES TO RETURN', JSON.stringify(resolvedRulesToReturn, null, 2)); + const resolvedRulesToReturn = await Promise.all(rulesToReturn); return resolvedRulesToReturn; }; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index f160c81dc3fd6..eb96187f615e6 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -31,16 +31,10 @@ export default ({ getService }: FtrProviderContext): void => { describe('importing rules with an index', () => { beforeEach(async () => { await createSignalsIndex(supertest, log); - await esArchiver.load( - 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' - ); }); afterEach(async () => { await deleteSignalsIndex(supertest, log); - await esArchiver.unload( - 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' - ); await deleteAllAlerts(supertest, log); }); @@ -484,27 +478,15 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it.only('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + it('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + await esArchiver.load( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); const spaceId = '714-space'; - const connectorId = 'cf92c3c5-203e-54be-bb57-9348afc6651e'; - - // const rule1: ReturnType = { - // ...getSimpleRule('rule-1'), - // actions: [ - // { - // group: 'default', - // id: '51b17790-544e-11ec-a349-11361cc441c4', - // action_type_id: '.slack', - // params: {}, - // }, - // ], - // }; - - const connectorRes = await supertest - .get(`/s/${spaceId}/api/actions/connectors`) //${connectorId}`) - .set('kbn-xsrf', 'true') - .send(); - console.error('DOES THE CONNECTOR ID EXIST?', JSON.stringify(connectorRes, null, 2)); + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + // it + const connectorId = '51b17790-544e-11ec-a349-11361cc441c4'; const rule1 = { id: '53aad690-544e-11ec-a349-11361cc441c4', @@ -553,7 +535,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: '51b17790-544e-11ec-a349-11361cc441c4', //'cf92c3c5-203e-54be-bb57-9348afc6651e', // '51b17790-544e-11ec-a349-11361cc441c4' + id: connectorId, params: { message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', }, @@ -568,8 +550,14 @@ export default ({ getService }: FtrProviderContext): void => { const { body } = await supertest .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) .set('kbn-xsrf', 'true') - .attach('file', buffer, 'rules.ndjson'); - console.error('BODY', JSON.stringify(body, null, 2)); + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.eql(true); + expect(body.success_count).to.eql(1); + expect(body.errors.length).to.eql(0); + await esArchiver.unload( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); }); }); }); From 45e40bd5c6fa63900ec788793a583d0e99cc5ef4 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 9 Dec 2021 12:13:46 -0500 Subject: [PATCH 06/19] more cleanup --- .../detection_engine/routes/rules/utils.ts | 2 +- .../tests/resolve_read_rules.ts | 24 ++++++++--------- .../resolve_read_rules/7_14/data.json | 26 ------------------- .../resolve_read_rules/7_14/mappings.json | 3 --- 4 files changed, 13 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 6363b1249d668..9b1ceed4f4784 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -250,7 +250,7 @@ export const getInvalidConnectors = async ( actionsClient: ActionsClient ): Promise<[BulkError[], PromiseFromStreams[]]> => { const actionsFind = await actionsClient.getAll(); - const actionIds = new Set(actionsFind.flatMap((action) => [action.id])); + const actionIds = new Set(actionsFind.map((action) => [action.id])); const { errors, rulesAcc } = rules.reduce( (acc, parsedRule) => { if (parsedRule instanceof Error) { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts index 09ed0b78e4e27..c8c05bb8cdc6c 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts @@ -37,19 +37,19 @@ export default ({ getService }: FtrProviderContext) => { ); }); - it.only('should create a "migrated" rule where querying for the new SO _id will resolve the new object and not return the outcome field when outcome === exactMatch', async () => { + it('should create a "migrated" rule where querying for the new SO _id will resolve the new object and not return the outcome field when outcome === exactMatch', async () => { // link to the new URL with migrated SO id 74f3e6d7-b7bb-477d-ac28-92ee22728e6e - // const URL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=90e3ca0e-71f7-513a-b60a-ac678efd8887`; - // const readRulesAliasMatchRes = await supertest.get(URL).set('kbn-xsrf', 'true').send(); - // expect(readRulesAliasMatchRes.body.outcome).to.eql('aliasMatch'); - // // now that we have the migrated alias_target_id, let's attempt an 'exactMatch' query - // // the result of which should have the outcome as undefined when querying the read rules api. - // const exactMatchURL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=${readRulesAliasMatchRes.body.alias_target_id}`; - // const readRulesExactMatchRes = await supertest - // .get(exactMatchURL) - // .set('kbn-xsrf', 'true') - // .send(); - // expect(readRulesExactMatchRes.body.outcome).to.eql(undefined); + const URL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=90e3ca0e-71f7-513a-b60a-ac678efd8887`; + const readRulesAliasMatchRes = await supertest.get(URL).set('kbn-xsrf', 'true').send(); + expect(readRulesAliasMatchRes.body.outcome).to.eql('aliasMatch'); + // now that we have the migrated alias_target_id, let's attempt an 'exactMatch' query + // the result of which should have the outcome as undefined when querying the read rules api. + const exactMatchURL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=${readRulesAliasMatchRes.body.alias_target_id}`; + const readRulesExactMatchRes = await supertest + .get(exactMatchURL) + .set('kbn-xsrf', 'true') + .send(); + expect(readRulesExactMatchRes.body.outcome).to.eql(undefined); }); it('should create a rule and a "conflicting rule" where the SO _id matches the sourceId (see legacy-url-alias SO) of a migrated rule', async () => { diff --git a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json index 89247b1d094a5..fc078b6164b2e 100644 --- a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json +++ b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/data.json @@ -99,29 +99,3 @@ } } } - -{ - "type": "doc", - "value": { - "index": ".kibana_1", - "id": "action:cf92c3c5-203e-54be-bb57-9348afc6651e", - "source": { - "action": { - "actionTypeId": ".slack", - "name": "7.16 test connector", - "isMissingSecrets": false, - "config": {}, - "secrets": "fEO5Lk2AxFWtXNpTyg2DZKaZCjeCfMh/DGch02neTGJ/Hzu+w4DXigUVuUtgynNyRbhe7TbnTzi44jVg39WB3VR3yoWSFtvV/W7NHa3B1Kr3za1S3V4XCIu/CMIk0k8vnQMiNGiMuolwws6UjvQk8fiVJygjznhEGc66TMuAmKdz7fM=" - }, - "type": "action", - "references": [], - "namespaces": ["714-space"], - "originId": "51b17790-544e-11ec-a349-11361cc441c4", - "migrationVersion": { - "action": "8.0.0" - }, - "coreMigrationVersion": "8.0.0", - "updated_at": "2021-12-03T15:33:09.651Z" - } - } -} diff --git a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json index 9ff4456560aa7..069f70badce4e 100644 --- a/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json +++ b/x-pack/test/functional/es_archives/security_solution/resolve_read_rules/7_14/mappings.json @@ -63,9 +63,6 @@ "actionTypeId": { "type": "keyword" }, - "isMissingSecrets": { - "type": "boolean" - }, "config": { "enabled": false, "type": "object" From 9adc3dbae9a3a4689e3eafe19174b31ba94cbf7c Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 9 Dec 2021 15:41:09 -0500 Subject: [PATCH 07/19] updates comments and a little bit of cleanup --- .../detection_engine/routes/rules/utils.ts | 68 +++++++++++++++---- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 9b1ceed4f4784..4cb20799051b1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -195,25 +195,37 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; +/** + * Query for a saved object with a given origin id and replace the + * id in the provided action with the _id from the query result + * @param action + * @param esClient + * @returns + */ const swapActionIds = async (action: Action, esClient: ElasticsearchClient): Promise => { try { - const resolveResponse = await esClient.search( - { - index: '.kibana', - body: { - query: { - term: { - originId: { - value: action.id, - }, + // actions are hidden SO types so we need to query + // with the elasticsearch client.. unless there + // is a better way to do this.. + const actionWithRequestedOriginId = await esClient.search({ + index: '.kibana', + body: { + query: { + term: { + originId: { + // we are attempting to import a rule that contains an action. + // That action (connector) has the old, pre-8.0 _id. + // We need to swap the old, pre-8.0 _id for the new id (alias_target_id) + // and replace the id of the action in the rule with the new _id + value: action.id, }, }, }, }, - { meta: true } - ); - if (resolveResponse.body.hits.hits.length === 1) { - return { ...action, id: resolveResponse.body.hits.hits[0]._id.split(':')[1] }; + }); + if (actionWithRequestedOriginId.body.hits.hits.length === 1) { + // id is of the form 'action:1234-5678...' so I need to split off the 'action' prefix + return { ...action, id: actionWithRequestedOriginId.body.hits.hits[0]._id.split(':')[1] }; } } catch (exc) { return action; @@ -221,6 +233,36 @@ const swapActionIds = async (action: Action, esClient: ElasticsearchClient): Pro return action; }; +/** + * In 8.0 all saved objects made in a non-default space will have their + * _id's regenerated. Security Solution rules have references to the + * actions SO id inside the 'actions' param. + * When users import these rules, we need to ensure any rule with + * an action that has an old, pre-8.0 id will need to be updated + * to point to the new _id for that action (alias_target_id) + * + * ex: + * import rule.ndjson: + * { + * rule_id: 'myrule_id' + * name: 'my favorite rule' + * ... + * actions:[{id: '1111-2222-3333-4444', group...}] + * } + * + * In 8.0 the 'id' field of this action is no longer a reference + * to the _id of the action (connector). Querying against the connector + * endpoint for this id will yield 0 results / 404. + * + * The solution: If we query the .kibana index for '1111-2222-3333-4444' as an originId, + * we should get the original connector back + * (with the new, migrated 8.0 _id of 'xxxx-yyyy-zzzz-0000') and can then replace + * '1111-2222-3333-4444' in the example above with 'xxxx-yyyy-zzzz-0000' + * And the rule will then import successfully. + * @param rules + * @param esClient + * @returns + */ export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], esClient: ElasticsearchClient From 14f2d951ac647ff8df422359ab15d4c88794028f Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 9 Dec 2021 15:55:53 -0500 Subject: [PATCH 08/19] undo change --- .../server/lib/detection_engine/routes/rules/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 4cb20799051b1..1b68c820e5f83 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -292,7 +292,7 @@ export const getInvalidConnectors = async ( actionsClient: ActionsClient ): Promise<[BulkError[], PromiseFromStreams[]]> => { const actionsFind = await actionsClient.getAll(); - const actionIds = new Set(actionsFind.map((action) => [action.id])); + const actionIds = new Set(actionsFind.map((action) => action.id)); const { errors, rulesAcc } = rules.reduce( (acc, parsedRule) => { if (parsedRule instanceof Error) { From a8ac8be144eea37d1c7692f4336a0e33154dc0e1 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 9 Dec 2021 16:16:01 -0500 Subject: [PATCH 09/19] cleanup --- .../security_and_spaces/tests/index.ts | 92 +++++++++---------- .../tests/resolve_read_rules.ts | 1 + 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index 9505dbf0e6adb..db616baa0678a 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -13,60 +13,60 @@ export default ({ loadTestFile }: FtrProviderContext): void => { describe('', function () { this.tags('ciGroup11'); - // loadTestFile(require.resolve('./aliases')); - // loadTestFile(require.resolve('./create_endpoint_exceptions')); - // loadTestFile(require.resolve('./add_actions')); - // loadTestFile(require.resolve('./update_actions')); - // loadTestFile(require.resolve('./add_prepackaged_rules')); - // loadTestFile(require.resolve('./check_privileges')); - // loadTestFile(require.resolve('./create_index')); - // loadTestFile(require.resolve('./create_rules')); - // loadTestFile(require.resolve('./create_rules_bulk')); - // loadTestFile(require.resolve('./create_ml')); - // loadTestFile(require.resolve('./create_threat_matching')); - // loadTestFile(require.resolve('./create_exceptions')); - // loadTestFile(require.resolve('./delete_rules')); - // loadTestFile(require.resolve('./delete_rules_bulk')); - // loadTestFile(require.resolve('./export_rules')); - // loadTestFile(require.resolve('./find_rules')); - // loadTestFile(require.resolve('./find_statuses')); - // loadTestFile(require.resolve('./generating_signals')); - // loadTestFile(require.resolve('./get_prepackaged_rules_status')); + loadTestFile(require.resolve('./aliases')); + loadTestFile(require.resolve('./create_endpoint_exceptions')); + loadTestFile(require.resolve('./add_actions')); + loadTestFile(require.resolve('./update_actions')); + loadTestFile(require.resolve('./add_prepackaged_rules')); + loadTestFile(require.resolve('./check_privileges')); + loadTestFile(require.resolve('./create_index')); + loadTestFile(require.resolve('./create_rules')); + loadTestFile(require.resolve('./create_rules_bulk')); + loadTestFile(require.resolve('./create_ml')); + loadTestFile(require.resolve('./create_threat_matching')); + loadTestFile(require.resolve('./create_exceptions')); + loadTestFile(require.resolve('./delete_rules')); + loadTestFile(require.resolve('./delete_rules_bulk')); + loadTestFile(require.resolve('./export_rules')); + loadTestFile(require.resolve('./find_rules')); + loadTestFile(require.resolve('./find_statuses')); + loadTestFile(require.resolve('./generating_signals')); + loadTestFile(require.resolve('./get_prepackaged_rules_status')); loadTestFile(require.resolve('./import_rules')); - // loadTestFile(require.resolve('./read_rules')); - // loadTestFile(require.resolve('./resolve_read_rules')); - // loadTestFile(require.resolve('./update_rules')); - // loadTestFile(require.resolve('./update_rules_bulk')); - // loadTestFile(require.resolve('./patch_rules_bulk')); - // loadTestFile(require.resolve('./perform_bulk_action')); - // loadTestFile(require.resolve('./patch_rules')); - // loadTestFile(require.resolve('./read_privileges')); - // loadTestFile(require.resolve('./open_close_signals')); - // loadTestFile(require.resolve('./get_signals_migration_status')); - // loadTestFile(require.resolve('./create_signals_migrations')); - // loadTestFile(require.resolve('./finalize_signals_migrations')); - // loadTestFile(require.resolve('./delete_signals_migrations')); - // loadTestFile(require.resolve('./timestamps')); - // loadTestFile(require.resolve('./runtime')); - // loadTestFile(require.resolve('./throttle')); - // loadTestFile(require.resolve('./ignore_fields')); - // loadTestFile(require.resolve('./migrations')); + loadTestFile(require.resolve('./read_rules')); + loadTestFile(require.resolve('./resolve_read_rules')); + loadTestFile(require.resolve('./update_rules')); + loadTestFile(require.resolve('./update_rules_bulk')); + loadTestFile(require.resolve('./patch_rules_bulk')); + loadTestFile(require.resolve('./perform_bulk_action')); + loadTestFile(require.resolve('./patch_rules')); + loadTestFile(require.resolve('./read_privileges')); + loadTestFile(require.resolve('./open_close_signals')); + loadTestFile(require.resolve('./get_signals_migration_status')); + loadTestFile(require.resolve('./create_signals_migrations')); + loadTestFile(require.resolve('./finalize_signals_migrations')); + loadTestFile(require.resolve('./delete_signals_migrations')); + loadTestFile(require.resolve('./timestamps')); + loadTestFile(require.resolve('./runtime')); + loadTestFile(require.resolve('./throttle')); + loadTestFile(require.resolve('./ignore_fields')); + loadTestFile(require.resolve('./migrations')); }); // That split here enable us on using a different ciGroup to run the tests // listed on ./exception_operators_data_types/index - // describe('', function () { - // loadTestFile(require.resolve('./exception_operators_data_types/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./exception_operators_data_types/index')); + }); // That split here enable us on using a different ciGroup to run the tests // listed on ./keyword_family/index - // describe('', function () { - // loadTestFile(require.resolve('./keyword_family/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./keyword_family/index')); + }); - // describe('', function () { - // loadTestFile(require.resolve('./alerts/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./alerts/index')); + }); }); }; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts index c8c05bb8cdc6c..a7333ef8716f2 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/resolve_read_rules.ts @@ -42,6 +42,7 @@ export default ({ getService }: FtrProviderContext) => { const URL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=90e3ca0e-71f7-513a-b60a-ac678efd8887`; const readRulesAliasMatchRes = await supertest.get(URL).set('kbn-xsrf', 'true').send(); expect(readRulesAliasMatchRes.body.outcome).to.eql('aliasMatch'); + // now that we have the migrated alias_target_id, let's attempt an 'exactMatch' query // the result of which should have the outcome as undefined when querying the read rules api. const exactMatchURL = `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}?id=${readRulesAliasMatchRes.body.alias_target_id}`; From 67836fa9a328d059e2a7fb50ed4da495f3fcf055 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 10 Dec 2021 10:19:39 -0500 Subject: [PATCH 10/19] fix typecheck failure --- .../lib/detection_engine/routes/rules/import_rules_route.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index 3e33d118417d9..c5027fea01ae1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -31,8 +31,6 @@ import { buildSiemResponse, } from '../utils'; -import { patchRules } from '../../rules/patch_rules'; -import { legacyMigrate } from '../../rules/utils'; import { getTupleDuplicateErrorsAndUniqueRules, getInvalidConnectors, From 83b7d705cb9ef0ecdd0c66c6b4d8d338823467aa Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 14 Dec 2021 21:38:33 -0500 Subject: [PATCH 11/19] some error handling and added jest tests for swapActionIds and migrateLegacyActionsIds --- .../routes/rules/utils.test.ts | 216 +++++++++++++++++- .../detection_engine/routes/rules/utils.ts | 52 ++++- 2 files changed, 256 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index 7f1628db33cd8..c241304a08899 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -7,6 +7,7 @@ import { Readable } from 'stream'; import { createPromiseFromStreams } from '@kbn/utils'; +import { Action, ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types'; import { transformAlertToRule, @@ -19,6 +20,8 @@ import { getDuplicates, getTupleDuplicateErrorsAndUniqueRules, getInvalidConnectors, + swapActionIds, + migrateLegacyActionsIds, } from './utils'; import { getAlertMock } from '../__mocks__/request_responses'; import { INTERNAL_IDENTIFIER } from '../../../../../common/constants'; @@ -30,7 +33,6 @@ import { createRulesAndExceptionsStreamFromNdJson } from '../../rules/create_rul import { RuleAlertType } from '../../rules/types'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock'; -import { ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types'; import { CreateRulesBulkSchema } from '../../../../../common/detection_engine/schemas/request'; import { getMlRuleParams, @@ -652,6 +654,218 @@ describe.each([ }); }); + describe('swapActionIds', () => { + const mockAction: Action = { + group: 'group string', + id: 'some-7.x-id', + action_type_id: '.slack', + params: {}, + }; + beforeEach(() => { + clients.core.elasticsearch.client.asInternalUser.search.mockReset(); + clients.core.elasticsearch.client.asInternalUser.search.mockClear(); + }); + + test('returns original action if Elasticsearch query fails', async () => { + clients.core.elasticsearch.client.asInternalUser.search.mockRejectedValueOnce( + new Error('failed to query') + ); + const result = await swapActionIds( + mockAction, + clients.core.elasticsearch.client.asInternalUser + ); + expect(result).toEqual(mockAction); + }); + + test('returns original action if Elasticsearch query returns no hits', async () => { + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { total: 0, hits: [] }, + }, + })); + const result = await swapActionIds( + mockAction, + clients.core.elasticsearch.client.asInternalUser + ); + expect(result).toEqual(mockAction); + }); + + test('returns error if conflicting action connectors are found -> two hits found with same originId', async () => { + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 2, + hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], + }, + }, + })); + const result = await swapActionIds( + mockAction, + clients.core.elasticsearch.client.asInternalUser + ); + expect(result instanceof Error).toBeTruthy(); + expect((result as unknown as Error).message).toEqual( + 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.' + ); + }); + + test('returns action with new migrated _id if a single hit is found when querying by action connector originId', async () => { + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 1, + hits: [{ _id: 'action:new-post-8.0-id' }], + }, + }, + })); + const result = await swapActionIds( + mockAction, + clients.core.elasticsearch.client.asInternalUser + ); + expect(result).toEqual({ ...mockAction, id: 'new-post-8.0-id' }); + }); + }); + + describe('migrateLegacyActionsIds', () => { + const mockAction: Action = { + group: 'group string', + id: 'some-7.x-id', + action_type_id: '.slack', + params: {}, + }; + test('returns import rules schemas + migrated action', async () => { + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockAction], + }; + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 1, + hits: [{ _id: 'action:new-post-8.0-id' }], + }, + }, + })); + + const res = await migrateLegacyActionsIds( + // @ts-expect-error + [rule], + clients.core.elasticsearch.client.asInternalUser + ); + expect(res).toEqual([{ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }]); + }); + + test('returns import rules schemas + multiple migrated action', async () => { + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockAction, { ...mockAction, id: 'different-id' }], + }; + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementation(async () => ({ + body: { + hits: { + total: 1, + hits: [{ _id: 'action:new-post-8.0-id' }], + }, + }, + })); + + const res = await migrateLegacyActionsIds( + // @ts-expect-error + [rule], + clients.core.elasticsearch.client.asInternalUser + ); + expect(res).toEqual([ + { + ...rule, + actions: [ + { ...mockAction, id: 'new-post-8.0-id' }, + { ...mockAction, id: 'new-post-8.0-id' }, + ], + }, + ]); + }); + + test('returns import rules schemas + migrated action resulting in error', async () => { + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockAction], + }; + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 2, + hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], + }, + }, + })); + + const res = await migrateLegacyActionsIds( + // @ts-expect-error + [rule], + clients.core.elasticsearch.client.asInternalUser + ); + expect(res[0] instanceof Error).toBeTruthy(); + expect((res[0] as unknown as Error).message).toEqual( + JSON.stringify({ + rule_id: 'rule-1', + error: { + status_code: 409, + message: + 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.', + }, + }) + ); + }); + test('returns import multiple rules schemas + migrated action, one success and one error', async () => { + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockAction], + }; + + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 1, + hits: [{ _id: 'action:new-post-8.0-id' }], + }, + }, + })); + // @ts-expect-error + clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ + body: { + hits: { + total: 2, + hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], + }, + }, + })); + + const res = await migrateLegacyActionsIds( + // @ts-expect-error + [rule, rule], + clients.core.elasticsearch.client.asInternalUser + ); + expect(res[0]).toEqual({ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }); + expect(res[1] instanceof Error).toBeTruthy(); + expect((res[1] as unknown as Error).message).toEqual( + JSON.stringify({ + rule_id: 'rule-1', + error: { + status_code: 409, + message: + 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.', + }, + }) + ); + }); + }); describe('getInvalidConnectors', () => { beforeEach(() => { clients.actionsClient.getAll.mockReset(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 1b68c820e5f83..be530f524bb43 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -202,7 +202,10 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( * @param esClient * @returns */ -const swapActionIds = async (action: Action, esClient: ElasticsearchClient): Promise => { +export const swapActionIds = async ( + action: Action, + esClient: ElasticsearchClient +): Promise => { try { // actions are hidden SO types so we need to query // with the elasticsearch client.. unless there @@ -226,6 +229,10 @@ const swapActionIds = async (action: Action, esClient: ElasticsearchClient): Pro if (actionWithRequestedOriginId.body.hits.hits.length === 1) { // id is of the form 'action:1234-5678...' so I need to split off the 'action' prefix return { ...action, id: actionWithRequestedOriginId.body.hits.hits[0]._id.split(':')[1] }; + } else if (actionWithRequestedOriginId.body.hits.hits.length > 1) { + return new Error( + `action connector with originId: ${action.id} had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.` + ); } } catch (exc) { return action; @@ -268,16 +275,39 @@ export const migrateLegacyActionsIds = async ( esClient: ElasticsearchClient ): Promise => { const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); - const rulesToReturn = rules.map(async (rule) => { - if (isImportRule(rule)) { - const actionsMap = rule.actions.map((action: Action) => swapActionIds(action, esClient)); - const newActions = await Promise.all(actionsMap); - return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; - } - return rule; - }); - const resolvedRulesToReturn = await Promise.all(rulesToReturn); - return resolvedRulesToReturn; + return Promise.all( + rules.map(async (rule) => { + if (isImportRule(rule)) { + // can we swap the pre 8.0 action connector(s) id with the new, + // post-8.0 action id (swap the originId for the new _id?) + const newActions: Array = await Promise.all( + rule.actions.map((action: Action) => swapActionIds(action, esClient)) + ); + + // any errors discovered while trying to migrate + // and swap the action connector ids? + const actionMigrationErrors = newActions.filter( + (action): action is Error => action instanceof Error + ); + + if (actionMigrationErrors == null || actionMigrationErrors.length === 0) { + return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; + } + // return an Error object with the rule_id and the error messages + // for the actions associated with that rule. + return new Error( + JSON.stringify( + createBulkErrorObject({ + ruleId: rule.rule_id, + statusCode: 409, + message: `${actionMigrationErrors.map((error: Error) => error.message).join(',')}`, + }) + ) + ); + } + return rule; + }) + ); }; /** From 33dffbbe495d972713f72e074c896a5b341a743c Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 14 Dec 2021 22:01:06 -0500 Subject: [PATCH 12/19] only find origin id for action saved objects --- .../detection_engine/routes/rules/utils.ts | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index be530f524bb43..617c14adfbc98 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -214,14 +214,27 @@ export const swapActionIds = async ( index: '.kibana', body: { query: { - term: { - originId: { - // we are attempting to import a rule that contains an action. - // That action (connector) has the old, pre-8.0 _id. - // We need to swap the old, pre-8.0 _id for the new id (alias_target_id) - // and replace the id of the action in the rule with the new _id - value: action.id, - }, + bool: { + filter: [ + { + term: { + originId: { + // we are attempting to import a rule that contains an action. + // That action (connector) has the old, pre-8.0 _id. + // We need to swap the old, pre-8.0 _id for the new id (alias_target_id) + // and replace the id of the action in the rule with the new _id + value: action.id, + }, + }, + }, + { + term: { + type: { + value: 'action', + }, + }, + }, + ], }, }, }, From 50a2b40bf03bd5ebe0afae4210b4c38c355e9f06 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 16 Dec 2021 13:51:17 -0500 Subject: [PATCH 13/19] replacd es client with scoped SO client including hidden objects --- .../routes/rules/import_rules_route.ts | 6 +- .../detection_engine/routes/rules/utils.ts | 57 ++++------- .../security_and_spaces/tests/import_rules.ts | 3 +- .../security_and_spaces/tests/index.ts | 98 +++++++++---------- 4 files changed, 74 insertions(+), 90 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index c5027fea01ae1..fed6e6f70308f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -77,7 +77,9 @@ export const importRulesRoute = ( const rulesClient = context.alerting.getRulesClient(); const actionsClient = context.actions.getActionsClient(); const esClient = context.core.elasticsearch.client; - const savedObjectsClient = context.core.savedObjects.client; + const savedObjectsClient = context.core.savedObjects.getClient({ + includedHiddenTypes: ['action'], + }); // context.core.savedObjects.client; const siemClient = context.securitySolution.getAppClient(); const exceptionsClient = context.lists?.getExceptionListClient(); @@ -133,7 +135,7 @@ export const importRulesRoute = ( const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( parsedObjectsWithoutDuplicateErrors, - esClient.asInternalUser + savedObjectsClient ); const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 617c14adfbc98..c110528070004 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -8,7 +8,7 @@ import { countBy } from 'lodash/fp'; import uuid from 'uuid'; import { Action } from '@kbn/securitysolution-io-ts-alerting-types'; -import { ElasticsearchClient } from 'kibana/server'; +import { ElasticsearchClient, SavedObjectsClientContract } from 'kibana/server'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; @@ -195,6 +195,10 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; +const createQueryTerm = (input: string) => input.replace(/\\/g, '\\\\').replace(/\"/g, '\\"'); +const createQuery = (type: string, id: string, rawIdPrefix: string) => + `"${createQueryTerm(`${rawIdPrefix}${type}:${id}`)}" | "${createQueryTerm(id)}"`; + /** * Query for a saved object with a given origin id and replace the * id in the provided action with the _id from the query result @@ -204,45 +208,22 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( */ export const swapActionIds = async ( action: Action, - esClient: ElasticsearchClient + savedObjectsClient: SavedObjectsClientContract ): Promise => { try { - // actions are hidden SO types so we need to query - // with the elasticsearch client.. unless there - // is a better way to do this.. - const actionWithRequestedOriginId = await esClient.search({ - index: '.kibana', - body: { - query: { - bool: { - filter: [ - { - term: { - originId: { - // we are attempting to import a rule that contains an action. - // That action (connector) has the old, pre-8.0 _id. - // We need to swap the old, pre-8.0 _id for the new id (alias_target_id) - // and replace the id of the action in the rule with the new _id - value: action.id, - }, - }, - }, - { - term: { - type: { - value: 'action', - }, - }, - }, - ], - }, - }, - }, + // TODO: might need to pass down namespace + // but for now it's blank to do local testing. + const search = createQuery('action', action.id, ''); + const foundAction = await savedObjectsClient.find({ + type: 'action', + search, + rootSearchFields: ['_id', 'originId'], }); - if (actionWithRequestedOriginId.body.hits.hits.length === 1) { + + if (foundAction.saved_objects.length === 1) { // id is of the form 'action:1234-5678...' so I need to split off the 'action' prefix - return { ...action, id: actionWithRequestedOriginId.body.hits.hits[0]._id.split(':')[1] }; - } else if (actionWithRequestedOriginId.body.hits.hits.length > 1) { + return { ...action, id: foundAction.saved_objects[0].id }; + } else if (foundAction.saved_objects.length > 1) { return new Error( `action connector with originId: ${action.id} had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.` ); @@ -285,7 +266,7 @@ export const swapActionIds = async ( */ export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], - esClient: ElasticsearchClient + savedObjectsClient: SavedObjectsClientContract ): Promise => { const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); return Promise.all( @@ -294,7 +275,7 @@ export const migrateLegacyActionsIds = async ( // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) const newActions: Array = await Promise.all( - rule.actions.map((action: Action) => swapActionIds(action, esClient)) + rule.actions.map((action: Action) => swapActionIds(action, savedObjectsClient)) ); // any errors discovered while trying to migrate diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 35e6e6a88a1e0..4e1d058c4d6e7 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -486,7 +486,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + it.only('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { await esArchiver.load( 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' ); @@ -560,6 +560,7 @@ export default ({ getService }: FtrProviderContext): void => { .set('kbn-xsrf', 'true') .attach('file', buffer, 'rules.ndjson') .expect(200); + console.error('WHAT HAPPENED', JSON.stringify(body, null, 2)); expect(body.success).to.eql(true); expect(body.success_count).to.eql(1); expect(body.errors.length).to.eql(0); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index 55b7327670631..15d2992db7887 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -13,64 +13,64 @@ export default ({ loadTestFile }: FtrProviderContext): void => { describe('', function () { this.tags('ciGroup11'); - loadTestFile(require.resolve('./aliases')); - loadTestFile(require.resolve('./create_endpoint_exceptions')); - loadTestFile(require.resolve('./add_actions')); - loadTestFile(require.resolve('./update_actions')); - loadTestFile(require.resolve('./add_prepackaged_rules')); - loadTestFile(require.resolve('./check_privileges')); - loadTestFile(require.resolve('./create_index')); - loadTestFile(require.resolve('./create_rules')); - loadTestFile(require.resolve('./preview_rules')); - loadTestFile(require.resolve('./create_rules_bulk')); - loadTestFile(require.resolve('./create_ml')); - loadTestFile(require.resolve('./create_threat_matching')); - loadTestFile(require.resolve('./create_exceptions')); - loadTestFile(require.resolve('./delete_rules')); - loadTestFile(require.resolve('./delete_rules_bulk')); - loadTestFile(require.resolve('./export_rules')); - loadTestFile(require.resolve('./find_rules')); - loadTestFile(require.resolve('./generating_signals')); - loadTestFile(require.resolve('./get_prepackaged_rules_status')); + // loadTestFile(require.resolve('./aliases')); + // loadTestFile(require.resolve('./create_endpoint_exceptions')); + // loadTestFile(require.resolve('./add_actions')); + // loadTestFile(require.resolve('./update_actions')); + // loadTestFile(require.resolve('./add_prepackaged_rules')); + // loadTestFile(require.resolve('./check_privileges')); + // loadTestFile(require.resolve('./create_index')); + // loadTestFile(require.resolve('./create_rules')); + // loadTestFile(require.resolve('./preview_rules')); + // loadTestFile(require.resolve('./create_rules_bulk')); + // loadTestFile(require.resolve('./create_ml')); + // loadTestFile(require.resolve('./create_threat_matching')); + // loadTestFile(require.resolve('./create_exceptions')); + // loadTestFile(require.resolve('./delete_rules')); + // loadTestFile(require.resolve('./delete_rules_bulk')); + // loadTestFile(require.resolve('./export_rules')); + // loadTestFile(require.resolve('./find_rules')); + // loadTestFile(require.resolve('./generating_signals')); + // loadTestFile(require.resolve('./get_prepackaged_rules_status')); loadTestFile(require.resolve('./import_rules')); - loadTestFile(require.resolve('./read_rules')); - loadTestFile(require.resolve('./resolve_read_rules')); - loadTestFile(require.resolve('./update_rules')); - loadTestFile(require.resolve('./update_rules_bulk')); - loadTestFile(require.resolve('./patch_rules_bulk')); - loadTestFile(require.resolve('./perform_bulk_action')); - loadTestFile(require.resolve('./patch_rules')); - loadTestFile(require.resolve('./read_privileges')); - loadTestFile(require.resolve('./open_close_signals')); - loadTestFile(require.resolve('./get_signals_migration_status')); - loadTestFile(require.resolve('./create_signals_migrations')); - loadTestFile(require.resolve('./finalize_signals_migrations')); - loadTestFile(require.resolve('./delete_signals_migrations')); - loadTestFile(require.resolve('./timestamps')); - loadTestFile(require.resolve('./runtime')); - loadTestFile(require.resolve('./throttle')); - loadTestFile(require.resolve('./ignore_fields')); - loadTestFile(require.resolve('./migrations')); + // loadTestFile(require.resolve('./read_rules')); + // loadTestFile(require.resolve('./resolve_read_rules')); + // loadTestFile(require.resolve('./update_rules')); + // loadTestFile(require.resolve('./update_rules_bulk')); + // loadTestFile(require.resolve('./patch_rules_bulk')); + // loadTestFile(require.resolve('./perform_bulk_action')); + // loadTestFile(require.resolve('./patch_rules')); + // loadTestFile(require.resolve('./read_privileges')); + // loadTestFile(require.resolve('./open_close_signals')); + // loadTestFile(require.resolve('./get_signals_migration_status')); + // loadTestFile(require.resolve('./create_signals_migrations')); + // loadTestFile(require.resolve('./finalize_signals_migrations')); + // loadTestFile(require.resolve('./delete_signals_migrations')); + // loadTestFile(require.resolve('./timestamps')); + // loadTestFile(require.resolve('./runtime')); + // loadTestFile(require.resolve('./throttle')); + // loadTestFile(require.resolve('./ignore_fields')); + // loadTestFile(require.resolve('./migrations')); }); // That split here enable us on using a different ciGroup to run the tests // listed on ./exception_operators_data_types/index - describe('', function () { - loadTestFile(require.resolve('./exception_operators_data_types/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./exception_operators_data_types/index')); + // }); // That split here enable us on using a different ciGroup to run the tests // listed on ./keyword_family/index - describe('', function () { - loadTestFile(require.resolve('./keyword_family/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./keyword_family/index')); + // }); - describe('', function () { - loadTestFile(require.resolve('./alerts/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./alerts/index')); + // }); - describe('', function () { - loadTestFile(require.resolve('./telemetry/index')); - }); + // describe('', function () { + // loadTestFile(require.resolve('./telemetry/index')); + // }); }); }; From 9e0be05866de6a32d8d949bb294f8971d3489298 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 17 Dec 2021 17:56:54 -0500 Subject: [PATCH 14/19] fix jest tests to use mocked SO client --- .../routes/rules/utils.test.ts | 164 ++++++++---------- .../detection_engine/routes/rules/utils.ts | 2 +- .../security_and_spaces/tests/import_rules.ts | 3 +- .../security_and_spaces/tests/index.ts | 98 +++++------ 4 files changed, 128 insertions(+), 139 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index c241304a08899..bcec781b96784 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -53,7 +53,7 @@ describe.each([ ['Legacy', false], ['RAC', true], ])('utils - %s', (_, isRuleRegistryEnabled) => { - const { clients } = requestContextMock.createTools(); + const { clients, context } = requestContextMock.createTools(); describe('transformAlertToRule', () => { test('should work with a full data set', () => { @@ -661,50 +661,42 @@ describe.each([ action_type_id: '.slack', params: {}, }; + const soClient = clients.core.savedObjects.getClient(); beforeEach(() => { - clients.core.elasticsearch.client.asInternalUser.search.mockReset(); - clients.core.elasticsearch.client.asInternalUser.search.mockClear(); + soClient.find.mockReset(); + soClient.find.mockClear(); }); test('returns original action if Elasticsearch query fails', async () => { - clients.core.elasticsearch.client.asInternalUser.search.mockRejectedValueOnce( - new Error('failed to query') - ); - const result = await swapActionIds( - mockAction, - clients.core.elasticsearch.client.asInternalUser - ); + clients.core.savedObjects + .getClient() + .find.mockRejectedValueOnce(new Error('failed to query')); + const result = await swapActionIds(mockAction, soClient); expect(result).toEqual(mockAction); }); test('returns original action if Elasticsearch query returns no hits', async () => { - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { total: 0, hits: [] }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [], })); - const result = await swapActionIds( - mockAction, - clients.core.elasticsearch.client.asInternalUser - ); + const result = await swapActionIds(mockAction, soClient); expect(result).toEqual(mockAction); }); test('returns error if conflicting action connectors are found -> two hits found with same originId', async () => { - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 2, - hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'fake id 1', type: 'action', attributes: {}, references: [] }, + { score: 0, id: 'fake id 2', type: 'action', attributes: {}, references: [] }, + ], })); - const result = await swapActionIds( - mockAction, - clients.core.elasticsearch.client.asInternalUser - ); + const result = await swapActionIds(mockAction, soClient); expect(result instanceof Error).toBeTruthy(); expect((result as unknown as Error).message).toEqual( 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.' @@ -712,19 +704,15 @@ describe.each([ }); test('returns action with new migrated _id if a single hit is found when querying by action connector originId', async () => { - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 1, - hits: [{ _id: 'action:new-post-8.0-id' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + ], })); - const result = await swapActionIds( - mockAction, - clients.core.elasticsearch.client.asInternalUser - ); + const result = await swapActionIds(mockAction, soClient); expect(result).toEqual({ ...mockAction, id: 'new-post-8.0-id' }); }); }); @@ -736,25 +724,29 @@ describe.each([ action_type_id: '.slack', params: {}, }; + const soClient = clients.core.savedObjects.getClient(); + beforeEach(() => { + soClient.find.mockReset(); + soClient.find.mockClear(); + }); test('returns import rules schemas + migrated action', async () => { const rule: ReturnType = { ...getCreateRulesSchemaMock('rule-1'), actions: [mockAction], }; - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 1, - hits: [{ _id: 'action:new-post-8.0-id' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + ], })); const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - clients.core.elasticsearch.client.asInternalUser + soClient ); expect(res).toEqual([{ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }]); }); @@ -764,20 +756,19 @@ describe.each([ ...getCreateRulesSchemaMock('rule-1'), actions: [mockAction, { ...mockAction, id: 'different-id' }], }; - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementation(async () => ({ - body: { - hits: { - total: 1, - hits: [{ _id: 'action:new-post-8.0-id' }], - }, - }, + soClient.find.mockImplementation(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + ], })); const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - clients.core.elasticsearch.client.asInternalUser + soClient ); expect(res).toEqual([ { @@ -795,20 +786,20 @@ describe.each([ ...getCreateRulesSchemaMock('rule-1'), actions: [mockAction], }; - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 2, - hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + { score: 0, id: 'new-post-8.0-id-2', type: 'action', attributes: {}, references: [] }, + ], })); const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - clients.core.elasticsearch.client.asInternalUser + soClient ); expect(res[0] instanceof Error).toBeTruthy(); expect((res[0] as unknown as Error).message).toEqual( @@ -828,29 +819,28 @@ describe.each([ actions: [mockAction], }; - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 1, - hits: [{ _id: 'action:new-post-8.0-id' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + ], })); - // @ts-expect-error - clients.core.elasticsearch.client.asInternalUser.search.mockImplementationOnce(async () => ({ - body: { - hits: { - total: 2, - hits: [{ fakeActionKey: 'fakeActionValue' }, { fakeActionKey: 'fakeActionValue2' }], - }, - }, + soClient.find.mockImplementationOnce(async () => ({ + total: 0, + per_page: 0, + page: 1, + saved_objects: [ + { score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] }, + { score: 0, id: 'new-post-8.0-id-2', type: 'action', attributes: {}, references: [] }, + ], })); const res = await migrateLegacyActionsIds( // @ts-expect-error [rule, rule], - clients.core.elasticsearch.client.asInternalUser + soClient ); expect(res[0]).toEqual({ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }); expect(res[1] instanceof Error).toBeTruthy(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index c110528070004..9690d7cbd954b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -8,7 +8,7 @@ import { countBy } from 'lodash/fp'; import uuid from 'uuid'; import { Action } from '@kbn/securitysolution-io-ts-alerting-types'; -import { ElasticsearchClient, SavedObjectsClientContract } from 'kibana/server'; +import { SavedObjectsClientContract } from 'kibana/server'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 4e1d058c4d6e7..35e6e6a88a1e0 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -486,7 +486,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it.only('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + it('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { await esArchiver.load( 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' ); @@ -560,7 +560,6 @@ export default ({ getService }: FtrProviderContext): void => { .set('kbn-xsrf', 'true') .attach('file', buffer, 'rules.ndjson') .expect(200); - console.error('WHAT HAPPENED', JSON.stringify(body, null, 2)); expect(body.success).to.eql(true); expect(body.success_count).to.eql(1); expect(body.errors.length).to.eql(0); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index 15d2992db7887..55b7327670631 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -13,64 +13,64 @@ export default ({ loadTestFile }: FtrProviderContext): void => { describe('', function () { this.tags('ciGroup11'); - // loadTestFile(require.resolve('./aliases')); - // loadTestFile(require.resolve('./create_endpoint_exceptions')); - // loadTestFile(require.resolve('./add_actions')); - // loadTestFile(require.resolve('./update_actions')); - // loadTestFile(require.resolve('./add_prepackaged_rules')); - // loadTestFile(require.resolve('./check_privileges')); - // loadTestFile(require.resolve('./create_index')); - // loadTestFile(require.resolve('./create_rules')); - // loadTestFile(require.resolve('./preview_rules')); - // loadTestFile(require.resolve('./create_rules_bulk')); - // loadTestFile(require.resolve('./create_ml')); - // loadTestFile(require.resolve('./create_threat_matching')); - // loadTestFile(require.resolve('./create_exceptions')); - // loadTestFile(require.resolve('./delete_rules')); - // loadTestFile(require.resolve('./delete_rules_bulk')); - // loadTestFile(require.resolve('./export_rules')); - // loadTestFile(require.resolve('./find_rules')); - // loadTestFile(require.resolve('./generating_signals')); - // loadTestFile(require.resolve('./get_prepackaged_rules_status')); + loadTestFile(require.resolve('./aliases')); + loadTestFile(require.resolve('./create_endpoint_exceptions')); + loadTestFile(require.resolve('./add_actions')); + loadTestFile(require.resolve('./update_actions')); + loadTestFile(require.resolve('./add_prepackaged_rules')); + loadTestFile(require.resolve('./check_privileges')); + loadTestFile(require.resolve('./create_index')); + loadTestFile(require.resolve('./create_rules')); + loadTestFile(require.resolve('./preview_rules')); + loadTestFile(require.resolve('./create_rules_bulk')); + loadTestFile(require.resolve('./create_ml')); + loadTestFile(require.resolve('./create_threat_matching')); + loadTestFile(require.resolve('./create_exceptions')); + loadTestFile(require.resolve('./delete_rules')); + loadTestFile(require.resolve('./delete_rules_bulk')); + loadTestFile(require.resolve('./export_rules')); + loadTestFile(require.resolve('./find_rules')); + loadTestFile(require.resolve('./generating_signals')); + loadTestFile(require.resolve('./get_prepackaged_rules_status')); loadTestFile(require.resolve('./import_rules')); - // loadTestFile(require.resolve('./read_rules')); - // loadTestFile(require.resolve('./resolve_read_rules')); - // loadTestFile(require.resolve('./update_rules')); - // loadTestFile(require.resolve('./update_rules_bulk')); - // loadTestFile(require.resolve('./patch_rules_bulk')); - // loadTestFile(require.resolve('./perform_bulk_action')); - // loadTestFile(require.resolve('./patch_rules')); - // loadTestFile(require.resolve('./read_privileges')); - // loadTestFile(require.resolve('./open_close_signals')); - // loadTestFile(require.resolve('./get_signals_migration_status')); - // loadTestFile(require.resolve('./create_signals_migrations')); - // loadTestFile(require.resolve('./finalize_signals_migrations')); - // loadTestFile(require.resolve('./delete_signals_migrations')); - // loadTestFile(require.resolve('./timestamps')); - // loadTestFile(require.resolve('./runtime')); - // loadTestFile(require.resolve('./throttle')); - // loadTestFile(require.resolve('./ignore_fields')); - // loadTestFile(require.resolve('./migrations')); + loadTestFile(require.resolve('./read_rules')); + loadTestFile(require.resolve('./resolve_read_rules')); + loadTestFile(require.resolve('./update_rules')); + loadTestFile(require.resolve('./update_rules_bulk')); + loadTestFile(require.resolve('./patch_rules_bulk')); + loadTestFile(require.resolve('./perform_bulk_action')); + loadTestFile(require.resolve('./patch_rules')); + loadTestFile(require.resolve('./read_privileges')); + loadTestFile(require.resolve('./open_close_signals')); + loadTestFile(require.resolve('./get_signals_migration_status')); + loadTestFile(require.resolve('./create_signals_migrations')); + loadTestFile(require.resolve('./finalize_signals_migrations')); + loadTestFile(require.resolve('./delete_signals_migrations')); + loadTestFile(require.resolve('./timestamps')); + loadTestFile(require.resolve('./runtime')); + loadTestFile(require.resolve('./throttle')); + loadTestFile(require.resolve('./ignore_fields')); + loadTestFile(require.resolve('./migrations')); }); // That split here enable us on using a different ciGroup to run the tests // listed on ./exception_operators_data_types/index - // describe('', function () { - // loadTestFile(require.resolve('./exception_operators_data_types/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./exception_operators_data_types/index')); + }); // That split here enable us on using a different ciGroup to run the tests // listed on ./keyword_family/index - // describe('', function () { - // loadTestFile(require.resolve('./keyword_family/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./keyword_family/index')); + }); - // describe('', function () { - // loadTestFile(require.resolve('./alerts/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./alerts/index')); + }); - // describe('', function () { - // loadTestFile(require.resolve('./telemetry/index')); - // }); + describe('', function () { + loadTestFile(require.resolve('./telemetry/index')); + }); }); }; From 2ff74dae67684d1453a667b2f150802dd5ae00bc Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 29 Dec 2021 15:15:50 -0500 Subject: [PATCH 15/19] add more integration tests --- .../routes/rules/import_rules_route.ts | 7 +- .../routes/rules/utils.test.ts | 2 +- .../security_and_spaces/tests/import_rules.ts | 241 ++++++++++++------ .../import_rule_connector/data.json | 46 ++++ 4 files changed, 214 insertions(+), 82 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index fa97fc0279d9c..630e7907d4f55 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -78,9 +78,10 @@ export const importRulesRoute = ( const rulesClient = context.alerting.getRulesClient(); const actionsClient = context.actions.getActionsClient(); const esClient = context.core.elasticsearch.client; - const savedObjectsClient = context.core.savedObjects.getClient({ + const actionSOClient = context.core.savedObjects.getClient({ includedHiddenTypes: ['action'], - }); // context.core.savedObjects.client; + }); + const savedObjectsClient = context.core.savedObjects.client; const siemClient = context.securitySolution.getAppClient(); const exceptionsClient = context.lists?.getExceptionListClient(); @@ -135,7 +136,7 @@ export const importRulesRoute = ( const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( parsedObjectsWithoutDuplicateErrors, - savedObjectsClient + actionSOClient ); const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index bcec781b96784..ff05175e0bc4f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -53,7 +53,7 @@ describe.each([ ['Legacy', false], ['RAC', true], ])('utils - %s', (_, isRuleRegistryEnabled) => { - const { clients, context } = requestContextMock.createTools(); + const { clients } = requestContextMock.createTools(); describe('transformAlertToRule', () => { test('should work with a full data set', () => { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 0fa710008ff36..81e0374b96738 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -29,6 +29,67 @@ import { } from '../../../../plugins/lists/common/schemas/request/import_exceptions_schema.mock'; import { deleteAllExceptions } from '../../../lists_api_integration/utils'; +const getImportRuleBuffer = (connectorId: string) => { + const rule1 = { + id: '53aad690-544e-11ec-a349-11361cc441c4', + updated_at: '2021-12-03T15:33:13.271Z', + updated_by: 'elastic', + created_at: '2021-12-03T15:33:13.271Z', + created_by: 'elastic', + name: '7.16 test with action', + tags: [], + interval: '5m', + enabled: true, + description: 'test', + risk_score: 21, + severity: 'low', + license: '', + output_index: '.siem-signals-devin-hurley-7', + meta: { from: '1m', kibana_siem_app_url: 'http://0.0.0.0:5601/s/7/app/security' }, + author: [], + false_positives: [], + from: 'now-360s', + rule_id: 'aa525d7c-8948-439f-b32d-27e00c750246', + max_signals: 100, + risk_score_mapping: [], + severity_mapping: [], + threat: [], + to: 'now', + references: [], + version: 1, + exceptions_list: [], + immutable: false, + type: 'query', + language: 'kuery', + index: [ + 'apm-*-transaction*', + 'traces-apm*', + 'auditbeat-*', + 'endgame-*', + 'filebeat-*', + 'logs-*', + 'packetbeat-*', + 'winlogbeat-*', + ], + query: '*:*', + filters: [], + throttle: '1h', + actions: [ + { + group: 'default', + id: connectorId, + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + }, + ], + }; + const rule1String = JSON.stringify(rule1); + const buffer = Buffer.from(`${rule1String}\n`); + return buffer; +}; + // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); @@ -530,86 +591,110 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { - await esArchiver.load( - 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' - ); - const spaceId = '714-space'; - // connectorId is from the 7.x connector here - // x-pack/test/functional/es_archives/security_solution/import_rule_connector - // it - const connectorId = '51b17790-544e-11ec-a349-11361cc441c4'; - - const rule1 = { - id: '53aad690-544e-11ec-a349-11361cc441c4', - updated_at: '2021-12-03T15:33:13.271Z', - updated_by: 'elastic', - created_at: '2021-12-03T15:33:13.271Z', - created_by: 'elastic', - name: '7.16 test with action', - tags: [], - interval: '5m', - enabled: true, - description: 'test', - risk_score: 21, - severity: 'low', - license: '', - output_index: '.siem-signals-devin-hurley-7', - meta: { from: '1m', kibana_siem_app_url: 'http://0.0.0.0:5601/s/7/app/security' }, - author: [], - false_positives: [], - from: 'now-360s', - rule_id: 'aa525d7c-8948-439f-b32d-27e00c750246', - max_signals: 100, - risk_score_mapping: [], - severity_mapping: [], - threat: [], - to: 'now', - references: [], - version: 1, - exceptions_list: [], - immutable: false, - type: 'query', - language: 'kuery', - index: [ - 'apm-*-transaction*', - 'traces-apm*', - 'auditbeat-*', - 'endgame-*', - 'filebeat-*', - 'logs-*', - 'packetbeat-*', - 'winlogbeat-*', - ], - query: '*:*', - filters: [], - throttle: '1h', - actions: [ - { - group: 'default', - id: connectorId, - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - action_type_id: '.slack', - }, - ], - }; + describe('migrate pre-8.0 action connector ids', () => { + const defaultSpaceActionConnectorId = '61b17790-544e-11ec-a349-11361cc441c4'; + const space714ActionConnectorId = '51b17790-544e-11ec-a349-11361cc441c4'; + beforeEach(async () => { + await esArchiver.load( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); + }); + afterEach(async () => { + await esArchiver.unload( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); + }); - const rule1String = JSON.stringify(rule1); - const buffer = Buffer.from(`${rule1String}\n`); + it('importing a non-default-space 7.16 rule with a connector made in the non-default space should result in a 200', async () => { + const spaceId = '714-space'; + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + const buffer = getImportRuleBuffer(space714ActionConnectorId); - const { body } = await supertest - .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) - .set('kbn-xsrf', 'true') - .attach('file', buffer, 'rules.ndjson') - .expect(200); - expect(body.success).to.eql(true); - expect(body.success_count).to.eql(1); - expect(body.errors.length).to.eql(0); - await esArchiver.unload( - 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' - ); + const { body } = await supertest + .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.eql(true); + expect(body.success_count).to.eql(1); + expect(body.errors.length).to.eql(0); + }); + + // When objects become share-capable we will either add / update this test + it('importing a non-default-space 7.16 rule with a connector made in the non-default space into the default space should result in a 404', async () => { + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + const buffer = getImportRuleBuffer(space714ActionConnectorId); + + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.equal(false); + expect(body.errors[0].error.status_code).to.equal(404); + expect(body.errors[0].error.message).to.equal( + `1 connector is missing. Connector id missing is: ${space714ActionConnectorId}` + ); + }); + + // When objects become share-capable we will either add / update this test + it('importing a non-default-space 7.16 rule with a connector made in the non-default space into a different non-default space should result in a 404', async () => { + const spaceId = '4567-space'; + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + // it + const buffer = getImportRuleBuffer(space714ActionConnectorId); + + const { body } = await supertest + .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.equal(false); + expect(body.errors[0].error.status_code).to.equal(404); + expect(body.errors[0].error.message).to.equal( + `1 connector is missing. Connector id missing is: ${space714ActionConnectorId}` + ); + }); + + it('importing a default-space 7.16 rule with a connector made in the default space into the default space should result in a 200', async () => { + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + // it + const buffer = getImportRuleBuffer(defaultSpaceActionConnectorId); + + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.equal(true); + expect(body.success_count).to.eql(1); + expect(body.errors.length).to.eql(0); + }); + it('importing a default-space 7.16 rule with a connector made in the default space into a non-default space should result in a 404', async () => { + await esArchiver.load( + 'x-pack/test/functional/es_archives/security_solution/import_rule_connector' + ); + const spaceId = '4567-space'; + // connectorId is from the 7.x connector here + // x-pack/test/functional/es_archives/security_solution/import_rule_connector + // it + const buffer = getImportRuleBuffer(defaultSpaceActionConnectorId); + + const { body } = await supertest + .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', buffer, 'rules.ndjson') + .expect(200); + expect(body.success).to.equal(false); + expect(body.errors[0].error.status_code).to.equal(404); + expect(body.errors[0].error.message).to.equal( + `1 connector is missing. Connector id missing is: ${defaultSpaceActionConnectorId}` + ); + }); }); describe('importing with exceptions', () => { diff --git a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json index a97034c3e3cab..c705a2f439af4 100644 --- a/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json +++ b/x-pack/test/functional/es_archives/security_solution/import_rule_connector/data.json @@ -21,6 +21,29 @@ } } +{ + "type" : "doc", + "value": { + "index" : ".kibana_1", + "id" : "space:4567-space", + "source" : { + "space" : { + "name" : "4567-space", + "initials" : "t", + "color" : "#B9A888", + "disabledFeatures" : [ ], + "imageUrl" : "" + }, + "type" : "space", + "references" : [ ], + "migrationVersion" : { + "space" : "6.6.0" + }, + "updated_at" : "2021-10-11T14:49:07.012Z" + } + } +} + { "type": "doc", @@ -45,3 +68,26 @@ } } } + +{ + "type": "doc", + "value": { + "index": ".kibana_1", + "id": "action:61b17790-544e-11ec-a349-11361cc441c4", + "source": { + "action": { + "actionTypeId": ".slack", + "name": "7.16 test connector", + "isMissingSecrets": false, + "config": {}, + "secrets": "fEO5Lk2AxFWtXNpTyg2DZKaZCjeCfMh/DGch02neTGJ/Hzu+w4DXigUVuUtgynNyRbhe7TbnTzi44jVg39WB3VR3yoWSFtvV/W7NHa3B1Kr3za1S3V4XCIu/CMIk0k8vnQMiNGiMuolwws6UjvQk8fiVJygjznhEGc66TMuAmKdz7fM=" + }, + "type": "action", + "references": [], + "migrationVersion": { + "action": "7.14.0" + }, + "updated_at": "2021-12-03T15:33:09.651Z" + } + } +} From ced84107570132a26e7036c489c35e50834e881d Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 29 Dec 2021 15:37:13 -0500 Subject: [PATCH 16/19] fixes comments --- .../server/lib/detection_engine/routes/rules/utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 9690d7cbd954b..e05a7f6394b43 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -195,6 +195,8 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( return [Array.from(errors.values()), Array.from(rulesAcc.values())]; }; +// functions copied from here +// https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/src/core/server/saved_objects/import/lib/check_origin_conflicts.ts#L55-L57 const createQueryTerm = (input: string) => input.replace(/\\/g, '\\\\').replace(/\"/g, '\\"'); const createQuery = (type: string, id: string, rawIdPrefix: string) => `"${createQueryTerm(`${rawIdPrefix}${type}:${id}`)}" | "${createQueryTerm(id)}"`; @@ -221,7 +223,6 @@ export const swapActionIds = async ( }); if (foundAction.saved_objects.length === 1) { - // id is of the form 'action:1234-5678...' so I need to split off the 'action' prefix return { ...action, id: foundAction.saved_objects[0].id }; } else if (foundAction.saved_objects.length > 1) { return new Error( @@ -261,7 +262,7 @@ export const swapActionIds = async ( * '1111-2222-3333-4444' in the example above with 'xxxx-yyyy-zzzz-0000' * And the rule will then import successfully. * @param rules - * @param esClient + * @param savedObjectsClient SO client exposing hidden 'actions' SO type * @returns */ export const migrateLegacyActionsIds = async ( From 0262eb49d14f4ea738ed7c7d8627bcde37a7f46e Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 4 Jan 2022 12:56:47 -0500 Subject: [PATCH 17/19] updates from PR comments --- .../detection_engine/routes/rules/utils.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index e05a7f6394b43..b0d508b8297b1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -9,6 +9,7 @@ import { countBy } from 'lodash/fp'; import uuid from 'uuid'; import { Action } from '@kbn/securitysolution-io-ts-alerting-types'; import { SavedObjectsClientContract } from 'kibana/server'; +import pMap from 'p-map'; import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema'; import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema'; @@ -30,6 +31,7 @@ import { SanitizedAlert } from '../../../../../../alerting/common'; import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rule_actions_saved_object'; type PromiseFromStreams = ImportRulesSchemaDecoded | Error; +const MAX_CONCURRENT_SEARCHES = 10; export const getIdError = ({ id, @@ -198,8 +200,8 @@ export const getTupleDuplicateErrorsAndUniqueRules = ( // functions copied from here // https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/src/core/server/saved_objects/import/lib/check_origin_conflicts.ts#L55-L57 const createQueryTerm = (input: string) => input.replace(/\\/g, '\\\\').replace(/\"/g, '\\"'); -const createQuery = (type: string, id: string, rawIdPrefix: string) => - `"${createQueryTerm(`${rawIdPrefix}${type}:${id}`)}" | "${createQueryTerm(id)}"`; +const createQuery = (type: string, id: string) => + `"${createQueryTerm(`${type}:${id}`)}" | "${createQueryTerm(id)}"`; /** * Query for a saved object with a given origin id and replace the @@ -213,9 +215,7 @@ export const swapActionIds = async ( savedObjectsClient: SavedObjectsClientContract ): Promise => { try { - // TODO: might need to pass down namespace - // but for now it's blank to do local testing. - const search = createQuery('action', action.id, ''); + const search = createQuery('action', action.id); const foundAction = await savedObjectsClient.find({ type: 'action', search, @@ -226,7 +226,7 @@ export const swapActionIds = async ( return { ...action, id: foundAction.saved_objects[0].id }; } else if (foundAction.saved_objects.length > 1) { return new Error( - `action connector with originId: ${action.id} had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.` + `Found two action connectors with originId or _id: ${action.id} The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details` ); } } catch (exc) { @@ -270,8 +270,10 @@ export const migrateLegacyActionsIds = async ( savedObjectsClient: SavedObjectsClientContract ): Promise => { const isImportRule = (r: unknown): r is ImportRulesSchemaDecoded => !(r instanceof Error); - return Promise.all( - rules.map(async (rule) => { + + return pMap( + rules, + async (rule) => { if (isImportRule(rule)) { // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) @@ -279,14 +281,17 @@ export const migrateLegacyActionsIds = async ( rule.actions.map((action: Action) => swapActionIds(action, savedObjectsClient)) ); - // any errors discovered while trying to migrate - // and swap the action connector ids? + // were there any errors discovered while trying to migrate and swap the action connector ids? const actionMigrationErrors = newActions.filter( (action): action is Error => action instanceof Error ); + const newlyMigratedActions: Action[] = newActions.filter( + (action): action is Action => !(action instanceof Error) + ); + if (actionMigrationErrors == null || actionMigrationErrors.length === 0) { - return { ...rule, actions: newActions } as ImportRulesSchemaDecoded; + return { ...rule, actions: newlyMigratedActions }; } // return an Error object with the rule_id and the error messages // for the actions associated with that rule. @@ -301,7 +306,8 @@ export const migrateLegacyActionsIds = async ( ); } return rule; - }) + }, + { concurrency: MAX_CONCURRENT_SEARCHES } ); }; From 8ee2dc4e6e5dc1049c8b254a0faefd62c15ea8b9 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 4 Jan 2022 14:03:51 -0500 Subject: [PATCH 18/19] updates expected error message in jest test --- .../server/lib/detection_engine/routes/rules/utils.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index ff05175e0bc4f..8b4a9cf250226 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -699,7 +699,7 @@ describe.each([ const result = await swapActionIds(mockAction, soClient); expect(result instanceof Error).toBeTruthy(); expect((result as unknown as Error).message).toEqual( - 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.' + 'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details' ); }); @@ -808,7 +808,7 @@ describe.each([ error: { status_code: 409, message: - 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.', + 'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details', }, }) ); @@ -850,7 +850,7 @@ describe.each([ error: { status_code: 409, message: - 'action connector with originId: some-7.x-id had conflicts. Please resolve these conflicts either in the file you are attempting to upload or resolve the conflicting action connector in Kibana.', + 'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details', }, }) ); From a90ec5ab359097ddfa9661226ae1ca404a176f4e Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 4 Jan 2022 14:25:32 -0500 Subject: [PATCH 19/19] nest pMaps - pMap over rule.actions in case the number of promises is larger than max call stack. --- .../server/lib/detection_engine/routes/rules/utils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index b0d508b8297b1..a52a2804d3c7e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -277,8 +277,10 @@ export const migrateLegacyActionsIds = async ( if (isImportRule(rule)) { // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) - const newActions: Array = await Promise.all( - rule.actions.map((action: Action) => swapActionIds(action, savedObjectsClient)) + const newActions: Array = await pMap( + rule.actions, + (action: Action) => swapActionIds(action, savedObjectsClient), + { concurrency: MAX_CONCURRENT_SEARCHES } ); // were there any errors discovered while trying to migrate and swap the action connector ids?