[Security solution] [Endpoint] Adds API check for existing policies w…

…hen saving/creating trusted apps (elastic#106110) (elastic#106975) * Adds API check for existing policies when saving/creating trusted apps * Fixes wrong error message * Fixes tests and replaces policy id by policy name in error message * Updates error message text * Address pr comments and remove old comments in code * Addressed some pr comments and added type to the error response in order to know which kind of response it is * removed unused imports Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: David Sánchez <[email protected]>
yctercero · Jul 28, 2021 · 3e2a915 · 3e2a915
1 parent 9c28da7
commit 3e2a915
Show file tree

Hide file tree

Showing 8 changed files with 377 additions and 52 deletions.
diff --git a/x-pack/plugins/security_solution/public/common/types.ts b/x-pack/plugins/security_solution/public/common/types.ts
@@ -5,10 +5,12 @@
  * 2.0.
  */
 
+import { ResponseErrorAttributes } from 'kibana/server';
 export interface ServerApiError {
   statusCode: number;
   error: string;
   message: string;
+  attributes?: ResponseErrorAttributes | undefined;
 }
 
 export interface SecuritySolutionUiConfigType {

diff --git a/...lution/public/management/pages/trusted_apps/view/components/create_trusted_app_flyout.tsx b/...lution/public/management/pages/trusted_apps/view/components/create_trusted_app_flyout.tsx
@@ -66,14 +66,6 @@ export const CreateTrustedAppFlyout = memo<CreateTrustedAppFlyoutProps>(
 
     const dataTestSubj = flyoutProps['data-test-subj'];
 
-    const creationErrorsMessage = useMemo<string | undefined>(
-      () =>
-        creationErrors
-          ? CREATE_TRUSTED_APP_ERROR[creationErrors.message.replace(/(\[(.*)\]\: )/, '')] ||
-            creationErrors.message
-          : undefined,
-      [creationErrors]
-    );
     const policies = useMemo<CreateTrustedAppFormProps['policies']>(() => {
       return {
         // Casting is needed due to the use of `Immutable<>` on the return value from the selector above
@@ -82,6 +74,24 @@ export const CreateTrustedAppFlyout = memo<CreateTrustedAppFlyoutProps>(
       };
     }, [isLoadingPolicies, policyList]);
 
+    const creationErrorsMessage = useMemo<string | undefined>(() => {
+      let errorMessage = creationErrors
+        ? CREATE_TRUSTED_APP_ERROR[creationErrors.message.replace(/(\[(.*)\]\: )/, '')] ||
+          creationErrors.message
+        : undefined;
+
+      if (
+        creationErrors &&
+        creationErrors.attributes &&
+        creationErrors.attributes.type === 'TrustedApps/PolicyNotFound'
+      ) {
+        policies.options.forEach((policy) => {
+          errorMessage = errorMessage?.replace(policy.id, policy.name);
+        });
+      }
+      return errorMessage;
+    }, [creationErrors, policies]);
+
     const getTestId = useTestIdGenerator(dataTestSubj);
 
     const handleCancelClick = useCallback(() => {

diff --git a/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/errors.ts b/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/errors.ts
@@ -12,6 +12,17 @@ export class TrustedAppNotFoundError extends Error {
     super(`Trusted Application (${id}) not found`);
   }
 }
+export class TrustedAppPolicyNotExistsError extends Error {
+  public readonly type = 'TrustedApps/PolicyNotFound';
+
+  constructor(name: string, policyIds: string[]) {
+    super(
+      `Trusted Application (${name}) is assigned with a policy that no longer exists: ${policyIds.join(
+        ', '
+      )}`
+    );
+  }
+}
 
 export class TrustedAppVersionConflictError extends Error {
   constructor(id: string, public sourceError: Error) {

diff --git a/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/handlers.test.ts b/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/handlers.test.ts
@@ -5,10 +5,14 @@
  * 2.0.
  */
 
-import { KibanaResponseFactory } from 'kibana/server';
+import { KibanaResponseFactory, SavedObjectsClientContract } from 'kibana/server';
 
 import { xpackMocks } from '../../../fixtures';
-import { loggingSystemMock, httpServerMock } from '../../../../../../../src/core/server/mocks';
+import {
+  loggingSystemMock,
+  httpServerMock,
+  savedObjectsClientMock,
+} from '../../../../../../../src/core/server/mocks';
 import { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types';
 import { listMock } from '../../../../../lists/server/mocks';
 import { ExceptionListClient } from '../../../../../lists/server';
@@ -32,9 +36,16 @@ import {
   getTrustedAppsUpdateRouteHandler,
 } from './handlers';
 import type { SecuritySolutionRequestHandlerContext } from '../../../types';
-import { TrustedAppNotFoundError, TrustedAppVersionConflictError } from './errors';
+import {
+  TrustedAppNotFoundError,
+  TrustedAppVersionConflictError,
+  TrustedAppPolicyNotExistsError,
+} from './errors';
 import { updateExceptionListItemImplementationMock } from './test_utils';
 import { Logger } from '@kbn/logging';
+import { PackagePolicyServiceInterface } from '../../../../../fleet/server';
+import { createPackagePolicyServiceMock } from '../../../../../fleet/server/mocks';
+import { getPackagePoliciesResponse, getTrustedAppByPolicy } from './mocks';
 
 const EXCEPTION_LIST_ITEM: ExceptionListItemSchema = {
   _version: 'abc123',
@@ -88,7 +99,14 @@ const TRUSTED_APP: TrustedApp = {
   ],
 };
 
+const packagePolicyClient = createPackagePolicyServiceMock() as jest.Mocked<PackagePolicyServiceInterface>;
+const savedObjectClient = savedObjectsClientMock.create() as jest.Mocked<SavedObjectsClientContract>;
+
 describe('handlers', () => {
+  beforeEach(() => {
+    packagePolicyClient.getByIDs.mockReset();
+  });
+
   const createAppContextMock = () => {
     const context = {
       logFactory: loggingSystemMock.create(),
@@ -97,6 +115,9 @@ describe('handlers', () => {
       experimentalFeatures: parseExperimentalConfigValue(createMockConfig().enableExperimental),
     };
 
+    context.service.getPackagePolicyService = () => packagePolicyClient;
+    context.service.getScopedSavedObjectsClient = () => savedObjectClient;
+
     // Ensure that `logFactory.get()` always returns the same instance for the same given prefix
     const instances = new Map<string, ReturnType<typeof context.logFactory.get>>();
     const logFactoryGetMock = context.logFactory.get.getMockImplementation();
@@ -224,6 +245,28 @@ describe('handlers', () => {
         )
       ).rejects.toThrowError(error);
     });
+
+    it("should return error when policy doesn't exists", async () => {
+      const mockResponse = httpServerMock.createResponseFactory();
+      packagePolicyClient.getByIDs.mockReset();
+      packagePolicyClient.getByIDs.mockResolvedValueOnce(getPackagePoliciesResponse());
+
+      const trustedAppByPolicy = getTrustedAppByPolicy();
+      await createTrustedAppHandler(
+        createHandlerContextMock(),
+        httpServerMock.createKibanaRequest({ body: trustedAppByPolicy }),
+        mockResponse
+      );
+
+      const error = new TrustedAppPolicyNotExistsError(trustedAppByPolicy.name, [
+        '9da95be9-9bee-4761-a8c4-28d6d9bd8c71',
+      ]);
+
+      expect(appContextMock.logFactory.get('trusted_apps').error).toHaveBeenCalledWith(error);
+      expect(mockResponse.badRequest).toHaveBeenCalledWith({
+        body: { message: error.message, attributes: { type: error.type } },
+      });
+    });
   });
 
   describe('getTrustedAppsListRouteHandler', () => {
@@ -508,5 +551,23 @@ describe('handlers', () => {
         body: expect.any(TrustedAppVersionConflictError),
       });
     });
+
+    it("should return error when policy doesn't exists", async () => {
+      packagePolicyClient.getByIDs.mockReset();
+      packagePolicyClient.getByIDs.mockResolvedValueOnce(getPackagePoliciesResponse());
+
+      const trustedAppByPolicy = getTrustedAppByPolicy();
+      await updateHandler(
+        createHandlerContextMock(),
+        httpServerMock.createKibanaRequest({ body: trustedAppByPolicy }),
+        mockResponse
+      );
+
+      expect(appContextMock.logFactory.get('trusted_apps').error).toHaveBeenCalledWith(
+        new TrustedAppPolicyNotExistsError(trustedAppByPolicy.name, [
+          '9da95be9-9bee-4761-a8c4-28d6d9bd8c71',
+        ])
+      );
+    });
   });
 });
diff --git a/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/handlers.ts b/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/handlers.ts
@@ -28,7 +28,12 @@ import {
   getTrustedAppsSummary,
   updateTrustedApp,
 } from './service';
-import { TrustedAppNotFoundError, TrustedAppVersionConflictError } from './errors';
+import {
+  TrustedAppNotFoundError,
+  TrustedAppVersionConflictError,
+  TrustedAppPolicyNotExistsError,
+} from './errors';
+import { PackagePolicyServiceInterface } from '../../../../../fleet/server';
 
 const getBodyAfterFeatureFlagCheck = (
   body: PutTrustedAppUpdateRequest | PostTrustedAppCreateRequest,
@@ -54,6 +59,18 @@ const exceptionListClientFromContext = (
   return exceptionLists;
 };
 
+const packagePolicyClientFromEndpointContext = (
+  endpointAppContext: EndpointAppContext
+): PackagePolicyServiceInterface => {
+  const packagePolicy = endpointAppContext.service.getPackagePolicyService();
+
+  if (!packagePolicy) {
+    throw new Error('Package policy service not found');
+  }
+
+  return packagePolicy;
+};
+
 const errorHandler = <E extends Error>(
   logger: Logger,
   res: KibanaResponseFactory,
@@ -64,6 +81,11 @@ const errorHandler = <E extends Error>(
     return res.notFound({ body: error });
   }
 
+  if (error instanceof TrustedAppPolicyNotExistsError) {
+    logger.error(error);
+    return res.badRequest({ body: { message: error.message, attributes: { type: error.type } } });
+  }
+
   if (error instanceof TrustedAppVersionConflictError) {
     logger.error(error);
     return res.conflict({ body: error });
@@ -150,7 +172,12 @@ export const getTrustedAppsCreateRouteHandler = (
       const body = getBodyAfterFeatureFlagCheck(req.body, endpointAppContext);
 
       return res.ok({
-        body: await createTrustedApp(exceptionListClientFromContext(context), body),
+        body: await createTrustedApp(
+          exceptionListClientFromContext(context),
+          context.core.savedObjects.client,
+          packagePolicyClientFromEndpointContext(endpointAppContext),
+          body
+        ),
       });
     } catch (error) {
       return errorHandler(logger, res, error);
@@ -173,7 +200,13 @@ export const getTrustedAppsUpdateRouteHandler = (
       const body = getBodyAfterFeatureFlagCheck(req.body, endpointAppContext);
 
       return res.ok({
-        body: await updateTrustedApp(exceptionListClientFromContext(context), req.params.id, body),
+        body: await updateTrustedApp(
+          exceptionListClientFromContext(context),
+          context.core.savedObjects.client,
+          packagePolicyClientFromEndpointContext(endpointAppContext),
+          req.params.id,
+          body
+        ),
       });
     } catch (error) {
       return errorHandler(logger, res, error);

diff --git a/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/mocks.ts b/x-pack/plugins/security_solution/server/endpoint/routes/trusted_apps/mocks.ts
@@ -0,0 +1,62 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { PackagePolicy } from '../../../../../fleet/common';
+
+import {
+  ConditionEntryField,
+  OperatingSystem,
+  TrustedApp,
+} from '../../../../common/endpoint/types';
+import { createConditionEntry } from './mapping';
+
+export const getTrustedAppByPolicy = function (): TrustedApp {
+  return {
+    id: '123',
+    version: 'abc123',
+    created_at: '11/11/2011T11:11:11.111',
+    created_by: 'admin',
+    updated_at: '11/11/2011T11:11:11.111',
+    updated_by: 'admin',
+    name: 'linux trusted app 1',
+    description: 'Linux trusted app 1',
+    os: OperatingSystem.LINUX,
+    effectScope: {
+      type: 'policy',
+      policies: ['e5cbb9cf-98aa-4303-a04b-6a1165915079', '9da95be9-9bee-4761-a8c4-28d6d9bd8c71'],
+    },
+    entries: [
+      createConditionEntry(ConditionEntryField.HASH, 'match', '1234234659af249ddf3e40864e9fb241'),
+      createConditionEntry(ConditionEntryField.PATH, 'match', '/bin/malware'),
+    ],
+  };
+};
+
+export const getPackagePoliciesResponse = function (): PackagePolicy[] {
+  return [
+    // Next line is ts-ignored as this is the response when the policy doesn't exists but the type is complaining about it.
+    // @ts-ignore
+    { id: '9da95be9-9bee-4761-a8c4-28d6d9bd8c71', version: undefined },
+    {
+      id: 'e5cbb9cf-98aa-4303-a04b-6a1165915079',
+      version: 'Wzc0NDk5LDFd',
+      name: 'EI 2',
+      description: '',
+      namespace: 'default',
+      policy_id: '9fd2ac50-e86f-11eb-a87f-51e16104076a',
+      enabled: true,
+      output_id: '',
+      inputs: [],
+      package: { name: 'endpoint', title: 'Endpoint Security', version: '0.20.1' },
+      revision: 3,
+      created_at: '2021-07-19T09:00:45.608Z',
+      created_by: 'elastic',
+      updated_at: '2021-07-19T09:02:47.193Z',
+      updated_by: 'system',
+    },
+  ];
+};