[Alerting] Replaced single invalidateApiKey request with the bulk. (#87401)

* Replaced single invalidateApiKey request with the bulk

* fixed failing test

* Extended invalidate method to support multiple invalidation. Updated fleets plugin usage of this API.

* fixed due to comments
This commit is contained in:
Yuliia Naumenko 2021-01-07 07:17:24 -08:00 committed by GitHub
parent f4042dd566
commit 794c6b3b08
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 69 additions and 65 deletions

View file

@ -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<InvalidateAPIKeyResult> => {
if (!securityPluginStart) {
@ -193,30 +193,35 @@ 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<InvalidatePendingApiKey>(
'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;
}

View file

@ -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

View file

@ -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,

View file

@ -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(

View file

@ -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;

View file

@ -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({

View file

@ -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<InvalidateAPIKeyResult>({
body: { ids: [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<InvalidateAPIKeyResult>({
body: { ids: [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;
}

View file

@ -9,6 +9,6 @@ export {
CreateAPIKeyResult,
InvalidateAPIKeyResult,
CreateAPIKeyParams,
InvalidateAPIKeyParams,
InvalidateAPIKeysParams,
GrantAPIKeyResult,
} from './api_keys';

View file

@ -28,6 +28,6 @@ export type {
CreateAPIKeyResult,
InvalidateAPIKeyResult,
CreateAPIKeyParams,
InvalidateAPIKeyParams,
InvalidateAPIKeysParams,
GrantAPIKeyResult,
} from './api_keys';

View file

@ -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';