[7.x] [Security Solution] Keep original note creator (#74203)

x-pack/plugins/security_solution/server/graphql/note/resolvers.ts

@@ -81,10 +81,16 @@ export const createNoteResolvers = (
  return true;
  },
  async persistNote(root, args, { req }) {
- return libs.note.persistNote(req, args.noteId || null, args.version || null, {
- ...args.note,
- timelineId: args.note.timelineId || null,
- });
+ return libs.note.persistNote(
+ req,
+ args.noteId || null,
+ args.version || null,
+ {
+ ...args.note,
+ timelineId: args.note.timelineId || null,
+ },
+ true
+ );
  },
  },
 });

x-pack/plugins/security_solution/server/lib/note/saved_object.ts

@@ -49,7 +49,8 @@ export interface Note {
  request: FrameworkRequest,
  noteId: string | null,
  version: string | null,
- note: SavedNote
+ note: SavedNote,
+ overrideOwner: boolean
  ) => Promise<ResponseNote>;
  convertSavedObjectToSavedNote: (
  savedObject: unknown,
@@ -136,7 +137,8 @@ export const persistNote = async (
  request: FrameworkRequest,
  noteId: string | null,
  version: string | null,
- note: SavedNote
+ note: SavedNote,
+ overrideOwner: boolean = true
 ): Promise<ResponseNote> => {
  try {
  const savedObjectsClient = request.context.core.savedObjects.client;
@@ -163,14 +165,14 @@ export const persistNote = async (
  note: convertSavedObjectToSavedNote(
  await savedObjectsClient.create(
  noteSavedObjectType,
- pickSavedNote(noteId, note, request.user)
+ overrideOwner ? pickSavedNote(noteId, note, request.user) : note
  ),
  timelineVersionSavedObject != null ? timelineVersionSavedObject : undefined
  ),
  };
  }
 
- // Update new note
+ // Update existing note
 
  const existingNote = await getSavedNote(request, noteId);
  return {
@@ -180,7 +182,7 @@ export const persistNote = async (
  await savedObjectsClient.update(
  noteSavedObjectType,
  noteId,
- pickSavedNote(noteId, note, request.user),
+ overrideOwner ? pickSavedNote(noteId, note, request.user) : note,
  {
  version: existingNote.version || undefined,
  }

x-pack/plugins/security_solution/server/lib/timeline/routes/create_timelines_route.ts

@@ -20,6 +20,7 @@ import {
  TimelineStatusActions,
 } from './utils/common';
 import { createTimelines } from './utils/create_timelines';
+import { DEFAULT_ERROR } from './utils/failure_cases';
 
 export const createTimelinesRoute = (
  router: IRouter,
@@ -85,7 +86,7 @@ export const createTimelinesRoute = (
  return siemResponse.error(
  compareTimelinesStatus.checkIsFailureCases(TimelineStatusActions.create) || {
  statusCode: 405,
- body: 'update timeline error',
+ body: DEFAULT_ERROR,
  }
  );
  }

x-pack/plugins/security_solution/server/lib/timeline/routes/import_timelines_route.test.ts

@@ -46,6 +46,7 @@ describe('import timelines', () => {
  let mockPersistTimeline: jest.Mock;
  let mockPersistPinnedEventOnTimeline: jest.Mock;
  let mockPersistNote: jest.Mock;
+ let mockGetNote: jest.Mock;
  let mockGetTupleDuplicateErrorsAndUniqueTimeline: jest.Mock;
 
  beforeEach(() => {
@@ -69,6 +70,7 @@ describe('import timelines', () => {
  mockPersistTimeline = jest.fn();
  mockPersistPinnedEventOnTimeline = jest.fn();
  mockPersistNote = jest.fn();
+ mockGetNote = jest.fn();
  mockGetTupleDuplicateErrorsAndUniqueTimeline = jest.fn();
 
  jest.doMock('../create_timelines_stream_from_ndjson', () => {
@@ -113,6 +115,37 @@ describe('import timelines', () => {
  jest.doMock('../../note/saved_object', () => {
  return {
  persistNote: mockPersistNote,
+ getNote: mockGetNote
+ .mockResolvedValueOnce({
+ noteId: 'd2649d40-6bc5-11ea-86f0-5db0048c6086',
+ version: 'WzExNjQsMV0=',
+ eventId: undefined,
+ note: 'original note',
+ created: '1584830796960',
+ createdBy: 'original author A',
+ updated: '1584830796960',
+ updatedBy: 'original author A',
+ })
+ .mockResolvedValueOnce({
+ noteId: '73ac2370-6bc2-11ea-a90b-f5341fb7a189',
+ version: 'WzExMjgsMV0=',
+ eventId: 'ZaAi8nAB5OldxqFfdhke',
+ note: 'original event note',
+ created: '1584830796960',
+ createdBy: 'original author B',
+ updated: '1584830796960',
+ updatedBy: 'original author B',
+ })
+ .mockResolvedValue({
+ noteId: 'f7b71620-6bc2-11ea-a0b6-33c7b2a78885',
+ version: 'WzExMzUsMV0=',
+ eventId: 'ZaAi8nAB5OldxqFfdhke',
+ note: 'event note2',
+ created: '1584830796960',
+ createdBy: 'angela',
+ updated: '1584830796960',
+ updatedBy: 'angela',
+ }),
  };
  });
 
@@ -213,6 +246,14 @@ describe('import timelines', () => {
  );
  });
 
+ test('should Check if note exists', async () => {
+ const mockRequest = getImportTimelinesRequest();
+ await server.inject(mockRequest, context);
+ expect(mockGetNote.mock.calls[0][1]).toEqual(
+ mockUniqueParsedObjects[0].globalNotes[0].noteId
+ );
+ });
+
  test('should Create notes', async () => {
  const mockRequest = getImportTimelinesRequest();
  await server.inject(mockRequest, context);
@@ -237,20 +278,67 @@ describe('import timelines', () => {
  expect(mockPersistNote.mock.calls[0][2]).toEqual(mockCreatedTimeline.version);
  });
 
- test('should provide new notes when Creating notes for a timeline', async () => {
+ test('should provide new notes with original author info when Creating notes for a timeline', async () => {
+ const mockRequest = getImportTimelinesRequest();
+ await server.inject(mockRequest, context);
+ expect(mockPersistNote.mock.calls[0][3]).toEqual({
+ eventId: undefined,
+ note: 'original note',
+ created: '1584830796960',
+ createdBy: 'original author A',
+ updated: '1584830796960',
+ updatedBy: 'original author A',
+ timelineId: mockCreatedTimeline.savedObjectId,
+ });
+ expect(mockPersistNote.mock.calls[1][3]).toEqual({
+ eventId: mockUniqueParsedObjects[0].eventNotes[0].eventId,
+ note: 'original event note',
+ created: '1584830796960',
+ createdBy: 'original author B',
+ updated: '1584830796960',
+ updatedBy: 'original author B',
+ timelineId: mockCreatedTimeline.savedObjectId,
+ });
+ expect(mockPersistNote.mock.calls[2][3]).toEqual({
+ eventId: mockUniqueParsedObjects[0].eventNotes[1].eventId,
+ note: 'event note2',
+ created: '1584830796960',
+ createdBy: 'angela',
+ updated: '1584830796960',
+ updatedBy: 'angela',
+ timelineId: mockCreatedTimeline.savedObjectId,
+ });
+ });
+
+ test('should keep current author if note does not exist when Creating notes for a timeline', async () => {
+ mockGetNote.mockReset();
+ mockGetNote.mockRejectedValue(new Error());
+
  const mockRequest = getImportTimelinesRequest();
  await server.inject(mockRequest, context);
  expect(mockPersistNote.mock.calls[0][3]).toEqual({
+ created: mockUniqueParsedObjects[0].globalNotes[0].created,
+ createdBy: mockUniqueParsedObjects[0].globalNotes[0].createdBy,
+ updated: mockUniqueParsedObjects[0].globalNotes[0].updated,
+ updatedBy: mockUniqueParsedObjects[0].globalNotes[0].updatedBy,
  eventId: undefined,
  note: mockUniqueParsedObjects[0].globalNotes[0].note,
  timelineId: mockCreatedTimeline.savedObjectId,
  });
  expect(mockPersistNote.mock.calls[1][3]).toEqual({
+ created: mockUniqueParsedObjects[0].eventNotes[0].created,
+ createdBy: mockUniqueParsedObjects[0].eventNotes[0].createdBy,
+ updated: mockUniqueParsedObjects[0].eventNotes[0].updated,
+ updatedBy: mockUniqueParsedObjects[0].eventNotes[0].updatedBy,
  eventId: mockUniqueParsedObjects[0].eventNotes[0].eventId,
  note: mockUniqueParsedObjects[0].eventNotes[0].note,
  timelineId: mockCreatedTimeline.savedObjectId,
  });
  expect(mockPersistNote.mock.calls[2][3]).toEqual({
+ created: mockUniqueParsedObjects[0].eventNotes[1].created,
+ createdBy: mockUniqueParsedObjects[0].eventNotes[1].createdBy,
+ updated: mockUniqueParsedObjects[0].eventNotes[1].updated,
+ updatedBy: mockUniqueParsedObjects[0].eventNotes[1].updatedBy,
  eventId: mockUniqueParsedObjects[0].eventNotes[1].eventId,
  note: mockUniqueParsedObjects[0].eventNotes[1].note,
  timelineId: mockCreatedTimeline.savedObjectId,
@@ -573,6 +661,10 @@ describe('import timeline templates', () => {
  expect(mockPersistNote.mock.calls[0][3]).toEqual({
  eventId: undefined,
  note: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].note,
+ created: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].created,
+ createdBy: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].createdBy,
+ updated: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].updated,
+ updatedBy: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].updatedBy,
  timelineId: mockCreatedTemplateTimeline.savedObjectId,
  });
  });
@@ -721,6 +813,10 @@ describe('import timeline templates', () => {
  expect(mockPersistNote.mock.calls[0][3]).toEqual({
  eventId: undefined,
  note: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].note,
+ created: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].created,
+ createdBy: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].createdBy,
+ updated: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].updated,
+ updatedBy: mockUniqueParsedTemplateTimelineObjects[0].globalNotes[0].updatedBy,
  timelineId: mockCreatedTemplateTimeline.savedObjectId,
  });
  });

x-pack/plugins/security_solution/server/lib/timeline/routes/utils/create_timelines.ts

@@ -45,26 +45,56 @@ export const savePinnedEvents = (
  )
  );
 
-export const saveNotes = (
+const getNewNote = async (
  frameworkRequest: FrameworkRequest,
+ note: NoteResult,
  timelineSavedObjectId: string,
- timelineVersion?: string | null,
- existingNoteIds?: string[],
- newNotes?: NoteResult[]
-) => {
- return Promise.all(
- newNotes?.map((note) => {
- const newNote: SavedNote = {
+ overrideOwner: boolean
+): Promise<SavedNote> => {
+ let savedNote = note;
+ try {
+ savedNote = await noteLib.getNote(frameworkRequest, note.noteId);
+ // eslint-disable-next-line no-empty
+ } catch (e) {}
+ return overrideOwner
+ ? {
  eventId: note.eventId,
  note: note.note,
  timelineId: timelineSavedObjectId,
+ }
+ : {
+ eventId: savedNote.eventId,
+ note: savedNote.note,
+ created: savedNote.created,
+ createdBy: savedNote.createdBy,
+ updated: savedNote.updated,
+ updatedBy: savedNote.updatedBy,
+ timelineId: timelineSavedObjectId,
  };
+};
 
+export const saveNotes = async (
+ frameworkRequest: FrameworkRequest,
+ timelineSavedObjectId: string,
+ timelineVersion?: string | null,
+ existingNoteIds?: string[],
+ newNotes?: NoteResult[],
+ overrideOwner: boolean = true
+) => {
+ return Promise.all(
+ newNotes?.map(async (note) => {
+ const newNote = await getNewNote(
+ frameworkRequest,
+ note,
+ timelineSavedObjectId,
+ overrideOwner
+ );
  return noteLib.persistNote(
  frameworkRequest,
- existingNoteIds?.find((nId) => nId === note.noteId) ?? null,
+ overrideOwner ? existingNoteIds?.find((nId) => nId === note.noteId) ?? null : null,
  timelineVersion ?? null,
- newNote
+ newNote,
+ overrideOwner
  );
  }) ?? []
  );
@@ -75,12 +105,18 @@ interface CreateTimelineProps {
  timeline: SavedTimeline;
  timelineSavedObjectId?: string | null;
  timelineVersion?: string | null;
+ overrideNotesOwner?: boolean;
  pinnedEventIds?: string[] | null;
  notes?: NoteResult[];
  existingNoteIds?: string[];
  isImmutable?: boolean;
 }
 
+/** allow overrideNotesOwner means overriding by current username,
+ * disallow overrideNotesOwner means keep the original username.
+ * overrideNotesOwner = false only happens when import timeline templates,
+ * as we want to keep the original creator for notes
+ **/
 export const createTimelines = async ({
  frameworkRequest,
  timeline,
@@ -90,6 +126,7 @@ export const createTimelines = async ({
  notes = [],
  existingNoteIds = [],
  isImmutable,
+ overrideNotesOwner = true,
 }: CreateTimelineProps): Promise<ResponseTimeline> => {
  const responseTimeline = await saveTimelines(
  frameworkRequest,
@@ -119,7 +156,8 @@ export const createTimelines = async ({
  timelineSavedObjectId ?? newTimelineSavedObjectId,
  newTimelineVersion,
  existingNoteIds,
- notes
+ notes,
+ overrideNotesOwner
  ),
  ];
  }

x-pack/plugins/security_solution/server/lib/timeline/routes/utils/failure_cases.ts

@@ -37,6 +37,7 @@ export const NOT_ALLOW_UPDATE_TIMELINE_TYPE_ERROR_MESSAGE =
  'You cannot convert a Timeline template to a timeline, or a timeline to a Timeline template.';
 export const UPDAT_TIMELINE_VIA_IMPORT_NOT_ALLOWED_ERROR_MESSAGE =
  'You cannot update a timeline via imports. Use the UI to modify existing timelines.';
+export const DEFAULT_ERROR = `Something has gone wrong. We didn't handle something properly. To help us fix this, please upload your file to https://discuss.elastic.co/c/security/siem.`;
 
 const isUpdatingStatus = (
  isHandlingTemplateTimeline: boolean,