Skip to content

Commit

Permalink
Replaced single invalidateApiKey request with the bulk
Browse files Browse the repository at this point in the history
  • Loading branch information
YulNaumenko committed Jan 5, 2021
1 parent dddd8e4 commit 820e660
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 17 deletions.
29 changes: 16 additions & 13 deletions x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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<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 {
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++;
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand All @@ -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: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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<InvalidateAPIKeyResult>({
body: { ids: [params.id] },
body: { ids: params.ids },
})
).body;
this.logger.debug('API key was invalidated successfully');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ export {
InvalidateAPIKeyResult,
CreateAPIKeyParams,
InvalidateAPIKeyParams,
InvalidateAPIKeysParams,
GrantAPIKeyResult,
} from './api_keys';
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/authentication/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ export type {
InvalidateAPIKeyResult,
CreateAPIKeyParams,
InvalidateAPIKeyParams,
InvalidateAPIKeysParams,
GrantAPIKeyResult,
} from './api_keys';
1 change: 1 addition & 0 deletions x-pack/plugins/security/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
export type {
CreateAPIKeyResult,
InvalidateAPIKeyParams,
InvalidateAPIKeysParams,
InvalidateAPIKeyResult,
GrantAPIKeyResult,
} from './authentication';
Expand Down

0 comments on commit 820e660

Please sign in to comment.