[Security solution] [Endpoint] Remove linked policy from trusted apps…

… when removing endpoint integration (#108347) * Remove policy from trusted app when this is removed from fleet * Fleet: run package delete external callbacks when the Agent Policy is deleted
elastic · Aug 19, 2021 · 79e63cc · 79e63cc
1 parent 8d1ebea
commit 79e63cc
Show file tree

Hide file tree

Showing 13 changed files with 340 additions and 24 deletions.
diff --git a/x-pack/plugins/apm/server/lib/fleet/register_fleet_policy_callbacks.ts b/x-pack/plugins/apm/server/lib/fleet/register_fleet_policy_callbacks.ts
@@ -7,8 +7,7 @@
 
 import { APMPlugin, APMRouteHandlerResources } from '../..';
 import {
-  ExternalCallback,
-  PostPackagePolicyDeleteCallback,
+  PostPackagePolicyCreateCallback,
   PutPackagePolicyUpdateCallback,
 } from '../../../../fleet/server';
 import {
@@ -60,7 +59,9 @@ export async function registerFleetPolicyCallbacks({
   });
 }
 
-type ExternalCallbackParams = Parameters<ExternalCallback[1]>;
+type ExternalCallbackParams =
+  | Parameters<PostPackagePolicyCreateCallback>
+  | Parameters<PutPackagePolicyUpdateCallback>;
 export type PackagePolicy = NewPackagePolicy | UpdatePackagePolicy;
 type Context = ExternalCallbackParams[1];
 type Request = ExternalCallbackParams[2];
@@ -81,7 +82,7 @@ function registerPackagePolicyExternalCallback({
   logger: NonNullable<APMPlugin['logger']>;
 }) {
   const callbackFn:
-    | PostPackagePolicyDeleteCallback
+    | PostPackagePolicyCreateCallback
     | PutPackagePolicyUpdateCallback = async (
     packagePolicy: PackagePolicy,
     context: Context,

diff --git a/x-pack/plugins/fleet/common/mocks.ts b/x-pack/plugins/fleet/common/mocks.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import type { NewPackagePolicy, PackagePolicy } from './types';
+import type { NewPackagePolicy, PackagePolicy, DeletePackagePoliciesResponse } from './types';
 
 export const createNewPackagePolicyMock = (): NewPackagePolicy => {
   return {
@@ -45,3 +45,14 @@ export const createPackagePolicyMock = (): PackagePolicy => {
     ],
   };
 };
+
+export const deletePackagePolicyMock = (): DeletePackagePoliciesResponse => {
+  const newPackagePolicy = createNewPackagePolicyMock();
+  return [
+    {
+      id: 'c6d16e42-c32d-4dce-8a88-113cfe276ad1',
+      success: true,
+      package: newPackagePolicy.package,
+    },
+  ];
+};
diff --git a/x-pack/plugins/fleet/server/mocks/index.ts b/x-pack/plugins/fleet/server/mocks/index.ts
@@ -62,7 +62,7 @@ export const xpackMocks = {
   createRequestHandlerContext: createCoreRequestHandlerContextMock,
 };
 
-export const createPackagePolicyServiceMock = () => {
+export const createPackagePolicyServiceMock = (): jest.Mocked<PackagePolicyServiceInterface> => {
   return {
     compilePackagePolicyInputs: jest.fn(),
     buildPackagePolicyFromPackage: jest.fn(),
@@ -75,10 +75,11 @@ export const createPackagePolicyServiceMock = () => {
     listIds: jest.fn(),
     update: jest.fn(),
     runExternalCallbacks: jest.fn(),
+    runDeleteExternalCallbacks: jest.fn(),
     upgrade: jest.fn(),
     getUpgradeDryRunDiff: jest.fn(),
     getUpgradePackagePolicyInfo: jest.fn(),
-  } as jest.Mocked<PackagePolicyServiceInterface>;
+  };
 };
 
 /**

diff --git a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
@@ -181,6 +181,7 @@ export const deletePackagePolicyHandler: RequestHandler<
     } catch (error) {
       const logger = appContextService.getLogger();
       logger.error(`An error occurred executing external callback: ${error}`);
+      logger.error(error);
     }
     return response.ok({
       body,

diff --git a/x-pack/plugins/fleet/server/services/agent_policy.test.ts b/x-pack/plugins/fleet/server/services/agent_policy.test.ts
@@ -12,6 +12,9 @@ import type { AgentPolicy, NewAgentPolicy, Output } from '../types';
 import { agentPolicyService } from './agent_policy';
 import { agentPolicyUpdateEventHandler } from './agent_policy_update';
 
+import { getAgentsByKuery } from './agents';
+import { packagePolicyService } from './package_policy';
+
 function getSavedObjectMock(agentPolicyAttributes: any) {
   const mock = savedObjectsClientMock.create();
   mock.get.mockImplementation(async (type: string, id: string) => {
@@ -63,6 +66,8 @@ jest.mock('./output', () => {
 });
 
 jest.mock('./agent_policy_update');
+jest.mock('./agents');
+jest.mock('./package_policy');
 
 function getAgentPolicyUpdateMock() {
   return (agentPolicyUpdateEventHandler as unknown) as jest.Mock<
@@ -123,6 +128,36 @@ describe('agent policy', () => {
     });
   });
 
+  describe('delete', () => {
+    let soClient: ReturnType<typeof savedObjectsClientMock.create>;
+    let esClient: ReturnType<typeof elasticsearchServiceMock.createClusterClient>['asInternalUser'];
+
+    beforeEach(() => {
+      soClient = getSavedObjectMock({ revision: 1, package_policies: ['package-1'] });
+      esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
+
+      (getAgentsByKuery as jest.Mock).mockResolvedValue({
+        agents: [],
+        total: 0,
+        page: 1,
+        perPage: 10,
+      });
+
+      (packagePolicyService.delete as jest.Mock).mockResolvedValue([
+        {
+          id: 'package-1',
+        },
+      ]);
+    });
+
+    it('should run package policy delete external callbacks', async () => {
+      await agentPolicyService.delete(soClient, esClient, 'mocked');
+      expect(packagePolicyService.runDeleteExternalCallbacks).toHaveBeenCalledWith([
+        { id: 'package-1' },
+      ]);
+    });
+  });
+
   describe('bumpRevision', () => {
     it('should call agentPolicyUpdateEventHandler with updated event once', async () => {
       const soClient = getSavedObjectMock({

diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts
@@ -45,6 +45,7 @@ import type {
   FleetServerPolicy,
   Installation,
   Output,
+  DeletePackagePoliciesResponse,
 } from '../../common';
 import { AgentPolicyNameExistsError, HostedAgentPolicyRestrictionRelatedError } from '../errors';
 import {
@@ -616,14 +617,21 @@ class AgentPolicyService {
     }
 
     if (agentPolicy.package_policies && agentPolicy.package_policies.length) {
-      await packagePolicyService.delete(
+      const deletedPackagePolicies: DeletePackagePoliciesResponse = await packagePolicyService.delete(
         soClient,
         esClient,
         agentPolicy.package_policies as string[],
         {
           skipUnassignFromAgentPolicies: true,
         }
       );
+      try {
+        await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies);
+      } catch (error) {
+        const logger = appContextService.getLogger();
+        logger.error(`An error occurred executing external callback: ${error}`);
+        logger.error(error);
+      }
     }
 
     if (agentPolicy.is_preconfigured) {

diff --git a/x-pack/plugins/fleet/server/services/package_policy.test.ts b/x-pack/plugins/fleet/server/services/package_policy.test.ts
@@ -20,6 +20,12 @@ import type { PutPackagePolicyUpdateCallback, PostPackagePolicyCreateCallback }
 
 import { createAppContextStartContractMock, xpackMocks } from '../mocks';
 
+import type { PostPackagePolicyDeleteCallback } from '../types';
+
+import type { DeletePackagePoliciesResponse } from '../../common';
+
+import { IngestManagerError } from '../errors';
+
 import { packagePolicyService } from './package_policy';
 import { appContextService } from './app_context';
 
@@ -815,6 +821,78 @@ describe('Package policy service', () => {
     });
   });
 
+  describe('runDeleteExternalCallbacks', () => {
+    let callbackOne: jest.MockedFunction<PostPackagePolicyDeleteCallback>;
+    let callbackTwo: jest.MockedFunction<PostPackagePolicyDeleteCallback>;
+    let callingOrder: string[];
+    let deletedPackagePolicies: DeletePackagePoliciesResponse;
+
+    beforeEach(() => {
+      appContextService.start(createAppContextStartContractMock());
+      callingOrder = [];
+      deletedPackagePolicies = [
+        { id: 'a', success: true },
+        { id: 'a', success: true },
+      ];
+      callbackOne = jest.fn(async (deletedPolicies) => {
+        callingOrder.push('one');
+      });
+      callbackTwo = jest.fn(async (deletedPolicies) => {
+        callingOrder.push('two');
+      });
+      appContextService.addExternalCallback('postPackagePolicyDelete', callbackOne);
+      appContextService.addExternalCallback('postPackagePolicyDelete', callbackTwo);
+    });
+
+    afterEach(() => {
+      appContextService.stop();
+    });
+
+    it('should execute external callbacks', async () => {
+      await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies);
+
+      expect(callbackOne).toHaveBeenCalledWith(deletedPackagePolicies);
+      expect(callbackTwo).toHaveBeenCalledWith(deletedPackagePolicies);
+      expect(callingOrder).toEqual(['one', 'two']);
+    });
+
+    it("should execute all external callbacks even if one throw's", async () => {
+      callbackOne.mockImplementation(async (deletedPolicies) => {
+        callingOrder.push('one');
+        throw new Error('foo');
+      });
+      await expect(
+        packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies)
+      ).rejects.toThrow(IngestManagerError);
+      expect(callingOrder).toEqual(['one', 'two']);
+    });
+
+    it('should provide an array of errors encountered by running external callbacks', async () => {
+      let error: IngestManagerError;
+      const callbackOneError = new Error('foo 1');
+      const callbackTwoError = new Error('foo 2');
+
+      callbackOne.mockImplementation(async (deletedPolicies) => {
+        callingOrder.push('one');
+        throw callbackOneError;
+      });
+      callbackTwo.mockImplementation(async (deletedPolicies) => {
+        callingOrder.push('two');
+        throw callbackTwoError;
+      });
+
+      await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies).catch((e) => {
+        error = e;
+      });
+
+      expect(error!.message).toEqual(
+        '2 encountered while executing package delete external callbacks'
+      );
+      expect(error!.meta).toEqual([callbackOneError, callbackTwoError]);
+      expect(callingOrder).toEqual(['one', 'two']);
+    });
+  });
+
   describe('runExternalCallbacks', () => {
     let context: ReturnType<typeof xpackMocks.createRequestHandlerContext>;
     let request: KibanaRequest;

diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts
@@ -428,9 +428,9 @@ class PackagePolicyService {
           name: packagePolicy.name,
           success: true,
           package: {
-            name: packagePolicy.name,
-            title: '',
-            version: packagePolicy.version || '',
+            name: packagePolicy.package?.name || '',
+            title: packagePolicy.package?.title || '',
+            version: packagePolicy.package?.version || '',
           },
         });
       } catch (error) {
@@ -642,7 +642,9 @@ class PackagePolicyService {
 
   public async runExternalCallbacks<A extends ExternalCallback[0]>(
     externalCallbackType: A,
-    packagePolicy: NewPackagePolicy | DeletePackagePoliciesResponse,
+    packagePolicy: A extends 'postPackagePolicyDelete'
+      ? DeletePackagePoliciesResponse
+      : NewPackagePolicy,
     context: RequestHandlerContext,
     request: KibanaRequest
   ): Promise<A extends 'postPackagePolicyDelete' ? void : NewPackagePolicy>;
@@ -653,14 +655,7 @@ class PackagePolicyService {
     request: KibanaRequest
   ): Promise<NewPackagePolicy | void> {
     if (externalCallbackType === 'postPackagePolicyDelete') {
-      const externalCallbacks = appContextService.getExternalCallbacks(externalCallbackType);
-      if (externalCallbacks && externalCallbacks.size > 0) {
-        for (const callback of externalCallbacks) {
-          if (Array.isArray(packagePolicy)) {
-            await callback(packagePolicy, context, request);
-          }
-        }
-      }
+      return await this.runDeleteExternalCallbacks(packagePolicy as DeletePackagePoliciesResponse);
     } else {
       if (!Array.isArray(packagePolicy)) {
         let newData = packagePolicy;
@@ -682,6 +677,32 @@ class PackagePolicyService {
       }
     }
   }
+
+  public async runDeleteExternalCallbacks(
+    deletedPackagePolicies: DeletePackagePoliciesResponse
+  ): Promise<void> {
+    const externalCallbacks = appContextService.getExternalCallbacks('postPackagePolicyDelete');
+    const errorsThrown: Error[] = [];
+
+    if (externalCallbacks && externalCallbacks.size > 0) {
+      for (const callback of externalCallbacks) {
+        // Failures from an external callback should not prevent other external callbacks from being
+        // executed. Errors (if any) will be collected and `throw`n after processing the entire set
+        try {
+          await callback(deletedPackagePolicies);
+        } catch (error) {
+          errorsThrown.push(error);
+        }
+      }
+
+      if (errorsThrown.length > 0) {
+        throw new IngestManagerError(
+          `${errorsThrown.length} encountered while executing package delete external callbacks`,
+          errorsThrown
+        );
+      }
+    }
+  }
 }
 
 function assignStreamIdToInput(packagePolicyId: string, input: NewPackagePolicyInput) {

diff --git a/x-pack/plugins/fleet/server/types/extensions.ts b/x-pack/plugins/fleet/server/types/extensions.ts
@@ -7,16 +7,16 @@
 
 import type { KibanaRequest, RequestHandlerContext } from 'kibana/server';
 
+import type { DeepReadonly } from 'utility-types';
+
 import type {
   DeletePackagePoliciesResponse,
   NewPackagePolicy,
   UpdatePackagePolicy,
 } from '../../common';
 
 export type PostPackagePolicyDeleteCallback = (
-  deletedPackagePolicies: DeletePackagePoliciesResponse,
-  context: RequestHandlerContext,
-  request: KibanaRequest
+  deletedPackagePolicies: DeepReadonly<DeletePackagePoliciesResponse>
 ) => Promise<void>;
 
 export type PostPackagePolicyCreateCallback = (

diff --git a/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts b/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts
@@ -22,6 +22,7 @@ import { PluginStartContract as AlertsPluginStartContract } from '../../../alert
 import {
   getPackagePolicyCreateCallback,
   getPackagePolicyUpdateCallback,
+  getPackagePolicyDeleteCallback,
 } from '../fleet_integration/fleet_integration';
 import { ManifestManager } from './services/artifacts';
 import { AppClientFactory } from '../client';
@@ -102,6 +103,11 @@ export class EndpointAppContextService {
         'packagePolicyUpdate',
         getPackagePolicyUpdateCallback(dependencies.logger, dependencies.licenseService)
       );
+
+      dependencies.registerIngestCallback(
+        'postPackagePolicyDelete',
+        getPackagePolicyDeleteCallback(dependencies.exceptionListsClient, this.experimentalFeatures)
+      );
     }
   }