forked from elastic/kibana
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #5 from yctercero/cleanup
Cleans up PR: removes console log statements removes auth audit logger in rule registry as this will be taken care of in alerting adds audit event logger for our client adds tests for client, client factory, utils, routes
- Loading branch information
Showing
15 changed files
with
923 additions
and
587 deletions.
There are no files selected for viewing
400 changes: 0 additions & 400 deletions
400
x-pack/plugins/rule_registry/server/alert_data_client/alert_client.ts
This file was deleted.
Oops, something went wrong.
27 changes: 27 additions & 0 deletions
27
x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import type { PublicMethodsOf } from '@kbn/utility-types'; | ||
import { AlertsClient } from './alerts_client'; | ||
|
||
type Schema = PublicMethodsOf<AlertsClient>; | ||
export type AlertsClientMock = jest.Mocked<Schema>; | ||
|
||
const createAlertsClientMock = () => { | ||
const mocked: AlertsClientMock = { | ||
get: jest.fn(), | ||
getAlertsIndex: jest.fn(), | ||
update: jest.fn(), | ||
}; | ||
return mocked; | ||
}; | ||
|
||
export const alertsClientMock: { | ||
create: () => AlertsClientMock; | ||
} = { | ||
create: createAlertsClientMock, | ||
}; |
193 changes: 193 additions & 0 deletions
193
x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,193 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
import { PublicMethodsOf } from '@kbn/utility-types'; | ||
import { AlertTypeParams } from '../../../alerting/server'; | ||
import { | ||
ReadOperations, | ||
AlertingAuthorization, | ||
WriteOperations, | ||
AlertingAuthorizationEntity, | ||
// eslint-disable-next-line @kbn/eslint/no-restricted-paths | ||
} from '../../../alerting/server/authorization'; | ||
import { Logger, ElasticsearchClient } from '../../../../../src/core/server'; | ||
import { alertAuditEvent, AlertAuditAction } from './audit_events'; | ||
import { RuleDataPluginService } from '../rule_data_plugin_service'; | ||
import { AuditLogger } from '../../../security/server'; | ||
import { ParsedTechnicalFields } from '../../common/parse_technical_fields'; | ||
|
||
export interface ConstructorOptions { | ||
logger: Logger; | ||
authorization: PublicMethodsOf<AlertingAuthorization>; | ||
auditLogger?: AuditLogger; | ||
esClient: ElasticsearchClient; | ||
ruleDataService: RuleDataPluginService; | ||
} | ||
|
||
export interface UpdateOptions<Params extends AlertTypeParams> { | ||
id: string; | ||
data: { | ||
status: string; | ||
}; | ||
// observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 | ||
assetName: string; | ||
} | ||
|
||
interface GetAlertParams { | ||
id: string; | ||
// observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 | ||
assetName: string; | ||
} | ||
|
||
export class AlertsClient { | ||
private readonly logger: Logger; | ||
private readonly auditLogger?: AuditLogger; | ||
private readonly authorization: PublicMethodsOf<AlertingAuthorization>; | ||
private readonly esClient: ElasticsearchClient; | ||
private readonly ruleDataService: RuleDataPluginService; | ||
|
||
constructor({ | ||
auditLogger, | ||
authorization, | ||
logger, | ||
esClient, | ||
ruleDataService, | ||
}: ConstructorOptions) { | ||
this.logger = logger; | ||
this.authorization = authorization; | ||
this.esClient = esClient; | ||
this.auditLogger = auditLogger; | ||
this.ruleDataService = ruleDataService; | ||
} | ||
|
||
/** | ||
* we are "hard coding" this string similar to how rule registry is doing it | ||
* x-pack/plugins/apm/server/plugin.ts:191 | ||
*/ | ||
public getAlertsIndex(assetName: string) { | ||
return this.ruleDataService?.getFullAssetName(assetName); | ||
} | ||
|
||
private async fetchAlert({ id, assetName }: GetAlertParams): Promise<ParsedTechnicalFields> { | ||
try { | ||
const result = await this.esClient.get<ParsedTechnicalFields>({ | ||
index: this.getAlertsIndex(assetName), | ||
id, | ||
}); | ||
|
||
if ( | ||
result == null || | ||
result.body == null || | ||
result.body._source == null || | ||
result.body._source['rule.id'] == null || | ||
result.body._source['kibana.rac.alert.owner'] == null | ||
) { | ||
const errorMessage = `[rac] - Unable to retrieve alert details for alert with id of "${id}".`; | ||
this.logger.debug(errorMessage); | ||
throw new Error(errorMessage); | ||
} | ||
|
||
return result.body._source; | ||
} catch (error) { | ||
const errorMessage = `[rac] - Unable to retrieve alert with id of "${id}".`; | ||
this.logger.debug(errorMessage); | ||
throw error; | ||
} | ||
} | ||
|
||
public async get({ id, assetName }: GetAlertParams): Promise<ParsedTechnicalFields> { | ||
try { | ||
// first search for the alert by id, then use the alert info to check if user has access to it | ||
const alert = await this.fetchAlert({ | ||
id, | ||
assetName, | ||
}); | ||
|
||
// this.authorization leverages the alerting plugin's authorization | ||
// client exposed to us for reuse | ||
await this.authorization.ensureAuthorized({ | ||
ruleTypeId: alert['rule.id'], | ||
consumer: alert['kibana.rac.alert.owner'], | ||
operation: ReadOperations.Get, | ||
entity: AlertingAuthorizationEntity.Alert, | ||
}); | ||
|
||
this.auditLogger?.log( | ||
alertAuditEvent({ | ||
action: AlertAuditAction.GET, | ||
id, | ||
}) | ||
); | ||
|
||
return alert; | ||
} catch (error) { | ||
this.logger.debug(`[rac] - Error fetching alert with id of "${id}"`); | ||
this.auditLogger?.log( | ||
alertAuditEvent({ | ||
action: AlertAuditAction.GET, | ||
id, | ||
error, | ||
}) | ||
); | ||
throw error; | ||
} | ||
} | ||
|
||
public async update<Params extends AlertTypeParams = never>({ | ||
id, | ||
data, | ||
assetName, | ||
}: UpdateOptions<Params>): Promise<ParsedTechnicalFields | null | undefined> { | ||
try { | ||
// TODO: use MGET | ||
const alert = await this.fetchAlert({ | ||
id, | ||
assetName, | ||
}); | ||
|
||
await this.authorization.ensureAuthorized({ | ||
ruleTypeId: alert['rule.id'], | ||
consumer: alert['kibana.rac.alert.owner'], | ||
operation: WriteOperations.Update, | ||
entity: AlertingAuthorizationEntity.Alert, | ||
}); | ||
|
||
const index = this.getAlertsIndex(assetName); | ||
|
||
const updateParameters = { | ||
id, | ||
index, | ||
body: { | ||
doc: { | ||
'kibana.rac.alert.status': data.status, | ||
}, | ||
}, | ||
}; | ||
|
||
const res = await this.esClient.update<ParsedTechnicalFields, unknown, unknown, unknown>( | ||
updateParameters | ||
); | ||
|
||
this.auditLogger?.log( | ||
alertAuditEvent({ | ||
action: AlertAuditAction.UPDATE, | ||
id, | ||
}) | ||
); | ||
|
||
return res.body.get?._source; | ||
} catch (error) { | ||
this.auditLogger?.log( | ||
alertAuditEvent({ | ||
action: AlertAuditAction.UPDATE, | ||
id, | ||
error, | ||
}) | ||
); | ||
throw error; | ||
} | ||
} | ||
} |
94 changes: 94 additions & 0 deletions
94
x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { Request } from '@hapi/hapi'; | ||
|
||
import { AlertsClientFactory, AlertsClientFactoryProps } from './alerts_client_factory'; | ||
import { ElasticsearchClient, KibanaRequest } from 'src/core/server'; | ||
import { loggingSystemMock } from 'src/core/server/mocks'; | ||
import { securityMock } from '../../../security/server/mocks'; | ||
import { AuditLogger } from '../../../security/server'; | ||
import { ruleDataPluginServiceMock } from '../rule_data_plugin_service/rule_data_plugin_service.mock'; | ||
import { alertingAuthorizationMock } from '../../../alerting/server/authorization/alerting_authorization.mock'; | ||
import { RuleDataPluginServiceConstructorOptions } from '../rule_data_plugin_service'; | ||
|
||
jest.mock('./alerts_client'); | ||
|
||
const securityPluginSetup = securityMock.createSetup(); | ||
const ruleDataServiceMock = ruleDataPluginServiceMock.create( | ||
{} as RuleDataPluginServiceConstructorOptions | ||
); | ||
const alertingAuthMock = alertingAuthorizationMock.create(); | ||
|
||
const alertsClientFactoryParams: jest.Mocked<AlertsClientFactoryProps> = { | ||
logger: loggingSystemMock.create().get(), | ||
getAlertingAuthorization: (_: KibanaRequest) => alertingAuthMock, | ||
securityPluginSetup, | ||
esClient: {} as ElasticsearchClient, | ||
ruleDataService: ruleDataServiceMock, | ||
}; | ||
|
||
const fakeRequest = ({ | ||
app: {}, | ||
headers: {}, | ||
getBasePath: () => '', | ||
path: '/', | ||
route: { settings: {} }, | ||
url: { | ||
href: '/', | ||
}, | ||
raw: { | ||
req: { | ||
url: '/', | ||
}, | ||
}, | ||
} as unknown) as Request; | ||
|
||
const auditLogger = { | ||
log: jest.fn(), | ||
} as jest.Mocked<AuditLogger>; | ||
|
||
beforeEach(() => { | ||
jest.resetAllMocks(); | ||
|
||
securityPluginSetup.audit.asScoped.mockReturnValue(auditLogger); | ||
}); | ||
|
||
test('creates an alerts client with proper constructor arguments', async () => { | ||
const factory = new AlertsClientFactory(); | ||
factory.initialize({ ...alertsClientFactoryParams }); | ||
const request = KibanaRequest.from(fakeRequest); | ||
await factory.create(request); | ||
|
||
expect(jest.requireMock('./alerts_client').AlertsClient).toHaveBeenCalledWith({ | ||
authorization: alertingAuthMock, | ||
logger: alertsClientFactoryParams.logger, | ||
auditLogger, | ||
esClient: {}, | ||
ruleDataService: ruleDataServiceMock, | ||
}); | ||
}); | ||
|
||
test('throws an error if already initialized', () => { | ||
const factory = new AlertsClientFactory(); | ||
factory.initialize({ ...alertsClientFactoryParams }); | ||
|
||
expect(() => | ||
factory.initialize({ ...alertsClientFactoryParams }) | ||
).toThrowErrorMatchingInlineSnapshot(`"AlertsClientFactory (RAC) already initialized"`); | ||
}); | ||
|
||
test('throws an error if ruleDataService not available', () => { | ||
const factory = new AlertsClientFactory(); | ||
|
||
expect(() => | ||
factory.initialize({ | ||
...alertsClientFactoryParams, | ||
ruleDataService: null, | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot(`"Rule registry data service required for alerts client"`); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.