Skip to content

Commit

Permalink
[SIEM] Migrate DE Routes to NP (#58292)
Browse files Browse the repository at this point in the history
* Define a very basic SiemClient

We're going to add our client to the route handler context. Currently,
it's sole purpose is to provide us with the signalsIndex to use for the
request.

This will allow us to stop passing around most uses of config and
getSpaceId, as they were used for this same purpose.

* WIP: Convert first DE route to NP

This is one of the more comprehensive routes, so I think we're good to
convert the rest.

* Abstract joi/NP validator to helper

We're gonna need this all over the place. Until our schema can generate
an NP-compatible type, we can use this helper to generate an equivalent
validator.

* Second route down

* Another one

* updateRulesRoute

* Fix exports

* patchRulesRoute

* Normalize request param type names

* findRulesRoute

* addPrepackagedRulesRoute

* getPrepackagedRulesStatusRoute

* createRulesBulkRoute

* updateRulesBulkRoute

* patchRulesBulkRoute

* deleteRulesBulkRoute

* importRulesRoute

* exportRulesRoute

* findRulesStatusesRoute

* setSignalsStatusRoute

* querySignalsRoute

* Remove unused type

* createIndexRoute

* Convert readIndexRoute

Removes support for a HEAD request here, becuase it was only used by the
signal_index_exists script (which is now gone). A GET request will have
the same semantics, with a few more bytes in the response.

* deleteIndexRoute

* readTagsRoute

* readPrivilegesRoute

We're incorrectly accessing request.auth in here still, fixing that
next.

* We are always authenticated until we support optional auth

* Clean up our route initialization

* Remove our now-unused ClientsService
* Remove unused getIndex helper and references
* Remove legacy route initialization (partial application)

All our routing tests are still totally broken. Coming up!

* Reference name property in context registration

This should be replaced with a reference to the constant, but at least
this doesn't add a third way it's being referenced.

* Convert our first route tests to NP

The API for our test utils isn't final, but this at least gets some
tests passing. We'll see how it handles a more complex example.

* Create Rules tests

* Update addPrepackagedRules tests

There were a lot of incorrect tests in here due to some incorrect route
registration: we were asserting a 404 but receiving a false positive
because the route we wanted didn't exist.

* Read Privileges Route tests

* Delete Rules route tests

* Use more permissive validation in request mock

* createRulesBulkRoute tests

* deleteRulesBulkRoute tests

* More explicit result mock

It was very unclear in the tests what this mock represented.

* findRulesRoute tests

* getPrepackagedRulesRoute tests

* PatchRulesBulkRoute tests

* Convert migrated tests to newest interface

* PatchRulesRoute tests

Also fixes a potential false positive in our patch_rule_bulk_route
validation tests by providing a more realistic payload with a single key
missing.

* Rename file for consistency

* UpdateRulesBulkRoute tests

* deleteRulesBulkRoute tests

* findRulesStatusRoute tests

* updateRulesRoute tests

* setSignalStatusRoute tests

* querySignalsRoute tests

This actually caught a bug where we were not resolving our response  before returning

* Update schema tests following rename of type

* Converts Import Rules route tests to NP form

Most of these tests are failing due to our request not being parsed by
Hapi as it previously was. Once we figure out how to generate a
post-middleware request with a file stream, these should be easily
fixed.

* Remove unused import

This was the last remaining reference to hapi in our server plugin. yay!

* Remove unnecessary tests

We're already covering our error paths here.

* Hit success branch of bulk patch route test

* Add test around error path in our route

* Update our router to validate requests by default

This gives us two important behaviors:

1. Requests to inject() will be populated with default values
2. We can test request validation independent of a handler call
  * This allows more straightforward assertions as we don't have to
  disambiguate between a schema rejection and a data rejection.

* Update route validation tests with new interface

We don't need to reach into our route in the test, nor ts-ignore it.

* Update ImportRules route tests for NP testing

Gets rid of the multipart-form processing that Hapi would convert for us
into a Readable stream. Instead, we generate our own post-middleware
requst that's got a stream on it.

* Remove unnecessary router method

A bug in an initial implementation of inject() lead me to believe that
validations were removing the stream from our requests; this turned out
to be false! YAGNI

* Add an adapter for our route responses

This provides a uniform interface of { body, status, calls }, where body
and status come from the highest-status call (in the case of many). In a
case where we build multiple responses, the preference is to return the
most problematic one first. If there's an issue, one can look into the
other calls and see what's going on.

This breaks the tests and is not fully implemented, but this will allow
us to change how we build responses without affecting our tests.

* Fix remaining assertions in one test file

Helped flesh out our adapter a bit more.

* Update our response adapter to represent how NP transforms our errors

Most importantly, we return a statusCode but not a status_code.

* Update tests to interface with our Response type

This makes them robust to framework/implementation changes.

* Generate our error responses with our siemResponse wrapper

This adds the status_code key that we desire in our error responses.

Tests were updated as well, and they're currently failing because they
expect statusCode, not status_code.

* Update test assertions to look for status_code

* Remove unused import

* Return a meaningful error if an invalid request was injected

This ensures we a) mimic platform behavior so that b) we don't risk a
false positive if our invalid request were to somehow succeed.

* Return a useful error when no route has been registered

* Add back POST variant of delete_rules_bulk_route

Some browsers do not support a request body for DELETE requests.

* Allow headers to be passed to our error response

This is used by Apollo to set some cache/allow headers in the case of
specific bad requests.

* Add back our header-passing from Apollo errors

I also inverted the logic here to handle the special case first.

* Add some tests around our error response helper

* Fix types of our error wrapper

* Move router logic into separate function

This could be decomposed further but getRoute becomes more verbose, so
meh.

* Convert our mock server to a class

It makes the shared state (mocks) of these functions more explicit, and also
does away with some dumb tuple-returns I had (a consequence of trying to
make everything pure).

* Remove need for a route spy

Instead of mocking certain router methods with the same spy and then
retrieving its calls, we can simply iterate over the calls of the router
methods we care about. This is a little less logic and a little more
straightforward.

* Update test with updated copy

We're consolidating on "signals index" when referring to the data index
where signals are stored.

* Remove unneeded type assertion from our route validation factory

We implicitly cast our return value here when we provide the generic
type parameter, so there's no reason for the explicit cast.

* Use exact message assertions

* Export our SiemRequestContext type for consumers

This will move to server/index in NP.

* Make our SiemClient properties readonly

We don't want consumers mutating our state.

* Throw error if SiemClientFactory has not been properly set up

* Remove unnecessary spread

* Use reduce's type argument instead of an explicit assertion

* Remove unnecessary optional chaining

This was a holdover from when we captured calls that had no body, but is
now unnecessary.

* Remove unnecessary headers on mock requests

* Remove non-null assertion in favor of constructor assignment

* Prefer type annotations to explicit casting

If we annotate that both incoming pieces of our headers are of the
correct type, then we can spread them into an object of the same type
and avoid the index signature issue.

* Skip test failure do to handling of authentication

We need to thread through the utility that provides us this same
functionality.

* Mock our cluster calls with realistic data

* Remove TODO as this is addressed in a later PR

Co-authored-by: Josh Dover <[email protected]>
  • Loading branch information
rylnd and joshdover authored Mar 4, 2020
1 parent 5539d69 commit 875e2a5
Show file tree
Hide file tree
Showing 68 changed files with 2,701 additions and 3,423 deletions.
24 changes: 24 additions & 0 deletions x-pack/legacy/plugins/siem/server/client/client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* 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 { SiemClient } from './client';
import { createMockConfig } from '../lib/detection_engine/routes/__mocks__';

describe('SiemClient', () => {
describe('#signalsIndex', () => {
it('returns the index scoped to the specified spaceId', () => {
let mockConfig = createMockConfig();
mockConfig = () => ({
get: jest.fn(() => 'mockSignalsIndex'),
has: jest.fn(),
});
const spaceId = 'fooSpace';
const client = new SiemClient(spaceId, mockConfig);

expect(client.signalsIndex).toEqual('mockSignalsIndex-fooSpace');
});
});
});
21 changes: 21 additions & 0 deletions x-pack/legacy/plugins/siem/server/client/client.ts
Original file line number Diff line number Diff line change
@@ -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 { Legacy } from 'kibana';

import { APP_ID, SIGNALS_INDEX_KEY } from '../../common/constants';

export class SiemClient {
public readonly signalsIndex: string;

constructor(private spaceId: string, private config: Legacy.Server['config']) {
const configuredSignalsIndex = this.config().get<string>(
`xpack.${APP_ID}.${SIGNALS_INDEX_KEY}`
);

this.signalsIndex = `${configuredSignalsIndex}-${this.spaceId}`;
}
}
42 changes: 42 additions & 0 deletions x-pack/legacy/plugins/siem/server/client/factory.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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 { httpServerMock } from '../../../../../../src/core/server/mocks';
import { SiemClientFactory } from './factory';
import { SiemClient } from './client';

jest.mock('./client');
const mockClient = SiemClient as jest.Mock;

describe('SiemClientFactory', () => {
describe('#create', () => {
it('constructs a client with the current spaceId', () => {
const factory = new SiemClientFactory();
const mockRequest = httpServerMock.createKibanaRequest();
factory.setup({ getSpaceId: () => 'mockSpace', config: jest.fn() });
factory.create(mockRequest);

expect(mockClient).toHaveBeenCalledWith('mockSpace', expect.anything());
});

it('constructs a client with the default spaceId if spaces are disabled', () => {
const factory = new SiemClientFactory();
const mockRequest = httpServerMock.createKibanaRequest();
factory.setup({ getSpaceId: undefined, config: jest.fn() });
factory.create(mockRequest);

expect(mockClient).toHaveBeenCalledWith('default', expect.anything());
});

it('cannot call create without calling setup first', () => {
const factory = new SiemClientFactory();
const mockRequest = httpServerMock.createKibanaRequest();
expect(() => factory.create(mockRequest)).toThrow(
'Cannot create SiemClient as config is not present. Did you forget to call setup()?'
);
});
});
});
36 changes: 36 additions & 0 deletions x-pack/legacy/plugins/siem/server/client/factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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 { Legacy } from 'kibana';

import { KibanaRequest } from '../../../../../../src/core/server';
import { SiemClient } from './client';

interface SetupDependencies {
getSpaceId?: (request: KibanaRequest) => string | undefined;
config: Legacy.Server['config'];
}

export class SiemClientFactory {
private getSpaceId?: SetupDependencies['getSpaceId'];
private config?: SetupDependencies['config'];

public setup({ getSpaceId, config }: SetupDependencies) {
this.getSpaceId = getSpaceId;
this.config = config;
}

public create(request: KibanaRequest): SiemClient {
if (this.config == null) {
throw new Error(
'Cannot create SiemClient as config is not present. Did you forget to call setup()?'
);
}

const spaceId = this.getSpaceId?.(request) ?? 'default';
return new SiemClient(spaceId, this.config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { ClientsService, GetScopedClients } from './clients';
export { SiemClient } from './client';
export { SiemClientFactory } from './factory';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Hapi from 'hapi';
import { requestContextMock } from './request_context';
import { serverMock } from './server';
import { requestMock } from './request';
import { responseMock } from './response_factory';

export { clientsServiceMock } from './clients_service_mock';

export const createMockServer = () => {
const server = new Hapi.Server({ port: 0 });

return {
route: server.route.bind(server),
inject: server.inject.bind(server),
};
};
export { requestMock, requestContextMock, responseMock, serverMock };

export const createMockConfig = () => () => ({
get: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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 { httpServerMock } from '../../../../../../../../../src/core/server/mocks';

export const requestMock = {
create: httpServerMock.createKibanaRequest,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* 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 { RequestHandlerContext } from '../../../../../../../../../src/core/server';
import {
coreMock,
elasticsearchServiceMock,
savedObjectsClientMock,
} from '../../../../../../../../../src/core/server/mocks';
import { alertsClientMock } from '../../../../../../../../plugins/alerting/server/mocks';
import { actionsClientMock } from '../../../../../../../../plugins/actions/server/mocks';

const createMockClients = () => ({
actionsClient: actionsClientMock.create(),
alertsClient: alertsClientMock.create(),
clusterClient: elasticsearchServiceMock.createScopedClusterClient(),
savedObjectsClient: savedObjectsClientMock.create(),
siemClient: { signalsIndex: 'mockSignalsIndex' },
});

const createRequestContextMock = (
clients: ReturnType<typeof createMockClients> = createMockClients()
) => {
const coreContext = coreMock.createRequestHandlerContext();
return ({
actions: { getActionsClient: jest.fn(() => clients.actionsClient) },
alerting: { getAlertsClient: jest.fn(() => clients.alertsClient) },
core: {
...coreContext,
elasticsearch: { ...coreContext.elasticsearch, dataClient: clients.clusterClient },
savedObjects: { client: clients.savedObjectsClient },
},
siem: { getSiemClient: jest.fn(() => clients.siemClient) },
} as unknown) as RequestHandlerContext;
};

const createTools = () => {
const clients = createMockClients();
const context = createRequestContextMock(clients);

return { clients, context };
};

export const requestContextMock = {
create: createRequestContextMock,
createMockClients,
createTools,
};
Loading

0 comments on commit 875e2a5

Please sign in to comment.