From c54e6a9063a3f3a237506e0275408a856c1df39e Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 16 Oct 2024 09:17:31 +1100 Subject: [PATCH] [8.x] [Security Solution][Notes] - fix incorrect get_notes api for documentIds and savedObjectIds query parameters and adding api integration tests (#196225) (#196394) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Notes] - fix incorrect get_notes api for documentIds and savedObjectIds query parameters and adding api integration tests (#196225)](https://github.com/elastic/kibana/pull/196225) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Philippe Oberti --- .../lib/timeline/routes/notes/get_notes.ts | 104 +++--- .../trial_license_complete_tier/helpers.ts | 40 +- .../trial_license_complete_tier/notes.ts | 343 +++++++++++++++++- 3 files changed, 440 insertions(+), 47 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/timeline/routes/notes/get_notes.ts b/x-pack/plugins/security_solution/server/lib/timeline/routes/notes/get_notes.ts index 0f3440d8ed13..925379baedad 100644 --- a/x-pack/plugins/security_solution/server/lib/timeline/routes/notes/get_notes.ts +++ b/x-pack/plugins/security_solution/server/lib/timeline/routes/notes/get_notes.ts @@ -9,6 +9,8 @@ import type { IKibanaResponse } from '@kbn/core-http-server'; import { transformError } from '@kbn/securitysolution-es-utils'; import type { SortOrder } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { buildRouteValidationWithZod } from '@kbn/zod-helpers'; +import type { SavedObjectsFindOptions } from '@kbn/core-saved-objects-api-server'; +import { nodeBuilder } from '@kbn/es-query'; import { timelineSavedObjectType } from '../../saved_object_mappings'; import type { SecuritySolutionPluginRouter } from '../../../../types'; import { MAX_UNASSOCIATED_NOTES, NOTE_URL } from '../../../../../common/constants'; @@ -43,78 +45,90 @@ export const getNotesRoute = (router: SecuritySolutionPluginRouter) => { uiSettings: { client: uiSettingsClient }, } = await frameworkRequest.context.core; const maxUnassociatedNotes = await uiSettingsClient.get(MAX_UNASSOCIATED_NOTES); + + // if documentIds is provided, we will search for all the notes associated with the documentIds const documentIds = queryParams.documentIds ?? null; - const savedObjectIds = queryParams.savedObjectIds ?? null; if (documentIds != null) { + // search for multiple document ids (like retrieving all the notes for all the alerts within a table) if (Array.isArray(documentIds)) { - const docIdSearchString = documentIds?.join(' | '); - const options = { + const options: SavedObjectsFindOptions = { type: noteSavedObjectType, - search: docIdSearchString, + filter: nodeBuilder.or( + documentIds.map((documentId: string) => + nodeBuilder.is(`${noteSavedObjectType}.attributes.eventId`, documentId) + ) + ), page: 1, perPage: maxUnassociatedNotes, }; const res = await getAllSavedNote(frameworkRequest, options); const body: GetNotesResponse = res ?? {}; return response.ok({ body }); - } else { - const options = { - type: noteSavedObjectType, - search: documentIds, - page: 1, - perPage: maxUnassociatedNotes, - }; - const res = await getAllSavedNote(frameworkRequest, options); - return response.ok({ body: res ?? {} }); } - } else if (savedObjectIds != null) { + + // searching for all the notes associated with a specific document id + const options: SavedObjectsFindOptions = { + type: noteSavedObjectType, + filter: nodeBuilder.is(`${noteSavedObjectType}.attributes.eventId`, documentIds), + page: 1, + perPage: maxUnassociatedNotes, + }; + const res = await getAllSavedNote(frameworkRequest, options); + return response.ok({ body: res ?? {} }); + } + + // if savedObjectIds is provided, we will search for all the notes associated with the savedObjectIds + const savedObjectIds = queryParams.savedObjectIds ?? null; + if (savedObjectIds != null) { + // search for multiple saved object ids if (Array.isArray(savedObjectIds)) { - const soIdSearchString = savedObjectIds?.join(' | '); - const options = { + const options: SavedObjectsFindOptions = { type: noteSavedObjectType, - hasReference: { + hasReference: savedObjectIds.map((savedObjectId: string) => ({ type: timelineSavedObjectType, - id: soIdSearchString, - }, + id: savedObjectId, + })), page: 1, perPage: maxUnassociatedNotes, }; const res = await getAllSavedNote(frameworkRequest, options); const body: GetNotesResponse = res ?? {}; return response.ok({ body }); - } else { - const options = { - type: noteSavedObjectType, - hasReference: { - type: timelineSavedObjectType, - id: savedObjectIds, - }, - perPage: maxUnassociatedNotes, - }; - const res = await getAllSavedNote(frameworkRequest, options); - const body: GetNotesResponse = res ?? {}; - return response.ok({ body }); } - } else { - const perPage = queryParams?.perPage ? parseInt(queryParams.perPage, 10) : 10; - const page = queryParams?.page ? parseInt(queryParams.page, 10) : 1; - const search = queryParams?.search ?? undefined; - const sortField = queryParams?.sortField ?? undefined; - const sortOrder = (queryParams?.sortOrder as SortOrder) ?? undefined; - const filter = queryParams?.filter; - const options = { + + // searching for all the notes associated with a specific saved object id + const options: SavedObjectsFindOptions = { type: noteSavedObjectType, - perPage, - page, - search, - sortField, - sortOrder, - filter, + hasReference: { + type: timelineSavedObjectType, + id: savedObjectIds, + }, + perPage: maxUnassociatedNotes, }; const res = await getAllSavedNote(frameworkRequest, options); const body: GetNotesResponse = res ?? {}; return response.ok({ body }); } + + // retrieving all the notes following the query parameters + const perPage = queryParams?.perPage ? parseInt(queryParams.perPage, 10) : 10; + const page = queryParams?.page ? parseInt(queryParams.page, 10) : 1; + const search = queryParams?.search ?? undefined; + const sortField = queryParams?.sortField ?? undefined; + const sortOrder = (queryParams?.sortOrder as SortOrder) ?? undefined; + const filter = queryParams?.filter; + const options: SavedObjectsFindOptions = { + type: noteSavedObjectType, + perPage, + page, + search, + sortField, + sortOrder, + filter, + }; + const res = await getAllSavedNote(frameworkRequest, options); + const body: GetNotesResponse = res ?? {}; + return response.ok({ body }); } catch (err) { const error = transformError(err); const siemResponse = buildSiemResponse(response); diff --git a/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/helpers.ts b/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/helpers.ts index 9f40373976c2..a5944dc8c614 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/helpers.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/helpers.ts @@ -7,7 +7,11 @@ import type SuperTest from 'supertest'; import { v4 as uuidv4 } from 'uuid'; -import { TimelineTypeEnum } from '@kbn/security-solution-plugin/common/api/timeline'; +import { BareNote, TimelineTypeEnum } from '@kbn/security-solution-plugin/common/api/timeline'; +import { NOTE_URL } from '@kbn/security-solution-plugin/common/constants'; +import type { Client } from '@elastic/elasticsearch'; +import { SECURITY_SOLUTION_SAVED_OBJECT_INDEX } from '@kbn/core-saved-objects-server'; +import { noteSavedObjectType } from '@kbn/security-solution-plugin/server/lib/timeline/saved_object_mappings'; export const createBasicTimeline = async (supertest: SuperTest.Agent, titleToSaved: string) => await supertest @@ -38,3 +42,37 @@ export const createBasicTimelineTemplate = async ( timelineType: TimelineTypeEnum.template, }, }); + +export const deleteAllNotes = async (es: Client): Promise => { + await es.deleteByQuery({ + index: SECURITY_SOLUTION_SAVED_OBJECT_INDEX, + q: `type:${noteSavedObjectType}`, + wait_for_completion: true, + refresh: true, + body: {}, + }); +}; + +export const createNote = async ( + supertest: SuperTest.Agent, + note: { + documentId?: string; + savedObjectId?: string; + user?: string; + text: string; + } +) => + await supertest + .patch(NOTE_URL) + .set('kbn-xsrf', 'true') + .send({ + note: { + eventId: note.documentId || '', + timelineId: note.savedObjectId || '', + created: Date.now(), + createdBy: note.user || 'elastic', + updated: Date.now(), + updatedBy: note.user || 'elastic', + note: note.text, + } as BareNote, + }); diff --git a/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/notes.ts b/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/notes.ts index 666e36325fd7..dabb453f8015 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/notes.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/investigation/saved_objects/trial_license_complete_tier/notes.ts @@ -6,7 +6,9 @@ */ import expect from '@kbn/expect'; - +import { v4 as uuidv4 } from 'uuid'; +import { Note } from '@kbn/security-solution-plugin/common/api/timeline'; +import { createNote, deleteAllNotes } from './helpers'; import { FtrProviderContext } from '../../../../../api_integration/ftr_provider_context'; export default function ({ getService }: FtrProviderContext) { @@ -14,6 +16,8 @@ export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); describe('Note - Saved Objects', () => { + const es = getService('es'); + before(() => kibanaServer.savedObjects.cleanStandardList()); after(() => kibanaServer.savedObjects.cleanStandardList()); @@ -70,5 +74,342 @@ export default function ({ getService }: FtrProviderContext) { expect(responseToTest.body.data!.persistNote.note.version).to.not.be.eql(version); }); }); + + describe('get notes', () => { + beforeEach(async () => { + await deleteAllNotes(es); + }); + + const eventId1 = uuidv4(); + const eventId2 = uuidv4(); + const eventId3 = uuidv4(); + const timelineId1 = uuidv4(); + const timelineId2 = uuidv4(); + const timelineId3 = uuidv4(); + + it('should retrieve all the notes for a document id', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { documentId: eventId2, text: 'associated with event-2 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + savedObjectId: timelineId2, + text: 'associated with timeline-2 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { + documentId: eventId2, + savedObjectId: timelineId2, + text: 'associated with event-2 and timeline-2', + }), + createNote(supertest, { text: 'associated with nothing' }), + createNote(supertest, { + text: `associated with nothing but has ${eventId1} in the text`, + }), + ]); + + const response = await supertest + .get(`/api/note?documentIds=${eventId1}`) + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(2); + notes.forEach((note: Note) => expect(note.eventId).to.be(eventId1)); + }); + + it('should retrieve all the notes for multiple document ids', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { documentId: eventId2, text: 'associated with event-2 only' }), + createNote(supertest, { documentId: eventId3, text: 'associated with event-3 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + savedObjectId: timelineId2, + text: 'associated with timeline-2 only', + }), + createNote(supertest, { + savedObjectId: timelineId3, + text: 'associated with timeline-3 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { + documentId: eventId2, + savedObjectId: timelineId2, + text: 'associated with event-2 and timeline-2', + }), + createNote(supertest, { + documentId: eventId3, + savedObjectId: timelineId3, + text: 'associated with event-3 and timeline-3', + }), + createNote(supertest, { text: 'associated with nothing' }), + createNote(supertest, { + text: `associated with nothing but has ${eventId1} in the text`, + }), + createNote(supertest, { + text: `associated with nothing but has ${eventId2} in the text`, + }), + createNote(supertest, { + text: `associated with nothing but has ${eventId3} in the text`, + }), + ]); + + const response = await supertest + .get(`/api/note?documentIds=${eventId1}&documentIds=${eventId2}`) + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(4); + notes.forEach((note: Note) => { + expect(note.eventId).to.not.be(eventId3); + expect(note.eventId).to.not.be(''); + }); + }); + + it('should retrieve all the notes for a saved object id', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { documentId: eventId2, text: 'associated with event-2 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + savedObjectId: timelineId2, + text: 'associated with timeline-2 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { + documentId: eventId2, + savedObjectId: timelineId2, + text: 'associated with event-2 and timeline-2', + }), + createNote(supertest, { text: 'associated with nothing' }), + createNote(supertest, { + text: `associated with nothing but has ${timelineId1} in the text`, + }), + ]); + + const response = await supertest + .get(`/api/note?savedObjectIds=${timelineId1}`) + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(2); + notes.forEach((note: Note) => expect(note.timelineId).to.be(timelineId1)); + }); + + it('should retrieve all the notes for multiple saved object ids', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { documentId: eventId2, text: 'associated with event-2 only' }), + createNote(supertest, { documentId: eventId3, text: 'associated with event-3 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + savedObjectId: timelineId2, + text: 'associated with timeline-2 only', + }), + createNote(supertest, { + savedObjectId: timelineId3, + text: 'associated with timeline-3 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { + documentId: eventId2, + savedObjectId: timelineId2, + text: 'associated with event-2 and timeline-2', + }), + createNote(supertest, { + documentId: eventId3, + savedObjectId: timelineId3, + text: 'associated with event-3 and timeline-3', + }), + createNote(supertest, { text: 'associated with nothing' }), + createNote(supertest, { + text: `associated with nothing but has ${timelineId1} in the text`, + }), + createNote(supertest, { + text: `associated with nothing but has ${timelineId2} in the text`, + }), + createNote(supertest, { + text: `associated with nothing but has ${timelineId3} in the text`, + }), + ]); + + const response = await supertest + .get(`/api/note?savedObjectIds=${timelineId1}&savedObjectIds=${timelineId2}`) + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(4); + notes.forEach((note: Note) => { + expect(note.timelineId).to.not.be(timelineId3); + expect(note.timelineId).to.not.be(''); + }); + }); + + it('should retrieve all notes without any query params', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { text: 'associated with nothing' }), + ]); + + const response = await supertest + .get('/api/note') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount } = response.body; + + expect(totalCount).to.be(4); + }); + + it('should retrieve notes considering perPage query parameter', async () => { + await Promise.all([ + createNote(supertest, { text: 'first note' }), + createNote(supertest, { text: 'second note' }), + createNote(supertest, { text: 'third note' }), + ]); + + const response = await supertest + .get('/api/note?perPage=1') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(3); + expect(notes.length).to.be(1); + }); + + it('should retrieve considering page query parameter', async () => { + await createNote(supertest, { text: 'first note' }); + await createNote(supertest, { text: 'second note' }); + await createNote(supertest, { text: 'third note' }); + + const response = await supertest + .get('/api/note?perPage=1&page=2') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(3); + expect(notes.length).to.be(1); + expect(notes[0].note).to.be('second note'); + }); + + it('should retrieve considering search query parameter', async () => { + await Promise.all([ + createNote(supertest, { documentId: eventId1, text: 'associated with event-1 only' }), + createNote(supertest, { + savedObjectId: timelineId1, + text: 'associated with timeline-1 only', + }), + createNote(supertest, { + documentId: eventId1, + savedObjectId: timelineId1, + text: 'associated with event-1 and timeline-1', + }), + createNote(supertest, { text: 'associated with nothing' }), + ]); + + const response = await supertest + .get('/api/note?search=event') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount } = response.body; + + expect(totalCount).to.be(2); + }); + + // TODO why can't we sort on every field? (I tested for the note field (or a random field like abc) and the endpoint crashes) + it('should retrieve considering sortField query parameters', async () => { + await Promise.all([ + createNote(supertest, { documentId: '1', text: 'note 1' }), + createNote(supertest, { documentId: '2', text: 'note 2' }), + createNote(supertest, { documentId: '3', text: 'note 3' }), + ]); + + const response = await supertest + .get('/api/note?sortField=eventId') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(3); + expect(notes[0].eventId).to.be('1'); + expect(notes[1].eventId).to.be('2'); + expect(notes[2].eventId).to.be('3'); + }); + + it('should retrieve considering sortOrder query parameters', async () => { + await Promise.all([ + createNote(supertest, { documentId: '1', text: 'note 1' }), + createNote(supertest, { documentId: '2', text: 'note 2' }), + createNote(supertest, { documentId: '3', text: 'note 3' }), + ]); + + const response = await supertest + .get('/api/note?sortField=eventId&sortOrder=desc') + .set('kbn-xsrf', 'true') + .set('elastic-api-version', '2023-10-31'); + + const { totalCount, notes } = response.body; + + expect(totalCount).to.be(3); + expect(notes[0].eventId).to.be('3'); + expect(notes[1].eventId).to.be('2'); + expect(notes[2].eventId).to.be('1'); + }); + + // TODO should add more tests for the filter query parameter (I don't know how it's supposed to work) + + // TODO should add more tests for the MAX_UNASSOCIATED_NOTES advanced settings values + }); }); }