From 1ee97c3c8f3780cde8c23edb03b37738b506aefa Mon Sep 17 00:00:00 2001 From: Paul Tavares <56442535+paul-tavares@users.noreply.github.com> Date: Tue, 25 Feb 2025 14:52:08 -0500 Subject: [PATCH] [Security Solution][Endpoint] Add validation to artifact create/update APIs for management of `ownerSpaceId` (#211325) ## Summary #### Changes in support of space awareness > currently behind feature flag: `endpointManagementSpaceAwarenessEnabled` - Add logic to the server-side Lists plugin extension points for endpoint artifacts to ensure that only a user with the new Global Artifact Management privilege can update/change/add `ownerSpaceId` tags on an artifact - Added validation to all endpoint artifacts (Trusted Apps, Event Filters, Blocklists, Host Isolation Exceptions and Endpoint Exceptions) #### Other changes: - Fix UI bug that failed to display artifact submit API failures. API errors are now displayed in the artifact's respective edit/create forms if encountered - Fixed a bug where "unknown" artifact `tags` were being dropped whenever the artifact assignment (global, per-policy) was updated in the UI ## Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../shared/fleet/common/constants/authz.ts | 6 + .../endpoint/service/artifacts/utils.ts | 41 +- .../endpoint/service/authz/authz.test.ts | 4 + .../common/endpoint/service/authz/authz.ts | 4 + .../common/endpoint/types/authz.ts | 3 + .../formatted_error/formatted_error.tsx | 2 +- .../artifacts/use_get_updated_tags.test.tsx | 10 +- .../hooks/artifacts/use_get_updated_tags.tsx | 55 +- .../view/components/blocklist_form.test.tsx | 16 +- .../view/components/blocklist_form.tsx | 20 +- .../view/components/form.test.tsx | 32 +- .../event_filters/view/components/form.tsx | 1197 +++++++++-------- .../view/components/form.tsx | 23 +- .../integration_tests/form.test.tsx | 22 +- .../view/components/form.test.tsx | 25 + .../trusted_apps/view/components/form.tsx | 22 +- .../endpoint/common/role_and_user_loader.ts | 42 +- .../server/endpoint/errors.ts | 8 +- .../server/endpoint/mocks/mocks.ts | 6 +- .../lib/base_response_actions_client.ts | 5 + .../handlers/exceptions_pre_update_handler.ts | 10 +- .../validators/base_validator.test.ts | 106 ++ .../endpoint/validators/base_validator.ts | 90 +- .../validators/blocklist_validator.ts | 2 + .../endpoint_exceptions_validator.ts | 8 +- .../validators/event_filter_validator.ts | 3 + .../host_isolation_exceptions_validator.ts | 6 +- .../endpoint/validators/mocks.ts | 11 + .../validators/trusted_app_validator.ts | 2 + ...rity_solution_edr_workflows_roles_users.ts | 11 + .../trial_license_complete_tier/artifacts.ts | 339 +++++ .../trial_license_complete_tier/index.ts | 1 + .../space_awareness.ts | 82 -- .../tsconfig.json | 1 + 34 files changed, 1455 insertions(+), 760 deletions(-) create mode 100644 x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/artifacts.ts diff --git a/x-pack/platform/plugins/shared/fleet/common/constants/authz.ts b/x-pack/platform/plugins/shared/fleet/common/constants/authz.ts index 81928bfeb5060..4363f45acf9d8 100644 --- 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 = 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/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts b/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts index e9026f092c7eb..eb321be24c375 100644 --- a/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts +++ b/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/artifacts/utils.ts @@ -44,9 +44,21 @@ export const getPolicyIdsFromArtifact = (item: Pick 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,6 +135,14 @@ 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 */ @@ -127,7 +150,7 @@ export const getArtifactOwnerSpaceIds = ( item: Partial> ): 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> ): boolean => { - return (item.tags ?? []).some((tag) => tag.startsWith(OWNER_SPACE_ID_TAG_PREFIX)); + return (item.tags ?? []).some((tag) => isOwnerSpaceIdTag(tag)); }; diff --git a/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.test.ts b/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.test.ts index 576ff622261c8..58418c2602351 100644 --- a/x-pack/solutions/security/plugins/security_solution/common/endpoint/service/authz/authz.test.ts +++ b/x-pack/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 index 518e5bf1596f4..d228ab5bdda1d 100644 --- 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 index ff34074b5e6ee..21fa45b77595d 100644 --- 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/x-pack/solutions/security/plugins/security_solution/public/management/components/formatted_error/formatted_error.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/components/formatted_error/formatted_error.tsx index e7007edd9e6fd..84f5725edbede 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/components/formatted_error/formatted_error.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/components/formatted_error/formatted_error.tsx @@ -18,7 +18,7 @@ export const ObjectContent = memo(({ data }) => { {Object.entries(data).map(([key, value]) => { return ( -
+
{key} {': '} {value} diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.test.tsx index 1461f68fd1aaf..e286a12cb054b 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.test.tsx +++ b/x-pack/solutions/security/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/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.tsx index 0c651304c72d1..bf5a5d67d77e1 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/hooks/artifacts/use_get_updated_tags.tsx +++ b/x-pack/solutions/security/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 = (tagType: keyof TagFilters, newTags: string[]) => string[]; -type GetTagsUpdatedBy = (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 = (tagsToUpdate: keyof TagFilters, newTags: st * @param filters * @returns `getTagsUpdatedBy(tagCategory, ['new', 'tags'])` */ -export const useGetUpdatedTags = ( +export const useGetUpdatedTags = ( exception: Partial>, - filters: TagFilters -): { + filters: TagFilters = DEFAULT_FILTERS as unknown as TagFilters +): Readonly<{ getTagsUpdatedBy: GetTagsUpdatedBy; -} => { +}> => { const getTagsUpdatedBy: GetTagsUpdatedBy = 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/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx index 1c2ed0fd028b9..974a91e89005f 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.test.tsx +++ b/x-pack/solutions/security/plugins/security_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,7 +503,7 @@ 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(); @@ -510,10 +511,10 @@ describe('blocklist form', () => { 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); + }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.tsx index fcdcc016c3f37..5cfb6a93904dc 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/blocklist/view/components/blocklist_form.tsx @@ -29,6 +29,7 @@ import { isOneOfOperator, isOperator } from '@kbn/securitysolution-list-utils'; import { uniq } from 'lodash'; import { ListOperatorEnum, ListOperatorTypeEnum } from '@kbn/securitysolution-io-ts-list-types'; +import { FormattedError } from '../../../../components/formatted_error'; import { OS_TITLES } from '../../../../common/translations'; import type { ArtifactFormComponentOnChangeCallbackProps, @@ -62,6 +63,7 @@ import { } from '../../../../../../common/endpoint/service/artifacts'; import type { PolicyData } from '../../../../../../common/endpoint/types'; import { useTestIdGenerator } from '../../../../hooks/use_test_id_generator'; +import { useGetUpdatedTags } from '../../../../hooks/artifacts'; const testIdPrefix = 'blocklist-form'; @@ -113,7 +115,7 @@ function isValid(itemValidation: ItemValidation): boolean { // eslint-disable-next-line react/display-name export const BlockListForm = memo( - ({ item, policies, policiesIsLoading, onChange, mode }) => { + ({ item, policies, policiesIsLoading, onChange, mode, error: submitError }) => { const [nameVisited, setNameVisited] = useState(false); const [valueVisited, setValueVisited] = useState({ value: false }); // Use object to trigger re-render const warningsRef = useRef({ name: {}, value: {} }); @@ -123,6 +125,7 @@ export const BlockListForm = memo( const isGlobal = useMemo(() => isArtifactGlobal(item), [item]); const [wasByPolicy, setWasByPolicy] = useState(!isArtifactGlobal(item)); const [hasFormChanged, setHasFormChanged] = useState(false); + const { getTagsUpdatedBy } = useGetUpdatedTags(item); const showAssignmentSection = useMemo(() => { return ( @@ -546,8 +549,7 @@ export const BlockListForm = memo( const handleOnPolicyChange = useCallback( (change: EffectedPolicySelection) => { - const tags = getArtifactTagsByPolicySelection(change); - + const tags = getTagsUpdatedBy('policySelection', getArtifactTagsByPolicySelection(change)); const nextItem = { ...item, tags }; // Preserve old selected policies when switching to global @@ -561,11 +563,19 @@ export const BlockListForm = memo( }); setHasFormChanged(true); }, - [validateValues, onChange, item] + [getTagsUpdatedBy, item, validateValues, onChange] ); return ( - + + ) : undefined + } + isInvalid={!!submitError} + >

{DETAILS_HEADER}

 diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx index a1f66513c8f34..edfc84aabb008 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx @@ -31,6 +31,7 @@ import { FILTER_PROCESS_DESCENDANTS_TAG, GLOBAL_ARTIFACT_TAG, } from '../../../../../../common/endpoint/service/artifacts/constants'; +import type { IHttpFetchError } from '@kbn/core-http-browser'; jest.mock('../../../../../common/lib/kibana'); jest.mock('../../../../../common/containers/source'); @@ -365,6 +366,25 @@ describe('Event filter form', () => { // 'true' // ); }); + + it('should preserve other tags when updating artifact assignment', async () => { + formProps.item.tags = ['some:random_tag']; + render(); + const policyId = formProps.policies[0].id; + // move to per-policy and select the first + await userEvent.click( + renderResult.getByTestId('eventFilters-form-effectedPolicies-perPolicy') + ); + await userEvent.click(renderResult.getByTestId(`policy-${policyId}`)); + + expect(formProps.onChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + item: expect.objectContaining({ + tags: ['some:random_tag', 'policy:id-0'], + }), + }) + ); + }); }); describe('Policy section with downgraded license', () => { @@ -520,8 +540,8 @@ describe('Event filter form', () => { rerenderWithLatestProps(); await userEvent.click(renderResult.getByTestId(`${formPrefix}-effectedPolicies-global`)); expect(latestUpdatedItem.tags).toStrictEqual([ - GLOBAL_ARTIFACT_TAG, FILTER_PROCESS_DESCENDANTS_TAG, + GLOBAL_ARTIFACT_TAG, ]); rerenderWithLatestProps(); @@ -542,8 +562,8 @@ describe('Event filter form', () => { renderResult.getByTestId('eventFilters-form-effectedPolicies-perPolicy') ); expect(latestUpdatedItem.tags).toStrictEqual([ - ...perPolicyTags, FILTER_PROCESS_DESCENDANTS_TAG, + ...perPolicyTags, ]); }); @@ -884,5 +904,13 @@ describe('Event filter form', () => { ) ).not.toBeNull(); }); + + it('should display form submission errors', () => { + const message = 'oh oh - error'; + formProps.error = new Error(message) as IHttpFetchError; + const { getByTestId } = render(); + + expect(getByTestId('eventFilters-form-submitError').textContent).toMatch(message); + }); }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx index 783fda9a806b2..417de22fa879f 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx @@ -41,6 +41,7 @@ import { OperatingSystem } from '@kbn/securitysolution-utils'; import { getExceptionBuilderComponentLazy } from '@kbn/lists-plugin/public'; import type { OnChangeProps } from '@kbn/lists-plugin/public'; import type { ValueSuggestionsGetFn } from '@kbn/unified-search-plugin/public/autocomplete/providers/value_suggestion_provider'; +import { FormattedError } from '../../../../components/formatted_error'; import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features'; import { useGetUpdatedTags } from '../../../../hooks/artifacts'; import { @@ -48,11 +49,7 @@ import { PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY, PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY_TEXT, } from '../../../../../../common/endpoint/service/artifacts/constants'; -import { - isFilterProcessDescendantsEnabled, - isFilterProcessDescendantsTag, - isPolicySelectionTag, -} from '../../../../../../common/endpoint/service/artifacts/utils'; +import { isFilterProcessDescendantsEnabled } from '../../../../../../common/endpoint/service/artifacts/utils'; import { ENDPOINT_FIELDS_SEARCH_STRATEGY, eventsIndexPattern, @@ -101,12 +98,6 @@ const osOptions: Array> = OPERATING_SYSTEM inputDisplay: OS_TITLES[os], })); -// Defines the tag categories for Event Filters, using the given order. -const TAG_FILTERS = Object.freeze({ - policySelection: isPolicySelectionTag, - processDescendantsFiltering: isFilterProcessDescendantsTag, -}); - const getAddedFieldsCounts = (formFields: string[]): { [k: string]: number } => formFields.reduce<{ [k: string]: number }>((allFields, field) => { if (field in allFields) { @@ -148,613 +139,637 @@ type EventFilterItemEntries = Array<{ }>; export const EventFiltersForm: React.FC = - memo(({ allowSelectOs = true, item: exception, policies, policiesIsLoading, onChange, mode }) => { - const getTestId = useTestIdGenerator('eventFilters-form'); - const { http } = useKibana().services; - - const getSuggestionsFn = useCallback( - ({ field, query }) => { - const eventFiltersAPIClient = new EventFiltersApiClient(http); - return eventFiltersAPIClient.getSuggestions({ field: field.name, query }); - }, - [http] - ); - - const autocompleteSuggestions = useSuggestions(getSuggestionsFn); - const [hasFormChanged, setHasFormChanged] = useState(false); - const [hasNameError, toggleHasNameError] = useState(!exception.name); - const [newComment, setNewComment] = useState(''); - const [hasCommentError, setHasCommentError] = useState(false); - const [hasBeenInputNameVisited, setHasBeenInputNameVisited] = useState(false); - const [selectedPolicies, setSelectedPolicies] = useState([]); - const isPlatinumPlus = useLicense().isPlatinumPlus(); - const isGlobal = useMemo(() => isArtifactGlobal(exception), [exception]); - const [wasByPolicy, setWasByPolicy] = useState(!isArtifactGlobal(exception)); - const [hasDuplicateFields, setHasDuplicateFields] = useState(false); - const [hasWildcardWithWrongOperator, setHasWildcardWithWrongOperator] = useState( - hasWrongOperatorWithWildcard([exception]) - ); - - const [hasPartialCodeSignatureWarning, setHasPartialCodeSignatureWarning] = - useState(false); - - // This value has to be memoized to avoid infinite useEffect loop on useFetchIndex - const indexNames = useMemo(() => [eventsIndexPattern], []); - const [isIndexPatternLoading, { indexPatterns }] = useFetchIndex( - indexNames, - undefined, - ENDPOINT_FIELDS_SEARCH_STRATEGY - ); - const { getTagsUpdatedBy } = useGetUpdatedTags(exception, TAG_FILTERS); - const euiTheme = useEuiTheme(); - - const isFilterProcessDescendantsFeatureEnabled = useIsExperimentalFeatureEnabled( - 'filterProcessDescendantsForEventFiltersEnabled' - ); - - const isFilterProcessDescendantsSelected = useMemo( - () => isFilterProcessDescendantsEnabled(exception), - [exception] - ); - - const [areConditionsValid, setAreConditionsValid] = useState( - !!exception.entries.length || false - ); - // compute this for initial render only - const existingComments = useMemo( - () => (exception as ExceptionListItemSchema)?.comments, - // eslint-disable-next-line react-hooks/exhaustive-deps - [] - ); - - const showAssignmentSection = useMemo(() => { - return ( - isPlatinumPlus || - (mode === 'edit' && (!isGlobal || (wasByPolicy && isGlobal && hasFormChanged))) + memo( + ({ + allowSelectOs = true, + item: exception, + policies, + policiesIsLoading, + onChange, + mode, + error: submitError, + }) => { + const getTestId = useTestIdGenerator('eventFilters-form'); + const { http } = useKibana().services; + + const getSuggestionsFn = useCallback( + ({ field, query }) => { + const eventFiltersAPIClient = new EventFiltersApiClient(http); + return eventFiltersAPIClient.getSuggestions({ field: field.name, query }); + }, + [http] ); - }, [mode, isGlobal, hasFormChanged, isPlatinumPlus, wasByPolicy]); - const isFormValid = useMemo(() => { - // verify that it has legit entries - // and not just default entry without values - return ( - !hasNameError && - !hasCommentError && - !!exception.entries.length && - (exception.entries as EventFilterItemEntries).some((e) => e.value !== '' || e.value.length) + const autocompleteSuggestions = useSuggestions(getSuggestionsFn); + const [hasFormChanged, setHasFormChanged] = useState(false); + const [hasNameError, toggleHasNameError] = useState(!exception.name); + const [newComment, setNewComment] = useState(''); + const [hasCommentError, setHasCommentError] = useState(false); + const [hasBeenInputNameVisited, setHasBeenInputNameVisited] = useState(false); + const [selectedPolicies, setSelectedPolicies] = useState([]); + const isPlatinumPlus = useLicense().isPlatinumPlus(); + const isGlobal = useMemo(() => isArtifactGlobal(exception), [exception]); + const [wasByPolicy, setWasByPolicy] = useState(!isArtifactGlobal(exception)); + const [hasDuplicateFields, setHasDuplicateFields] = useState(false); + const [hasWildcardWithWrongOperator, setHasWildcardWithWrongOperator] = useState( + hasWrongOperatorWithWildcard([exception]) ); - }, [hasCommentError, hasNameError, exception.entries]); - - const processChanged = useCallback( - (updatedItem?: Partial) => { - const item = updatedItem - ? { - ...exception, - ...updatedItem, - } - : exception; - cleanupEntries(item); - onChange({ - item, - isValid: isFormValid && areConditionsValid, - confirmModalLabels: hasWildcardWithWrongOperator - ? CONFIRM_WARNING_MODAL_LABELS( - i18n.translate('xpack.securitySolution.eventFilter.flyoutForm.confirmModal.name', { - defaultMessage: 'event filter', - }) - ) - : undefined, - }); - }, - [areConditionsValid, exception, isFormValid, onChange, hasWildcardWithWrongOperator] - ); - - // set initial state of `wasByPolicy` that checks - // if the initial state of the exception was by policy or not - useEffect(() => { - if (!hasFormChanged && exception.tags) { - setWasByPolicy(!isArtifactGlobal({ tags: exception.tags })); - } - }, [exception.tags, hasFormChanged]); - - // select policies if editing - useEffect(() => { - if (hasFormChanged) return; - const policyIds = exception.tags ? getPolicyIdsFromArtifact({ tags: exception.tags }) : []; - - if (!policyIds.length) return; - const policiesData = policies.filter((policy) => policyIds.includes(policy.id)); - setSelectedPolicies(policiesData); - }, [hasFormChanged, exception, policies]); - - const eventFilterItem = useMemo(() => { - const ef: ArtifactFormComponentProps['item'] = exception; - ef.entries = exception.entries.length - ? (exception.entries as ExceptionListItemSchema['entries']) - : defaultConditionEntry(); - - // TODO: `id` gets added to the exception.entries item - // Is there a simpler way to this? - cleanupEntries(ef); - - setAreConditionsValid(!!exception.entries.length); - return ef; - }, [exception]); - - // name and handler - const handleOnChangeName = useCallback( - (event: React.ChangeEvent) => { - if (!exception) return; - const name = event.target.value.trim(); - toggleHasNameError(!name); - processChanged({ name }); - if (!hasFormChanged) setHasFormChanged(true); - }, - [exception, hasFormChanged, processChanged] - ); - - const nameInputMemo = useMemo( - () => ( - - !hasBeenInputNameVisited && setHasBeenInputNameVisited(true)} - /> - - ), - [getTestId, hasNameError, handleOnChangeName, hasBeenInputNameVisited, exception?.name] - ); - - // description and handler - const handleOnDescriptionChange = useCallback( - (event: React.ChangeEvent) => { - if (!exception) return; - if (!hasFormChanged) setHasFormChanged(true); - processChanged({ description: event.target.value.toString().trim() }); - }, - [exception, hasFormChanged, processChanged] - ); - const descriptionInputMemo = useMemo( - () => ( - - - - ), - [exception?.description, getTestId, handleOnDescriptionChange] - ); - - // selected OS and handler - const selectedOs = useMemo((): OperatingSystem => { - if (!exception?.os_types?.length) { - return OperatingSystem.WINDOWS; - } - return exception.os_types[0] as OperatingSystem; - }, [exception?.os_types]); - - const handleOnOsChange = useCallback( - (os: OperatingSystem) => { - if (!exception) return; - processChanged({ - os_types: [os], - entries: exception.entries, - }); - if (!hasFormChanged) setHasFormChanged(true); - }, - [exception, hasFormChanged, processChanged] - ); - - const osInputMemo = useMemo( - () => ( - - (false); + + // This value has to be memoized to avoid infinite useEffect loop on useFetchIndex + const indexNames = useMemo(() => [eventsIndexPattern], []); + const [isIndexPatternLoading, { indexPatterns }] = useFetchIndex( + indexNames, + undefined, + ENDPOINT_FIELDS_SEARCH_STRATEGY + ); + const { getTagsUpdatedBy } = useGetUpdatedTags(exception); + const euiTheme = useEuiTheme(); + + const isFilterProcessDescendantsFeatureEnabled = useIsExperimentalFeatureEnabled( + 'filterProcessDescendantsForEventFiltersEnabled' + ); + + const isFilterProcessDescendantsSelected = useMemo( + () => isFilterProcessDescendantsEnabled(exception), + [exception] + ); + + const [areConditionsValid, setAreConditionsValid] = useState( + !!exception.entries.length || false + ); + // compute this for initial render only + const existingComments = useMemo( + () => (exception as ExceptionListItemSchema)?.comments, + // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ); + + const showAssignmentSection = useMemo(() => { + return ( + isPlatinumPlus || + (mode === 'edit' && (!isGlobal || (wasByPolicy && isGlobal && hasFormChanged))) + ); + }, [mode, isGlobal, hasFormChanged, isPlatinumPlus, wasByPolicy]); + + const isFormValid = useMemo(() => { + // verify that it has legit entries + // and not just default entry without values + return ( + !hasNameError && + !hasCommentError && + !!exception.entries.length && + (exception.entries as EventFilterItemEntries).some( + (e) => e.value !== '' || e.value.length + ) + ); + }, [hasCommentError, hasNameError, exception.entries]); + + const processChanged = useCallback( + (updatedItem?: Partial) => { + const item = updatedItem + ? { + ...exception, + ...updatedItem, + } + : exception; + cleanupEntries(item); + onChange({ + item, + isValid: isFormValid && areConditionsValid, + confirmModalLabels: hasWildcardWithWrongOperator + ? CONFIRM_WARNING_MODAL_LABELS( + i18n.translate( + 'xpack.securitySolution.eventFilter.flyoutForm.confirmModal.name', + { + defaultMessage: 'event filter', + } + ) + ) + : undefined, + }); + }, + [areConditionsValid, exception, isFormValid, onChange, hasWildcardWithWrongOperator] + ); + + // set initial state of `wasByPolicy` that checks + // if the initial state of the exception was by policy or not + useEffect(() => { + if (!hasFormChanged && exception.tags) { + setWasByPolicy(!isArtifactGlobal({ tags: exception.tags })); + } + }, [exception.tags, hasFormChanged]); + + // select policies if editing + useEffect(() => { + if (hasFormChanged) return; + const policyIds = exception.tags ? getPolicyIdsFromArtifact({ tags: exception.tags }) : []; + + if (!policyIds.length) return; + const policiesData = policies.filter((policy) => policyIds.includes(policy.id)); + setSelectedPolicies(policiesData); + }, [hasFormChanged, exception, policies]); + + const eventFilterItem = useMemo(() => { + const ef: ArtifactFormComponentProps['item'] = exception; + ef.entries = exception.entries.length + ? (exception.entries as ExceptionListItemSchema['entries']) + : defaultConditionEntry(); + + // TODO: `id` gets added to the exception.entries item + // Is there a simpler way to this? + cleanupEntries(ef); + + setAreConditionsValid(!!exception.entries.length); + return ef; + }, [exception]); + + // name and handler + const handleOnChangeName = useCallback( + (event: React.ChangeEvent) => { + if (!exception) return; + const name = event.target.value.trim(); + toggleHasNameError(!name); + processChanged({ name }); + if (!hasFormChanged) setHasFormChanged(true); + }, + [exception, hasFormChanged, processChanged] + ); + + const nameInputMemo = useMemo( + () => ( + + !hasBeenInputNameVisited && setHasBeenInputNameVisited(true)} + /> + + ), + [getTestId, hasNameError, handleOnChangeName, hasBeenInputNameVisited, exception?.name] + ); + + // description and handler + const handleOnDescriptionChange = useCallback( + (event: React.ChangeEvent) => { + if (!exception) return; + if (!hasFormChanged) setHasFormChanged(true); + processChanged({ description: event.target.value.toString().trim() }); + }, + [exception, hasFormChanged, processChanged] + ); + const descriptionInputMemo = useMemo( + () => ( + + + + ), + [exception?.description, getTestId, handleOnDescriptionChange] + ); + + // selected OS and handler + const selectedOs = useMemo((): OperatingSystem => { + if (!exception?.os_types?.length) { + return OperatingSystem.WINDOWS; + } + return exception.os_types[0] as OperatingSystem; + }, [exception?.os_types]); + + const handleOnOsChange = useCallback( + (os: OperatingSystem) => { + if (!exception) return; + processChanged({ + os_types: [os], + entries: exception.entries, + }); + if (!hasFormChanged) setHasFormChanged(true); + }, + [exception, hasFormChanged, processChanged] + ); + + const osInputMemo = useMemo( + () => ( + + + + ), + [handleOnOsChange, selectedOs] + ); + + // comments and handler + const handleOnChangeComment = useCallback( + (value: string) => { + if (!exception) return; + setNewComment(value); + processChanged({ comments: [{ comment: value }] }); + if (!hasFormChanged) setHasFormChanged(true); + }, + [exception, hasFormChanged, processChanged] + ); + const commentsInputMemo = useMemo( + () => ( + - - ), - [handleOnOsChange, selectedOs] - ); - - // comments and handler - const handleOnChangeComment = useCallback( - (value: string) => { - if (!exception) return; - setNewComment(value); - processChanged({ comments: [{ comment: value }] }); - if (!hasFormChanged) setHasFormChanged(true); - }, - [exception, hasFormChanged, processChanged] - ); - const commentsInputMemo = useMemo( - () => ( - - ), - [existingComments, handleOnChangeComment, newComment] - ); - - // comments - const commentsSection = useMemo( - () => ( - <> - -

- -

- - - -

- -

- - - {commentsInputMemo} - - ), - [commentsInputMemo] - ); - - // details - const detailsSection = useMemo( - () => ( - <> - -

- -

- - - -

{ABOUT_EVENT_FILTERS}

- - - {nameInputMemo} - {descriptionInputMemo} - - ), - [nameInputMemo, descriptionInputMemo] - ); - - const handleFilterTypeOnChange = useCallback( - (id: string) => { - const newTagsForDescendants = id === 'descendants' ? [FILTER_PROCESS_DESCENDANTS_TAG] : []; - - const tags = getTagsUpdatedBy('processDescendantsFiltering', newTagsForDescendants); - - processChanged({ tags }); - if (!hasFormChanged) setHasFormChanged(true); - }, - [getTagsUpdatedBy, hasFormChanged, processChanged] - ); - - const filterTypeOptions: EuiButtonGroupOptionProps[] = useMemo(() => { - return [ - { - id: 'events', - label: ( + ), + [existingComments, handleOnChangeComment, newComment] + ); + + // comments + const commentsSection = useMemo( + () => ( + <> + +

+ +

+ + - +

+ +

- ), - iconType: isFilterProcessDescendantsSelected ? 'empty' : 'checkInCircleFilled', - 'data-test-subj': getTestId('filterEventsButton'), + + {commentsInputMemo} + + ), + [commentsInputMemo] + ); + + // details + const detailsSection = useMemo( + () => ( + <> + +

+ +

+ + + +

{ABOUT_EVENT_FILTERS}

+ + + {nameInputMemo} + {descriptionInputMemo} + + ), + [nameInputMemo, descriptionInputMemo] + ); + + const handleFilterTypeOnChange = useCallback( + (id: string) => { + const newTagsForDescendants = + id === 'descendants' ? [FILTER_PROCESS_DESCENDANTS_TAG] : []; + + const tags = getTagsUpdatedBy('processDescendantsFiltering', newTagsForDescendants); + + processChanged({ tags }); + if (!hasFormChanged) setHasFormChanged(true); }, - { - id: 'descendants', - label: ( - + [getTagsUpdatedBy, hasFormChanged, processChanged] + ); + + const filterTypeOptions: EuiButtonGroupOptionProps[] = useMemo(() => { + return [ + { + id: 'events', + label: ( - - - ), - iconType: isFilterProcessDescendantsSelected ? 'checkInCircleFilled' : 'empty', - 'data-test-subj': getTestId('filterProcessDescendantsButton'), - }, - ]; - }, [getTestId, isFilterProcessDescendantsSelected]); + ), + iconType: isFilterProcessDescendantsSelected ? 'empty' : 'checkInCircleFilled', + 'data-test-subj': getTestId('filterEventsButton'), + }, + { + id: 'descendants', + label: ( + + + + + + + ), + iconType: isFilterProcessDescendantsSelected ? 'checkInCircleFilled' : 'empty', + 'data-test-subj': getTestId('filterProcessDescendantsButton'), + }, + ]; + }, [getTestId, isFilterProcessDescendantsSelected]); + + const filterTypeSubsection = useMemo(() => { + if (!isFilterProcessDescendantsFeatureEnabled) return null; + + return ( + <> + + + + {isFilterProcessDescendantsSelected && ( + <> + + + + {PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY_TEXT} + + + )} + + ); + }, [ + isFilterProcessDescendantsFeatureEnabled, + handleFilterTypeOnChange, + euiTheme.euiTheme.size.l, + filterTypeOptions, + isFilterProcessDescendantsSelected, + getTestId, + ]); - const filterTypeSubsection = useMemo(() => { - if (!isFilterProcessDescendantsFeatureEnabled) return null; + // conditions and handler + const handleOnBuilderChange = useCallback( + (arg: OnChangeProps) => { + const hasDuplicates = + (!hasFormChanged && arg.exceptionItems[0] === undefined) || + isEqual(arg.exceptionItems[0]?.entries, exception?.entries); - return ( - <> - - + if (hasDuplicates) { + const addedFields = arg.exceptionItems[0]?.entries.map((e) => e.field) || ['']; - {isFilterProcessDescendantsSelected && ( - <> - + if (isFilterProcessDescendantsFeatureEnabled && isFilterProcessDescendantsSelected) { + addedFields.push(PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY.field); + } + + setHasDuplicateFields(computeHasDuplicateFields(getAddedFieldsCounts(addedFields))); + if (!hasFormChanged) setHasFormChanged(true); + return; + } else { + setHasDuplicateFields(false); + } + + // handle wildcard with wrong operator case + setHasWildcardWithWrongOperator(hasWrongOperatorWithWildcard(arg.exceptionItems)); + setHasPartialCodeSignatureWarning(hasPartialCodeSignatureEntry(arg.exceptionItems)); + + const updatedItem: Partial = + arg.exceptionItems[0] !== undefined + ? { + ...arg.exceptionItems[0], + name: exception?.name ?? '', + description: exception?.description ?? '', + comments: exception?.comments ?? [], + os_types: exception?.os_types ?? [OperatingSystem.WINDOWS], + tags: exception?.tags ?? [], + meta: exception.meta, + } + : { + ...exception, + entries: [{ field: '', operator: 'included', type: 'match', value: '' }], + }; + const hasValidConditions = + arg.exceptionItems[0] !== undefined + ? !(arg.errorExists && !arg.exceptionItems[0]?.entries?.length) + : false; + + setAreConditionsValid(hasValidConditions); + processChanged(updatedItem); + if (!hasFormChanged) setHasFormChanged(true); + }, + [ + exception, + hasFormChanged, + isFilterProcessDescendantsFeatureEnabled, + isFilterProcessDescendantsSelected, + processChanged, + ] + ); + const exceptionBuilderComponentMemo = useMemo( + () => + getExceptionBuilderComponentLazy({ + allowLargeValueLists: false, + httpService: http, + autocompleteService: autocompleteSuggestions, + exceptionListItems: [eventFilterItem as ExceptionListItemSchema], + listType: EVENT_FILTER_LIST_TYPE, + listId: ENDPOINT_EVENT_FILTERS_LIST_ID, + listNamespaceType: 'agnostic', + ruleName: RULE_NAME, + indexPatterns, + isOrDisabled: true, + isOrHidden: true, + isAndDisabled: false, + isNestedDisabled: false, + dataTestSubj: 'alert-exception-builder', + idAria: 'alert-exception-builder', + onChange: handleOnBuilderChange, + operatorsList: EVENT_FILTERS_OPERATORS, + osTypes: exception.os_types, + showValueListModal: ShowValueListModal, + }), + [ + autocompleteSuggestions, + handleOnBuilderChange, + http, + indexPatterns, + exception, + eventFilterItem, + ] + ); + + // conditions + const criteriaSection = useMemo( + () => ( + <> + +

- - {PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY_TEXT} - - - )} - +

+ + + +

+ {allowSelectOs ? ( + + ) : ( + + )} +

+ + + {allowSelectOs ? ( + <> + {osInputMemo} + + + ) : null} + {filterTypeSubsection} + {exceptionBuilderComponentMemo} + + ), + [allowSelectOs, exceptionBuilderComponentMemo, osInputMemo, filterTypeSubsection] ); - }, [ - isFilterProcessDescendantsFeatureEnabled, - handleFilterTypeOnChange, - euiTheme.euiTheme.size.l, - filterTypeOptions, - isFilterProcessDescendantsSelected, - getTestId, - ]); - - // conditions and handler - const handleOnBuilderChange = useCallback( - (arg: OnChangeProps) => { - const hasDuplicates = - (!hasFormChanged && arg.exceptionItems[0] === undefined) || - isEqual(arg.exceptionItems[0]?.entries, exception?.entries); - - if (hasDuplicates) { - const addedFields = arg.exceptionItems[0]?.entries.map((e) => e.field) || ['']; - - if (isFilterProcessDescendantsFeatureEnabled && isFilterProcessDescendantsSelected) { - addedFields.push(PROCESS_DESCENDANT_EVENT_FILTER_EXTRA_ENTRY.field); + + // policy and handler + const handleOnPolicyChange = useCallback( + (change: EffectedPolicySelection) => { + const policySelectionTags = getArtifactTagsByPolicySelection(change); + + // Preserve old selected policies when switching to global + if (!change.isGlobal) { + setSelectedPolicies(change.selected); } - setHasDuplicateFields(computeHasDuplicateFields(getAddedFieldsCounts(addedFields))); + const tags = getTagsUpdatedBy('policySelection', policySelectionTags); + processChanged({ tags }); if (!hasFormChanged) setHasFormChanged(true); - return; - } else { - setHasDuplicateFields(false); - } + }, + [getTagsUpdatedBy, processChanged, hasFormChanged] + ); - // handle wildcard with wrong operator case - setHasWildcardWithWrongOperator(hasWrongOperatorWithWildcard(arg.exceptionItems)); - setHasPartialCodeSignatureWarning(hasPartialCodeSignatureEntry(arg.exceptionItems)); + const policiesSection = useMemo( + () => ( + + ), + [ + selectedPolicies, + policies, + isGlobal, + policiesIsLoading, + isPlatinumPlus, + handleOnPolicyChange, + getTestId, + ] + ); - const updatedItem: Partial = - arg.exceptionItems[0] !== undefined - ? { - ...arg.exceptionItems[0], - name: exception?.name ?? '', - description: exception?.description ?? '', - comments: exception?.comments ?? [], - os_types: exception?.os_types ?? [OperatingSystem.WINDOWS], - tags: exception?.tags ?? [], - meta: exception.meta, - } - : { - ...exception, - entries: [{ field: '', operator: 'included', type: 'match', value: '' }], - }; - const hasValidConditions = - arg.exceptionItems[0] !== undefined - ? !(arg.errorExists && !arg.exceptionItems[0]?.entries?.length) - : false; - - setAreConditionsValid(hasValidConditions); - processChanged(updatedItem); - if (!hasFormChanged) setHasFormChanged(true); - }, - [ - exception, - hasFormChanged, - isFilterProcessDescendantsFeatureEnabled, - isFilterProcessDescendantsSelected, - processChanged, - ] - ); - const exceptionBuilderComponentMemo = useMemo( - () => - getExceptionBuilderComponentLazy({ - allowLargeValueLists: false, - httpService: http, - autocompleteService: autocompleteSuggestions, - exceptionListItems: [eventFilterItem as ExceptionListItemSchema], - listType: EVENT_FILTER_LIST_TYPE, - listId: ENDPOINT_EVENT_FILTERS_LIST_ID, - listNamespaceType: 'agnostic', - ruleName: RULE_NAME, - indexPatterns, - isOrDisabled: true, - isOrHidden: true, - isAndDisabled: false, - isNestedDisabled: false, - dataTestSubj: 'alert-exception-builder', - idAria: 'alert-exception-builder', - onChange: handleOnBuilderChange, - operatorsList: EVENT_FILTERS_OPERATORS, - osTypes: exception.os_types, - showValueListModal: ShowValueListModal, - }), - [ - autocompleteSuggestions, - handleOnBuilderChange, - http, - indexPatterns, - exception, - eventFilterItem, - ] - ); - - // conditions - const criteriaSection = useMemo( - () => ( - <> - -

- -

- - - -

- {allowSelectOs ? ( - - ) : ( + useEffect(() => { + processChanged(); + }, [processChanged]); + + if (isIndexPatternLoading || !exception) { + return ; + } + + return ( + + ) : undefined + } + isInvalid={!!submitError} + > + {detailsSection} + + {criteriaSection} + {hasWildcardWithWrongOperator && } + {hasPartialCodeSignatureWarning && } + {hasDuplicateFields && ( + <> + + - )} -

- - - {allowSelectOs ? ( + + + )} + {showAssignmentSection && ( <> - {osInputMemo} - + + {policiesSection} - ) : null} - {filterTypeSubsection} - {exceptionBuilderComponentMemo} - - ), - [allowSelectOs, exceptionBuilderComponentMemo, osInputMemo, filterTypeSubsection] - ); - - // policy and handler - const handleOnPolicyChange = useCallback( - (change: EffectedPolicySelection) => { - const policySelectionTags = getArtifactTagsByPolicySelection(change); - - // Preserve old selected policies when switching to global - if (!change.isGlobal) { - setSelectedPolicies(change.selected); - } - - const tags = getTagsUpdatedBy('policySelection', policySelectionTags); - processChanged({ tags }); - if (!hasFormChanged) setHasFormChanged(true); - }, - [processChanged, getTagsUpdatedBy, hasFormChanged] - ); - - const policiesSection = useMemo( - () => ( - - ), - [ - selectedPolicies, - policies, - isGlobal, - policiesIsLoading, - isPlatinumPlus, - handleOnPolicyChange, - getTestId, - ] - ); - - useEffect(() => { - processChanged(); - }, [processChanged]); - - if (isIndexPatternLoading || !exception) { - return ; + )} + + {commentsSection} + + ); } - - return ( - - {detailsSection} - - {criteriaSection} - {hasWildcardWithWrongOperator && } - {hasPartialCodeSignatureWarning && } - {hasDuplicateFields && ( - <> - - - - - - )} - {showAssignmentSection && ( - <> - - {policiesSection} - - )} - - {commentsSection} - - ); - }); + ); EventFiltersForm.displayName = 'EventFiltersForm'; diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/form.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/form.tsx index 62e0683c4c44e..902b5dab749fb 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/form.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/form.tsx @@ -41,6 +41,7 @@ import { } from './translations'; import type { ArtifactFormComponentProps } from '../../../../components/artifact_list_page'; import { FormattedError } from '../../../../components/formatted_error'; +import { useGetUpdatedTags } from '../../../../hooks/artifacts'; export const testIdPrefix = 'hostIsolationExceptions-form'; @@ -66,8 +67,8 @@ export const HostIsolationExceptionsForm = memo( const [hasBeenInputIpVisited, setHasBeenInputIpVisited] = useState(false); const [hasNameError, setHasNameError] = useState(!exception.name); const [hasIpError, setHasIpError] = useState(!ipEntry.value); - const getTestId = useTestIdGenerator(testIdPrefix); + const { getTagsUpdatedBy } = useGetUpdatedTags(exception); const [selectedPolicies, setSelectedPolicies] = useState({ isGlobal: isArtifactGlobal(exception), @@ -137,11 +138,14 @@ export const HostIsolationExceptionsForm = memo( setSelectedPolicies(selection); } - notifyOfChange({ - tags: getArtifactTagsByPolicySelection(selection), - }); + const tags = getTagsUpdatedBy( + 'policySelection', + getArtifactTagsByPolicySelection(selection) + ); + + notifyOfChange({ tags }); }, - [notifyOfChange] + [getTagsUpdatedBy, notifyOfChange] ); const handleOnDescriptionChange = useCallback( @@ -254,7 +258,14 @@ export const HostIsolationExceptionsForm = memo( return ( } + error={ + error && ( + + ) + } isInvalid={!!error} data-test-subj="hostIsolationExceptions-form" > diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/integration_tests/form.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/integration_tests/form.test.tsx index cea586a2ce496..c881dbaf88f25 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/integration_tests/form.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/host_isolation_exceptions/view/components/integration_tests/form.test.tsx @@ -25,7 +25,7 @@ import { isEffectedPolicySelected, } from '../../../../../components/effected_policy_select/test_utils'; import { BY_POLICY_ARTIFACT_TAG_PREFIX } from '../../../../../../../common/endpoint/service/artifacts'; -import type { HttpFetchOptionsWithPath } from '@kbn/core/public'; +import type { HttpFetchOptionsWithPath, IHttpFetchError } from '@kbn/core/public'; import { testIdPrefix } from '../form'; jest.mock('../../../../../../common/components/user_privileges'); @@ -295,5 +295,25 @@ describe('When on the host isolation exceptions entry form', () => { renderResult.queryByTestId(`${testIdPrefix}-effectedPolicies-policiesSelectable`) ).toBeTruthy(); }); + + // FIXME:PT not sure why this test is not working but I have spent several hours now on it and can't + // figure it out. Skipping for now and will try to come back to it. + it.skip('should display form submission errors', async () => { + const error = new Error('oh oh - error') as IHttpFetchError; + exceptionsApiMock.responseProvider.exceptionUpdate.mockImplementation(() => { + throw error; + }); + + const { getByTestId } = await render(); + await userEvent.click(getByTestId('hostIsolationExceptionsListPage-flyout-submitButton')); + + await waitFor(() => { + expect(exceptionsApiMock.responseProvider.exceptionUpdate).toHaveBeenCalled(); + }); + + expect(getByTestId('hostIsolationExceptions-form-submitError').textContent).toMatch( + 'oh oh - error' + ); + }); }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.test.tsx index a9cd7b9b89092..c67a3754ea21a 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.test.tsx @@ -26,6 +26,7 @@ import { forceHTMLElementOffsetWidth } from '../../../../components/effected_pol import type { PolicyData, TrustedAppConditionEntry } from '../../../../../../common/endpoint/types'; import { EndpointDocGenerator } from '../../../../../../common/endpoint/generate_data'; +import type { IHttpFetchError } from '@kbn/core-http-browser'; jest.mock('../../../../../common/hooks/use_license', () => { const licenseServiceInstance = { @@ -194,6 +195,14 @@ describe('Trusted apps form', () => { cleanup(); }); + it('should display form submission errors', () => { + const message = 'oh oh - failed'; + formProps.error = new Error(message) as IHttpFetchError; + render(); + + expect(renderResult.getByTestId(`${formPrefix}-submitError`).textContent).toMatch(message); + }); + describe('Details and Conditions', () => { beforeEach(() => render()); @@ -409,6 +418,22 @@ describe('Trusted apps form', () => { render(); expect(renderResult.queryByTestId('loading-spinner')).not.toBeNull(); }); + + it('should preserve other tags when policies are updated', async () => { + formProps.item.tags = ['some:unknown_tag']; + const policyId = formProps.policies[0].id; + render(); + await userEvent.click(renderResult.getByTestId(`${formPrefix}-effectedPolicies-perPolicy`)); + await userEvent.click(renderResult.getByTestId(`policy-${policyId}`)); + + expect(formProps.onChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + item: expect.objectContaining({ + tags: ['some:unknown_tag', `policy:${policyId}`], + }), + }) + ); + }); }); describe('the Policy Selection area when the license downgrades to gold or below', () => { diff --git a/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.tsx b/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.tsx index 8a9bd0e2dd840..cd4145191693b 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/trusted_apps/view/components/form.tsx @@ -29,6 +29,8 @@ import { OperatingSystem, } from '@kbn/securitysolution-utils'; import { WildCardWithWrongOperatorCallout } from '@kbn/securitysolution-exception-list-components'; +import { useGetUpdatedTags } from '../../../../hooks/artifacts'; +import { FormattedError } from '../../../../components/formatted_error'; import type { TrustedAppConditionEntry, NewTrustedApp, @@ -235,7 +237,7 @@ const defaultConditionEntry = (): TrustedAppConditionEntry( - ({ item, policies, policiesIsLoading, onChange, mode }) => { + ({ item, policies, policiesIsLoading, onChange, mode, error: submitError }) => { const getTestId = useTestIdGenerator('trustedApps-form'); const [visited, setVisited] = useState< Partial<{ @@ -248,6 +250,7 @@ export const TrustedAppsForm = memo( const isGlobal = useMemo(() => isArtifactGlobal(item), [item]); const [wasByPolicy, setWasByPolicy] = useState(!isArtifactGlobal(item)); const [hasFormChanged, setHasFormChanged] = useState(false); + const { getTagsUpdatedBy } = useGetUpdatedTags(item); useEffect(() => { if (!hasFormChanged && item.tags) { @@ -301,9 +304,9 @@ export const TrustedAppsForm = memo( const handleOnPolicyChange = useCallback( (change: EffectedPolicySelection) => { - const tags = getArtifactTagsByPolicySelection(change); - + const tags = getTagsUpdatedBy('policySelection', getArtifactTagsByPolicySelection(change)); const nextItem = { ...item, tags }; + // Preserve old selected policies when switching to global if (!change.isGlobal) { setSelectedPolicies(change.selected); @@ -311,7 +314,7 @@ export const TrustedAppsForm = memo( processChanged(nextItem); setHasFormChanged(true); }, - [item, processChanged] + [getTagsUpdatedBy, item, processChanged] ); const handleOnNameOrDescriptionChange = useCallback< @@ -480,7 +483,16 @@ export const TrustedAppsForm = memo( }, [item]); return ( - + + ) : undefined + } + isInvalid={!!submitError} + >

{DETAILS_HEADER}

 diff --git a/x-pack/solutions/security/plugins/security_solution/scripts/endpoint/common/role_and_user_loader.ts b/x-pack/solutions/security/plugins/security_solution/scripts/endpoint/common/role_and_user_loader.ts index eb493e22dafaf..7ad64641e0904 100644 --- a/x-pack/solutions/security/plugins/security_solution/scripts/endpoint/common/role_and_user_loader.ts +++ b/x-pack/solutions/security/plugins/security_solution/scripts/endpoint/common/role_and_user_loader.ts @@ -12,6 +12,8 @@ import type { Role } from '@kbn/security-plugin/common'; import type { ToolingLog } from '@kbn/tooling-log'; import { inspect } from 'util'; import type { AxiosError } from 'axios'; +import { cloneDeep } from 'lodash'; +import { dump } from './utils'; import type { EndpointSecurityRoleDefinitions } from './roles_users'; import { getAllEndpointSecurityRoles } from './roles_users'; import { catchAxiosErrorFormatAndThrow } from '../../../common/endpoint/format_axios_error'; @@ -71,7 +73,7 @@ export class RoleAndUserLoader = Record { + public async load(name: keyof R): Promise { const role = this.roles[name]; if (!role) { @@ -85,7 +87,7 @@ export class RoleAndUserLoader = Record> { + public async loadAll(): Promise> { const response = {} as Record; for (const [name, role] of Object.entries(this.roles)) { @@ -108,10 +110,42 @@ export class RoleAndUserLoader = Record { + const roleDeleteResponse = await this.kbnClient.request({ + method: 'DELETE', + path: `/api/security/role/${roleAndUserName}`, + headers: { ...COMMON_API_HEADERS }, + }); + + this.logger.info(`Deleted role ${roleAndUserName}`); + this.logger.verbose(dump(roleDeleteResponse)); + + const userDeleteResponse = await this.kbnClient.request({ + method: 'DELETE', + path: `/internal/security/users/${roleAndUserName}`, + headers: { ...COMMON_API_HEADERS }, + }); + + this.logger.info(`Deleted user ${roleAndUserName}`); + this.logger.verbose(dump(userDeleteResponse)); + } + + /** + * Get a copy of a predefined Role definition + * @param name + */ + public getPreDefinedRole(name: keyof R): Role { + return cloneDeep(this.roles[name]); + } + protected async createRole(role: Role): Promise { const { name: roleName, ...roleDefinition } = role; - this.logger.debug(`creating role:`, roleDefinition); + this.logger.debug(`creating role [${roleName}]:`, dump(roleDefinition, 10)); await this.kbnClient .request({ @@ -144,7 +178,7 @@ export class RoleAndUserLoader = Record DEFAULT_SPACE_ID), + getActiveSpace: jest.fn(async () => ({ + id: DEFAULT_SPACE_ID, + name: 'default', + disabledFeatures: [], + })), } as unknown as jest.Mocked; }; diff --git a/x-pack/solutions/security/plugins/security_solution/server/endpoint/services/actions/clients/lib/base_response_actions_client.ts b/x-pack/solutions/security/plugins/security_solution/server/endpoint/services/actions/clients/lib/base_response_actions_client.ts index 8973dd0c30a9d..6b09dff0b3eae 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/endpoint/services/actions/clients/lib/base_response_actions_client.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/endpoint/services/actions/clients/lib/base_response_actions_client.ts @@ -503,6 +503,8 @@ export abstract class ResponseActionsClientImpl implements ResponseActionsClient : {}), }; + this.log.debug(() => `creating action request document:\n${stringify(doc)}`); + try { const logsEndpointActionsResult = await this.options.esClient.index( { @@ -526,6 +528,9 @@ export abstract class ResponseActionsClientImpl implements ResponseActionsClient return doc; } catch (err) { this.sendActionCreationErrorTelemetry(actionRequest.command, err); + this.log.debug( + () => `attempt to index document into ${ENDPOINT_ACTIONS_INDEX} failed:\n${stringify(err)}` + ); if (!(err instanceof ResponseActionsClientError)) { throw new ResponseActionsClientError( diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_update_handler.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_update_handler.ts index 810b569ecf8a6..9db07c3d7e59f 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_update_handler.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_update_handler.ts @@ -76,7 +76,10 @@ export const getExceptionsPreUpdateItemHandler = ( endpointAppContextService, request ); - const validatedItem = await hostIsolationExceptionValidator.validatePreUpdateItem(data); + const validatedItem = await hostIsolationExceptionValidator.validatePreUpdateItem( + data, + currentSavedItem + ); hostIsolationExceptionValidator.notifyFeatureUsage( data as ExceptionItemLikeOptions, 'HOST_ISOLATION_EXCEPTION_BY_POLICY' @@ -105,7 +108,10 @@ export const getExceptionsPreUpdateItemHandler = ( endpointAppContextService, request ); - const validatedItem = await endpointExceptionValidator.validatePreUpdateItem(data); + const validatedItem = await endpointExceptionValidator.validatePreUpdateItem( + data, + currentSavedItem + ); endpointExceptionValidator.notifyFeatureUsage( data as ExceptionItemLikeOptions, 'ENDPOINT_EXCEPTIONS' diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.test.ts index 0f5da118ac2ae..cee2770d6e8bf 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.test.ts @@ -7,6 +7,7 @@ import { EndpointAppContextService } from '../../../endpoint/endpoint_app_context_services'; import { + createMockEndpointAppContextService, createMockEndpointAppContextServiceSetupContract, createMockEndpointAppContextServiceStartContract, } from '../../../endpoint/mocks'; @@ -22,6 +23,10 @@ import { GLOBAL_ARTIFACT_TAG, } from '../../../../common/endpoint/service/artifacts'; import { securityMock } from '@kbn/security-plugin/server/mocks'; +import { setArtifactOwnerSpaceId } from '../../../../common/endpoint/service/artifacts/utils'; +import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; +import { getEndpointAuthzInitialStateMock } from '../../../../common/endpoint/service/authz/mocks'; +import type { EndpointAuthz } from '../../../../common/endpoint/types/authz'; describe('When using Artifacts Exceptions BaseValidator', () => { let endpointAppContextServices: EndpointAppContextService; @@ -66,6 +71,11 @@ describe('When using Artifacts Exceptions BaseValidator', () => { }; }); + afterEach(() => { + // @ts-expect-error setting variable to undefined + validator = undefined; + }); + it('should use default endpoint authz (no access) when `request` is not provided', async () => { const baseValidator = new BaseValidatorMock(endpointAppContextServices); @@ -186,4 +196,100 @@ describe('When using Artifacts Exceptions BaseValidator', () => { false ); }); + + describe('with space awareness', () => { + const noGlobalArtifactManagementAuthzMessage = + 'EndpointArtifactError: Endpoint authorization failure. Management of "ownerSpaceId" tag requires global artifact management privilege'; + let authzMock: EndpointAuthz; + + beforeEach(() => { + authzMock = getEndpointAuthzInitialStateMock(); + endpointAppContextServices = createMockEndpointAppContextService(); + // @ts-expect-error updating a readonly field + endpointAppContextServices.experimentalFeatures.endpointManagementSpaceAwarenessEnabled = + true; + (endpointAppContextServices.getEndpointAuthz as jest.Mock).mockResolvedValue(authzMock); + setArtifactOwnerSpaceId(exceptionLikeItem, DEFAULT_SPACE_ID); + validator = new BaseValidatorMock(endpointAppContextServices, kibanaRequest); + }); + + describe('#validateCreateOnwerSpaceIds()', () => { + it('should error if adding an spaceOwnerId but has no global artifact management authz', async () => { + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + authzMock.canManageGlobalArtifacts = false; + + await expect(validator._validateCreateOwnerSpaceIds(exceptionLikeItem)).rejects.toThrow( + noGlobalArtifactManagementAuthzMessage + ); + }); + + it('should allow spaceOwnerId tag matching current space even if no global artifact management authz', async () => { + authzMock.canManageGlobalArtifacts = false; + + await expect( + validator._validateCreateOwnerSpaceIds(exceptionLikeItem) + ).resolves.toBeUndefined(); + }); + + it('should allow additional spaceOwnerId tags if user has global artifact management authz', async () => { + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + + await expect( + validator._validateCreateOwnerSpaceIds(exceptionLikeItem) + ).resolves.toBeUndefined(); + }); + + it('should not error if feature flag is disabled', async () => { + // @ts-expect-error updating a readonly field + endpointAppContextServices.experimentalFeatures.endpointManagementSpaceAwarenessEnabled = + false; + authzMock.canManageGlobalArtifacts = false; + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + setArtifactOwnerSpaceId(exceptionLikeItem, 'bar'); + + await expect( + validator._validateCreateOwnerSpaceIds(exceptionLikeItem) + ).resolves.toBeUndefined(); + }); + }); + + describe('#validateUpdateOnwerSpaceIds()', () => { + let savedExceptionLikeItem: ExceptionItemLikeOptions; + + beforeEach(() => { + savedExceptionLikeItem = createExceptionItemLikeOptionsMock(); + setArtifactOwnerSpaceId(exceptionLikeItem, DEFAULT_SPACE_ID); + }); + + it('should error if changing spaceOwnerId but has no global artifact management authz', async () => { + authzMock.canManageGlobalArtifacts = false; + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + + await expect( + validator._validateUpdateOwnerSpaceIds(exceptionLikeItem, savedExceptionLikeItem) + ).rejects.toThrow(noGlobalArtifactManagementAuthzMessage); + }); + + it('should allow changes to spaceOwnerId tags if user has global artifact management authz', async () => { + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + + await expect( + validator._validateUpdateOwnerSpaceIds(exceptionLikeItem, savedExceptionLikeItem) + ).resolves.toBeUndefined(); + }); + + it('should not error if feature flag is disabled', async () => { + // @ts-expect-error updating a readonly field + endpointAppContextServices.experimentalFeatures.endpointManagementSpaceAwarenessEnabled = + false; + authzMock.canManageGlobalArtifacts = false; + setArtifactOwnerSpaceId(exceptionLikeItem, 'foo'); + setArtifactOwnerSpaceId(exceptionLikeItem, 'bar'); + + await expect( + validator._validateUpdateOwnerSpaceIds(exceptionLikeItem, savedExceptionLikeItem) + ).resolves.toBeUndefined(); + }); + }); + }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.ts index dfcb222a3a675..f01436755a985 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/base_validator.ts @@ -11,7 +11,12 @@ import { isEqual } from 'lodash/fp'; import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import { OperatingSystem } from '@kbn/securitysolution-utils'; -import { setArtifactOwnerSpaceId } from '../../../../common/endpoint/service/artifacts/utils'; +import { i18n } from '@kbn/i18n'; +import { ENDPOINT_AUTHZ_ERROR_MESSAGE } from '../../../endpoint/errors'; +import { + getArtifactOwnerSpaceIds, + setArtifactOwnerSpaceId, +} from '../../../../common/endpoint/service/artifacts/utils'; import type { FeatureKeys } from '../../../endpoint/services'; import type { EndpointAuthz } from '../../../../common/endpoint/types/authz'; import type { EndpointAppContextService } from '../../../endpoint/endpoint_app_context_services'; @@ -24,6 +29,14 @@ import { import { EndpointArtifactExceptionValidationError } from './errors'; import { EndpointExceptionsValidationError } from './endpoint_exception_errors'; +const NO_GLOBAL_ARTIFACT_AUTHZ_MESSAGE = i18n.translate( + 'xpack.securitySolution.baseValidator.noGlobalArtifactAuthzApiMessage', + { + defaultMessage: + 'Management of "ownerSpaceId" tag requires global artifact management privilege', + } +); + export const BasicEndpointExceptionDataSchema = schema.object( { // must have a name @@ -87,7 +100,7 @@ export class BaseValidator { protected async validateHasPrivilege(privilege: keyof EndpointAuthz): Promise { if (!(await this.endpointAuthzPromise)[privilege]) { - throw new EndpointArtifactExceptionValidationError('Endpoint authorization failure', 403); + throw new EndpointArtifactExceptionValidationError(ENDPOINT_AUTHZ_ERROR_MESSAGE, 403); } } @@ -101,13 +114,13 @@ export class BaseValidator { protected async validateCanManageEndpointArtifacts(): Promise { if (!(await this.endpointAuthzPromise).canAccessEndpointManagement) { - throw new EndpointArtifactExceptionValidationError('Endpoint authorization failure', 403); + throw new EndpointArtifactExceptionValidationError(ENDPOINT_AUTHZ_ERROR_MESSAGE, 403); } } protected async validateCanIsolateHosts(): Promise { if (!(await this.endpointAuthzPromise).canIsolateHost) { - throw new EndpointArtifactExceptionValidationError('Endpoint authorization failure', 403); + throw new EndpointArtifactExceptionValidationError(ENDPOINT_AUTHZ_ERROR_MESSAGE, 403); } } @@ -198,6 +211,65 @@ export class BaseValidator { return false; } + protected async validateUpdateOwnerSpaceIds( + updatedItem: Partial>, + currentItem: Pick + ): Promise { + if ( + this.endpointAppContext.experimentalFeatures.endpointManagementSpaceAwarenessEnabled && + this.wasOwnerSpaceIdTagsChanged(updatedItem, currentItem) && + !(await this.endpointAuthzPromise).canManageGlobalArtifacts + ) { + throw new EndpointArtifactExceptionValidationError( + `Endpoint authorization failure. ${NO_GLOBAL_ARTIFACT_AUTHZ_MESSAGE}`, + 403 + ); + } + } + + protected async validateCreateOwnerSpaceIds(item: ExceptionItemLikeOptions): Promise { + if ( + this.endpointAppContext.experimentalFeatures.endpointManagementSpaceAwarenessEnabled && + item.tags && + item.tags.length > 0 + ) { + if ((await this.endpointAuthzPromise).canManageGlobalArtifacts) { + return; + } + + const ownerSpaceIds = getArtifactOwnerSpaceIds(item); + const activeSpaceId = await this.getActiveSpaceId(); + + if ( + ownerSpaceIds.length > 1 || + (ownerSpaceIds.length === 1 && ownerSpaceIds[0] !== activeSpaceId) + ) { + throw new EndpointArtifactExceptionValidationError( + `Endpoint authorization failure. ${NO_GLOBAL_ARTIFACT_AUTHZ_MESSAGE}`, + 403 + ); + } + } + } + + protected wasOwnerSpaceIdTagsChanged( + updatedItem: Partial>, + currentItem: Pick + ): boolean { + return !isEqual(getArtifactOwnerSpaceIds(updatedItem), getArtifactOwnerSpaceIds(currentItem)); + } + + protected async getActiveSpaceId(): Promise { + if (!this.request) { + throw new EndpointArtifactExceptionValidationError( + 'Unable to determine space id. Missing HTTP Request object', + 500 + ); + } + + return (await this.endpointAppContext.getActiveSpace(this.request)).id; + } + /** * Update the artifact item (if necessary) with a `ownerSpaceId` tag using the HTTP request's active space * @param item @@ -207,15 +279,7 @@ export class BaseValidator { item: Partial> ): Promise { if (this.endpointAppContext.experimentalFeatures.endpointManagementSpaceAwarenessEnabled) { - if (!this.request) { - throw new EndpointArtifactExceptionValidationError( - 'Unable to determine space id. Missing HTTP Request object', - 500 - ); - } - - const spaceId = (await this.endpointAppContext.getActiveSpace(this.request)).id; - setArtifactOwnerSpaceId(item, spaceId); + setArtifactOwnerSpaceId(item, await this.getActiveSpaceId()); } } } diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/blocklist_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/blocklist_validator.ts index a7fda50a2b6cc..107469be36a11 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/blocklist_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/blocklist_validator.ts @@ -243,6 +243,7 @@ export class BlocklistValidator extends BaseValidator { await this.validateBlocklistData(item); await this.validateCanCreateByPolicyArtifacts(item); await this.validateByPolicyItem(item); + await this.validateCreateOwnerSpaceIds(item); await this.setOwnerSpaceId(item); @@ -299,6 +300,7 @@ export class BlocklistValidator extends BaseValidator { } await this.validateByPolicyItem(updatedItem); + await this.validateUpdateOwnerSpaceIds(updatedItem, currentItem); if (!hasArtifactOwnerSpaceId(_updatedItem)) { await this.setOwnerSpaceId(_updatedItem); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/endpoint_exceptions_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/endpoint_exceptions_validator.ts index 292389253417e..5989bc06efb37 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/endpoint_exceptions_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/endpoint_exceptions_validator.ts @@ -10,6 +10,7 @@ import type { UpdateExceptionListItemOptions, } from '@kbn/lists-plugin/server'; import { ENDPOINT_LIST_ID } from '@kbn/securitysolution-list-constants'; +import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import { hasArtifactOwnerSpaceId } from '../../../../common/endpoint/service/artifacts/utils'; import { BaseValidator } from './base_validator'; @@ -28,14 +29,19 @@ export class EndpointExceptionsValidator extends BaseValidator { async validatePreCreateItem(item: CreateExceptionListItemOptions) { await this.validateHasWritePrivilege(); + await this.validateCreateOwnerSpaceIds(item); await this.setOwnerSpaceId(item); return item; } - async validatePreUpdateItem(item: UpdateExceptionListItemOptions) { + async validatePreUpdateItem( + item: UpdateExceptionListItemOptions, + currentItem: ExceptionListItemSchema + ) { await this.validateHasWritePrivilege(); + await this.validateUpdateOwnerSpaceIds(item, currentItem); if (!hasArtifactOwnerSpaceId(item)) { await this.setOwnerSpaceId(item); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/event_filter_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/event_filter_validator.ts index 8b67ffef51e20..3b1b4a44f74c8 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/event_filter_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/event_filter_validator.ts @@ -60,6 +60,8 @@ export class EventFilterValidator extends BaseValidator { await this.validateByPolicyItem(item); } + await this.validateCreateOwnerSpaceIds(item); + await this.setOwnerSpaceId(item); return item; @@ -86,6 +88,7 @@ export class EventFilterValidator extends BaseValidator { } await this.validateByPolicyItem(updatedItem); + await this.validateUpdateOwnerSpaceIds(_updatedItem, currentItem); if (!hasArtifactOwnerSpaceId(_updatedItem)) { await this.setOwnerSpaceId(_updatedItem); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/host_isolation_exceptions_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/host_isolation_exceptions_validator.ts index 7921da93dc20d..353a5eea00402 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/host_isolation_exceptions_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/host_isolation_exceptions_validator.ts @@ -12,6 +12,7 @@ import type { CreateExceptionListItemOptions, UpdateExceptionListItemOptions, } from '@kbn/lists-plugin/server'; +import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import { hasArtifactOwnerSpaceId } from '../../../../common/endpoint/service/artifacts/utils'; import { BaseValidator, BasicEndpointExceptionDataSchema } from './base_validator'; import { EndpointArtifactExceptionValidationError } from './errors'; @@ -79,6 +80,7 @@ export class HostIsolationExceptionsValidator extends BaseValidator { await this.validateHasWritePrivilege(); await this.validateHostIsolationData(item); await this.validateByPolicyItem(item); + await this.validateCreateOwnerSpaceIds(item); await this.setOwnerSpaceId(item); @@ -86,13 +88,15 @@ export class HostIsolationExceptionsValidator extends BaseValidator { } async validatePreUpdateItem( - _updatedItem: UpdateExceptionListItemOptions + _updatedItem: UpdateExceptionListItemOptions, + currentItem: ExceptionListItemSchema ): Promise { const updatedItem = _updatedItem as ExceptionItemLikeOptions; await this.validateHasWritePrivilege(); await this.validateHostIsolationData(updatedItem); await this.validateByPolicyItem(updatedItem); + await this.validateUpdateOwnerSpaceIds(_updatedItem, currentItem); if (!hasArtifactOwnerSpaceId(_updatedItem)) { await this.setOwnerSpaceId(_updatedItem); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/mocks.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/mocks.ts index 9f57a7945f4a3..dd77c78fcb3b8 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/mocks.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/mocks.ts @@ -45,6 +45,17 @@ export class BaseValidatorMock extends BaseValidator { ): boolean { return this.wasByPolicyEffectScopeChanged(updatedItem, currentItem); } + + _validateCreateOwnerSpaceIds(item: ExceptionItemLikeOptions): Promise { + return this.validateCreateOwnerSpaceIds(item); + } + + _validateUpdateOwnerSpaceIds( + updatedItem: Partial>, + currentItem: Pick + ): Promise { + return this.validateUpdateOwnerSpaceIds(updatedItem, currentItem); + } } export const createExceptionItemLikeOptionsMock = ( diff --git a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/trusted_app_validator.ts b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/trusted_app_validator.ts index da0b405a9ccb3..6492a22980fff 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/trusted_app_validator.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lists_integration/endpoint/validators/trusted_app_validator.ts @@ -207,6 +207,7 @@ export class TrustedAppValidator extends BaseValidator { await this.validateTrustedAppData(item); await this.validateCanCreateByPolicyArtifacts(item); await this.validateByPolicyItem(item); + await this.validateCreateOwnerSpaceIds(item); await this.setOwnerSpaceId(item); @@ -258,6 +259,7 @@ export class TrustedAppValidator extends BaseValidator { } await this.validateByPolicyItem(updatedItem); + await this.validateUpdateOwnerSpaceIds(_updatedItem, currentItem); if (!hasArtifactOwnerSpaceId(_updatedItem)) { await this.setOwnerSpaceId(_updatedItem); diff --git a/x-pack/test/security_solution_api_integration/config/services/security_solution_edr_workflows_roles_users.ts b/x-pack/test/security_solution_api_integration/config/services/security_solution_edr_workflows_roles_users.ts index a61d9b24e1a41..51a9c887a562b 100644 --- a/x-pack/test/security_solution_api_integration/config/services/security_solution_edr_workflows_roles_users.ts +++ b/x-pack/test/security_solution_api_integration/config/services/security_solution_edr_workflows_roles_users.ts @@ -12,6 +12,7 @@ import { getAllEndpointSecurityRoles, } from '@kbn/security-solution-plugin/scripts/endpoint/common/roles_users'; +import { EndpointSecurityTestRolesLoader } from '@kbn/security-solution-plugin/scripts/endpoint/common/role_and_user_loader'; import { FtrProviderContext } from '../../ftr_provider_context_edr_workflows'; export const ROLE = ENDPOINT_SECURITY_ROLE_NAMES; @@ -20,10 +21,17 @@ const rolesMapping = getAllEndpointSecurityRoles(); export function RolesUsersProvider({ getService }: FtrProviderContext) { const security = getService('security'); + const kbnServer = getService('kibanaServer'); + const log = getService('log'); + return { + /** Endpoint security test roles loader */ + loader: new EndpointSecurityTestRolesLoader(kbnServer, log), + /** * Creates an user with specific values * @param user + * @deprecated use `.loader.*` methods instead */ async createUser(user: { name: string; roles: string[]; password?: string }): Promise { const { name, roles, password } = user; @@ -33,6 +41,7 @@ export function RolesUsersProvider({ getService }: FtrProviderContext) { /** * Deletes specified users by username * @param names[] + * @deprecated use `.loader.*` methods instead */ async deleteUsers(names: string[]): Promise { for (const name of names) { @@ -43,6 +52,7 @@ export function RolesUsersProvider({ getService }: FtrProviderContext) { /** * Creates a role using predefined role config if defined or a custom one. It also allows define extra privileges. * @param options + * @deprecated use `.loader.*` methods instead */ async createRole(options: { predefinedRole?: EndpointSecurityRoleNames; @@ -87,6 +97,7 @@ export function RolesUsersProvider({ getService }: FtrProviderContext) { /** * Deletes specified roles by name * @param roles[] + * @deprecated use `.loader.*` methods instead */ async deleteRoles(roles: string[]): Promise { for (const role of roles) { diff --git a/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/artifacts.ts b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/artifacts.ts new file mode 100644 index 0000000000000..3856075caa3d6 --- /dev/null +++ b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/artifacts.ts @@ -0,0 +1,339 @@ +/* + * 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 TestAgent from 'supertest/lib/agent'; +import { ensureSpaceIdExists } from '@kbn/security-solution-plugin/scripts/endpoint/common/spaces'; +import { + ENDPOINT_ARTIFACT_LISTS, + EXCEPTION_LIST_ITEM_URL, +} from '@kbn/securitysolution-list-constants'; +import expect from '@kbn/expect'; +import { + buildPerPolicyTag, + buildSpaceOwnerIdTag, +} from '@kbn/security-solution-plugin/common/endpoint/service/artifacts/utils'; +import { addSpaceIdToPath } from '@kbn/spaces-plugin/common'; +import { exceptionItemToCreateExceptionItem } from '@kbn/security-solution-plugin/common/endpoint/data_generators/exceptions_list_item_generator'; +import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; +import { Role } from '@kbn/security-plugin-types-common'; +import { GLOBAL_ARTIFACT_TAG } from '@kbn/security-solution-plugin/common/endpoint/service/artifacts'; +import { PolicyTestResourceInfo } from '../../../../../security_solution_endpoint/services/endpoint_policy'; +import { createSupertestErrorLogger } from '../../utils'; +import { ArtifactTestData } from '../../../../../security_solution_endpoint/services/endpoint_artifacts'; +import { FtrProviderContext } from '../../../../ftr_provider_context_edr_workflows'; + +export default function ({ getService }: FtrProviderContext) { + const utils = getService('securitySolutionUtils'); + const rolesUsersProvider = getService('rolesUsersProvider'); + const endpointArtifactTestResources = getService('endpointArtifactTestResources'); + const policyTestResources = getService('endpointPolicyTestResources'); + const kbnServer = getService('kibanaServer'); + const log = getService('log'); + + // @skipInServerless: due to the fact that the serverless builtin roles are not yet updated with new privilege + // and tests below are currently creating a new role/user + describe('@ess @skipInServerless, @skipInServerlessMKI Endpoint Artifacts space awareness support', function () { + const spaceOneId = 'space_one'; + const spaceTwoId = 'space_two'; + + let artifactManagerRole: Role; + let globalArtifactManagerRole: Role; + let supertestArtifactManager: TestAgent; + let supertestGlobalArtifactManager: TestAgent; + let spaceOnePolicy: PolicyTestResourceInfo; + + before(async () => { + // For testing, we're using the `t3_analyst` role which already has All privileges + // to all artifacts and manipulating that role definition to create two new roles/users + artifactManagerRole = Object.assign( + rolesUsersProvider.loader.getPreDefinedRole('t3_analyst'), + { name: 'artifactManager' } + ); + + if (artifactManagerRole.kibana[0].feature.siemV2.includes('global_artifact_management_all')) { + artifactManagerRole.kibana[0].feature.siemV2 = + artifactManagerRole.kibana[0].feature.siemV2.filter( + (privilege) => privilege !== 'global_artifact_management_all' + ); + } + + globalArtifactManagerRole = Object.assign( + rolesUsersProvider.loader.getPreDefinedRole('t3_analyst'), + { name: 'globalArtifactManager' } + ); + + if ( + !globalArtifactManagerRole.kibana[0].feature.siemV2.includes( + 'global_artifact_management_all' + ) + ) { + globalArtifactManagerRole.kibana[0].feature.siemV2.push('global_artifact_management_all'); + } + + const [artifactManagerUser, globalArtifactManagerUser] = await Promise.all([ + rolesUsersProvider.loader.create(artifactManagerRole), + rolesUsersProvider.loader.create(globalArtifactManagerRole), + ]); + + supertestArtifactManager = await utils.createSuperTest( + artifactManagerUser.username, + artifactManagerUser.password + ); + supertestGlobalArtifactManager = await utils.createSuperTest( + globalArtifactManagerUser.username, + globalArtifactManagerUser.password + ); + + await Promise.all([ + ensureSpaceIdExists(kbnServer, spaceOneId, { log }), + ensureSpaceIdExists(kbnServer, spaceTwoId, { log }), + ]); + + spaceOnePolicy = await policyTestResources.createPolicy(); + }); + + // the endpoint uses data streams and es archiver does not support deleting them at the moment so we need + // to do it manually + after(async () => { + if (artifactManagerRole) { + await rolesUsersProvider.loader.delete(artifactManagerRole.name); + // @ts-expect-error + artifactManagerRole = undefined; + } + + if (globalArtifactManagerRole) { + await rolesUsersProvider.loader.delete(globalArtifactManagerRole.name); + // @ts-expect-error + globalArtifactManagerRole = undefined; + } + + if (spaceOnePolicy) { + await spaceOnePolicy.cleanup(); + // @ts-expect-error + spaceOnePolicy = undefined; + } + }); + + const artifactLists = Object.keys(ENDPOINT_ARTIFACT_LISTS); + + for (const artifactList of artifactLists) { + const listInfo = + ENDPOINT_ARTIFACT_LISTS[artifactList as keyof typeof ENDPOINT_ARTIFACT_LISTS]; + + describe(`for ${listInfo.name}`, () => { + let spaceOnePerPolicyArtifact: ArtifactTestData; + let spaceOneGlobalArtifact: ArtifactTestData; + + beforeEach(async () => { + spaceOnePerPolicyArtifact = await endpointArtifactTestResources.createArtifact( + listInfo.id, + { tags: [buildPerPolicyTag(spaceOnePolicy.packagePolicy.id)] }, + { supertest: supertestArtifactManager, spaceId: spaceOneId } + ); + + spaceOneGlobalArtifact = await endpointArtifactTestResources.createArtifact( + listInfo.id, + { tags: [GLOBAL_ARTIFACT_TAG] }, + { supertest: supertestGlobalArtifactManager, spaceId: spaceOneId } + ); + }); + + afterEach(async () => { + if (spaceOnePerPolicyArtifact) { + await spaceOnePerPolicyArtifact.cleanup(); + // @ts-expect-error assigning `undefined` + spaceOnePerPolicyArtifact = undefined; + } + }); + + it('should add owner space id when item is created', async () => { + expect(spaceOnePerPolicyArtifact.artifact.tags).to.include.string( + buildSpaceOwnerIdTag(spaceOneId) + ); + }); + + it('should not add owner space id during artifact update if one is already present', async () => { + const { body } = await supertestArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log)) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOnePerPolicyArtifact.artifact, + description: 'item was updated', + }) + ) + .expect(200); + + expect((body as ExceptionListItemSchema).tags).to.eql( + spaceOnePerPolicyArtifact.artifact.tags + ); + }); + + describe('and user does NOT have global artifact management privilege', () => { + it('should error when attempting to create artifact with additional owner space id tags', async () => { + await supertestArtifactManager + .post(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log).ignoreCodes([403])) + .send( + Object.assign( + exceptionItemToCreateExceptionItem({ + ...spaceOnePerPolicyArtifact.artifact, + tags: [buildSpaceOwnerIdTag('foo')], + }), + { item_id: undefined } + ) + ) + .expect(403); + }); + + it('should error when attempting to update artifact with different owner space id tags', async () => { + await supertestArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log).ignoreCodes([403])) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOnePerPolicyArtifact.artifact, + tags: [buildSpaceOwnerIdTag('foo')], + }) + ) + .expect(403); + }); + + // TODO:PT Un-skip in next PR. I got a little ahead of myself and added a test for the change that wil come with the next PR. + it.skip('should error if attempting to update a global artifact', async () => { + await supertestArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log).ignoreCodes([403])) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOneGlobalArtifact.artifact, + description: 'updating a global here', + }) + ) + .expect(403); + }); + + // TODO:PT Un-skip in next PR. I got a little ahead of myself and added a test for the change that wil come with the next PR. + it.skip('should error when attempting to change a global artifact to per-policy', async () => { + await supertestArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log).ignoreCodes([403])) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOneGlobalArtifact.artifact, + tags: spaceOneGlobalArtifact.artifact.tags + .filter((tag) => tag !== GLOBAL_ARTIFACT_TAG) + .concat(buildPerPolicyTag(spaceOnePolicy.packagePolicy.id)), + }) + ) + .expect(403); + }); + }); + + describe('and user has privilege to manage global artifacts', () => { + it('should allow creating artifact with additional owner space id tags', async () => { + const { body } = await supertestGlobalArtifactManager + .post(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log)) + .send( + Object.assign( + exceptionItemToCreateExceptionItem({ + ...spaceOnePerPolicyArtifact.artifact, + tags: [buildSpaceOwnerIdTag('foo')], + }), + { item_id: undefined } + ) + ) + .expect(200); + + expect((body as ExceptionListItemSchema).tags).to.eql([ + buildSpaceOwnerIdTag('foo'), + buildSpaceOwnerIdTag(spaceOneId), + ]); + }); + + it('should add owner space id when item is updated without having an owner tag', async () => { + const { body } = await supertestGlobalArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log)) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOnePerPolicyArtifact.artifact, + tags: [], + }) + ) + .expect(200); + + expect((body as ExceptionListItemSchema).tags).to.eql([ + buildSpaceOwnerIdTag(spaceOneId), + ]); + }); + + it('should allow creation of global artifacts', async () => { + // test is already covered by the fact that we created a global artifact for testing + expect(spaceOneGlobalArtifact.artifact).to.not.equal(undefined); + }); + + it('should allow updating of global artifacts', async () => { + await supertestGlobalArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log)) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOneGlobalArtifact.artifact, + description: 'updating of global artifacts', + }) + ) + .expect(200); + }); + + it('should allow converting global artifact to per-policy', async () => { + await supertestGlobalArtifactManager + .put(addSpaceIdToPath('/', spaceOneId, EXCEPTION_LIST_ITEM_URL)) + .set('elastic-api-version', '2023-10-31') + .set('x-elastic-internal-origin', 'kibana') + .set('kbn-xsrf', 'true') + .on('error', createSupertestErrorLogger(log)) + .send( + exceptionItemToCreateExceptionItem({ + ...spaceOneGlobalArtifact.artifact, + tags: spaceOneGlobalArtifact.artifact.tags + .filter((tag) => tag !== GLOBAL_ARTIFACT_TAG) + .concat(buildPerPolicyTag(spaceOnePolicy.packagePolicy.id)), + }) + ) + .expect(200); + }); + }); + }); + } + }); +} diff --git a/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/index.ts b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/index.ts index 729b88f25c578..0b0815530ffa8 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/index.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/index.ts @@ -57,5 +57,6 @@ export default function endpointAPIIntegrationTests(providerContext: FtrProvider }); loadTestFile(require.resolve('./space_awareness')); + loadTestFile(require.resolve('./artifacts')); }); } diff --git a/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/space_awareness.ts b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/space_awareness.ts index e9f4a1289d6fe..9c83451111f95 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/space_awareness.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/space_awareness.ts @@ -15,21 +15,12 @@ import { HOST_METADATA_GET_ROUTE, HOST_METADATA_LIST_ROUTE, } from '@kbn/security-solution-plugin/common/endpoint/constants'; -import { - ENDPOINT_ARTIFACT_LISTS, - EXCEPTION_LIST_ITEM_URL, -} from '@kbn/securitysolution-list-constants'; -import { buildSpaceOwnerIdTag } from '@kbn/security-solution-plugin/common/endpoint/service/artifacts/utils'; -import { exceptionItemToCreateExceptionItem } from '@kbn/security-solution-plugin/common/endpoint/data_generators/exceptions_list_item_generator'; -import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; -import { ArtifactTestData } from '../../../../../security_solution_endpoint/services/endpoint_artifacts'; import { createSupertestErrorLogger } from '../../utils'; import { FtrProviderContext } from '../../../../ftr_provider_context_edr_workflows'; export default function ({ getService }: FtrProviderContext) { const utils = getService('securitySolutionUtils'); const endpointTestresources = getService('endpointTestResources'); - const endpointArtifactTestResources = getService('endpointArtifactTestResources'); const kbnServer = getService('kibanaServer'); const log = getService('log'); @@ -195,78 +186,5 @@ export default function ({ getService }: FtrProviderContext) { expect(body.data[dataSpaceB.hosts[0].agent.id].found).to.eql(false); }); }); - - describe(`Artifact management (via Lists plugin)`, () => { - const artifactLists = Object.keys(ENDPOINT_ARTIFACT_LISTS); - - for (const artifactList of artifactLists) { - const listInfo = - ENDPOINT_ARTIFACT_LISTS[artifactList as keyof typeof ENDPOINT_ARTIFACT_LISTS]; - - describe(`for ${listInfo.name}`, () => { - let itemDataSpaceA: ArtifactTestData; - - beforeEach(async () => { - itemDataSpaceA = await endpointArtifactTestResources.createArtifact( - listInfo.id, - { tags: [] }, - { supertest: adminSupertest, spaceId: dataSpaceA.spaceId } - ); - }); - - afterEach(async () => { - if (itemDataSpaceA) { - await itemDataSpaceA.cleanup(); - // @ts-expect-error assigning `undefined` - itemDataSpaceA = undefined; - } - }); - - it('should add owner space id when item is created', async () => { - expect(itemDataSpaceA.artifact.tags).to.include.string( - buildSpaceOwnerIdTag(dataSpaceA.spaceId) - ); - }); - - it('should not add owner space id during artifact update if one is already present', async () => { - const { body } = await adminSupertest - .put(addSpaceIdToPath('/', dataSpaceA.spaceId, EXCEPTION_LIST_ITEM_URL)) - .set('elastic-api-version', '2023-10-31') - .set('x-elastic-internal-origin', 'kibana') - .set('kbn-xsrf', 'true') - .on('error', createSupertestErrorLogger(log)) - .send( - exceptionItemToCreateExceptionItem({ - ...itemDataSpaceA.artifact, - description: 'item was updated', - }) - ) - .expect(200); - - expect((body as ExceptionListItemSchema).tags).to.eql(itemDataSpaceA.artifact.tags); - }); - - it('should add owner space id when item is updated, if one is not present', async () => { - const { body } = await adminSupertest - .put(addSpaceIdToPath('/', dataSpaceA.spaceId, EXCEPTION_LIST_ITEM_URL)) - .set('elastic-api-version', '2023-10-31') - .set('x-elastic-internal-origin', 'kibana') - .set('kbn-xsrf', 'true') - .on('error', createSupertestErrorLogger(log)) - .send( - exceptionItemToCreateExceptionItem({ - ...itemDataSpaceA.artifact, - tags: [], - }) - ) - .expect(200); - - expect((body as ExceptionListItemSchema).tags).to.eql([ - buildSpaceOwnerIdTag(dataSpaceA.spaceId), - ]); - }); - }); - } - }); }); } diff --git a/x-pack/test/security_solution_api_integration/tsconfig.json b/x-pack/test/security_solution_api_integration/tsconfig.json index 02d2d42cd43a6..4d122d4adc29c 100644 --- a/x-pack/test/security_solution_api_integration/tsconfig.json +++ b/x-pack/test/security_solution_api_integration/tsconfig.json @@ -55,5 +55,6 @@ "@kbn/test-suites-src", "@kbn/openapi-common", "@kbn/scout-info", + "@kbn/security-plugin-types-common", ] }