From e59e506c1abbbce68e6eb1884cabab29e1025f70 Mon Sep 17 00:00:00 2001 From: Bandini <63824432+bandinib-amzn@users.noreply.github.com> Date: Tue, 27 Feb 2024 09:48:56 -0800 Subject: [PATCH] [Manual Backport 2.x] Refactor getClient and getLegacyClient to use authentication method registry (#5881) (#5940) * Refactor getClient and getLegacyClient to use authentication method registry (#5881) * Refactor getClient and getLegacyClient to use authentication method registry Signed-off-by: Bandini Bhopi * Adds changelog and UT Signed-off-by: Bandini Bhopi --------- Signed-off-by: Bandini Bhopi (cherry picked from commit d56b04d0aac6aed21825f44eb70bb4a9b512dde0) * Reverted accidental changes Signed-off-by: Bandini Bhopi --------- Signed-off-by: Bandini Bhopi --- CHANGELOG.md | 1 + .../data_source/common/data_sources/types.ts | 2 + .../authentication_methods_registry.mock.ts | 14 +++++ .../data_source/server/auth_registry/index.ts | 2 + .../client/configure_client.test.mocks.ts | 5 ++ .../server/client/configure_client.test.ts | 56 +++++++++++++++++- .../server/client/configure_client.ts | 48 ++++++++++++---- .../configure_legacy_client.test.mocks.ts | 5 ++ .../legacy/configure_legacy_client.test.ts | 57 ++++++++++++++++++- .../server/legacy/configure_legacy_client.ts | 40 ++++++++++--- src/plugins/data_source/server/plugin.ts | 7 ++- .../server/routes/test_connection.ts | 5 +- src/plugins/data_source/server/types.ts | 4 ++ .../server/util/credential_provider.ts | 14 +++++ 14 files changed, 237 insertions(+), 23 deletions(-) create mode 100644 src/plugins/data_source/server/auth_registry/authentication_methods_registry.mock.ts create mode 100644 src/plugins/data_source/server/util/credential_provider.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a63933584df1..7b641739a8b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) +- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) ### 🐛 Bug Fixes diff --git a/src/plugins/data_source/common/data_sources/types.ts b/src/plugins/data_source/common/data_sources/types.ts index d30e5ee710c8..38c14d18ccc4 100644 --- a/src/plugins/data_source/common/data_sources/types.ts +++ b/src/plugins/data_source/common/data_sources/types.ts @@ -14,6 +14,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes { credentials: UsernamePasswordTypedContent | SigV4Content | undefined | AuthTypeContent; }; lastUpdatedTime?: string; + name: AuthType | string; } export interface AuthTypeContent { @@ -30,6 +31,7 @@ export interface SigV4Content extends SavedObjectAttributes { secretKey: string; region: string; service?: SigV4ServiceName; + sessionToken?: string; } export interface UsernamePasswordTypedContent extends SavedObjectAttributes { diff --git a/src/plugins/data_source/server/auth_registry/authentication_methods_registry.mock.ts b/src/plugins/data_source/server/auth_registry/authentication_methods_registry.mock.ts new file mode 100644 index 000000000000..41e63798556e --- /dev/null +++ b/src/plugins/data_source/server/auth_registry/authentication_methods_registry.mock.ts @@ -0,0 +1,14 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { IAuthenticationMethodRegistery } from './authentication_methods_registry'; + +const create = () => + (({ + getAllAuthenticationMethods: jest.fn(), + getAuthenticationMethod: jest.fn(), + } as unknown) as jest.Mocked); + +export const authenticationMethodRegisteryMock = { create }; diff --git a/src/plugins/data_source/server/auth_registry/index.ts b/src/plugins/data_source/server/auth_registry/index.ts index 9352afd8b661..919585a26b96 100644 --- a/src/plugins/data_source/server/auth_registry/index.ts +++ b/src/plugins/data_source/server/auth_registry/index.ts @@ -7,3 +7,5 @@ export { IAuthenticationMethodRegistery, AuthenticationMethodRegistery, } from './authentication_methods_registry'; + +export { authenticationMethodRegisteryMock } from './authentication_methods_registry.mock'; diff --git a/src/plugins/data_source/server/client/configure_client.test.mocks.ts b/src/plugins/data_source/server/client/configure_client.test.mocks.ts index a9e6da2a1ad0..98d4811086fd 100644 --- a/src/plugins/data_source/server/client/configure_client.test.mocks.ts +++ b/src/plugins/data_source/server/client/configure_client.test.mocks.ts @@ -16,3 +16,8 @@ export const parseClientOptionsMock = jest.fn(); jest.doMock('./client_config', () => ({ parseClientOptions: parseClientOptionsMock, })); + +export const authRegistryCredentialProviderMock = jest.fn(); +jest.doMock('../util/credential_provider', () => ({ + authRegistryCredentialProvider: authRegistryCredentialProviderMock, +})); diff --git a/src/plugins/data_source/server/client/configure_client.test.ts b/src/plugins/data_source/server/client/configure_client.test.ts index fa4b820a912e..573e766741ca 100644 --- a/src/plugins/data_source/server/client/configure_client.test.ts +++ b/src/plugins/data_source/server/client/configure_client.test.ts @@ -13,7 +13,11 @@ import { SigV4Content, } from '../../common/data_sources/types'; import { DataSourcePluginConfigType } from '../../config'; -import { ClientMock, parseClientOptionsMock } from './configure_client.test.mocks'; +import { + ClientMock, + parseClientOptionsMock, + authRegistryCredentialProviderMock, +} from './configure_client.test.mocks'; import { OpenSearchClientPoolSetup } from './client_pool'; import { configureClient } from './configure_client'; import { ClientOptions } from '@opensearch-project/opensearch-next'; @@ -21,8 +25,12 @@ import { ClientOptions } from '@opensearch-project/opensearch-next'; import { opensearchClientMock } from '../../../../core/server/opensearch/client/mocks'; import { cryptographyServiceSetupMock } from '../cryptography_service.mocks'; import { CryptographyServiceSetup } from '../cryptography_service'; -import { DataSourceClientParams } from '../types'; +import { DataSourceClientParams, AuthenticationMethod } from '../types'; import { CustomApiSchemaRegistry } from '../schema_registry'; +import { + IAuthenticationMethodRegistery, + authenticationMethodRegisteryMock, +} from '../auth_registry'; const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8'; @@ -40,6 +48,7 @@ describe('configureClient', () => { let usernamePasswordAuthContent: UsernamePasswordTypedContent; let sigV4AuthContent: SigV4Content; let customApiSchemaRegistry: CustomApiSchemaRegistry; + let authenticationMethodRegistery: jest.Mocked; beforeEach(() => { dsClient = opensearchClientMock.createInternalClient(); @@ -47,6 +56,7 @@ describe('configureClient', () => { savedObjectsMock = savedObjectsClientMock.create(); cryptographyMock = cryptographyServiceSetupMock.create(); customApiSchemaRegistry = new CustomApiSchemaRegistry(); + authenticationMethodRegistery = authenticationMethodRegisteryMock.create(); config = { enabled: true, @@ -242,4 +252,46 @@ describe('configureClient', () => { expect(savedObjectsMock.get).toHaveBeenCalledTimes(1); expect(decodeAndDecryptSpy).toHaveBeenCalledTimes(1); }); + + test('configureClient should retunrn client from authentication registery if method present in registry', async () => { + const name = 'typeA'; + const customAuthContent = { + region: 'us-east-1', + roleARN: 'test-role', + }; + savedObjectsMock.get.mockReset().mockResolvedValueOnce({ + id: DATA_SOURCE_ID, + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes: { + ...dataSourceAttr, + auth: { + type: AuthType.SigV4, + credentials: customAuthContent, + }, + }, + references: [], + }); + const authMethod: AuthenticationMethod = { + name, + authType: AuthType.SigV4, + credentialProvider: jest.fn(), + }; + authenticationMethodRegistery.getAuthenticationMethod.mockImplementation(() => authMethod); + + authRegistryCredentialProviderMock.mockReturnValue({ + credential: sigV4AuthContent, + type: AuthType.SigV4, + }); + + await configureClient( + { ...dataSourceClientParams, authRegistry: authenticationMethodRegistery }, + clientPoolSetup, + config, + logger + ); + expect(authRegistryCredentialProviderMock).toHaveBeenCalled(); + expect(authenticationMethodRegistery.getAuthenticationMethod).toHaveBeenCalledTimes(1); + expect(ClientMock).toHaveBeenCalledTimes(1); + expect(savedObjectsMock.get).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/plugins/data_source/server/client/configure_client.ts b/src/plugins/data_source/server/client/configure_client.ts index 6083a8527708..d12688df0707 100644 --- a/src/plugins/data_source/server/client/configure_client.ts +++ b/src/plugins/data_source/server/client/configure_client.ts @@ -7,7 +7,7 @@ import { Client, ClientOptions } from '@opensearch-project/opensearch-next'; import { Client as LegacyClient } from 'elasticsearch'; import { Credentials } from 'aws-sdk'; import { AwsSigv4Signer } from '@opensearch-project/opensearch-next/aws'; -import { Logger } from '../../../../../src/core/server'; +import { Logger, OpenSearchDashboardsRequest } from '../../../../../src/core/server'; import { AuthType, DataSourceAttributes, @@ -27,6 +27,8 @@ import { getDataSource, generateCacheKey, } from './configure_client_utils'; +import { IAuthenticationMethodRegistery } from '../auth_registry'; +import { authRegistryCredentialProvider } from '../util/credential_provider'; export const configureClient = async ( { @@ -35,6 +37,8 @@ export const configureClient = async ( cryptography, testClientDataSourceAttr, customApiSchemaRegistryPromise, + request, + authRegistry, }: DataSourceClientParams, openSearchClientPoolSetup: OpenSearchClientPoolSetup, config: DataSourcePluginConfigType, @@ -80,6 +84,8 @@ export const configureClient = async ( cryptography, rootClient, dataSourceId, + request, + authRegistry, requireDecryption ); } catch (error: any) { @@ -101,6 +107,8 @@ export const configureClient = async ( * @param config data source config * @param addClientToPool function to add client to client pool * @param dataSourceId id of data source saved Object + * @param request OpenSearch Dashboards incoming request to read client parameters from header. + * @param authRegistry registry to retrieve the credentials provider for the authentication method in order to return the client * @param requireDecryption false when creating test client before data source exists * @returns Promise of query client */ @@ -112,15 +120,31 @@ const getQueryClient = async ( cryptography?: CryptographyServiceSetup, rootClient?: Client, dataSourceId?: string, + request?: OpenSearchDashboardsRequest, + authRegistry?: IAuthenticationMethodRegistery, requireDecryption: boolean = true ): Promise => { - const { + let credential; + let { auth: { type }, - endpoint, + name, } = dataSourceAttr; + const { endpoint } = dataSourceAttr; + name = name ?? type; const clientOptions = parseClientOptions(config, endpoint, registeredSchema); const cacheKey = generateCacheKey(dataSourceAttr, dataSourceId); + const authenticationMethod = authRegistry?.getAuthenticationMethod(name); + if (authenticationMethod !== undefined) { + const credentialProvider = await authRegistryCredentialProvider(authenticationMethod, { + dataSourceAttr, + request, + cryptography, + }); + credential = credentialProvider.credential; + type = credentialProvider.type; + } + switch (type) { case AuthType.NoAuth: if (!rootClient) rootClient = new Client(clientOptions); @@ -129,9 +153,11 @@ const getQueryClient = async ( return rootClient.child(); case AuthType.UsernamePasswordType: - const credential = requireDecryption - ? await getCredential(dataSourceAttr, cryptography!) - : (dataSourceAttr.auth.credentials as UsernamePasswordTypedContent); + credential = + (credential as UsernamePasswordTypedContent) ?? + (requireDecryption + ? await getCredential(dataSourceAttr, cryptography!) + : (dataSourceAttr.auth.credentials as UsernamePasswordTypedContent)); if (!rootClient) rootClient = new Client(clientOptions); addClientToPool(cacheKey, type, rootClient); @@ -139,11 +165,13 @@ const getQueryClient = async ( return getBasicAuthClient(rootClient, credential); case AuthType.SigV4: - const awsCredential = requireDecryption - ? await getAWSCredential(dataSourceAttr, cryptography!) - : (dataSourceAttr.auth.credentials as SigV4Content); + credential = + (credential as SigV4Content) ?? + (requireDecryption + ? await getAWSCredential(dataSourceAttr, cryptography!) + : (dataSourceAttr.auth.credentials as SigV4Content)); - const awsClient = rootClient ? rootClient : getAWSClient(awsCredential, clientOptions); + const awsClient = rootClient ? rootClient : getAWSClient(credential, clientOptions); addClientToPool(cacheKey, type, awsClient); return awsClient; diff --git a/src/plugins/data_source/server/legacy/configure_legacy_client.test.mocks.ts b/src/plugins/data_source/server/legacy/configure_legacy_client.test.mocks.ts index e6c1b3363896..2f91e757fd28 100644 --- a/src/plugins/data_source/server/legacy/configure_legacy_client.test.mocks.ts +++ b/src/plugins/data_source/server/legacy/configure_legacy_client.test.mocks.ts @@ -16,3 +16,8 @@ export const parseClientOptionsMock = jest.fn(); jest.doMock('./client_config', () => ({ parseClientOptions: parseClientOptionsMock, })); + +export const authRegistryCredentialProviderMock = jest.fn(); +jest.doMock('../util/credential_provider', () => ({ + authRegistryCredentialProvider: authRegistryCredentialProviderMock, +})); diff --git a/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts b/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts index f5cae1307f5a..c55715ea3fe7 100644 --- a/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts +++ b/src/plugins/data_source/server/legacy/configure_legacy_client.test.ts @@ -10,12 +10,20 @@ import { AuthType, DataSourceAttributes, SigV4Content } from '../../common/data_ import { DataSourcePluginConfigType } from '../../config'; import { cryptographyServiceSetupMock } from '../cryptography_service.mocks'; import { CryptographyServiceSetup } from '../cryptography_service'; -import { DataSourceClientParams, LegacyClientCallAPIParams } from '../types'; +import { DataSourceClientParams, LegacyClientCallAPIParams, AuthenticationMethod } from '../types'; import { OpenSearchClientPoolSetup } from '../client'; import { ConfigOptions } from 'elasticsearch'; -import { ClientMock, parseClientOptionsMock } from './configure_legacy_client.test.mocks'; +import { + ClientMock, + parseClientOptionsMock, + authRegistryCredentialProviderMock, +} from './configure_legacy_client.test.mocks'; import { configureLegacyClient } from './configure_legacy_client'; import { CustomApiSchemaRegistry } from '../schema_registry'; +import { + IAuthenticationMethodRegistery, + authenticationMethodRegisteryMock, +} from '../auth_registry'; const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8'; @@ -29,6 +37,7 @@ describe('configureLegacyClient', () => { let configOptions: ConfigOptions; let dataSourceAttr: DataSourceAttributes; let sigV4AuthContent: SigV4Content; + let authenticationMethodRegistery: jest.Mocked; let mockOpenSearchClientInstance: { close: jest.Mock; @@ -48,6 +57,7 @@ describe('configureLegacyClient', () => { logger = loggingSystemMock.createLogger(); savedObjectsMock = savedObjectsClientMock.create(); cryptographyMock = cryptographyServiceSetupMock.create(); + authenticationMethodRegistery = authenticationMethodRegisteryMock.create(); config = { enabled: true, clientPool: { @@ -254,4 +264,47 @@ describe('configureLegacyClient', () => { expect(mockOpenSearchClientInstance.ping).toHaveBeenCalledTimes(1); expect(mockOpenSearchClientInstance.ping).toHaveBeenLastCalledWith(mockParams); }); + + test('configureLegacyClient should retunrn client from authentication registery if method present in registry', async () => { + const name = 'typeA'; + const customAuthContent = { + region: 'us-east-1', + roleARN: 'test-role', + }; + savedObjectsMock.get.mockReset().mockResolvedValueOnce({ + id: DATA_SOURCE_ID, + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes: { + ...dataSourceAttr, + auth: { + type: AuthType.SigV4, + credentials: customAuthContent, + }, + }, + references: [], + }); + const authMethod: AuthenticationMethod = { + name, + authType: AuthType.SigV4, + credentialProvider: jest.fn(), + }; + authenticationMethodRegistery.getAuthenticationMethod.mockImplementation(() => authMethod); + + authRegistryCredentialProviderMock.mockReturnValue({ + credential: sigV4AuthContent, + type: AuthType.SigV4, + }); + + await configureLegacyClient( + { ...dataSourceClientParams, authRegistry: authenticationMethodRegistery }, + callApiParams, + clientPoolSetup, + config, + logger + ); + expect(authRegistryCredentialProviderMock).toHaveBeenCalled(); + expect(authenticationMethodRegistery.getAuthenticationMethod).toHaveBeenCalledTimes(1); + expect(ClientMock).toHaveBeenCalledTimes(1); + expect(savedObjectsMock.get).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/plugins/data_source/server/legacy/configure_legacy_client.ts b/src/plugins/data_source/server/legacy/configure_legacy_client.ts index 1a7685b3b540..42dbd4aa2772 100644 --- a/src/plugins/data_source/server/legacy/configure_legacy_client.ts +++ b/src/plugins/data_source/server/legacy/configure_legacy_client.ts @@ -14,6 +14,7 @@ import { LegacyCallAPIOptions, LegacyOpenSearchErrorHelpers, Logger, + OpenSearchDashboardsRequest, } from '../../../../../src/core/server'; import { AuthType, @@ -34,6 +35,8 @@ import { getDataSource, generateCacheKey, } from '../client/configure_client_utils'; +import { IAuthenticationMethodRegistery } from '../auth_registry'; +import { authRegistryCredentialProvider } from '../util/credential_provider'; export const configureLegacyClient = async ( { @@ -41,6 +44,8 @@ export const configureLegacyClient = async ( savedObjects, cryptography, customApiSchemaRegistryPromise, + request, + authRegistry, }: DataSourceClientParams, callApiParams: LegacyClientCallAPIParams, openSearchClientPoolSetup: OpenSearchClientPoolSetup, @@ -65,7 +70,9 @@ export const configureLegacyClient = async ( config, registeredSchema, rootClient, - dataSourceId + dataSourceId, + request, + authRegistry ); } catch (error: any) { logger.debug( @@ -96,15 +103,31 @@ const getQueryClient = async ( config: DataSourcePluginConfigType, registeredSchema: any[], rootClient?: LegacyClient, - dataSourceId?: string + dataSourceId?: string, + request?: OpenSearchDashboardsRequest, + authRegistry?: IAuthenticationMethodRegistery ) => { - const { + let credential; + let { auth: { type }, - endpoint: nodeUrl, + name, } = dataSourceAttr; + const { endpoint: nodeUrl } = dataSourceAttr; + name = name ?? type; const clientOptions = parseClientOptions(config, nodeUrl, registeredSchema); const cacheKey = generateCacheKey(dataSourceAttr, dataSourceId); + const authenticationMethod = authRegistry?.getAuthenticationMethod(name); + if (authenticationMethod !== undefined) { + const credentialProvider = await authRegistryCredentialProvider(authenticationMethod, { + dataSourceAttr, + request, + cryptography, + }); + credential = credentialProvider.credential; + type = credentialProvider.type; + } + switch (type) { case AuthType.NoAuth: if (!rootClient) rootClient = new LegacyClient(clientOptions); @@ -117,7 +140,9 @@ const getQueryClient = async ( ); case AuthType.UsernamePasswordType: - const credential = await getCredential(dataSourceAttr, cryptography); + credential = + (credential as UsernamePasswordTypedContent) ?? + (await getCredential(dataSourceAttr, cryptography)); if (!rootClient) rootClient = new LegacyClient(clientOptions); addClientToPool(cacheKey, type, rootClient); @@ -125,9 +150,10 @@ const getQueryClient = async ( return getBasicAuthClient(rootClient, { endpoint, clientParams, options }, credential); case AuthType.SigV4: - const awsCredential = await getAWSCredential(dataSourceAttr, cryptography); + credential = + (credential as SigV4Content) ?? (await getAWSCredential(dataSourceAttr, cryptography)); - const awsClient = rootClient ? rootClient : getAWSClient(awsCredential, clientOptions); + const awsClient = rootClient ? rootClient : getAWSClient(credential, clientOptions); addClientToPool(cacheKey, type, awsClient); return await (callAPI.bind(null, awsClient) as LegacyAPICaller)( diff --git a/src/plugins/data_source/server/plugin.ts b/src/plugins/data_source/server/plugin.ts index 6bccfbfad662..c75e55809781 100644 --- a/src/plugins/data_source/server/plugin.ts +++ b/src/plugins/data_source/server/plugin.ts @@ -168,7 +168,8 @@ export class DataSourcePlugin implements Plugin, customApiSchemaRegistryPromise: Promise ): IContextProvider, 'dataSource'> => { - return (context, req) => { + return async (context, req) => { + const authRegistry = await authRegistryPromise; return { opensearch: { getClient: (dataSourceId: string) => { @@ -181,6 +182,8 @@ export class DataSourcePlugin implements Plugin ) => { + const authRegistry = await authRegistryPromise; router.post( { path: '/internal/data-source-management/validate', @@ -65,6 +66,8 @@ export const registerTestConnectionRoute = ( cryptography, dataSourceId, testClientDataSourceAttr: dataSourceAttr as DataSourceAttributes, + request, + authRegistry, } ); diff --git a/src/plugins/data_source/server/types.ts b/src/plugins/data_source/server/types.ts index 9bd70b142d8b..ede0194ed3ef 100644 --- a/src/plugins/data_source/server/types.ts +++ b/src/plugins/data_source/server/types.ts @@ -37,6 +37,10 @@ export interface DataSourceClientParams { testClientDataSourceAttr?: DataSourceAttributes; // custom API schema registry promise, required for getting registered custom API schema customApiSchemaRegistryPromise: Promise; + // When client parameters are required to be retrieved from the request header, the caller should provide the request. + request?: OpenSearchDashboardsRequest; + // To retrieve the credentials provider for the authentication method from the registry in order to return the client. + authRegistry?: IAuthenticationMethodRegistery; } export interface DataSourceCredentialsProviderOptions { diff --git a/src/plugins/data_source/server/util/credential_provider.ts b/src/plugins/data_source/server/util/credential_provider.ts new file mode 100644 index 000000000000..d737c932fd95 --- /dev/null +++ b/src/plugins/data_source/server/util/credential_provider.ts @@ -0,0 +1,14 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { DataSourceCredentialsProviderOptions, AuthenticationMethod } from '../types'; + +export const authRegistryCredentialProvider = async ( + authenticationMethod: AuthenticationMethod, + options: DataSourceCredentialsProviderOptions +) => ({ + credential: await authenticationMethod.credentialProvider(options), + type: authenticationMethod.authType, +});