From 7e719d5e24e3187f1db53e955b5c1ccef6b5d3ac Mon Sep 17 00:00:00 2001 From: Sander Philipse <94373878+sphilipse@users.noreply.github.com> Date: Fri, 28 Feb 2025 13:36:37 +0100 Subject: [PATCH 1/2] [Search] Don't error out on missing connectors permissions (#212622) ## Summary This stops errors on missing connectors permissions when fetching indices, which was preventing users without connectors permissions but with listing and write permissions to indices generally from seeing indices in the Search UI. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7590205f9a9dd79c8a204eb75b69782503d30937) # Conflicts: # x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_unattached_indices.ts # x-pack/solutions/search/plugins/enterprise_search/server/routes/enterprise_search/indices.ts --- .../server/lib/indices/fetch_index.test.ts | 16 ++++--- .../server/lib/indices/fetch_index.ts | 16 ++++--- .../lib/indices/fetch_unattached_indices.ts | 16 +++++-- .../lib/pipelines/get_index_pipeline.ts | 2 +- .../routes/enterprise_search/indices.ts | 43 ++++++++++++++----- 5 files changed, 66 insertions(+), 27 deletions(-) diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts index 3db722c0f58b2..be312f9840429 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts @@ -6,7 +6,7 @@ */ import { ByteSizeValue } from '@kbn/config-schema'; -import { IScopedClusterClient } from '@kbn/core/server'; +import { IScopedClusterClient, Logger } from '@kbn/core/server'; import { fetchConnectorByIndexName } from '@kbn/search-connectors'; @@ -55,6 +55,10 @@ describe('fetchIndex lib function', () => { jest.clearAllMocks(); }); + const logger = { + error: jest.fn(), + } as any as Logger; + const statsResponse = { indices: { index_name: { @@ -110,7 +114,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve(statsResponse)); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).resolves.toEqual(result); }); @@ -134,7 +138,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve(statsResponse)); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).resolves.toEqual({ ...result, connector: { doc: 'doc', service_type: 'some-service-type' }, @@ -213,7 +217,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve(statsResponse)); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).rejects.toEqual(new Error('404')); }); it('should throw a 404 error if the indexStats cannot be fonud', async () => { @@ -230,7 +234,7 @@ describe('fetchIndex lib function', () => { ); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).rejects.toEqual(new Error('404')); }); it('should throw a 404 error if the index stats indices cannot be fonud', async () => { @@ -245,7 +249,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve({})); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).rejects.toEqual(new Error('404')); }); }); diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.ts b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.ts index 41875daad3717..0b0c60039eaa2 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.ts @@ -5,11 +5,10 @@ * 2.0. */ -import { IScopedClusterClient } from '@kbn/core/server'; - -import {} from '../..'; +import { IScopedClusterClient, Logger } from '@kbn/core/server'; import { + Connector, CONNECTORS_JOBS_INDEX, ConnectorSyncJobDocument, fetchConnectorByIndexName, @@ -61,7 +60,8 @@ const hasInProgressSyncs = async ( export const fetchIndex = async ( client: IScopedClusterClient, - index: string + index: string, + logger: Logger ): Promise => { const indexDataResult = await client.asCurrentUser.indices.get({ index }); const indexData = indexDataResult[index]; @@ -73,8 +73,12 @@ export const fetchIndex = async ( throw new Error('404'); } const indexStats = indices[index]; - - const connector = await fetchConnectorByIndexName(client.asCurrentUser, index); + let connector: Connector | undefined; + try { + connector = await fetchConnectorByIndexName(client.asCurrentUser, index); + } catch (error) { + logger.error(`Error fetching connector for index ${index}: ${error}`); + } const hasInProgressSyncsResult = connector ? await hasInProgressSyncs(client, connector.id) : { inProgress: false, pending: false }; diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_unattached_indices.ts b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_unattached_indices.ts index d5547733bcab5..26cc00dff187d 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_unattached_indices.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_unattached_indices.ts @@ -6,7 +6,10 @@ */ import { IScopedClusterClient } from '@kbn/core/server'; -import { fetchConnectors } from '@kbn/search-connectors'; + +import { Connector, fetchConnectors } from '@kbn/search-connectors'; + +import { Crawler } from '../../../common/types/crawler'; import { isNotNullish } from '../../../common/utils/is_not_nullish'; import { fetchCrawlers } from '../crawler/fetch_crawlers'; @@ -23,8 +26,15 @@ export const fetchUnattachedIndices = async ( totalResults: number; }> => { const { indexNames } = await getUnattachedIndexData(client, searchQuery); - const connectors = await fetchConnectors(client.asCurrentUser, indexNames); - const crawlers = await fetchCrawlers(client, indexNames); + let connectors: Connector[] = []; + let crawlers: Crawler[] = []; + try { + connectors = await fetchConnectors(client.asCurrentUser, indexNames); + crawlers = await fetchCrawlers(client, indexNames); + } catch (error) { + connectors = []; + crawlers = []; + } const connectedIndexNames = [ ...connectors.map((con) => con.index_name).filter(isNotNullish), diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/lib/pipelines/get_index_pipeline.ts b/x-pack/solutions/search/plugins/enterprise_search/server/lib/pipelines/get_index_pipeline.ts index 54a916898740e..61c3b678690ec 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/lib/pipelines/get_index_pipeline.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/lib/pipelines/get_index_pipeline.ts @@ -18,7 +18,7 @@ export const getIndexPipelineParameters = async ( // we want to throw the error if getDefaultPipeline() fails so we're not catching it on purpose const [defaultPipeline, connector, customPipelineResp] = await Promise.all([ getDefaultPipeline(client), - fetchConnectorByIndexName(client.asCurrentUser, indexName), + fetchConnectorByIndexName(client.asCurrentUser, indexName).catch(() => null), client.asCurrentUser.ingest .getPipeline({ id: `${indexName}`, diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/routes/enterprise_search/indices.ts b/x-pack/solutions/search/plugins/enterprise_search/server/routes/enterprise_search/indices.ts index 70b46205c5e88..1664c451f5c57 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/routes/enterprise_search/indices.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/routes/enterprise_search/indices.ts @@ -19,6 +19,7 @@ import { deleteConnectorSecret, deleteConnectorById, updateConnectorIndexName, + Connector, } from '@kbn/search-connectors'; import { fetchConnectorByIndexName, @@ -26,6 +27,7 @@ import { } from '@kbn/search-connectors/lib/fetch_connectors'; import { DEFAULT_PIPELINE_NAME } from '../../../common/constants'; +import { Crawler } from '../../../common/types/crawler'; import { ErrorCode } from '../../../common/types/error_codes'; import { AlwaysShowPattern } from '../../../common/types/indices'; @@ -126,8 +128,17 @@ export function registerIndexRoutes({ from, size ); - const connectors = await fetchConnectors(client.asCurrentUser, indexNames); - const crawlers = await fetchCrawlers(client, indexNames); + + let connectors: Connector[] = []; + let crawlers: Crawler[] = []; + // If the user doesn't have permissions, fetchConnectors will error out. We still want to return indices in that case. + try { + connectors = await fetchConnectors(client.asCurrentUser, indexNames); + crawlers = await fetchCrawlers(client, indexNames); + } catch { + connectors = []; + crawlers = []; + } const enrichedIndices = indices.map((index) => ({ ...index, connector: connectors.find((connector) => connector.index_name === index.name), @@ -164,7 +175,7 @@ export function registerIndexRoutes({ const { client } = (await context.core).elasticsearch; try { - const index = await fetchIndex(client, indexName); + const index = await fetchIndex(client, indexName, log); return response.ok({ body: index, headers: { 'content-type': 'application/json' }, @@ -198,8 +209,15 @@ export function registerIndexRoutes({ const { client } = (await context.core).elasticsearch; try { - const crawler = await fetchCrawlerByIndexName(client, indexName); - const connector = await fetchConnectorByIndexName(client.asCurrentUser, indexName); + let connector: Connector | undefined; + let crawler: Crawler | undefined; + // users without permissions to fetch connectors should still see a result + try { + connector = await fetchConnectorByIndexName(client.asCurrentUser, indexName); + crawler = await fetchCrawlerByIndexName(client, indexName); + } catch (error) { + log.error(`Error fetching connector for index ${indexName}: ${error}`); + } if (crawler) { const crawlerRes = await enterpriseSearchRequestHandler.createRequest({ @@ -588,7 +606,15 @@ export function registerIndexRoutes({ }); } - const crawler = await fetchCrawlerByIndexName(client, request.body.index_name); + let connector: Connector | undefined; + let crawler: Crawler | undefined; + // users without permissions to fetch connectors should still be able to create an index + try { + connector = await fetchConnectorByIndexName(client.asCurrentUser, indexName); + crawler = await fetchCrawlerByIndexName(client, request.body.index_name); + } catch (error) { + log.error(`Error fetching connector for index ${indexName}: ${error}`); + } if (crawler) { return createError({ @@ -604,11 +630,6 @@ export function registerIndexRoutes({ }); } - const connector = await fetchConnectorByIndexName( - client.asCurrentUser, - request.body.index_name - ); - if (connector) { return createError({ errorCode: ErrorCode.CONNECTOR_DOCUMENT_ALREADY_EXISTS, From b803ffc72e2784406beeababb1b6102aafbd5e14 Mon Sep 17 00:00:00 2001 From: Sander Philipse Date: Tue, 4 Mar 2025 14:30:18 +0100 Subject: [PATCH 2/2] fix test --- .../enterprise_search/server/lib/indices/fetch_index.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts index be312f9840429..445340f97b169 100644 --- a/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts +++ b/x-pack/solutions/search/plugins/enterprise_search/server/lib/indices/fetch_index.test.ts @@ -163,7 +163,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve(statsResponse)); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).resolves.toEqual({ ...result, crawler: { @@ -198,7 +198,7 @@ describe('fetchIndex lib function', () => { mockClient.asCurrentUser.indices.stats.mockImplementation(() => Promise.resolve(statsResponse)); await expect( - fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name') + fetchIndex(mockClient as unknown as IScopedClusterClient, 'index_name', logger) ).resolves.toEqual({ ...result, connector: { doc: 'doc', service_type: ENTERPRISE_SEARCH_CONNECTOR_CRAWLER_SERVICE_TYPE },