Skip to content

Commit

Permalink
[8.13] [Search] Fix native connector API key management (elastic#177274
Browse files Browse the repository at this point in the history
…) (elastic#177439)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Search] Fix native connector API key management
(elastic#177274)](elastic#177274)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Navarone
Feekery","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-21T13:23:51Z","message":"[Search]
Fix native connector API key management (elastic#177274)\n\n- Generate API keys
when attaching an index to a connector\r\n- Remove unnecessary
`secret_id` args from API key generation code (it\r\ncan be found
directly from the connector doc)\r\n- Update native connector
configuration page to show a section for\r\nmanaging API keys (see
attached
screenshots).","sha":"a4e18355b87f48343364aaf67cd6605b0430b887","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:EnterpriseSearch","v8.13.0","v8.14.0"],"title":"[Search]
Fix native connector API key
management","number":177274,"url":"https://github.com/elastic/kibana/pull/177274","mergeCommit":{"message":"[Search]
Fix native connector API key management (elastic#177274)\n\n- Generate API keys
when attaching an index to a connector\r\n- Remove unnecessary
`secret_id` args from API key generation code (it\r\ncan be found
directly from the connector doc)\r\n- Update native connector
configuration page to show a section for\r\nmanaging API keys (see
attached
screenshots).","sha":"a4e18355b87f48343364aaf67cd6605b0430b887"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/177274","number":177274,"mergeCommit":{"message":"[Search]
Fix native connector API key management (elastic#177274)\n\n- Generate API keys
when attaching an index to a connector\r\n- Remove unnecessary
`secret_id` args from API key generation code (it\r\ncan be found
directly from the connector doc)\r\n- Update native connector
configuration page to show a section for\r\nmanaging API keys (see
attached
screenshots).","sha":"a4e18355b87f48343364aaf67cd6605b0430b887"}}]}]
BACKPORT-->

Co-authored-by: Navarone Feekery <[email protected]>
  • Loading branch information
kibanamachine and navarone-feekery authored Feb 21, 2024
1 parent 67990d8 commit 143f5cf
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ describe('generateConnectorApiKeyApiLogic', () => {
it('calls correct api', async () => {
const promise = Promise.resolve('result');
http.post.mockReturnValue(promise);
const result = generateApiKey({ indexName: 'indexName', isNative: false, secretId: null });
const result = generateApiKey({ indexName: 'indexName', isNative: false });
await nextTick();
expect(http.post).toHaveBeenCalledWith(
'/internal/enterprise_search/indices/indexName/api_key',
{
body: '{"is_native":false,"secret_id":null}',
body: '{"is_native":false}',
}
);
await expect(result).resolves.toEqual('result');
Expand All @@ -41,12 +41,12 @@ describe('generateConnectorApiKeyApiLogic', () => {
it('calls correct api', async () => {
const promise = Promise.resolve('result');
http.post.mockReturnValue(promise);
const result = generateApiKey({ indexName: 'indexName', isNative: true, secretId: '1234' });
const result = generateApiKey({ indexName: 'indexName', isNative: true });
await nextTick();
expect(http.post).toHaveBeenCalledWith(
'/internal/enterprise_search/indices/indexName/api_key',
{
body: '{"is_native":true,"secret_id":"1234"}',
body: '{"is_native":true}',
}
);
await expect(result).resolves.toEqual('result');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,13 @@ export interface ApiKey {
export const generateApiKey = async ({
indexName,
isNative,
secretId,
}: {
indexName: string;
isNative: boolean;
secretId: string | null;
}) => {
const route = `/internal/enterprise_search/indices/${indexName}/api_key`;
const params = {
is_native: isNative,
secret_id: secretId,
};
return await HttpLogic.values.http.post<ApiKey>(route, {
body: JSON.stringify(params),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export const ConnectorConfiguration: React.FC = () => {
indexName={indexName}
hasApiKey={!!connector.api_key_id}
isNative={false}
secretId={null}
/>
),
status: hasApiKey ? 'complete' : 'incomplete',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ import { CONNECTOR_ICONS } from '../../../shared/icons/connector_icons';
import { KibanaLogic } from '../../../shared/kibana';

import { EuiButtonTo } from '../../../shared/react_router_helpers';
import { GenerateConnectorApiKeyApiLogic } from '../../api/connector/generate_connector_api_key_api_logic';
import { CONNECTOR_DETAIL_TAB_PATH } from '../../routes';
import { hasConfiguredConfiguration } from '../../utils/has_configured_configuration';

import { SyncsContextMenu } from '../search_index/components/header_actions/syncs_context_menu';
import { ApiKeyConfig } from '../search_index/connector/api_key_configuration';
import { BETA_CONNECTORS, NATIVE_CONNECTORS } from '../search_index/connector/constants';
import { ConvertConnector } from '../search_index/connector/native_connector_configuration/convert_connector';
import { NativeConnectorConfigurationConfig } from '../search_index/connector/native_connector_configuration/native_connector_configuration_config';
Expand All @@ -51,6 +53,7 @@ export const NativeConnectorConfiguration: React.FC = () => {
const { connector } = useValues(ConnectorViewLogic);
const { config } = useValues(KibanaLogic);
const { errorConnectingMessage } = useValues(HttpLogic);
const { data: apiKeyData } = useValues(GenerateConnectorApiKeyApiLogic);

if (!connector) {
return <></>;
Expand Down Expand Up @@ -80,6 +83,8 @@ export const NativeConnectorConfiguration: React.FC = () => {
const hasResearched = hasDescription || hasConfigured || hasConfiguredAdvanced;
const icon = nativeConnector.icon;

const hasApiKey = !!(connector.api_key_id ?? apiKeyData);

// TODO service_type === "" is considered unknown/custom connector multipleplaces replace all of them with a better solution
const isBeta =
!connector.service_type ||
Expand Down Expand Up @@ -167,6 +172,23 @@ export const NativeConnectorConfiguration: React.FC = () => {
),
titleSize: 'xs',
},
{
children: (
<ApiKeyConfig
indexName={connector.index_name || ''}
hasApiKey={hasApiKey}
isNative
/>
),
status: hasApiKey ? 'complete' : 'incomplete',
title: i18n.translate(
'xpack.enterpriseSearch.content.indices.configurationConnector.nativeConnector.steps.manageApiKeyTitle',
{
defaultMessage: 'Manage API key',
}
),
titleSize: 'xs',
},
{
children: (
<EuiFlexGroup direction="column">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
EuiButton,
EuiSpacer,
EuiConfirmModal,
EuiCallOut,
} from '@elastic/eui';

import { i18n } from '@kbn/i18n';
Expand Down Expand Up @@ -65,8 +66,7 @@ export const ApiKeyConfig: React.FC<{
hasApiKey: boolean;
indexName: string;
isNative: boolean;
secretId: string | null;
}> = ({ hasApiKey, indexName, isNative, secretId }) => {
}> = ({ hasApiKey, indexName, isNative }) => {
const { makeRequest, apiReset } = useActions(GenerateConnectorApiKeyApiLogic);
const { data, status } = useValues(GenerateConnectorApiKeyApiLogic);
useEffect(() => {
Expand All @@ -78,7 +78,7 @@ export const ApiKeyConfig: React.FC<{
if (hasApiKey || data) {
setIsModalVisible(true);
} else {
makeRequest({ indexName, isNative, secretId });
makeRequest({ indexName, isNative });
}
};

Expand All @@ -89,7 +89,7 @@ export const ApiKeyConfig: React.FC<{
};

const onConfirm = () => {
makeRequest({ indexName, isNative, secretId });
makeRequest({ indexName, isNative });
setIsModalVisible(false);
};

Expand All @@ -102,7 +102,7 @@ export const ApiKeyConfig: React.FC<{
? i18n.translate(
'xpack.enterpriseSearch.content.indices.configurationConnector.nativeConnector.apiKey.description',
{
defaultMessage: `This native connector's API key {apiKeyName} is managed internally by Elasticsearch. The connector uses this API key to index documents into the {indexName} index. To rollover your API key, click "Generate API key".`,
defaultMessage: `This native connector's API key {apiKeyName} is managed internally by Elasticsearch. The connector uses this API key to index documents into the {indexName} index. To refresh your API key, click "Generate API key".`,
values: {
apiKeyName: `${indexName}-connector`,
indexName,
Expand All @@ -122,6 +122,33 @@ export const ApiKeyConfig: React.FC<{
)}
</EuiText>
</EuiFlexItem>
{!isNative || status === Status.LOADING ? (
<></>
) : indexName === '' ? (
<EuiCallOut
iconType="iInCircle"
title={i18n.translate(
'xpack.enterpriseSearch.content.connector_detail.configurationConnector.nativeConnector.apiKey.waitingForAttachedIndex',
{
defaultMessage:
'An API key will be automatically generated when an index is attached to this connector.',
}
)}
/>
) : !hasApiKey ? (
<EuiCallOut
iconType="warning"
color="danger"
title={i18n.translate(
'xpack.enterpriseSearch.content.connector_detail.configurationConnector.nativeConnector.apiKey.missing',
{
defaultMessage: 'This connector is missing an API key.',
}
)}
/>
) : (
<></>
)}
<EuiFlexItem>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center">
<EuiFlexItem grow={false}>
Expand All @@ -141,7 +168,7 @@ export const ApiKeyConfig: React.FC<{
</EuiFlexGroup>
</EuiFlexItem>

{data && (
{data && !isNative && (
<>
<EuiSpacer />
<EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ export const ConnectorConfiguration: React.FC = () => {
indexName={indexName}
hasApiKey={!!index.connector.api_key_id}
isNative={false}
secretId={null}
/>
),
status: hasApiKey ? 'complete' : 'incomplete',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,13 @@ export const NativeConnectorConfiguration: React.FC = () => {
},
{
children: (
<ApiKeyConfig
indexName={index.connector.name}
hasApiKey={hasApiKey}
isNative
secretId={index.connector.api_key_secret_id}
/>
<ApiKeyConfig indexName={index.connector.name} hasApiKey={hasApiKey} isNative />
),
status: hasApiKey ? 'complete' : 'incomplete',
title: i18n.translate(
'xpack.enterpriseSearch.content.indices.configurationConnector.nativeConnector.steps.regenerateApiKeyTitle',
'xpack.enterpriseSearch.content.indices.configurationConnector.nativeConnector.steps.manageApiKeyTitle',
{
defaultMessage: 'Regenerate API key',
defaultMessage: 'Manage API key',
}
),
titleSize: 'xs',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe('addConnector lib function', () => {
});

// native connector should generate API key and update secrets storage
expect(generateApiKey).toHaveBeenCalledWith(mockClient, 'index_name', true, null);
expect(generateApiKey).toHaveBeenCalledWith(mockClient, 'index_name', true);
});

it('should reject if index already exists', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export const addConnector = async (
input.isNative &&
input.serviceType !== ENTERPRISE_SEARCH_CONNECTOR_CRAWLER_SERVICE_TYPE
) {
await generateApiKey(client, index, true, null);
await generateApiKey(client, index, true);
}

return connector;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('generateApiKey lib function for connector clients', () => {
(updateConnectorSecret as jest.Mock).mockImplementation(() => undefined);

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', false, null)
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', false)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.index).not.toHaveBeenCalled();
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('generateApiKey lib function for connector clients', () => {
(updateConnectorSecret as jest.Mock).mockImplementation(() => undefined);

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'search-test', false, null)
generateApiKey(mockClient as unknown as IScopedClusterClient, 'search-test', false)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
name: 'search-test-connector',
Expand Down Expand Up @@ -153,7 +153,7 @@ describe('generateApiKey lib function for connector clients', () => {
(updateConnectorSecret as jest.Mock).mockImplementation(() => undefined);

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', false, null)
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', false)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
name: 'index_name-connector',
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('generateApiKey lib function for native connectors', () => {
(updateConnectorSecret as jest.Mock).mockImplementation(() => undefined);

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', true, null)
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', true)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.index).not.toHaveBeenCalled();
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('generateApiKey lib function for native connectors', () => {
(updateConnectorSecret as jest.Mock).mockImplementation(() => undefined);

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'search-test', true, null)
generateApiKey(mockClient as unknown as IScopedClusterClient, 'search-test', true)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
name: 'search-test-connector',
Expand Down Expand Up @@ -296,7 +296,7 @@ describe('generateApiKey lib function for native connectors', () => {
hits: [
{
_id: 'connectorId',
_source: { api_key_id: '1', doc: 'doc' },
_source: { api_key_id: '1', api_key_secret_id: '2', doc: 'doc' },
fields: { api_key_id: '1' },
},
],
Expand All @@ -317,7 +317,7 @@ describe('generateApiKey lib function for native connectors', () => {
}));

await expect(
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', true, '1234')
generateApiKey(mockClient as unknown as IScopedClusterClient, 'index_name', true)
).resolves.toEqual({ encoded: 'encoded', id: 'apiKeyId' });
expect(mockClient.asCurrentUser.security.createApiKey).toHaveBeenCalledWith({
name: 'index_name-connector',
Expand All @@ -334,14 +334,14 @@ describe('generateApiKey lib function for native connectors', () => {
},
});
expect(mockClient.asCurrentUser.index).toHaveBeenCalledWith({
document: { api_key_id: 'apiKeyId', api_key_secret_id: '1234', doc: 'doc' },
document: { api_key_id: 'apiKeyId', api_key_secret_id: '2', doc: 'doc' },
id: 'connectorId',
index: CONNECTORS_INDEX,
});
expect(mockClient.asCurrentUser.security.invalidateApiKey).toHaveBeenCalledWith({
ids: ['1'],
});
expect(createConnectorSecret).toBeCalledTimes(0);
expect(updateConnectorSecret).toHaveBeenCalledWith(mockClient.asCurrentUser, 'encoded', '1234');
expect(updateConnectorSecret).toHaveBeenCalledWith(mockClient.asCurrentUser, 'encoded', '2');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import { toAlphanumeric } from '../../../common/utils/to_alphanumeric';
export const generateApiKey = async (
client: IScopedClusterClient,
indexName: string,
isNative: boolean,
secretId: string | null
isNative: boolean
) => {
const aclIndexName = `${CONNECTORS_ACCESS_CONTROL_INDEX_PREFIX}${indexName}`;

Expand Down Expand Up @@ -49,7 +48,11 @@ export const generateApiKey = async (
const apiKeyFields = isNative
? {
api_key_id: apiKeyResult.id,
api_key_secret_id: await storeConnectorSecret(client, apiKeyResult.encoded, secretId),
api_key_secret_id: await storeConnectorSecret(
client,
apiKeyResult.encoded,
connector._source?.api_key_secret_id || null
),
}
: {
api_key_id: apiKeyResult.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { schema } from '@kbn/config-schema';
import { i18n } from '@kbn/i18n';
import {
CONNECTORS_INDEX,
deleteConnectorById,
deleteConnectorSecret,
fetchConnectorById,
Expand All @@ -32,6 +33,7 @@ import { addConnector } from '../../lib/connectors/add_connector';
import { startSync } from '../../lib/connectors/start_sync';
import { deleteAccessControlIndex } from '../../lib/indices/delete_access_control_index';
import { fetchIndexCounts } from '../../lib/indices/fetch_index_counts';
import { generateApiKey } from '../../lib/indices/generate_api_key';
import { deleteIndexPipelines } from '../../lib/pipelines/delete_pipelines';
import { getDefaultPipeline } from '../../lib/pipelines/get_default_pipeline';
import { updateDefaultPipeline } from '../../lib/pipelines/update_default_pipeline';
Expand Down Expand Up @@ -664,6 +666,14 @@ export function registerConnectorRoutes({ router, log }: RouteDependencies) {
method: 'PUT',
path: `/_connector/${connectorId}/_index_name`,
});

const connector = await fetchConnectorById(client.asCurrentUser, connectorId);
if (connector?.is_native) {
// generateApiKey will search for the connector doc based on index_name, so we need to refresh the index before that.
await client.asCurrentUser.indices.refresh({ index: CONNECTORS_INDEX });
await generateApiKey(client, indexName, true);
}

return response.ok();
} catch (error) {
if (isIndexNotFoundException(error)) {
Expand Down
Loading

0 comments on commit 143f5cf

Please sign in to comment.