Add tests to cover the migrations cases

x-pack/plugins/lists/common/constants.mock.ts

@@ -117,3 +117,6 @@ export const _VERSION = 'WzI5NywxXQ==';
 export const VERSION = 1;
 export const IMMUTABLE = false;
 export const IMPORT_TIMEOUT = moment.duration(5, 'minutes');
+
+/** Added in 8.7 */
+export const EXPIRE_TIME = '2023-04-24T19:00:00.000Z';

x-pack/plugins/lists/common/schemas/request/create_exception_list_item_schema.mock.ts

@@ -11,6 +11,7 @@ import {
   COMMENTS,
   DESCRIPTION,
   ENTRIES,
+  EXPIRE_TIME,
   ITEM_ID,
   ITEM_TYPE,
   LIST_ID,
@@ -60,3 +61,18 @@ export const getCreateExceptionListItemMinimalSchemaMockWithoutId =
     os_types: OS_TYPES,
     type: ITEM_TYPE,
   });
+
+/**
+ * Useful for testing newer exception list item versions, as the previous
+ * versions can be used to test migration cases
+ */
+export const getCreateExceptionListItemNewerVersionSchemaMock =
+  (): CreateExceptionListItemSchema => ({
+    description: DESCRIPTION,
+    entries: ENTRIES,
+    expire_time: EXPIRE_TIME,
+    list_id: LIST_ID,
+    name: NAME,
+    os_types: OS_TYPES,
+    type: ITEM_TYPE,
+  });

x-pack/plugins/lists/common/schemas/request/import_exceptions_schema.mock.ts

@@ -12,7 +12,7 @@ import {
   ImportExceptionsListSchema,
 } from '@kbn/securitysolution-io-ts-list-types';
 
-import { ENTRIES } from '../../constants.mock';
+import { ENTRIES, EXPIRE_TIME } from '../../constants.mock';
 
 export const getImportExceptionsListSchemaMock = (
   listId = 'detection_list_id'
@@ -23,6 +23,10 @@ export const getImportExceptionsListSchemaMock = (
   type: 'detection',
 });
 
+/** 
+  This mock holds the old properties of the Exception List item
+  so that we can test the migration test cases
+*/
 export const getImportExceptionsListItemSchemaMock = (
   itemId = 'item_id_1',
   listId = 'detection_list_id'
@@ -35,6 +39,23 @@ export const getImportExceptionsListItemSchemaMock = (
   type: 'simple',
 });
 
+/**  
+  This mock will hold the new properties of the Exception List item
+  so please keep it updated with the new ones and use it to test the 
+  new scenarios 
+*/
+export const getImportExceptionsListItemNewerVersionSchemaMock = (
+  itemId = 'item_id_1',
+  listId = 'detection_list_id'
+): ImportExceptionListItemSchema => ({
+  description: 'some description',
+  entries: ENTRIES,
+  expire_time: EXPIRE_TIME,
+  item_id: itemId,
+  list_id: listId,
+  name: 'Query with a rule id',
+  type: 'simple',
+});
 export const getImportExceptionsListSchemaDecodedMock = (
   listId = 'detection_list_id'
 ): ImportExceptionListSchemaDecoded => ({

x-pack/plugins/security_solution/cypress/e2e/exceptions/entry/flyout_validation.cy.ts

@@ -63,6 +63,7 @@ import {
 import { getExceptionList } from '../../../objects/exception';
 
 // Test Skipped until we fix the Flyout rerendering issue
+// https://github.com/elastic/kibana/issues/154994
 
 // NOTE: You might look at these tests and feel they're overkill,
 // but the exceptions flyout has a lot of logic making it difficult

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_export_rules.ts

@@ -8,8 +8,14 @@
 import expect from '@kbn/expect';
 import { EXCEPTION_LIST_ITEM_URL, EXCEPTION_LIST_URL } from '@kbn/securitysolution-list-constants';
 
-import { getCreateExceptionListItemMinimalSchemaMock } from '@kbn/lists-plugin/common/schemas/request/create_exception_list_item_schema.mock';
-import { getCreateExceptionListMinimalSchemaMock } from '@kbn/lists-plugin/common/schemas/request/create_exception_list_schema.mock';
+import {
+  getCreateExceptionListItemMinimalSchemaMock,
+  getCreateExceptionListItemNewerVersionSchemaMock,
+} from '@kbn/lists-plugin/common/schemas/request/create_exception_list_item_schema.mock';
+import {
+  getCreateExceptionListDetectionSchemaMock,
+  getCreateExceptionListMinimalSchemaMock,
+} from '@kbn/lists-plugin/common/schemas/request/create_exception_list_schema.mock';
 import { DETECTION_ENGINE_RULES_URL } from '@kbn/security-solution-plugin/common/constants';
 import { ROLES } from '@kbn/security-solution-plugin/common/test';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
@@ -46,81 +52,330 @@ export default ({ getService }: FtrProviderContext): void => {
       await deleteAllRules(supertest, log);
     });
 
-    it('should be able to reimport a rule referencing an exception list with existing comments', async () => {
-      // create an exception list
-      const { body: exceptionBody } = await supertestWithoutAuth
-        .post(EXCEPTION_LIST_URL)
-        .auth(ROLES.soc_manager, 'changeme')
-        .set('kbn-xsrf', 'true')
-        .send(getCreateExceptionListMinimalSchemaMock())
-        .expect(200);
+    describe('Endpoint Exception', () => {
+      /* 
+        After the 8.7 version, this test can be treated as testing exporting an old version of 
+        List Item because for ex the "expire_time" property is not part of the 
+        getCreateExceptionListMinimalSchemaMock and this is how we can differentiate between 
+        an Old version of a list item and a newer. The reason behind it is both List and Rule don't
+       keep the version so that we can use it to simulate migration cases
+    */
+      it('should be able to reimport a rule referencing an old version of endpoint exception list with existing comments', async () => {
+        // create an exception list
+        const { body: exceptionBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send(getCreateExceptionListMinimalSchemaMock())
+          .expect(200);
 
-      // create an exception list item
-      const { body: exceptionItemBody } = await supertestWithoutAuth
-        .post(EXCEPTION_LIST_ITEM_URL)
-        .auth(ROLES.soc_manager, 'changeme')
-        .set('kbn-xsrf', 'true')
-        .send({
-          ...getCreateExceptionListItemMinimalSchemaMock(),
-          comments: [{ comment: 'this exception item rocks' }],
-        })
-        .expect(200);
+        // create an exception list item
+        const { body: exceptionItemBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_ITEM_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send({
+            ...getCreateExceptionListItemMinimalSchemaMock(), // using Old version of Exception List
+            comments: [{ comment: 'this exception item rocks' }],
+          })
+          .expect(200);
 
-      const { body: exceptionItem } = await supertest
-        .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
-        .set('kbn-xsrf', 'true')
-        .expect(200);
+        const { body: exceptionItem } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
 
-      expect(exceptionItem.comments).to.eql([
-        {
-          comment: 'this exception item rocks',
-          created_at: `${exceptionItem.comments[0].created_at}`,
-          created_by: 'soc_manager',
-          id: `${exceptionItem.comments[0].id}`,
-        },
-      ]);
-
-      await createRule(supertest, log, {
-        ...getSimpleRule(),
-        exceptions_list: [
+        expect(exceptionItem.comments).to.eql([
           {
-            id: exceptionBody.id,
-            list_id: exceptionBody.list_id,
-            type: exceptionBody.type,
-            namespace_type: exceptionBody.namespace_type,
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItem.comments[0].created_at}`,
+            created_by: 'soc_manager',
+            id: `${exceptionItem.comments[0].id}`,
           },
-        ],
+        ]);
+
+        await createRule(supertest, log, {
+          ...getSimpleRule(),
+          exceptions_list: [
+            {
+              id: exceptionBody.id,
+              list_id: exceptionBody.list_id,
+              type: exceptionBody.type,
+              namespace_type: exceptionBody.namespace_type,
+            },
+          ],
+        });
+
+        const { body } = await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_export`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(200)
+          .parse(binaryToString);
+
+        await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_exceptions=true`)
+          .set('kbn-xsrf', 'true')
+          .attach('file', Buffer.from(body), 'rules.ndjson')
+          .expect(200);
+
+        const { body: exceptionItemFind2 } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // NOTE: Existing comment is uploaded successfully
+        // however, the meta now reflects who imported it,
+        // not who created the initial comment
+        expect(exceptionItemFind2.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItemFind2.comments[0].created_at}`,
+            created_by: 'elastic',
+            id: `${exceptionItemFind2.comments[0].id}`,
+          },
+        ]);
       });
 
-      const { body } = await supertest
-        .post(`${DETECTION_ENGINE_RULES_URL}/_export`)
-        .set('kbn-xsrf', 'true')
-        .send()
-        .expect(200)
-        .parse(binaryToString);
+      it('should be able to reimport a rule referencing a new version of endpoint exception list with existing comments', async () => {
+        // create an exception list
+        const { body: exceptionBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send(getCreateExceptionListMinimalSchemaMock())
+          .expect(200);
 
-      await supertest
-        .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_exceptions=true`)
-        .set('kbn-xsrf', 'true')
-        .attach('file', Buffer.from(body), 'rules.ndjson')
-        .expect(200);
+        // create an exception list item
+        const { body: exceptionItemBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_ITEM_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send({
+            ...getCreateExceptionListItemNewerVersionSchemaMock(), // using newer version of Exception List
+            comments: [{ comment: 'this exception item rocks' }],
+          })
+          .expect(200);
 
-      const { body: exceptionItemFind2 } = await supertest
-        .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
-        .set('kbn-xsrf', 'true')
-        .expect(200);
+        const { body: exceptionItem } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
 
-      // NOTE: Existing comment is uploaded successfully
-      // however, the meta now reflects who imported it,
-      // not who created the initial comment
-      expect(exceptionItemFind2.comments).to.eql([
-        {
-          comment: 'this exception item rocks',
-          created_at: `${exceptionItemFind2.comments[0].created_at}`,
-          created_by: 'elastic',
-          id: `${exceptionItemFind2.comments[0].id}`,
-        },
-      ]);
+        expect(exceptionItem.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItem.comments[0].created_at}`,
+            created_by: 'soc_manager',
+            id: `${exceptionItem.comments[0].id}`,
+          },
+        ]);
+
+        await createRule(supertest, log, {
+          ...getSimpleRule(),
+          exceptions_list: [
+            {
+              id: exceptionBody.id,
+              list_id: exceptionBody.list_id,
+              type: exceptionBody.type,
+              namespace_type: exceptionBody.namespace_type,
+            },
+          ],
+        });
+
+        const { body } = await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_export`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(200)
+          .parse(binaryToString);
+
+        await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_exceptions=true`)
+          .set('kbn-xsrf', 'true')
+          .attach('file', Buffer.from(body), 'rules.ndjson')
+          .expect(200);
+
+        const { body: exceptionItemFind2 } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // NOTE: Existing comment is uploaded successfully
+        // however, the meta now reflects who imported it,
+        // not who created the initial comment
+        expect(exceptionItemFind2.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItemFind2.comments[0].created_at}`,
+            created_by: 'elastic',
+            id: `${exceptionItemFind2.comments[0].id}`,
+          },
+        ]);
+      });
+    });
+
+    describe('Detection Exception', () => {
+      /* 
+        After the 8.7 version, this test can be treated as testing exporting an old version of 
+        List Item because for ex the "expire_time" property is not part of the 
+        getCreateExceptionListMinimalSchemaMock and this is how we can differentiate between 
+        an Old version of a list item and a newer. The reason behind it is both List and Rule don't
+       keep the version so that we can use it to simulate migration cases
+    */
+      it('should be able to reimport a rule referencing an old version of detection exception list with existing comments', async () => {
+        // create an exception list
+        const { body: exceptionBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send(getCreateExceptionListDetectionSchemaMock())
+          .expect(200);
+
+        // create an exception list item
+        const { body: exceptionItemBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_ITEM_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send({
+            ...getCreateExceptionListItemMinimalSchemaMock(), // using Old version of Exception List
+            comments: [{ comment: 'this exception item rocks' }],
+          })
+          .expect(200);
+
+        const { body: exceptionItem } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        expect(exceptionItem.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItem.comments[0].created_at}`,
+            created_by: 'soc_manager',
+            id: `${exceptionItem.comments[0].id}`,
+          },
+        ]);
+
+        await createRule(supertest, log, {
+          ...getSimpleRule(),
+          exceptions_list: [
+            {
+              id: exceptionBody.id,
+              list_id: exceptionBody.list_id,
+              type: exceptionBody.type,
+              namespace_type: exceptionBody.namespace_type,
+            },
+          ],
+        });
+
+        const { body } = await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_export`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(200)
+          .parse(binaryToString);
+
+        await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_exceptions=true`)
+          .set('kbn-xsrf', 'true')
+          .attach('file', Buffer.from(body), 'rules.ndjson')
+          .expect(200);
+
+        const { body: exceptionItemFind2 } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // NOTE: Existing comment is uploaded successfully
+        // however, the meta now reflects who imported it,
+        // not who created the initial comment
+        expect(exceptionItemFind2.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItemFind2.comments[0].created_at}`,
+            created_by: 'elastic',
+            id: `${exceptionItemFind2.comments[0].id}`,
+          },
+        ]);
+      });
+
+      it('should be able to reimport a rule referencing a new version of detection exception list with existing comments', async () => {
+        // create an exception list
+        const { body: exceptionBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send(getCreateExceptionListDetectionSchemaMock())
+          .expect(200);
+
+        // create an exception list item
+        const { body: exceptionItemBody } = await supertestWithoutAuth
+          .post(EXCEPTION_LIST_ITEM_URL)
+          .auth(ROLES.soc_manager, 'changeme')
+          .set('kbn-xsrf', 'true')
+          .send({
+            ...getCreateExceptionListItemNewerVersionSchemaMock(), // using newer version of Exception List
+            comments: [{ comment: 'this exception item rocks' }],
+          })
+          .expect(200);
+
+        const { body: exceptionItem } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        expect(exceptionItem.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItem.comments[0].created_at}`,
+            created_by: 'soc_manager',
+            id: `${exceptionItem.comments[0].id}`,
+          },
+        ]);
+
+        await createRule(supertest, log, {
+          ...getSimpleRule(),
+          exceptions_list: [
+            {
+              id: exceptionBody.id,
+              list_id: exceptionBody.list_id,
+              type: exceptionBody.type,
+              namespace_type: exceptionBody.namespace_type,
+            },
+          ],
+        });
+
+        const { body } = await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_export`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(200)
+          .parse(binaryToString);
+
+        await supertest
+          .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_exceptions=true`)
+          .set('kbn-xsrf', 'true')
+          .attach('file', Buffer.from(body), 'rules.ndjson')
+          .expect(200);
+
+        const { body: exceptionItemFind2 } = await supertest
+          .get(`${EXCEPTION_LIST_ITEM_URL}?item_id=${exceptionItemBody.item_id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // NOTE: Existing comment is uploaded successfully
+        // however, the meta now reflects who imported it,
+        // not who created the initial comment
+        expect(exceptionItemFind2.comments).to.eql([
+          {
+            comment: 'this exception item rocks',
+            created_at: `${exceptionItemFind2.comments[0].created_at}`,
+            created_by: 'elastic',
+            id: `${exceptionItemFind2.comments[0].id}`,
+          },
+        ]);
+      });
     });
   });
 };

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts

@@ -15,6 +15,7 @@ import {
   toNdJsonString,
   getImportExceptionsListItemSchemaMock,
   getImportExceptionsListSchemaMock,
+  getImportExceptionsListItemNewerVersionSchemaMock,
 } from '@kbn/lists-plugin/common/schemas/request/import_exceptions_schema.mock';
 import { ROLES } from '@kbn/security-solution-plugin/common/test';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
@@ -1421,6 +1422,53 @@ export default ({ getService }: FtrProviderContext): void => {
           await deleteAllExceptions(supertest, log);
         });
 
+        /* 
+         After the 8.7 version, this test can be treated as testing importing an old version of 
+         List Item as The "expire_time" property is not part of the getImportExceptionsListItemSchemaMock
+         and this is how we can differentiate between an Old version of a list item and a newer.
+         The reason behind it is both List and Rule don't keep the version so that we can use it to  
+         simulate migration cases
+        */
+        it('should be able to import a rule and an old version exception list, then delete it successfully', async () => {
+          const simpleRule = getSimpleRule('rule-1');
+
+          // import old exception version
+          const { body } = await supertest
+            .post(`${DETECTION_ENGINE_RULES_URL}/_import`)
+            .set('kbn-xsrf', 'true')
+            .attach(
+              'file',
+              Buffer.from(
+                toNdJsonString([
+                  simpleRule,
+                  getImportExceptionsListSchemaMock('test_list_id'),
+                  getImportExceptionsListItemSchemaMock('test_item_id', 'test_list_id'),
+                ])
+              ),
+              'rules.ndjson'
+            )
+            .expect(200);
+          expect(body).to.eql({
+            success: true,
+            success_count: 1,
+            rules_count: 1,
+            errors: [],
+            exceptions_errors: [],
+            exceptions_success: true,
+            exceptions_success_count: 1,
+            action_connectors_success: true,
+            action_connectors_success_count: 0,
+            action_connectors_errors: [],
+            action_connectors_warnings: [],
+          });
+
+          // delete the exception list item by its item_id
+          await supertest
+            .delete(`${EXCEPTION_LIST_ITEM_URL}?item_id=${'test_item_id'}`)
+            .set('kbn-xsrf', 'true')
+            .expect(200);
+        });
+
         it('should be able to import a rule and an exception list', async () => {
           const simpleRule = getSimpleRule('rule-1');
 
@@ -1433,7 +1481,7 @@ export default ({ getService }: FtrProviderContext): void => {
                 toNdJsonString([
                   simpleRule,
                   getImportExceptionsListSchemaMock('test_list_id'),
-                  getImportExceptionsListItemSchemaMock('test_item_id', 'test_list_id'),
+                  getImportExceptionsListItemNewerVersionSchemaMock('test_item_id', 'test_list_id'),
                 ])
               ),
               'rules.ndjson'