From 0bc9cd59554c82d5e7152e27255f1ffec85bc651 Mon Sep 17 00:00:00 2001 From: Zhongnan Su Date: Tue, 21 Feb 2023 13:58:36 -0800 Subject: [PATCH] [MD] Integrate test connection to support SigV4 auth type (#3456) Signed-off-by: Su Signed-off-by: David Sinclair --- CHANGELOG.md | 1 + .../server/client/configure_client.test.ts | 68 ++++++++++++++++-- .../server/client/configure_client.ts | 71 +++++++------------ .../server/client/configure_client_utils.ts | 10 ++- .../data_source/server/data_source_service.ts | 23 +----- .../server/routes/test_connection.ts | 20 ++++-- src/plugins/data_source/server/types.ts | 9 ++- .../create_form/create_data_source_form.tsx | 3 +- .../edit_form/edit_data_source_form.tsx | 53 ++++++++------ .../public/components/utils.test.ts | 4 +- .../public/components/utils.ts | 2 +- 11 files changed, 158 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 274f5ec6fe02..9f0fcd58b885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add disablePrototypePoisoningProtection configuration to prevent JS client from erroring when cluster utilizes JS reserved words ([#2992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2992)) - [Multiple DataSource] Add support for SigV4 authentication ([#3058](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3058)) - Make build scripts find and use the latest version of Node.js that satisfies `engines.node` ([#3467](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3467)) +- [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3456)) ### 🐛 Bug Fixes 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 e00b62f04882..1499ccd411c2 100644 --- a/src/plugins/data_source/server/client/configure_client.test.ts +++ b/src/plugins/data_source/server/client/configure_client.test.ts @@ -6,7 +6,12 @@ import { SavedObjectsClientContract } from '../../../../core/server'; import { loggingSystemMock, savedObjectsClientMock } from '../../../../core/server/mocks'; import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common'; -import { DataSourceAttributes, AuthType } from '../../common/data_sources/types'; +import { + DataSourceAttributes, + AuthType, + UsernamePasswordTypedContent, + SigV4Content, +} from '../../common/data_sources/types'; import { DataSourcePluginConfigType } from '../../config'; import { ClientMock, parseClientOptionsMock } from './configure_client.test.mocks'; import { OpenSearchClientPoolSetup } from './client_pool'; @@ -31,6 +36,8 @@ describe('configureClient', () => { let dataSourceAttr: DataSourceAttributes; let dsClient: ReturnType; let dataSourceClientParams: DataSourceClientParams; + let usernamePasswordAuthContent: UsernamePasswordTypedContent; + let sigV4AuthContent: SigV4Content; beforeEach(() => { dsClient = opensearchClientMock.createInternalClient(); @@ -51,15 +58,24 @@ describe('configureClient', () => { rejectUnauthorized: true, }, } as ClientOptions; + + usernamePasswordAuthContent = { + username: 'username', + password: 'password', + }; + + sigV4AuthContent = { + region: 'us-east-1', + accessKey: 'accessKey', + secretKey: 'secretKey', + }; + dataSourceAttr = { title: 'title', endpoint: 'http://localhost', auth: { type: AuthType.UsernamePasswordType, - credentials: { - username: 'username', - password: 'password', - }, + credentials: usernamePasswordAuthContent, }, } as DataSourceAttributes; @@ -126,6 +142,48 @@ describe('configureClient', () => { expect(client).toBe(dsClient.child.mock.results[0].value); }); + test('configure client with auth.type == sigv4, will first call decodeAndDecrypt()', async () => { + savedObjectsMock.get.mockReset().mockResolvedValueOnce({ + id: DATA_SOURCE_ID, + type: DATA_SOURCE_SAVED_OBJECT_TYPE, + attributes: { + ...dataSourceAttr, + auth: { + type: AuthType.SigV4, + credentials: sigV4AuthContent, + }, + }, + references: [], + }); + + const decodeAndDecryptSpy = jest.spyOn(cryptographyMock, 'decodeAndDecrypt').mockResolvedValue({ + decryptedText: 'accessKey', + encryptionContext: { endpoint: 'http://localhost' }, + }); + await configureClient(dataSourceClientParams, clientPoolSetup, config, logger); + + expect(ClientMock).toHaveBeenCalledTimes(1); + expect(savedObjectsMock.get).toHaveBeenCalledTimes(1); + expect(decodeAndDecryptSpy).toHaveBeenCalledTimes(2); + }); + + test('configure test client for non-exist datasource should not call saved object api, nor decode any credential', async () => { + const decodeAndDecryptSpy = jest.spyOn(cryptographyMock, 'decodeAndDecrypt').mockResolvedValue({ + decryptedText: 'password', + encryptionContext: { endpoint: 'http://localhost' }, + }); + const testClientParams: DataSourceClientParams = { + ...dataSourceClientParams, + testClientDataSourceAttr: dataSourceAttr, + dataSourceId: undefined, + }; + await configureClient(testClientParams, clientPoolSetup, config, logger); + + expect(ClientMock).toHaveBeenCalledTimes(1); + expect(savedObjectsMock.get).not.toHaveBeenCalled(); + expect(decodeAndDecryptSpy).not.toHaveBeenCalled(); + }); + test('configure client with auth.type == username_password and password contaminated', async () => { const decodeAndDecryptSpy = jest .spyOn(cryptographyMock, 'decodeAndDecrypt') diff --git a/src/plugins/data_source/server/client/configure_client.ts b/src/plugins/data_source/server/client/configure_client.ts index 03d5d3f21d0b..8b43ffa80b23 100644 --- a/src/plugins/data_source/server/client/configure_client.ts +++ b/src/plugins/data_source/server/client/configure_client.ts @@ -29,13 +29,35 @@ import { } from './configure_client_utils'; export const configureClient = async ( - { dataSourceId, savedObjects, cryptography }: DataSourceClientParams, + { dataSourceId, savedObjects, cryptography, testClientDataSourceAttr }: DataSourceClientParams, openSearchClientPoolSetup: OpenSearchClientPoolSetup, config: DataSourcePluginConfigType, logger: Logger ): Promise => { + let dataSource; + let requireDecryption = true; + try { - const dataSource = await getDataSource(dataSourceId!, savedObjects); + // configure test client + if (testClientDataSourceAttr) { + const { + auth: { type, credentials }, + } = testClientDataSourceAttr; + // handle test connection case when changing non-credential field of existing data source + if ( + dataSourceId && + ((type === AuthType.UsernamePasswordType && !credentials?.password) || + (type === AuthType.SigV4 && !credentials?.accessKey && !credentials?.secretKey)) + ) { + dataSource = await getDataSource(dataSourceId, savedObjects); + } else { + dataSource = testClientDataSourceAttr; + requireDecryption = false; + } + } else { + dataSource = await getDataSource(dataSourceId!, savedObjects); + } + const rootClient = getRootClient( dataSource, openSearchClientPoolSetup.getClientFromPool, @@ -48,7 +70,8 @@ export const configureClient = async ( config, cryptography, rootClient, - dataSourceId + dataSourceId, + requireDecryption ); } catch (error: any) { logger.error( @@ -59,46 +82,6 @@ export const configureClient = async ( } }; -export const configureTestClient = async ( - { savedObjects, cryptography, dataSourceId }: DataSourceClientParams, - dataSourceAttr: DataSourceAttributes, - openSearchClientPoolSetup: OpenSearchClientPoolSetup, - config: DataSourcePluginConfigType, - logger: Logger -): Promise => { - try { - const { - auth: { type, credentials }, - } = dataSourceAttr; - let requireDecryption = false; - - const rootClient = getRootClient( - dataSourceAttr, - openSearchClientPoolSetup.getClientFromPool, - dataSourceId - ) as Client; - - if (type === AuthType.UsernamePasswordType && !credentials?.password && dataSourceId) { - dataSourceAttr = await getDataSource(dataSourceId, savedObjects); - requireDecryption = true; - } - - return getQueryClient( - dataSourceAttr, - openSearchClientPoolSetup.addClientToPool, - config, - cryptography, - rootClient, - dataSourceId, - requireDecryption - ); - } catch (error: any) { - logger.error(`Failed to get test client. ${error}: ${error.stack}`); - // Re-throw as DataSourceError - throw createDataSourceError(error); - } -}; - /** * Create a child client object with given auth info. * @@ -108,7 +91,7 @@ export const configureTestClient = async ( * @param config data source config * @param addClientToPool function to add client to client pool * @param dataSourceId id of data source saved Object - * @param requireDecryption boolean + * @param requireDecryption false when creating test client before data source exists * @returns Promise of query client */ const getQueryClient = async ( diff --git a/src/plugins/data_source/server/client/configure_client_utils.ts b/src/plugins/data_source/server/client/configure_client_utils.ts index 293f52ff43a5..3ef8acc97b58 100644 --- a/src/plugins/data_source/server/client/configure_client_utils.ts +++ b/src/plugins/data_source/server/client/configure_client_utils.ts @@ -32,9 +32,17 @@ export const getRootClient = ( ): Client | LegacyClient | undefined => { const { auth: { type }, + lastUpdatedTime, } = dataSourceAttr; + let cachedClient; const cacheKey = generateCacheKey(dataSourceAttr, dataSourceId); - const cachedClient = getClientFromPool(cacheKey, type); + + // return undefined when building SigV4 test client with new credentials + if (type === AuthType.SigV4) { + cachedClient = dataSourceId && lastUpdatedTime ? getClientFromPool(cacheKey, type) : undefined; + } else { + cachedClient = getClientFromPool(cacheKey, type); + } return cachedClient; }; diff --git a/src/plugins/data_source/server/data_source_service.ts b/src/plugins/data_source/server/data_source_service.ts index e816a25a729f..36a8d2a5ce5f 100644 --- a/src/plugins/data_source/server/data_source_service.ts +++ b/src/plugins/data_source/server/data_source_service.ts @@ -8,8 +8,7 @@ import { DataSourcePluginConfigType } from '../config'; import { OpenSearchClientPool } from './client'; import { configureLegacyClient } from './legacy'; import { DataSourceClientParams } from './types'; -import { DataSourceAttributes } from '../common/data_sources'; -import { configureTestClient, configureClient } from './client/configure_client'; +import { configureClient } from './client/configure_client'; export interface DataSourceServiceSetup { getDataSourceClient: (params: DataSourceClientParams) => Promise; @@ -22,11 +21,6 @@ export interface DataSourceServiceSetup { options?: LegacyCallAPIOptions ) => Promise; }; - - getTestingClient: ( - params: DataSourceClientParams, - dataSource: DataSourceAttributes - ) => Promise; } export class DataSourceService { private readonly openSearchClientPool: OpenSearchClientPool; @@ -49,19 +43,6 @@ export class DataSourceService { return configureClient(params, opensearchClientPoolSetup, config, this.logger); }; - const getTestingClient = async ( - params: DataSourceClientParams, - dataSource: DataSourceAttributes - ): Promise => { - return configureTestClient( - params, - dataSource, - opensearchClientPoolSetup, - config, - this.logger - ); - }; - const getDataSourceLegacyClient = (params: DataSourceClientParams) => { return { callAPI: ( @@ -79,7 +60,7 @@ export class DataSourceService { }; }; - return { getDataSourceClient, getDataSourceLegacyClient, getTestingClient }; + return { getDataSourceClient, getDataSourceLegacyClient }; } start() {} diff --git a/src/plugins/data_source/server/routes/test_connection.ts b/src/plugins/data_source/server/routes/test_connection.ts index 8f90577b045f..b32148a57420 100644 --- a/src/plugins/data_source/server/routes/test_connection.ts +++ b/src/plugins/data_source/server/routes/test_connection.ts @@ -5,7 +5,7 @@ import { schema } from '@osd/config-schema'; import { IRouter, OpenSearchClient } from 'opensearch-dashboards/server'; -import { DataSourceAttributes } from '../../common/data_sources'; +import { AuthType, DataSourceAttributes } from '../../common/data_sources'; import { DataSourceConnectionValidator } from './data_source_connection_validator'; import { DataSourceServiceSetup } from '../data_source_service'; import { CryptographyServiceSetup } from '../cryptography_service'; @@ -26,14 +26,20 @@ export const registerTestConnectionRoute = ( auth: schema.maybe( schema.object({ type: schema.oneOf([ - schema.literal('username_password'), - schema.literal('no_auth'), + schema.literal(AuthType.UsernamePasswordType), + schema.literal(AuthType.NoAuth), + schema.literal(AuthType.SigV4), ]), credentials: schema.oneOf([ schema.object({ username: schema.string(), password: schema.string(), }), + schema.object({ + region: schema.string(), + accessKey: schema.string(), + secretKey: schema.string(), + }), schema.literal(null), ]), }) @@ -46,13 +52,13 @@ export const registerTestConnectionRoute = ( const { dataSourceAttr, id: dataSourceId } = request.body; try { - const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getTestingClient( + const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getDataSourceClient( { - dataSourceId, savedObjects: context.core.savedObjects.client, cryptography, - }, - dataSourceAttr as DataSourceAttributes + dataSourceId, + testClientDataSourceAttr: dataSourceAttr as DataSourceAttributes, + } ); const dsValidator = new DataSourceConnectionValidator(dataSourceClient); diff --git a/src/plugins/data_source/server/types.ts b/src/plugins/data_source/server/types.ts index 913218e40d4b..68a840ebbbcb 100644 --- a/src/plugins/data_source/server/types.ts +++ b/src/plugins/data_source/server/types.ts @@ -8,6 +8,7 @@ import { OpenSearchClient, SavedObjectsClientContract, } from 'src/core/server'; +import { DataSourceAttributes } from '../common/data_sources'; import { CryptographyServiceSetup } from './cryptography_service'; import { DataSourceError } from './lib/error'; @@ -19,11 +20,13 @@ export interface LegacyClientCallAPIParams { } export interface DataSourceClientParams { - // id is optional when creating test client - dataSourceId?: string; - // this saved objects client is used to fetch data source on behalf of users, caller should pass scoped saved objects client + // to fetch data source on behalf of users, caller should pass scoped saved objects client savedObjects: SavedObjectsClientContract; cryptography: CryptographyServiceSetup; + // optional when creating test client, required for normal client + dataSourceId?: string; + // required when creating test client + testClientDataSourceAttr?: DataSourceAttributes; } export interface DataSourcePluginRequestContext { diff --git a/src/plugins/data_source_management/public/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx b/src/plugins/data_source_management/public/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx index 6c4cb6a97588..f069d2a1f0f3 100644 --- a/src/plugins/data_source_management/public/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx +++ b/src/plugins/data_source_management/public/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx @@ -581,8 +581,7 @@ export class CreateDataSourceForm extends React.Component< diff --git a/src/plugins/data_source_management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx b/src/plugins/data_source_management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx index fafca5b724b4..af17eb2d6944 100644 --- a/src/plugins/data_source_management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx +++ b/src/plugins/data_source_management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx @@ -346,25 +346,40 @@ export class EditDataSourceForm extends React.Component { this.setState({ isLoading: true }); - const existingAuthType = this.props.existingDataSource.auth.type; + const isNewCredential = !!(this.state.auth.type !== this.props.existingDataSource.auth.type); - try { - const isNewCredential = !!( - existingAuthType === AuthType.NoAuth && this.state.auth.type !== existingAuthType - ); - const formValues: DataSourceAttributes = { - title: this.state.title, - description: this.state.description, - endpoint: this.props.existingDataSource.endpoint, - auth: { - ...this.state.auth, - credentials: { - ...this.state.auth.credentials, - password: isNewCredential ? this.state.auth.credentials.password : '', - }, - }, - }; + let credentials = this.state.auth.credentials; + switch (this.state.auth.type) { + case AuthType.UsernamePasswordType: + credentials = { + username: this.state.auth.credentials?.username, + password: isNewCredential ? this.state.auth.credentials?.password : '', + } as UsernamePasswordTypedContent; + break; + case AuthType.SigV4: + credentials = { + region: this.state.auth.credentials?.region, + accessKey: isNewCredential ? this.state.auth.credentials?.accessKey : '', + secretKey: isNewCredential ? this.state.auth.credentials?.secretKey : '', + } as SigV4Content; + break; + case AuthType.NoAuth: + credentials = undefined; + break; + + default: + break; + } + + const formValues: DataSourceAttributes = { + title: this.state.title, + description: this.state.description, + endpoint: this.state.endpoint, + auth: { ...this.state.auth, credentials }, + }; + + try { await this.props.handleTestConnection(formValues); this.props.displayToastMessage({ @@ -532,9 +547,7 @@ export class EditDataSourceForm extends React.Component { Array [ "/internal/data-source-management/validate", Object { - "body": "{\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}", + "body": "{\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\"}}}", }, ], ] @@ -170,7 +170,7 @@ describe('DataSourceManagement: Utils.ts', () => { Array [ "/internal/data-source-management/validate", Object { - "body": "{\\"id\\":\\"test1234\\",\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}", + "body": "{\\"id\\":\\"test1234\\",\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\"}}}", }, ], ] diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index 3d9d6f51b413..539edbca970b 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -90,7 +90,7 @@ export async function testConnection( endpoint, auth: { type, - credentials: type === AuthType.NoAuth ? null : { ...credentials }, + credentials, }, }, };