Extended invalidate method to support multiple invalidation. Updated …

…fleets plugin usage of this API.
elastic · Jan 7, 2021 · d6be4c8 · d6be4c8
1 parent f3211eb
commit d6be4c8
Show file tree

Hide file tree

Showing 9 changed files with 47 additions and 55 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
@@ -203,23 +203,25 @@ async function invalidateApiKeys(
       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++;
-        } catch (err) {
-          logger.error(
-            `Failed to cleanup api key "${apiKeyObj.attributes.apiKeyId}". Error: ${err.message}`
-          );
-        }
-      })
-    );
+  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}"`);
+  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
@@ -76,10 +76,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.invalidateAPIKey(soClient, [agent.access_api_key_id])
       : undefined,
     agent.default_api_key_id
-      ? APIKeyService.invalidateAPIKey(soClient, agent.default_api_key_id)
+      ? APIKeyService.invalidateAPIKey(soClient, [agent.default_api_key_id])
       : undefined,
   ]);
 
@@ -124,16 +124,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.invalidateAPIKey(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
@@ -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 invalidateAPIKey(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/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 invalidateAPIKey(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
@@ -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'],

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
@@ -38,13 +38,6 @@ interface GrantAPIKeyParams {
   access_token?: string;
 }
 
-/**
- * Represents the params for invalidating an API key
- */
-export interface InvalidateAPIKeyParams {
-  id: string;
-}
-
 /**
  * Represents the params for invalidating multiple API keys
  */
@@ -229,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 {
@@ -247,28 +240,34 @@ 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;
     }
 
     return result;
   }
 
   /**
-   * Tries to invalidate an API keys by using the internal user.
-   * @param params The params to invalidate an API keys.
+   * Tries to invalidate the API keys by using the internal user.
+   * @param params The params to invalidate the API keys.
    */
   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 {
@@ -278,9 +277,11 @@ export class APIKeys {
           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
@@ -9,7 +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
@@ -28,7 +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
@@ -24,7 +24,6 @@ import {
 // functions or removal of exports should be considered as a breaking change.
 export type {
   CreateAPIKeyResult,
-  InvalidateAPIKeyParams,
   InvalidateAPIKeysParams,
   InvalidateAPIKeyResult,
   GrantAPIKeyResult,