From e0be9c90262b816b503d1318d347440a396591d5 Mon Sep 17 00:00:00 2001 From: Sergi Massaneda Date: Tue, 30 Jan 2024 16:19:55 +0100 Subject: [PATCH] fix privileges and last check issue --- .../index_properties/index.tsx | 4 +- .../summary_table/helpers.tsx | 2 +- .../impl/data_quality/helpers.ts | 2 +- .../impl/data_quality/types.ts | 2 +- .../use_results_rollup/helpers.test.ts | 2 +- .../use_results_rollup/helpers.ts | 3 +- .../data_quality/use_results_rollup/index.tsx | 25 ++-- .../server/routes/results/get_results.test.ts | 7 + .../server/routes/results/get_results.ts | 14 +- .../routes/results/post_results.test.ts | 56 ++++---- .../server/routes/results/post_results.ts | 13 +- .../server/routes/results/privileges.test.ts | 129 ++++++++++++++++++ .../server/routes/results/privileges.ts | 40 ++++++ 13 files changed, 241 insertions(+), 58 deletions(-) create mode 100644 x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.test.ts create mode 100644 x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.ts diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/index_properties/index.tsx b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/index_properties/index.tsx index d5171859dba1a6..6e1819d35537d6 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/index_properties/index.tsx +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/index_properties/index.tsx @@ -249,6 +249,8 @@ const IndexPropertiesComponent: React.FC = ({ }) : EMPTY_MARKDOWN_COMMENTS; + const checkedAt = partitionedFieldMetadata ? Date.now() : undefined; + const updatedRollup = { ...patternRollup, results: { @@ -262,7 +264,7 @@ const IndexPropertiesComponent: React.FC = ({ markdownComments, pattern, sameFamily: indexSameFamily, - checkedAt: Date.now(), + checkedAt, }, }, }; diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/summary_table/helpers.tsx b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/summary_table/helpers.tsx index b785e9c6b1c302..55e26200f0e44a 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/summary_table/helpers.tsx +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/data_quality_panel/summary_table/helpers.tsx @@ -245,7 +245,7 @@ export const getSummaryTableColumns = ({ name: i18n.LAST_CHECK, render: (_, { checkedAt }) => ( - {moment(checkedAt).isValid() ? moment(checkedAt).fromNow() : EMPTY_STAT} + {checkedAt && moment(checkedAt).isValid() ? moment(checkedAt).fromNow() : EMPTY_STAT} ), sortable: true, diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/helpers.ts b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/helpers.ts index 42e015856d10af..2107e7d3949daf 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/helpers.ts +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/helpers.ts @@ -484,7 +484,7 @@ export const formatStorageResult = ({ batchId: report.batchId, indexName: result.indexName, isCheckAll: report.isCheckAll, - checkedAt: result.checkedAt, + checkedAt: result.checkedAt ?? Date.now(), docsCount: result.docsCount ?? 0, totalFieldCount: partitionedFieldMetadata.all.length, ecsFieldCount: partitionedFieldMetadata.ecsCompliant.length, diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/types.ts b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/types.ts index 1d267348970d50..f462777e1fc0bd 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/types.ts +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/types.ts @@ -107,7 +107,7 @@ export interface DataQualityCheckResult { markdownComments: string[]; sameFamily: number | undefined; pattern: string; - checkedAt: number; + checkedAt: number | undefined; } export interface PatternRollup { diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.test.ts b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.test.ts index b6bae29b0bbef6..b948b9584f996d 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.test.ts +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.test.ts @@ -453,7 +453,7 @@ describe('helpers', () => { indexName: '.ds-packetbeat-8.6.1-2023.02.04-000001', markdownComments: [], pattern: 'packetbeat-*', - checkedAt: expect.any(Number), + checkedAt: undefined, }, }, sizeInBytes: 1464758182, diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.ts b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.ts index 8bfb517df0baaa..07f51572b6ba23 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.ts +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/helpers.ts @@ -140,6 +140,7 @@ export const updateResultOnCheckCompleted = ({ const incompatible = partitionedFieldMetadata?.incompatible.length; const sameFamily = partitionedFieldMetadata?.sameFamily.length; + const checkedAt = partitionedFieldMetadata ? Date.now() : undefined; return { ...patternRollups, @@ -156,7 +157,7 @@ export const updateResultOnCheckCompleted = ({ markdownComments, pattern, sameFamily, - checkedAt: Date.now(), + checkedAt, }, }, }, diff --git a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/index.tsx b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/index.tsx index 8f48c631b7aee1..907ffca5a45f68 100644 --- a/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/index.tsx +++ b/x-pack/packages/security-solution/ecs_data_quality_dashboard/impl/data_quality/use_results_rollup/index.tsx @@ -111,18 +111,19 @@ export const useResultsRollup = ({ ilmPhases, patterns }: Props): UseResultsRoll useEffect(() => { if (!isEmpty(storedPatternsResults)) { - setPatternRollups((current) => { - const updatedRollups = { ...current }; - storedPatternsResults.forEach((storedPatternResult) => { - const { pattern, results } = storedPatternResult; - updatedRollups[pattern] = { - ...current[pattern], - pattern, - results, - }; - }); - return updatedRollups; - }); + setPatternRollups((current) => + storedPatternsResults.reduce( + (acc, { pattern, results }) => ({ + ...acc, + [pattern]: { + ...current[pattern], + pattern, + results, + }, + }), + current + ) + ); } }, [storedPatternsResults]); diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.test.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.test.ts index 52d2a84cb5cb75..452cf71956240b 100644 --- a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.test.ts +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.test.ts @@ -32,6 +32,13 @@ const searchResponse = { Record >; +const mockCheckIndicesPrivileges = jest + .fn() + .mockResolvedValue({ [resultDocument.indexName]: true }); +jest.mock('./privileges', () => ({ + checkIndicesPrivileges: (params: unknown) => mockCheckIndicesPrivileges(params), +})); + describe('getResultsRoute route', () => { describe('querying', () => { let server: ReturnType; diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.ts index bd3f935fadc6d6..18d221f60301da 100644 --- a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.ts +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/get_results.ts @@ -15,6 +15,7 @@ import type { ResultDocument } from '../../schemas/result'; import { API_DEFAULT_ERROR_MESSAGE } from '../../translations'; import type { DataQualityDashboardRequestHandlerContext } from '../../types'; import { API_RESULTS_INDEX_NOT_AVAILABLE } from './translations'; +import { checkIndicesPrivileges } from './privileges'; export const getQuery = (indexName: string[]) => ({ size: 0, @@ -66,18 +67,19 @@ export const getResultsRoute = ( const { pattern } = request.query; // Get authorized indices - const userEsClient = services.core.elasticsearch.client.asCurrentUser; - const indices = await userEsClient.indices.get({ index: pattern }); - const authorizedIndexNames = Object.keys(indices); + const { client } = services.core.elasticsearch; + const indicesResponse = await client.asCurrentUser.indices.get({ index: pattern }); + const indices = Object.keys(indicesResponse); + + const hadIndexPrivileges = await checkIndicesPrivileges({ client, indices }); + const authorizedIndexNames = indices.filter((indexName) => hadIndexPrivileges[indexName]); if (authorizedIndexNames.length === 0) { return response.ok({ body: [] }); } // Get the latest result of each pattern const query = { index, ...getQuery(authorizedIndexNames) }; - const internalEsClient = services.core.elasticsearch.client.asInternalUser; - - const { aggregations } = await internalEsClient.search< + const { aggregations } = await client.asInternalUser.search< ResultDocument, Record >(query); diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.test.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.test.ts index 680d6801bfab1f..8694cf1e558b6c 100644 --- a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.test.ts +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.test.ts @@ -17,6 +17,13 @@ import type { } from '@elastic/elasticsearch/lib/api/types'; import { resultDocument } from './results.mock'; +const mockCheckIndicesPrivileges = jest + .fn() + .mockResolvedValue({ [resultDocument.indexName]: true }); +jest.mock('./privileges', () => ({ + checkIndicesPrivileges: (params: unknown) => mockCheckIndicesPrivileges(params), +})); + describe('postResultsRoute route', () => { describe('indexation', () => { let server: ReturnType; @@ -110,20 +117,16 @@ describe('postResultsRoute route', () => { }); it('should authorize pattern', async () => { - const mockHasPrivileges = - context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; - mockHasPrivileges.mockResolvedValueOnce({ - has_all_requested: true, - } as unknown as SecurityHasPrivilegesResponse); + // const mockHasPrivileges = + // context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; + // mockHasPrivileges.mockResolvedValueOnce({ + // has_all_requested: true, + // } as unknown as SecurityHasPrivilegesResponse); const response = await server.inject(req, requestContextMock.convertContext(context)); - expect(mockHasPrivileges).toHaveBeenCalledWith({ - index: [ - { - names: [resultDocument.indexName], - privileges: ['all', 'read', 'view_index_metadata'], - }, - ], + expect(mockCheckIndicesPrivileges).toHaveBeenCalledWith({ + client: context.core.elasticsearch.client, + indices: [resultDocument.indexName], }); expect(context.core.elasticsearch.client.asInternalUser.index).toHaveBeenCalled(); expect(response.status).toEqual(200); @@ -131,20 +134,17 @@ describe('postResultsRoute route', () => { }); it('should not index unauthorized pattern', async () => { - const mockHasPrivileges = - context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; - mockHasPrivileges.mockResolvedValueOnce({ - has_all_requested: false, - } as unknown as SecurityHasPrivilegesResponse); + // const mockHasPrivileges = + // context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; + // mockHasPrivileges.mockResolvedValueOnce({ + // has_all_requested: false, + // } as unknown as SecurityHasPrivilegesResponse); + mockCheckIndicesPrivileges.mockResolvedValueOnce({ [resultDocument.indexName]: false }); const response = await server.inject(req, requestContextMock.convertContext(context)); - expect(mockHasPrivileges).toHaveBeenCalledWith({ - index: [ - { - names: [resultDocument.indexName], - privileges: ['all', 'read', 'view_index_metadata'], - }, - ], + expect(mockCheckIndicesPrivileges).toHaveBeenCalledWith({ + client: context.core.elasticsearch.client, + indices: [resultDocument.indexName], }); expect(context.core.elasticsearch.client.asInternalUser.index).not.toHaveBeenCalled(); @@ -154,9 +154,11 @@ describe('postResultsRoute route', () => { it('handles pattern authorization error', async () => { const errorMessage = 'Error!'; - const mockHasPrivileges = - context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; - mockHasPrivileges.mockRejectedValueOnce({ message: errorMessage }); + // const mockHasPrivileges = + // context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; + // mockHasPrivileges.mockRejectedValueOnce({ message: errorMessage }); + + mockCheckIndicesPrivileges.mockRejectedValueOnce(Error(errorMessage)); const response = await server.inject(req, requestContextMock.convertContext(context)); expect(response.status).toEqual(500); diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.ts index 2d85113ec4072a..72e25a4e98abbd 100644 --- a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.ts +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/post_results.ts @@ -13,6 +13,7 @@ import { buildRouteValidation } from '../../schemas/common'; import { PostResultBody } from '../../schemas/result'; import { API_DEFAULT_ERROR_MESSAGE } from '../../translations'; import type { DataQualityDashboardRequestHandlerContext } from '../../types'; +import { checkIndicesPrivileges } from './privileges'; import { API_RESULTS_INDEX_NOT_AVAILABLE } from './translations'; export const postResultsRoute = ( @@ -46,20 +47,18 @@ export const postResultsRoute = ( } try { + const { client } = services.core.elasticsearch; + // Confirm user has authorization for the indexName payload const { indexName } = request.body; - const userEsClient = services.core.elasticsearch.client.asCurrentUser; - const privileges = await userEsClient.security.hasPrivileges({ - index: [{ names: [indexName], privileges: ['all', 'read', 'view_index_metadata'] }], - }); - if (!privileges.has_all_requested) { + const hadIndexPrivileges = await checkIndicesPrivileges({ client, indices: [indexName] }); + if (!hadIndexPrivileges[indexName]) { return response.ok({ body: { result: 'noop' } }); } // Index the result const body = { '@timestamp': Date.now(), ...request.body }; - const esClient = services.core.elasticsearch.client.asInternalUser; - const outcome = await esClient.index({ index, body }); + const outcome = await client.asInternalUser.index({ index, body }); return response.ok({ body: { result: outcome.result } }); } catch (err) { diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.test.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.test.ts new file mode 100644 index 00000000000000..2833e3f030fdd5 --- /dev/null +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.test.ts @@ -0,0 +1,129 @@ +/* + * 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 type { SecurityHasPrivilegesResponse } from '@elastic/elasticsearch/lib/api/types'; +import { requestContextMock } from '../../__mocks__/request_context'; +import { checkIndicesPrivileges } from './privileges'; + +// const mockHasPrivileges = +// context.core.elasticsearch.client.asCurrentUser.security.hasPrivileges; +// mockHasPrivileges.mockResolvedValueOnce({ +// has_all_requested: true, +// } as unknown as SecurityHasPrivilegesResponse); + +describe('checkIndicesPrivileges', () => { + const { context } = requestContextMock.createTools(); + const { client } = context.core.elasticsearch; + + beforeEach(() => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValue({ + index: { + index1: { + read: true, + view_index_metadata: true, + manage: true, + monitor: true, + }, + index2: { + read: true, + view_index_metadata: true, + manage: true, + monitor: true, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + }); + + it('should return true if user has required privileges', async () => { + const result = await checkIndicesPrivileges({ client, indices: ['index1', 'index2'] }); + expect(result).toEqual({ index1: true, index2: true }); + }); + + it('should return true if only monitor privileges is missing', async () => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValueOnce({ + index: { + index1: { + read: true, + view_index_metadata: true, + manage: true, + monitor: false, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + const result = await checkIndicesPrivileges({ client, indices: ['index1'] }); + + expect(result).toEqual({ index1: true }); + }); + + it('should return true if only manage privileges is missing', async () => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValueOnce({ + index: { + index1: { + read: true, + view_index_metadata: true, + manage: false, + monitor: true, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + + const result = await checkIndicesPrivileges({ client, indices: ['index1'] }); + + expect(result).toEqual({ index1: true }); + }); + + it('should return false if both manage and monitor privileges is missing', async () => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValueOnce({ + index: { + index1: { + read: true, + view_index_metadata: true, + manage: false, + monitor: false, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + + const result = await checkIndicesPrivileges({ client, indices: ['index1'] }); + + expect(result).toEqual({ index1: false }); + }); + + it('should return false if only read privilege is missing', async () => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValueOnce({ + index: { + index1: { + read: false, + view_index_metadata: true, + manage: true, + monitor: true, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + + const result = await checkIndicesPrivileges({ client, indices: ['index1'] }); + + expect(result).toEqual({ index1: false }); + }); + + it('should return false if only view_index_metadata privilege is missing', async () => { + client.asCurrentUser.security.hasPrivileges.mockResolvedValueOnce({ + index: { + index1: { + read: true, + view_index_metadata: false, + manage: true, + monitor: true, + }, + }, + } as unknown as SecurityHasPrivilegesResponse); + + const result = await checkIndicesPrivileges({ client, indices: ['index1'] }); + + expect(result).toEqual({ index1: false }); + }); +}); diff --git a/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.ts b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.ts new file mode 100644 index 00000000000000..ebc97b34b4af9c --- /dev/null +++ b/x-pack/plugins/ecs_data_quality_dashboard/server/routes/results/privileges.ts @@ -0,0 +1,40 @@ +/* + * 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 type { IScopedClusterClient } from '@kbn/core-elasticsearch-server'; + +/** + * Checks user has the required privileges to do a results check for the given indices. + * In order to be allowed to do a result check user needs: + * `read`, `view_index_metadata` and (`monitor` or `manage`) index privileges. + */ +export const checkIndicesPrivileges = async ({ + client, + indices, +}: { + client: IScopedClusterClient; + indices: string[]; +}) => { + const privileges = await client.asCurrentUser.security.hasPrivileges({ + index: [{ names: indices, privileges: ['read', 'view_index_metadata', 'monitor', 'manage'] }], + }); + + const hasRequiredIndexPrivilege: Record = {}; + Object.entries(privileges.index).forEach(([indexName, indexPrivileges]) => { + if ( + indexPrivileges?.read && + indexPrivileges?.view_index_metadata && + (indexPrivileges?.monitor || indexPrivileges?.manage) + ) { + hasRequiredIndexPrivilege[indexName] = true; + } else { + hasRequiredIndexPrivilege[indexName] = false; + } + }); + + return hasRequiredIndexPrivilege; +};