From 3d8691fe962e904ba102bfc6dcc03e0188dcd16c Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 7 Jan 2021 07:17:24 -0800 Subject: [PATCH 1/2] resolve merge conflicts --- .../invalidate_pending_api_keys/task.ts | 44 +++++++++++-------- .../fleet/server/services/agents/unenroll.ts | 15 ++----- .../services/api_keys/enrollment_api_key.ts | 4 +- .../fleet/server/services/api_keys/index.ts | 2 +- .../server/services/api_keys/security.ts | 4 +- .../authentication/api_keys/api_keys.test.ts | 16 +++---- .../authentication/api_keys/api_keys.ts | 42 +++++++++++------- .../server/authentication/api_keys/index.ts | 2 +- .../security/server/authentication/index.ts | 2 +- x-pack/plugins/security/server/index.ts | 2 +- 10 files changed, 69 insertions(+), 64 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 91c3f5954d6d..5e26a5776f9f 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) { @@ -194,28 +194,34 @@ async function invalidateApiKeys( securityPluginStart?: SecurityPluginStart ) { 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 { - try { - await savedObjectsClient.delete('api_key_pending_invalidation', apiKeyObj.id); - totalInvalidated++; - } catch (err) { - logger.error( - `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` - ); - } - } + return decryptedApiKey.attributes.apiKeyId; }) ); - logger.debug(`Total invalidated api keys "${totalInvalidated}"`); + if (apiKeyIds.length > 0) { + 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++; + } catch (err) { + logger.error( + `Failed to delete invalidated API key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}` + ); + } + }) + ); + } + } + logger.debug(`Total invalidated API keys "${totalInvalidated}"`); return totalInvalidated; } diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll.ts b/x-pack/plugins/fleet/server/services/agents/unenroll.ts index 60533e128514..9c2b2bdfe7f6 100644 --- a/x-pack/plugins/fleet/server/services/agents/unenroll.ts +++ b/x-pack/plugins/fleet/server/services/agents/unenroll.ts @@ -3,7 +3,6 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { chunk } from 'lodash'; import { SavedObjectsClientContract } from 'src/core/server'; import { AgentSOAttributes } from '../../types'; import { AGENT_SAVED_OBJECT_TYPE } from '../../constants'; @@ -76,10 +75,10 @@ export async function forceUnenrollAgent(soClient: SavedObjectsClientContract, a await Promise.all([ agent.access_api_key_id - ? APIKeyService.invalidateAPIKey(soClient, agent.access_api_key_id) + ? APIKeyService.invalidateAPIKeys(soClient, [agent.access_api_key_id]) : undefined, agent.default_api_key_id - ? APIKeyService.invalidateAPIKey(soClient, agent.default_api_key_id) + ? APIKeyService.invalidateAPIKeys(soClient, [agent.default_api_key_id]) : undefined, ]); @@ -124,16 +123,8 @@ export async function forceUnenrollAgents( }); // Invalidate all API keys - // ES doesn't provide a bulk invalidate API, so this could take a long time depending on - // number of keys to invalidate. We run these in batches to avoid overloading ES. if (apiKeys.length) { - const BATCH_SIZE = 500; - const batches = chunk(apiKeys, BATCH_SIZE); - for (const apiKeysBatch of batches) { - await Promise.all( - apiKeysBatch.map((apiKey) => APIKeyService.invalidateAPIKey(soClient, apiKey)) - ); - } + APIKeyService.invalidateAPIKeys(soClient, apiKeys); } // Update the necessary agents diff --git a/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts b/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts index b9d0cf883d35..8f67753392e6 100644 --- a/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts +++ b/x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts @@ -9,7 +9,7 @@ import Boom from '@hapi/boom'; import { SavedObjectsClientContract, SavedObject } from 'src/core/server'; import { EnrollmentAPIKey, EnrollmentAPIKeySOAttributes } from '../../types'; import { ENROLLMENT_API_KEYS_SAVED_OBJECT_TYPE } from '../../constants'; -import { createAPIKey, invalidateAPIKey } from './security'; +import { createAPIKey, invalidateAPIKeys } from './security'; import { agentPolicyService } from '../agent_policy'; import { appContextService } from '../app_context'; import { normalizeKuery } from '../saved_object'; @@ -66,7 +66,7 @@ export async function getEnrollmentAPIKey(soClient: SavedObjectsClientContract, export async function deleteEnrollmentApiKey(soClient: SavedObjectsClientContract, id: string) { const enrollmentApiKey = await getEnrollmentAPIKey(soClient, id); - await invalidateAPIKey(soClient, enrollmentApiKey.api_key_id); + await invalidateAPIKeys(soClient, [enrollmentApiKey.api_key_id]); await soClient.update(ENROLLMENT_API_KEYS_SAVED_OBJECT_TYPE, id, { active: false, diff --git a/x-pack/plugins/fleet/server/services/api_keys/index.ts b/x-pack/plugins/fleet/server/services/api_keys/index.ts index d1a4a21dec10..bc756a311dc7 100644 --- a/x-pack/plugins/fleet/server/services/api_keys/index.ts +++ b/x-pack/plugins/fleet/server/services/api_keys/index.ts @@ -10,7 +10,7 @@ import { EnrollmentAPIKeySOAttributes, EnrollmentAPIKey } from '../../types'; import { createAPIKey } from './security'; import { escapeSearchQueryPhrase } from '../saved_object'; -export { invalidateAPIKey } from './security'; +export { invalidateAPIKeys } from './security'; export * from './enrollment_api_key'; export async function generateOutputApiKey( diff --git a/x-pack/plugins/fleet/server/services/api_keys/security.ts b/x-pack/plugins/fleet/server/services/api_keys/security.ts index 9a32da3cff46..a22776435e93 100644 --- a/x-pack/plugins/fleet/server/services/api_keys/security.ts +++ b/x-pack/plugins/fleet/server/services/api_keys/security.ts @@ -64,7 +64,7 @@ export async function authenticate(callCluster: CallESAsCurrentUser) { } } -export async function invalidateAPIKey(soClient: SavedObjectsClientContract, id: string) { +export async function invalidateAPIKeys(soClient: SavedObjectsClientContract, ids: string[]) { const adminUser = await outputService.getAdminUser(soClient); if (!adminUser) { throw new Error('No admin user configured'); @@ -88,7 +88,7 @@ export async function invalidateAPIKey(soClient: SavedObjectsClientContract, id: try { const res = await security.authc.apiKeys.invalidate(request, { - id, + ids, }); return res; 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 1e5d3baab83e..bdfb0b45406d 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 @@ -289,7 +289,7 @@ describe('API Keys', () => { it('returns null when security feature is disabled', async () => { mockLicense.isEnabled.mockReturnValue(false); const result = await apiKeys.invalidate(httpServerMock.createKibanaRequest(), { - id: '123', + ids: ['123'], }); expect(result).toBeNull(); expect( @@ -309,7 +309,7 @@ describe('API Keys', () => { }) ); const result = await apiKeys.invalidate(httpServerMock.createKibanaRequest(), { - id: '123', + ids: ['123'], }); expect(result).toEqual({ invalidated_api_keys: ['api-key-id-1'], @@ -323,7 +323,7 @@ describe('API Keys', () => { }); }); - it(`Only passes id as a parameter`, async () => { + it(`Only passes ids as a parameter`, async () => { mockLicense.isEnabled.mockReturnValue(true); mockScopedClusterClient.asCurrentUser.security.invalidateApiKey.mockResolvedValueOnce( securityMock.createApiResponse({ @@ -335,7 +335,7 @@ describe('API Keys', () => { }) ); const result = await apiKeys.invalidate(httpServerMock.createKibanaRequest(), { - id: '123', + ids: ['123'], name: 'abc', } as any); expect(result).toEqual({ @@ -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: [], @@ -383,7 +383,7 @@ describe('API Keys', () => { }); }); - it('Only passes id as a parameter', async () => { + it('Only passes ids as a parameter', async () => { mockLicense.isEnabled.mockReturnValue(true); mockClusterClient.asInternalUser.security.invalidateApiKey.mockResolvedValueOnce( securityMock.createApiResponse({ @@ -395,7 +395,7 @@ describe('API Keys', () => { }) ); const result = await apiKeys.invalidateAsInternalUser({ - id: '123', + ids: ['123'], name: 'abc', } as any); expect(result).toEqual({ 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 212b5755549f..4deb6f3f4dcc 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 @@ -39,10 +39,10 @@ interface GrantAPIKeyParams { } /** - * Represents the params for invalidating an API key + * Represents the params for invalidating multiple API keys */ -export interface InvalidateAPIKeyParams { - id: string; +export interface InvalidateAPIKeysParams { + ids: string[]; } /** @@ -222,16 +222,16 @@ export class APIKeys { } /** - * Tries to invalidate an API key. + * Tries to invalidate an API keys. * @param request Request instance. - * @param params The params to invalidate an API key. + * @param params The params to invalidate an API keys. */ - async invalidate(request: KibanaRequest, params: InvalidateAPIKeyParams) { + async invalidate(request: KibanaRequest, params: InvalidateAPIKeysParams) { if (!this.license.isEnabled()) { return null; } - this.logger.debug('Trying to invalidate an API key as current user'); + this.logger.debug(`Trying to invalidate ${params.ids.length} an API key as current user`); let result; try { @@ -240,12 +240,18 @@ export class APIKeys { await this.clusterClient .asScoped(request) .asCurrentUser.security.invalidateApiKey({ - body: { id: params.id }, + body: { ids: params.ids }, }) ).body; - this.logger.debug('API key was invalidated successfully as current user'); + this.logger.debug( + `API keys by ids=[${params.ids.join(', ')}] was invalidated successfully as current user` + ); } catch (e) { - this.logger.error(`Failed to invalidate API key as current user: ${e.message}`); + this.logger.error( + `Failed to invalidate API keys by ids=[${params.ids.join(', ')}] as current user: ${ + e.message + }` + ); throw e; } @@ -253,27 +259,29 @@ export class APIKeys { } /** - * Tries to invalidate an API key by using the internal user. - * @param params The params to invalidate an API key. + * Tries to invalidate the API keys by using the internal user. + * @param params The params to invalidate the API keys. */ - async invalidateAsInternalUser(params: InvalidateAPIKeyParams) { + async invalidateAsInternalUser(params: InvalidateAPIKeysParams) { if (!this.license.isEnabled()) { return null; } - this.logger.debug('Trying to invalidate an API key'); + this.logger.debug(`Trying to invalidate ${params.ids.length} API keys`); let result; try { // Internal user needs `cluster:admin/xpack/security/api_key/invalidate` privilege to use this API result = ( await this.clusterClient.asInternalUser.security.invalidateApiKey({ - body: { id: params.id }, + body: { ids: params.ids }, }) ).body; - this.logger.debug('API key was invalidated successfully'); + this.logger.debug(`API keys by ids=[${params.ids.join(', ')}] was invalidated successfully`); } catch (e) { - this.logger.error(`Failed to invalidate API key: ${e.message}`); + this.logger.error( + `Failed to invalidate API keys by ids=[${params.ids.join(', ')}]: ${e.message}` + ); throw e; } 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 e0b6d03ea2c4..021eaeb0bd97 100644 --- a/x-pack/plugins/security/server/authentication/api_keys/index.ts +++ b/x-pack/plugins/security/server/authentication/api_keys/index.ts @@ -9,6 +9,6 @@ export { CreateAPIKeyResult, 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 b43ffd86ae5e..839596eafcc5 100644 --- a/x-pack/plugins/security/server/authentication/index.ts +++ b/x-pack/plugins/security/server/authentication/index.ts @@ -28,6 +28,6 @@ export type { CreateAPIKeyResult, 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 5d51b88d8293..3233708a5d23 100644 --- a/x-pack/plugins/security/server/index.ts +++ b/x-pack/plugins/security/server/index.ts @@ -24,7 +24,7 @@ import { // functions or removal of exports should be considered as a breaking change. export type { CreateAPIKeyResult, - InvalidateAPIKeyParams, + InvalidateAPIKeysParams, InvalidateAPIKeyResult, GrantAPIKeyResult, } from './authentication'; From 70529a2943cfd2e58742efa05aa4e13e2ee8e5d6 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Thu, 7 Jan 2021 09:38:20 -0800 Subject: [PATCH 2/2] fixed faling tests --- .../server/authentication/api_keys/api_keys.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 bdfb0b45406d..3f495ea16169 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 @@ -318,7 +318,7 @@ describe('API Keys', () => { }); expect(mockScopedClusterClient.asCurrentUser.security.invalidateApiKey).toHaveBeenCalledWith({ body: { - id: '123', + ids: ['123'], }, }); }); @@ -345,7 +345,7 @@ describe('API Keys', () => { }); expect(mockScopedClusterClient.asCurrentUser.security.invalidateApiKey).toHaveBeenCalledWith({ body: { - id: '123', + ids: ['123'], }, }); }); @@ -378,7 +378,7 @@ describe('API Keys', () => { }); expect(mockClusterClient.asInternalUser.security.invalidateApiKey).toHaveBeenCalledWith({ body: { - id: '123', + ids: ['123'], }, }); }); @@ -405,7 +405,7 @@ describe('API Keys', () => { }); expect(mockClusterClient.asInternalUser.security.invalidateApiKey).toHaveBeenCalledWith({ body: { - id: '123', + ids: ['123'], }, }); });