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

* keep original note creator

* unit test

* fix types

* typo

* wording
This commit is contained in:
Angela Chuang 2020-08-04 20:35:00 +01:00 committed by GitHub
parent ea4d36f713
commit 8e0e228dbf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 172 additions and 26 deletions

View file

@ -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
);
},
},
});

View file

@ -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,
}

View file

@ -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,
}
);
}

View file

@ -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,
});
});

View file

@ -45,26 +45,56 @@ export const savePinnedEvents = (
)
);
export const saveNotes = (
const getNewNote = async (
frameworkRequest: FrameworkRequest,
note: NoteResult,
timelineSavedObjectId: string,
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[]
newNotes?: NoteResult[],
overrideOwner: boolean = true
) => {
return Promise.all(
newNotes?.map((note) => {
const newNote: SavedNote = {
eventId: note.eventId,
note: note.note,
timelineId: timelineSavedObjectId,
};
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
),
];
}

View file

@ -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,

View file

@ -26,6 +26,7 @@ import { createPromiseFromStreams } from '../../../../../../../../src/legacy/uti
import { getTupleDuplicateErrorsAndUniqueTimeline } from './get_timelines_from_stream';
import { CompareTimelinesStatus } from './compare_timelines_status';
import { TimelineStatusActions } from './common';
import { DEFAULT_ERROR } from './failure_cases';
export type ImportedTimeline = SavedTimeline & {
savedObjectId: string | null;
@ -96,7 +97,6 @@ export const setTimeline = (
};
const CHUNK_PARSED_OBJECT_SIZE = 10;
const DEFAULT_IMPORT_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.`;
export const importTimelines = async (
file: Readable,
@ -173,6 +173,7 @@ export const importTimelines = async (
pinnedEventIds: isTemplateTimeline ? null : pinnedEventIds,
notes: isTemplateTimeline ? globalNotes : [...globalNotes, ...eventNotes],
isImmutable,
overrideNotesOwner: false,
});
resolve({
@ -186,7 +187,7 @@ export const importTimelines = async (
const errorMessage = compareTimelinesStatus.checkIsFailureCases(
TimelineStatusActions.createViaImport
);
const message = errorMessage?.body ?? DEFAULT_IMPORT_ERROR;
const message = errorMessage?.body ?? DEFAULT_ERROR;
resolve(
createBulkErrorObject({
@ -206,6 +207,7 @@ export const importTimelines = async (
notes: globalNotes,
existingNoteIds: compareTimelinesStatus.timelineInput.data?.noteIds,
isImmutable,
overrideNotesOwner: false,
});
resolve({
@ -218,7 +220,7 @@ export const importTimelines = async (
TimelineStatusActions.updateViaImport
);
const message = errorMessage?.body ?? DEFAULT_IMPORT_ERROR;
const message = errorMessage?.body ?? DEFAULT_ERROR;
resolve(
createBulkErrorObject({