Skip to content

Commit

Permalink
[8.8] [Security Solution] getDataViewStateFromIndexFields was using w…
Browse files Browse the repository at this point in the history
…rong type as part of a cast (#158594) (#158784)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] getDataViewStateFromIndexFields was using wrong
type as part of a cast
(#158594)](#158594)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-31T21:13:36Z","message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Threat
Hunting:Investigations","v8.9.0","v8.8.1"],"number":158594,"url":"https://github.com/elastic/kibana/pull/158594","mergeCommit":{"message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/158594","number":158594,"mergeCommit":{"message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d"}},{"branch":"8.8","label":"v8.8.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
kqualters-elastic and kibanamachine authored Jun 1, 2023
1 parent 06ee73f commit cc69e5b
Show file tree
Hide file tree
Showing 27 changed files with 130 additions and 238 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ import type {
MappingRuntimeField,
MappingRuntimeFields,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { RuntimeFieldSpec, RuntimePrimitiveTypes } from '@kbn/data-views-plugin/common';
import type { BoolQuery } from '@kbn/es-query';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

type RunTimeMappings =
| Record<string, Omit<RuntimeFieldSpec, 'type'> & { type: RuntimePrimitiveTypes }>
| undefined;

interface BoolAgg {
bool: BoolQuery;
}
Expand All @@ -29,7 +34,7 @@ export interface GroupingQueryArgs {
from: string;
groupByField: string;
rootAggregations?: NamedAggregation[];
runtimeMappings?: MappingRuntimeFields;
runtimeMappings?: RunTimeMappings;
additionalAggregationsRoot?: NamedAggregation[];
pageNumber?: number;
uniqueValue: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import type {
TimelineRequestSortField,
TimelineStrategyResponseType,
} from '@kbn/timelines-plugin/common/search_strategy';
import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/types';
import { dataTableActions, Direction, TableId } from '@kbn/securitysolution-data-table';
import type { RunTimeMappings } from '../../store/sourcerer/model';
import { TimelineEventsQueries } from '../../../../common/search_strategy';
import type { KueryFilterQueryKind } from '../../../../common/types';
import type { ESQuery } from '../../../../common/typed_json';
Expand Down Expand Up @@ -77,7 +77,7 @@ export interface UseTimelineEventsProps {
indexNames: string[];
language?: KueryFilterQueryKind;
limit: number;
runtimeMappings: MappingRuntimeFields;
runtimeMappings: RunTimeMappings;
skip?: boolean;
sort?: TimelineRequestSortField[];
startDate: string;
Expand Down Expand Up @@ -321,7 +321,7 @@ export const useTimelineEventsHandler = ({
querySize: prevRequest?.pagination.querySize ?? 0,
sort: prevRequest?.sort ?? initSortDefault,
timerange: prevRequest?.timerange ?? {},
runtimeMappings: prevRequest?.runtimeMappings ?? {},
runtimeMappings: (prevRequest?.runtimeMappings ?? {}) as RunTimeMappings,
filterStatus: prevRequest?.filterStatus,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
import numeral from '@elastic/numeral';
import { css } from '@emotion/react';
import type { EuiMarkdownEditorUiPluginEditorProps } from '@elastic/eui/src/components/markdown_editor/markdown_types';
import { DataView } from '@kbn/data-views-plugin/public';
import { FormattedMessage } from '@kbn/i18n-react';
import type { Filter } from '@kbn/es-query';
import { FilterStateStore } from '@kbn/es-query';
Expand Down Expand Up @@ -245,7 +246,16 @@ const InsightEditorComponent = ({
ui: { FiltersBuilderLazy },
},
uiSettings,
fieldFormats,
} = useKibana().services;
const dataView = useMemo(() => {
if (sourcererDataView != null) {
return new DataView({ spec: sourcererDataView, fieldFormats });
} else {
return null;
}
}, [sourcererDataView, fieldFormats]);

const [providers, setProviders] = useState<Provider[][]>([[]]);
const dateRangeChoices = useMemo(() => {
const settings: Array<{ from: string; to: string; display: string }> = uiSettings.get(
Expand Down Expand Up @@ -419,11 +429,11 @@ const InsightEditorComponent = ({
/>
</EuiFormRow>
<EuiFormRow label={i18n.FILTER_BUILDER} helpText={i18n.FILTER_BUILDER_TEXT} fullWidth>
{sourcererDataView ? (
{dataView ? (
<FiltersBuilderLazy
filters={filtersStub}
onChange={onChange}
dataView={sourcererDataView}
dataView={dataView}
maxDepth={2}
/>
) : (
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
* 2.0.
*/

import type { IndexField } from '../../../../common/search_strategy/index_fields';
import { getBrowserFields, getAllBrowserFields } from '.';
import type { IndexFieldSearch } from './use_data_view';
import { useDataView } from './use_data_view';
import { mocksSource } from './mock';
Expand All @@ -27,32 +25,6 @@ jest.mock('../../lib/kibana');
jest.mock('../../lib/apm/use_track_http_request');

describe('source/index.tsx', () => {
describe('getAllBrowserFields', () => {
test('it returns an array of all fields in the BrowserFields argument', () => {
expect(
getAllBrowserFields(getBrowserFields('title 1', mocksSource.indexFields as IndexField[]))
).toMatchSnapshot();
});
});
describe('getBrowserFields', () => {
test('it returns an empty object given an empty array', () => {
const fields = getBrowserFields('title 1', []);
expect(fields).toEqual({});
});

test('it returns the same input given the same title and same fields length', () => {
const oldFields = getBrowserFields('title 1', mocksSource.indexFields as IndexField[]);
const newFields = getBrowserFields('title 1', mocksSource.indexFields as IndexField[]);
// Since it is memoized it will return the same object instance
expect(newFields).toBe(oldFields);
});

test('it transforms input into output as expected', () => {
const fields = getBrowserFields('title 2', mocksSource.indexFields as IndexField[]);
expect(fields).toMatchSnapshot();
});
});

describe('useDataView hook', () => {
const mockSearchResponse = {
...mocksSource,
Expand All @@ -74,8 +46,8 @@ describe('source/index.tsx', () => {
data: {
dataViews: {
...useKibana().services.data.dataViews,
get: async (dataViewId: string, displayErrors?: boolean, refreshFields = false) =>
Promise.resolve({
get: async (dataViewId: string, displayErrors?: boolean, refreshFields = false) => {
const dataViewMock = {
id: dataViewId,
matchedIndices: refreshFields
? ['hello', 'world', 'refreshed']
Expand All @@ -88,7 +60,12 @@ describe('source/index.tsx', () => {
type: 'keyword',
},
}),
}),
};
return Promise.resolve({
toSpec: () => dataViewMock,
...dataViewMock,
});
},
getFieldsForWildcard: async () => Promise.resolve(),
},
search: {
Expand Down Expand Up @@ -178,7 +155,6 @@ describe('source/index.tsx', () => {
const {
payload: { patternList: newPatternList },
} = mockDispatch.mock.calls[1][0];

expect(patternList).not.toBe(newPatternList);
expect(patternList).not.toContain('refreshed*');
expect(newPatternList).toContain('refreshed*');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { isEmpty, isEqual, keyBy, pick } from 'lodash/fp';
import memoizeOne from 'memoize-one';
import { useCallback, useEffect, useRef, useState } from 'react';
import type { DataViewBase } from '@kbn/es-query';
import type { BrowserField, BrowserFields, IndexField } from '@kbn/timelines-plugin/common';
import type { DataView, IIndexPatternFieldList } from '@kbn/data-views-plugin/common';
import { getCategory } from '@kbn/triggers-actions-ui-plugin/public';
import type { BrowserField, BrowserFields } from '@kbn/timelines-plugin/common';
import type { IIndexPatternFieldList } from '@kbn/data-views-plugin/common';
import type { DataViewSpec } from '@kbn/data-views-plugin/public';

import { useKibana } from '../../lib/kibana';
import * as i18n from './translations';
Expand Down Expand Up @@ -72,35 +72,6 @@ export const getIndexFields = memoizeOne(
newArgs[2] === lastArgs[2]
);

/**
* HOT Code path where the fields can be 16087 in length or larger. This is
* VERY mutatious on purpose to improve the performance of the transform.
*/
export const getBrowserFields = memoizeOne(
(_title: string, fields: IndexField[]): BrowserFields => {
// Adds two dangerous casts to allow for mutations within this function
type DangerCastForMutation = Record<string, {}>;
type DangerCastForBrowserFieldsMutation = Record<
string,
Omit<BrowserField, 'fields'> & { fields: Record<string, BrowserField> }
>;

// We mutate this instead of using lodash/set to keep this as fast as possible
return fields.reduce<DangerCastForBrowserFieldsMutation>((accumulator, field) => {
const category = getCategory(field.name);
if (accumulator[category] == null) {
(accumulator as DangerCastForMutation)[category] = {};
}
if (accumulator[category].fields == null) {
accumulator[category].fields = {};
}
accumulator[category].fields[field.name] = field as unknown as BrowserField;
return accumulator;
}, {});
},
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1].length === lastArgs[1].length
);

const DEFAULT_BROWSER_FIELDS = {};
const DEFAULT_INDEX_PATTERNS = { fields: [], title: '' };
interface FetchIndexReturn {
Expand All @@ -115,7 +86,7 @@ interface FetchIndexReturn {
indexes: string[];
indexExists: boolean;
indexPatterns: DataViewBase;
dataView: DataView | undefined;
dataView: DataViewSpec | undefined;
}

/**
Expand Down Expand Up @@ -149,17 +120,18 @@ export const useFetchIndex = (
setState({ ...state, loading: true });
abortCtrl.current = new AbortController();
const dv = await data.dataViews.create({ title: iNames.join(','), allowNoIndex: true });
const dataView = dv.toSpec();
const { browserFields } = getDataViewStateFromIndexFields(
iNames,
dv.fields,
dataView.fields,
includeUnmapped
);

previousIndexesName.current = dv.getIndexPattern().split(',');

setState({
loading: false,
dataView: dv,
dataView,
browserFields,
indexes: dv.getIndexPattern().split(','),
indexExists: dv.getIndexPattern().split(',').length > 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { MappingRuntimeFieldType } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { DEFAULT_INDEX_PATTERN } from '../../../../common/constants';
import type { BrowserFields } from '../../../../common/search_strategy/index_fields';

Expand Down Expand Up @@ -577,11 +577,13 @@ export const mockBrowserFields: BrowserFields = {
},
};

export const mockRuntimeMappings: MappingRuntimeFields = {
const runTimeType: MappingRuntimeFieldType = 'keyword' as const;

export const mockRuntimeMappings = {
'@a.runtime.field': {
script: {
source: 'emit("Radical dude: " + doc[\'host.name\'].value)',
},
type: 'keyword',
type: runTimeType,
},
};
Loading

0 comments on commit cc69e5b

Please sign in to comment.