From da77d5f3b5bb837013ece72065f4161ce97df4ac Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Thu, 6 Feb 2020 19:58:01 -0500 Subject: [PATCH 1/5] update annotation routes to use NP router --- .../models/annotation_service/annotation.ts | 10 +- .../server/models/annotation_service/index.ts | 7 +- .../server/new_platform/annotations_schema.ts | 21 +++ .../plugins/ml/server/routes/annotations.js | 78 --------- .../plugins/ml/server/routes/annotations.ts | 152 ++++++++++++++++++ .../plugins/ml/server/routes/apidoc.json | 6 +- 6 files changed, 188 insertions(+), 86 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts delete mode 100644 x-pack/legacy/plugins/ml/server/routes/annotations.js create mode 100644 x-pack/legacy/plugins/ml/server/routes/annotations.ts diff --git a/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.ts b/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.ts index addcdcb376b93..399305ea2603e 100644 --- a/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.ts +++ b/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.ts @@ -6,6 +6,7 @@ import Boom from 'boom'; import _ from 'lodash'; +import { RequestHandlerContext } from 'src/core/server'; import { ANNOTATION_TYPE } from '../../../common/constants/annotations'; import { @@ -67,7 +68,8 @@ export type callWithRequestType = ( params: annotationProviderParams ) => Promise; -export function annotationProvider(callWithRequest: callWithRequestType) { +export function annotationProvider(context: RequestHandlerContext) { + const callAsCurrentUser = context.ml!.mlClient.callAsCurrentUser; async function indexAnnotation(annotation: Annotation, username: string) { if (isAnnotation(annotation) === false) { // No need to translate, this will not be exposed in the UI. @@ -94,7 +96,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) { delete params.body.key; } - return await callWithRequest('index', params); + return await callAsCurrentUser('index', params); } async function getAnnotations({ @@ -213,7 +215,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) { }; try { - const resp = await callWithRequest('search', params); + const resp = await callAsCurrentUser('search', params); if (resp.error !== undefined && resp.message !== undefined) { // No need to translate, this will not be exposed in the UI. @@ -252,7 +254,7 @@ export function annotationProvider(callWithRequest: callWithRequestType) { refresh: 'wait_for', }; - return await callWithRequest('delete', param); + return await callAsCurrentUser('delete', param); } return { diff --git a/x-pack/legacy/plugins/ml/server/models/annotation_service/index.ts b/x-pack/legacy/plugins/ml/server/models/annotation_service/index.ts index a30ea572a2723..9847ce1db6552 100644 --- a/x-pack/legacy/plugins/ml/server/models/annotation_service/index.ts +++ b/x-pack/legacy/plugins/ml/server/models/annotation_service/index.ts @@ -4,10 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { annotationProvider, callWithRequestType } from './annotation'; +import { RequestHandlerContext } from 'src/core/server'; +import { annotationProvider } from './annotation'; -export function annotationServiceProvider(callWithRequest: callWithRequestType) { +export function annotationServiceProvider(context: RequestHandlerContext) { return { - ...annotationProvider(callWithRequest), + ...annotationProvider(context), }; } diff --git a/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts b/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts new file mode 100644 index 0000000000000..f8ef3c9f715de --- /dev/null +++ b/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { schema } from '@kbn/config-schema'; + +export const indexAnnotationSchema = { + timestamp: schema.number(), + end_timestamp: schema.number(), + annotation: schema.string(), + job_id: schema.string(), + type: schema.string(), + create_time: schema.maybe(schema.number()), + create_username: schema.maybe(schema.string()), + modified_time: schema.maybe(schema.number()), + modified_username: schema.maybe(schema.string()), + _id: schema.maybe(schema.string()), + key: schema.maybe(schema.string()), +}; diff --git a/x-pack/legacy/plugins/ml/server/routes/annotations.js b/x-pack/legacy/plugins/ml/server/routes/annotations.js deleted file mode 100644 index e7cb38184dc18..0000000000000 --- a/x-pack/legacy/plugins/ml/server/routes/annotations.js +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import Boom from 'boom'; -import _ from 'lodash'; -import { i18n } from '@kbn/i18n'; - -import { callWithRequestFactory } from '../client/call_with_request_factory'; -import { isAnnotationsFeatureAvailable } from '../lib/check_annotations'; -import { wrapError } from '../client/errors'; -import { annotationServiceProvider } from '../models/annotation_service'; - -import { ANNOTATION_USER_UNKNOWN } from '../../common/constants/annotations'; - -function getAnnotationsFeatureUnavailableErrorMessage() { - return Boom.badRequest( - i18n.translate('xpack.ml.routes.annotations.annotationsFeatureUnavailableErrorMessage', { - defaultMessage: - 'Index and aliases required for the annotations feature have not been' + - ' created or are not accessible for the current user.', - }) - ); -} -export function annotationRoutes({ commonRouteConfig, elasticsearchPlugin, route }) { - route({ - method: 'POST', - path: '/api/ml/annotations', - handler(request) { - const callWithRequest = callWithRequestFactory(elasticsearchPlugin, request); - const { getAnnotations } = annotationServiceProvider(callWithRequest); - return getAnnotations(request.payload).catch(resp => wrapError(resp)); - }, - config: { - ...commonRouteConfig, - }, - }); - - route({ - method: 'PUT', - path: '/api/ml/annotations/index', - async handler(request) { - const callWithRequest = callWithRequestFactory(elasticsearchPlugin, request); - const annotationsFeatureAvailable = await isAnnotationsFeatureAvailable(callWithRequest); - if (annotationsFeatureAvailable === false) { - return getAnnotationsFeatureUnavailableErrorMessage(); - } - - const { indexAnnotation } = annotationServiceProvider(callWithRequest); - const username = _.get(request, 'auth.credentials.username', ANNOTATION_USER_UNKNOWN); - return indexAnnotation(request.payload, username).catch(resp => wrapError(resp)); - }, - config: { - ...commonRouteConfig, - }, - }); - - route({ - method: 'DELETE', - path: '/api/ml/annotations/delete/{annotationId}', - async handler(request) { - const callWithRequest = callWithRequestFactory(elasticsearchPlugin, request); - const annotationsFeatureAvailable = await isAnnotationsFeatureAvailable(callWithRequest); - if (annotationsFeatureAvailable === false) { - return getAnnotationsFeatureUnavailableErrorMessage(); - } - - const annotationId = request.params.annotationId; - const { deleteAnnotation } = annotationServiceProvider(callWithRequest); - return deleteAnnotation(annotationId).catch(resp => wrapError(resp)); - }, - config: { - ...commonRouteConfig, - }, - }); -} diff --git a/x-pack/legacy/plugins/ml/server/routes/annotations.ts b/x-pack/legacy/plugins/ml/server/routes/annotations.ts new file mode 100644 index 0000000000000..8b2d6cb897c21 --- /dev/null +++ b/x-pack/legacy/plugins/ml/server/routes/annotations.ts @@ -0,0 +1,152 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import Boom from 'boom'; +import _ from 'lodash'; +import { i18n } from '@kbn/i18n'; + +import { schema } from '@kbn/config-schema'; +// @ts-ignore +import { isAnnotationsFeatureAvailable } from '../lib/check_annotations'; +import { annotationServiceProvider } from '../models/annotation_service'; +import { wrapError } from '../client/error_wrapper'; +import { licensePreRoutingFactory } from '../new_platform/licence_check_pre_routing_factory'; +import { RouteInitialization } from '../new_platform/plugin'; +import { indexAnnotationSchema } from '../new_platform/annotations_schema'; + +import { ANNOTATION_USER_UNKNOWN } from '../../common/constants/annotations'; + +function getAnnotationsFeatureUnavailableErrorMessage() { + return Boom.badRequest( + i18n.translate('xpack.ml.routes.annotations.annotationsFeatureUnavailableErrorMessage', { + defaultMessage: + 'Index and aliases required for the annotations feature have not been' + + ' created or are not accessible for the current user.', + }) + ); +} +/** + * Routes for annotations + */ +export function annotationRoutes({ xpackMainPlugin, router }: RouteInitialization) { + /** + * @apiGroup Annotations + * + * @api {post} /api/ml/annotations Gets annotations + * @apiName GetAnnotations + * @apiDescription Gets annotations. + * + * @apiParam {String[]} jobIds List of job IDs + * @apiParam {String} earliestMs + * @apiParam {Number} latestMs + * @apiParam {Number} maxAnnotations Max limit of annotations returned + * + * @apiSuccess {Boolean} success + * @apiSuccess {Object} annotations + */ + router.post( + { + path: '/api/ml/annotations', + validate: { + body: schema.object({ + jobIds: schema.arrayOf(schema.string()), + earliestMs: schema.number(), + latestMs: schema.number(), + maxAnnotations: schema.number(), + }), + }, + }, + licensePreRoutingFactory(xpackMainPlugin, async (context, request, response) => { + try { + const { getAnnotations } = annotationServiceProvider(context); + const resp = await getAnnotations(request.body); + + return response.ok({ + body: resp, + }); + } catch (e) { + return response.customError(wrapError(e)); + } + }) + ); + + /** + * @apiGroup Annotations + * + * @api {put} /api/ml/annotations/index Index annotation + * @apiName IndexAnnotations + * @apiDescription Index the annotation. + * + * @apiParam {Object} annotation + * @apiParam {String} username + */ + router.put( + { + path: '/api/ml/annotations/index', + validate: { + body: schema.object(indexAnnotationSchema), + }, + }, + licensePreRoutingFactory(xpackMainPlugin, async (context, request, response) => { + try { + const annotationsFeatureAvailable = await isAnnotationsFeatureAvailable( + context.ml!.mlClient.callAsCurrentUser + ); + if (annotationsFeatureAvailable === false) { + throw getAnnotationsFeatureUnavailableErrorMessage(); + } + + const { indexAnnotation } = annotationServiceProvider(context); + const username = _.get(request, 'auth.credentials.username', ANNOTATION_USER_UNKNOWN); + const resp = await indexAnnotation(request.body, username); + + return response.ok({ + body: resp, + }); + } catch (e) { + return response.customError(wrapError(e)); + } + }) + ); + + /** + * @apiGroup Annotations + * + * @api {delete} /api/ml/annotations/index Deletes annotation + * @apiName DeleteAnnotation + * @apiDescription Deletes specified annotation + * + * @apiParam {String} annotationId + */ + router.delete( + { + path: '/api/ml/annotations/delete/{annotationId}', + validate: { + params: schema.object({ annotationId: schema.string() }), + }, + }, + licensePreRoutingFactory(xpackMainPlugin, async (context, request, response) => { + try { + const annotationsFeatureAvailable = await isAnnotationsFeatureAvailable( + context.ml!.mlClient.callAsCurrentUser + ); + if (annotationsFeatureAvailable === false) { + throw getAnnotationsFeatureUnavailableErrorMessage(); + } + + const annotationId = request.params.annotationId; + const { deleteAnnotation } = annotationServiceProvider(context); + const resp = await deleteAnnotation(annotationId); + + return response.ok({ + body: resp, + }); + } catch (e) { + return response.customError(wrapError(e)); + } + }) + ); +} diff --git a/x-pack/legacy/plugins/ml/server/routes/apidoc.json b/x-pack/legacy/plugins/ml/server/routes/apidoc.json index 3c041bed99214..919592f8ed62a 100644 --- a/x-pack/legacy/plugins/ml/server/routes/apidoc.json +++ b/x-pack/legacy/plugins/ml/server/routes/apidoc.json @@ -46,6 +46,10 @@ "RecognizeIndex", "GetModule", "SetupModule", - "CheckExistingModuleJobs" + "CheckExistingModuleJobs", + "Annotations", + "GetAnnotations", + "IndexAnnotations", + "DeleteAnnotation" ] } From b44ced474e2c1e3a2522c6d5e81b942de3f7e645 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Thu, 6 Feb 2020 20:11:22 -0500 Subject: [PATCH 2/5] create ts file for feature check --- .../ml/server/lib/check_annotations/index.d.ts | 11 +++++++++++ .../plugins/ml/server/lib/check_annotations/index.js | 8 ++++---- x-pack/legacy/plugins/ml/server/routes/annotations.ts | 1 - 3 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/server/lib/check_annotations/index.d.ts diff --git a/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.d.ts b/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.d.ts new file mode 100644 index 0000000000000..dbd08eacd3ca2 --- /dev/null +++ b/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.d.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { IScopedClusterClient } from 'src/core/server'; + +export function isAnnotationsFeatureAvailable( + callAsCurrentUser: IScopedClusterClient['callAsCurrentUser'] +): boolean; diff --git a/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.js b/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.js index 7773d01625aaf..186c27b0326d7 100644 --- a/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.js +++ b/x-pack/legacy/plugins/ml/server/lib/check_annotations/index.js @@ -16,20 +16,20 @@ import { // - ML_ANNOTATIONS_INDEX_PATTERN index is present // - ML_ANNOTATIONS_INDEX_ALIAS_READ alias is present // - ML_ANNOTATIONS_INDEX_ALIAS_WRITE alias is present -export async function isAnnotationsFeatureAvailable(callWithRequest) { +export async function isAnnotationsFeatureAvailable(callAsCurrentUser) { try { const indexParams = { index: ML_ANNOTATIONS_INDEX_PATTERN }; - const annotationsIndexExists = await callWithRequest('indices.exists', indexParams); + const annotationsIndexExists = await callAsCurrentUser('indices.exists', indexParams); if (!annotationsIndexExists) return false; - const annotationsReadAliasExists = await callWithRequest('indices.existsAlias', { + const annotationsReadAliasExists = await callAsCurrentUser('indices.existsAlias', { name: ML_ANNOTATIONS_INDEX_ALIAS_READ, }); if (!annotationsReadAliasExists) return false; - const annotationsWriteAliasExists = await callWithRequest('indices.existsAlias', { + const annotationsWriteAliasExists = await callAsCurrentUser('indices.existsAlias', { name: ML_ANNOTATIONS_INDEX_ALIAS_WRITE, }); if (!annotationsWriteAliasExists) return false; diff --git a/x-pack/legacy/plugins/ml/server/routes/annotations.ts b/x-pack/legacy/plugins/ml/server/routes/annotations.ts index 8b2d6cb897c21..0ddc4d5b37ffa 100644 --- a/x-pack/legacy/plugins/ml/server/routes/annotations.ts +++ b/x-pack/legacy/plugins/ml/server/routes/annotations.ts @@ -9,7 +9,6 @@ import _ from 'lodash'; import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; -// @ts-ignore import { isAnnotationsFeatureAvailable } from '../lib/check_annotations'; import { annotationServiceProvider } from '../models/annotation_service'; import { wrapError } from '../client/error_wrapper'; From f65d53f3abd0d8ca2bf0cbe21c598579b1a6ae53 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Fri, 7 Feb 2020 13:50:29 -0500 Subject: [PATCH 3/5] update schema to allow null value for earliest/latestMs --- .../ml/server/new_platform/annotations_schema.ts | 9 +++++++++ .../plugins/ml/server/routes/annotations.ts | 15 +++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts b/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts index f8ef3c9f715de..7d3d6aabb129c 100644 --- a/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts +++ b/x-pack/legacy/plugins/ml/server/new_platform/annotations_schema.ts @@ -19,3 +19,12 @@ export const indexAnnotationSchema = { _id: schema.maybe(schema.string()), key: schema.maybe(schema.string()), }; + +export const getAnnotationsSchema = { + jobIds: schema.arrayOf(schema.string()), + earliestMs: schema.oneOf([schema.nullable(schema.number()), schema.maybe(schema.number())]), + latestMs: schema.oneOf([schema.nullable(schema.number()), schema.maybe(schema.number())]), + maxAnnotations: schema.number(), +}; + +export const deleteAnnotationSchema = { annotationId: schema.string() }; diff --git a/x-pack/legacy/plugins/ml/server/routes/annotations.ts b/x-pack/legacy/plugins/ml/server/routes/annotations.ts index 0ddc4d5b37ffa..78f87095bf0fe 100644 --- a/x-pack/legacy/plugins/ml/server/routes/annotations.ts +++ b/x-pack/legacy/plugins/ml/server/routes/annotations.ts @@ -14,7 +14,11 @@ import { annotationServiceProvider } from '../models/annotation_service'; import { wrapError } from '../client/error_wrapper'; import { licensePreRoutingFactory } from '../new_platform/licence_check_pre_routing_factory'; import { RouteInitialization } from '../new_platform/plugin'; -import { indexAnnotationSchema } from '../new_platform/annotations_schema'; +import { + deleteAnnotationSchema, + getAnnotationsSchema, + indexAnnotationSchema, +} from '../new_platform/annotations_schema'; import { ANNOTATION_USER_UNKNOWN } from '../../common/constants/annotations'; @@ -50,12 +54,7 @@ export function annotationRoutes({ xpackMainPlugin, router }: RouteInitializatio { path: '/api/ml/annotations', validate: { - body: schema.object({ - jobIds: schema.arrayOf(schema.string()), - earliestMs: schema.number(), - latestMs: schema.number(), - maxAnnotations: schema.number(), - }), + body: schema.object(getAnnotationsSchema), }, }, licensePreRoutingFactory(xpackMainPlugin, async (context, request, response) => { @@ -124,7 +123,7 @@ export function annotationRoutes({ xpackMainPlugin, router }: RouteInitializatio { path: '/api/ml/annotations/delete/{annotationId}', validate: { - params: schema.object({ annotationId: schema.string() }), + params: schema.object(deleteAnnotationSchema), }, }, licensePreRoutingFactory(xpackMainPlugin, async (context, request, response) => { From 8ec7007882204f4b3a6a62b6ebebda351e54a19c Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Fri, 7 Feb 2020 17:06:43 -0500 Subject: [PATCH 4/5] update annotations model test --- .../annotation_service/annotation.test.ts | 64 ++++++++++++------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.test.ts b/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.test.ts index ea16eb8870014..7e0649d15bfb0 100644 --- a/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.test.ts +++ b/x-pack/legacy/plugins/ml/server/models/annotation_service/annotation.test.ts @@ -6,6 +6,7 @@ import getAnnotationsRequestMock from './__mocks__/get_annotations_request.json'; import getAnnotationsResponseMock from './__mocks__/get_annotations_response.json'; +import { RequestHandlerContext } from 'src/core/server'; import { ANNOTATION_TYPE } from '../../../common/constants/annotations'; import { ML_ANNOTATIONS_INDEX_ALIAS_WRITE } from '../../../common/constants/index_patterns'; @@ -19,23 +20,30 @@ const acknowledgedResponseMock = { acknowledged: true }; const jobIdMock = 'jobIdMock'; describe('annotation_service', () => { - let callWithRequestSpy: jest.Mock; + let callWithRequestSpy: any; beforeEach(() => { - callWithRequestSpy = jest.fn((action: string) => { - switch (action) { - case 'delete': - case 'index': - return Promise.resolve(acknowledgedResponseMock); - case 'search': - return Promise.resolve(getAnnotationsResponseMock); - } - }); + callWithRequestSpy = ({ + ml: { + mlClient: { + callAsCurrentUser: jest.fn((action: string) => { + switch (action) { + case 'delete': + case 'index': + return Promise.resolve(acknowledgedResponseMock); + case 'search': + return Promise.resolve(getAnnotationsResponseMock); + } + }), + }, + }, + } as unknown) as RequestHandlerContext; }); describe('deleteAnnotation()', () => { it('should delete annotation', async done => { const { deleteAnnotation } = annotationServiceProvider(callWithRequestSpy); + const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser; const annotationMockId = 'mockId'; const deleteParamsMock: DeleteParams = { @@ -46,8 +54,8 @@ describe('annotation_service', () => { const response = await deleteAnnotation(annotationMockId); - expect(callWithRequestSpy.mock.calls[0][0]).toBe('delete'); - expect(callWithRequestSpy.mock.calls[0][1]).toEqual(deleteParamsMock); + expect(mockFunct.mock.calls[0][0]).toBe('delete'); + expect(mockFunct.mock.calls[0][1]).toEqual(deleteParamsMock); expect(response).toBe(acknowledgedResponseMock); done(); }); @@ -56,6 +64,7 @@ describe('annotation_service', () => { describe('getAnnotation()', () => { it('should get annotations for specific job', async done => { const { getAnnotations } = annotationServiceProvider(callWithRequestSpy); + const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser; const indexAnnotationArgsMock: IndexAnnotationArgs = { jobIds: [jobIdMock], @@ -66,8 +75,8 @@ describe('annotation_service', () => { const response: GetResponse = await getAnnotations(indexAnnotationArgsMock); - expect(callWithRequestSpy.mock.calls[0][0]).toBe('search'); - expect(callWithRequestSpy.mock.calls[0][1]).toEqual(getAnnotationsRequestMock); + expect(mockFunct.mock.calls[0][0]).toBe('search'); + expect(mockFunct.mock.calls[0][1]).toEqual(getAnnotationsRequestMock); expect(Object.keys(response.annotations)).toHaveLength(1); expect(response.annotations[jobIdMock]).toHaveLength(2); expect(isAnnotations(response.annotations[jobIdMock])).toBeTruthy(); @@ -81,9 +90,15 @@ describe('annotation_service', () => { message: 'mock error message', }; - const callWithRequestSpyError = jest.fn(() => { - return Promise.resolve(mockEsError); - }); + const callWithRequestSpyError = ({ + ml: { + mlClient: { + callAsCurrentUser: jest.fn(() => { + return Promise.resolve(mockEsError); + }), + }, + }, + } as unknown) as RequestHandlerContext; const { getAnnotations } = annotationServiceProvider(callWithRequestSpyError); @@ -103,6 +118,7 @@ describe('annotation_service', () => { describe('indexAnnotation()', () => { it('should index annotation', async done => { const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy); + const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser; const annotationMock: Annotation = { annotation: 'Annotation text', @@ -114,10 +130,10 @@ describe('annotation_service', () => { const response = await indexAnnotation(annotationMock, usernameMock); - expect(callWithRequestSpy.mock.calls[0][0]).toBe('index'); + expect(mockFunct.mock.calls[0][0]).toBe('index'); // test if the annotation has been correctly augmented - const indexParamsCheck = callWithRequestSpy.mock.calls[0][1]; + const indexParamsCheck = mockFunct.mock.calls[0][1]; const annotation = indexParamsCheck.body; expect(annotation.create_username).toBe(usernameMock); expect(annotation.modified_username).toBe(usernameMock); @@ -130,6 +146,7 @@ describe('annotation_service', () => { it('should remove ._id and .key before updating annotation', async done => { const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy); + const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser; const annotationMock: Annotation = { _id: 'mockId', @@ -143,10 +160,10 @@ describe('annotation_service', () => { const response = await indexAnnotation(annotationMock, usernameMock); - expect(callWithRequestSpy.mock.calls[0][0]).toBe('index'); + expect(mockFunct.mock.calls[0][0]).toBe('index'); // test if the annotation has been correctly augmented - const indexParamsCheck = callWithRequestSpy.mock.calls[0][1]; + const indexParamsCheck = mockFunct.mock.calls[0][1]; const annotation = indexParamsCheck.body; expect(annotation.create_username).toBe(usernameMock); expect(annotation.modified_username).toBe(usernameMock); @@ -161,6 +178,7 @@ describe('annotation_service', () => { it('should update annotation text and the username for modified_username', async done => { const { getAnnotations, indexAnnotation } = annotationServiceProvider(callWithRequestSpy); + const mockFunct = callWithRequestSpy.ml.mlClient.callAsCurrentUser; const indexAnnotationArgsMock: IndexAnnotationArgs = { jobIds: [jobIdMock], @@ -184,9 +202,9 @@ describe('annotation_service', () => { await indexAnnotation(annotation, modifiedUsernameMock); - expect(callWithRequestSpy.mock.calls[1][0]).toBe('index'); + expect(mockFunct.mock.calls[1][0]).toBe('index'); // test if the annotation has been correctly updated - const indexParamsCheck = callWithRequestSpy.mock.calls[1][1]; + const indexParamsCheck = mockFunct.mock.calls[1][1]; const modifiedAnnotation = indexParamsCheck.body; expect(modifiedAnnotation.annotation).toBe(modifiedAnnotationText); expect(modifiedAnnotation.create_username).toBe(originalUsernameMock); From 8a9892a32bab06aa1eaf96426b7f35f5460414c7 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Mon, 10 Feb 2020 14:06:28 -0500 Subject: [PATCH 5/5] use NP security plugin to access user info --- x-pack/legacy/plugins/ml/index.ts | 2 +- x-pack/legacy/plugins/ml/server/new_platform/plugin.ts | 2 ++ x-pack/legacy/plugins/ml/server/routes/annotations.ts | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/ml/index.ts b/x-pack/legacy/plugins/ml/index.ts index 7262c83b6867d..0ef5e14e44f71 100755 --- a/x-pack/legacy/plugins/ml/index.ts +++ b/x-pack/legacy/plugins/ml/index.ts @@ -86,7 +86,7 @@ export const ml = (kibana: any) => { const { usageCollection, cloud, home } = kbnServer.newPlatform.setup.plugins; const plugins = { elasticsearch: server.plugins.elasticsearch, // legacy - security: server.plugins.security, + security: server.newPlatform.setup.plugins.security, xpackMain: server.plugins.xpack_main, spaces: server.plugins.spaces, home, diff --git a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts index 28b2ddc4c3467..68ab88744278e 100644 --- a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts +++ b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts @@ -106,6 +106,7 @@ export interface RouteInitialization { xpackMainPlugin: MlXpackMainPlugin; savedObjects?: SavedObjectsLegacyService; spacesPlugin: any; + securityPlugin: any; cloud?: CloudSetup; } export interface UsageInitialization { @@ -212,6 +213,7 @@ export class Plugin { elasticsearchService: core.elasticsearch, xpackMainPlugin: plugins.xpackMain, spacesPlugin: plugins.spaces, + securityPlugin: plugins.security, }; const extendedRouteInitializationDeps: RouteInitialization = { diff --git a/x-pack/legacy/plugins/ml/server/routes/annotations.ts b/x-pack/legacy/plugins/ml/server/routes/annotations.ts index 78f87095bf0fe..20f52b4b051c4 100644 --- a/x-pack/legacy/plugins/ml/server/routes/annotations.ts +++ b/x-pack/legacy/plugins/ml/server/routes/annotations.ts @@ -34,7 +34,7 @@ function getAnnotationsFeatureUnavailableErrorMessage() { /** * Routes for annotations */ -export function annotationRoutes({ xpackMainPlugin, router }: RouteInitialization) { +export function annotationRoutes({ xpackMainPlugin, router, securityPlugin }: RouteInitialization) { /** * @apiGroup Annotations * @@ -98,8 +98,8 @@ export function annotationRoutes({ xpackMainPlugin, router }: RouteInitializatio } const { indexAnnotation } = annotationServiceProvider(context); - const username = _.get(request, 'auth.credentials.username', ANNOTATION_USER_UNKNOWN); - const resp = await indexAnnotation(request.body, username); + const user = securityPlugin.authc.getCurrentUser(request) || {}; + const resp = await indexAnnotation(request.body, user.username || ANNOTATION_USER_UNKNOWN); return response.ok({ body: resp,