[Search] [Onboarding] Search api key refactor (#199790)

Refactor search api key - get rid from useReduces - simplify logic in provider - create request hooks - fix multiple initialization --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
2025-04-23 17:28:26 -04:00 · 2024-11-27 15:57:46 +01:00 · 2024-11-27 15:57:46 +01:00 · 1f4bfa9c4b
commit 1f4bfa9c4b
parent ba3278b151
10 changed files with 185 additions and 166 deletions
--- 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<ApiKeyFormProps> = ({ 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 (
      <FormInfoField
        label={hasTitle ? titleLocale : undefined}
-        value={displayedApiKey}
+        value={status === Status.showPreviewKey ? apiKey : API_KEY_MASK}
        copyValue={apiKey}
        dataTestSubj="apiKeyFormAPIKey"
        copyValueDataTestSubj="APIKeyButtonCopy"
        actions={[
          <EuiButtonIcon
-            iconType={apiKeyIsVisible ? 'eyeClosed' : 'eye'}
+            iconType={status === Status.showPreviewKey ? 'eyeClosed' : 'eye'}
            color="text"
            onClick={toggleApiKeyVisibility}
            data-test-subj="showAPIKeyButton"
--- 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
@ -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<APIKeyCreationResponse | undefined>({
+    mutationFn: async () => {
+      try {
+        if (!http?.post) {
+          throw new Error('HTTP service is unavailable');
+        }
+
+        return await http.post<APIKeyCreationResponse>(APIRoutes.API_KEYS);
+      } catch (err) {
+        onError(err);
+      }
+    },
+    onSuccess: (receivedApiKey) => {
+      if (receivedApiKey) {
+        onSuccess(receivedApiKey);
+      }
+    },
+  });
+
+  return createApiKey;
+};
--- 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
@ -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<boolean>) => {
+  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;
+};
--- 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,176 +7,109 @@
 * License v3.0 only", or the "Server Side Public License, v 1".
 */

-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 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';

 const API_KEY_STORAGE_KEY = 'searchApiKey';
-const API_KEY_MASK = '•'.repeat(60);
-
-interface ApiKeyState {
-  status: Status;
-  apiKey: string | null;
-}

 interface APIKeyContext {
-  displayedApiKey: string | null;
  apiKey: string | null;
  toggleApiKeyVisibility: () => 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<APIKeyContext>({
-  displayedApiKey: null,
  apiKey: null,
  toggleApiKeyVisibility: () => {},
  updateApiKey: () => {},
  status: Status.uninitialized,
-  apiKeyIsVisible: false,
  initialiseKey: () => {},
 });

 export const SearchApiKeyProvider: React.FC<React.PropsWithChildren> = ({ children }) => {
-  const { http } = useKibana().services;
-  const [state, dispatch] = useReducer(reducer, initialState);
-
+  const isInitialising = useRef(false);
+  const [apiKey, setApiKey] = useState<string | null>(null);
+  const [status, setStatus] = useState<Status>(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 handleShowKeyVisibility = useCallback(() => {
-    dispatch({ type: 'TOGGLE_API_KEY_VISIBILITY' });
+  const toggleApiKeyVisibility = useCallback(() => {
+    setStatus((prevStatus) =>
+      prevStatus === Status.showHiddenKey ? Status.showPreviewKey : Status.showHiddenKey
+    );
  }, []);
-  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<APIKeyCreationResponse | undefined>({
-    mutationFn: async () => {
-      try {
-        if (!http?.post) {
-          throw new Error('HTTP service is unavailable');
-        }
-
-        return await http.post<APIKeyCreationResponse>(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(
          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) {
+        setStatus(Status.showCreateButton);
+      } else if (err.response?.status === 403) {
+        setStatus(Status.showUserPrivilegesError);
+      } else {
+        throw err;
      }
    },
  });
+  const initialiseKey = useCallback(async () => {
+    if (status !== Status.uninitialized || isInitialising.current) {
+      return;
+    }

-  useEffect(() => {
-    const initialiseApiKey = async () => {
-      try {
-        if (state.status === Status.loading) {
-          const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY);
+    isInitialising.current = true;

-          if (storedKey) {
-            const { id, encoded } = JSON.parse(storedKey);
+    try {
+      setStatus(Status.loading);
+      const storedKey = sessionStorage.getItem(API_KEY_STORAGE_KEY);

-            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();
-          }
+      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();
        }
-      } catch (e) {
-        dispatch({ type: 'CLEAR_API_KEY' });
+      } else {
+        await createApiKey();
      }
-    };
+    } catch (e) {
+      setApiKey(null);
+      setStatus(Status.showCreateButton);
+    } finally {
+      isInitialising.current = false;
+    }
+  }, [status, createApiKey, validateApiKey]);

-    initialiseApiKey();
-  }, [state.status, createApiKey, validateApiKey]);
-
-  const value: APIKeyContext = {
-    displayedApiKey: state.status === Status.showPreviewKey ? state.apiKey : API_KEY_MASK,
-    apiKey: state.apiKey,
-    toggleApiKeyVisibility: handleShowKeyVisibility,
-    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 <ApiKeyContext.Provider value={value}>{children}</ApiKeyContext.Provider>;
 };
--- 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);
--- 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({
--- 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 (
    <EuiPanel
--- a/x-pack/plugins/search_indices/public/components/shared/create_index_code_view.tsx
+++ b/x-pack/plugins/search_indices/public/components/shared/create_index_code_view.tsx
@ -44,15 +44,15 @@ export const CreateIndexCodeView = ({
  const selectedCodeExamples = useCreateIndexCodingExamples();

  const elasticsearchUrl = useElasticsearchUrl();
-  const { apiKey, apiKeyIsVisible } = useSearchApiKey();
+  const { apiKey } = useSearchApiKey();

  const codeParams = useMemo(() => {
    return {
      indexName: indexName || undefined,
      elasticsearchURL: elasticsearchUrl,
-      apiKey: apiKeyIsVisible && apiKey ? apiKey : undefined,
+      apiKey: apiKey || undefined,
    };
-  }, [indexName, elasticsearchUrl, apiKeyIsVisible, apiKey]);
+  }, [indexName, elasticsearchUrl, apiKey]);
  const selectedCodeExample = useMemo(() => {
    return selectedCodeExamples[selectedLanguage];
  }, [selectedLanguage, selectedCodeExamples]);
--- 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();
    },
--- 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,15 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
          await pageObjects.svlSearchIndexDetailPage.expectConnectionDetails();
        });

-        it.skip('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.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);
+            await pageObjects.svlApiKeys.expectAPIKeyAvailable();
+            const apiKey = await pageObjects.svlApiKeys.getAPIKeyFromUI();
+            await pageObjects.svlSearchIndexDetailPage.expectAPIKeyToBeVisibleInCodeBlock(apiKey);
+          });
        });

        it('should have quick stats', async () => {
@ -107,11 +110,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],
              },
@ -305,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();
+        });
      });
    });
  });