[Security solution][Endpoint] Users can filter trusted apps by policy name #106710

dasansol92 · 2021-07-26T11:55:59Z

Summary

Adds dropdown that allows users filter trusted apps by policy.
It's displayed only when trustedAppsByPolicyEnabled FF is enabled.
Adds unit test and fixed existing ones.
Renames search component.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…w filtered by policy

…nly when refresh button is clicked

…cause now policies are loaded at the trusted apps list

…policy_name-747

elasticmachine · 2021-07-26T15:11:40Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/middleware.test.ts

...ck/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.tsx

…policy_name-747

joeypoon

just some nits but otherwise looks good.

joeypoon · 2021-07-27T15:14:08Z

x-pack/plugins/security_solution/public/management/common/routing.test.ts

      };

      expect(getTrustedAppsListPath(location)).toEqual(
-        '/administration/trusted_apps?page_index=2&page_size=20&view_type=list&show=create'
+        '/administration/trusted_apps?page_index=2&page_size=20&view_type=list&show=create&filter=test&included_policies=globally&excluded_policies=unassigned'


non-blocking nit: could use string templating here for location.filter and the location included/excluded policies.

joeypoon · 2021-07-27T20:12:33Z

...ck/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.tsx

  const doEntriesExist = useTrustedAppsSelector(entriesExist) === true;
-  const navigationCallback = useTrustedAppsNavigateCallback((query: string) => ({ filter: query }));
+  const didEntriesExist = useTrustedAppsSelector(prevEntriesExist) === true;


can we just do useTrustedAppsSelector(prevEntriesExist) without the === true?

OMG, totally right! Don't know why I write this, I think it came from a copy paste of line 58

dasansol92 · 2021-07-28T11:21:02Z

@elasticmachine merge upstream

…policy_name-747

paul-tavares

Left a few comments and suggestion. let me know if you have any questions. This (feature) is looking awesome

paul-tavares · 2021-07-29T16:21:30Z

x-pack/plugins/security_solution/public/management/common/utils.ts

@@ -19,3 +21,31 @@ export const parseQueryFilterToKQL = (filter: string, fields: Readonly<string[]>

  return `(${kuery})`;
 };
+
+const getPolicyQuery = (policy: string): string => {


policy (to me) makes it sound like its the actual endpoint policy. maybe rename it to policyId instead?

x-pack/plugins/security_solution/public/management/common/utils.ts

paul-tavares · 2021-07-29T16:27:27Z

...ugins/security_solution/public/management/components/policies_selector/policies_selector.tsx

+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [policies]);
+
+    const onButtonClick = () => {


can you wrap this in useCallback()? Same for the closePopover below as well.

Yeah, I thought that but following react docs: https://reactjs.org/docs/hooks-reference.html#usestate
It says:

React guarantees that setState function identity is stable and won’t change on re-renders. This is why it’s safe to omit from the useEffect or useCallback dependency list.

As this functions only has setState actions I think is one of those cases you don't need to wrap it in a useCallback as it wouldn't be better in terms on performance.

Note: I'm using the setState param function to avoid memoize the current value.

setIsPopoverOpen((prevIsPopoverOpen) => !prevIsPopoverOpen);

🤔 - true. the setter function provided back by useState() is guaranteed to now change and thus you can omit that setter function from the dependencies of useCallback.

One of the benefits of using useCallback() is that the returned function is always the same until the dependencies change. Because onButtonClick is given as a prop to another component, and because its value (the function) is a new function instance (because its not wrapped in useCallback()), that will trigger a re-render of the downstream component (prop changed).

☝️ ( 😁 ) - still optional

Oh right, I'm still wondering if it will improve something as there is no changing state/prop related? So the returned function still being the same right?

No, every time this component is rendered, the onButtonClick will be a new instance (function) because it's not cached. So when that is used below as a prop value, it will cause that comment below to also re-render (because the prop value is new (oldOnButtonClick !== onButtonClick)

Argh! I see, got it. Thanks!!

Done in: 6e92a6b

paul-tavares · 2021-07-29T16:30:48Z

...ugins/security_solution/public/management/components/policies_selector/policies_selector.tsx

+  onChangeSelection: (items: Item[]) => void;
+}
+
+export interface Item {


Naming suggestion: PolicySelectionItem (just because Item is so generic)

Totally agree, this was cp from eui docs

paul-tavares · 2021-07-29T16:31:37Z

.../security_solution/public/management/components/policies_selector/policies_selector.test.tsx

+  const policy = generator.generatePolicyPackagePolicy();
+  policy.name = 'test policy A';
+  policy.id = 'abc123';
+  const getElement = (params: Partial<PoliciesSelectorProps>) => {


Could you set these on every test run so that we avoid mutable state being used between tests?

paul-tavares · 2021-07-29T16:42:35Z

x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/mocks.ts

+
+export const getPolicyResponse = (): GetPolicyListResponse => ({
+  items: [
+    {


Do we have a generator for policy response? if so, I would suggest inializing a generator here with a seed and then getting the mocked policy response with it, rather than to define the entire response here.

x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/selectors.ts

paul-tavares · 2021-07-29T16:52:00Z

...ugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.test.tsx

@@ -922,6 +936,27 @@ describe('When on the Trusted Apps Page', () => {
          backButtonUrl: '/fleet',
        });
      });
+
+      const priorMockImplementation = coreStart.http.get.getMockImplementation();
+      // @ts-ignore


why the ts-ignore? is it due to the callback overload signatures? If so, I have gotten around this by just accepting options as the argument and doing a type assertion in the body of the function. see x-pack/plugins/security_solution/public/common/mock/endpoint/http_handler_mock_factory.ts:199 for reference

Changing this by using the getLastLoadedResourceState solves that

Sorry I don't remember why I said that. This was copy&pasted from line 766.

...ck/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.tsx

…ourceState helper function to get the prev state. Use generated data for policies in tests

…47' of github.com:dasansol92/kibana into feat/olm-users_can_filter_Trusted_Apps_by_policy_name-747

…policy_name-747

paul-tavares

Approving 🔥 🔥 🔥

I did leave a few comments that might need follow up, but nothing that should hold this up for now.

dasansol92 · 2021-08-02T08:32:53Z

@elasticmachine merge upstream

…policy_name-747

dasansol92 · 2021-08-02T12:15:04Z

@elasticmachine merge upstream

…policy_name-747

dasansol92 · 2021-08-04T12:32:05Z

@elasticmachine merge upstream

…policy_name-747

paul-tavares

🔥 🔥 🔥

🚢

x-pack/plugins/security_solution/public/management/common/utils.ts

paul-tavares · 2021-08-04T13:45:00Z

...ugins/security_solution/public/management/components/policies_selector/policies_selector.tsx

+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [policies]);
+
+    const onButtonClick = () => {


☝️ ( 😁 ) - still optional

paul-tavares · 2021-08-04T13:46:57Z

...ugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.test.tsx

@@ -922,6 +936,27 @@ describe('When on the Trusted Apps Page', () => {
          backButtonUrl: '/fleet',
        });
      });
+
+      const priorMockImplementation = coreStart.http.get.getMockImplementation();
+      // @ts-ignore


kibanamachine · 2021-08-04T16:17:17Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2342	2345	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	6.4MB	6.4MB	+10.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	211.9KB	211.9KB	+34.0B

History

💚 Build #142925 succeeded 5c74842
💚 Build #142872 succeeded f43b67c
💔 Build #142656 failed e71271a
💔 Build #142617 failed c5919de
💔 Build #142260 failed 53b9315

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-08-05T13:36:08Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

… name (elastic#106710) * Allow users select policies from a dropdown * Policy filters are passed throguh the API call and the results are now filtered by policy * Moved policies selector inside search component and triggers search only when refresh button is clicked * Fixes tests * Triggers policy filter when policy is selected. Also fix unit test because now policies are loaded at the trusted apps list * Renamed components and added an index.ts for the exports * Adds unit tests for policies selector component * Fix unit tests and changed camelcase by snack case for url params * adds multilang * Fixes i18n keys * Move mock resonse to the mocks file * Use string templating in test * remove === true from boolean comparison * Set function in useCallback. Renames some variables and types. Use reourceState helper function to get the prev state. Use generated data for policies in tests * Fix ts errors * Removes unused type and fix type name for Item * Puts exclude clause on policy dropdown behind a feature flag * Adds missing feature flags in some tests and in global reducer * Fix test adding useExperimentalValua mock for FF * Wrapp handlers in a useCallback in order to prevent useless rerenders Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

… name (#106710) (#107750) * Allow users select policies from a dropdown * Policy filters are passed throguh the API call and the results are now filtered by policy * Moved policies selector inside search component and triggers search only when refresh button is clicked * Fixes tests * Triggers policy filter when policy is selected. Also fix unit test because now policies are loaded at the trusted apps list * Renamed components and added an index.ts for the exports * Adds unit tests for policies selector component * Fix unit tests and changed camelcase by snack case for url params * adds multilang * Fixes i18n keys * Move mock resonse to the mocks file * Use string templating in test * remove === true from boolean comparison * Set function in useCallback. Renames some variables and types. Use reourceState helper function to get the prev state. Use generated data for policies in tests * Fix ts errors * Removes unused type and fix type name for Item * Puts exclude clause on policy dropdown behind a feature flag * Adds missing feature flags in some tests and in global reducer * Fix test adding useExperimentalValua mock for FF * Wrapp handlers in a useCallback in order to prevent useless rerenders Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: David Sánchez <davidsansol92@gmail.com>

… name (elastic#106710) * Allow users select policies from a dropdown * Policy filters are passed throguh the API call and the results are now filtered by policy * Moved policies selector inside search component and triggers search only when refresh button is clicked * Fixes tests * Triggers policy filter when policy is selected. Also fix unit test because now policies are loaded at the trusted apps list * Renamed components and added an index.ts for the exports * Adds unit tests for policies selector component * Fix unit tests and changed camelcase by snack case for url params * adds multilang * Fixes i18n keys * Move mock resonse to the mocks file * Use string templating in test * remove === true from boolean comparison * Set function in useCallback. Renames some variables and types. Use reourceState helper function to get the prev state. Use generated data for policies in tests * Fix ts errors * Removes unused type and fix type name for Item * Puts exclude clause on policy dropdown behind a feature flag * Adds missing feature flags in some tests and in global reducer * Fix test adding useExperimentalValua mock for FF * Wrapp handlers in a useCallback in order to prevent useless rerenders Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

dasansol92 added 10 commits July 20, 2021 16:10

Allow users select policies from a dropdown

56d93dd

Policy filters are passed throguh the API call and the results are no…

fb6dddd

…w filtered by policy

Moved policies selector inside search component and triggers search o…

68ccb21

…nly when refresh button is clicked

Fixes tests

fe46136

Triggers policy filter when policy is selected. Also fix unit test be…

707ab2b

…cause now policies are loaded at the trusted apps list

Renamed components and added an index.ts for the exports

f7e609b

Adds unit tests for policies selector component

1914f53

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

6c862ce

…policy_name-747

Fix unit tests and changed camelcase by snack case for url params

4c52330

adds multilang

f984f85

dasansol92 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution auto-backport Deprecated - use backport:version if exact versions are needed v7.15.0 labels Jul 26, 2021

Fixes i18n keys

3a90eb5

dasansol92 marked this pull request as ready for review July 26, 2021 15:11

dasansol92 requested a review from a team as a code owner July 26, 2021 15:11

dasansol92 requested review from joeypoon and parkiino July 26, 2021 15:11

dasansol92 commented Jul 27, 2021

View reviewed changes

x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/middleware.test.ts Outdated Show resolved Hide resolved

dasansol92 commented Jul 27, 2021

View reviewed changes

...ck/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.tsx Show resolved Hide resolved

dasansol92 added 2 commits July 27, 2021 10:20

Move mock resonse to the mocks file

f6a7c91

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

010fe3d

…policy_name-747

joeypoon approved these changes Jul 27, 2021

View reviewed changes

dasansol92 added 2 commits July 28, 2021 10:05

Use string templating in test

c27d7da

remove === true from boolean comparison

4bacb68

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

43ffeb0

…policy_name-747

paul-tavares reviewed Jul 29, 2021

View reviewed changes

dasansol92 added 5 commits July 30, 2021 12:52

Set function in useCallback. Renames some variables and types. Use re…

90ad245

…ourceState helper function to get the prev state. Use generated data for policies in tests

Merge branch 'feat/olm-users_can_filter_Trusted_Apps_by_policy_name-7…

309f68f

…47' of github.com:dasansol92/kibana into feat/olm-users_can_filter_Trusted_Apps_by_policy_name-747

Fix ts errors

29636c8

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

08e9e8e

…policy_name-747

Removes unused type and fix type name for Item

0a7f265

paul-tavares approved these changes Jul 30, 2021

View reviewed changes

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

54cc44c

…policy_name-747

kibanamachine and others added 2 commits August 2, 2021 08:15

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

53b9315

…policy_name-747

Puts exclude clause on policy dropdown behind a feature flag

c5919de

dasansol92 requested a review from a team as a code owner August 3, 2021 13:06

dasansol92 requested a review from paul-tavares August 3, 2021 13:06

dasansol92 added 2 commits August 3, 2021 16:20

Adds missing feature flags in some tests and in global reducer

e71271a

Fix test adding useExperimentalValua mock for FF

f43b67c

Merge branch 'master' into feat/olm-users_can_filter_Trusted_Apps_by_…

5c74842

…policy_name-747

paul-tavares approved these changes Aug 4, 2021

View reviewed changes

Wrapp handlers in a useCallback in order to prevent useless rerenders

6e92a6b

dasansol92 merged commit d56b22f into elastic:master Aug 5, 2021

kibanamachine mentioned this pull request Aug 5, 2021

[7.x] [Security solution][Endpoint] Users can filter trusted apps by policy name (#106710) #107750

Merged

dasansol92 mentioned this pull request Aug 12, 2021

[Security solution] 'Add Trusted Application' button blink when we search trusted application with non existing value. #108363

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security solution][Endpoint] Users can filter trusted apps by policy name #106710

[Security solution][Endpoint] Users can filter trusted apps by policy name #106710

dasansol92 commented Jul 26, 2021 •

edited

Loading

elasticmachine commented Jul 26, 2021

joeypoon left a comment

joeypoon Jul 27, 2021

joeypoon Jul 27, 2021

dasansol92 Jul 28, 2021

dasansol92 commented Jul 28, 2021

paul-tavares left a comment

paul-tavares Jul 29, 2021

paul-tavares Jul 29, 2021

dasansol92 Jul 30, 2021

paul-tavares Jul 30, 2021

paul-tavares Aug 4, 2021

dasansol92 Aug 4, 2021

paul-tavares Aug 4, 2021

dasansol92 Aug 4, 2021

dasansol92 Aug 4, 2021

paul-tavares Jul 29, 2021

dasansol92 Jul 30, 2021

paul-tavares Jul 29, 2021

paul-tavares Jul 29, 2021

paul-tavares Jul 29, 2021

dasansol92 Jul 30, 2021

paul-tavares Aug 4, 2021

dasansol92 Aug 4, 2021

paul-tavares left a comment

dasansol92 commented Aug 2, 2021

dasansol92 commented Aug 2, 2021

dasansol92 commented Aug 4, 2021

paul-tavares left a comment

paul-tavares Aug 4, 2021

paul-tavares Aug 4, 2021

kibanamachine commented Aug 4, 2021

kibanamachine commented Aug 5, 2021

[Security solution][Endpoint] Users can filter trusted apps by policy name #106710

[Security solution][Endpoint] Users can filter trusted apps by policy name #106710

Conversation

dasansol92 commented Jul 26, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jul 26, 2021

joeypoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasansol92 commented Jul 28, 2021

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

dasansol92 commented Aug 2, 2021

dasansol92 commented Aug 2, 2021

dasansol92 commented Aug 4, 2021

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Aug 4, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

kibanamachine commented Aug 5, 2021

💚 Backport successful

dasansol92 commented Jul 26, 2021 •

edited

Loading