Skip to content

Commit

Permalink
[SIEM][Detection Engine] Change security model to use SIEM permissions
Browse files Browse the repository at this point in the history
## Summary

* Changes incorrect `access:signals-all` to be the correct `access:siem`
* Adds a boom transformer to push back better error messages to the client in some cases

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https:/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https:/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https:/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https:/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https:/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https:/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
  • Loading branch information
FrankHassanabad committed Nov 27, 2019
1 parent f62d59a commit 371ebd8
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import { RulesRequest } from '../alerts/types';
import { createRulesSchema } from './schemas';
import { ServerFacade } from '../../../types';
import { readRules } from '../alerts/read_rules';
import { transformOrError } from './utils';
import { transformOrError, transformError } from './utils';

export const createCreateRulesRoute: Hapi.ServerRoute = {
method: 'POST',
path: DETECTION_ENGINE_RULES_URL,
options: {
tags: ['access:signals-all'],
tags: ['access:siem'],
validate: {
options: {
abortEarly: false,
Expand Down Expand Up @@ -62,42 +62,46 @@ export const createCreateRulesRoute: Hapi.ServerRoute = {
return headers.response().code(404);
}

if (ruleId != null) {
const rule = await readRules({ alertsClient, ruleId });
if (rule != null) {
return new Boom(`rule_id ${ruleId} already exists`, { statusCode: 409 });
try {
if (ruleId != null) {
const rule = await readRules({ alertsClient, ruleId });
if (rule != null) {
return new Boom(`rule_id ${ruleId} already exists`, { statusCode: 409 });
}
}
}

const createdRule = await createRules({
alertsClient,
actionsClient,
description,
enabled,
falsePositives,
filter,
from,
immutable,
query,
language,
outputIndex,
savedId,
meta,
filters,
ruleId: ruleId != null ? ruleId : uuid.v4(),
index,
interval,
maxSignals,
riskScore,
name,
severity,
tags,
to,
type,
threats,
references,
});
return transformOrError(createdRule);
const createdRule = await createRules({
alertsClient,
actionsClient,
description,
enabled,
falsePositives,
filter,
from,
immutable,
query,
language,
outputIndex,
savedId,
meta,
filters,
ruleId: ruleId != null ? ruleId : uuid.v4(),
index,
interval,
maxSignals,
riskScore,
name,
severity,
tags,
to,
type,
threats,
references,
});
return transformOrError(createdRule);
} catch (err) {
return transformError(err);
}
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import { deleteRules } from '../alerts/delete_rules';
import { ServerFacade } from '../../../types';
import { queryRulesSchema } from './schemas';
import { QueryRequest } from '../alerts/types';
import { getIdError, transformOrError } from './utils';
import { getIdError, transformOrError, transformError } from './utils';

export const createDeleteRulesRoute: Hapi.ServerRoute = {
method: 'DELETE',
path: DETECTION_ENGINE_RULES_URL,
options: {
tags: ['access:signals-all'],
tags: ['access:siem'],
validate: {
options: {
abortEarly: false,
Expand All @@ -35,17 +35,21 @@ export const createDeleteRulesRoute: Hapi.ServerRoute = {
return headers.response().code(404);
}

const rule = await deleteRules({
actionsClient,
alertsClient,
id,
ruleId,
});
try {
const rule = await deleteRules({
actionsClient,
alertsClient,
id,
ruleId,
});

if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
}
} catch (err) {
return transformError(err);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import { findRules } from '../alerts/find_rules';
import { FindRulesRequest } from '../alerts/types';
import { findRulesSchema } from './schemas';
import { ServerFacade } from '../../../types';
import { transformFindAlertsOrError } from './utils';
import { transformFindAlertsOrError, transformError } from './utils';

export const createFindRulesRoute: Hapi.ServerRoute = {
method: 'GET',
path: `${DETECTION_ENGINE_RULES_URL}/_find`,
options: {
tags: ['access:signals-all'],
tags: ['access:siem'],
validate: {
options: {
abortEarly: false,
Expand All @@ -34,15 +34,19 @@ export const createFindRulesRoute: Hapi.ServerRoute = {
return headers.response().code(404);
}

const rules = await findRules({
alertsClient,
perPage: query.per_page,
page: query.page,
sortField: query.sort_field,
sortOrder: query.sort_order,
filter: query.filter,
});
return transformFindAlertsOrError(rules);
try {
const rules = await findRules({
alertsClient,
perPage: query.per_page,
page: query.page,
sortField: query.sort_field,
sortOrder: query.sort_order,
filter: query.filter,
});
return transformFindAlertsOrError(rules);
} catch (err) {
return transformError(err);
}
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import Hapi from 'hapi';
import { isFunction } from 'lodash/fp';
import { DETECTION_ENGINE_RULES_URL } from '../../../../common/constants';
import { getIdError, transformOrError } from './utils';
import { getIdError, transformOrError, transformError } from './utils';

import { readRules } from '../alerts/read_rules';
import { ServerFacade } from '../../../types';
Expand All @@ -18,7 +18,7 @@ export const createReadRulesRoute: Hapi.ServerRoute = {
method: 'GET',
path: DETECTION_ENGINE_RULES_URL,
options: {
tags: ['access:signals-all'],
tags: ['access:siem'],
validate: {
options: {
abortEarly: false,
Expand All @@ -34,15 +34,19 @@ export const createReadRulesRoute: Hapi.ServerRoute = {
if (!alertsClient || !actionsClient) {
return headers.response().code(404);
}
const rule = await readRules({
alertsClient,
id,
ruleId,
});
if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
try {
const rule = await readRules({
alertsClient,
id,
ruleId,
});
if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
}
} catch (err) {
return transformError(err);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import { updateRules } from '../alerts/update_rules';
import { UpdateRulesRequest } from '../alerts/types';
import { updateRulesSchema } from './schemas';
import { ServerFacade } from '../../../types';
import { getIdError, transformOrError } from './utils';
import { getIdError, transformOrError, transformError } from './utils';

export const createUpdateRulesRoute: Hapi.ServerRoute = {
method: 'PUT',
path: DETECTION_ENGINE_RULES_URL,
options: {
tags: ['access:signals-all'],
tags: ['access:siem'],
validate: {
options: {
abortEarly: false,
Expand Down Expand Up @@ -61,39 +61,43 @@ export const createUpdateRulesRoute: Hapi.ServerRoute = {
return headers.response().code(404);
}

const rule = await updateRules({
alertsClient,
actionsClient,
description,
enabled,
falsePositives,
filter,
from,
immutable,
query,
language,
outputIndex,
savedId,
meta,
filters,
id,
ruleId,
index,
interval,
maxSignals,
riskScore,
name,
severity,
tags,
to,
type,
threats,
references,
});
if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
try {
const rule = await updateRules({
alertsClient,
actionsClient,
description,
enabled,
falsePositives,
filter,
from,
immutable,
query,
language,
outputIndex,
savedId,
meta,
filters,
id,
ruleId,
index,
interval,
maxSignals,
riskScore,
name,
severity,
tags,
to,
type,
threats,
references,
});
if (rule != null) {
return transformOrError(rule);
} else {
return getIdError({ id, ruleId });
}
} catch (err) {
return transformError(err);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
*/

import Boom from 'boom';

import {
transformAlertToRule,
getIdError,
transformFindAlertsOrError,
transformOrError,
transformError,
} from './utils';
import { getResult } from './__mocks__/request_responses';

Expand Down Expand Up @@ -474,4 +476,41 @@ describe('utils', () => {
expect((output as Boom).message).toEqual('Internal error transforming');
});
});

describe('transformError', () => {
test('returns boom if it is a boom object', () => {
const boom = new Boom('');
const transformed = transformError(boom);
expect(transformed).toBe(boom);
});

test('returns a boom if it is some non boom object that has a statusCode', () => {
const error: Error & { statusCode?: number } = {
statusCode: 403,
name: 'some name',
message: 'some message',
};
const transformed = transformError(error);
expect(Boom.isBoom(transformed)).toBe(true);
});

test('returns a boom with the message set', () => {
const error: Error & { statusCode?: number } = {
statusCode: 403,
name: 'some name',
message: 'some message',
};
const transformed = transformError(error);
expect(transformed.message).toBe('some message');
});

test('does not return a boom if it is some non boom object but it does not have a status Code.', () => {
const error: Error = {
name: 'some name',
message: 'some message',
};
const transformed = transformError(error);
expect(Boom.isBoom(transformed)).toBe(false);
});
});
});
Loading

0 comments on commit 371ebd8

Please sign in to comment.