Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Endpoint] Creates generic policy tab artifact component to be used for all of our artifacts #126685

Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5f8acfd
Initial commit adding a generic component for policy artifact tabs
dasansol92 Mar 1, 2022
d8ef30d
Adds labels and translations. Also fixes loading state using isRefetc…
dasansol92 Mar 2, 2022
fc1c6f2
Fix checks and wrong count update when removing/adding items
dasansol92 Mar 2, 2022
399e009
Fixes translations and adds extra privileges checks
dasansol92 Mar 3, 2022
474b3e8
Merge branch 'main' into feat/olm-create_generic_policy_tab_artifact_…
dasansol92 Mar 3, 2022
f565083
Use hook to retrieve url params instead of redux
dasansol92 Mar 3, 2022
bfb832c
Fixes wrong loading state in flyout
dasansol92 Mar 3, 2022
1e228a6
Extracts conditional artifact logic from generic components and adds …
dasansol92 Mar 4, 2022
39ee3b8
Include new changes in policy tabs component
dasansol92 Mar 4, 2022
fced0a2
Adds policy artifact flyout unit test
dasansol92 Mar 4, 2022
9f1fe11
Adds policy artifacts list unit test
dasansol92 Mar 4, 2022
a130ea1
Adds policy artifacts delete modal unit test
dasansol92 Mar 4, 2022
b575edd
Adds external privileges checks on unit tests
dasansol92 Mar 4, 2022
5320a7a
Uses FormattedMessage to include EuiLink inside the translation
dasansol92 Mar 4, 2022
c69b0e3
Uses FormattedMessage
dasansol92 Mar 4, 2022
4211893
Generate new ExceptionsListApiClient instance when http changes
dasansol92 Mar 4, 2022
c4b01a7
Removes existing policy tab artifacts code in favor of generic component
dasansol92 Mar 7, 2022
0fc7869
Merge branch 'main' into feat/olm-create_generic_policy_tab_artifact_…
dasansol92 Mar 7, 2022
8ff085a
Update translation files
dasansol92 Mar 7, 2022
1a5bb51
Merge branch 'main' into feat/olm-create_generic_policy_tab_artifact_…
dasansol92 Mar 9, 2022
d99ba65
Include pr suggestions
dasansol92 Mar 9, 2022
92ee2c3
Uses useUrlPagination hook for pagination
dasansol92 Mar 9, 2022
cbc8ef9
Fix checks
dasansol92 Mar 9, 2022
ed6c258
Fixes uni test
dasansol92 Mar 9, 2022
63f8f9a
Reorder use_list_artifact hook params and move custom getInstance fun…
dasansol92 Mar 9, 2022
c5e2545
Fix typos and added pr feedback
dasansol92 Mar 9, 2022
999a339
Merge branch 'main' into feat/olm-create_generic_policy_tab_artifact_…
kibanamachine Mar 9, 2022
f1bd38f
Fix typo in searchableFields props and vars
dasansol92 Mar 10, 2022
ef61835
Merge branch 'feat/olm-create_generic_policy_tab_artifact_component_t…
dasansol92 Mar 10, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,39 @@ import { MANAGEMENT_DEFAULT_PAGE, MANAGEMENT_DEFAULT_PAGE_SIZE } from '../../com
import { parsePoliciesAndFilterToKql, parseQueryFilterToKQL } from '../../common/utils';
import { ExceptionsListApiClient } from '../../services/exceptions_list/exceptions_list_api_client';

const DEFAULT_OPTIONS = Object.freeze({});

export function useListArtifact(
exceptionListApiClient: ExceptionsListApiClient,
searcheableFields: string[],
options: {
filter: string;
page: number;
perPage: number;
policies: string[];
filter?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooooo... I changed this hook in my PR as well and:

  • Moved the order of the input props around to: apiClient, options, searchableFields and then query options.
  • made searchableFields optional -defaults to the DEFAULT_EXCEPTION_LIST_ITEM_SEARCHABLE_FIELDS
  • made options be a Partial<> instead of forcing the user to define every attribute. Looks like this in mine:
export function useListArtifact(
  exceptionListApiClient: ExceptionsListApiClient,
  options: Partial<{
    filter: string;
    page: number;
    perPage: number;
    policies: string[];
  }> = {},
  searchableFields: MaybeImmutable<string[]> = DEFAULT_EXCEPTION_LIST_ITEM_SEARCHABLE_FIELDS,
  customQueryOptions: UseQueryOptions<FoundExceptionListItemSchema, HttpFetchError> = {}
): QueryObserverResult<FoundExceptionListItemSchema, HttpFetchError> {
  const {
    filter = '',
    page = MANAGEMENT_DEFAULT_PAGE + 1,
    perPage = MANAGEMENT_DEFAULT_PAGE_SIZE,
    policies,
  } = options;

I made these changes in order to keep the hook's primary responsibility (to get a list of data out) simple and down to a minimal set of input args needed to use it.

Ref: https://github.com/elastic/kibana/pull/126400/files#diff-01c4c157be58054f3b6dc1977f5bba98738cc8461f4f6d15b97710b3f4b12759R17

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done: 63f8f9a

page?: number;
perPage?: number;
policies?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should also support a string for policies so that it is easier to just pass in the URL param value. that being said, once I work on a better URL param parser hook, this might be a non-issue as that "parser" would likely take care of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this hook level I think is better to have an array and not a string. This can avoid us having wrong strings wit wrong structures coming from url. Agree this can change once we have a policy hook for parsing

excludedPolicies?: string[];
} = {
filter: '',
page: MANAGEMENT_DEFAULT_PAGE,
page: MANAGEMENT_DEFAULT_PAGE + 1,
perPage: MANAGEMENT_DEFAULT_PAGE_SIZE,
policies: [],
excludedPolicies: [],
},
customQueryOptions: UseQueryOptions<FoundExceptionListItemSchema, HttpFetchError>
customQueryOptions: UseQueryOptions<
FoundExceptionListItemSchema,
HttpFetchError
> = DEFAULT_OPTIONS,
customQueryIds: string[] = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are introducing this just so that you can invalidate queries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly!

): QueryObserverResult<FoundExceptionListItemSchema, HttpFetchError> {
const { filter, page, perPage, policies } = options;
const { filter, page, perPage, policies, excludedPolicies } = options;

return useQuery<FoundExceptionListItemSchema, HttpFetchError>(
['list', exceptionListApiClient, options],
[...customQueryIds, 'list', exceptionListApiClient, options],
() => {
return exceptionListApiClient.find({
filter: parsePoliciesAndFilterToKql({
policies,
excludedPolicies,
kuery: parseQueryFilterToKQL(filter, searcheableFields),
}),
perPage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { QueryObserverResult, useQuery, UseQueryOptions } from 'react-query';
import { parsePoliciesAndFilterToKql, parseQueryFilterToKQL } from '../../common/utils';
import { ExceptionsListApiClient } from '../../services/exceptions_list/exceptions_list_api_client';

const DEFAULT_OPTIONS = Object.freeze({});

export function useSummaryArtifact(
exceptionListApiClient: ExceptionsListApiClient,
searcheableFields: string[],
Expand All @@ -20,7 +22,7 @@ export function useSummaryArtifact(
filter: '',
policies: [],
},
customQueryOptions: UseQueryOptions<ExceptionListSummarySchema, HttpFetchError>
customQueryOptions: UseQueryOptions<ExceptionListSummarySchema, HttpFetchError> = DEFAULT_OPTIONS
): QueryObserverResult<ExceptionListSummarySchema, HttpFetchError> {
const { filter, policies } = options;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ import {
ENDPOINT_HOST_ISOLATION_EXCEPTIONS_LIST_NAME,
} from '@kbn/securitysolution-list-constants';

export const SEARCHABLE_FIELDS: Readonly<string[]> = [
dasansol92 marked this conversation as resolved.
Show resolved Hide resolved
`item_id`,
`name`,
`description`,
`entries.value`,
];

export const HOST_ISOLATION_EXCEPTIONS_LIST_TYPE: ExceptionListType =
ExceptionListTypeEnum.ENDPOINT_HOST_ISOLATION_EXCEPTIONS;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from '../../../common/constants';
import { getHostIsolationExceptionsListPath } from '../../../common/routing';
import { parsePoliciesAndFilterToKql, parseQueryFilterToKQL } from '../../../common/utils';
import { SEARCHABLE_FIELDS } from '../constants';
import {
getHostIsolationExceptionItems,
getHostIsolationExceptionSummary,
Expand Down Expand Up @@ -85,8 +86,6 @@ export function useCanSeeHostIsolationExceptionsMenu(): boolean {
return canSeeMenu;
}

const SEARCHABLE_FIELDS: Readonly<string[]> = [`item_id`, `name`, `description`, `entries.value`];

export function useFetchHostIsolationExceptionsList({
filter,
page,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
*/

import { PolicySettingsAction } from './policy_settings_action';
import { PolicyTrustedAppsAction } from './policy_trusted_apps_action';

export type PolicyDetailsAction = PolicySettingsAction | PolicyTrustedAppsAction;
export type PolicyDetailsAction = PolicySettingsAction;

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,18 @@

import { ImmutableMiddlewareFactory } from '../../../../../../common/store';
import { MiddlewareRunnerContext, PolicyDetailsState } from '../../../types';
import { policyTrustedAppsMiddlewareRunner } from './policy_trusted_apps_middleware';
import { policySettingsMiddlewareRunner } from './policy_settings_middleware';
import { TrustedAppsHttpService } from '../../../../trusted_apps/service';

export const policyDetailsMiddlewareFactory: ImmutableMiddlewareFactory<PolicyDetailsState> = (
coreStart
) => {
// Initialize services needed by Policy middleware
const trustedAppsService = new TrustedAppsHttpService(coreStart.http);
const middlewareContext: MiddlewareRunnerContext = {
coreStart,
trustedAppsService,
};

return (store) => (next) => async (action) => {
next(action);

policySettingsMiddlewareRunner(middlewareContext, store, action);
policyTrustedAppsMiddlewareRunner(middlewareContext, store, action);
};
};
Loading