From d2a9aba03553e83cff407a662983f47d81b84afb Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Tue, 5 Nov 2024 15:04:40 +0100 Subject: [PATCH 01/13] Move mutation queries to hooks --- .../src/hooks/use_create_api_key.ts | 43 ++++++++++++++++ .../src/hooks/use_validate_api_key.ts | 33 ++++++++++++ .../src/providers/search_api_key_provider.tsx | 51 +++++-------------- 3 files changed, 89 insertions(+), 38 deletions(-) create mode 100644 packages/kbn-search-api-keys-components/src/hooks/use_create_api_key.ts create mode 100644 packages/kbn-search-api-keys-components/src/hooks/use_validate_api_key.ts diff --git a/packages/kbn-search-api-keys-components/src/hooks/use_create_api_key.ts b/packages/kbn-search-api-keys-components/src/hooks/use_create_api_key.ts new file mode 100644 index 0000000000000..9c8159b061224 --- /dev/null +++ b/packages/kbn-search-api-keys-components/src/hooks/use_create_api_key.ts @@ -0,0 +1,43 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { useMutation } from '@tanstack/react-query'; +import type { APIKeyCreationResponse } from '@kbn/search-api-keys-server/types'; +import { useKibana } from '@kbn/kibana-react-plugin/public'; +import { APIRoutes } from '../types'; + +export const useCreateApiKey = ({ + onSuccess, + onError, +}: { + onSuccess(key: APIKeyCreationResponse): void; + onError(err: XMLHttpRequest): void; +}) => { + const { http } = useKibana().services; + const { mutateAsync: createApiKey } = useMutation({ + mutationFn: async () => { + try { + if (!http?.post) { + throw new Error('HTTP service is unavailable'); + } + + return await http.post(APIRoutes.API_KEYS); + } catch (err) { + onError(err); + } + }, + onSuccess: (receivedApiKey) => { + if (receivedApiKey) { + onSuccess(receivedApiKey); + } + }, + }); + + return createApiKey; +}; diff --git a/packages/kbn-search-api-keys-components/src/hooks/use_validate_api_key.ts b/packages/kbn-search-api-keys-components/src/hooks/use_validate_api_key.ts new file mode 100644 index 0000000000000..43b75990f032e --- /dev/null +++ b/packages/kbn-search-api-keys-components/src/hooks/use_validate_api_key.ts @@ -0,0 +1,33 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { useMutation } from '@tanstack/react-query'; +import { useKibana } from '@kbn/kibana-react-plugin/public'; +import { APIRoutes } from '../types'; + +export const useValidateApiKey = (): ((id: string) => Promise) => { + const { http } = useKibana().services; + const { mutateAsync: validateApiKey } = useMutation(async (id: string) => { + try { + if (!http?.post) { + throw new Error('HTTP service is unavailable'); + } + + const response = await http.post<{ isValid: boolean }>(APIRoutes.API_KEY_VALIDITY, { + body: JSON.stringify({ id }), + }); + + return response.isValid; + } catch (err) { + return false; + } + }); + + return validateApiKey; +}; diff --git a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx index a3d3ce2aea830..e1710f5a206c1 100644 --- a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx +++ b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx @@ -8,11 +8,9 @@ */ import React, { useCallback, useReducer, createContext, useEffect } from 'react'; -import { useMutation } from '@tanstack/react-query'; -import { useKibana } from '@kbn/kibana-react-plugin/public'; -import type { APIKeyCreationResponse } from '@kbn/search-api-keys-server/types'; -import { APIRoutes } from '../types'; +import { useCreateApiKey } from '../hooks/use_create_api_key'; import { Status } from '../constants'; +import { useValidateApiKey } from '../hooks/use_validate_api_key'; const API_KEY_STORAGE_KEY = 'searchApiKey'; const API_KEY_MASK = '•'.repeat(60); @@ -73,7 +71,6 @@ export const ApiKeyContext = createContext({ }); export const SearchApiKeyProvider: React.FC = ({ children }) => { - const { http } = useKibana().services; const [state, dispatch] = useReducer(reducer, initialState); const updateApiKey = useCallback(({ id, encoded }: { id: string; encoded: string }) => { @@ -86,39 +83,8 @@ export const SearchApiKeyProvider: React.FC = ({ childr const initialiseKey = useCallback(() => { dispatch({ type: 'SET_STATUS', status: Status.loading }); }, []); - const { mutateAsync: validateApiKey } = useMutation(async (id: string) => { - try { - if (!http?.post) { - throw new Error('HTTP service is unavailable'); - } - - const response = await http.post<{ isValid: boolean }>(APIRoutes.API_KEY_VALIDITY, { - body: JSON.stringify({ id }), - }); - - return response.isValid; - } catch (err) { - return false; - } - }); - const { mutateAsync: createApiKey } = useMutation({ - mutationFn: async () => { - try { - if (!http?.post) { - throw new Error('HTTP service is unavailable'); - } - - return await http.post(APIRoutes.API_KEYS); - } catch (err) { - if (err.response?.status === 400) { - dispatch({ type: 'SET_STATUS', status: Status.showCreateButton }); - } else if (err.response?.status === 403) { - dispatch({ type: 'SET_STATUS', status: Status.showUserPrivilegesError }); - } else { - throw err; - } - } - }, + const validateApiKey = useValidateApiKey(); + const createApiKey = useCreateApiKey({ onSuccess: (receivedApiKey) => { if (receivedApiKey) { sessionStorage.setItem( @@ -132,6 +98,15 @@ export const SearchApiKeyProvider: React.FC = ({ childr }); } }, + onError: (err) => { + if (err.response?.status === 400) { + dispatch({ type: 'SET_STATUS', status: Status.showCreateButton }); + } else if (err.response?.status === 403) { + dispatch({ type: 'SET_STATUS', status: Status.showUserPrivilegesError }); + } else { + throw err; + } + }, }); useEffect(() => { From b058ff30305fa2b829e4a57b2613687375627ca0 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Fri, 8 Nov 2024 22:36:22 +0100 Subject: [PATCH 02/13] Refactor initialise key logic --- .../src/providers/search_api_key_provider.tsx | 63 +++++++++---------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx index e1710f5a206c1..ecbf59ebe45bb 100644 --- a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx +++ b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import React, { useCallback, useReducer, createContext, useEffect } from 'react'; +import React, { useCallback, useReducer, createContext } from 'react'; import { useCreateApiKey } from '../hooks/use_create_api_key'; import { Status } from '../constants'; import { useValidateApiKey } from '../hooks/use_validate_api_key'; @@ -77,12 +77,9 @@ export const SearchApiKeyProvider: React.FC = ({ childr sessionStorage.setItem(API_KEY_STORAGE_KEY, JSON.stringify({ id, encoded })); dispatch({ type: 'SET_API_KEY', apiKey: encoded, status: Status.showHiddenKey }); }, []); - const handleShowKeyVisibility = useCallback(() => { + const toggleApiKeyVisibility = useCallback(() => { dispatch({ type: 'TOGGLE_API_KEY_VISIBILITY' }); }, []); - const initialiseKey = useCallback(() => { - dispatch({ type: 'SET_STATUS', status: Status.loading }); - }, []); const validateApiKey = useValidateApiKey(); const createApiKey = useCreateApiKey({ onSuccess: (receivedApiKey) => { @@ -108,45 +105,41 @@ export const SearchApiKeyProvider: React.FC = ({ childr } }, }); - - useEffect(() => { - const initialiseApiKey = async () => { - try { - if (state.status === Status.loading) { - const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); - - if (storedKey) { - const { id, encoded } = JSON.parse(storedKey); - - if (await validateApiKey(id)) { - dispatch({ - type: 'SET_API_KEY', - apiKey: encoded, - status: Status.showHiddenKey, - }); - } else { - sessionStorage.removeItem(API_KEY_STORAGE_KEY); - dispatch({ - type: 'CLEAR_API_KEY', - }); - await createApiKey(); - } + const initialiseKey = useCallback(async () => { + try { + if (state.status === Status.uninitialized) { + dispatch({ type: 'SET_STATUS', status: Status.loading }); + const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); + + if (storedKey) { + const { id, encoded } = JSON.parse(storedKey); + + if (await validateApiKey(id)) { + dispatch({ + type: 'SET_API_KEY', + apiKey: encoded, + status: Status.showHiddenKey, + }); } else { + sessionStorage.removeItem(API_KEY_STORAGE_KEY); + dispatch({ + type: 'CLEAR_API_KEY', + }); await createApiKey(); } + } else { + await createApiKey(); } - } catch (e) { - dispatch({ type: 'CLEAR_API_KEY' }); } - }; - - initialiseApiKey(); - }, [state.status, createApiKey, validateApiKey]); + } catch (e) { + dispatch({ type: 'CLEAR_API_KEY' }); + } + }, [createApiKey, state.status, validateApiKey]); const value: APIKeyContext = { displayedApiKey: state.status === Status.showPreviewKey ? state.apiKey : API_KEY_MASK, apiKey: state.apiKey, - toggleApiKeyVisibility: handleShowKeyVisibility, + toggleApiKeyVisibility, updateApiKey, status: state.status, apiKeyIsVisible: state.status === Status.showPreviewKey, From b14197550fc15aa2967769667cd9d48625f41100 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Fri, 8 Nov 2024 23:34:54 +0100 Subject: [PATCH 03/13] Move values out of provider to a more suitable place --- .../src/components/api_key_form.tsx | 11 +- .../src/providers/search_api_key_provider.tsx | 104 ++++++------------ .../components/start/create_index_code.tsx | 6 +- 3 files changed, 40 insertions(+), 81 deletions(-) diff --git a/packages/kbn-search-api-keys-components/src/components/api_key_form.tsx b/packages/kbn-search-api-keys-components/src/components/api_key_form.tsx index 0a94f3e336897..e7d4d2808433c 100644 --- a/packages/kbn-search-api-keys-components/src/components/api_key_form.tsx +++ b/packages/kbn-search-api-keys-components/src/components/api_key_form.tsx @@ -23,30 +23,31 @@ import { ApiKeyFlyoutWrapper } from './api_key_flyout_wrapper'; import { useSearchApiKey } from '../hooks/use_search_api_key'; import { Status } from '../constants'; +const API_KEY_MASK = '•'.repeat(60); + interface ApiKeyFormProps { hasTitle?: boolean; } export const ApiKeyForm: React.FC = ({ hasTitle = true }) => { const [showFlyout, setShowFlyout] = useState(false); - const { apiKey, status, updateApiKey, toggleApiKeyVisibility, displayedApiKey, apiKeyIsVisible } = - useSearchApiKey(); + const { apiKey, status, updateApiKey, toggleApiKeyVisibility } = useSearchApiKey(); const titleLocale = i18n.translate('searchApiKeysComponents.apiKeyForm.title', { defaultMessage: 'API Key', }); - if (apiKey && displayedApiKey) { + if (apiKey) { return ( void; updateApiKey: ({ id, encoded }: { id: string; encoded: string }) => void; status: Status; - apiKeyIsVisible: boolean; initialiseKey: () => void; } -type Action = - | { type: 'SET_API_KEY'; apiKey: string; status: Status } - | { type: 'SET_STATUS'; status: Status } - | { type: 'CLEAR_API_KEY' } - | { type: 'TOGGLE_API_KEY_VISIBILITY' }; - -const initialState: ApiKeyState = { - apiKey: null, - status: Status.uninitialized, -}; - -const reducer = (state: ApiKeyState, action: Action): ApiKeyState => { - switch (action.type) { - case 'SET_API_KEY': - return { ...state, apiKey: action.apiKey, status: action.status }; - case 'SET_STATUS': - return { ...state, status: action.status }; - case 'TOGGLE_API_KEY_VISIBILITY': - return { - ...state, - status: - state.status === Status.showHiddenKey ? Status.showPreviewKey : Status.showHiddenKey, - }; - case 'CLEAR_API_KEY': - return { ...state, apiKey: null, status: Status.showCreateButton }; - default: - return state; - } -}; - export const ApiKeyContext = createContext({ - displayedApiKey: null, apiKey: null, toggleApiKeyVisibility: () => {}, updateApiKey: () => {}, status: Status.uninitialized, - apiKeyIsVisible: false, initialiseKey: () => {}, }); export const SearchApiKeyProvider: React.FC = ({ children }) => { - const [state, dispatch] = useReducer(reducer, initialState); - + const [apiKey, setApiKey] = useState(null); + const [status, setStatus] = useState(Status.uninitialized); const updateApiKey = useCallback(({ id, encoded }: { id: string; encoded: string }) => { sessionStorage.setItem(API_KEY_STORAGE_KEY, JSON.stringify({ id, encoded })); - dispatch({ type: 'SET_API_KEY', apiKey: encoded, status: Status.showHiddenKey }); + setApiKey(encoded); + setStatus(Status.showHiddenKey); }, []); const toggleApiKeyVisibility = useCallback(() => { - dispatch({ type: 'TOGGLE_API_KEY_VISIBILITY' }); + setStatus((prevStatus) => + prevStatus === Status.showHiddenKey ? Status.showPreviewKey : Status.showHiddenKey + ); }, []); const validateApiKey = useValidateApiKey(); const createApiKey = useCreateApiKey({ @@ -88,18 +51,15 @@ export const SearchApiKeyProvider: React.FC = ({ childr API_KEY_STORAGE_KEY, JSON.stringify({ id: receivedApiKey.id, encoded: receivedApiKey.encoded }) ); - dispatch({ - type: 'SET_API_KEY', - apiKey: receivedApiKey.encoded, - status: Status.showHiddenKey, - }); + setApiKey(receivedApiKey.encoded); + setStatus(Status.showHiddenKey); } }, onError: (err) => { if (err.response?.status === 400) { - dispatch({ type: 'SET_STATUS', status: Status.showCreateButton }); + setStatus(Status.showCreateButton); } else if (err.response?.status === 403) { - dispatch({ type: 'SET_STATUS', status: Status.showUserPrivilegesError }); + setStatus(Status.showUserPrivilegesError); } else { throw err; } @@ -107,24 +67,20 @@ export const SearchApiKeyProvider: React.FC = ({ childr }); const initialiseKey = useCallback(async () => { try { - if (state.status === Status.uninitialized) { - dispatch({ type: 'SET_STATUS', status: Status.loading }); + if (status === Status.uninitialized) { + setStatus(Status.loading); const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); if (storedKey) { const { id, encoded } = JSON.parse(storedKey); if (await validateApiKey(id)) { - dispatch({ - type: 'SET_API_KEY', - apiKey: encoded, - status: Status.showHiddenKey, - }); + setApiKey(encoded); + setStatus(Status.showHiddenKey); } else { sessionStorage.removeItem(API_KEY_STORAGE_KEY); - dispatch({ - type: 'CLEAR_API_KEY', - }); + setApiKey(null); + setStatus(Status.showCreateButton); await createApiKey(); } } else { @@ -132,19 +88,21 @@ export const SearchApiKeyProvider: React.FC = ({ childr } } } catch (e) { - dispatch({ type: 'CLEAR_API_KEY' }); + setApiKey(null); + setStatus(Status.showCreateButton); } - }, [createApiKey, state.status, validateApiKey]); + }, [createApiKey, status, validateApiKey]); - const value: APIKeyContext = { - displayedApiKey: state.status === Status.showPreviewKey ? state.apiKey : API_KEY_MASK, - apiKey: state.apiKey, - toggleApiKeyVisibility, - updateApiKey, - status: state.status, - apiKeyIsVisible: state.status === Status.showPreviewKey, - initialiseKey, - }; + const value: APIKeyContext = useMemo( + () => ({ + apiKey, + toggleApiKeyVisibility, + updateApiKey, + status, + initialiseKey, + }), + [apiKey, status, toggleApiKeyVisibility, updateApiKey, initialiseKey] + ); return {children}; }; diff --git a/x-pack/plugins/search_indices/public/components/start/create_index_code.tsx b/x-pack/plugins/search_indices/public/components/start/create_index_code.tsx index fadfe1c7dcb90..cc6cb1bf48c5f 100644 --- a/x-pack/plugins/search_indices/public/components/start/create_index_code.tsx +++ b/x-pack/plugins/search_indices/public/components/start/create_index_code.tsx @@ -50,15 +50,15 @@ export const CreateIndexCodeView = ({ [usageTracker, changeCodingLanguage] ); const elasticsearchUrl = useElasticsearchUrl(); - const { apiKey, apiKeyIsVisible } = useSearchApiKey(); + const { apiKey } = useSearchApiKey(); const codeParams = useMemo(() => { return { indexName: createIndexForm.indexName || undefined, elasticsearchURL: elasticsearchUrl, - apiKey: apiKeyIsVisible && apiKey ? apiKey : undefined, + apiKey: apiKey || undefined, }; - }, [createIndexForm.indexName, elasticsearchUrl, apiKeyIsVisible, apiKey]); + }, [createIndexForm.indexName, elasticsearchUrl, apiKey]); const selectedCodeExample = useMemo(() => { return selectedCodeExamples[selectedLanguage]; }, [selectedLanguage, selectedCodeExamples]); From 680ddb33ba740f4b0bbb180404528496336536d6 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Sat, 9 Nov 2024 00:22:22 +0100 Subject: [PATCH 04/13] Call init only once --- .../src/hooks/use_search_api_key.ts | 8 +++-- .../src/providers/search_api_key_provider.tsx | 30 ++++++++++--------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts b/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts index 9d0bd4826f9de..16ad402c85a41 100644 --- a/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts +++ b/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts @@ -7,13 +7,17 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { useContext, useEffect } from 'react'; +import { useContext, useEffect, useRef } from 'react'; import { ApiKeyContext } from '../providers/search_api_key_provider'; export const useSearchApiKey = () => { + const isInitRef = useRef(false); const { initialiseKey, ...context } = useContext(ApiKeyContext); useEffect(() => { - initialiseKey(); + if (!isInitRef.current) { + isInitRef.current = true; + initialiseKey(); + } }, [initialiseKey]); return context; }; diff --git a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx index 0864f548461fc..301ac61ec32c3 100644 --- a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx +++ b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx @@ -66,26 +66,28 @@ export const SearchApiKeyProvider: React.FC = ({ childr }, }); const initialiseKey = useCallback(async () => { + if (status !== Status.uninitialized) { + return; + } + try { - if (status === Status.uninitialized) { - setStatus(Status.loading); - const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); + setStatus(Status.loading); + const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); - if (storedKey) { - const { id, encoded } = JSON.parse(storedKey); + if (storedKey) { + const { id, encoded } = JSON.parse(storedKey); - if (await validateApiKey(id)) { - setApiKey(encoded); - setStatus(Status.showHiddenKey); - } else { - sessionStorage.removeItem(API_KEY_STORAGE_KEY); - setApiKey(null); - setStatus(Status.showCreateButton); - await createApiKey(); - } + if (await validateApiKey(id)) { + setApiKey(encoded); + setStatus(Status.showHiddenKey); } else { + sessionStorage.removeItem(API_KEY_STORAGE_KEY); + setApiKey(null); + setStatus(Status.showCreateButton); await createApiKey(); } + } else { + await createApiKey(); } } catch (e) { setApiKey(null); From e6160afe600f45b5c9ac6b61f34c69a479a78664 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Tue, 12 Nov 2024 16:33:11 +0100 Subject: [PATCH 05/13] Fix code types --- .../index_documents/add_documents_code_example.tsx | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/search_indices/public/components/index_documents/add_documents_code_example.tsx b/x-pack/plugins/search_indices/public/components/index_documents/add_documents_code_example.tsx index cdd773f4e6a81..e84ca5bd47be0 100644 --- a/x-pack/plugins/search_indices/public/components/index_documents/add_documents_code_example.tsx +++ b/x-pack/plugins/search_indices/public/components/index_documents/add_documents_code_example.tsx @@ -60,7 +60,7 @@ export const AddDocumentsCodeExample = ({ generateSampleDocument(codeSampleMappings, `Example text ${num}`) ); }, [codeSampleMappings]); - const { apiKey, apiKeyIsVisible } = useSearchApiKey(); + const { apiKey } = useSearchApiKey(); const codeParams: IngestCodeSnippetParameters = useMemo(() => { return { indexName, @@ -68,17 +68,9 @@ export const AddDocumentsCodeExample = ({ sampleDocuments, indexHasMappings, mappingProperties: codeSampleMappings, - apiKey: apiKeyIsVisible && apiKey ? apiKey : undefined, + apiKey: apiKey || undefined, }; - }, [ - indexName, - elasticsearchUrl, - sampleDocuments, - codeSampleMappings, - indexHasMappings, - apiKeyIsVisible, - apiKey, - ]); + }, [indexName, elasticsearchUrl, sampleDocuments, codeSampleMappings, indexHasMappings, apiKey]); return ( Date: Wed, 13 Nov 2024 20:00:43 +0100 Subject: [PATCH 06/13] Prevent multiple call for creatApiKey --- .../src/hooks/use_search_api_key.ts | 8 ++------ .../src/providers/search_api_key_provider.tsx | 11 ++++++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts b/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts index 16ad402c85a41..9d0bd4826f9de 100644 --- a/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts +++ b/packages/kbn-search-api-keys-components/src/hooks/use_search_api_key.ts @@ -7,17 +7,13 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { useContext, useEffect, useRef } from 'react'; +import { useContext, useEffect } from 'react'; import { ApiKeyContext } from '../providers/search_api_key_provider'; export const useSearchApiKey = () => { - const isInitRef = useRef(false); const { initialiseKey, ...context } = useContext(ApiKeyContext); useEffect(() => { - if (!isInitRef.current) { - isInitRef.current = true; - initialiseKey(); - } + initialiseKey(); }, [initialiseKey]); return context; }; diff --git a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx index 301ac61ec32c3..04cf9655b26ea 100644 --- a/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx +++ b/packages/kbn-search-api-keys-components/src/providers/search_api_key_provider.tsx @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import React, { useCallback, createContext, useState, useMemo } from 'react'; +import React, { useCallback, createContext, useState, useMemo, useRef } from 'react'; import { useCreateApiKey } from '../hooks/use_create_api_key'; import { Status } from '../constants'; import { useValidateApiKey } from '../hooks/use_validate_api_key'; @@ -31,6 +31,7 @@ export const ApiKeyContext = createContext({ }); export const SearchApiKeyProvider: React.FC = ({ children }) => { + const isInitialising = useRef(false); const [apiKey, setApiKey] = useState(null); const [status, setStatus] = useState(Status.uninitialized); const updateApiKey = useCallback(({ id, encoded }: { id: string; encoded: string }) => { @@ -66,10 +67,12 @@ export const SearchApiKeyProvider: React.FC = ({ childr }, }); const initialiseKey = useCallback(async () => { - if (status !== Status.uninitialized) { + if (status !== Status.uninitialized || isInitialising.current) { return; } + isInitialising.current = true; + try { setStatus(Status.loading); const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY); @@ -92,8 +95,10 @@ export const SearchApiKeyProvider: React.FC = ({ childr } catch (e) { setApiKey(null); setStatus(Status.showCreateButton); + } finally { + isInitialising.current = false; } - }, [createApiKey, status, validateApiKey]); + }, [status, createApiKey, validateApiKey]); const value: APIKeyContext = useMemo( () => ({ From 40eb96259520f19caf8e88592c308f5f296d05c1 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Thu, 14 Nov 2024 14:34:50 +0100 Subject: [PATCH 07/13] Remove skip label from api test --- .../functional/test_suites/search/search_index_detail.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts index 0070ce7e2cb43..04e409eef48a4 100644 --- a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts +++ b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts @@ -54,7 +54,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await pageObjects.svlSearchIndexDetailPage.expectConnectionDetails(); }); - it.skip('should show api key', async () => { + it('should show api key', async () => { await pageObjects.svlApiKeys.deleteAPIKeys(); await svlSearchNavigation.navigateToIndexDetailPage(indexName); await pageObjects.svlApiKeys.expectAPIKeyAvailable(); From 7c9470d971f5f59f9455f0920136384072a52b3e Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Thu, 14 Nov 2024 15:10:23 +0100 Subject: [PATCH 08/13] Remove skip label from flaky test and add fix for it --- .../functional/test_suites/search/search_index_detail.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts index 04e409eef48a4..96ca1da696b1f 100644 --- a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts +++ b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts @@ -107,11 +107,11 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await pageObjects.embeddedConsole.clickEmbeddedConsoleControlBar(); }); - // FLAKY: https://github.com/elastic/kibana/issues/197144 - describe.skip('With data', () => { + describe('With data', () => { before(async () => { await es.index({ index: indexName, + refresh: true, body: { my_field: [1, 0, 1], }, From 3f5e7c5540ca50f64844ddd07a80ee1d9f91179d Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Thu, 14 Nov 2024 18:52:46 +0100 Subject: [PATCH 09/13] Wrap api test in its own describe --- .../test_suites/search/search_index_detail.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts index 96ca1da696b1f..0152ec8237269 100644 --- a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts +++ b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts @@ -54,12 +54,14 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await pageObjects.svlSearchIndexDetailPage.expectConnectionDetails(); }); - it('should show api key', async () => { - await pageObjects.svlApiKeys.deleteAPIKeys(); - await svlSearchNavigation.navigateToIndexDetailPage(indexName); - await pageObjects.svlApiKeys.expectAPIKeyAvailable(); - const apiKey = await pageObjects.svlApiKeys.getAPIKeyFromUI(); - await pageObjects.svlSearchIndexDetailPage.expectAPIKeyToBeVisibleInCodeBlock(apiKey); + describe('API key details', () => { + it('should show api key', async () => { + await pageObjects.svlApiKeys.deleteAPIKeys(); + await svlSearchNavigation.navigateToIndexDetailPage(indexName); + await pageObjects.svlApiKeys.expectAPIKeyAvailable(); + const apiKey = await pageObjects.svlApiKeys.getAPIKeyFromUI(); + await pageObjects.svlSearchIndexDetailPage.expectAPIKeyToBeVisibleInCodeBlock(apiKey); + }); }); it('should have quick stats', async () => { From 16917a5cbc69676b15f37789c498359c3b54e475 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Sun, 24 Nov 2024 13:03:21 +0100 Subject: [PATCH 10/13] Skip flaky --- .../functional/test_suites/search/search_index_detail.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts index f351a0acde52c..650e368b50d84 100644 --- a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts +++ b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts @@ -54,7 +54,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await pageObjects.svlSearchIndexDetailPage.expectConnectionDetails(); }); - describe('API key details', () => { + describe.skip('API key details', () => { + // Flaky test related with deleting API keys it('should show api key', async () => { await pageObjects.svlApiKeys.deleteAPIKeys(); await svlSearchNavigation.navigateToIndexDetailPage(indexName); From bc89f68ab728f4c123d61a4b3181d4a2185c90cb Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Wed, 27 Nov 2024 13:20:41 +0100 Subject: [PATCH 11/13] Add check privileges more than viewer role --- packages/kbn-search-api-keys-server/src/lib/privileges.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/kbn-search-api-keys-server/src/lib/privileges.ts b/packages/kbn-search-api-keys-server/src/lib/privileges.ts index fc5ad1f896746..9507e92b02eea 100644 --- a/packages/kbn-search-api-keys-server/src/lib/privileges.ts +++ b/packages/kbn-search-api-keys-server/src/lib/privileges.ts @@ -19,9 +19,15 @@ export async function fetchUserStartPrivileges( // and can also have permissions for index monitoring const securityCheck = await client.security.hasPrivileges({ cluster: ['manage'], + index: [ + { + names: ['*'], + privileges: ['read', 'write'], + }, + ], }); - return securityCheck?.cluster?.manage ?? false; + return securityCheck.has_all_requested ?? false; } catch (e) { logger.error(`Error checking user privileges for search API Keys`); logger.error(e); From f4e62186c0e198d2b4afd54e9408a89c0d2bf64d Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Wed, 27 Nov 2024 13:33:14 +0100 Subject: [PATCH 12/13] Check firstly if user has privileges to create api key --- .../src/routes/routes.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/kbn-search-api-keys-server/src/routes/routes.ts b/packages/kbn-search-api-keys-server/src/routes/routes.ts index 77a08644f34a5..0e4eb81309c87 100644 --- a/packages/kbn-search-api-keys-server/src/routes/routes.ts +++ b/packages/kbn-search-api-keys-server/src/routes/routes.ts @@ -71,14 +71,6 @@ export function registerSearchApiKeysRoutes(router: IRouter, logger: Logger) { try { const core = await context.core; const client = core.elasticsearch.client.asCurrentUser; - const clusterHasApiKeys = await fetchClusterHasApiKeys(client, logger); - - if (clusterHasApiKeys) { - return response.customError({ - body: { message: 'Project already has API keys' }, - statusCode: 400, - }); - } const canCreateApiKeys = await fetchUserStartPrivileges(client, logger); @@ -89,6 +81,15 @@ export function registerSearchApiKeysRoutes(router: IRouter, logger: Logger) { }); } + const clusterHasApiKeys = await fetchClusterHasApiKeys(client, logger); + + if (clusterHasApiKeys) { + return response.customError({ + body: { message: 'Project already has API keys' }, + statusCode: 400, + }); + } + const apiKey = await createAPIKey(API_KEY_NAME, client, logger); return response.ok({ From 7a479729dfcdfcfe03f96f206945c1d8b4908ac3 Mon Sep 17 00:00:00 2001 From: Yan Savitski Date: Wed, 27 Nov 2024 13:58:57 +0100 Subject: [PATCH 13/13] Add test to show no privileges --- .../test_serverless/functional/page_objects/svl_api_keys.ts | 4 ++++ .../functional/test_suites/search/search_index_detail.ts | 3 +++ 2 files changed, 7 insertions(+) diff --git a/x-pack/test_serverless/functional/page_objects/svl_api_keys.ts b/x-pack/test_serverless/functional/page_objects/svl_api_keys.ts index 217a17263b1f8..2228967f29155 100644 --- a/x-pack/test_serverless/functional/page_objects/svl_api_keys.ts +++ b/x-pack/test_serverless/functional/page_objects/svl_api_keys.ts @@ -44,6 +44,10 @@ export function SvlApiKeysProvider({ getService, getPageObjects }: FtrProviderCo expect(sessionStorageKey.encoded).to.eql(apiKey); }, + async expectAPIKeyNoPrivileges() { + await testSubjects.existOrFail('apiKeyFormNoUserPrivileges'); + }, + async getAPIKeyFromSessionStorage() { return getAPIKeyFromSessionStorage(); }, diff --git a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts index 650e368b50d84..097da2201e1e7 100644 --- a/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts +++ b/x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts @@ -308,6 +308,9 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await pageObjects.svlSearchIndexDetailPage.expectDeleteIndexButtonExistsInMoreOptions(); await pageObjects.svlSearchIndexDetailPage.expectDeleteIndexButtonToBeDisabled(); }); + it('show no privileges to create api key', async () => { + await pageObjects.svlApiKeys.expectAPIKeyNoPrivileges(); + }); }); }); });