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.

Sign up for GitHub

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

[Cloud Security] Clicking on Contextual Flyout popout Icon now opens page in new tab #196763

Merged
merged 15 commits into from
Oct 27, 2024
Merged

[Cloud Security] Clicking on Contextual Flyout popout Icon now opens page in new tab #196763

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
82ff137
initial
animehart Oct 17, 2024
f09dce8
opens new tab
animehart Oct 17, 2024
5dd8c78
added more field to make search result unique
animehart Oct 18, 2024
ac0df66
readded UI Metrics + updated some filter
animehart Oct 18, 2024
9d2632e
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Oct 18, 2024
e313319
update name
animehart Oct 18, 2024
35cc693
Merge branch 'contextual-flyout-open-new-tab' of github.com:animehart…
animehart Oct 18, 2024
c63540f
fix bundle size issue
animehart Oct 18, 2024
19135a8
addressing PR part 1
animehart Oct 18, 2024
cdb9f8c
addressing PR comments
animehart Oct 24, 2024
3303859
fix quick check
animehart Oct 24, 2024
bf7e3ee
check types fix
animehart Oct 24, 2024
d32c49b
Merge branch 'main' into contextual-flyout-open-new-tab
animehart Oct 24, 2024
3a98256
addressing pr comments
animehart Oct 24, 2024
41c2c90
Merge branch 'main' into contextual-flyout-open-new-tab
animehart Oct 27, 2024
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
3 changes: 2 additions & 1 deletion x-pack/packages/kbn-cloud-security-posture/common/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export const STATUS_ROUTE_PATH = '/internal/cloud_security_posture/status';
export const STATUS_API_CURRENT_VERSION = '1';

/** The base path for all cloud security posture pages. */
export const CLOUD_SECURITY_POSTURE_BASE_PATH = '/cloud_security_posture';
export const CLOUD_SECURITY_POSTURE_PATH = 'cloud_security_posture';
maxcold marked this conversation as resolved.
Show resolved Hide resolved
export const CLOUD_SECURITY_POSTURE_BASE_PATH = '/' + CLOUD_SECURITY_POSTURE_PATH;

export const CDR_MISCONFIGURATIONS_DATA_VIEW_ID_PREFIX =
'security_solution_cdr_latest_misconfigurations';
Expand Down
28 changes: 28 additions & 0 deletions x-pack/packages/kbn-cloud-security-posture/public/src/hooks/use_get_navigation_url_params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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 { CoreStart } from '@kbn/core-lifecycle-browser';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { useCallback } from 'react';
import { CspClientPluginStartDeps } from '../types';
import { encodeQueryUrl, queryFilters } from '../utils/query_utils';
import { NavFilter } from './use_navigate_findings';

export const useGetNavigationUrlParams = () => {
const { services } = useKibana<CoreStart & CspClientPluginStartDeps>();

return useCallback(
(filterParams: NavFilter = {}, groupBy?: string[]) => {
const filters = queryFilters(filterParams);

const searchParams = new URLSearchParams(encodeQueryUrl(services, filters, groupBy));

return `?${searchParams.toString()}`;
},
[services]
Copy link
Contributor

Choose a reason for hiding this comment

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

can be replaced with services.data as we only rely on data service

);
};
4 changes: 2 additions & 2 deletions x-pack/packages/kbn-cloud-security-posture/public/src/hooks/use_navigate_findings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type FilterValue = string | number | NegatedValue;

export type NavFilter = Record<string, FilterValue>;

const createFilter = (key: string, filterValue: FilterValue, dataViewId: string): Filter => {
export const createFilter = (key: string, filterValue: FilterValue, dataViewId: string): Filter => {
maxcold marked this conversation as resolved.
Show resolved Hide resolved
let negate = false;
let value = filterValue;
if (typeof filterValue === 'object') {
Expand Down Expand Up @@ -56,8 +56,8 @@ const createFilter = (key: string, filterValue: FilterValue, dataViewId: string)
};
const useNavigate = (pathname: string, dataViewId = SECURITY_DEFAULT_DATA_VIEW_ID) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need to fallback to SECURITY_DEFAULT_DATA_VIEW_ID as the fallback already a part of queryFilters

const history = useHistory();
const { services } = useKibana<CoreStart & CspClientPluginStartDeps>();

const { services } = useKibana<CoreStart & CspClientPluginStartDeps>();
return useCallback(
(filterParams: NavFilter = {}, groupBy?: string[]) => {
const filters = Object.entries(filterParams).map(([key, filterValue]) =>
maxcold marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
26 changes: 26 additions & 0 deletions x-pack/packages/kbn-cloud-security-posture/public/src/utils/query_utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { CoreStart } from '@kbn/core-lifecycle-browser';
import { encode, decode } from '@kbn/rison';
import type { LocationDescriptorObject } from 'history';
import { Filter } from '@kbn/es-query';
import { SECURITY_DEFAULT_DATA_VIEW_ID } from '@kbn/cloud-security-posture-common';
import { CspClientPluginStartDeps } from '../types';
import { createFilter, NavFilter } from '../hooks/use_navigate_findings';

const encodeRison = (v: any): string | undefined => {
try {
Expand Down Expand Up @@ -38,3 +43,24 @@ export const decodeQuery = <T extends unknown>(search?: string): Partial<T> | un
if (!risonQuery) return;
return decodeRison<T>(risonQuery);
};

export const encodeQueryUrl = (
maxcold marked this conversation as resolved.
Show resolved Hide resolved
services: Partial<CoreStart> & CoreStart & CspClientPluginStartDeps,
filters: Filter[],
groupBy?: string[]
): any => {
return encodeQuery({
query: services.data.query.queryString.getDefaultQuery(),
filters,
...(groupBy && { groupBy }),
});
};

export const queryFilters = (
filterParams: NavFilter = {},
dataViewId = SECURITY_DEFAULT_DATA_VIEW_ID
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the use of SECURITY_DEFAULT_DATA_VIEW_ID . Similarly I don't understand why useNavigateFindings and useNavigateVulnerabilities differ in the data view id logic. Similarly to CDR_MISCONFIGURATIONS_DATA_VIEW_ID_PREFIX we have CDR_VULNERABILITIES_DATA_VIEW_ID_PREFIX. Why don't we use it in useNavigateVulnerabilities then? And in your new logic you only use SECURITY_DEFAULT_DATA_VIEW_ID , so we have a variety of difference here. @opauloh do you recall anything about the use of data view ids for the navigation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey that was added on this PR

If we don't specify the DataView on the filter when navigating to the findings page, the FilterManager will fallback to the default in the sorcerer (logs-*). This doesn't prevent filtering from working, but it doesn't allow for editing the filter, since the SearchBar component is configured to accept only the Latest Misconfigurations Dataview (or the Latest Vulnerabilities DataView on the Vulnerabilities tab).

image

So the fix was to specify the Dataview for each filter, and then the edit filter works:

image

Should probably have added it as a comment, which I suggest @animehart to add so we can recall when going over it in the future.

Another solution could be to update the navigation to set the sorcerer index pattern instead of setting it per filter, but that also involves setting a Dataview in the navigation logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the detailed clarification, makes sense! Agree that we need to add a comment as this Data View logic pieces are quite complicated to grasp without all the context. I think this logic should also be added to the vulnerability navigation logic, right now it looks like it is not part of it, unless I missed that in the code

Copy link
Contributor

Choose a reason for hiding this comment

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

@animehart we still don't pass vulnerabiltiy dataview id to the navigation, while for findings we do. Should we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxcold
if there aren't anything that can be broken by it then I think we should

Copy link
Contributor

Choose a reason for hiding this comment

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

as we plan to backport this to 8.16 let's not add this new logic, just to be on the safe side. We can do it separately for 8.17

): Filter[] => {
return Object.entries(filterParams).map(([key, filterValue]) =>
createFilter(key, filterValue, dataViewId)
);
};
1 change: 1 addition & 0 deletions x-pack/packages/kbn-cloud-security-posture/public/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@
"@kbn/ui-theme",
"@kbn/i18n-react",
"@kbn/rison",
"@kbn/core-lifecycle-browser",
]
}
45 changes: 28 additions & 17 deletions ...cloud_security_posture/components/csp_details/misconfiguration_findings_details_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@

import React, { memo, useEffect, useState } from 'react';
import type { Criteria, EuiBasicTableColumn } from '@elastic/eui';
import { EuiSpacer, EuiIcon, EuiPanel, EuiLink, EuiText, EuiBasicTable } from '@elastic/eui';
import { EuiSpacer, EuiPanel, EuiText, EuiBasicTable, EuiIcon } from '@elastic/eui';
import { useMisconfigurationFindings } from '@kbn/cloud-security-posture/src/hooks/use_misconfiguration_findings';
import { i18n } from '@kbn/i18n';
import type { CspFinding, CspFindingResult } from '@kbn/cloud-security-posture-common';
import { buildEntityFlyoutPreviewQuery } from '@kbn/cloud-security-posture-common';
import { euiThemeVars } from '@kbn/ui-theme';
import { DistributionBar } from '@kbn/security-solution-distribution-bar';
import { useNavigateFindings } from '@kbn/cloud-security-posture/src/hooks/use_navigate_findings';
import type { CspBenchmarkRuleMetadata } from '@kbn/cloud-security-posture-common/schema/rules/latest';
import { CspEvaluationBadge } from '@kbn/cloud-security-posture';
import {
Expand All @@ -24,6 +23,9 @@ import {
uiMetricService,
} from '@kbn/cloud-security-posture-common/utils/ui_metrics';
import { METRIC_TYPE } from '@kbn/analytics';
import { useGetNavigationUrlParams } from '@kbn/cloud-security-posture/src/hooks/use_get_navigation_url_params';
import { SecurityPageName } from '@kbn/deeplinks-security';
import { SecuritySolutionLinkAnchor } from '../../../common/components/links';

type MisconfigurationFindingDetailFields = Pick<CspFinding, 'result' | 'rule' | 'resource'>;

Expand Down Expand Up @@ -114,18 +116,14 @@ export const MisconfigurationFindingsDetailsTable = memo(
}
};

const navToFindings = useNavigateFindings();
const navUrlParams = useGetNavigationUrlParams();

const navToFindingsByRuleAndResourceId = (ruleId: string, resourceId: string) => {
navToFindings({ 'rule.id': ruleId, 'resource.id': resourceId });
const getFindingsPageUrlFilteredByRuleAndResourceId = (ruleId: string, resourceId: string) => {
return navUrlParams({ 'rule.id': ruleId, 'resource.id': resourceId });
};

const navToFindingsByName = (name: string, queryField: 'host.name' | 'user.name') => {
uiMetricService.trackUiMetric(
METRIC_TYPE.CLICK,
NAV_TO_FINDINGS_BY_RULE_NAME_FRPOM_ENTITY_FLYOUT
);
navToFindings({ [queryField]: name }, ['rule.name']);
const getFindingsPageUrl = (name: string, queryField: 'host.name' | 'user.name') => {
maxcold marked this conversation as resolved.
Show resolved Hide resolved
return navUrlParams({ [queryField]: name }, ['rule.name']);
};

const columns: Array<EuiBasicTableColumn<MisconfigurationFindingDetailFields>> = [
Expand All @@ -134,13 +132,23 @@ export const MisconfigurationFindingsDetailsTable = memo(
name: '',
width: '5%',
render: (rule: CspBenchmarkRuleMetadata, finding: MisconfigurationFindingDetailFields) => (
<EuiLink
<SecuritySolutionLinkAnchor
deepLinkId={SecurityPageName.cloudSecurityPostureFindings}
path={`configurations${getFindingsPageUrlFilteredByRuleAndResourceId(
maxcold marked this conversation as resolved.
Show resolved Hide resolved
rule?.id,
finding?.resource?.id
)}`}
target={'_blank'}
external={false}
onClick={() => {
navToFindingsByRuleAndResourceId(rule?.id, finding?.resource?.id);
uiMetricService.trackUiMetric(
METRIC_TYPE.CLICK,
NAV_TO_FINDINGS_BY_RULE_NAME_FRPOM_ENTITY_FLYOUT
);
}}
>
<EuiIcon type={'popout'} />
</EuiLink>
</SecuritySolutionLinkAnchor>
),
},
{
Expand Down Expand Up @@ -170,13 +178,16 @@ export const MisconfigurationFindingsDetailsTable = memo(
return (
<>
<EuiPanel hasShadow={false}>
<EuiLink
<SecuritySolutionLinkAnchor
deepLinkId={SecurityPageName.cloudSecurityPostureFindings}
path={`configurations${getFindingsPageUrl(queryName, fieldName)}`}
target={'_blank'}
external={false}
onClick={() => {
uiMetricService.trackUiMetric(
METRIC_TYPE.CLICK,
NAV_TO_FINDINGS_BY_HOST_NAME_FRPOM_ENTITY_FLYOUT
);
navToFindingsByName(queryName, fieldName);
}}
>
{i18n.translate(
Expand All @@ -186,7 +197,7 @@ export const MisconfigurationFindingsDetailsTable = memo(
}
)}
<EuiIcon type={'popout'} />
</EuiLink>
</SecuritySolutionLinkAnchor>
<EuiSpacer size="xl" />
<DistributionBar stats={getFindingsStats(passedFindings, failedFindings)} />
<EuiSpacer size="l" />
Expand Down
51 changes: 32 additions & 19 deletions .../cloud_security_posture/components/csp_details/vulnerabilities_findings_details_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@

import React, { memo, useEffect, useState } from 'react';
import type { Criteria, EuiBasicTableColumn } from '@elastic/eui';
import { EuiSpacer, EuiIcon, EuiPanel, EuiLink, EuiText, EuiBasicTable } from '@elastic/eui';
import { EuiSpacer, EuiPanel, EuiText, EuiBasicTable, EuiIcon } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import type { VulnSeverity } from '@kbn/cloud-security-posture-common';
import { buildEntityFlyoutPreviewQuery } from '@kbn/cloud-security-posture-common';
import { DistributionBar } from '@kbn/security-solution-distribution-bar';
import { useNavigateVulnerabilities } from '@kbn/cloud-security-posture/src/hooks/use_navigate_findings';
import { useVulnerabilitiesFindings } from '@kbn/cloud-security-posture/src/hooks/use_vulnerabilities_findings';
import type {
CspVulnerabilityFinding,
Expand All @@ -29,6 +28,9 @@ import {
uiMetricService,
} from '@kbn/cloud-security-posture-common/utils/ui_metrics';
import { METRIC_TYPE } from '@kbn/analytics';
import { SecurityPageName } from '@kbn/deeplinks-security';
import { useGetNavigationUrlParams } from '@kbn/cloud-security-posture/src/hooks/use_get_navigation_url_params';
import { SecuritySolutionLinkAnchor } from '../../../common/components/links';

type VulnerabilitiesFindingDetailFields = Pick<
CspVulnerabilityFinding,
Expand All @@ -38,6 +40,7 @@ type VulnerabilitiesFindingDetailFields = Pick<
interface VulnerabilitiesPackage extends Vulnerability {
package: {
name: string;
version: string;
};
}

Expand Down Expand Up @@ -94,19 +97,23 @@ export const VulnerabilitiesFindingsDetailsTable = memo(({ queryName }: { queryN
}
};

const navToVulnerabilities = useNavigateVulnerabilities();
const navUrlParams = useGetNavigationUrlParams();
Copy link
Contributor

@JordanSh JordanSh Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

since useGetNavigationUrlParams returns a function its better to name the return value getNavUrlParams
so if you want to extract its own return value it would make sense to read as in:

const getNavUrlParams = useGetNavUrlParams()
const navUrlParams = getNavUrlParams({stuff})


const navToVulnerabilitiesByName = (name: string, queryField: 'host.name' | 'user.name') => {
navToVulnerabilities({ [queryField]: name });
const getVulnerabilityUrl = (name: string, queryField: 'host.name' | 'user.name') => {
return navUrlParams({ [queryField]: name });
};

const navToVulnerabilityByVulnerabilityAndResourceId = (
const getVulnerabilityUrlFilteredByVulnerabilityAndResourceId = (
vulnerabilityId: string,
resourceId: string
resourceId: string,
vulnerabilityPackageName: string,
vulnerabilityPackageVersion: string
) => {
navToVulnerabilities({
return navUrlParams({
'vulnerability.id': vulnerabilityId,
'resource.id': resourceId,
'vulnerability.package.name': vulnerabilityPackageName,
'vulnerability.package.version': vulnerabilityPackageVersion,
});
};

Expand All @@ -119,16 +126,19 @@ export const VulnerabilitiesFindingsDetailsTable = memo(({ queryName }: { queryN
vulnerability: VulnerabilitiesPackage,
finding: VulnerabilitiesFindingDetailFields
) => (
<EuiLink
onClick={() => {
navToVulnerabilityByVulnerabilityAndResourceId(
vulnerability?.id,
finding?.resource?.id || ''
);
}}
<SecuritySolutionLinkAnchor
deepLinkId={SecurityPageName.cloudSecurityPostureFindings}
path={`vulnerabilities${getVulnerabilityUrlFilteredByVulnerabilityAndResourceId(
vulnerability?.id,
finding?.resource?.id || '',
vulnerability?.package?.name,
vulnerability?.package?.version
)}`}
target={'_blank'}
external={false}
>
<EuiIcon type={'popout'} />
</EuiLink>
</SecuritySolutionLinkAnchor>
),
},
{
Expand Down Expand Up @@ -189,20 +199,23 @@ export const VulnerabilitiesFindingsDetailsTable = memo(({ queryName }: { queryN
return (
<>
<EuiPanel hasShadow={false}>
<EuiLink
<SecuritySolutionLinkAnchor
deepLinkId={SecurityPageName.cloudSecurityPostureFindings}
path={`vulnerabilities${getVulnerabilityUrl(queryName, 'host.name')}`}
target={'_blank'}
external={false}
onClick={() => {
uiMetricService.trackUiMetric(
METRIC_TYPE.CLICK,
NAV_TO_FINDINGS_BY_HOST_NAME_FRPOM_ENTITY_FLYOUT
);
navToVulnerabilitiesByName(queryName, 'host.name');
}}
>
{i18n.translate('xpack.securitySolution.flyout.left.insights.vulnerability.tableTitle', {
defaultMessage: 'Vulnerability ',
})}
<EuiIcon type={'popout'} />
</EuiLink>
</SecuritySolutionLinkAnchor>
<EuiSpacer size="xl" />
<DistributionBar
stats={getVulnerabilityStats({
Expand Down