Skip to content

Commit

Permalink
only writes an outcome of 'success' if operation is an access operati…
Browse files Browse the repository at this point in the history 
…on, adds jest tests for when user is unauthorized to access given alert
  • Loading branch information
@dhurley14
dhurley14 committed Aug 9, 2021
1 parent ba1857e commit 36838ab
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 7 deletions.
14 changes: 11 additions & 3 deletions x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
WriteOperations,
AlertingAuthorizationEntity,
} from '../../../alerting/server';
import { Logger, ElasticsearchClient } from '../../../../../src/core/server';
import { Logger, ElasticsearchClient, EcsEventOutcome } from '../../../../../src/core/server';
import { alertAuditEvent, operationAlertAuditActionMap } from './audit_events';
import { AuditLogger } from '../../../security/server';
import {
Expand Down Expand Up @@ -102,6 +102,14 @@ export class AlertsClient {
this.spaceId = this.authorization.getSpaceId();
}

private getOutcome(
operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get
): { outcome: EcsEventOutcome } {
return {
outcome: operation === WriteOperations.Update ? 'unknown' : 'success',
};
}

/**
* Accepts an array of ES documents and executes ensureAuthorized for the given operation
* @param items
Expand Down Expand Up @@ -231,7 +239,7 @@ export class AlertsClient {
alertAuditEvent({
action: operationAlertAuditActionMap[operation],
id: item._id,
outcome: 'unknown',
...this.getOutcome(operation),
})
)
);
Expand Down Expand Up @@ -275,7 +283,7 @@ export class AlertsClient {
alertAuditEvent({
action: operationAlertAuditActionMap[operation],
id,
...(operation === WriteOperations.Update ? { outcome: 'unknown' } : { operation }),
...this.getOutcome(operation),
})
);
}
Expand Down
94 changes: 90 additions & 4 deletions x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
* 2.0.
*/

import { ALERT_OWNER, ALERT_STATUS, SPACE_IDS } from '@kbn/rule-data-utils';
import { ALERT_OWNER, ALERT_STATUS, RULE_ID, SPACE_IDS } from '@kbn/rule-data-utils';
import { AlertsClient, ConstructorOptions } from '../alerts_client';
import { loggingSystemMock } from '../../../../../../src/core/server/mocks';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock';
import { AuditLogger } from '../../../../security/server';
import { AlertingAuthorizationEntity } from '../../../../alerting/server';

const alertingAuthMock = alertingAuthorizationMock.create();
const esClientMock = elasticsearchClientMock.createElasticsearchClient();
Expand All @@ -26,13 +27,35 @@ const alertsClientParams: jest.Mocked<ConstructorOptions> = {
auditLogger,
};

const DEFAULT_SPACE = 'test_default_space_id';

beforeEach(() => {
jest.resetAllMocks();
alertingAuthMock.getSpaceId.mockImplementation(() => 'test_default_space_id');
alertingAuthMock.getSpaceId.mockImplementation(() => DEFAULT_SPACE);
// @ts-expect-error
alertingAuthMock.getAuthorizationFilter.mockImplementation(async () =>
Promise.resolve({ filter: [] })
);

alertingAuthMock.ensureAuthorized.mockImplementation(
// @ts-expect-error
async ({
ruleTypeId,
consumer,
operation,
entity,
}: {
ruleTypeId: string;
consumer: string;
operation: string;
entity: typeof AlertingAuthorizationEntity.Alert;
}) => {
if (ruleTypeId === 'apm.error_rate' && consumer === 'apm') {
return Promise.resolve();
}
return Promise.reject(new Error(`Unauthorized for ${ruleTypeId} and ${consumer}`));
}
);
});

describe('get()', () => {
Expand Down Expand Up @@ -177,8 +200,71 @@ describe('get()', () => {

expect(auditLogger.log).toHaveBeenCalledWith({
error: undefined,
event: { action: 'alert_get', category: ['database'], outcome: 'unknown', type: ['access'] },
message: 'User is accessing alert [id=NoxgpHkBqbdrfX07MqXV]',
event: { action: 'alert_get', category: ['database'], outcome: 'success', type: ['access'] },
message: 'User has accessed alert [id=NoxgpHkBqbdrfX07MqXV]',
});
});

test('audit error access if user is unauthorized for given alert', async () => {
const indexName = '.alerts-observability-apm.alerts';
const fakeAlertId = 'myfakeid1';
// fakeRuleTypeId will cause authz to fail
const fakeRuleTypeId = 'fake.rule';
const alertsClient = new AlertsClient(alertsClientParams);
esClientMock.search.mockResolvedValueOnce(
elasticsearchClientMock.createApiResponse({
body: {
took: 5,
timed_out: false,
_shards: {
total: 1,
successful: 1,
failed: 0,
skipped: 0,
},
hits: {
total: 1,
max_score: 999,
hits: [
{
found: true,
_type: 'alert',
_version: 1,
_seq_no: 362,
_primary_term: 2,
_id: fakeAlertId,
_index: indexName,
_source: {
[RULE_ID]: fakeRuleTypeId,
[ALERT_OWNER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
],
},
},
})
);

await expect(alertsClient.get({ id: fakeAlertId, index: '.alerts-observability-apm' })).rejects
.toThrowErrorMatchingInlineSnapshot(`
"Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"null\\" and operation get
Error: Error: Unauthorized for fake.rule and apm"
`);

expect(auditLogger.log).toHaveBeenLastCalledWith({
message: `Failed attempt to access alert [id=${fakeAlertId}]`,
event: {
action: 'alert_get',
category: ['database'],
outcome: 'failure',
type: ['access'],
},
error: {
code: 'Error',
message: 'Unauthorized for fake.rule and apm',
},
});
});

Expand Down
90 changes: 90 additions & 0 deletions x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { loggingSystemMock } from '../../../../../../src/core/server/mocks';
import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock';
import { AuditLogger } from '../../../../security/server';
import { AlertingAuthorizationEntity } from '../../../../alerting/server';

const alertingAuthMock = alertingAuthorizationMock.create();
const esClientMock = elasticsearchClientMock.createElasticsearchClient();
Expand All @@ -35,6 +36,26 @@ beforeEach(() => {
alertingAuthMock.getAuthorizationFilter.mockImplementation(async () =>
Promise.resolve({ filter: [] })
);

alertingAuthMock.ensureAuthorized.mockImplementation(
// @ts-expect-error
async ({
ruleTypeId,
consumer,
operation,
entity,
}: {
ruleTypeId: string;
consumer: string;
operation: string;
entity: typeof AlertingAuthorizationEntity.Alert;
}) => {
if (ruleTypeId === 'apm.error_rate' && consumer === 'apm') {
return Promise.resolve();
}
return Promise.reject(new Error(`Unauthorized for ${ruleTypeId} and ${consumer}`));
}
);
});

describe('update()', () => {
Expand Down Expand Up @@ -191,6 +212,75 @@ describe('update()', () => {
});
});

test('audit error update if user is unauthorized for given alert', async () => {
const indexName = '.alerts-observability-apm.alerts';
const fakeAlertId = 'myfakeid1';
// fakeRuleTypeId will cause authz to fail
const fakeRuleTypeId = 'fake.rule';
const alertsClient = new AlertsClient(alertsClientParams);
esClientMock.search.mockResolvedValueOnce(
elasticsearchClientMock.createApiResponse({
body: {
took: 5,
timed_out: false,
_shards: {
total: 1,
successful: 1,
failed: 0,
skipped: 0,
},
hits: {
total: 1,
max_score: 999,
hits: [
{
found: true,
_type: 'alert',
_version: 1,
_seq_no: 362,
_primary_term: 2,
_id: fakeAlertId,
_index: indexName,
_source: {
[RULE_ID]: fakeRuleTypeId,
[ALERT_OWNER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
],
},
},
})
);

await expect(
alertsClient.update({
id: fakeAlertId,
status: 'closed',
_version: '1',
index: '.alerts-observability-apm',
})
).rejects.toThrowErrorMatchingInlineSnapshot(`
"Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"null\\" and operation update
Error: Error: Unauthorized for fake.rule and apm"
`);

expect(auditLogger.log).toHaveBeenLastCalledWith({
message: `Failed attempt to update alert [id=${fakeAlertId}]`,
event: {
action: 'alert_update',
category: ['database'],
outcome: 'failure',
type: ['change'],
},
error: {
code: 'Error',
message: 'Unauthorized for fake.rule and apm',
},
});
});

test(`throws an error if ES client get fails`, async () => {
const error = new Error('something went wrong on update');
const alertsClient = new AlertsClient(alertsClientParams);
Expand Down

0 comments on commit 36838ab

Please sign in to comment.