Skip to content

Commit

Permalink
execute ensure authorized in the search after loop, adds jest tests t…
Browse files Browse the repository at this point in the history 
…o make sure we log an error to the audit log for the case when ensureAuthorized throws an authorization error
  • Loading branch information
@dhurley14
dhurley14 committed Aug 4, 2021
1 parent 65ba2e1 commit db2453b
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 27 deletions.
91 changes: 64 additions & 27 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 @@ -102,6 +102,65 @@ export class AlertsClient {
this.spaceId = this.authorization.getSpaceId();
}

private async ensureAllAuthorized(
items: Array<{
_id: string;
// this is typed kind of crazy to fit the output of es api response to this
_source?:
| { [RULE_ID]?: string | null | undefined; [ALERT_OWNER]?: string | null | undefined }
| null
| undefined;
}>,
operation: ReadOperations.Find | ReadOperations.Get | WriteOperations.Update
) {
const { hitIds, ownersAndRuleTypeIds } = items.reduce(
(acc, hit) => ({
hitIds: [hit._id, ...acc.hitIds],
ownersAndRuleTypeIds: [
{
[RULE_ID]: hit?._source?.[RULE_ID],
[ALERT_OWNER]: hit?._source?.[ALERT_OWNER],
},
],
}),
{ hitIds: [], ownersAndRuleTypeIds: [] } as {
hitIds: string[];
ownersAndRuleTypeIds: Array<{
[RULE_ID]: string | null | undefined;
[ALERT_OWNER]: string | null | undefined;
}>;
}
);

const assertString = (hit: unknown): hit is string => hit !== null && hit !== undefined;

return Promise.all(
ownersAndRuleTypeIds.map((hit) => {
const alertOwner = hit?.[ALERT_OWNER];
const ruleId = hit?.[RULE_ID];
if (hit != null && assertString(alertOwner) && assertString(ruleId)) {
return this.authorization.ensureAuthorized({
ruleTypeId: ruleId,
consumer: alertOwner,
operation,
entity: AlertingAuthorizationEntity.Alert,
});
}
})
).catch((error) => {
for (const hitId of hitIds) {
this.auditLogger?.log(
alertAuditEvent({
action: operationAlertAuditActionMap[operation],
id: hitId,
error,
})
);
}
throw error;
});
}

/**
* This will be used as a part of the "find" api
* In the future we will add an "aggs" param
Expand Down Expand Up @@ -159,6 +218,8 @@ export class AlertsClient {
throw Boom.badData(errorMessage);
}

await this.ensureAllAuthorized(result.body.hits.hits, operation);

result?.body.hits.hits.map((item) =>
this.auditLogger?.log(
alertAuditEvent({
Expand Down Expand Up @@ -200,33 +261,9 @@ export class AlertsClient {
ids,
},
});
await Promise.all(
mgetRes.body.docs.map((item) => {
if (
item._source != null &&
item._source[RULE_ID] != null &&
item._source[ALERT_OWNER] != null
) {
return this.authorization.ensureAuthorized({
ruleTypeId: item._source[RULE_ID],
consumer: item._source[ALERT_OWNER],
operation,
entity: AlertingAuthorizationEntity.Alert,
});
}
})
).catch((error) => {
for (const id of ids) {
this.auditLogger?.log(
alertAuditEvent({
action: operationAlertAuditActionMap[operation],
id,
error,
})
);
}
throw error;
});

await this.ensureAllAuthorized(mgetRes.body.docs, operation);

for (const id of ids) {
this.auditLogger?.log(
alertAuditEvent({
Expand Down
114 changes: 114 additions & 0 deletions x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,55 @@ describe('bulkUpdate()', () => {
error: undefined,
});
});

test('audit error access if user is unauthorized for given alert', async () => {
const fakeAlertId = 'myfakeid1';
const indexName = '.alerts-observability-apm.alerts';
const alertsClient = new AlertsClient(alertsClientParams);
esClientMock.mget.mockResolvedValueOnce(
elasticsearchClientMock.createApiResponse({
body: {
docs: [
{
_id: fakeAlertId,
_index: indexName,
_source: {
[RULE_ID]: 'apm.error_rate',
[ALERT_OWNER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
],
},
})
);
alertingAuthMock.ensureAuthorized.mockRejectedValueOnce(
new Error('bulk update by ids test error')
);
await expect(
alertsClient.bulkUpdate({
ids: [fakeAlertId],
query: undefined,
index: indexName,
status: 'closed',
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"bulk update by ids test error"`);

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: 'bulk update by ids test error',
},
});
});
// test('throws an error if ES client fetch fails', async () => {});
// test('throws an error if ES client bulk update fails', async () => {});
// test('throws an error if ES client updateByQuery fails', async () => {});
Expand Down Expand Up @@ -161,6 +210,71 @@ describe('bulkUpdate()', () => {
error: undefined,
});
});

test('audit error access if user is unauthorized for given alert', async () => {
const fakeAlertId = 'myfakeid1';
const indexName = '.alerts-observability-apm.alerts';
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: [
{
_id: fakeAlertId,
_index: '.alerts-observability-apm.alerts',
_source: {
[RULE_ID]: 'apm.error_rate',
[ALERT_OWNER]: 'apm',
[ALERT_STATUS]: 'open',
[SPACE_IDS]: [DEFAULT_SPACE],
},
},
],
},
},
})
);
alertingAuthMock.ensureAuthorized.mockRejectedValueOnce(
new Error('bulk update by query test error')
);
await expect(
alertsClient.bulkUpdate({
ids: undefined,
query: `${ALERT_STATUS}: open`,
index: indexName,
status: 'closed',
})
).rejects.toThrowErrorMatchingInlineSnapshot(`
"queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update
Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update
Error: Error: bulk update by query test error"
`);

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: 'bulk update by query test error',
},
});
});
// test('throws an error if ES client fetch fails', async () => {});
// test('throws an error if ES client bulk update fails', async () => {});
// test('throws an error if ES client updateByQuery fails', async () => {});
Expand Down

0 comments on commit db2453b

Please sign in to comment.