From 820e6609ff8ec879953e09722517e55e94efb314 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 5 Jan 2021 14:42:26 -0800 Subject: [PATCH] Replaced single invalidateApiKey request with the bulk --- .../invalidate_pending_api_keys/task.ts | 29 ++++++++++--------- .../authentication/api_keys/api_keys.test.ts | 4 +-- .../authentication/api_keys/api_keys.ts | 11 +++++-- .../server/authentication/api_keys/index.ts | 1 + .../security/server/authentication/index.ts | 1 + x-pack/plugins/security/server/index.ts | 1 + 6 files changed, 30 insertions(+), 17 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 1c7320d3df6f3..d28edf425e950 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 @@ -12,7 +12,7 @@ import { SavedObjectsClientContract, } from 'kibana/server'; import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; -import { InvalidateAPIKeyParams, SecurityPluginStart } from '../../../security/server'; +import { InvalidateAPIKeysParams, SecurityPluginStart } from '../../../security/server'; import { RunContext, TaskManagerSetupContract, @@ -27,8 +27,8 @@ import { InvalidatePendingApiKey } from '../types'; const TASK_TYPE = 'alerts_invalidate_api_keys'; export const TASK_ID = `Alerts-${TASK_TYPE}`; -const invalidateAPIKey = async ( - params: InvalidateAPIKeyParams, +const invalidateAPIKeys = async ( + params: InvalidateAPIKeysParams, securityPluginStart?: SecurityPluginStart ): Promise => { if (!securityPluginStart) { @@ -193,19 +193,22 @@ async function invalidateApiKeys( encryptedSavedObjectsClient: EncryptedSavedObjectsClient, securityPluginStart?: SecurityPluginStart ) { - // TODO: This could probably send a single request to ES now that the invalidate API supports multiple ids in a single request let totalInvalidated = 0; - await Promise.all( + const apiKeyIds = await Promise.all( apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { const decryptedApiKey = await encryptedSavedObjectsClient.getDecryptedAsInternalUser( 'api_key_pending_invalidation', apiKeyObj.id ); - const apiKeyId = decryptedApiKey.attributes.apiKeyId; - const response = await invalidateAPIKey({ id: apiKeyId }, securityPluginStart); - if (response.apiKeysEnabled === true && response.result.error_count > 0) { - logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); - } else { + return decryptedApiKey.attributes.apiKeyId; + }) + ); + const response = await invalidateAPIKeys({ ids: apiKeyIds }, securityPluginStart); + if (response.apiKeysEnabled === true && response.result.error_count > 0) { + logger.error(`Failed to invalidate API Keys [ids="${apiKeyIds.join(', ')}"]`); + } else { + await Promise.all( + apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { try { await savedObjectsClient.delete('api_key_pending_invalidation', apiKeyObj.id); totalInvalidated++; @@ -214,9 +217,9 @@ async function invalidateApiKeys( `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` ); } - } - }) - ); + }) + ); + } logger.debug(`Total invalidated api keys "${totalInvalidated}"`); return totalInvalidated; } diff --git a/x-pack/plugins/security/server/authentication/api_keys/api_keys.test.ts b/x-pack/plugins/security/server/authentication/api_keys/api_keys.test.ts index c43ed9e1248f6..61496d294d56f 100644 --- a/x-pack/plugins/security/server/authentication/api_keys/api_keys.test.ts +++ b/x-pack/plugins/security/server/authentication/api_keys/api_keys.test.ts @@ -354,7 +354,7 @@ describe('API Keys', () => { describe('invalidateAsInternalUser()', () => { it('returns null when security feature is disabled', async () => { mockLicense.isEnabled.mockReturnValue(false); - const result = await apiKeys.invalidateAsInternalUser({ id: '123' }); + const result = await apiKeys.invalidateAsInternalUser({ ids: ['123'] }); expect(result).toBeNull(); expect(mockClusterClient.asInternalUser.security.invalidateApiKey).not.toHaveBeenCalled(); }); @@ -370,7 +370,7 @@ describe('API Keys', () => { }, }) ); - const result = await apiKeys.invalidateAsInternalUser({ id: '123' }); + const result = await apiKeys.invalidateAsInternalUser({ ids: ['123'] }); expect(result).toEqual({ invalidated_api_keys: ['api-key-id-1'], previously_invalidated_api_keys: [], diff --git a/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts b/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts index a42efb678fcea..a3e8b68977f09 100644 --- a/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts +++ b/x-pack/plugins/security/server/authentication/api_keys/api_keys.ts @@ -45,6 +45,13 @@ export interface InvalidateAPIKeyParams { id: string; } +/** + * Represents the params for invalidating multiple API keys + */ +export interface InvalidateAPIKeysParams { + ids: string[]; +} + /** * The return value when creating an API key in Elasticsearch. The API key returned by this API * can then be used by sending a request with a Authorization header with a value having the @@ -256,7 +263,7 @@ export class APIKeys { * Tries to invalidate an API key by using the internal user. * @param params The params to invalidate an API key. */ - async invalidateAsInternalUser(params: InvalidateAPIKeyParams) { + async invalidateAsInternalUser(params: InvalidateAPIKeysParams) { if (!this.license.isEnabled()) { return null; } @@ -268,7 +275,7 @@ export class APIKeys { // Internal user needs `cluster:admin/xpack/security/api_key/invalidate` privilege to use this API result = ( await this.clusterClient.asInternalUser.security.invalidateApiKey({ - body: { ids: [params.id] }, + body: { ids: params.ids }, }) ).body; this.logger.debug('API key was invalidated successfully'); diff --git a/x-pack/plugins/security/server/authentication/api_keys/index.ts b/x-pack/plugins/security/server/authentication/api_keys/index.ts index e0b6d03ea2c42..c40460d69ddac 100644 --- a/x-pack/plugins/security/server/authentication/api_keys/index.ts +++ b/x-pack/plugins/security/server/authentication/api_keys/index.ts @@ -10,5 +10,6 @@ export { InvalidateAPIKeyResult, CreateAPIKeyParams, InvalidateAPIKeyParams, + InvalidateAPIKeysParams, GrantAPIKeyResult, } from './api_keys'; diff --git a/x-pack/plugins/security/server/authentication/index.ts b/x-pack/plugins/security/server/authentication/index.ts index b43ffd86ae5ed..0206e827b4178 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -29,5 +29,6 @@ export type { InvalidateAPIKeyResult, CreateAPIKeyParams, InvalidateAPIKeyParams, + InvalidateAPIKeysParams, GrantAPIKeyResult, } from './api_keys'; diff --git a/x-pack/plugins/security/server/index.ts b/x-pack/plugins/security/server/index.ts index 5d51b88d82939..d60d9f528bddd 100644 --- a/x-pack/plugins/security/server/index.ts +++ b/x-pack/plugins/security/server/index.ts @@ -25,6 +25,7 @@ import { export type { CreateAPIKeyResult, InvalidateAPIKeyParams, + InvalidateAPIKeysParams, InvalidateAPIKeyResult, GrantAPIKeyResult, } from './authentication';