[9.0] [Security Solution][Endpoint] Add validation to artifact create…

…/update APIs for management of `ownerSpaceId` (#211325) (#212446) # Backport This will backport the following commits from `main` to `9.0`: - [[Security Solution][Endpoint] Add validation to artifact create/update APIs for management of `ownerSpaceId` (#211325)](#211325)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Feb 26, 2025 · a54f4dc · a54f4dc
1 parent 068928d
commit a54f4dc
Show file tree

Hide file tree

Showing 34 changed files with 1,455 additions and 760 deletions.
diff --git a/x-pack/platform/plugins/shared/fleet/common/constants/authz.ts b/x-pack/platform/plugins/shared/fleet/common/constants/authz.ts
@@ -108,6 +108,12 @@ export const ENDPOINT_PRIVILEGES: Record<string, PrivilegeMapObject> = deepFreez
     privilegeType: 'api',
     privilegeName: 'readEventFilters',
   },
+  writeGlobalArtifacts: {
+    appId: DEFAULT_APP_CATEGORIES.security.id,
+    privilegeSplit: '-',
+    privilegeType: 'api',
+    privilegeName: 'writeGlobalArtifacts',
+  },
   writePolicyManagement: {
     appId: DEFAULT_APP_CATEGORIES.security.id,
     privilegeSplit: '-',

diff --git a/...k/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts b/...k/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts
@@ -44,9 +44,21 @@ export const getPolicyIdsFromArtifact = (item: Pick<ExceptionListItemSchema, 'ta
   return policyIds;
 };
 
+/**
+ * Given an Artifact tag value, utility will return a boolean indicating if that tag is
+ * tracking artifact assignment (global/per-policy)
+ */
 export const isPolicySelectionTag: TagFilter = (tag) =>
   tag.startsWith(BY_POLICY_ARTIFACT_TAG_PREFIX) || tag === GLOBAL_ARTIFACT_TAG;
 
+/**
+ * Builds the per-policy tag that should be stored in the artifact's `tags` array
+ * @param policyId
+ */
+export const buildPerPolicyTag = (policyId: string): string => {
+  return `${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`;
+};
+
 /**
  * Return a list of artifact policy tags based on a current
  * selection by the EffectedPolicySelection component.
@@ -57,7 +69,7 @@ export const getArtifactTagsByPolicySelection = (selection: EffectedPolicySelect
   }
 
   return selection.selected.map((policy) => {
-    return `${BY_POLICY_ARTIFACT_TAG_PREFIX}${policy.id}`;
+    return buildPerPolicyTag(policy.id);
   });
 };
 
@@ -79,13 +91,16 @@ export const getEffectedPolicySelectionByTags = (
     };
   }
   const selected: PolicyData[] = tags.reduce((acc, tag) => {
-    // edge case: a left over tag with a non-existed policy
-    // will be removed by verifying the policy exists
-    const id = tag.split(':')[1];
-    const foundPolicy = policies.find((policy) => policy.id === id);
-    if (foundPolicy !== undefined) {
-      acc.push(foundPolicy);
+    if (tag.startsWith(BY_POLICY_ARTIFACT_TAG_PREFIX)) {
+      const id = tag.split(':')[1];
+      const foundPolicy = policies.find((policy) => policy.id === id);
+
+      // edge case: a left over tag with a non-existed policy will be removed by verifying the policy exists
+      if (foundPolicy !== undefined) {
+        acc.push(foundPolicy);
+      }
     }
+
     return acc;
   }, [] as PolicyData[]);
 
@@ -120,14 +135,22 @@ export const createExceptionListItemForCreate = (listId: string): CreateExceptio
   };
 };
 
+/**
+ * Checks the provided `tag` string to see if it is an owner apace ID tag
+ * @param tag
+ */
+export const isOwnerSpaceIdTag = (tag: string): boolean => {
+  return tag.startsWith(OWNER_SPACE_ID_TAG_PREFIX);
+};
+
 /**
  * Returns an array with all owner space IDs for the artifact
  */
 export const getArtifactOwnerSpaceIds = (
   item: Partial<Pick<ExceptionListItemSchema, 'tags'>>
 ): string[] => {
   return (item.tags ?? []).reduce((acc, tag) => {
-    if (tag.startsWith(OWNER_SPACE_ID_TAG_PREFIX)) {
+    if (isOwnerSpaceIdTag(tag)) {
       acc.push(tag.substring(OWNER_SPACE_ID_TAG_PREFIX.length));
     }
 
@@ -176,5 +199,5 @@ export const setArtifactOwnerSpaceId = (
 export const hasArtifactOwnerSpaceId = (
   item: Partial<Pick<ExceptionListItemSchema, 'tags'>>
 ): boolean => {
-  return (item.tags ?? []).some((tag) => tag.startsWith(OWNER_SPACE_ID_TAG_PREFIX));
+  return (item.tags ?? []).some((tag) => isOwnerSpaceIdTag(tag));
 };
diff --git a/.../solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.test.ts b/.../solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.test.ts
@@ -179,6 +179,7 @@ describe('Endpoint Authz service', () => {
       ['canReadEventFilters', 'readEventFilters'],
       ['canReadWorkflowInsights', 'readWorkflowInsights'],
       ['canWriteWorkflowInsights', 'writeWorkflowInsights'],
+      ['canManageGlobalArtifacts', 'writeGlobalArtifacts'],
     ])('%s should be true if `packagePrivilege.%s` is `true`', (auth) => {
       const authz = calculateEndpointAuthz(licenseService, fleetAuthz, userRoles);
       expect(authz[auth]).toBe(true);
@@ -220,6 +221,7 @@ describe('Endpoint Authz service', () => {
       ['canReadEventFilters', ['readEventFilters']],
       ['canWriteWorkflowInsights', ['writeWorkflowInsights']],
       ['canReadWorkflowInsights', ['readWorkflowInsights']],
+      ['canManageGlobalArtifacts', ['writeGlobalArtifacts']],
       // all dependent privileges are false and so it should be false
       ['canAccessResponseConsole', responseConsolePrivileges],
     ])('%s should be false if `packagePrivilege.%s` is `false`', (auth, privileges) => {
@@ -271,6 +273,7 @@ describe('Endpoint Authz service', () => {
       ['canReadEventFilters', ['readEventFilters']],
       ['canWriteWorkflowInsights', ['writeWorkflowInsights']],
       ['canReadWorkflowInsights', ['readWorkflowInsights']],
+      ['canManageGlobalArtifacts', ['writeGlobalArtifacts']],
       // all dependent privileges are false and so it should be false
       ['canAccessResponseConsole', responseConsolePrivileges],
     ])(
@@ -339,6 +342,7 @@ describe('Endpoint Authz service', () => {
         canWriteExecuteOperations: false,
         canWriteScanOperations: false,
         canWriteFileOperations: false,
+        canManageGlobalArtifacts: false,
         canWriteTrustedApplications: false,
         canWriteWorkflowInsights: false,
         canReadTrustedApplications: false,

diff --git a/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.ts b/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.ts
@@ -97,6 +97,8 @@ export const calculateEndpointAuthz = (
   const canReadEndpointExceptions = hasAuth('showEndpointExceptions');
   const canWriteEndpointExceptions = hasAuth('crudEndpointExceptions');
 
+  const canManageGlobalArtifacts = hasAuth('writeGlobalArtifacts');
+
   const canReadWorkflowInsights = hasAuth('readWorkflowInsights');
   const canWriteWorkflowInsights = hasAuth('writeWorkflowInsights');
 
@@ -156,6 +158,7 @@ export const calculateEndpointAuthz = (
     canReadEventFilters,
     canReadEndpointExceptions,
     canWriteEndpointExceptions,
+    canManageGlobalArtifacts,
   };
 
   // Response console is only accessible when license is Enterprise and user has access to any
@@ -212,6 +215,7 @@ export const getEndpointAuthzInitialState = (): EndpointAuthz => {
     canReadEventFilters: false,
     canReadEndpointExceptions: false,
     canWriteEndpointExceptions: false,
+    canManageGlobalArtifacts: false,
     canReadWorkflowInsights: false,
     canWriteWorkflowInsights: false,
   };

diff --git a/x-pack/solutions/security/plugins/security_solution/common/endpoint/types/authz.ts b/x-pack/solutions/security/plugins/security_solution/common/endpoint/types/authz.ts
@@ -93,6 +93,9 @@ export interface EndpointAuthz {
   canReadEndpointExceptions: boolean;
   /** if the user has read permissions for endpoint exceptions */
   canWriteEndpointExceptions: boolean;
+  /** If user is allowed to manage global artifacts. Introduced support for spaces feature */
+  canManageGlobalArtifacts: boolean;
+
   /** if the user has write permissions for workflow insights */
   canWriteWorkflowInsights: boolean;
   /** if the user has read permissions for workflow insights */

diff --git a/...lugins/security_solution/public/management/components/formatted_error/formatted_error.tsx b/...lugins/security_solution/public/management/components/formatted_error/formatted_error.tsx
@@ -18,7 +18,7 @@ export const ObjectContent = memo<ObjectContentProps>(({ data }) => {
     <EuiText size="s">
       {Object.entries(data).map(([key, value]) => {
         return (
-          <div key={key} className="eui-textBreakAll">
+          <div key={key} className="eui-textBreakWord">
             <strong>{key}</strong>
             {': '}
             {value}

diff --git a/...plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.test.tsx b/...plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.test.tsx
@@ -142,7 +142,7 @@ describe('useGetUpdatedTags hook', () => {
       // add first
       rerender({ exception: { tags }, filters: getFiltersInOrder() });
       tags = result.current.getTagsUpdatedBy('first', ['first:brie']);
-      expect(tags).toStrictEqual(['first:brie', 'special_second', 'third:spaghetti']);
+      expect(tags).toStrictEqual(['special_second', 'third:spaghetti', 'first:brie']);
     });
 
     it('should update category order on any change if filter is changed (although it should not)', () => {
@@ -155,11 +155,9 @@ describe('useGetUpdatedTags hook', () => {
       expect(tags).toStrictEqual([
         'first:mozzarella',
         'first:roquefort',
-
-        'second:shiraz',
-
         'third:tagliatelle',
         'third:penne',
+        'second:shiraz',
       ]);
 
       const newFilterOrder = {
@@ -172,12 +170,10 @@ describe('useGetUpdatedTags hook', () => {
       tags = result.current.getTagsUpdatedBy('third', ['third:spaghetti']);
 
       expect(tags).toStrictEqual([
-        'third:spaghetti',
-
         'first:mozzarella',
         'first:roquefort',
-
         'second:shiraz',
+        'third:spaghetti',
       ]);
     });
 

diff --git a/...rity/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.tsx b/...rity/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.tsx
@@ -8,20 +8,31 @@
 import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types';
 import { useCallback } from 'react';
 import type { TagFilter } from '../../../../common/endpoint/service/artifacts/utils';
+import {
+  isOwnerSpaceIdTag,
+  isFilterProcessDescendantsTag,
+  isPolicySelectionTag,
+} from '../../../../common/endpoint/service/artifacts/utils';
 
-type TagFiltersType = {
-  [tagCategory in string]: TagFilter;
-};
+interface TagFiltersType {
+  [tagCategory: string]: TagFilter;
+}
+
+type GetTagsUpdatedBy<TagFilters> = (tagType: keyof TagFilters, newTags: string[]) => string[];
 
-type GetTagsUpdatedBy<TagFilters> = (tagsToUpdate: keyof TagFilters, newTags: string[]) => string[];
+const DEFAULT_FILTERS = Object.freeze({
+  policySelection: isPolicySelectionTag,
+  processDescendantsFiltering: isFilterProcessDescendantsTag,
+  ownerSpaceId: isOwnerSpaceIdTag,
+} as const);
 
 /**
- * A hook to be used to generate a new `tags` array that contains multiple 'categories' of tags,
- * e.g. policy assignment, some special settings, in a desired order.
+ * A hook that returns a callback for using in updating the complete list of `tags` on an artifact.
+ * The callback will replace a given type of tag with new set of values - example: update the list
+ * of tags on an artifact with a new list of policy assignment tags.
  *
- * The hook excepts a `filter` object that contain a simple filter function for every tag
- * category. The `filter` object should contain the filters in the same ORDER as the categories
- * should appear in the `tags` array.
+ * The hook uses a `filter` object (can be overwritten on input) that contain a simple filter
+ * function that is used to identify tags for that category.
  *
  * ```
  * const FILTERS_IN_ORDER = { // preferably defined out of the component
@@ -42,25 +53,21 @@ type GetTagsUpdatedBy<TagFilters> = (tagsToUpdate: keyof TagFilters, newTags: st
  * @param filters
  * @returns `getTagsUpdatedBy(tagCategory, ['new', 'tags'])`
  */
-export const useGetUpdatedTags = <TagFilters extends TagFiltersType>(
+export const useGetUpdatedTags = <TagFilters extends TagFiltersType = typeof DEFAULT_FILTERS>(
   exception: Partial<Pick<ExceptionListItemSchema, 'tags'>>,
-  filters: TagFilters
-): {
+  filters: TagFilters = DEFAULT_FILTERS as unknown as TagFilters
+): Readonly<{
   getTagsUpdatedBy: GetTagsUpdatedBy<TagFilters>;
-} => {
+}> => {
   const getTagsUpdatedBy: GetTagsUpdatedBy<TagFilters> = useCallback(
-    (tagsToUpdate, newTags) => {
-      const tagCategories = Object.keys(filters);
-
-      const arrayOfTagArrays: string[][] = tagCategories.map((category) => {
-        if (tagsToUpdate === category) {
-          return newTags;
-        }
-
-        return (exception.tags ?? []).filter(filters[category]);
-      });
+    (tagType, newTags) => {
+      if (!filters[tagType]) {
+        throw new Error(
+          `getTagsUpdateBy() was called with an unknown tag type: ${String(tagType)}`
+        );
+      }
 
-      return arrayOfTagArrays.flat();
+      return (exception.tags ?? []).filter((tag) => !filters[tagType](tag)).concat(...newTags);
     },
     [exception, filters]
   );

diff --git a/...curity_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx b/...curity_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx
@@ -26,6 +26,7 @@ import type { PolicyData } from '../../../../../../common/endpoint/types';
 import { GLOBAL_ARTIFACT_TAG } from '../../../../../../common/endpoint/service/artifacts';
 import { ListOperatorEnum, ListOperatorTypeEnum } from '@kbn/securitysolution-io-ts-list-types';
 import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants';
+import type { IHttpFetchError } from '@kbn/core/public';
 
 jest.mock('../../../../../common/hooks/use_license', () => {
   const licenseServiceInstance = {
@@ -491,7 +492,7 @@ describe('blocklist form', () => {
     expect(screen.getByTestId('blocklist-form-effectedPolicies-global')).toBeEnabled();
   });
 
-  it('should correctly edit policies', async () => {
+  it('should correctly edit policies and retain all other tags', async () => {
     const policies: PolicyData[] = [
       {
         id: 'policy-id-123',
@@ -502,18 +503,18 @@ describe('blocklist form', () => {
         name: 'some-policy-456',
       },
     ] as PolicyData[];
-    render(createProps({ policies }));
+    render(createProps({ policies, item: createItem({ tags: ['some:random_tag'] }) }));
     const byPolicyButton = screen.getByTestId('blocklist-form-effectedPolicies-perPolicy');
     await user.click(byPolicyButton);
     expect(byPolicyButton).toBeEnabled();
 
     await user.click(screen.getByText(policies[1].name));
     const expected = createOnChangeArgs({
       item: createItem({
-        tags: [`policy:${policies[1].id}`],
+        tags: ['some:random_tag', `policy:${policies[1].id}`],
       }),
     });
-    expect(onChangeSpy).toHaveBeenCalledWith(expected);
+    expect(onChangeSpy).toHaveBeenLastCalledWith(expected);
   });
 
   it('should correctly retain selected policies when toggling between global/by policy', async () => {
@@ -567,4 +568,11 @@ describe('blocklist form', () => {
     });
     expect(onChangeSpy).toHaveBeenCalledWith(expected);
   });
+
+  it('should display submit errors', async () => {
+    const message = 'foo - something went wrong';
+    const { getByTestId } = render(createProps({ error: new Error(message) as IHttpFetchError }));
+
+    expect(getByTestId('blocklist-form-submitError').textContent).toMatch(message);
+  });
 });