[8.16] [Security GenAI] When a "global" Knowledge Base entry is updated to "private", a duplicate "private" entry gets created and the global entry remains unchanged (#197157) (#197516) (#197918)

# Backport This will backport the following commits from `main` to `8.16`: - [[Security GenAI] When a "global" Knowledge Base entry is updated to "private", a duplicate "private" entry gets created and the global entry remains unchanged (#197157) (#197516)](https://github.com/elastic/kibana/pull/197516)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>
2025-04-24 01:38:56 -04:00 · 2024-10-26 11:26:30 +11:00 · 2024-10-26 11:26:30 +11:00 · f25e599d25
commit f25e599d25
parent b33a0c4a5a
5 changed files with 280 additions and 36 deletions
--- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/document_entry_editor.tsx
+++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/document_entry_editor.tsx
@ -14,18 +14,28 @@ import {
  EuiIcon,
  EuiText,
 } from '@elastic/eui';
-import React, { useCallback } from 'react';
+import React, { useCallback, useMemo } from 'react';
 import { DocumentEntry } from '@kbn/elastic-assistant-common';
 import * as i18n from './translations';
+import { isGlobalEntry } from './helpers';

 interface Props {
  entry?: DocumentEntry;
+  originalEntry?: DocumentEntry;
  setEntry: React.Dispatch<React.SetStateAction<Partial<DocumentEntry>>>;
  hasManageGlobalKnowledgeBase: boolean;
 }

 export const DocumentEntryEditor: React.FC<Props> = React.memo(
-  ({ entry, setEntry, hasManageGlobalKnowledgeBase }) => {
+  ({ entry, setEntry, hasManageGlobalKnowledgeBase, originalEntry }) => {
+    const privateUsers = useMemo(() => {
+      const originalUsers = originalEntry?.users;
+      if (originalEntry && !isGlobalEntry(originalEntry)) {
+        return originalUsers;
+      }
+      return undefined;
+    }, [originalEntry]);
+
    // Name
    const setName = useCallback(
      (e: React.ChangeEvent<HTMLInputElement>) =>
@ -38,12 +48,13 @@ export const DocumentEntryEditor: React.FC<Props> = React.memo(
      (value: string) =>
        setEntry((prevEntry) => ({
          ...prevEntry,
-          users: value === i18n.SHARING_GLOBAL_OPTION_LABEL ? [] : undefined,
+          users: value === i18n.SHARING_GLOBAL_OPTION_LABEL ? [] : privateUsers,
        })),
-      [setEntry]
+      [privateUsers, setEntry]
    );
    const sharingOptions = [
      {
+        'data-test-subj': 'sharing-private-option',
        value: i18n.SHARING_PRIVATE_OPTION_LABEL,
        inputDisplay: (
          <EuiText size={'s'}>
@ -57,6 +68,7 @@ export const DocumentEntryEditor: React.FC<Props> = React.memo(
        ),
      },
      {
+        'data-test-subj': 'sharing-global-option',
        value: i18n.SHARING_GLOBAL_OPTION_LABEL,
        inputDisplay: (
          <EuiText size={'s'}>
@ -111,6 +123,7 @@ export const DocumentEntryEditor: React.FC<Props> = React.memo(
          fullWidth
        >
          <EuiSuperSelect
+            data-test-subj="sharing-select"
            options={sharingOptions}
            valueOfSelected={selectedSharingOption}
            onChange={setSharingOptions}
--- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index.test.tsx
+++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index.test.tsx
@ -35,6 +35,7 @@ const mockContext = {
  selectedSettingsTab: null,
  assistantAvailability: {
    isAssistantEnabled: true,
+    hasManageGlobalKnowledgeBase: true,
  },
 };
 jest.mock('../../assistant_context');
@ -64,23 +65,67 @@ const wrapper = (props: { children: React.ReactNode }) => (
  </I18nProvider>
 );
 describe('KnowledgeBaseSettingsManagement', () => {
+  const mockCreateEntry = jest.fn();
+  const mockUpdateEntry = jest.fn();
+  const mockDeleteEntry = jest.fn();
  const mockData = [
-    { id: '1', name: 'Test Entry 1', type: 'document', kbResource: 'user', users: [{ id: 'hi' }] },
+    {
+      id: '1',
+      createdAt: '2024-10-21T18:54:14.773Z',
+      createdBy: 'u_user_id_1',
+      updatedAt: '2024-10-23T17:33:15.933Z',
+      updatedBy: 'u_user_id_1',
+      users: [{ name: 'Test User 1' }],
+      name: 'Test Entry 1',
+      namespace: 'default',
+      type: 'document',
+      kbResource: 'user',
+      source: 'user',
+      text: 'Very nice text',
+    },
    {
      id: '2',
-      name: 'Test Entry 2',
-      type: 'index',
-      kbResource: 'global',
+      createdAt: '2024-10-25T09:55:56.596Z',
+      createdBy: 'u_user_id_2',
+      updatedAt: '2024-10-25T09:55:56.596Z',
+      updatedBy: 'u_user_id_2',
      users: [],
-      index: 'missing-index',
+      name: 'Test Entry 2',
+      namespace: 'default',
+      type: 'index',
+      index: 'index-1',
+      field: 'semantic_field1',
+      description: 'Test description',
+      queryDescription: 'Test query instruction',
    },
    {
      id: '3',
+      createdAt: '2024-10-25T09:55:56.596Z',
+      createdBy: 'u_user_id_1',
+      updatedAt: '2024-10-25T09:55:56.596Z',
+      updatedBy: 'u_user_id_1',
+      users: [{ name: 'Test User 1' }],
      name: 'Test Entry 3',
+      namespace: 'default',
      type: 'index',
-      kbResource: 'private',
-      users: [{ id: 'fake-user' }],
      index: 'index-2',
+      field: 'semantic_field2',
+      description: 'Test description',
+      queryDescription: 'Test query instruction',
+    },
+    {
+      id: '4',
+      createdAt: '2024-10-21T18:54:14.773Z',
+      createdBy: 'u_user_id_3',
+      updatedAt: '2024-10-23T17:33:15.933Z',
+      updatedBy: 'u_user_id_3',
+      users: [],
+      name: 'Test Entry 4',
+      namespace: 'default',
+      type: 'document',
+      kbResource: 'user',
+      source: 'user',
+      text: 'Very nice text',
    },
  ];

@ -114,15 +159,15 @@ describe('KnowledgeBaseSettingsManagement', () => {
      closeFlyout: jest.fn(),
    });
    (useCreateKnowledgeBaseEntry as jest.Mock).mockReturnValue({
-      mutateAsync: jest.fn(),
+      mutateAsync: mockCreateEntry,
      isLoading: false,
    });
    (useUpdateKnowledgeBaseEntries as jest.Mock).mockReturnValue({
-      mutateAsync: jest.fn(),
+      mutateAsync: mockUpdateEntry,
      isLoading: false,
    });
    (useDeleteKnowledgeBaseEntries as jest.Mock).mockReturnValue({
-      mutateAsync: jest.fn(),
+      mutateAsync: mockDeleteEntry,
      isLoading: false,
    });
  });
@ -258,6 +303,124 @@ describe('KnowledgeBaseSettingsManagement', () => {
    expect(screen.queryByTestId('delete-entry-confirmation')).not.toBeInTheDocument();
  });

+  it('does not create a duplicate document entry when switching sharing option twice', async () => {
+    (useFlyoutModalVisibility as jest.Mock).mockReturnValue({
+      isFlyoutOpen: true,
+      openFlyout: jest.fn(),
+      closeFlyout: jest.fn(),
+    });
+    render(<KnowledgeBaseSettingsManagement dataViews={mockDataViews} />, {
+      wrapper,
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getAllByTestId('edit-button')[0]);
+    });
+    expect(screen.getByTestId('flyout')).toBeVisible();
+
+    await waitFor(() => {
+      expect(screen.getByText('Edit document entry')).toBeInTheDocument();
+    });
+
+    const updatedName = 'New Entry Name';
+    await waitFor(() => {
+      const nameInput = screen.getByTestId('entryNameInput');
+      fireEvent.change(nameInput, { target: { value: updatedName } });
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getByTestId('sharing-select'));
+      fireEvent.click(screen.getByTestId('sharing-global-option'));
+      fireEvent.click(screen.getByTestId('sharing-select'));
+      fireEvent.click(screen.getByTestId('sharing-private-option'));
+      fireEvent.click(screen.getByTestId('save-button'));
+    });
+
+    await waitFor(() => {
+      expect(mockUpdateEntry).toHaveBeenCalledTimes(1);
+    });
+    expect(mockCreateEntry).toHaveBeenCalledTimes(0);
+    expect(mockUpdateEntry).toHaveBeenCalledWith([{ ...mockData[0], name: updatedName }]);
+  });
+
+  it('does not create a duplicate index entry when switching sharing option twice', async () => {
+    (useFlyoutModalVisibility as jest.Mock).mockReturnValue({
+      isFlyoutOpen: true,
+      openFlyout: jest.fn(),
+      closeFlyout: jest.fn(),
+    });
+    render(<KnowledgeBaseSettingsManagement dataViews={mockDataViews} />, {
+      wrapper,
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getAllByTestId('edit-button')[2]);
+    });
+    expect(screen.getByTestId('flyout')).toBeVisible();
+
+    await waitFor(() => {
+      expect(screen.getByText('Edit index entry')).toBeInTheDocument();
+    });
+
+    const updatedName = 'New Entry Name';
+    await waitFor(() => {
+      const nameInput = screen.getByTestId('entry-name');
+      fireEvent.change(nameInput, { target: { value: updatedName } });
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getByTestId('sharing-select'));
+      fireEvent.click(screen.getByTestId('sharing-global-option'));
+      fireEvent.click(screen.getByTestId('sharing-select'));
+      fireEvent.click(screen.getByTestId('sharing-private-option'));
+      fireEvent.click(screen.getByTestId('save-button'));
+    });
+
+    await waitFor(() => {
+      expect(mockUpdateEntry).toHaveBeenCalledTimes(1);
+    });
+    expect(mockCreateEntry).toHaveBeenCalledTimes(0);
+    expect(mockUpdateEntry).toHaveBeenCalledWith([{ ...mockData[2], name: updatedName }]);
+  });
+
+  it('shows duplicate entry modal when making global to private entry update', async () => {
+    (useFlyoutModalVisibility as jest.Mock).mockReturnValue({
+      isFlyoutOpen: true,
+      openFlyout: jest.fn(),
+      closeFlyout: jest.fn(),
+    });
+    render(<KnowledgeBaseSettingsManagement dataViews={mockDataViews} />, {
+      wrapper,
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getAllByTestId('edit-button')[3]);
+    });
+    expect(screen.getByTestId('flyout')).toBeVisible();
+
+    await waitFor(() => {
+      expect(screen.getByText('Edit document entry')).toBeInTheDocument();
+    });
+
+    await waitFor(() => {
+      fireEvent.click(screen.getByTestId('sharing-select'));
+      fireEvent.click(screen.getByTestId('sharing-private-option'));
+      fireEvent.click(screen.getByTestId('save-button'));
+    });
+
+    expect(screen.getByTestId('create-duplicate-entry-modal')).toBeInTheDocument();
+    await waitFor(() => {
+      fireEvent.click(screen.getByTestId('confirmModalConfirmButton'));
+    });
+    expect(screen.queryByTestId('create-duplicate-entry-modal')).not.toBeInTheDocument();
+
+    await waitFor(() => {
+      expect(mockCreateEntry).toHaveBeenCalledTimes(1);
+    });
+    expect(mockUpdateEntry).toHaveBeenCalledTimes(0);
+    expect(mockCreateEntry).toHaveBeenCalledWith({ ...mockData[3], users: undefined });
+  });
+
  it('shows warning icon for index entries with missing indices', async () => {
    render(<KnowledgeBaseSettingsManagement dataViews={mockDataViews} />, {
      wrapper,
--- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index.tsx
+++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index.tsx
@ -83,6 +83,12 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
  const isKbSetup = isKnowledgeBaseSetup(kbStatus);

  const [deleteKBItem, setDeleteKBItem] = useState<DocumentEntry | IndexEntry | null>(null);
+  const [duplicateKBItem, setDuplicateKBItem] = useState<KnowledgeBaseEntryCreateProps | null>(
+    null
+  );
+  const [originalEntry, setOriginalEntry] = useState<DocumentEntry | IndexEntry | undefined>(
+    undefined
+  );

  // Only needed for legacy settings management
  const { knowledgeBase, setUpdatedKnowledgeBaseSettings, resetSettings, saveSettings } =
@ -146,25 +152,6 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
  });
  const isModifyingEntry = isCreatingEntry || isUpdatingEntries || isDeletingEntries;

-  // Flyout Save/Cancel Actions
-  const onSaveConfirmed = useCallback(async () => {
-    if (isKnowledgeBaseEntryResponse(selectedEntry)) {
-      await updateEntries([selectedEntry]);
-      closeFlyout();
-    } else if (isKnowledgeBaseEntryCreateProps(selectedEntry)) {
-      await createEntry(selectedEntry);
-      closeFlyout();
-    } else if (isKnowledgeBaseEntryCreateProps(selectedEntry)) {
-      createEntry(selectedEntry);
-      closeFlyout();
-    }
-  }, [closeFlyout, selectedEntry, createEntry, updateEntries]);
-
-  const onSaveCancelled = useCallback(() => {
-    setSelectedEntry(undefined);
-    closeFlyout();
-  }, [closeFlyout]);
-
  const {
    data: entries,
    isFetching: isFetchingEntries,
@ -175,6 +162,27 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
    enabled: enableKnowledgeBaseByDefault,
  });

+  // Flyout Save/Cancel Actions
+  const onSaveConfirmed = useCallback(async () => {
+    if (isKnowledgeBaseEntryResponse(selectedEntry)) {
+      await updateEntries([selectedEntry]);
+      closeFlyout();
+    } else if (isKnowledgeBaseEntryCreateProps(selectedEntry)) {
+      if (originalEntry) {
+        setDuplicateKBItem(selectedEntry);
+        return;
+      }
+      await createEntry(selectedEntry);
+      closeFlyout();
+    }
+  }, [selectedEntry, originalEntry, updateEntries, closeFlyout, createEntry]);
+
+  const onSaveCancelled = useCallback(() => {
+    setOriginalEntry(undefined);
+    setSelectedEntry(undefined);
+    closeFlyout();
+  }, [closeFlyout]);
+
  const { value: existingIndices } = useAsync(() => {
    const indices: string[] = [];
    entries.data.forEach((entry) => {
@ -206,6 +214,7 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
        },
        onEditActionClicked: ({ id }: KnowledgeBaseEntryResponse) => {
          const entry = entries.data.find((e) => e.id === id);
+          setOriginalEntry(entry);
          setSelectedEntry(entry);
          openFlyout();
        },
@ -294,6 +303,18 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
    }
  }, [deleteEntry, deleteKBItem, setDeleteKBItem]);

+  const handleCancelDuplicateEntry = useCallback(() => {
+    setDuplicateKBItem(null);
+  }, [setDuplicateKBItem]);
+
+  const handleDuplicateEntry = useCallback(async () => {
+    if (duplicateKBItem) {
+      await createEntry(duplicateKBItem);
+      closeFlyout();
+      setDuplicateKBItem(null);
+    }
+  }, [closeFlyout, createEntry, duplicateKBItem]);
+
  if (!enableKnowledgeBaseByDefault) {
    return (
      <>
@ -392,6 +413,7 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
          {selectedEntry?.type === DocumentEntryType.value ? (
            <DocumentEntryEditor
              entry={selectedEntry as DocumentEntry}
+              originalEntry={originalEntry as DocumentEntry}
              setEntry={
                setSelectedEntry as React.Dispatch<React.SetStateAction<Partial<DocumentEntry>>>
              }
@ -400,6 +422,7 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
          ) : (
            <IndexEntryEditor
              entry={selectedEntry as IndexEntry}
+              originalEntry={originalEntry as IndexEntry}
              dataViews={dataViews}
              setEntry={
                setSelectedEntry as React.Dispatch<React.SetStateAction<Partial<IndexEntry>>>
@ -425,6 +448,19 @@ export const KnowledgeBaseSettingsManagement: React.FC<Params> = React.memo(({ d
          <p>{i18n.DELETE_ENTRY_CONFIRMATION_CONTENT}</p>
        </EuiConfirmModal>
      )}
+      {duplicateKBItem && (
+        <EuiConfirmModal
+          title={i18n.DUPLICATE_ENTRY_CONFIRMATION_TITLE}
+          onCancel={handleCancelDuplicateEntry}
+          onConfirm={handleDuplicateEntry}
+          cancelButtonText={CANCEL_BUTTON_TEXT}
+          confirmButtonText={i18n.SAVE_BUTTON_TEXT}
+          defaultFocusedButton="confirm"
+          data-test-subj="create-duplicate-entry-modal"
+        >
+          <p>{i18n.DUPLICATE_ENTRY_CONFIRMATION_CONTENT}</p>
+        </EuiConfirmModal>
+      )}
      <KnowledgeBaseTour isKbSettingsPage />
    </>
  );
--- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index_entry_editor.tsx
+++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index_entry_editor.tsx
@ -21,16 +21,26 @@ import React, { useCallback, useMemo } from 'react';
 import { IndexEntry } from '@kbn/elastic-assistant-common';
 import { DataViewsContract } from '@kbn/data-views-plugin/public';
 import * as i18n from './translations';
+import { isGlobalEntry } from './helpers';

 interface Props {
  dataViews: DataViewsContract;
  entry?: IndexEntry;
+  originalEntry?: IndexEntry;
  setEntry: React.Dispatch<React.SetStateAction<Partial<IndexEntry>>>;
  hasManageGlobalKnowledgeBase: boolean;
 }

 export const IndexEntryEditor: React.FC<Props> = React.memo(
-  ({ dataViews, entry, setEntry, hasManageGlobalKnowledgeBase }) => {
+  ({ dataViews, entry, setEntry, hasManageGlobalKnowledgeBase, originalEntry }) => {
+    const privateUsers = useMemo(() => {
+      const originalUsers = originalEntry?.users;
+      if (originalEntry && !isGlobalEntry(originalEntry)) {
+        return originalUsers;
+      }
+      return undefined;
+    }, [originalEntry]);
+
    // Name
    const setName = useCallback(
      (e: React.ChangeEvent<HTMLInputElement>) =>
@ -43,9 +53,9 @@ export const IndexEntryEditor: React.FC<Props> = React.memo(
      (value: string) =>
        setEntry((prevEntry) => ({
          ...prevEntry,
-          users: value === i18n.SHARING_GLOBAL_OPTION_LABEL ? [] : undefined,
+          users: value === i18n.SHARING_GLOBAL_OPTION_LABEL ? [] : privateUsers,
        })),
-      [setEntry]
+      [privateUsers, setEntry]
    );
    const sharingOptions = [
      {
--- a/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/translations.ts
+++ b/x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/translations.ts
@ -336,6 +336,28 @@ export const PRIVATE = i18n.translate(
  }
 );

+export const SAVE_BUTTON_TEXT = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettingsManagement.saveButtonText',
+  {
+    defaultMessage: 'Save',
+  }
+);
+
+export const DUPLICATE_ENTRY_CONFIRMATION_TITLE = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettingsManagement.duplicateEntryConfirmationTitle',
+  {
+    defaultMessage: 'Duplicate entry?',
+  }
+);
+
+export const DUPLICATE_ENTRY_CONFIRMATION_CONTENT = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettingsManagement.duplicateEntryConfirmationContent',
+  {
+    defaultMessage:
+      'Changing a knowledge base entry from global to private will create a private copy of the original global entry. Please delete the global entry if you would like to revoke the content for other users.',
+  }
+);
+
 export const MISSING_INDEX_ERROR = i18n.translate(
  'xpack.elasticAssistant.assistant.settings.knowledgeBaseSettingsManagement.missingIndexError',
  {