From 08d6a6178a7e5f43a6e3ad24ec811cc422de53e9 Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Sun, 15 Mar 2020 19:05:39 -0400 Subject: [PATCH 1/6] EMT-248: initial check in to see if we get a clean build --- .../common/types/rest_spec/agent.ts | 8 +- .../server/routes/agent/acks_handlers.test.ts | 100 ++++++++++++++++ .../server/routes/agent/acks_handlers.ts | 72 ++++++++++++ .../server/routes/agent/handlers.ts | 36 +----- .../server/routes/agent/index.ts | 11 +- .../server/services/agents/acks.test.ts | 84 +++++++++++++ .../server/services/agents/acks.ts | 90 ++++++++++++-- .../server/services/agents/crud.ts | 2 +- .../server/types/models/agent.ts | 5 + .../server/types/rest_spec/agent.ts | 4 +- .../api_integration/apis/fleet/agents/acks.ts | 111 +++++++++++++++++- .../es_archives/fleet/agents/data.json | 12 ++ 12 files changed, 479 insertions(+), 56 deletions(-) create mode 100644 x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts create mode 100644 x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts create mode 100644 x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts diff --git a/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts b/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts index af919d973b7d9..c09320d400481 100644 --- a/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts +++ b/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts @@ -69,13 +69,19 @@ export interface PostAgentEnrollResponse { export interface PostAgentAcksRequest { body: { - action_ids: string[]; + events: AgentEvent[]; }; params: { agentId: string; }; } +export interface PostAgentAcksResponse { + action: string; + success: boolean; + items: AgentAction[]; +} + export interface PostAgentUnenrollRequest { body: { kuery: string } | { ids: string[] }; } diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts new file mode 100644 index 0000000000000..cc75dbcce4445 --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts @@ -0,0 +1,100 @@ +/* + * 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 { postAgentAcksHandlerBuilder } from './acks_handlers'; +import { + KibanaResponseFactory, + RequestHandlerContext, + SavedObjectsClientContract, +} from 'kibana/server'; +import { httpServerMock, savedObjectsClientMock } from '../../../../../../src/core/server/mocks'; +import { PostAgentAcksResponse } from '../../../common/types/rest_spec'; +import { AckEventSchema } from '../../types/models'; +import { AcksService } from '../../services/agents'; + +describe('test acks schema', () => { + it('validate that ack event schema expect action id', async () => { + expect(() => + AckEventSchema.validate({ + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + agent_id: 'agent', + message: 'hello', + payload: 'payload', + }) + ).toThrow(Error); + + expect( + AckEventSchema.validate({ + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + agent_id: 'agent', + action_id: 'actionId', + message: 'hello', + payload: 'payload', + }) + ).toBeTruthy(); + }); +}); + +describe('test acks handlers', () => { + let mockResponse: jest.Mocked; + let mockSavedObjectsClient: jest.Mocked; + + beforeEach(() => { + mockSavedObjectsClient = savedObjectsClientMock.create(); + mockResponse = httpServerMock.createResponseFactory(); + }); + + it('should succeed on valid agent event', async () => { + const mockRequest = httpServerMock.createKibanaRequest({ + headers: { + authorization: 'ApiKey TmVqTDBIQUJsRkw1em52R1ZIUF86NS1NaTItdHFUTHFHbThmQW1Fb0ljUQ==', + }, + body: { + events: [ + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: 'action1', + agent_id: 'agent', + message: 'message', + }, + ], + }, + }); + + const ackService: AcksService = { + acknowledgeAgentActions: jest.fn().mockReturnValueOnce([ + { + type: 'CONFIG_CHANGE', + id: 'action1', + }, + ]), + getAgentByAccessAPIKeyId: jest.fn().mockReturnValueOnce({ + id: 'agent', + }), + getSavedObjectsClientContract: jest.fn().mockReturnValueOnce(mockSavedObjectsClient), + saveAgentEvents: jest.fn(), + } as jest.Mocked; + + const postAgentAcksHandler = postAgentAcksHandlerBuilder(ackService); + await postAgentAcksHandler(({} as unknown) as RequestHandlerContext, mockRequest, mockResponse); + expect(mockResponse.ok.mock.calls[0][0]?.body as PostAgentAcksResponse).toEqual({ + action: 'acks', + success: true, + items: [ + { + type: 'CONFIG_CHANGE', + id: 'action1', + }, + ], + }); + }); +}); diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts new file mode 100644 index 0000000000000..8fb6467d74d67 --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts @@ -0,0 +1,72 @@ +/* + * 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. + */ + +// handlers that handle events from agents in response to actions received + +import { RequestHandler } from 'kibana/server'; +import { TypeOf } from '@kbn/config-schema'; +import { PostAgentAcksRequestSchema } from '../../types/rest_spec'; +import * as APIKeyService from '../../services/api_keys'; +import { AcksService } from '../../services/agents'; +import { AgentEvent } from '../../../common/types/models'; +import { PostAgentAcksResponse } from '../../../common/types/rest_spec'; + +export const postAgentAcksHandlerBuilder = function( + ackService: AcksService +): RequestHandler< + TypeOf, + undefined, + TypeOf +> { + return async (context, request, response) => { + try { + const soClient = ackService.getSavedObjectsClientContract(request); + const res = APIKeyService.parseApiKey(request.headers); + const agent = await ackService.getAgentByAccessAPIKeyId(soClient, res.apiKeyId as string); + const agentEvents = request.body.events as AgentEvent[]; + + // validate that all events are for the authorized agent obtained from the api key + const notAuthorizedAgentEvent = agentEvents.filter( + agentEvent => agentEvent.agent_id !== agent.id + ); + + if (notAuthorizedAgentEvent && notAuthorizedAgentEvent.length > 0) { + return response.badRequest({ + body: + 'agent events contains events with different agent id from currently authorized agent', + }); + } + + const agentActions = await ackService.acknowledgeAgentActions(soClient, agent, agentEvents); + + if (agentActions.length > 0) { + await ackService.saveAgentEvents(soClient, agentEvents); + } + + const body: PostAgentAcksResponse = { + action: 'acks', + success: true, + items: [...agentActions], + }; + + return response.ok({ body }); + } catch (e) { + // eslint-disable-next-line no-console + console.log(JSON.stringify(e.message)); + if (e.isBoom) { + return response.customError({ + statusCode: e.output.statusCode, + body: { message: e.message }, + }); + } + + return response.customError({ + statusCode: 500, + body: { message: e.message }, + }); + } + }; +}; diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts b/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts index cb4e4d557d74f..cf1fd2476f310 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts @@ -23,7 +23,6 @@ import { GetOneAgentEventsRequestSchema, PostAgentCheckinRequestSchema, PostAgentEnrollRequestSchema, - PostAgentAcksRequestSchema, PostAgentUnenrollRequestSchema, GetAgentStatusRequestSchema, } from '../../types'; @@ -31,7 +30,7 @@ import * as AgentService from '../../services/agents'; import * as APIKeyService from '../../services/api_keys'; import { appContextService } from '../../services/app_context'; -function getInternalUserSOClient(request: KibanaRequest) { +export function getInternalUserSOClient(request: KibanaRequest) { // soClient as kibana internal users, be carefull on how you use it, security is not enabled return appContextService.getSavedObjects().getScopedClient(request, { excludedWrappers: ['security'], @@ -210,39 +209,6 @@ export const postAgentCheckinHandler: RequestHandler< } }; -export const postAgentAcksHandler: RequestHandler< - TypeOf, - undefined, - TypeOf -> = async (context, request, response) => { - try { - const soClient = getInternalUserSOClient(request); - const res = APIKeyService.parseApiKey(request.headers); - const agent = await AgentService.getAgentByAccessAPIKeyId(soClient, res.apiKeyId as string); - - await AgentService.acknowledgeAgentActions(soClient, agent, request.body.action_ids); - - const body = { - action: 'acks', - success: true, - }; - - return response.ok({ body }); - } catch (e) { - if (e.isBoom) { - return response.customError({ - statusCode: e.output.statusCode, - body: { message: e.message }, - }); - } - - return response.customError({ - statusCode: 500, - body: { message: e.message }, - }); - } -}; - export const postAgentEnrollHandler: RequestHandler< undefined, undefined, diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts index 8a65fa9c50e8b..c85629ea22ad9 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts @@ -31,10 +31,12 @@ import { getAgentEventsHandler, postAgentCheckinHandler, postAgentEnrollHandler, - postAgentAcksHandler, postAgentsUnenrollHandler, getAgentStatusForConfigHandler, + getInternalUserSOClient, } from './handlers'; +import { postAgentAcksHandlerBuilder } from './acks_handlers'; +import * as AgentService from '../../services/agents'; export const registerRoutes = (router: IRouter) => { // Get one @@ -101,7 +103,12 @@ export const registerRoutes = (router: IRouter) => { validate: PostAgentAcksRequestSchema, options: { tags: [] }, }, - postAgentAcksHandler + postAgentAcksHandlerBuilder({ + acknowledgeAgentActions: AgentService.acknowledgeAgentActions, + getAgentByAccessAPIKeyId: AgentService.getAgentByAccessAPIKeyId, + getSavedObjectsClientContract: getInternalUserSOClient, + saveAgentEvents: AgentService.saveAgentEvents, + }) ); router.post( diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts new file mode 100644 index 0000000000000..9a02c031e50f5 --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts @@ -0,0 +1,84 @@ +/* + * 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 { savedObjectsClientMock } from '../../../../../../src/core/server/saved_objects/service/saved_objects_client.mock'; +import { Agent, AgentAction, AgentEvent } from '../../../common/types/models'; +import { AGENT_TYPE_PERMANENT } from '../../../common/constants'; +import { acknowledgeAgentActions } from './acks'; +import { isBoom } from 'boom'; + +describe('test agent acks services', () => { + it('should succeed on valid and matched actions', async () => { + const mockSavedObjectsClient = savedObjectsClientMock.create(); + const agentActions = await acknowledgeAgentActions( + mockSavedObjectsClient, + ({ + id: 'id', + type: AGENT_TYPE_PERMANENT, + actions: [ + { + type: 'CONFIG_CHANGE', + id: 'action1', + sent_at: '2020-03-14T19:45:02.620Z', + timestamp: '2019-01-04T14:32:03.36764-05:00', + created_at: '2020-03-14T19:45:02.620Z', + }, + ], + } as unknown) as Agent, + [ + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: 'action1', + agent_id: 'id', + } as AgentEvent, + ] + ); + expect(agentActions).toEqual([ + ({ + type: 'CONFIG_CHANGE', + id: 'action1', + sent_at: '2020-03-14T19:45:02.620Z', + timestamp: '2019-01-04T14:32:03.36764-05:00', + created_at: '2020-03-14T19:45:02.620Z', + } as unknown) as AgentAction, + ]); + }); + + it('should fail for actions that cannot be found on agent actions list', async () => { + const mockSavedObjectsClient = savedObjectsClientMock.create(); + try { + await acknowledgeAgentActions( + mockSavedObjectsClient, + ({ + id: 'id', + type: AGENT_TYPE_PERMANENT, + actions: [ + { + type: 'CONFIG_CHANGE', + id: 'action1', + sent_at: '2020-03-14T19:45:02.620Z', + timestamp: '2019-01-04T14:32:03.36764-05:00', + created_at: '2020-03-14T19:45:02.620Z', + }, + ], + } as unknown) as Agent, + [ + ({ + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: 'action2', + agent_id: 'id', + } as unknown) as AgentEvent, + ] + ); + expect(true).toBeFalsy(); + } catch (e) { + expect(isBoom(e)).toBeTruthy(); + } + }); +}); diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts index 1732ff9cf5b5c..8633a7f3afb76 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts @@ -4,25 +4,91 @@ * you may not use this file except in compliance with the Elastic License. */ -import { SavedObjectsClientContract } from 'kibana/server'; -import { Agent, AgentSOAttributes } from '../../types'; -import { AGENT_SAVED_OBJECT_TYPE } from '../../constants'; +import { + KibanaRequest, + SavedObjectsBulkCreateObject, + SavedObjectsBulkResponse, + SavedObjectsClientContract, +} from 'kibana/server'; +import Boom from 'boom'; +import { + Agent, + AgentAction, + AgentEvent, + AgentEventSOAttributes, + AgentSOAttributes, +} from '../../types'; +import { AGENT_EVENT_SAVED_OBJECT_TYPE, AGENT_SAVED_OBJECT_TYPE } from '../../constants'; export async function acknowledgeAgentActions( soClient: SavedObjectsClientContract, agent: Agent, - actionIds: string[] -) { + agentEvents: AgentEvent[] +): Promise { const now = new Date().toISOString(); - const updatedActions = agent.actions.map(action => { - if (action.sent_at) { - return action; + const agentActionMap: Map = new Map( + agent.actions.map(agentAction => [agentAction.id, agentAction]) + ); + + const matchedUpdatedActions: AgentAction[] = []; + + agentEvents.forEach(agentEvent => { + if (agentActionMap.has(agentEvent.action_id!)) { + const action = agentActionMap.get(agentEvent.action_id!) as AgentAction; + if (!action.sent_at) { + action.sent_at = now; + } + matchedUpdatedActions.push(action); + } else { + throw Boom.badRequest('all actions should belong to current agent'); } - return { ...action, sent_at: actionIds.indexOf(action.id) >= 0 ? now : undefined }; }); - await soClient.update(AGENT_SAVED_OBJECT_TYPE, agent.id, { - actions: updatedActions, - }); + if (matchedUpdatedActions.length > 0) { + await soClient.update(AGENT_SAVED_OBJECT_TYPE, agent.id, { + actions: matchedUpdatedActions, + }); + } + + return matchedUpdatedActions; +} + +export async function saveAgentEvents( + soClient: SavedObjectsClientContract, + events: AgentEvent[] +): Promise> { + const objects: Array> = events.map( + eventData => { + return { + attributes: { + ...eventData, + payload: eventData.payload ? JSON.stringify(eventData.payload) : undefined, + }, + type: AGENT_EVENT_SAVED_OBJECT_TYPE, + }; + } + ); + + return await soClient.bulkCreate(objects); +} + +export interface AcksService { + acknowledgeAgentActions: ( + soClient: SavedObjectsClientContract, + agent: Agent, + actionIds: AgentEvent[] + ) => Promise; + + getAgentByAccessAPIKeyId: ( + soClient: SavedObjectsClientContract, + accessAPIKeyId: string + ) => Promise; + + getSavedObjectsClientContract: (kibanaRequest: KibanaRequest) => SavedObjectsClientContract; + + saveAgentEvents: ( + soClient: SavedObjectsClientContract, + events: AgentEvent[] + ) => Promise>; } diff --git a/x-pack/plugins/ingest_manager/server/services/agents/crud.ts b/x-pack/plugins/ingest_manager/server/services/agents/crud.ts index bcd825fee8725..cdbdf164e834d 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/crud.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/crud.ts @@ -68,7 +68,7 @@ export async function getAgent(soClient: SavedObjectsClientContract, agentId: st export async function getAgentByAccessAPIKeyId( soClient: SavedObjectsClientContract, accessAPIKeyId: string -) { +): Promise { const response = await soClient.find({ type: AGENT_SAVED_OBJECT_TYPE, searchFields: ['access_api_key_id'], diff --git a/x-pack/plugins/ingest_manager/server/types/models/agent.ts b/x-pack/plugins/ingest_manager/server/types/models/agent.ts index 276dddf9e3d1c..e0d252faaaf87 100644 --- a/x-pack/plugins/ingest_manager/server/types/models/agent.ts +++ b/x-pack/plugins/ingest_manager/server/types/models/agent.ts @@ -44,6 +44,11 @@ const AgentEventBase = { stream_id: schema.maybe(schema.string()), }; +export const AckEventSchema = schema.object({ + ...AgentEventBase, + ...{ action_id: schema.string() }, +}); + export const AgentEventSchema = schema.object({ ...AgentEventBase, }); diff --git a/x-pack/plugins/ingest_manager/server/types/rest_spec/agent.ts b/x-pack/plugins/ingest_manager/server/types/rest_spec/agent.ts index 92422274d5cf4..9fe84c12521ad 100644 --- a/x-pack/plugins/ingest_manager/server/types/rest_spec/agent.ts +++ b/x-pack/plugins/ingest_manager/server/types/rest_spec/agent.ts @@ -5,7 +5,7 @@ */ import { schema } from '@kbn/config-schema'; -import { AgentEventSchema, AgentTypeSchema } from '../models'; +import { AckEventSchema, AgentEventSchema, AgentTypeSchema } from '../models'; export const GetAgentsRequestSchema = { query: schema.object({ @@ -45,7 +45,7 @@ export const PostAgentEnrollRequestSchema = { export const PostAgentAcksRequestSchema = { body: schema.object({ - action_ids: schema.arrayOf(schema.string()), + events: schema.arrayOf(AckEventSchema), }), params: schema.object({ agentId: schema.string(), diff --git a/x-pack/test/api_integration/apis/fleet/agents/acks.ts b/x-pack/test/api_integration/apis/fleet/agents/acks.ts index 1ab54554d62f0..e0768bc54bbc9 100644 --- a/x-pack/test/api_integration/apis/fleet/agents/acks.ts +++ b/x-pack/test/api_integration/apis/fleet/agents/acks.ts @@ -59,7 +59,7 @@ export default function(providerContext: FtrProviderContext) { .expect(401); }); - it('should return a 200 if this a valid acks access', async () => { + it('should return a 200 if this a valid acks request', async () => { const { body: apiResponse } = await supertest .post(`/api/ingest_manager/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') @@ -68,12 +68,117 @@ export default function(providerContext: FtrProviderContext) { `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` ) .send({ - action_ids: ['action1'], + events: [ + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a1', + agent_id: 'agent1', + message: 'hello', + payload: 'payload', + }, + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-05T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a2', + agent_id: 'agent1', + message: 'hello2', + payload: 'payload2', + }, + ], }) .expect(200); - expect(apiResponse.action).to.be('acks'); expect(apiResponse.success).to.be(true); + const { body: eventResponse } = await supertest + .get(`/api/ingest_manager/fleet/agents/agent1/events`) + .set('kbn-xsrf', 'xx') + .set( + 'Authorization', + `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` + ) + .expect(200); + const expectedEvents = eventResponse.list.filter( + item => + item.action_id === '48cebde1-c906-4893-b89f-595d943b72a1' || + item.action_id === '48cebde1-c906-4893-b89f-595d943b72a2' + ); + expect(expectedEvents.length).to.eql(2); + const expectedEvent = eventResponse.list.find( + item => item.action_id === '48cebde1-c906-4893-b89f-595d943b72a1' + ); + expect(expectedEvent).to.eql({ + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a1', + agent_id: 'agent1', + message: 'hello', + payload: 'payload', + }); + }); + + it('should return a 400 when request event list contains event for another agent id', async () => { + const { body: apiResponse } = await supertest + .post(`/api/ingest_manager/fleet/agents/agent1/acks`) + .set('kbn-xsrf', 'xx') + .set( + 'Authorization', + `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` + ) + .send({ + events: [ + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a1', + agent_id: 'agent2', + message: 'hello', + payload: 'payload', + }, + ], + }) + .expect(400); + expect(apiResponse.message).to.eql( + 'agent events contains events with different agent id from currently authorized agent' + ); + }); + + it('should return a 400 when request event list contains action that does not belong to agent current actions', async () => { + const { body: apiResponse } = await supertest + .post(`/api/ingest_manager/fleet/agents/agent1/acks`) + .set('kbn-xsrf', 'xx') + .set( + 'Authorization', + `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` + ) + .send({ + events: [ + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a1', + agent_id: 'agent1', + message: 'hello', + payload: 'payload', + }, + { + type: 'ACTION_RESULT', + subtype: 'CONFIG', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: 'does-not-exist', + agent_id: 'agent1', + message: 'hello', + payload: 'payload', + }, + ], + }) + .expect(400); + expect(apiResponse.message).to.eql('all actions should belong to current agent'); }); }); } diff --git a/x-pack/test/functional/es_archives/fleet/agents/data.json b/x-pack/test/functional/es_archives/fleet/agents/data.json index 36928018d15a0..9b29767d5162d 100644 --- a/x-pack/test/functional/es_archives/fleet/agents/data.json +++ b/x-pack/test/functional/es_archives/fleet/agents/data.json @@ -23,6 +23,18 @@ "type": "PAUSE", "created_at": "2019-09-04T15:01:07+0000", "sent_at": "2019-09-04T15:03:07+0000" + }, + { + "created_at" : "2020-03-15T03:47:15.129Z", + "id" : "48cebde1-c906-4893-b89f-595d943b72a1", + "type" : "CONFIG_CHANGE", + "sent_at": "2020-03-04T15:03:07+0000" + }, + { + "created_at" : "2020-03-16T03:47:15.129Z", + "id" : "48cebde1-c906-4893-b89f-595d943b72a2", + "type" : "CONFIG_CHANGE", + "sent_at": "2020-03-04T15:03:07+0000" }] } } From 2fe97cd4f10ca5b30976142a96beceb7c3c443c3 Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Sun, 15 Mar 2020 20:25:54 -0400 Subject: [PATCH 2/6] EMT-248: fix build --- x-pack/test/api_integration/apis/fleet/agents/acks.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/test/api_integration/apis/fleet/agents/acks.ts b/x-pack/test/api_integration/apis/fleet/agents/acks.ts index e0768bc54bbc9..e451c93979eb4 100644 --- a/x-pack/test/api_integration/apis/fleet/agents/acks.ts +++ b/x-pack/test/api_integration/apis/fleet/agents/acks.ts @@ -101,13 +101,13 @@ export default function(providerContext: FtrProviderContext) { ) .expect(200); const expectedEvents = eventResponse.list.filter( - item => + (item: Record) => item.action_id === '48cebde1-c906-4893-b89f-595d943b72a1' || item.action_id === '48cebde1-c906-4893-b89f-595d943b72a2' ); expect(expectedEvents.length).to.eql(2); - const expectedEvent = eventResponse.list.find( - item => item.action_id === '48cebde1-c906-4893-b89f-595d943b72a1' + const expectedEvent = expectedEvents.find( + (item: Record) => item.action_id === '48cebde1-c906-4893-b89f-595d943b72a1' ); expect(expectedEvent).to.eql({ type: 'ACTION_RESULT', From 42f760bd2ce7d63a9f165c1455fc869aea10a07e Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Sun, 15 Mar 2020 22:17:00 -0400 Subject: [PATCH 3/6] EMT-248: cruft. --- .../plugins/ingest_manager/server/routes/agent/acks_handlers.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts index 8fb6467d74d67..81d774fc78b6e 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts @@ -54,8 +54,6 @@ export const postAgentAcksHandlerBuilder = function( return response.ok({ body }); } catch (e) { - // eslint-disable-next-line no-console - console.log(JSON.stringify(e.message)); if (e.isBoom) { return response.customError({ statusCode: e.output.statusCode, From acbc6e0e266cd493745b427463b835f7a6d6b36d Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Mon, 16 Mar 2020 10:10:54 -0400 Subject: [PATCH 4/6] EMT-248: remove action from response --- .../plugins/ingest_manager/common/types/rest_spec/agent.ts | 1 - .../server/routes/agent/acks_handlers.test.ts | 6 ------ .../ingest_manager/server/routes/agent/acks_handlers.ts | 1 - 3 files changed, 8 deletions(-) diff --git a/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts b/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts index c09320d400481..7bbaf42422bb2 100644 --- a/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts +++ b/x-pack/plugins/ingest_manager/common/types/rest_spec/agent.ts @@ -79,7 +79,6 @@ export interface PostAgentAcksRequest { export interface PostAgentAcksResponse { action: string; success: boolean; - items: AgentAction[]; } export interface PostAgentUnenrollRequest { diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts index cc75dbcce4445..84923d5c33664 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.test.ts @@ -89,12 +89,6 @@ describe('test acks handlers', () => { expect(mockResponse.ok.mock.calls[0][0]?.body as PostAgentAcksResponse).toEqual({ action: 'acks', success: true, - items: [ - { - type: 'CONFIG_CHANGE', - id: 'action1', - }, - ], }); }); }); diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts index 81d774fc78b6e..53b677bb1389e 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/acks_handlers.ts @@ -49,7 +49,6 @@ export const postAgentAcksHandlerBuilder = function( const body: PostAgentAcksResponse = { action: 'acks', success: true, - items: [...agentActions], }; return response.ok({ body }); From 2bf3e10ab714e100c88f8e23a6f9a71c636fd467 Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Mon, 16 Mar 2020 14:45:40 -0400 Subject: [PATCH 5/6] EMT-248: add restrictions for event types --- .../server/services/agents/acks.test.ts | 34 +++++++++++++++++++ .../server/services/agents/acks.ts | 5 +++ .../api_integration/apis/fleet/agents/acks.ts | 27 +++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts index 9a02c031e50f5..3c07463e3af5d 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts @@ -81,4 +81,38 @@ describe('test agent acks services', () => { expect(isBoom(e)).toBeTruthy(); } }); + + it('should fail for events that have types not in the allowed acknowledgement type list', async () => { + const mockSavedObjectsClient = savedObjectsClientMock.create(); + try { + await acknowledgeAgentActions( + mockSavedObjectsClient, + ({ + id: 'id', + type: AGENT_TYPE_PERMANENT, + actions: [ + { + type: 'CONFIG_CHANGE', + id: 'action1', + sent_at: '2020-03-14T19:45:02.620Z', + timestamp: '2019-01-04T14:32:03.36764-05:00', + created_at: '2020-03-14T19:45:02.620Z', + }, + ], + } as unknown) as Agent, + [ + ({ + type: 'ACTION', + subtype: 'FAILED', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: 'action1', + agent_id: 'id', + } as unknown) as AgentEvent, + ] + ); + expect(true).toBeFalsy(); + } catch (e) { + expect(isBoom(e)).toBeTruthy(); + } + }); }); diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts index 8633a7f3afb76..e9464e7a62c72 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts @@ -20,6 +20,8 @@ import { } from '../../types'; import { AGENT_EVENT_SAVED_OBJECT_TYPE, AGENT_SAVED_OBJECT_TYPE } from '../../constants'; +const ALLOWED_ACKNOWLEDGEMENT_TYPE: string[] = ['ACTION_RESULT']; + export async function acknowledgeAgentActions( soClient: SavedObjectsClientContract, agent: Agent, @@ -34,6 +36,9 @@ export async function acknowledgeAgentActions( const matchedUpdatedActions: AgentAction[] = []; agentEvents.forEach(agentEvent => { + if (ALLOWED_ACKNOWLEDGEMENT_TYPE.indexOf(agentEvent.type) < 0) { + throw Boom.badRequest(`${agentEvent.type} not allowed for acknowledgment only ACTION_RESULT`); + } if (agentActionMap.has(agentEvent.action_id!)) { const action = agentActionMap.get(agentEvent.action_id!) as AgentAction; if (!action.sent_at) { diff --git a/x-pack/test/api_integration/apis/fleet/agents/acks.ts b/x-pack/test/api_integration/apis/fleet/agents/acks.ts index e451c93979eb4..a2eba2c23c39d 100644 --- a/x-pack/test/api_integration/apis/fleet/agents/acks.ts +++ b/x-pack/test/api_integration/apis/fleet/agents/acks.ts @@ -180,5 +180,32 @@ export default function(providerContext: FtrProviderContext) { .expect(400); expect(apiResponse.message).to.eql('all actions should belong to current agent'); }); + + it('should return a 400 when request event list contains action types that are not allowed for acknowledgement', async () => { + const { body: apiResponse } = await supertest + .post(`/api/ingest_manager/fleet/agents/agent1/acks`) + .set('kbn-xsrf', 'xx') + .set( + 'Authorization', + `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` + ) + .send({ + events: [ + { + type: 'ACTION', + subtype: 'FAILED', + timestamp: '2019-01-04T14:32:03.36764-05:00', + action_id: '48cebde1-c906-4893-b89f-595d943b72a1', + agent_id: 'agent1', + message: 'hello', + payload: 'payload', + }, + ], + }) + .expect(400); + expect(apiResponse.message).to.eql( + 'ACTION not allowed for acknowledgment only ACTION_RESULT' + ); + }); }); } From 16bdfb236eefba6e66146d5f0dd7107a41c0f409 Mon Sep 17 00:00:00 2001 From: nnamdifrankie Date: Mon, 16 Mar 2020 15:29:22 -0400 Subject: [PATCH 6/6] EMT-248: clean up code a tad --- .../plugins/ingest_manager/server/services/agents/acks.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts index e9464e7a62c72..892d8cdbe657f 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts @@ -36,7 +36,7 @@ export async function acknowledgeAgentActions( const matchedUpdatedActions: AgentAction[] = []; agentEvents.forEach(agentEvent => { - if (ALLOWED_ACKNOWLEDGEMENT_TYPE.indexOf(agentEvent.type) < 0) { + if (!isAllowedType(agentEvent.type)) { throw Boom.badRequest(`${agentEvent.type} not allowed for acknowledgment only ACTION_RESULT`); } if (agentActionMap.has(agentEvent.action_id!)) { @@ -59,6 +59,10 @@ export async function acknowledgeAgentActions( return matchedUpdatedActions; } +function isAllowedType(eventType: string): boolean { + return ALLOWED_ACKNOWLEDGEMENT_TYPE.indexOf(eventType) >= 0; +} + export async function saveAgentEvents( soClient: SavedObjectsClientContract, events: AgentEvent[]