Skip to content

Commit

Permalink
[SIEM] 7.7. NP Cleanup (#61713)
Browse files Browse the repository at this point in the history
* Remove unused legacy services

These were migrated to NP in a previous PR.

* Remove unused legacy request types

* Type our siem client as optional

If a plugin does not have siem enabled, they won't get our client.

While it seems unlikely to be in a situation where our routes are being
hit but our client is unavailable, we will return a 404 in that case,
similar to the unavailability of actions/alerting.

This also removes some redundant checks on action/alerting clients.

* Remove more redundant dependency checks

In general, we use optional chaining to ignore any intermediate null
values, and defer checks to our actual dependencies, e.g. alertsClient
or siemClient.
  • Loading branch information
rylnd authored Mar 30, 2020
1 parent 5042a0d commit 9ff8be6
Show file tree
Hide file tree
Showing 32 changed files with 138 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@ export const createIndexRoute = (router: IRouter) => {

try {
const clusterClient = context.core.elasticsearch.dataClient;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();
const callCluster = clusterClient.callAsCurrentUser;

if (!siemClient) {
return siemResponse.error({ statusCode: 404 });
}

const index = siemClient.signalsIndex;
const indexExists = await getIndexExists(callCluster, index);
if (indexExists) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ export const deleteIndexRoute = (router: IRouter) => {

try {
const clusterClient = context.core.elasticsearch.dataClient;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!siemClient) {
return siemResponse.error({ statusCode: 404 });
}

const callCluster = clusterClient.callAsCurrentUser;
const index = siemClient.signalsIndex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ export const readIndexRoute = (router: IRouter) => {

try {
const clusterClient = context.core.elasticsearch.dataClient;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!siemClient) {
return siemResponse.error({ statusCode: 404 });
}

const index = siemClient.signalsIndex;
const indexExists = await getIndexExists(clusterClient.callAsCurrentUser, index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ describe('read_privileges route', () => {
expect(response.status).toEqual(500);
expect(response.body).toEqual({ message: 'Test error', status_code: 500 });
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(getPrivilegeRequest(), contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});
});

describe('when security plugin is disabled', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@ export const readPrivilegesRoute = (
},
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);

try {
const clusterClient = context.core.elasticsearch.dataClient;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!siemClient) {
return siemResponse.error({ statusCode: 404 });
}

const index = siemClient.signalsIndex;
const clusterPrivileges = await readPrivileges(clusterClient.callAsCurrentUser, index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('add_prepackaged_rules_route', () => {
addPrepackedRulesRoute(server.router);
});

describe('status codes with actionClient and alertClient', () => {
describe('status codes', () => {
test('returns 200 when creating with a valid actionClient and alertClient', async () => {
const request = addPrepackagedRulesRequest();
const response = await server.inject(request, context);
Expand Down Expand Up @@ -96,6 +96,13 @@ describe('add_prepackaged_rules_route', () => {
),
});
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(addPrepackagedRulesRequest(), contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});
});

describe('responses', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,13 @@ export const addPrepackedRulesRoute = (router: IRouter) => {
const siemResponse = buildSiemResponse(response);

try {
if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!actionsClient || !alertsClient) {
if (!siemClient || !actionsClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('create_rules_bulk', () => {
createRulesBulkRoute(server.router);
});

describe('status codes with actionClient and alertClient', () => {
describe('status codes', () => {
test('returns 200 when creating a single rule with a valid actionClient and alertClient', async () => {
const response = await server.inject(getReadBulkRequest(), context);
expect(response.status).toEqual(200);
Expand All @@ -54,6 +54,13 @@ describe('create_rules_bulk', () => {
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(getReadBulkRequest(), contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});
});

describe('unhappy paths', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,12 @@ export const createRulesBulkRoute = (router: IRouter) => {
},
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);
if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!actionsClient || !alertsClient) {
if (!siemClient || !actionsClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ describe('create_rules', () => {
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(getCreateRequest(), contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

it('returns 200 if license is not platinum', async () => {
(context.licensing.license.hasAtLeast as jest.Mock).mockReturnValue(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,13 @@ export const createRulesRoute = (router: IRouter): void => {

try {
validateLicenseForRuleType({ license: context.licensing.license, ruleType: type });
if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.siem.getSiemClient();
const siemClient = context.siem?.getSiemClient();

if (!actionsClient || !alertsClient) {
if (!siemClient || !actionsClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,8 @@ export const deleteRulesBulkRoute = (router: IRouter) => {
const handler: Handler = async (context, request, response) => {
const siemResponse = buildSiemResponse(response);

if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!actionsClient || !alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,9 @@ export const deleteRulesRoute = (router: IRouter) => {

try {
const { id, rule_id: ruleId } = request.query;
if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}

const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!actionsClient || !alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ export const exportRulesRoute = (router: IRouter, config: LegacyServices['config
},
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);
if (!context.alerting) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const alertsClient = context.alerting?.getAlertsClient();

if (!alertsClient) {
return siemResponse.error({ statusCode: 404 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ export const findRulesRoute = (router: IRouter) => {

try {
const { query } = request;
if (!context.alerting) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const alertsClient = context.alerting?.getAlertsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ export const findRulesStatusesRoute = (router: IRouter) => {
async (context, request, response) => {
const { query } = request;
const siemResponse = buildSiemResponse(response);
if (!context.alerting) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const alertsClient = context.alerting?.getAlertsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ export const getPrepackagedRulesStatusRoute = (router: IRouter) => {
},
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);
if (!context.alerting) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const alertsClient = context.alerting?.getAlertsClient();

if (!alertsClient) {
return siemResponse.error({ statusCode: 404 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ describe('import_rules_route', () => {
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(request, contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});
});

describe('unhappy paths', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,27 @@ export const importRulesRoute = (router: IRouter, config: LegacyServices['config
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);

if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.siem.getSiemClient();
try {
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.siem?.getSiemClient();

if (!actionsClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}
if (!siemClient || !actionsClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

const { filename } = request.body.file.hapi;
const fileExtension = extname(filename).toLowerCase();
if (fileExtension !== '.ndjson') {
return siemResponse.error({
statusCode: 400,
body: `Invalid file extension ${fileExtension}`,
});
}
const { filename } = request.body.file.hapi;
const fileExtension = extname(filename).toLowerCase();
if (fileExtension !== '.ndjson') {
return siemResponse.error({
statusCode: 400,
body: `Invalid file extension ${fileExtension}`,
});
}

const objectLimit = config().get<number>('savedObjects.maxImportExportSize');
try {
const objectLimit = config().get<number>('savedObjects.maxImportExportSize');
const readStream = createRulesStreamFromNdJson(objectLimit);
const parsedObjects = await createPromiseFromStreams<PromiseFromStreams[]>([
request.body.file,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,8 @@ export const patchRulesBulkRoute = (router: IRouter) => {
async (context, request, response) => {
const siemResponse = buildSiemResponse(response);

if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!actionsClient || !alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,8 @@ export const patchRulesRoute = (router: IRouter) => {
validateLicenseForRuleType({ license: context.licensing.license, ruleType: type });
}

if (!context.alerting || !context.actions) {
return siemResponse.error({ statusCode: 404 });
}

const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const alertsClient = context.alerting?.getAlertsClient();
const actionsClient = context.actions?.getActionsClient();
const savedObjectsClient = context.core.savedObjects.client;

if (!actionsClient || !alertsClient) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ export const readRulesRoute = (router: IRouter) => {
const { id, rule_id: ruleId } = request.query;
const siemResponse = buildSiemResponse(response);

if (!context.alerting) {
return siemResponse.error({ statusCode: 404 });
}
const alertsClient = context.alerting.getAlertsClient();
const alertsClient = context.alerting?.getAlertsClient();
const savedObjectsClient = context.core.savedObjects.client;

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ describe('update_rules_bulk', () => {
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

it('returns 404 if siem client is unavailable', async () => {
const { siem, ...contextWithoutSiem } = context;
const response = await server.inject(getUpdateBulkRequest(), contextWithoutSiem);
expect(response.status).toEqual(404);
expect(response.body).toEqual({ message: 'Not Found', status_code: 404 });
});

test('returns an error if update throws', async () => {
clients.alertsClient.update.mockImplementation(() => {
throw new Error('Test error');
Expand Down
Loading

0 comments on commit 9ff8be6

Please sign in to comment.