From 04ba8513c3323a5abd4b87ef672e3a9d64e29eff Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 30 Oct 2020 16:03:29 -0700 Subject: [PATCH 01/26] Used SO for saving the API key IDs that should be deleted and create a configuration option where can set an execution interval for a TM task which will get the data from this SO and remove marked for delete keys. --- .../server/alerts_client/alerts_client.ts | 14 +- x-pack/plugins/alerts/server/config.test.ts | 19 +++ x-pack/plugins/alerts/server/config.ts | 16 +++ x-pack/plugins/alerts/server/index.ts | 8 +- .../invalidate_pending_api_keys/task.ts | 134 ++++++++++++++++++ x-pack/plugins/alerts/server/plugin.test.ts | 19 ++- x-pack/plugins/alerts/server/plugin.ts | 48 ++++++- .../alerts/server/saved_objects/index.ts | 8 ++ .../mappings_invalidate_api_key.json | 9 ++ x-pack/plugins/alerts/server/types.ts | 10 ++ .../alert_types/index_threshold/alert_type.ts | 6 + 11 files changed, 281 insertions(+), 10 deletions(-) create mode 100644 x-pack/plugins/alerts/server/config.test.ts create mode 100644 x-pack/plugins/alerts/server/config.ts create mode 100644 x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts create mode 100644 x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 88abce7298622..33da41b15c272 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -629,10 +629,16 @@ export class AlertsClient { try { const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; - const response = await this.invalidateAPIKey({ id: apiKeyId }); - if (response.apiKeysEnabled === true && response.result.error_count > 0) { - this.logger.error(`Failed to invalidate API Key [id="${apiKeyId}"]`); - } + // const response = await this.invalidateAPIKey({ id: apiKeyId }); + // if (response.apiKeysEnabled === true && response.result.error_count > 0) { + // this.logger.error(`Failed to invalidate API Key [id="${apiKeyId}"]`); + // } + const markedToDelete = await this.unsecuredSavedObjectsClient.create( + 'invalidatePendingApiKey', + { + apiKeyId, + } + ); } catch (e) { this.logger.error(`Failed to invalidate API Key: ${e.message}`); } diff --git a/x-pack/plugins/alerts/server/config.test.ts b/x-pack/plugins/alerts/server/config.test.ts new file mode 100644 index 0000000000000..11afb9c7bc193 --- /dev/null +++ b/x-pack/plugins/alerts/server/config.test.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { configSchema } from './config'; + +describe('config validation', () => { + test('alerts defaults', () => { + const config: Record = {}; + expect(configSchema.validate(config)).toMatchInlineSnapshot(` + Object { + "invalidateApiKeysTask": Object { + "interval": "5m", + }, + } + `); + }); +}); diff --git a/x-pack/plugins/alerts/server/config.ts b/x-pack/plugins/alerts/server/config.ts new file mode 100644 index 0000000000000..b762e947ca9fe --- /dev/null +++ b/x-pack/plugins/alerts/server/config.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { schema, TypeOf } from '@kbn/config-schema'; +import { validateDurationSchema } from './lib'; + +export const configSchema = schema.object({ + invalidateApiKeysTask: schema.object({ + interval: schema.string({ validate: validateDurationSchema, defaultValue: '5m' }), + }), +}); + +export type AlertsConfig = TypeOf; diff --git a/x-pack/plugins/alerts/server/index.ts b/x-pack/plugins/alerts/server/index.ts index 1e442c5196cf2..7d905be4a5186 100644 --- a/x-pack/plugins/alerts/server/index.ts +++ b/x-pack/plugins/alerts/server/index.ts @@ -5,8 +5,9 @@ */ import type { PublicMethodsOf } from '@kbn/utility-types'; import { AlertsClient as AlertsClientClass } from './alerts_client'; -import { PluginInitializerContext } from '../../../../src/core/server'; +import { PluginConfigDescriptor, PluginInitializerContext } from '../../../../src/core/server'; import { AlertingPlugin } from './plugin'; +import { configSchema } from './config'; export type AlertsClient = PublicMethodsOf; @@ -28,5 +29,10 @@ export { PluginSetupContract, PluginStartContract } from './plugin'; export { FindResult } from './alerts_client'; export { AlertInstance } from './alert_instance'; export { parseDuration } from './lib'; +import { AlertsConfigType } from './types'; export const plugin = (initContext: PluginInitializerContext) => new AlertingPlugin(initContext); + +export const config: PluginConfigDescriptor = { + schema: configSchema, +}; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts new file mode 100644 index 0000000000000..c09d894f5030a --- /dev/null +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -0,0 +1,134 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Logger, ISavedObjectsRepository } from 'kibana/server'; +import { InvalidateAPIKeyParams } from '../../../security/server'; +import { + RunContext, + TaskManagerSetupContract, + TaskManagerStartContract, +} from '../../../task_manager/server'; +import { InvalidateAPIKeyResult } from '../alerts_client'; +import { AlertsConfig } from '../config'; +import { InvalidatePendingApiKey } from '../types'; + +const TASK_TYPE = 'alerts_invalidate_api_keys'; +export const TASK_ID = `Alerts-${TASK_TYPE}`; + +export function initializeAlertsInvalidateApiKeys( + logger: Logger, + internalSavedObjectsRepository: Promise<{ + createInternalRepository: () => ISavedObjectsRepository; + }>, + taskManager: TaskManagerSetupContract, + invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise +) { + registerAlertsInvalidateApiKeysTask( + logger, + internalSavedObjectsRepository, + taskManager, + invalidateAPIKey + ); +} + +export function scheduleAlertsInvalidateApiKeys( + logger: Logger, + config: Promise, + taskManager?: TaskManagerStartContract +) { + if (taskManager) { + scheduleTasks(logger, taskManager, config); + } +} + +function registerAlertsInvalidateApiKeysTask( + logger: Logger, + internalSavedObjectsRepository: Promise<{ + createInternalRepository: () => ISavedObjectsRepository; + }>, + taskManager: TaskManagerSetupContract, + invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise +) { + taskManager.registerTaskDefinitions({ + [TASK_TYPE]: { + title: 'Invalidate Alerts API Keys', + createTaskRunner: taskRunner(logger, internalSavedObjectsRepository, invalidateAPIKey), + }, + }); +} + +async function scheduleTasks( + logger: Logger, + taskManager: TaskManagerStartContract, + config: Promise +) { + const interval = (await config).invalidateApiKeysTask.interval; + try { + await taskManager.ensureScheduled({ + id: TASK_ID, + taskType: TASK_TYPE, + schedule: { + interval, + }, + state: {}, + params: {}, + }); + } catch (e) { + logger.debug(`Error scheduling task, received ${e.message}`); + } +} + +function taskRunner( + logger: Logger, + internalSavedObjectsRepository: Promise<{ + createInternalRepository: () => ISavedObjectsRepository; + }>, + invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise +) { + return ({ taskInstance }: RunContext) => { + const { state } = taskInstance; + return { + async run() { + try { + const repository = (await internalSavedObjectsRepository).createInternalRepository(); + const res = await repository.find({ + type: 'invalidatePendingApiKey', + }); + res.saved_objects.forEach(async (obj) => { + const response = await invalidateAPIKey({ id: obj.attributes.apiKeyId }); + if (response.apiKeysEnabled === true && response.result.error_count > 0) { + logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); + } else { + try { + await repository.delete('invalidatePendingApiKey', obj.id); + } catch (err) { + // Skip the cleanup error + logger.error( + `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` + ); + } + } + }); + // TODO: clean all + return { + state: { + runs: (state.runs || 0) + 1, + total_removed: 0, + }, + }; + } catch (errMsg) { + logger.warn(`Error executing alerting health check task: ${errMsg}`); + return { + state: { + runs: (state.runs || 0) + 1, + total_removed: 0, + }, + }; + } + }, + }; + }; +} diff --git a/x-pack/plugins/alerts/server/plugin.test.ts b/x-pack/plugins/alerts/server/plugin.test.ts index ece7d31d2f7fd..bbded1a105d65 100644 --- a/x-pack/plugins/alerts/server/plugin.test.ts +++ b/x-pack/plugins/alerts/server/plugin.test.ts @@ -13,11 +13,16 @@ import { eventLogServiceMock } from '../../event_log/server/event_log_service.mo import { KibanaRequest, CoreSetup } from 'kibana/server'; import { featuresPluginMock } from '../../features/server/mocks'; import { KibanaFeature } from '../../features/server'; +import { AlertsConfig } from './config'; describe('Alerting Plugin', () => { describe('setup()', () => { it('should log warning when Encrypted Saved Objects plugin is using an ephemeral encryption key', async () => { - const context = coreMock.createPluginInitializerContext(); + const context = coreMock.createPluginInitializerContext({ + invalidateApiKeysTask: { + interval: '5m', + }, + }); const plugin = new AlertingPlugin(context); const coreSetup = coreMock.createSetup(); @@ -55,7 +60,11 @@ describe('Alerting Plugin', () => { */ describe('getAlertsClientWithRequest()', () => { it('throws error when encryptedSavedObjects plugin has usingEphemeralEncryptionKey set to true', async () => { - const context = coreMock.createPluginInitializerContext(); + const context = coreMock.createPluginInitializerContext({ + invalidateApiKeysTask: { + interval: '5m', + }, + }); const plugin = new AlertingPlugin(context); const coreSetup = coreMock.createSetup(); @@ -98,7 +107,11 @@ describe('Alerting Plugin', () => { }); it(`doesn't throw error when encryptedSavedObjects plugin has usingEphemeralEncryptionKey set to false`, async () => { - const context = coreMock.createPluginInitializerContext(); + const context = coreMock.createPluginInitializerContext({ + invalidateApiKeysTask: { + interval: '5m', + }, + }); const plugin = new AlertingPlugin(context); const coreSetup = coreMock.createSetup(); diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 565b2992b1f7a..06dd7a418f90a 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -6,14 +6,14 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import { first, map } from 'rxjs/operators'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; -import { SecurityPluginSetup } from '../../security/server'; +import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../security/server'; import { EncryptedSavedObjectsPluginSetup, EncryptedSavedObjectsPluginStart, } from '../../encrypted_saved_objects/server'; import { TaskManagerSetupContract, TaskManagerStartContract } from '../../task_manager/server'; import { SpacesPluginSetup, SpacesServiceSetup } from '../../spaces/server'; -import { AlertsClient } from './alerts_client'; +import { AlertsClient, InvalidateAPIKeyResult } from './alerts_client'; import { AlertTypeRegistry } from './alert_type_registry'; import { TaskRunnerFactory } from './task_runner'; import { AlertsClientFactory } from './alerts_client_factory'; @@ -61,6 +61,11 @@ import { initializeAlertingTelemetry, scheduleAlertingTelemetry } from './usage/ import { IEventLogger, IEventLogService, IEventLogClientService } from '../../event_log/server'; import { PluginStartContract as FeaturesPluginStart } from '../../features/server'; import { setupSavedObjects } from './saved_objects'; +import { + initializeAlertsInvalidateApiKeys, + scheduleAlertsInvalidateApiKeys, +} from './invalidate_pending_api_keys/task'; +import { AlertsConfig } from './config'; export const EVENT_LOG_PROVIDER = 'alerting'; export const EVENT_LOG_ACTIONS = { @@ -98,6 +103,7 @@ export interface AlertingPluginsStart { } export class AlertingPlugin { + private readonly config: Promise; private readonly logger: Logger; private alertTypeRegistry?: AlertTypeRegistry; private readonly taskRunnerFactory: TaskRunnerFactory; @@ -114,6 +120,7 @@ export class AlertingPlugin { private eventLogger?: IEventLogger; constructor(initializerContext: PluginInitializerContext) { + this.config = initializerContext.config.create().pipe(first()).toPromise(); this.logger = initializerContext.logger.get('plugins', 'alerting'); this.taskRunnerFactory = new TaskRunnerFactory(); this.alertsClientFactory = new AlertsClientFactory(); @@ -185,6 +192,32 @@ export class AlertingPlugin { }); } + const invalidateAPIKey = async ( + params: InvalidateAPIKeyParams + ): Promise => { + if (!this.security) { + return { apiKeysEnabled: false }; + } + const invalidateAPIKeyResult = await this.security.authc.invalidateAPIKeyAsInternalUser( + params + ); + // Null when Elasticsearch security is disabled + if (!invalidateAPIKeyResult) { + return { apiKeysEnabled: false }; + } + return { + apiKeysEnabled: true, + result: invalidateAPIKeyResult, + }; + }; + + initializeAlertsInvalidateApiKeys( + this.logger, + this.createSOInternalRepositoryContext(core), + plugins.taskManager, + invalidateAPIKey + ); + core.http.registerRouteHandlerContext('alerting', this.createRouteHandlerContext(core)); // Routes @@ -274,12 +307,23 @@ export class AlertingPlugin { scheduleAlertingTelemetry(this.telemetryLogger, plugins.taskManager); + scheduleAlertsInvalidateApiKeys(this.telemetryLogger, this.config, plugins.taskManager); + return { listTypes: alertTypeRegistry!.list.bind(this.alertTypeRegistry!), getAlertsClientWithRequest, }; } + private createSOInternalRepositoryContext = async (core: CoreSetup) => { + const [{ savedObjects }] = await core.getStartServices(); + return { + createInternalRepository: () => { + return savedObjects.createInternalRepository(['alert']); + }, + }; + }; + private createRouteHandlerContext = ( core: CoreSetup ): IContextProvider, 'alerting'> => { diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 9aa1f86676eaa..3052da0bd7b25 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -6,6 +6,7 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; +import mappingsInvalidateApiKey from './mappings_invalidate_api_key.json'; import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; @@ -42,6 +43,13 @@ export function setupSavedObjects( mappings: mappings.alert, }); + savedObjects.registerType({ + name: 'invalidatePendingApiKey', + hidden: true, + namespaceType: 'single', + mappings: mappingsInvalidateApiKey.invalidatePendingApiKey, + }); + // Encrypted attributes encryptedSavedObjects.registerType({ type: 'alert', diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json b/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json new file mode 100644 index 0000000000000..f845c992adc8f --- /dev/null +++ b/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json @@ -0,0 +1,9 @@ +{ + "invalidatePendingApiKey": { + "properties": { + "apiKeyId": { + "type": "keyword" + } + } + } +} diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 42eef9bba10e5..29ae2678f91fe 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -172,4 +172,14 @@ export interface AlertingPlugin { start: PluginStartContract; } +export interface AlertsConfigType { + healthCheck: { + interval: string; + }; +} + +export interface InvalidatePendingApiKey { + apiKeyId: string; +} + export type AlertTypeRegistry = PublicMethodsOf; diff --git a/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts b/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts index 2a1ed429b7fe1..b664f54669bc5 100644 --- a/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts +++ b/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts @@ -125,6 +125,12 @@ export function getAlertType(service: Service): AlertType { + setTimeout(resolve, ms); + }); + } + await sleep(5000); const callCluster = services.callCluster; const date = new Date().toISOString(); From 8c149fc374750304534f0368f6c46b6f95857cb2 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Sun, 1 Nov 2020 21:58:40 -0800 Subject: [PATCH 02/26] removed invalidateApiKey from AlertsClient --- .../alerts/server/alerts_client.mock.ts | 1 + .../server/alerts_client/alerts_client.ts | 23 ++++--------------- .../server/alerts_client/tests/create.test.ts | 3 +-- .../server/alerts_client/tests/delete.test.ts | 13 +++++------ .../alerts_client/tests/disable.test.ts | 13 +++++------ .../server/alerts_client/tests/enable.test.ts | 7 +++--- .../server/alerts_client/tests/find.test.ts | 1 - .../server/alerts_client/tests/get.test.ts | 1 - .../tests/get_alert_instance_summary.test.ts | 1 - .../tests/get_alert_state.test.ts | 1 - .../tests/list_alert_types.test.ts | 1 - .../alerts_client/tests/mute_all.test.ts | 1 - .../alerts_client/tests/mute_instance.test.ts | 1 - .../alerts_client/tests/unmute_all.test.ts | 1 - .../tests/unmute_instance.test.ts | 1 - .../server/alerts_client/tests/update.test.ts | 7 +++--- .../tests/update_api_key.test.ts | 13 +++++------ .../alerts_client_conflict_retries.test.ts | 5 ++-- .../alerts/server/alerts_client_factory.ts | 18 +-------------- 19 files changed, 34 insertions(+), 78 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client.mock.ts b/x-pack/plugins/alerts/server/alerts_client.mock.ts index c9063457fac80..541d7f678d47b 100644 --- a/x-pack/plugins/alerts/server/alerts_client.mock.ts +++ b/x-pack/plugins/alerts/server/alerts_client.mock.ts @@ -26,6 +26,7 @@ const createAlertsClientMock = () => { unmuteInstance: jest.fn(), listAlertTypes: jest.fn(), getAlertInstanceSummary: jest.fn(), + invalidateApiKey: jest.fn(), }; return mocked; }; diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 33da41b15c272..b3e23aaa0114b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -71,7 +71,6 @@ export interface ConstructorOptions { namespace?: string; getUserName: () => Promise; createAPIKey: (name: string) => Promise; - invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise; getActionsClient: () => Promise; getEventLogClient: () => Promise; kibanaVersion: PluginInitializerContext['env']['packageInfo']['version']; @@ -156,9 +155,6 @@ export class AlertsClient { private readonly authorization: AlertsAuthorization; private readonly alertTypeRegistry: AlertTypeRegistry; private readonly createAPIKey: (name: string) => Promise; - private readonly invalidateAPIKey: ( - params: InvalidateAPIKeyParams - ) => Promise; private readonly getActionsClient: () => Promise; private readonly actionsAuthorization: ActionsAuthorization; private readonly getEventLogClient: () => Promise; @@ -175,7 +171,6 @@ export class AlertsClient { namespace, getUserName, createAPIKey, - invalidateAPIKey, encryptedSavedObjectsClient, getActionsClient, actionsAuthorization, @@ -191,7 +186,6 @@ export class AlertsClient { this.unsecuredSavedObjectsClient = unsecuredSavedObjectsClient; this.authorization = authorization; this.createAPIKey = createAPIKey; - this.invalidateAPIKey = invalidateAPIKey; this.encryptedSavedObjectsClient = encryptedSavedObjectsClient; this.getActionsClient = getActionsClient; this.actionsAuthorization = actionsAuthorization; @@ -622,25 +616,18 @@ export class AlertsClient { } } - private async invalidateApiKey({ apiKey }: { apiKey: string | null }): Promise { + public async invalidateApiKey({ apiKey }: { apiKey: string | null }): Promise { if (!apiKey) { return; } try { const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; - // const response = await this.invalidateAPIKey({ id: apiKeyId }); - // if (response.apiKeysEnabled === true && response.result.error_count > 0) { - // this.logger.error(`Failed to invalidate API Key [id="${apiKeyId}"]`); - // } - const markedToDelete = await this.unsecuredSavedObjectsClient.create( - 'invalidatePendingApiKey', - { - apiKeyId, - } - ); + await this.unsecuredSavedObjectsClient.create('invalidatePendingApiKey', { + apiKeyId, + }); } catch (e) { - this.logger.error(`Failed to invalidate API Key: ${e.message}`); + this.logger.error(`Failed to mark for invalidate API Key: ${e.message}`); } } diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index bce1af203fb0e..d2a0fc31896e8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -34,7 +34,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -732,7 +731,7 @@ describe('create()', () => { `"Test failure"` ); expect(taskManager.schedule).not.toHaveBeenCalled(); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); test('attempts to remove saved object if scheduling failed', async () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index d9b253c3a56e8..885686b3305e6 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -32,7 +32,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -98,7 +97,7 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); @@ -112,7 +111,7 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith('alert', '1'); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'delete(): Failed to load API key to invalidate on alert 1: Fail' @@ -142,14 +141,14 @@ describe('delete()', () => { }); await alertsClient.delete({ id: '1' }); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test('swallows error when invalidate API key throws', async () => { - alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); + // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); await alertsClient.delete({ id: '1' }); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'Failed to invalidate API Key: Fail' ); @@ -159,7 +158,7 @@ describe('delete()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail')); await alertsClient.delete({ id: '1' }); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'delete(): Failed to load API key to invalidate on alert 1: Fail' ); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index d0557df622028..a101c175a119e 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -145,7 +144,7 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { @@ -188,7 +187,7 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test(`doesn't disable already disabled alerts`, async () => { @@ -204,14 +203,14 @@ describe('disable()', () => { await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); expect(taskManager.remove).not.toHaveBeenCalled(); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test(`doesn't invalidate when no API key is used`, async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce(existingAlert); await alertsClient.disable({ id: '1' }); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test('swallows error when failing to load decrypted saved object', async () => { @@ -220,7 +219,7 @@ describe('disable()', () => { await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); expect(taskManager.remove).toHaveBeenCalled(); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'disable(): Failed to load API key to invalidate on alert 1: Fail' ); @@ -235,7 +234,7 @@ describe('disable()', () => { }); test('swallows error when invalidate API key throws', async () => { - alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); + // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); await alertsClient.disable({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index f098bbcad8d05..d0226b9d3440b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -34,7 +34,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -163,7 +162,7 @@ describe('enable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( 'alert', @@ -227,7 +226,7 @@ describe('enable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); test(`doesn't enable already enabled alerts`, async () => { @@ -321,7 +320,7 @@ describe('enable()', () => { ); expect(alertsClientParams.getUserName).toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts index c1adaddc80d9e..1b3a776bd23e0 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts @@ -35,7 +35,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts index 004230403de2e..5c0d80f159b31 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts index a53e49337f385..d9036811dbc5c 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts @@ -39,7 +39,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_state.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_state.test.ts index 8b32f05f6d5a1..79a064beba166 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_state.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_state.test.ts @@ -34,7 +34,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts index b2f5c5498f848..8cbe47655ef68 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts index 88199dfd1f7b9..868fa3d8c6aa2 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts @@ -32,7 +32,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts index cd7112b3551b3..05ca741f480ca 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts index 07666c1cc6261..5ef1af9b6f0ee 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts index 97711b8c14579..88692239ac2fe 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts @@ -33,7 +33,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 1dcde6addb9bf..5a86a5702cc39 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -38,7 +38,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -732,7 +731,7 @@ describe('update()', () => { }); it('swallows error when invalidate API key throws', async () => { - alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); + // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.bulkGet.mockResolvedValueOnce({ saved_objects: [ { @@ -965,8 +964,8 @@ describe('update()', () => { }, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index 1f3b567b2c031..ab1b295a8e4a9 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -32,7 +32,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -121,7 +120,7 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); }); test('falls back to SOC when getDecryptedAsInternalUser throws an error', async () => { @@ -160,11 +159,11 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test('swallows error when invalidate API key throws', async () => { - alertsClientParams.invalidateAPIKey.mockRejectedValue(new Error('Fail')); + // alertsClientParams.invalidateAPIKey.mockRejectedValue(new Error('Fail')); await alertsClient.updateApiKey({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( @@ -181,7 +180,7 @@ describe('updateApiKey()', () => { 'updateApiKey(): Failed to load API key to invalidate on alert 1: Fail' ); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); }); test('throws when unsecuredSavedObjectsClient update fails and invalidates newly created API key', async () => { @@ -194,8 +193,8 @@ describe('updateApiKey()', () => { await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( `"Fail"` ); - expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); - expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); + // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); + // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts index b1ac5ac4c6783..a2e0e75adec03 100644 --- a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts @@ -45,7 +45,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger, encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), @@ -306,14 +305,14 @@ beforeEach(() => { jest.resetAllMocks(); alertsClientParams.createAPIKey.mockResolvedValue({ apiKeysEnabled: false }); - alertsClientParams.invalidateAPIKey.mockResolvedValue({ + /* alertsClientParams.invalidateAPIKey.mockResolvedValue({ apiKeysEnabled: true, result: { invalidated_api_keys: [], previously_invalidated_api_keys: [], error_count: 0, }, - }); + }); */ alertsClientParams.getUserName.mockResolvedValue('elastic'); taskManager.runNow.mockResolvedValue({ id: '' }); diff --git a/x-pack/plugins/alerts/server/alerts_client_factory.ts b/x-pack/plugins/alerts/server/alerts_client_factory.ts index eccd810391307..1d58869d009f7 100644 --- a/x-pack/plugins/alerts/server/alerts_client_factory.ts +++ b/x-pack/plugins/alerts/server/alerts_client_factory.ts @@ -14,7 +14,7 @@ import { PluginStartContract as ActionsPluginStartContract } from '../../actions import { AlertsClient } from './alerts_client'; import { ALERTS_FEATURE_ID } from '../common'; import { AlertTypeRegistry, SpaceIdToNamespaceFunction } from './types'; -import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../security/server'; +import { SecurityPluginSetup } from '../../security/server'; import { EncryptedSavedObjectsClient } from '../../encrypted_saved_objects/server'; import { TaskManagerStartContract } from '../../task_manager/server'; import { PluginStartContract as FeaturesPluginStart } from '../../features/server'; @@ -129,22 +129,6 @@ export class AlertsClientFactory { result: createAPIKeyResult, }; }, - async invalidateAPIKey(params: InvalidateAPIKeyParams) { - if (!securityPluginSetup) { - return { apiKeysEnabled: false }; - } - const invalidateAPIKeyResult = await securityPluginSetup.authc.invalidateAPIKeyAsInternalUser( - params - ); - // Null when Elasticsearch security is disabled - if (!invalidateAPIKeyResult) { - return { apiKeysEnabled: false }; - } - return { - apiKeysEnabled: true, - result: invalidateAPIKeyResult, - }; - }, async getActionsClient() { return actions.getActionsClientWithRequest(request); }, From c4afdfe4ae4214384ab28e76650e9403e8982693 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 2 Nov 2020 08:44:02 -0800 Subject: [PATCH 03/26] Fixed type checks --- .../server/alerts_client/alerts_client.ts | 1 - .../invalidate_pending_api_keys/task.ts | 37 +++++++++++++++---- x-pack/plugins/alerts/server/plugin.ts | 21 +---------- x-pack/plugins/alerts/server/types.ts | 2 +- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 6770a21763ec5..efa0e1d680eb4 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -30,7 +30,6 @@ import { } from '../types'; import { validateAlertTypeParams, alertExecutionStatusFromRaw } from '../lib'; import { - InvalidateAPIKeyParams, GrantAPIKeyResult as SecurityPluginGrantAPIKeyResult, InvalidateAPIKeyResult as SecurityPluginInvalidateAPIKeyResult, } from '../../../security/server'; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index c09d894f5030a..7464ab21192e1 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -5,7 +5,7 @@ */ import { Logger, ISavedObjectsRepository } from 'kibana/server'; -import { InvalidateAPIKeyParams } from '../../../security/server'; +import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { RunContext, TaskManagerSetupContract, @@ -18,19 +18,39 @@ import { InvalidatePendingApiKey } from '../types'; const TASK_TYPE = 'alerts_invalidate_api_keys'; export const TASK_ID = `Alerts-${TASK_TYPE}`; +const invalidateAPIKey = async ( + params: InvalidateAPIKeyParams, + securityPluginSetup?: SecurityPluginSetup +): Promise => { + if (!securityPluginSetup) { + return { apiKeysEnabled: false }; + } + const invalidateAPIKeyResult = await securityPluginSetup.authc.invalidateAPIKeyAsInternalUser( + params + ); + // Null when Elasticsearch security is disabled + if (!invalidateAPIKeyResult) { + return { apiKeysEnabled: false }; + } + return { + apiKeysEnabled: true, + result: invalidateAPIKeyResult, + }; +}; + export function initializeAlertsInvalidateApiKeys( logger: Logger, internalSavedObjectsRepository: Promise<{ createInternalRepository: () => ISavedObjectsRepository; }>, taskManager: TaskManagerSetupContract, - invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise + securityPluginSetup?: SecurityPluginSetup ) { registerAlertsInvalidateApiKeysTask( logger, internalSavedObjectsRepository, taskManager, - invalidateAPIKey + securityPluginSetup ); } @@ -50,12 +70,12 @@ function registerAlertsInvalidateApiKeysTask( createInternalRepository: () => ISavedObjectsRepository; }>, taskManager: TaskManagerSetupContract, - invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise + securityPluginSetup?: SecurityPluginSetup ) { taskManager.registerTaskDefinitions({ [TASK_TYPE]: { title: 'Invalidate Alerts API Keys', - createTaskRunner: taskRunner(logger, internalSavedObjectsRepository, invalidateAPIKey), + createTaskRunner: taskRunner(logger, internalSavedObjectsRepository, securityPluginSetup), }, }); } @@ -86,7 +106,7 @@ function taskRunner( internalSavedObjectsRepository: Promise<{ createInternalRepository: () => ISavedObjectsRepository; }>, - invalidateAPIKey: (params: InvalidateAPIKeyParams) => Promise + securityPluginSetup?: SecurityPluginSetup ) { return ({ taskInstance }: RunContext) => { const { state } = taskInstance; @@ -98,7 +118,10 @@ function taskRunner( type: 'invalidatePendingApiKey', }); res.saved_objects.forEach(async (obj) => { - const response = await invalidateAPIKey({ id: obj.attributes.apiKeyId }); + const response = await invalidateAPIKey( + { id: obj.attributes.apiKeyId }, + securityPluginSetup + ); if (response.apiKeysEnabled === true && response.result.error_count > 0) { logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); } else { diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 06dd7a418f90a..044300da49ddf 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -192,30 +192,11 @@ export class AlertingPlugin { }); } - const invalidateAPIKey = async ( - params: InvalidateAPIKeyParams - ): Promise => { - if (!this.security) { - return { apiKeysEnabled: false }; - } - const invalidateAPIKeyResult = await this.security.authc.invalidateAPIKeyAsInternalUser( - params - ); - // Null when Elasticsearch security is disabled - if (!invalidateAPIKeyResult) { - return { apiKeysEnabled: false }; - } - return { - apiKeysEnabled: true, - result: invalidateAPIKeyResult, - }; - }; - initializeAlertsInvalidateApiKeys( this.logger, this.createSOInternalRepositoryContext(core), plugins.taskManager, - invalidateAPIKey + this.security ); core.http.registerRouteHandlerContext('alerting', this.createRouteHandlerContext(core)); diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 29ae2678f91fe..fe15ad9ff4714 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -173,7 +173,7 @@ export interface AlertingPlugin { } export interface AlertsConfigType { - healthCheck: { + invalidateApiKeysTask: { interval: string; }; } From 5a0d5c2acf6541ff96e0dec4c00794060b7b1ab9 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 2 Nov 2020 13:56:25 -0800 Subject: [PATCH 04/26] Fixed jest tests --- .../server/alerts_client/alerts_client.ts | 2 +- .../server/alerts_client/tests/create.test.ts | 15 ++++- .../server/alerts_client/tests/delete.test.ts | 51 +++++++++++++--- .../alerts_client/tests/disable.test.ts | 58 ++++++++++++++++--- .../alerts/server/alerts_client/tests/lib.ts | 8 --- .../server/alerts_client/tests/update.test.ts | 46 ++++++++++++--- .../tests/update_api_key.test.ts | 56 +++++++++++++++--- .../alerts_client_conflict_retries.test.ts | 2 +- .../server/alerts_client_factory.test.ts | 2 - 9 files changed, 194 insertions(+), 46 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index efa0e1d680eb4..b8b1522108dc2 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -626,7 +626,7 @@ export class AlertsClient { apiKeyId, }); } catch (e) { - this.logger.error(`Failed to mark for invalidate API Key: ${e.message}`); + this.logger.error(`Failed to mark for invalidating API Key: ${e.message}`); } } diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index d2a0fc31896e8..d27de6292f1f6 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -707,7 +707,7 @@ describe('create()', () => { expect(taskManager.schedule).not.toHaveBeenCalled(); }); - test('throws error and invalidates API key when create saved object fails', async () => { + test('throws error and add API key to invalidatePendingApiKey SO when create saved object fails', async () => { const data = getMockData(); alertsClientParams.createAPIKey.mockResolvedValueOnce({ apiKeysEnabled: true, @@ -727,11 +727,22 @@ describe('create()', () => { ], }); unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Test failure')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); await expect(alertsClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot( `"Test failure"` ); expect(taskManager.schedule).not.toHaveBeenCalled(); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); + expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ + apiKeyId: '123', + }); }); test('attempts to remove saved object if scheduling failed', async () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 885686b3305e6..79cc2a5a83ecf 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -93,11 +93,21 @@ describe('delete()', () => { }); test('successfully removes an alert', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); const result = await alertsClient.delete({ id: '1' }); expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); @@ -106,12 +116,20 @@ describe('delete()', () => { test('falls back to SOC.get when getDecryptedAsInternalUser throws an error', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); const result = await alertsClient.delete({ id: '1' }); expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith('alert', '1'); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'delete(): Failed to load API key to invalidate on alert 1: Fail' @@ -132,6 +150,14 @@ describe('delete()', () => { }); test(`doesn't invalidate API key when apiKey is null`, async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue({ ...existingAlert, attributes: { @@ -141,24 +167,33 @@ describe('delete()', () => { }); await alertsClient.delete({ id: '1' }); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('swallows error when invalidate API key throws', async () => { - // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); - + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.delete({ id: '1' }); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to invalidate API Key: Fail' + 'Failed to mark for invalidating API Key: Fail' ); }); test('swallows error when getDecryptedAsInternalUser throws an error', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail')); await alertsClient.delete({ id: '1' }); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'delete(): Failed to load API key to invalidate on alert 1: Fail' ); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index a101c175a119e..40b7bb036d60d 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -107,6 +107,14 @@ describe('disable()', () => { }); test('disables an alert', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: '123', + }, + references: [], + }); await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { @@ -144,11 +152,21 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith('alert', '1'); @@ -187,7 +205,7 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test(`doesn't disable already disabled alerts`, async () => { @@ -200,26 +218,51 @@ describe('disable()', () => { }, }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); + await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.update).not.toHaveBeenCalled(); expect(taskManager.remove).not.toHaveBeenCalled(); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test(`doesn't invalidate when no API key is used`, async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce(existingAlert); await alertsClient.disable({ id: '1' }); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('swallows error when failing to load decrypted saved object', async () => { + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); await alertsClient.disable({ id: '1' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); expect(taskManager.remove).toHaveBeenCalled(); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'disable(): Failed to load API key to invalidate on alert 1: Fail' ); @@ -234,11 +277,10 @@ describe('disable()', () => { }); test('swallows error when invalidate API key throws', async () => { - // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); - + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.disable({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to invalidate API Key: Fail' + 'Failed to mark for invalidating API Key: Fail' ); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts index 5ebb4e90d4b50..028a7c6737474 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts @@ -46,14 +46,6 @@ export function getBeforeSetup( ) { jest.resetAllMocks(); alertsClientParams.createAPIKey.mockResolvedValue({ apiKeysEnabled: false }); - alertsClientParams.invalidateAPIKey.mockResolvedValue({ - apiKeysEnabled: true, - result: { - invalidated_api_keys: [], - previously_invalidated_api_keys: [], - error_count: 0, - }, - }); alertsClientParams.getUserName.mockResolvedValue('elastic'); taskManager.runNow.mockResolvedValue({ id: '' }); const actionsClient = actionsClientMock.create(); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 5a86a5702cc39..a800b80325cca 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -160,6 +160,14 @@ describe('update()', () => { }, ], }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: '234', + }, + references: [], + }); const result = await alertsClient.update({ id: '1', data: { @@ -240,7 +248,7 @@ describe('update()', () => { namespace: 'default', }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` @@ -375,6 +383,22 @@ describe('update()', () => { }, ], }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: '234', + }, + references: [], + }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: '234', + }, + references: [], + }); const result = await alertsClient.update({ id: '1', data: { @@ -422,7 +446,7 @@ describe('update()', () => { "updatedAt": 2019-02-12T21:01:22.479Z, } `); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` @@ -529,6 +553,14 @@ describe('update()', () => { }, ], }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: '234', + }, + references: [], + }); const result = await alertsClient.update({ id: '1', data: { @@ -577,7 +609,7 @@ describe('update()', () => { "updatedAt": 2019-02-12T21:01:22.479Z, } `); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(3); expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` @@ -731,7 +763,6 @@ describe('update()', () => { }); it('swallows error when invalidate API key throws', async () => { - // alertsClientParams.invalidateAPIKey.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.bulkGet.mockResolvedValueOnce({ saved_objects: [ { @@ -774,6 +805,7 @@ describe('update()', () => { }, ], }); + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); // add ApiKey to invalidate await alertsClient.update({ id: '1', data: { @@ -796,7 +828,7 @@ describe('update()', () => { }, }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to invalidate API Key: Fail' + 'Failed to mark for invalidating API Key: Fail' ); }); @@ -964,8 +996,8 @@ describe('update()', () => { }, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith({ apiKeyId: '123' }); + expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ apiKeyId: '234' }); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index ab1b295a8e4a9..c8f2991c7d83f 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -79,6 +79,14 @@ describe('updateApiKey()', () => { beforeEach(() => { alertsClient = new AlertsClient(alertsClientParams); unsecuredSavedObjectsClient.get.mockResolvedValue(existingAlert); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue(existingEncryptedAlert); alertsClientParams.createAPIKey.mockResolvedValueOnce({ apiKeysEnabled: true, @@ -120,11 +128,21 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); }); test('falls back to SOC when getDecryptedAsInternalUser throws an error', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); await alertsClient.updateApiKey({ id: '1' }); expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith('alert', '1'); @@ -159,28 +177,36 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('swallows error when invalidate API key throws', async () => { - // alertsClientParams.invalidateAPIKey.mockRejectedValue(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.updateApiKey({ id: '1' }); - expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to invalidate API Key: Fail' - ); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); }); test('swallows error when getting decrypted object throws', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); await alertsClient.updateApiKey({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'updateApiKey(): Failed to load API key to invalidate on alert 1: Fail' ); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); }); test('throws when unsecuredSavedObjectsClient update fails and invalidates newly created API key', async () => { @@ -189,12 +215,24 @@ describe('updateApiKey()', () => { result: { id: '234', name: '234', api_key: 'abc' }, }); unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidatePendingApiKey', + attributes: { + apiKeyId: 'test', + }, + references: [], + }); await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( `"Fail"` ); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' }); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' }); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '123', + }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + apiKeyId: '234', + }); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts index a2e0e75adec03..06b2b3bb56449 100644 --- a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts @@ -114,7 +114,7 @@ async function update(success: boolean) { ); return expectConflict(success, err, 'create'); } - expectSuccess(success, 2, 'create'); + expectSuccess(success, 3, 'create'); // only checking the debug messages in this test expect(logger.debug).nthCalledWith(1, `alertsClient.update('alert-id') conflict, retrying ...`); diff --git a/x-pack/plugins/alerts/server/alerts_client_factory.test.ts b/x-pack/plugins/alerts/server/alerts_client_factory.test.ts index 3cf6666e90eb0..aa31edb603d3d 100644 --- a/x-pack/plugins/alerts/server/alerts_client_factory.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_factory.test.ts @@ -125,7 +125,6 @@ test('creates an alerts client with proper constructor arguments when security i getActionsClient: expect.any(Function), getEventLogClient: expect.any(Function), createAPIKey: expect.any(Function), - invalidateAPIKey: expect.any(Function), encryptedSavedObjectsClient: alertsClientFactoryParams.encryptedSavedObjectsClient, kibanaVersion: '7.10.0', }); @@ -167,7 +166,6 @@ test('creates an alerts client with proper constructor arguments', async () => { namespace: 'default', getUserName: expect.any(Function), createAPIKey: expect.any(Function), - invalidateAPIKey: expect.any(Function), encryptedSavedObjectsClient: alertsClientFactoryParams.encryptedSavedObjectsClient, getActionsClient: expect.any(Function), getEventLogClient: expect.any(Function), From 077c927e6b4380a3d0f07fea4982e73e2053b901 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 2 Nov 2020 14:00:59 -0800 Subject: [PATCH 05/26] Removed test code --- .../server/alert_types/index_threshold/alert_type.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts b/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts index b664f54669bc5..2a1ed429b7fe1 100644 --- a/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts +++ b/x-pack/plugins/stack_alerts/server/alert_types/index_threshold/alert_type.ts @@ -125,12 +125,6 @@ export function getAlertType(service: Service): AlertType { - setTimeout(resolve, ms); - }); - } - await sleep(5000); const callCluster = services.callCluster; const date = new Date().toISOString(); From 60edfb3103a0dbe00fd7300dba29d63e5af5819a Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 2 Nov 2020 15:02:28 -0800 Subject: [PATCH 06/26] Changed SO name --- .../server/alerts_client/alerts_client.ts | 2 +- .../server/alerts_client/tests/create.test.ts | 2 +- .../server/alerts_client/tests/delete.test.ts | 12 +++++----- .../alerts_client/tests/disable.test.ts | 12 +++++----- .../server/alerts_client/tests/update.test.ts | 8 +++---- .../tests/update_api_key.test.ts | 23 +++++++++++-------- .../invalidate_pending_api_keys/task.ts | 4 ++-- .../alerts/server/saved_objects/index.ts | 13 +++++++---- .../mappings_invalidate_api_key.json | 9 -------- 9 files changed, 42 insertions(+), 43 deletions(-) delete mode 100644 x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index b8b1522108dc2..e167fb8764587 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -622,7 +622,7 @@ export class AlertsClient { try { const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; - await this.unsecuredSavedObjectsClient.create('invalidatePendingApiKey', { + await this.unsecuredSavedObjectsClient.create('invalidate_pending_api_key', { apiKeyId, }); } catch (e) { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index d27de6292f1f6..6229ea24154d8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -729,7 +729,7 @@ describe('create()', () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Test failure')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 79cc2a5a83ecf..02018a9e34df6 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -95,7 +95,7 @@ describe('delete()', () => { test('successfully removes an alert', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -105,7 +105,7 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '123', }); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { @@ -118,7 +118,7 @@ describe('delete()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -152,7 +152,7 @@ describe('delete()', () => { test(`doesn't invalidate API key when apiKey is null`, async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -173,7 +173,7 @@ describe('delete()', () => { test('swallows error when invalidate API key throws', async () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.delete({ id: '1' }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '123', }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( @@ -184,7 +184,7 @@ describe('delete()', () => { test('swallows error when getDecryptedAsInternalUser throws an error', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index 40b7bb036d60d..0baea8ce4d9fc 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -109,7 +109,7 @@ describe('disable()', () => { test('disables an alert', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: '123', }, @@ -152,7 +152,7 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '123', }); }); @@ -161,7 +161,7 @@ describe('disable()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -220,7 +220,7 @@ describe('disable()', () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -236,7 +236,7 @@ describe('disable()', () => { test(`doesn't invalidate when no API key is used`, async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -251,7 +251,7 @@ describe('disable()', () => { test('swallows error when failing to load decrypted saved object', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index a800b80325cca..7cdae887a53d6 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -162,7 +162,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: '234', }, @@ -385,7 +385,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: '234', }, @@ -393,7 +393,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: '234', }, @@ -555,7 +555,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: '234', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index c8f2991c7d83f..fb6ffd87c1018 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -81,7 +81,7 @@ describe('updateApiKey()', () => { unsecuredSavedObjectsClient.get.mockResolvedValue(existingAlert); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -128,7 +128,7 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '123', }); }); @@ -137,7 +137,7 @@ describe('updateApiKey()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -185,7 +185,7 @@ describe('updateApiKey()', () => { await alertsClient.updateApiKey({ id: '1' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '123', }); }); @@ -194,7 +194,7 @@ describe('updateApiKey()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -217,7 +217,7 @@ describe('updateApiKey()', () => { unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', attributes: { apiKeyId: 'test', }, @@ -227,10 +227,13 @@ describe('updateApiKey()', () => { await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( `"Fail"` ); - expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith('invalidatePendingApiKey', { - apiKeyId: '123', - }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidatePendingApiKey', { + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( + 'invalidate_pending_api_key', + { + apiKeyId: '123', + } + ); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { apiKeyId: '234', }); }); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 7464ab21192e1..acd918bce8bb4 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -115,7 +115,7 @@ function taskRunner( try { const repository = (await internalSavedObjectsRepository).createInternalRepository(); const res = await repository.find({ - type: 'invalidatePendingApiKey', + type: 'invalidate_pending_api_key', }); res.saved_objects.forEach(async (obj) => { const response = await invalidateAPIKey( @@ -126,7 +126,7 @@ function taskRunner( logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); } else { try { - await repository.delete('invalidatePendingApiKey', obj.id); + await repository.delete('invalidate_pending_api_key', obj.id); } catch (err) { // Skip the cleanup error logger.error( diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 3052da0bd7b25..44eb990a7a34d 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -6,7 +6,6 @@ import { SavedObjectsServiceSetup } from 'kibana/server'; import mappings from './mappings.json'; -import mappingsInvalidateApiKey from './mappings_invalidate_api_key.json'; import { getMigrations } from './migrations'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; @@ -44,10 +43,16 @@ export function setupSavedObjects( }); savedObjects.registerType({ - name: 'invalidatePendingApiKey', - hidden: true, + name: 'invalidate_pending_api_key', + hidden: false, namespaceType: 'single', - mappings: mappingsInvalidateApiKey.invalidatePendingApiKey, + mappings: { + properties: { + apiKeyId: { + type: 'keyword', + }, + }, + }, }); // Encrypted attributes diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json b/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json deleted file mode 100644 index f845c992adc8f..0000000000000 --- a/x-pack/plugins/alerts/server/saved_objects/mappings_invalidate_api_key.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "invalidatePendingApiKey": { - "properties": { - "apiKeyId": { - "type": "keyword" - } - } - } -} From 19c3b741fc1a188eec3607d377875c5c89613c85 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 2 Nov 2020 15:04:29 -0800 Subject: [PATCH 07/26] fixed type cheks --- x-pack/plugins/alerts/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 044300da49ddf..b2d0b91382685 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -6,14 +6,14 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import { first, map } from 'rxjs/operators'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; -import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../security/server'; +import { SecurityPluginSetup } from '../../security/server'; import { EncryptedSavedObjectsPluginSetup, EncryptedSavedObjectsPluginStart, } from '../../encrypted_saved_objects/server'; import { TaskManagerSetupContract, TaskManagerStartContract } from '../../task_manager/server'; import { SpacesPluginSetup, SpacesServiceSetup } from '../../spaces/server'; -import { AlertsClient, InvalidateAPIKeyResult } from './alerts_client'; +import { AlertsClient } from './alerts_client'; import { AlertTypeRegistry } from './alert_type_registry'; import { TaskRunnerFactory } from './task_runner'; import { AlertsClientFactory } from './alerts_client_factory'; From 7d11d22df99fd788f6077e572ebfecd10a1b7e1b Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Wed, 4 Nov 2020 10:26:52 -0800 Subject: [PATCH 08/26] Moved invalidate logic out of alerts client --- .../server/alerts_client/alerts_client.ts | 75 +++++++++++++------ .../alerts_client/tests/aggregate.test.ts | 1 - .../server/alerts_client/tests/enable.test.ts | 39 +++++++++- .../alerts_client_conflict_retries.test.ts | 8 -- .../add_to_invalidate_api_keys.ts | 25 +++++++ .../invalidate_pending_api_keys/task.ts | 12 +-- .../alerting_api_integration/common/config.ts | 1 + 7 files changed, 119 insertions(+), 42 deletions(-) create mode 100644 x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 64acaf4d1738a..a9b6e27ea5cd3 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -13,6 +13,7 @@ import { SavedObjectReference, SavedObject, PluginInitializerContext, + ISavedObjectsRepository, } from 'src/core/server'; import { esKuery } from '../../../../../src/plugins/data/server'; import { ActionsClient, ActionsAuthorization } from '../../../actions/server'; @@ -47,6 +48,7 @@ import { IEvent } from '../../../event_log/server'; import { parseDuration } from '../../common/parse_duration'; import { retryIfConflicts } from '../lib/retry_if_conflicts'; import { partiallyUpdateAlert } from '../saved_objects'; +import { addToInvalidateApiKeys } from '../invalidate_pending_api_keys/add_to_invalidate_api_keys'; export interface RegistryAlertTypeWithAuth extends RegistryAlertType { authorizedConsumers: string[]; @@ -256,7 +258,11 @@ export class AlertsClient { ); } catch (e) { // Avoid unused API key - this.invalidateApiKey({ apiKey: rawAlert.apiKey }); + addToInvalidateApiKeys( + { apiKey: rawAlert.apiKey }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); throw e; } if (data.enabled) { @@ -480,7 +486,13 @@ export class AlertsClient { await Promise.all([ taskIdToRemove ? deleteTaskIfItExists(this.taskManager, taskIdToRemove) : null, - apiKeyToInvalidate ? this.invalidateApiKey({ apiKey: apiKeyToInvalidate }) : null, + apiKeyToInvalidate + ? addToInvalidateApiKeys( + { apiKey: apiKeyToInvalidate }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ) + : null, ]); return removeResult; @@ -519,7 +531,11 @@ export class AlertsClient { await Promise.all([ alertSavedObject.attributes.apiKey - ? this.invalidateApiKey({ apiKey: alertSavedObject.attributes.apiKey }) + ? addToInvalidateApiKeys( + { apiKey: alertSavedObject.attributes.apiKey }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ) : null, (async () => { if ( @@ -584,7 +600,11 @@ export class AlertsClient { ); } catch (e) { // Avoid unused API key - this.invalidateApiKey({ apiKey: createAttributes.apiKey }); + addToInvalidateApiKeys( + { apiKey: createAttributes.apiKey }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); throw e; } @@ -664,27 +684,20 @@ export class AlertsClient { await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version }); } catch (e) { // Avoid unused API key - this.invalidateApiKey({ apiKey: updateAttributes.apiKey }); + addToInvalidateApiKeys( + { apiKey: updateAttributes.apiKey }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); throw e; } if (apiKeyToInvalidate) { - await this.invalidateApiKey({ apiKey: apiKeyToInvalidate }); - } - } - - public async invalidateApiKey({ apiKey }: { apiKey: string | null }): Promise { - if (!apiKey) { - return; - } - - try { - const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; - await this.unsecuredSavedObjectsClient.create('invalidate_pending_api_key', { - apiKeyId, - }); - } catch (e) { - this.logger.error(`Failed to mark for invalidating API Key: ${e.message}`); + await addToInvalidateApiKeys( + { apiKey: apiKeyToInvalidate }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); } } @@ -744,7 +757,11 @@ export class AlertsClient { await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version }); } catch (e) { // Avoid unused API key - this.invalidateApiKey({ apiKey: updateAttributes.apiKey }); + addToInvalidateApiKeys( + { apiKey: updateAttributes.apiKey }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); throw e; } const scheduledTask = await this.scheduleAlert( @@ -756,7 +773,11 @@ export class AlertsClient { scheduledTaskId: scheduledTask.id, }); if (apiKeyToInvalidate) { - await this.invalidateApiKey({ apiKey: apiKeyToInvalidate }); + await addToInvalidateApiKeys( + { apiKey: apiKeyToInvalidate }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ); } } } @@ -817,7 +838,13 @@ export class AlertsClient { attributes.scheduledTaskId ? deleteTaskIfItExists(this.taskManager, attributes.scheduledTaskId) : null, - apiKeyToInvalidate ? this.invalidateApiKey({ apiKey: apiKeyToInvalidate }) : null, + apiKeyToInvalidate + ? await addToInvalidateApiKeys( + { apiKey: apiKeyToInvalidate }, + this.logger, + (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + ) + : null, ]); } } diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts index 0f89fc6c9c25c..cc5d10c3346e8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts @@ -34,7 +34,6 @@ const alertsClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), - invalidateAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), encryptedSavedObjectsClient: encryptedSavedObjects, getActionsClient: jest.fn(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 59e9cfbe25689..ee1b6dc0d5992 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -156,13 +156,26 @@ describe('enable()', () => { updatedBy: 'elastic', }, }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidate_pending_api_key', + attributes: { + apiKeyId: '123', + }, + references: [], + }); await alertsClient.enable({ id: '1' }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - // expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( + 'invalidate_pending_api_key', + { + apiKeyId: '123', + } + ); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( 'alert', @@ -223,13 +236,23 @@ describe('enable()', () => { apiKey: Buffer.from('123:abc').toString('base64'), }, }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidate_pending_api_key', + attributes: { + apiKeyId: '123', + }, + references: [], + }); await alertsClient.enable({ id: '1' }); expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { + apiKeyId: '123', + }); }); test(`doesn't enable already enabled alerts`, async () => { @@ -317,13 +340,23 @@ describe('enable()', () => { }); unsecuredSavedObjectsClient.update.mockReset(); unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail to update')); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'invalidate_pending_api_key', + attributes: { + apiKeyId: '123', + }, + references: [], + }); await expect(alertsClient.enable({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( `"Fail to update"` ); expect(alertsClientParams.getUserName).toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); - // expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { + apiKeyId: '123', + }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts index 06b2b3bb56449..ca9389ece310c 100644 --- a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts @@ -305,14 +305,6 @@ beforeEach(() => { jest.resetAllMocks(); alertsClientParams.createAPIKey.mockResolvedValue({ apiKeysEnabled: false }); - /* alertsClientParams.invalidateAPIKey.mockResolvedValue({ - apiKeysEnabled: true, - result: { - invalidated_api_keys: [], - previously_invalidated_api_keys: [], - error_count: 0, - }, - }); */ alertsClientParams.getUserName.mockResolvedValue('elastic'); taskManager.runNow.mockResolvedValue({ id: '' }); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts new file mode 100644 index 0000000000000..5c53097debbf0 --- /dev/null +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { Logger, ISavedObjectsRepository } from 'src/core/server'; + +export const addToInvalidateApiKeys = async ( + { apiKey }: { apiKey: string | null }, + logger: Logger, + internalSavedObjectsRepository: ISavedObjectsRepository +): Promise => { + if (!apiKey) { + return; + } + + try { + const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; + await internalSavedObjectsRepository.create('invalidate_pending_api_key', { + apiKeyId, + }); + } catch (e) { + logger.error(`Failed to mark for invalidating API Key: ${e.message}`); + } +}; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index acd918bce8bb4..4a6aefc5d24fa 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -112,12 +112,13 @@ function taskRunner( const { state } = taskInstance; return { async run() { + let totalInvalidated = 0; try { const repository = (await internalSavedObjectsRepository).createInternalRepository(); - const res = await repository.find({ + const apiKeysToInvalidate = await repository.find({ type: 'invalidate_pending_api_key', }); - res.saved_objects.forEach(async (obj) => { + apiKeysToInvalidate.saved_objects.forEach(async (obj) => { const response = await invalidateAPIKey( { id: obj.attributes.apiKeyId }, securityPluginSetup @@ -127,19 +128,18 @@ function taskRunner( } else { try { await repository.delete('invalidate_pending_api_key', obj.id); + totalInvalidated++; } catch (err) { - // Skip the cleanup error logger.error( `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` ); } } }); - // TODO: clean all return { state: { runs: (state.runs || 0) + 1, - total_removed: 0, + total_invalidated: totalInvalidated, }, }; } catch (errMsg) { @@ -147,7 +147,7 @@ function taskRunner( return { state: { runs: (state.runs || 0) + 1, - total_removed: 0, + total_invalidated: totalInvalidated, }, }; } diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index f9fdfaed1c79b..a73f898e7d7b3 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -92,6 +92,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) ...xPackApiIntegrationTestsConfig.get('kbnTestServer.serverArgs'), `--xpack.actions.allowedHosts=${JSON.stringify(['localhost', 'some.non.existent.com'])}`, '--xpack.encryptedSavedObjects.encryptionKey="wuGNaIhoMpk5sO4UBxgr3NyW1sFcLgIf"', + '--xpack.alerts.invalidateApiKeysTask.interval="30s"', `--xpack.actions.enabledActionTypes=${JSON.stringify(enabledActionTypes)}`, ...actionsProxyUrl, From 7c452058dc17ce7d23118c6d2e80d622752c3d01 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Wed, 4 Nov 2020 10:30:44 -0800 Subject: [PATCH 09/26] fixed type check --- x-pack/plugins/alerts/server/alerts_client.mock.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/alerts_client.mock.ts b/x-pack/plugins/alerts/server/alerts_client.mock.ts index 9a51f3e6c52ed..b2ce2a1b356fb 100644 --- a/x-pack/plugins/alerts/server/alerts_client.mock.ts +++ b/x-pack/plugins/alerts/server/alerts_client.mock.ts @@ -27,7 +27,6 @@ const createAlertsClientMock = () => { unmuteInstance: jest.fn(), listAlertTypes: jest.fn(), getAlertInstanceSummary: jest.fn(), - invalidateApiKey: jest.fn(), }; return mocked; }; From 4d5d67d89638437a7d24b40178a2a805e9d255e6 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Wed, 4 Nov 2020 12:57:32 -0800 Subject: [PATCH 10/26] Added functional tests --- x-pack/plugins/alerts/server/plugin.ts | 2 +- .../alerting_api_integration/common/config.ts | 2 +- .../plugins/alerts/server/alert_types.ts | 21 +++++ .../fixtures/plugins/alerts/server/routes.ts | 26 ++++++ .../tests/alerting/update.ts | 90 +++++++++++++++++++ 5 files changed, 139 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 816b6f2bd3ffc..cfa2c59a1b7ed 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -302,7 +302,7 @@ export class AlertingPlugin { const [{ savedObjects }] = await core.getStartServices(); return { createInternalRepository: () => { - return savedObjects.createInternalRepository(['alert']); + return savedObjects.createInternalRepository(['alert', 'invalidate_pending_api_key']); }, }; }; diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index a73f898e7d7b3..f5ca896ee2fec 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -92,7 +92,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) ...xPackApiIntegrationTestsConfig.get('kbnTestServer.serverArgs'), `--xpack.actions.allowedHosts=${JSON.stringify(['localhost', 'some.non.existent.com'])}`, '--xpack.encryptedSavedObjects.encryptionKey="wuGNaIhoMpk5sO4UBxgr3NyW1sFcLgIf"', - '--xpack.alerts.invalidateApiKeysTask.interval="30s"', + '--xpack.alerts.invalidateApiKeysTask.interval="1m"', `--xpack.actions.enabledActionTypes=${JSON.stringify(enabledActionTypes)}`, ...actionsProxyUrl, diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts index d43c3363f86b1..b4e7826f4eb91 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts @@ -423,6 +423,26 @@ export function defineAlertTypes( throw new Error('this alert is intended to fail'); }, }; + const longRunningAlertType: AlertType = { + id: 'test.longRunning', + name: 'Test: Long Running', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + producer: 'alertsFixture', + defaultActionGroupId: 'default', + async executor() { + async function sleep(ms: number) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); + } + await sleep(10000); + }, + }; alerts.registerType(getAlwaysFiringAlertType()); alerts.registerType(getCumulativeFiringAlertType()); @@ -435,4 +455,5 @@ export function defineAlertTypes( alerts.registerType(onlyStateVariablesAlertType); alerts.registerType(getPatternFiringAlertType()); alerts.registerType(throwAlertType); + alerts.registerType(longRunningAlertType); } diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index 79fc8470a8ec2..54735990728a5 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -11,6 +11,7 @@ import { IKibanaResponse, } from 'kibana/server'; import { schema } from '@kbn/config-schema'; +import { InvalidatePendingApiKey } from '../../../../../../../plugins/alerts/server/types'; import { RawAlert } from '../../../../../../../plugins/alerts/server/types'; import { TaskInstance } from '../../../../../../../plugins/task_manager/server'; import { FixtureSetupDeps, FixtureStartDeps } from './plugin'; @@ -184,4 +185,29 @@ export function defineRoutes( return res.ok({ body: result }); } ); + + router.get( + { + path: '/api/alerts_fixture/alerting_invalidate_apiKeys', + validate: {}, + }, + async ( + context: RequestHandlerContext, + req: KibanaRequest, + res: KibanaResponseFactory + ): Promise> => { + try { + const [{ savedObjects }] = await core.getStartServices(); + const repository = savedObjects.createInternalRepository(['invalidate_pending_api_key']); + const apiKeysToInvalidate = await repository.find({ + type: 'invalidate_pending_api_key', + }); + return res.ok({ + body: { apiKeysToInvalidate }, + }); + } catch (err) { + return res.badRequest({ body: err }); + } + } + ); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 0ad2ca226ed5d..76937e13e16f3 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -31,6 +31,13 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { .then((response: SupertestResponse) => response.body); } + function getAlertingApiKeysToInvalidate() { + return supertest + .get(`/api/alerting_tasks/alerting_invalidate_apiKeys`) + .expect(200) + .then((response: SupertestResponse) => response.body); + } + describe('update', () => { const objectRemover = new ObjectRemover(supertest); @@ -835,6 +842,89 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { } }); + it('should handle updates for a long running alert type without failing the underlying tasks due to invalidated ApiKey', async () => { + const { body: createdAlert } = await supertest + .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) + .set('kbn-xsrf', 'foo') + .send({ + enabled: true, + name: 'abc', + tags: ['foo'], + alertTypeId: 'test.longRunning', + consumer: 'alertsFixture', + schedule: { interval: '1s' }, + throttle: '1m', + actions: [], + params: {}, + }) + .expect(200); + objectRemover.add(space.id, createdAlert.id, 'alert', 'alerts'); + + const updatedData = { + name: 'bcd', + tags: ['bar'], + params: { + foo: true, + }, + schedule: { interval: '1m' }, + actions: [], + throttle: '1m', + }; + const response = await supertestWithoutAuth + .put(`${getUrlPrefix(space.id)}/api/alerts/alert/${createdAlert.id}`) + .set('kbn-xsrf', 'foo') + .auth(user.username, user.password) + .send(updatedData); + + const apiKeyIds = (await getAlertingApiKeysToInvalidate()).docs[0]; + expect(apiKeyIds.apiKeysToInvalidate.length > 1).to.be(true); + + const statusUpdates: string[] = []; + await retry.try(async () => { + const alertTask = (await getAlertingTaskById(createdAlert.scheduledTaskId)).docs[0]; + statusUpdates.push(alertTask.status); + expect(alertTask.status).to.eql('idle'); + }); + + expect(statusUpdates.find((status) => status === 'failed')).to.be(undefined); + + await retry.try(async () => { + const apiKeyIdsRes = (await getAlertingApiKeysToInvalidate()).docs[0]; + expect(apiKeyIdsRes.apiKeysToInvalidate.length).to.be(1); + }); + + switch (scenario.id) { + case 'no_kibana_privileges at space1': + case 'space_1_all at space2': + case 'global_read at space1': + expect(response.statusCode).to.eql(403); + expect(response.body).to.eql({ + error: 'Forbidden', + message: getConsumerUnauthorizedErrorMessage( + 'update', + 'test.noop', + 'alertsFixture' + ), + statusCode: 403, + }); + break; + case 'superuser at space1': + case 'space_1_all at space1': + case 'space_1_all_alerts_none_actions at space1': + case 'space_1_all_with_restricted_fixture at space1': + expect(response.statusCode).to.eql(200); + await retry.try(async () => { + const alertTask = (await getAlertingTaskById(createdAlert.scheduledTaskId)).docs[0]; + expect(alertTask.status).to.eql('idle'); + // ensure the alert is rescheduled to a minute from now + ensureDatetimeIsWithinRange(Date.parse(alertTask.runAt), 60 * 1000); + }); + break; + default: + throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); + } + }); + it('should handle updates to an alert schedule by setting the new schedule for the underlying task', async () => { const { body: createdAlert } = await supertest .post(`${getUrlPrefix(space.id)}/api/alerts/alert`) From 53d5064f210e812afd34610b079590b7680f19d0 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 6 Nov 2020 15:10:41 -0800 Subject: [PATCH 11/26] Fixed due to comments --- .../server/alerts_client/alerts_client.ts | 39 +++--- .../server/alerts_client/tests/create.test.ts | 2 +- .../server/alerts_client/tests/delete.test.ts | 28 +++-- .../alerts_client/tests/disable.test.ts | 21 ++-- .../server/alerts_client/tests/enable.test.ts | 26 ++-- .../server/alerts_client/tests/update.test.ts | 10 +- .../tests/update_api_key.test.ts | 37 +++--- .../add_to_invalidate_api_keys.ts | 25 ---- .../mark_api_key_for_invalidation.ts | 24 ++++ .../invalidate_pending_api_keys/task.ts | 113 +++++++++--------- x-pack/plugins/alerts/server/plugin.ts | 21 +--- .../alerts/server/saved_objects/index.ts | 4 +- .../fixtures/plugins/alerts/server/routes.ts | 4 +- 13 files changed, 182 insertions(+), 172 deletions(-) delete mode 100644 x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts create mode 100644 x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index a9b6e27ea5cd3..e97b37f16faf0 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -13,7 +13,6 @@ import { SavedObjectReference, SavedObject, PluginInitializerContext, - ISavedObjectsRepository, } from 'src/core/server'; import { esKuery } from '../../../../../src/plugins/data/server'; import { ActionsClient, ActionsAuthorization } from '../../../actions/server'; @@ -48,7 +47,7 @@ import { IEvent } from '../../../event_log/server'; import { parseDuration } from '../../common/parse_duration'; import { retryIfConflicts } from '../lib/retry_if_conflicts'; import { partiallyUpdateAlert } from '../saved_objects'; -import { addToInvalidateApiKeys } from '../invalidate_pending_api_keys/add_to_invalidate_api_keys'; +import { markApiKeyForInvalidation } from '../invalidate_pending_api_keys/mark_api_key_for_invalidation'; export interface RegistryAlertTypeWithAuth extends RegistryAlertType { authorizedConsumers: string[]; @@ -258,10 +257,10 @@ export class AlertsClient { ); } catch (e) { // Avoid unused API key - addToInvalidateApiKeys( + markApiKeyForInvalidation( { apiKey: rawAlert.apiKey }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); throw e; } @@ -487,10 +486,10 @@ export class AlertsClient { await Promise.all([ taskIdToRemove ? deleteTaskIfItExists(this.taskManager, taskIdToRemove) : null, apiKeyToInvalidate - ? addToInvalidateApiKeys( + ? markApiKeyForInvalidation( { apiKey: apiKeyToInvalidate }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ) : null, ]); @@ -531,10 +530,10 @@ export class AlertsClient { await Promise.all([ alertSavedObject.attributes.apiKey - ? addToInvalidateApiKeys( + ? markApiKeyForInvalidation( { apiKey: alertSavedObject.attributes.apiKey }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ) : null, (async () => { @@ -600,10 +599,10 @@ export class AlertsClient { ); } catch (e) { // Avoid unused API key - addToInvalidateApiKeys( + markApiKeyForInvalidation( { apiKey: createAttributes.apiKey }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); throw e; } @@ -684,19 +683,19 @@ export class AlertsClient { await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version }); } catch (e) { // Avoid unused API key - addToInvalidateApiKeys( + markApiKeyForInvalidation( { apiKey: updateAttributes.apiKey }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); throw e; } if (apiKeyToInvalidate) { - await addToInvalidateApiKeys( + await markApiKeyForInvalidation( { apiKey: apiKeyToInvalidate }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); } } @@ -757,10 +756,10 @@ export class AlertsClient { await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version }); } catch (e) { // Avoid unused API key - addToInvalidateApiKeys( + markApiKeyForInvalidation( { apiKey: updateAttributes.apiKey }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); throw e; } @@ -773,10 +772,10 @@ export class AlertsClient { scheduledTaskId: scheduledTask.id, }); if (apiKeyToInvalidate) { - await addToInvalidateApiKeys( + await markApiKeyForInvalidation( { apiKey: apiKeyToInvalidate }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ); } } @@ -839,10 +838,10 @@ export class AlertsClient { ? deleteTaskIfItExists(this.taskManager, attributes.scheduledTaskId) : null, apiKeyToInvalidate - ? await addToInvalidateApiKeys( + ? await markApiKeyForInvalidation( { apiKey: apiKeyToInvalidate }, this.logger, - (this.unsecuredSavedObjectsClient as unknown) as ISavedObjectsRepository + this.unsecuredSavedObjectsClient ) : null, ]); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index f3e3f46ff5eff..e13700567f627 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -732,7 +732,7 @@ describe('create()', () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Test failure')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 02018a9e34df6..9e06274742ebc 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -95,7 +95,7 @@ describe('delete()', () => { test('successfully removes an alert', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -105,9 +105,12 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); @@ -118,7 +121,7 @@ describe('delete()', () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -152,7 +155,7 @@ describe('delete()', () => { test(`doesn't invalidate API key when apiKey is null`, async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -173,18 +176,21 @@ describe('delete()', () => { test('swallows error when invalidate API key throws', async () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.delete({ id: '1' }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for invalidating API Key: Fail' + 'Failed to mark for API key [id="123"] for invalidation: Fail' ); }); test('swallows error when getDecryptedAsInternalUser throws an error', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index 0baea8ce4d9fc..709d9ad2f19c8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -109,7 +109,7 @@ describe('disable()', () => { test('disables an alert', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', }, @@ -152,16 +152,19 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -220,7 +223,7 @@ describe('disable()', () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -236,7 +239,7 @@ describe('disable()', () => { test(`doesn't invalidate when no API key is used`, async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -251,7 +254,7 @@ describe('disable()', () => { test('swallows error when failing to load decrypted saved object', async () => { unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -280,7 +283,7 @@ describe('disable()', () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.disable({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for invalidating API Key: Fail' + 'Failed to mark for API key [id="123"] for invalidation: Fail' ); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index ee1b6dc0d5992..100e9b1236958 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -158,7 +158,7 @@ describe('enable()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', }, @@ -171,7 +171,7 @@ describe('enable()', () => { namespace: 'default', }); expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( - 'invalidate_pending_api_key', + 'api_key_pending_invalidation', { apiKeyId: '123', } @@ -238,7 +238,7 @@ describe('enable()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', }, @@ -250,9 +250,12 @@ describe('enable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); }); test(`doesn't enable already enabled alerts`, async () => { @@ -342,7 +345,7 @@ describe('enable()', () => { unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail to update')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', }, @@ -354,9 +357,12 @@ describe('enable()', () => { ); expect(alertsClientParams.getUserName).toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 7cdae887a53d6..5482d5a040e64 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -162,7 +162,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', }, @@ -385,7 +385,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', }, @@ -393,7 +393,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', }, @@ -555,7 +555,7 @@ describe('update()', () => { }); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', }, @@ -828,7 +828,7 @@ describe('update()', () => { }, }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for invalidating API Key: Fail' + 'Failed to mark for API key [id="123"] for invalidation: Fail' ); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index fb6ffd87c1018..cdf797ded4039 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -81,7 +81,7 @@ describe('updateApiKey()', () => { unsecuredSavedObjectsClient.get.mockResolvedValue(existingAlert); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -128,16 +128,19 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); }); test('falls back to SOC when getDecryptedAsInternalUser throws an error', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -185,16 +188,19 @@ describe('updateApiKey()', () => { await alertsClient.updateApiKey({ id: '1' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '123', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '123', + } + ); }); test('swallows error when getting decrypted object throws', async () => { encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -217,7 +223,7 @@ describe('updateApiKey()', () => { unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail')); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', }, @@ -228,14 +234,17 @@ describe('updateApiKey()', () => { `"Fail"` ); expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( - 'invalidate_pending_api_key', + 'api_key_pending_invalidation', { apiKeyId: '123', } ); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith('invalidate_pending_api_key', { - apiKeyId: '234', - }); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( + 'api_key_pending_invalidation', + { + apiKeyId: '234', + } + ); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts deleted file mode 100644 index 5c53097debbf0..0000000000000 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/add_to_invalidate_api_keys.ts +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -import { Logger, ISavedObjectsRepository } from 'src/core/server'; - -export const addToInvalidateApiKeys = async ( - { apiKey }: { apiKey: string | null }, - logger: Logger, - internalSavedObjectsRepository: ISavedObjectsRepository -): Promise => { - if (!apiKey) { - return; - } - - try { - const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; - await internalSavedObjectsRepository.create('invalidate_pending_api_key', { - apiKeyId, - }); - } catch (e) { - logger.error(`Failed to mark for invalidating API Key: ${e.message}`); - } -}; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts new file mode 100644 index 0000000000000..19c29faf3e3cb --- /dev/null +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts @@ -0,0 +1,24 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { Logger, SavedObjectsClientContract } from 'src/core/server'; + +export const markApiKeyForInvalidation = async ( + { apiKey }: { apiKey: string | null }, + logger: Logger, + internalSavedObjectsRepository: SavedObjectsClientContract +): Promise => { + if (!apiKey) { + return; + } + const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; + try { + await internalSavedObjectsRepository.create('api_key_pending_invalidation', { + apiKeyId, + }); + } catch (e) { + logger.error(`Failed to mark for API key [id="${apiKeyId}"] for invalidation: ${e.message}`); + } +}; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 4a6aefc5d24fa..8305eaf906e79 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Logger, ISavedObjectsRepository } from 'kibana/server'; +import { Logger, CoreStart } from 'kibana/server'; import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { RunContext, @@ -13,6 +13,7 @@ import { } from '../../../task_manager/server'; import { InvalidateAPIKeyResult } from '../alerts_client'; import { AlertsConfig } from '../config'; +import { AlertingPluginsStart } from '../plugin'; import { InvalidatePendingApiKey } from '../types'; const TASK_TYPE = 'alerts_invalidate_api_keys'; @@ -38,52 +39,24 @@ const invalidateAPIKey = async ( }; }; -export function initializeAlertsInvalidateApiKeys( +export function initializeApiKeyInvalidator( logger: Logger, - internalSavedObjectsRepository: Promise<{ - createInternalRepository: () => ISavedObjectsRepository; - }>, + coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, taskManager: TaskManagerSetupContract, securityPluginSetup?: SecurityPluginSetup ) { - registerAlertsInvalidateApiKeysTask( + registerApiKeyInvalitorTaskDefinition( logger, - internalSavedObjectsRepository, + coreStartServices, taskManager, securityPluginSetup ); } -export function scheduleAlertsInvalidateApiKeys( +export async function scheduleApiKeyInvalidatorTask( logger: Logger, config: Promise, - taskManager?: TaskManagerStartContract -) { - if (taskManager) { - scheduleTasks(logger, taskManager, config); - } -} - -function registerAlertsInvalidateApiKeysTask( - logger: Logger, - internalSavedObjectsRepository: Promise<{ - createInternalRepository: () => ISavedObjectsRepository; - }>, - taskManager: TaskManagerSetupContract, - securityPluginSetup?: SecurityPluginSetup -) { - taskManager.registerTaskDefinitions({ - [TASK_TYPE]: { - title: 'Invalidate Alerts API Keys', - createTaskRunner: taskRunner(logger, internalSavedObjectsRepository, securityPluginSetup), - }, - }); -} - -async function scheduleTasks( - logger: Logger, - taskManager: TaskManagerStartContract, - config: Promise + taskManager: TaskManagerStartContract ) { const interval = (await config).invalidateApiKeysTask.interval; try { @@ -101,11 +74,23 @@ async function scheduleTasks( } } +function registerApiKeyInvalitorTaskDefinition( + logger: Logger, + coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, + taskManager: TaskManagerSetupContract, + securityPluginSetup?: SecurityPluginSetup +) { + taskManager.registerTaskDefinitions({ + [TASK_TYPE]: { + title: 'Invalidate alert API Keys', + createTaskRunner: taskRunner(logger, coreStartServices, securityPluginSetup), + }, + }); +} + function taskRunner( logger: Logger, - internalSavedObjectsRepository: Promise<{ - createInternalRepository: () => ISavedObjectsRepository; - }>, + coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, securityPluginSetup?: SecurityPluginSetup ) { return ({ taskInstance }: RunContext) => { @@ -114,28 +99,40 @@ function taskRunner( async run() { let totalInvalidated = 0; try { - const repository = (await internalSavedObjectsRepository).createInternalRepository(); - const apiKeysToInvalidate = await repository.find({ - type: 'invalidate_pending_api_key', + const [{ savedObjects }] = await coreStartServices; + const repository = savedObjects.createInternalRepository(); + const totalApiKeysToInvalidate = await repository.find({ + type: 'api_key_pending_invalidation', + page: 1, + perPage: 0, }); - apiKeysToInvalidate.saved_objects.forEach(async (obj) => { - const response = await invalidateAPIKey( - { id: obj.attributes.apiKeyId }, - securityPluginSetup - ); - if (response.apiKeysEnabled === true && response.result.error_count > 0) { - logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); - } else { - try { - await repository.delete('invalidate_pending_api_key', obj.id); - totalInvalidated++; - } catch (err) { - logger.error( - `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` - ); + let perPageLimit = Math.min(totalApiKeysToInvalidate.total, 100); + while (perPageLimit <= totalApiKeysToInvalidate.total) { + const apiKeysToInvalidate = await repository.find({ + type: 'api_key_pending_invalidation', + page: 1, + perPage: totalApiKeysToInvalidate.total, + }); + apiKeysToInvalidate.saved_objects.forEach(async (obj) => { + const response = await invalidateAPIKey( + { id: obj.attributes.apiKeyId }, + securityPluginSetup + ); + if (response.apiKeysEnabled === true && response.result.error_count > 0) { + logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); + } else { + try { + await repository.delete('api_key_pending_invalidation', obj.id); + totalInvalidated++; + } catch (err) { + logger.error( + `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` + ); + } } - } - }); + }); + perPageLimit += 100; + } return { state: { runs: (state.runs || 0) + 1, diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index cfa2c59a1b7ed..642bbfaa85242 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -62,11 +62,11 @@ import { initializeAlertingTelemetry, scheduleAlertingTelemetry } from './usage/ import { IEventLogger, IEventLogService, IEventLogClientService } from '../../event_log/server'; import { PluginStartContract as FeaturesPluginStart } from '../../features/server'; import { setupSavedObjects } from './saved_objects'; +import { AlertsConfig } from './config'; import { - initializeAlertsInvalidateApiKeys, - scheduleAlertsInvalidateApiKeys, + initializeApiKeyInvalidator, + scheduleApiKeyInvalidatorTask, } from './invalidate_pending_api_keys/task'; -import { AlertsConfig } from './config'; export const EVENT_LOG_PROVIDER = 'alerting'; export const EVENT_LOG_ACTIONS = { @@ -193,9 +193,9 @@ export class AlertingPlugin { }); } - initializeAlertsInvalidateApiKeys( + initializeApiKeyInvalidator( this.logger, - this.createSOInternalRepositoryContext(core), + core.getStartServices(), plugins.taskManager, this.security ); @@ -290,7 +290,7 @@ export class AlertingPlugin { scheduleAlertingTelemetry(this.telemetryLogger, plugins.taskManager); - scheduleAlertsInvalidateApiKeys(this.telemetryLogger, this.config, plugins.taskManager); + scheduleApiKeyInvalidatorTask(this.telemetryLogger, this.config, plugins.taskManager); return { listTypes: alertTypeRegistry!.list.bind(this.alertTypeRegistry!), @@ -298,15 +298,6 @@ export class AlertingPlugin { }; } - private createSOInternalRepositoryContext = async (core: CoreSetup) => { - const [{ savedObjects }] = await core.getStartServices(); - return { - createInternalRepository: () => { - return savedObjects.createInternalRepository(['alert', 'invalidate_pending_api_key']); - }, - }; - }; - private createRouteHandlerContext = ( core: CoreSetup ): IContextProvider, 'alerting'> => { diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 44eb990a7a34d..48205eafc4008 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -43,8 +43,8 @@ export function setupSavedObjects( }); savedObjects.registerType({ - name: 'invalidate_pending_api_key', - hidden: false, + name: 'api_key_pending_invalidation', + hidden: true, namespaceType: 'single', mappings: { properties: { diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index 54735990728a5..7ba46a617ad2c 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -198,9 +198,9 @@ export function defineRoutes( ): Promise> => { try { const [{ savedObjects }] = await core.getStartServices(); - const repository = savedObjects.createInternalRepository(['invalidate_pending_api_key']); + const repository = savedObjects.createInternalRepository(['api_key_pending_invalidation']); const apiKeysToInvalidate = await repository.find({ - type: 'invalidate_pending_api_key', + type: 'api_key_pending_invalidation', }); return res.ok({ body: { apiKeysToInvalidate }, From 247fe90904db6ba1061a832e32f50d57f8c11fa7 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Sun, 8 Nov 2020 21:42:34 -0800 Subject: [PATCH 12/26] added configurable delay for invalidation task --- .../server/alerts_client/tests/create.test.ts | 3 ++ .../server/alerts_client/tests/enable.test.ts | 9 ++++ x-pack/plugins/alerts/server/config.test.ts | 1 + x-pack/plugins/alerts/server/config.ts | 1 + .../mark_api_key_for_invalidation.test.ts | 38 +++++++++++++ .../mark_api_key_for_invalidation.ts | 1 + .../invalidate_pending_api_keys/task.ts | 10 +++- .../plugins/alerts/server/lib/get_cadence.ts | 53 +++++++++++++++++++ x-pack/plugins/alerts/server/plugin.ts | 1 + .../alerts/server/saved_objects/index.ts | 3 ++ x-pack/plugins/alerts/server/types.ts | 1 + 11 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts create mode 100644 x-pack/plugins/alerts/server/lib/get_cadence.ts diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index e13700567f627..cf80692a3cb16 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -730,11 +730,13 @@ describe('create()', () => { ], }); unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Test failure')); + const createdAt = new Date().toISOString(); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt, }, references: [], }); @@ -745,6 +747,7 @@ describe('create()', () => { expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ apiKeyId: '123', + createdAt, }); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 100e9b1236958..dff7ddd887597 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -146,6 +146,7 @@ describe('enable()', () => { }); test('enables an alert', async () => { + const createdAt = new Date().toISOString(); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ ...existingAlert, attributes: { @@ -161,6 +162,7 @@ describe('enable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', + createdAt, }, references: [], }); @@ -174,6 +176,7 @@ describe('enable()', () => { 'api_key_pending_invalidation', { apiKeyId: '123', + createdAt, } ); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); @@ -229,6 +232,7 @@ describe('enable()', () => { }); test('invalidates API key if ever one existed prior to updating', async () => { + const createdAt = new Date().toISOString(); encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue({ ...existingAlert, attributes: { @@ -241,6 +245,7 @@ describe('enable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', + createdAt, }, references: [], }); @@ -254,6 +259,7 @@ describe('enable()', () => { 'api_key_pending_invalidation', { apiKeyId: '123', + createdAt, } ); }); @@ -337,6 +343,7 @@ describe('enable()', () => { }); test('throws error when failing to update the first time', async () => { + const createdAt = new Date().toISOString(); alertsClientParams.createAPIKey.mockResolvedValueOnce({ apiKeysEnabled: true, result: { id: '123', name: '123', api_key: 'abc' }, @@ -348,6 +355,7 @@ describe('enable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', + createdAt, }, references: [], }); @@ -361,6 +369,7 @@ describe('enable()', () => { 'api_key_pending_invalidation', { apiKeyId: '123', + createdAt, } ); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); diff --git a/x-pack/plugins/alerts/server/config.test.ts b/x-pack/plugins/alerts/server/config.test.ts index 11afb9c7bc193..c1fee4c1dac60 100644 --- a/x-pack/plugins/alerts/server/config.test.ts +++ b/x-pack/plugins/alerts/server/config.test.ts @@ -12,6 +12,7 @@ describe('config validation', () => { Object { "invalidateApiKeysTask": Object { "interval": "5m", + "removalDelay": "5m", }, } `); diff --git a/x-pack/plugins/alerts/server/config.ts b/x-pack/plugins/alerts/server/config.ts index b762e947ca9fe..ddbda34b8bccf 100644 --- a/x-pack/plugins/alerts/server/config.ts +++ b/x-pack/plugins/alerts/server/config.ts @@ -10,6 +10,7 @@ import { validateDurationSchema } from './lib'; export const configSchema = schema.object({ invalidateApiKeysTask: schema.object({ interval: schema.string({ validate: validateDurationSchema, defaultValue: '5m' }), + removalDelay: schema.string({ validate: validateDurationSchema, defaultValue: '5m' }), }), }); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts new file mode 100644 index 0000000000000..c38c9cdb68dbd --- /dev/null +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { loggingSystemMock, savedObjectsClientMock } from '../../../../../src/core/server/mocks'; +import { markApiKeyForInvalidation } from './mark_api_key_for_invalidation'; + +describe('markApiKeyForInvalidation', () => { + test('should call savedObjectsClient create with the proper params', async () => { + const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); + const createdAt = new Date().toISOString(); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'api_key_pending_invalidation', + attributes: { + apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', + }, + references: [], + }); + markApiKeyForInvalidation( + { apiKey: Buffer.from('123:abc').toString('base64') }, + loggingSystemMock.create().get(), + unsecuredSavedObjectsClient + ); + expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.create.mock.calls[0]).toHaveLength(2); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual( + 'api_key_pending_invalidation' + ); + expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchObject({ + apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', + }); + }); +}); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts index 19c29faf3e3cb..ee4fd38086396 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts @@ -17,6 +17,7 @@ export const markApiKeyForInvalidation = async ( try { await internalSavedObjectsRepository.create('api_key_pending_invalidation', { apiKeyId, + createdAt: new Date().toISOString(), }); } catch (e) { logger.error(`Failed to mark for API key [id="${apiKeyId}"] for invalidation: ${e.message}`); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 8305eaf906e79..8c54c4396d54b 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -13,6 +13,7 @@ import { } from '../../../task_manager/server'; import { InvalidateAPIKeyResult } from '../alerts_client'; import { AlertsConfig } from '../config'; +import { timePeriodBeforeDate } from '../lib/get_cadence'; import { AlertingPluginsStart } from '../plugin'; import { InvalidatePendingApiKey } from '../types'; @@ -43,12 +44,14 @@ export function initializeApiKeyInvalidator( logger: Logger, coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, taskManager: TaskManagerSetupContract, + config: Promise, securityPluginSetup?: SecurityPluginSetup ) { registerApiKeyInvalitorTaskDefinition( logger, coreStartServices, taskManager, + config, securityPluginSetup ); } @@ -78,12 +81,13 @@ function registerApiKeyInvalitorTaskDefinition( logger: Logger, coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, taskManager: TaskManagerSetupContract, + config: Promise, securityPluginSetup?: SecurityPluginSetup ) { taskManager.registerTaskDefinitions({ [TASK_TYPE]: { title: 'Invalidate alert API Keys', - createTaskRunner: taskRunner(logger, coreStartServices, securityPluginSetup), + createTaskRunner: taskRunner(logger, coreStartServices, config, securityPluginSetup), }, }); } @@ -91,6 +95,7 @@ function registerApiKeyInvalitorTaskDefinition( function taskRunner( logger: Logger, coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, + config: Promise, securityPluginSetup?: SecurityPluginSetup ) { return ({ taskInstance }: RunContext) => { @@ -101,8 +106,11 @@ function taskRunner( try { const [{ savedObjects }] = await coreStartServices; const repository = savedObjects.createInternalRepository(); + const configuredDelay = await (await config).invalidateApiKeysTask.removalDelay; + const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); const totalApiKeysToInvalidate = await repository.find({ type: 'api_key_pending_invalidation', + filter: `api_key_pending_invalidation.attributes.createdAt < ${delay}`, page: 1, perPage: 0, }); diff --git a/x-pack/plugins/alerts/server/lib/get_cadence.ts b/x-pack/plugins/alerts/server/lib/get_cadence.ts new file mode 100644 index 0000000000000..d09ed0c2122cd --- /dev/null +++ b/x-pack/plugins/alerts/server/lib/get_cadence.ts @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { memoize } from 'lodash'; + +export enum TimeUnit { + Minute = 'm', + Second = 's', + Hour = 'h', + Day = 'd', +} +const VALID_CADENCE = new Set(Object.values(TimeUnit)); +const CADENCE_IN_MS: Record = { + [TimeUnit.Second]: 1000, + [TimeUnit.Minute]: 60 * 1000, + [TimeUnit.Hour]: 60 * 60 * 1000, + [TimeUnit.Day]: 24 * 60 * 60 * 1000, +}; + +const isNumeric = (numAsStr: string) => /^\d+$/.test(numAsStr); + +export const parseIntervalAsMillisecond = memoize((value: string): number => { + const numericAsStr: string = value.slice(0, -1); + const numeric: number = parseInt(numericAsStr, 10); + const cadence: TimeUnit | string = value.slice(-1); + if ( + !VALID_CADENCE.has(cadence as TimeUnit) || + isNaN(numeric) || + numeric <= 0 || + !isNumeric(numericAsStr) + ) { + throw new Error( + `Invalid time value "${value}". Time must be of the form {number}m. Example: 5m.` + ); + } + return numeric * CADENCE_IN_MS[cadence as TimeUnit]; +}); + +/** + * Returns a date that is the specified interval from given date. + * + * @param {Date} date - The date to add interval to + * @param {string} interval - THe time of the form `Nm` such as `5m` + */ +export function timePeriodBeforeDate(date: Date, timePeriod: string): Date { + const result = new Date(date.valueOf()); + const milisecFromTime = parseIntervalAsMillisecond(timePeriod); + result.setMilliseconds(result.getMilliseconds() - milisecFromTime); + return result; +} diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 642bbfaa85242..be01a45576aca 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -197,6 +197,7 @@ export class AlertingPlugin { this.logger, core.getStartServices(), plugins.taskManager, + this.config, this.security ); diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 48205eafc4008..0be5c1d47ed12 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -51,6 +51,9 @@ export function setupSavedObjects( apiKeyId: { type: 'keyword', }, + createdAt: { + type: 'date', + }, }, }, }); diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index fe15ad9ff4714..41e5a624bf260 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -180,6 +180,7 @@ export interface AlertsConfigType { export interface InvalidatePendingApiKey { apiKeyId: string; + createdAt: string; } export type AlertTypeRegistry = PublicMethodsOf; From 121625c8a9f33f67cdeb08f21281c251a27b2586 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Sun, 8 Nov 2020 21:50:28 -0800 Subject: [PATCH 13/26] added interval to the task response --- .../alerts/server/invalidate_pending_api_keys/task.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 8c54c4396d54b..267617d1d4a7f 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -103,10 +103,11 @@ function taskRunner( return { async run() { let totalInvalidated = 0; + const configResult = await config; try { const [{ savedObjects }] = await coreStartServices; const repository = savedObjects.createInternalRepository(); - const configuredDelay = await (await config).invalidateApiKeysTask.removalDelay; + const configuredDelay = await configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); const totalApiKeysToInvalidate = await repository.find({ type: 'api_key_pending_invalidation', @@ -145,6 +146,9 @@ function taskRunner( state: { runs: (state.runs || 0) + 1, total_invalidated: totalInvalidated, + schedule: { + interval: configResult.invalidateApiKeysTask.interval, + }, }, }; } catch (errMsg) { @@ -153,6 +157,9 @@ function taskRunner( state: { runs: (state.runs || 0) + 1, total_invalidated: totalInvalidated, + schedule: { + interval: configResult.invalidateApiKeysTask.interval, + }, }, }; } From d6771682f51c1e10c5dce097c4ac1ec97c966dfa Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 09:19:04 -0800 Subject: [PATCH 14/26] Fixed jest tests --- .../server/alerts_client/tests/delete.test.ts | 18 ++++----- .../alerts_client/tests/disable.test.ts | 19 +++++---- .../server/alerts_client/tests/enable.test.ts | 29 ++++---------- .../server/alerts_client/tests/update.test.ts | 11 +++-- .../tests/update_api_key.test.ts | 40 +++++++------------ .../mark_api_key_for_invalidation.test.ts | 5 --- x-pack/plugins/alerts/server/plugin.test.ts | 3 ++ x-pack/plugins/alerts/server/types.ts | 1 + 8 files changed, 54 insertions(+), 72 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 9e06274742ebc..358510a774770 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -98,6 +98,7 @@ describe('delete()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -105,11 +106,8 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith('alert', '1'); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toBe( + 'api_key_pending_invalidation' ); expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', @@ -124,6 +122,7 @@ describe('delete()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -158,6 +157,7 @@ describe('delete()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -176,11 +176,8 @@ describe('delete()', () => { test('swallows error when invalidate API key throws', async () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.delete({ id: '1' }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toBe( + 'api_key_pending_invalidation' ); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( 'Failed to mark for API key [id="123"] for invalidation: Fail' @@ -193,6 +190,7 @@ describe('delete()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index 709d9ad2f19c8..b68eb24be4414 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -13,6 +13,7 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup } from './lib'; +import { InvalidatePendingApiKey } from '../../types'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -112,6 +113,7 @@ describe('disable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -152,12 +154,9 @@ describe('disable()', () => { } ); expect(taskManager.remove).toHaveBeenCalledWith('task-123'); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } - ); + expect( + (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId + ).toBe('123'); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { @@ -167,6 +166,7 @@ describe('disable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -226,6 +226,7 @@ describe('disable()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -241,7 +242,8 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -256,7 +258,8 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index dff7ddd887597..16e83c42d8930 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -14,6 +14,7 @@ import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { TaskStatus } from '../../../../task_manager/server'; import { getBeforeSetup } from './lib'; +import { InvalidatePendingApiKey } from '../../types'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -172,13 +173,7 @@ describe('enable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - createdAt, - } - ); + expect(unsecuredSavedObjectsClient.create).not.toBeCalledWith('api_key_pending_invalidation'); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith( 'alert', @@ -255,13 +250,9 @@ describe('enable()', () => { expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { namespace: 'default', }); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - createdAt, - } - ); + expect( + (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId + ).toBe('123'); }); test(`doesn't enable already enabled alerts`, async () => { @@ -365,13 +356,9 @@ describe('enable()', () => { ); expect(alertsClientParams.getUserName).toHaveBeenCalled(); expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - createdAt, - } - ); + expect( + (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId + ).toBe('123'); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 5482d5a040e64..9de47dde0fad5 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -10,7 +10,7 @@ import { savedObjectsClientMock, loggingSystemMock } from '../../../../../../src import { taskManagerMock } from '../../../../task_manager/server/mocks'; import { alertTypeRegistryMock } from '../../alert_type_registry.mock'; import { alertsAuthorizationMock } from '../../authorization/alerts_authorization.mock'; -import { IntervalSchedule } from '../../types'; +import { IntervalSchedule, InvalidatePendingApiKey } from '../../types'; import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/server/mocks'; import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; @@ -165,6 +165,7 @@ describe('update()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -388,6 +389,7 @@ describe('update()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -396,6 +398,7 @@ describe('update()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -558,6 +561,7 @@ describe('update()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -996,8 +1000,9 @@ describe('update()', () => { }, }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); - expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith({ apiKeyId: '123' }); - expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ apiKeyId: '234' }); + expect( + (unsecuredSavedObjectsClient.create.mock.calls[1][1] as InvalidatePendingApiKey).apiKeyId + ).toBe('234'); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index cdf797ded4039..cd6d63d4d8662 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -13,6 +13,7 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup } from './lib'; +import { InvalidatePendingApiKey } from '../../types'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -83,7 +84,8 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -128,11 +130,8 @@ describe('updateApiKey()', () => { }, { version: '123' } ); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toBe( + 'api_key_pending_invalidation' ); }); @@ -142,7 +141,8 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: '123', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -188,11 +188,8 @@ describe('updateApiKey()', () => { await alertsClient.updateApiKey({ id: '1' }); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalled(); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } + expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toBe( + 'api_key_pending_invalidation' ); }); @@ -203,6 +200,7 @@ describe('updateApiKey()', () => { type: 'api_key_pending_invalidation', attributes: { apiKeyId: 'test', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -225,7 +223,8 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: '234', + createdAt: '2019-02-12T21:01:22.479Z', }, references: [], }); @@ -233,18 +232,9 @@ describe('updateApiKey()', () => { await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot( `"Fail"` ); - expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '123', - } - ); - expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith( - 'api_key_pending_invalidation', - { - apiKeyId: '234', - } - ); + expect( + (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId + ).toBe('234'); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts index c38c9cdb68dbd..13f5a7dcd7eac 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts @@ -10,7 +10,6 @@ import { markApiKeyForInvalidation } from './mark_api_key_for_invalidation'; describe('markApiKeyForInvalidation', () => { test('should call savedObjectsClient create with the proper params', async () => { const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); - const createdAt = new Date().toISOString(); unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ id: '1', type: 'api_key_pending_invalidation', @@ -30,9 +29,5 @@ describe('markApiKeyForInvalidation', () => { expect(unsecuredSavedObjectsClient.create.mock.calls[0][0]).toEqual( 'api_key_pending_invalidation' ); - expect(unsecuredSavedObjectsClient.create.mock.calls[0][1]).toMatchObject({ - apiKeyId: '123', - createdAt: '2019-02-12T21:01:22.479Z', - }); }); }); diff --git a/x-pack/plugins/alerts/server/plugin.test.ts b/x-pack/plugins/alerts/server/plugin.test.ts index e8da662880de9..8685c247c6271 100644 --- a/x-pack/plugins/alerts/server/plugin.test.ts +++ b/x-pack/plugins/alerts/server/plugin.test.ts @@ -21,6 +21,7 @@ describe('Alerting Plugin', () => { const context = coreMock.createPluginInitializerContext({ invalidateApiKeysTask: { interval: '5m', + removalDelay: '5m', }, }); const plugin = new AlertingPlugin(context); @@ -63,6 +64,7 @@ describe('Alerting Plugin', () => { const context = coreMock.createPluginInitializerContext({ invalidateApiKeysTask: { interval: '5m', + removalDelay: '5m', }, }); const plugin = new AlertingPlugin(context); @@ -110,6 +112,7 @@ describe('Alerting Plugin', () => { const context = coreMock.createPluginInitializerContext({ invalidateApiKeysTask: { interval: '5m', + removalDelay: '5m', }, }); const plugin = new AlertingPlugin(context); diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 41e5a624bf260..5a59580e3838e 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -175,6 +175,7 @@ export interface AlertingPlugin { export interface AlertsConfigType { invalidateApiKeysTask: { interval: string; + removalDelay: string; }; } From 3faf8948a167f1a07f5638504afa67c31f683d4e Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 11:32:22 -0800 Subject: [PATCH 15/26] Fixed due to comments --- .../invalidate_pending_api_keys/task.ts | 80 ++++++++++++------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 267617d1d4a7f..c165d7bd0f8de 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -4,7 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Logger, CoreStart } from 'kibana/server'; +import { + Logger, + CoreStart, + SavedObjectsFindResponse, + ISavedObjectsRepository, +} from 'kibana/server'; import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { RunContext, @@ -109,39 +114,27 @@ function taskRunner( const repository = savedObjects.createInternalRepository(); const configuredDelay = await configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); - const totalApiKeysToInvalidate = await repository.find({ - type: 'api_key_pending_invalidation', - filter: `api_key_pending_invalidation.attributes.createdAt < ${delay}`, - page: 1, - perPage: 0, - }); - let perPageLimit = Math.min(totalApiKeysToInvalidate.total, 100); - while (perPageLimit <= totalApiKeysToInvalidate.total) { + + let hasApiKeysPendingInvalidation = true; + let pageNumber = 1; + const PAGE_SIZE = 100; + do { const apiKeysToInvalidate = await repository.find({ type: 'api_key_pending_invalidation', - page: 1, - perPage: totalApiKeysToInvalidate.total, - }); - apiKeysToInvalidate.saved_objects.forEach(async (obj) => { - const response = await invalidateAPIKey( - { id: obj.attributes.apiKeyId }, - securityPluginSetup - ); - if (response.apiKeysEnabled === true && response.result.error_count > 0) { - logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); - } else { - try { - await repository.delete('api_key_pending_invalidation', obj.id); - totalInvalidated++; - } catch (err) { - logger.error( - `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` - ); - } - } + filter: `api_key_pending_invalidation.attributes.createdAt < ${delay}`, + page: pageNumber, + perPage: PAGE_SIZE, }); - perPageLimit += 100; - } + totalInvalidated += manageToInvalidateApiKeys( + logger, + repository, + apiKeysToInvalidate, + securityPluginSetup + ); + hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > 0; + pageNumber++; + } while (hasApiKeysPendingInvalidation); + return { state: { runs: (state.runs || 0) + 1, @@ -167,3 +160,28 @@ function taskRunner( }; }; } + +function manageToInvalidateApiKeys( + logger: Logger, + repository: ISavedObjectsRepository, + apiKeysToInvalidate: SavedObjectsFindResponse, + securityPluginSetup?: SecurityPluginSetup +) { + let totalInvalidated = 0; + apiKeysToInvalidate.saved_objects.forEach(async (obj) => { + const response = await invalidateAPIKey({ id: obj.attributes.apiKeyId }, securityPluginSetup); + if (response.apiKeysEnabled === true && response.result.error_count > 0) { + logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); + } else { + try { + await repository.delete('api_key_pending_invalidation', obj.id); + totalInvalidated++; + } catch (err) { + logger.error( + `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` + ); + } + } + }); + return totalInvalidated; +} From 56cba8edfc4f48feccd67319993977cf9f973849 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 15:05:06 -0800 Subject: [PATCH 16/26] Fixed task --- .../alerts/server/alerts_client_factory.ts | 2 +- .../invalidate_pending_api_keys/task.ts | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client_factory.ts b/x-pack/plugins/alerts/server/alerts_client_factory.ts index 1d58869d009f7..069703be72f8a 100644 --- a/x-pack/plugins/alerts/server/alerts_client_factory.ts +++ b/x-pack/plugins/alerts/server/alerts_client_factory.ts @@ -94,7 +94,7 @@ export class AlertsClientFactory { alertTypeRegistry: this.alertTypeRegistry, unsecuredSavedObjectsClient: savedObjects.getScopedClient(request, { excludedWrappers: ['security'], - includedHiddenTypes: ['alert'], + includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }), authorization, actionsAuthorization: actions.getActionsAuthorizationWithRequest(request), diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index c165d7bd0f8de..cad57b9f54cb2 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -111,7 +111,9 @@ function taskRunner( const configResult = await config; try { const [{ savedObjects }] = await coreStartServices; - const repository = savedObjects.createInternalRepository(); + const repository = savedObjects.createInternalRepository([ + 'api_key_pending_invalidation', + ]); const configuredDelay = await configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); @@ -121,7 +123,7 @@ function taskRunner( do { const apiKeysToInvalidate = await repository.find({ type: 'api_key_pending_invalidation', - filter: `api_key_pending_invalidation.attributes.createdAt < ${delay}`, + filter: `api_key_pending_invalidation.attributes.createdAt <= ${delay}`, page: pageNumber, perPage: PAGE_SIZE, }); @@ -131,28 +133,28 @@ function taskRunner( apiKeysToInvalidate, securityPluginSetup ); - hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > 0; pageNumber++; + hasApiKeysPendingInvalidation = pageNumber * PAGE_SIZE < apiKeysToInvalidate.total; } while (hasApiKeysPendingInvalidation); return { state: { runs: (state.runs || 0) + 1, total_invalidated: totalInvalidated, - schedule: { - interval: configResult.invalidateApiKeysTask.interval, - }, + }, + schedule: { + interval: configResult.invalidateApiKeysTask.interval, }, }; } catch (errMsg) { - logger.warn(`Error executing alerting health check task: ${errMsg}`); + logger.warn(`Error executing alerting apiKey invalidation task: ${errMsg}`); return { state: { runs: (state.runs || 0) + 1, total_invalidated: totalInvalidated, - schedule: { - interval: configResult.invalidateApiKeysTask.interval, - }, + }, + schedule: { + interval: configResult.invalidateApiKeysTask.interval, }, }; } From 8dada64c9786cd8fe3c74c30e37b8552686ac99f Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 15:07:04 -0800 Subject: [PATCH 17/26] fixed paging --- .../plugins/alerts/server/invalidate_pending_api_keys/task.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index cad57b9f54cb2..8f10b8479ec61 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -133,8 +133,8 @@ function taskRunner( apiKeysToInvalidate, securityPluginSetup ); - pageNumber++; hasApiKeysPendingInvalidation = pageNumber * PAGE_SIZE < apiKeysToInvalidate.total; + pageNumber++; } while (hasApiKeysPendingInvalidation); return { From daad733ec49a765bdb10de353dc0b791a086f104 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 17:28:05 -0800 Subject: [PATCH 18/26] Fixed date filter --- .../server/invalidate_pending_api_keys/task.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 8f10b8479ec61..73ea0e6be4ab3 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -118,23 +118,23 @@ function taskRunner( const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); let hasApiKeysPendingInvalidation = true; - let pageNumber = 1; const PAGE_SIZE = 100; do { const apiKeysToInvalidate = await repository.find({ type: 'api_key_pending_invalidation', - filter: `api_key_pending_invalidation.attributes.createdAt <= ${delay}`, - page: pageNumber, + filter: `api_key_pending_invalidation.attributes.createdAt <= "${delay}"`, + page: 1, + sortField: 'createdAt', + sortOrder: 'asc', perPage: PAGE_SIZE, }); - totalInvalidated += manageToInvalidateApiKeys( + totalInvalidated += await manageToInvalidateApiKeys( logger, repository, apiKeysToInvalidate, securityPluginSetup ); - hasApiKeysPendingInvalidation = pageNumber * PAGE_SIZE < apiKeysToInvalidate.total; - pageNumber++; + hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > 0; } while (hasApiKeysPendingInvalidation); return { @@ -163,7 +163,7 @@ function taskRunner( }; } -function manageToInvalidateApiKeys( +async function manageToInvalidateApiKeys( logger: Logger, repository: ISavedObjectsRepository, apiKeysToInvalidate: SavedObjectsFindResponse, From 53e0a20e6a37b95852bae2a6edc4442cb36f08b4 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 9 Nov 2020 17:47:07 -0800 Subject: [PATCH 19/26] Fixed jest tests --- .../alerts/server/alerts_client_factory.test.ts | 4 ++-- .../server/invalidate_pending_api_keys/task.ts | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client_factory.test.ts b/x-pack/plugins/alerts/server/alerts_client_factory.test.ts index aa31edb603d3d..bdbfc726dab8f 100644 --- a/x-pack/plugins/alerts/server/alerts_client_factory.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_factory.test.ts @@ -92,7 +92,7 @@ test('creates an alerts client with proper constructor arguments when security i expect(savedObjectsService.getScopedClient).toHaveBeenCalledWith(request, { excludedWrappers: ['security'], - includedHiddenTypes: ['alert'], + includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }); const { AlertsAuthorization } = jest.requireMock('./authorization/alerts_authorization'); @@ -141,7 +141,7 @@ test('creates an alerts client with proper constructor arguments', async () => { expect(savedObjectsService.getScopedClient).toHaveBeenCalledWith(request, { excludedWrappers: ['security'], - includedHiddenTypes: ['alert'], + includedHiddenTypes: ['alert', 'api_key_pending_invalidation'], }); const { AlertsAuthorization } = jest.requireMock('./authorization/alerts_authorization'); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 73ea0e6be4ab3..d6094034299f6 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -134,6 +134,7 @@ function taskRunner( apiKeysToInvalidate, securityPluginSetup ); + hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > 0; } while (hasApiKeysPendingInvalidation); @@ -170,20 +171,23 @@ async function manageToInvalidateApiKeys( securityPluginSetup?: SecurityPluginSetup ) { let totalInvalidated = 0; - apiKeysToInvalidate.saved_objects.forEach(async (obj) => { - const response = await invalidateAPIKey({ id: obj.attributes.apiKeyId }, securityPluginSetup); + for (const apiKeyObj of apiKeysToInvalidate.saved_objects) { + const response = await invalidateAPIKey( + { id: apiKeyObj.attributes.apiKeyId }, + securityPluginSetup + ); if (response.apiKeysEnabled === true && response.result.error_count > 0) { - logger.error(`Failed to invalidate API Key [id="${obj.attributes.apiKeyId}"]`); + logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); } else { try { - await repository.delete('api_key_pending_invalidation', obj.id); + await repository.delete('api_key_pending_invalidation', apiKeyObj.id); totalInvalidated++; } catch (err) { logger.error( - `Failed to cleanup api key "${obj.attributes.apiKeyId}". Error: ${err.message}` + `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` ); } } - }); + } return totalInvalidated; } From c0f9f277e9fd316b426ad27595504a5ddbd77c47 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 10 Nov 2020 06:56:53 -0800 Subject: [PATCH 20/26] fixed due to comments --- .../invalidate_pending_api_keys/task.ts | 48 ++++++++++--------- .../plugins/alerts/server/alert_types.ts | 7 +-- .../fixtures/plugins/alerts/server/routes.ts | 2 +- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index d6094034299f6..c5b2caa71f7d7 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -114,7 +114,7 @@ function taskRunner( const repository = savedObjects.createInternalRepository([ 'api_key_pending_invalidation', ]); - const configuredDelay = await configResult.invalidateApiKeysTask.removalDelay; + const configuredDelay = configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); let hasApiKeysPendingInvalidation = true; @@ -128,14 +128,14 @@ function taskRunner( sortOrder: 'asc', perPage: PAGE_SIZE, }); - totalInvalidated += await manageToInvalidateApiKeys( + totalInvalidated += await invalidateApiKeys( logger, repository, apiKeysToInvalidate, securityPluginSetup ); - hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > 0; + hasApiKeysPendingInvalidation = apiKeysToInvalidate.total > PAGE_SIZE; } while (hasApiKeysPendingInvalidation); return { @@ -147,8 +147,8 @@ function taskRunner( interval: configResult.invalidateApiKeysTask.interval, }, }; - } catch (errMsg) { - logger.warn(`Error executing alerting apiKey invalidation task: ${errMsg}`); + } catch (e) { + logger.warn(`Error executing alerting apiKey invalidation task: ${e.message}`); return { state: { runs: (state.runs || 0) + 1, @@ -164,30 +164,32 @@ function taskRunner( }; } -async function manageToInvalidateApiKeys( +async function invalidateApiKeys( logger: Logger, repository: ISavedObjectsRepository, apiKeysToInvalidate: SavedObjectsFindResponse, securityPluginSetup?: SecurityPluginSetup ) { let totalInvalidated = 0; - for (const apiKeyObj of apiKeysToInvalidate.saved_objects) { - const response = await invalidateAPIKey( - { id: apiKeyObj.attributes.apiKeyId }, - securityPluginSetup - ); - if (response.apiKeysEnabled === true && response.result.error_count > 0) { - logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); - } else { - try { - await repository.delete('api_key_pending_invalidation', apiKeyObj.id); - totalInvalidated++; - } catch (err) { - logger.error( - `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` - ); + await Promise.all( + apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { + const response = await invalidateAPIKey( + { id: apiKeyObj.attributes.apiKeyId }, + securityPluginSetup + ); + if (response.apiKeysEnabled === true && response.result.error_count > 0) { + logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); + } else { + try { + await repository.delete('api_key_pending_invalidation', apiKeyObj.id); + totalInvalidated++; + } catch (err) { + logger.error( + `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` + ); + } } - } - } + }) + ); return totalInvalidated; } diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts index b4e7826f4eb91..a622e916d33b4 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts @@ -435,12 +435,7 @@ export function defineAlertTypes( producer: 'alertsFixture', defaultActionGroupId: 'default', async executor() { - async function sleep(ms: number) { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - } - await sleep(10000); + await new Promise((resolve) => setTimeout(resolve, 10000)); }, }; diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index 7ba46a617ad2c..277275eac0c75 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -188,7 +188,7 @@ export function defineRoutes( router.get( { - path: '/api/alerts_fixture/alerting_invalidate_apiKeys', + path: '/api/alerts_fixture/api_keys_pending_invalidation', validate: {}, }, async ( From 58975971e42b20a0f6727fea9e6ad53a21e4e84b Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 10 Nov 2020 12:16:17 -0800 Subject: [PATCH 21/26] fixed due to comments --- .../server/invalidate_pending_api_keys/task.ts | 1 + .../alerting_api_integration/common/config.ts | 2 +- .../fixtures/plugins/alerts/server/alert_types.ts | 2 +- .../fixtures/plugins/alerts/server/routes.ts | 10 +++++++--- .../security_and_spaces/tests/alerting/update.ts | 15 +++++---------- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index c5b2caa71f7d7..60b4aec36e3eb 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -191,5 +191,6 @@ async function invalidateApiKeys( } }) ); + logger.debug(`Total invalidated api keys "${totalInvalidated}"`); return totalInvalidated; } diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index f5ca896ee2fec..cb78e76bdd697 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -92,7 +92,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) ...xPackApiIntegrationTestsConfig.get('kbnTestServer.serverArgs'), `--xpack.actions.allowedHosts=${JSON.stringify(['localhost', 'some.non.existent.com'])}`, '--xpack.encryptedSavedObjects.encryptionKey="wuGNaIhoMpk5sO4UBxgr3NyW1sFcLgIf"', - '--xpack.alerts.invalidateApiKeysTask.interval="1m"', + '--xpack.alerts.invalidateApiKeysTask.interval="15s"', `--xpack.actions.enabledActionTypes=${JSON.stringify(enabledActionTypes)}`, ...actionsProxyUrl, diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts index a622e916d33b4..35155dfd9bd47 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/alert_types.ts @@ -435,7 +435,7 @@ export function defineAlertTypes( producer: 'alertsFixture', defaultActionGroupId: 'default', async executor() { - await new Promise((resolve) => setTimeout(resolve, 10000)); + await new Promise((resolve) => setTimeout(resolve, 5000)); }, }; diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index 277275eac0c75..a4ea3421b1580 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -198,12 +198,16 @@ export function defineRoutes( ): Promise> => { try { const [{ savedObjects }] = await core.getStartServices(); - const repository = savedObjects.createInternalRepository(['api_key_pending_invalidation']); - const apiKeysToInvalidate = await repository.find({ + const savedObjectsWithTasksAndAlerts = await savedObjects.getScopedClient(req, { + includedHiddenTypes: ['api_key_pending_invalidation'], + }); + const findResult = await savedObjectsWithTasksAndAlerts.find({ type: 'api_key_pending_invalidation', }); + // eslint-disable-next-line no-console + console.log(findResult); return res.ok({ - body: { apiKeysToInvalidate }, + body: { apiKeysToInvalidate: findResult.saved_objects }, }); } catch (err) { return res.badRequest({ body: err }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 463775e7562af..26e045d6c33b0 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -33,12 +33,12 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { function getAlertingApiKeysToInvalidate() { return supertest - .get(`/api/alerting_tasks/alerting_invalidate_apiKeys`) + .get(`/api/alerts_fixture/api_keys_pending_invalidation`) .expect(200) .then((response: SupertestResponse) => response.body); } - describe.skip('update', () => { + describe('update', () => { const objectRemover = new ObjectRemover(supertest); after(() => objectRemover.removeAll()); @@ -876,7 +876,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { .auth(user.username, user.password) .send(updatedData); - const apiKeyIds = (await getAlertingApiKeysToInvalidate()).docs[0]; + const apiKeyIds = await getAlertingApiKeysToInvalidate(); expect(apiKeyIds.apiKeysToInvalidate.length > 1).to.be(true); const statusUpdates: string[] = []; @@ -888,11 +888,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(statusUpdates.find((status) => status === 'failed')).to.be(undefined); - await retry.try(async () => { - const apiKeyIdsRes = (await getAlertingApiKeysToInvalidate()).docs[0]; - expect(apiKeyIdsRes.apiKeysToInvalidate.length).to.be(1); - }); - switch (scenario.id) { case 'no_kibana_privileges at space1': case 'space_1_all at space2': @@ -902,7 +897,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { error: 'Forbidden', message: getConsumerUnauthorizedErrorMessage( 'update', - 'test.noop', + 'test.longRunning', 'alertsFixture' ), statusCode: 403, @@ -968,7 +963,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { error: 'Forbidden', message: getConsumerUnauthorizedErrorMessage( 'update', - 'test.noop', + 'test.longRunning', 'alertsFixture' ), statusCode: 403, From f706b5ad6592bb86274d1258f70c630a5d8f3453 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 13 Nov 2020 11:55:01 -0800 Subject: [PATCH 22/26] Fixed e2e test --- .../common/fixtures/plugins/alerts/server/plugin.ts | 3 +++ .../security_and_spaces/tests/alerting/update.ts | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts index fbf3b798500d3..d832902fe066d 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts @@ -50,6 +50,7 @@ export class FixturePlugin implements Plugin 1).to.be(true); - const statusUpdates: string[] = []; await retry.try(async () => { const alertTask = (await getAlertingTaskById(createdAlert.scheduledTaskId)).docs[0]; @@ -913,6 +910,9 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(alertTask.status).to.eql('idle'); // ensure the alert is rescheduled to a minute from now ensureDatetimeIsWithinRange(Date.parse(alertTask.runAt), 60 * 1000); + + const apiKeyIds = await getAlertingApiKeysToInvalidate(); + expect(apiKeyIds.apiKeysToInvalidate.length).to.be(0); }); break; default: From b8c1eae9534f2f806b4126c73f7613ad23de6144 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 13 Nov 2020 14:27:24 -0800 Subject: [PATCH 23/26] Fixed e2e test --- .../security_and_spaces/tests/alerting/update.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index f3d694974945d..3609b08cd3a37 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -963,7 +963,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { error: 'Forbidden', message: getConsumerUnauthorizedErrorMessage( 'update', - 'test.longRunning', + 'test.noop', 'alertsFixture' ), statusCode: 403, From 4092f1386750c634e4aa126d5e555acee29a544d Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 16 Nov 2020 13:07:28 -0800 Subject: [PATCH 24/26] Fixed due to comments. Changed api key invalidation task to use SavedObjectClient --- .../server/alerts_client/tests/create.test.ts | 4 +- .../server/alerts_client/tests/delete.test.ts | 10 ++-- .../alerts_client/tests/disable.test.ts | 14 +++--- .../server/alerts_client/tests/enable.test.ts | 10 ++-- .../server/alerts_client/tests/update.test.ts | 12 ++--- .../tests/update_api_key.test.ts | 10 ++-- .../mark_api_key_for_invalidation.ts | 10 ++-- .../invalidate_pending_api_keys/task.ts | 49 ++++++++++++++----- .../alerts/server/saved_objects/index.ts | 2 +- 9 files changed, 73 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index cf80692a3cb16..1041cc13ba6d5 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -735,7 +735,7 @@ describe('create()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt, }, references: [], @@ -746,7 +746,7 @@ describe('create()', () => { expect(taskManager.schedule).not.toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt, }); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 358510a774770..1a56e96c2a140 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -97,7 +97,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -121,7 +121,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -180,7 +180,7 @@ describe('delete()', () => { 'api_key_pending_invalidation' ); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for API key [id="123"] for invalidation: Fail' + 'Failed to mark for API key [id="MTIzOmFiYw=="] for invalidation: Fail' ); }); @@ -189,7 +189,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index b68eb24be4414..f20c49992b521 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -112,7 +112,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('disable()', () => { expect(taskManager.remove).toHaveBeenCalledWith('task-123'); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('123'); + ).toBe('MTIz'); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { @@ -165,7 +165,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -225,7 +225,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -242,7 +242,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -258,7 +258,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -286,7 +286,7 @@ describe('disable()', () => { unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); await alertsClient.disable({ id: '1' }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for API key [id="123"] for invalidation: Fail' + 'Failed to mark for API key [id="MTIzOmFiYw=="] for invalidation: Fail' ); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 16e83c42d8930..6e9275c8c503a 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -162,7 +162,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt, }, references: [], @@ -239,7 +239,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt, }, references: [], @@ -252,7 +252,7 @@ describe('enable()', () => { }); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('123'); + ).toBe('MTIz'); }); test(`doesn't enable already enabled alerts`, async () => { @@ -345,7 +345,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt, }, references: [], @@ -358,7 +358,7 @@ describe('enable()', () => { expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('123'); + ).toBe('MTIz'); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 9de47dde0fad5..4fe3b7a9f58e9 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -164,7 +164,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -388,7 +388,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -397,7 +397,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -560,7 +560,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -832,7 +832,7 @@ describe('update()', () => { }, }); expect(alertsClientParams.logger.error).toHaveBeenCalledWith( - 'Failed to mark for API key [id="123"] for invalidation: Fail' + 'Failed to mark for API key [id="MTIzOmFiYw=="] for invalidation: Fail' ); }); @@ -1002,7 +1002,7 @@ describe('update()', () => { ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); expect( (unsecuredSavedObjectsClient.create.mock.calls[1][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('234'); + ).toBe('MjM0'); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index cd6d63d4d8662..365a8526f3198 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -84,7 +84,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -141,7 +141,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '123', + apiKeyId: 'MTIz', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -199,7 +199,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'test', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -223,7 +223,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: '234', + apiKeyId: 'MjM0', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -234,7 +234,7 @@ describe('updateApiKey()', () => { ); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('234'); + ).toBe('MjM0'); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts index ee4fd38086396..d145e8e33fa3d 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts @@ -8,18 +8,20 @@ import { Logger, SavedObjectsClientContract } from 'src/core/server'; export const markApiKeyForInvalidation = async ( { apiKey }: { apiKey: string | null }, logger: Logger, - internalSavedObjectsRepository: SavedObjectsClientContract + savedObjectsClient: SavedObjectsClientContract ): Promise => { if (!apiKey) { return; } - const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; try { - await internalSavedObjectsRepository.create('api_key_pending_invalidation', { + const apiKeyId = Buffer.from(Buffer.from(apiKey, 'base64').toString().split(':')[0]).toString( + 'base64' + ); + await savedObjectsClient.create('api_key_pending_invalidation', { apiKeyId, createdAt: new Date().toISOString(), }); } catch (e) { - logger.error(`Failed to mark for API key [id="${apiKeyId}"] for invalidation: ${e.message}`); + logger.error(`Failed to mark for API key [id="${apiKey}"] for invalidation: ${e.message}`); } }; diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index 60b4aec36e3eb..dd2f9684dccbf 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -8,7 +8,8 @@ import { Logger, CoreStart, SavedObjectsFindResponse, - ISavedObjectsRepository, + KibanaRequest, + SavedObjectsClientContract, } from 'kibana/server'; import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { @@ -97,6 +98,24 @@ function registerApiKeyInvalitorTaskDefinition( }); } +function getFakeKibanaRequest(basePath: string) { + const requestHeaders: Record = {}; + return ({ + headers: requestHeaders, + getBasePath: () => basePath, + path: '/', + route: { settings: {} }, + url: { + href: '/', + }, + raw: { + req: { + url: '/', + }, + }, + } as unknown) as KibanaRequest; +} + function taskRunner( logger: Logger, coreStartServices: Promise<[CoreStart, AlertingPluginsStart, unknown]>, @@ -110,17 +129,22 @@ function taskRunner( let totalInvalidated = 0; const configResult = await config; try { - const [{ savedObjects }] = await coreStartServices; - const repository = savedObjects.createInternalRepository([ - 'api_key_pending_invalidation', - ]); + const [{ savedObjects, http }] = await coreStartServices; + const savedObjectsClient = savedObjects.getScopedClient( + getFakeKibanaRequest(http.basePath.serverBasePath), + { + includedHiddenTypes: ['api_key_pending_invalidation'], + excludedWrappers: ['security'], + } + ); + const configuredDelay = configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); let hasApiKeysPendingInvalidation = true; const PAGE_SIZE = 100; do { - const apiKeysToInvalidate = await repository.find({ + const apiKeysToInvalidate = await savedObjectsClient.find({ type: 'api_key_pending_invalidation', filter: `api_key_pending_invalidation.attributes.createdAt <= "${delay}"`, page: 1, @@ -130,7 +154,7 @@ function taskRunner( }); totalInvalidated += await invalidateApiKeys( logger, - repository, + savedObjectsClient, apiKeysToInvalidate, securityPluginSetup ); @@ -166,22 +190,21 @@ function taskRunner( async function invalidateApiKeys( logger: Logger, - repository: ISavedObjectsRepository, + savedObjectsClient: SavedObjectsClientContract, apiKeysToInvalidate: SavedObjectsFindResponse, securityPluginSetup?: SecurityPluginSetup ) { let totalInvalidated = 0; await Promise.all( apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { - const response = await invalidateAPIKey( - { id: apiKeyObj.attributes.apiKeyId }, - securityPluginSetup - ); + const apiKeyId = Buffer.from(apiKeyObj.attributes.apiKeyId, 'base64').toString(); + + const response = await invalidateAPIKey({ id: apiKeyId }, securityPluginSetup); if (response.apiKeysEnabled === true && response.result.error_count > 0) { logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); } else { try { - await repository.delete('api_key_pending_invalidation', apiKeyObj.id); + await savedObjectsClient.delete('api_key_pending_invalidation', apiKeyObj.id); totalInvalidated++; } catch (err) { logger.error( diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index 0be5c1d47ed12..bbdc121f8f855 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -45,7 +45,7 @@ export function setupSavedObjects( savedObjects.registerType({ name: 'api_key_pending_invalidation', hidden: true, - namespaceType: 'single', + namespaceType: 'agnostic', mappings: { properties: { apiKeyId: { From 1cc9d52596193fd2833e52cc4732a11791c249c1 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 16 Nov 2020 18:01:15 -0800 Subject: [PATCH 25/26] Use encryptedSavedObjectClient --- .../server/alerts_client/tests/create.test.ts | 4 ++-- .../server/alerts_client/tests/delete.test.ts | 8 ++++---- .../server/alerts_client/tests/disable.test.ts | 12 ++++++------ .../server/alerts_client/tests/enable.test.ts | 10 +++++----- .../server/alerts_client/tests/update.test.ts | 10 +++++----- .../alerts_client/tests/update_api_key.test.ts | 10 +++++----- .../mark_api_key_for_invalidation.test.ts | 16 +++++++++++++++- .../mark_api_key_for_invalidation.ts | 4 +--- .../server/invalidate_pending_api_keys/task.ts | 15 +++++++++++---- .../plugins/alerts/server/saved_objects/index.ts | 6 ++++++ .../fixtures/plugins/alerts/server/routes.ts | 2 -- .../security_and_spaces/tests/alerting/update.ts | 11 ----------- 12 files changed, 60 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index 1041cc13ba6d5..ee407b1a6d50c 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -735,7 +735,7 @@ describe('create()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -746,7 +746,7 @@ describe('create()', () => { expect(taskManager.schedule).not.toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 1a56e96c2a140..e7b975aec8eb0 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -97,7 +97,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -121,7 +121,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -189,7 +189,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index f20c49992b521..11ce0027f82d8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -112,7 +112,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('disable()', () => { expect(taskManager.remove).toHaveBeenCalledWith('task-123'); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { @@ -165,7 +165,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -225,7 +225,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -242,7 +242,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -258,7 +258,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 6e9275c8c503a..16e83c42d8930 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -162,7 +162,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -239,7 +239,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -252,7 +252,7 @@ describe('enable()', () => { }); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); }); test(`doesn't enable already enabled alerts`, async () => { @@ -345,7 +345,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -358,7 +358,7 @@ describe('enable()', () => { expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 4fe3b7a9f58e9..ad58e36ade722 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -164,7 +164,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -388,7 +388,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -397,7 +397,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -560,7 +560,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -1002,7 +1002,7 @@ describe('update()', () => { ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); expect( (unsecuredSavedObjectsClient.create.mock.calls[1][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MjM0'); + ).toBe('234'); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index 365a8526f3198..af178a1fac5f5 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -84,7 +84,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -141,7 +141,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -199,7 +199,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -223,7 +223,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -234,7 +234,7 @@ describe('updateApiKey()', () => { ); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MjM0'); + ).toBe('234'); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts index 13f5a7dcd7eac..7b30c22c47f8a 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts @@ -19,7 +19,7 @@ describe('markApiKeyForInvalidation', () => { }, references: [], }); - markApiKeyForInvalidation( + await markApiKeyForInvalidation( { apiKey: Buffer.from('123:abc').toString('base64') }, loggingSystemMock.create().get(), unsecuredSavedObjectsClient @@ -30,4 +30,18 @@ describe('markApiKeyForInvalidation', () => { 'api_key_pending_invalidation' ); }); + + test('should log the proper error when savedObjectsClient create failed', async () => { + const logger = loggingSystemMock.create().get(); + const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); + await markApiKeyForInvalidation( + { apiKey: Buffer.from('123').toString('base64') }, + logger, + unsecuredSavedObjectsClient + ); + expect(logger.error).toHaveBeenCalledWith( + 'Failed to mark for API key [id="MTIz"] for invalidation: Fail' + ); + }); }); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts index d145e8e33fa3d..db25f5b3e19eb 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts @@ -14,9 +14,7 @@ export const markApiKeyForInvalidation = async ( return; } try { - const apiKeyId = Buffer.from(Buffer.from(apiKey, 'base64').toString().split(':')[0]).toString( - 'base64' - ); + const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; await savedObjectsClient.create('api_key_pending_invalidation', { apiKeyId, createdAt: new Date().toISOString(), diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index dd2f9684dccbf..77cbb9f4a4a85 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -11,6 +11,7 @@ import { KibanaRequest, SavedObjectsClientContract, } from 'kibana/server'; +import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { RunContext, @@ -129,7 +130,7 @@ function taskRunner( let totalInvalidated = 0; const configResult = await config; try { - const [{ savedObjects, http }] = await coreStartServices; + const [{ savedObjects, http }, { encryptedSavedObjects }] = await coreStartServices; const savedObjectsClient = savedObjects.getScopedClient( getFakeKibanaRequest(http.basePath.serverBasePath), { @@ -137,7 +138,9 @@ function taskRunner( excludedWrappers: ['security'], } ); - + const encryptedSavedObjectsClient = encryptedSavedObjects.getClient({ + includedHiddenTypes: ['api_key_pending_invalidation'], + }); const configuredDelay = configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); @@ -156,6 +159,7 @@ function taskRunner( logger, savedObjectsClient, apiKeysToInvalidate, + encryptedSavedObjectsClient, securityPluginSetup ); @@ -192,13 +196,16 @@ async function invalidateApiKeys( logger: Logger, savedObjectsClient: SavedObjectsClientContract, apiKeysToInvalidate: SavedObjectsFindResponse, + encryptedSavedObjectsClient: EncryptedSavedObjectsClient, securityPluginSetup?: SecurityPluginSetup ) { let totalInvalidated = 0; await Promise.all( apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { - const apiKeyId = Buffer.from(apiKeyObj.attributes.apiKeyId, 'base64').toString(); - + const decryptedApiKey = await encryptedSavedObjectsClient.getDecryptedAsInternalUser< + InvalidatePendingApiKey + >('api_key_pending_invalidation', apiKeyObj.id); + const apiKeyId = decryptedApiKey.attributes.apiKeyId; const response = await invalidateAPIKey({ id: apiKeyId }, securityPluginSetup); if (response.apiKeysEnabled === true && response.result.error_count > 0) { logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index bbdc121f8f855..da30273e93c6b 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -64,4 +64,10 @@ export function setupSavedObjects( attributesToEncrypt: new Set(['apiKey']), attributesToExcludeFromAAD: new Set(AlertAttributesExcludedFromAAD), }); + + // Encrypted attributes + encryptedSavedObjects.registerType({ + type: 'api_key_pending_invalidation', + attributesToEncrypt: new Set(['apiKeyId']), + }); } diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index a4ea3421b1580..d4c7635315f22 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -204,8 +204,6 @@ export function defineRoutes( const findResult = await savedObjectsWithTasksAndAlerts.find({ type: 'api_key_pending_invalidation', }); - // eslint-disable-next-line no-console - console.log(findResult); return res.ok({ body: { apiKeysToInvalidate: findResult.saved_objects }, }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 3609b08cd3a37..6164669a31a40 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -31,13 +31,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { .then((response: SupertestResponse) => response.body); } - function getAlertingApiKeysToInvalidate() { - return supertest - .get(`/api/alerts_fixture/api_keys_pending_invalidation`) - .expect(200) - .then((response: SupertestResponse) => response.body); - } - describe('update', () => { const objectRemover = new ObjectRemover(supertest); @@ -859,7 +852,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { }) .expect(200); objectRemover.add(space.id, createdAlert.id, 'alert', 'alerts'); - const updatedData = { name: 'bcd', tags: ['bar'], @@ -910,9 +902,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(alertTask.status).to.eql('idle'); // ensure the alert is rescheduled to a minute from now ensureDatetimeIsWithinRange(Date.parse(alertTask.runAt), 60 * 1000); - - const apiKeyIds = await getAlertingApiKeysToInvalidate(); - expect(apiKeyIds.apiKeysToInvalidate.length).to.be(0); }); break; default: From bd65fb3ba9e4ff656a7118c2b17474c969a651d2 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 16 Nov 2020 20:45:06 -0800 Subject: [PATCH 26/26] set back flaky test comment --- .../security_and_spaces/tests/alerting/update.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 6164669a31a40..9c3d2801c0886 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -31,7 +31,8 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { .then((response: SupertestResponse) => response.body); } - describe('update', () => { + // FLAKY: https://github.com/elastic/kibana/issues/82804 + describe.skip('update', () => { const objectRemover = new ObjectRemover(supertest); after(() => objectRemover.removeAll());