[Multiple DataSource] DataSource creation and edition page improvemen…

…t to better support registered auth types (#6122) * [Token Exchange Unification] State update for createDataSource and editDataSource pages Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * [Token Exchange Unification] rectify state for dataSource creation page and edit page Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * [UT] Add more test cases for util functions Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * [Token Exchange Unification] Update dataSource bottom banner control Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * Update changefile.md Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * Add comments Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * Code review change, fix typo Signed-off-by: Xinrui Bai <xinruiba@amazon.com> * Resolve comments, update typo in test cases Signed-off-by: Xinrui Bai <xinruiba@amazon.com> --------- Signed-off-by: Xinrui Bai <xinruiba@amazon.com>
opensearch-project · Mar 14, 2024 · 31e8481 · 31e8481
1 parent 3073926
commit 31e8481
Show file tree

Hide file tree

Showing 10 changed files with 147 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multiple Datasource] Adds a session token to AWS credentials ([#6103](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6103))
 - [Multiple Datasource] Add Vega support to MDS by specifying a data source name in the Vega spec ([#5975](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5975))
 - [Multiple Datasource] Test connection schema validation for registered auth types ([#6109](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6109))
+- [Multiple DataSource] DataSource creation and edition page improvement to better support registered auth types ([#6122](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6122))
 - [Workspace] Consume workspace id in saved object client ([#6014](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6014))
 - [Multiple Datasource] Export DataSourcePluginRequestContext at top level for plugins to use ([#6108](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6108))
 - [Multiple Datasource] Expose filterfn in datasource menu component to allow filter data sources before rendering in navigation bar ([#6113](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6113))

diff --git a/src/plugins/data_source_management/public/auth_registry/authentication_methods_registry.ts b/src/plugins/data_source_management/public/auth_registry/authentication_methods_registry.ts
@@ -13,7 +13,7 @@ export interface AuthenticationMethod {
     state: { [key: string]: any },
     setState: React.Dispatch<React.SetStateAction<any>>
   ) => React.JSX.Element;
-  crendentialFormField?: { [key: string]: string };
+  credentialFormField?: { [key: string]: string };
 }
 
 export type IAuthenticationMethodRegistery = Omit<

diff --git a/...ponents/create_data_source_wizard/components/create_form/create_data_source_form.test.tsx b/...ponents/create_data_source_wizard/components/create_form/create_data_source_form.test.tsx
@@ -534,7 +534,7 @@ describe('Datasource Management: Create Datasource form with registered Auth Typ
         inputDisplay: 'some input',
       },
       credentialForm: mockCredentialForm,
-      crendentialFormField: {
+      credentialFormField: {
         userNameRegistered: 'some filled in userName from registed auth credential form',
         passWordRegistered: 'some filled in password from registed auth credential form',
       },

diff --git a/...c/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx b/...c/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx
@@ -101,7 +101,7 @@ export class CreateDataSourceForm extends React.Component<
       auth: {
         type: initialSelectedAuthMethod?.name,
         credentials: {
-          ...initialSelectedAuthMethod?.crendentialFormField,
+          ...initialSelectedAuthMethod?.credentialFormField,
         },
       },
     };
@@ -153,15 +153,20 @@ export class CreateDataSourceForm extends React.Component<
   };
 
   onChangeAuthType = (authType: AuthType) => {
+    const credentials = this.state.auth.credentials;
+
+    const registeredAuthCredentials = extractRegisteredAuthTypeCredentials(
+      (credentials ?? {}) as { [key: string]: string },
+      authType,
+      this.authenticationMethodRegistery
+    );
+
     this.setState({
       auth: {
         ...this.state.auth,
         type: authType,
         credentials: {
-          ...this.state.auth.credentials,
-          service:
-            (this.state.auth.credentials.service as SigV4ServiceName) ||
-            SigV4ServiceName.OpenSearch,
+          ...registeredAuthCredentials,
         },
       },
     });

diff --git a/...nt/public/components/edit_data_source/components/edit_form/edit_data_source_form.test.tsx b/...nt/public/components/edit_data_source/components/edit_form/edit_data_source_form.test.tsx
@@ -410,7 +410,7 @@ describe('With Registered Authentication', () => {
         inputDisplay: 'some input',
       },
       credentialForm: mockedCredentialForm,
-      crendentialFormField: {},
+      credentialFormField: {},
     } as AuthenticationMethod;
 
     const mockedContext = mockManagementPlugin.createDataSourceManagementContext();

diff --git a/...agement/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx b/...agement/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx
@@ -24,6 +24,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@osd/i18n';
 import { FormattedMessage } from '@osd/i18n/react';
+import deepEqual from 'fast-deep-equal';
 import { AuthenticationMethodRegistery } from '../../../../auth_registry';
 import { SigV4Content, SigV4ServiceName } from '../../../../../../data_source/common/data_sources';
 import { Header } from '../header';
@@ -100,11 +101,7 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
       auth: {
         type: initialSelectedAuthMethod?.name,
         credentials: {
-          username: '',
-          password: '',
-          region: '',
-          accessKey: '',
-          secretKey: '',
+          ...initialSelectedAuthMethod?.credentialFormField,
         },
       },
       showUpdatePasswordModal: false,
@@ -127,10 +124,11 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
     if (this.props.existingDataSource) {
       const { title, description, endpoint, auth } = this.props.existingDataSource;
 
-      const authTypeCheckResults = {
-        isUserNamePassword: auth.type === AuthType.UsernamePasswordType,
-        isSigV4: auth.type === AuthType.SigV4,
-      };
+      const registeredAuthCredentials = extractRegisteredAuthTypeCredentials(
+        (auth.credentials ?? {}) as { [key: string]: string },
+        auth.type,
+        this.authenticationMethodRegistery
+      );
 
       this.setState({
         title,
@@ -139,14 +137,7 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
         auth: {
           type: auth.type,
           credentials: {
-            username: authTypeCheckResults.isUserNamePassword ? auth.credentials?.username : '',
-            password: authTypeCheckResults.isUserNamePassword ? this.maskedPassword : '',
-            service: authTypeCheckResults.isSigV4
-              ? auth.credentials?.service || SigV4ServiceName.OpenSearch
-              : '',
-            region: authTypeCheckResults.isSigV4 ? auth.credentials!.region : '',
-            accessKey: authTypeCheckResults.isSigV4 ? this.maskedPassword : '',
-            secretKey: authTypeCheckResults.isSigV4 ? this.maskedPassword : '',
+            ...registeredAuthCredentials,
           },
         },
       });
@@ -185,16 +176,24 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
   };
 
   onChangeAuthType = (authType: AuthType) => {
+    /* If the selected authentication type matches, utilize the existing data source's credentials directly.*/
+    const credentials =
+      this.props.existingDataSource && authType === this.props.existingDataSource.auth.type
+        ? this.props.existingDataSource.auth.credentials
+        : this.state.auth.credentials;
+    const registeredAuthCredentials = extractRegisteredAuthTypeCredentials(
+      (credentials ?? {}) as { [key: string]: string },
+      authType,
+      this.authenticationMethodRegistery
+    );
+
     this.setState(
       {
         auth: {
           ...this.state.auth,
           type: authType,
           credentials: {
-            ...this.state.auth.credentials,
-            service:
-              (this.state.auth.credentials?.service as SigV4ServiceName) ||
-              SigV4ServiceName.OpenSearch,
+            ...registeredAuthCredentials,
           },
         },
       },
@@ -427,6 +426,14 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
         break;
 
       default:
+        const currentCredentials = (this.state.auth.credentials ?? {}) as {
+          [key: string]: string;
+        };
+        credentials = extractRegisteredAuthTypeCredentials(
+          currentCredentials,
+          this.state.auth.type,
+          this.authenticationMethodRegistery
+        );
         break;
     }
 
@@ -1029,21 +1036,50 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
     const isServiceNameChanged =
       isAuthTypeSigV4Unchanged &&
       formValues.auth.credentials?.service !== auth.credentials?.service;
+    const isRegisteredAuthCredentialChanged = this.isRegisteredAuthCredentialUpdated();
 
     if (
       formValues.title !== title ||
       formValues.description !== description ||
       formValues.auth.type !== auth.type ||
       isUsernameChanged ||
       isRegionChanged ||
-      isServiceNameChanged
+      isServiceNameChanged ||
+      isRegisteredAuthCredentialChanged
     ) {
       this.setState({ showUpdateOptions: true });
     } else {
       this.setState({ showUpdateOptions: false });
     }
   };
 
+  isRegisteredAuthCredentialUpdated = () => {
+    const { auth } = this.props.existingDataSource;
+    const currentAuth = this.state.auth;
+
+    if (
+      currentAuth.type === AuthType.NoAuth ||
+      currentAuth.type === AuthType.UsernamePasswordType ||
+      currentAuth.type === AuthType.SigV4
+    ) {
+      return false;
+    }
+
+    const existingAuthCredentials = extractRegisteredAuthTypeCredentials(
+      (auth?.credentials ?? {}) as { [key: string]: string },
+      currentAuth.type,
+      this.authenticationMethodRegistery
+    );
+
+    const registeredAuthCredentials = extractRegisteredAuthTypeCredentials(
+      (currentAuth?.credentials ?? {}) as { [key: string]: string },
+      currentAuth.type,
+      this.authenticationMethodRegistery
+    );
+
+    return !deepEqual(existingAuthCredentials, registeredAuthCredentials);
+  };
+
   renderBottomBar = () => {
     return (
       <EuiBottomBar data-test-subj="datasource-edit-bottomBar" affordForDisplacement={true}>

diff --git a/src/plugins/data_source_management/public/components/utils.test.ts b/src/plugins/data_source_management/public/components/utils.test.ts
@@ -285,7 +285,7 @@ describe('DataSourceManagement: Utils.ts', () => {
           value: authTypeToBeTested,
           inputDisplay: 'some input',
         },
-        crendentialFormField: {
+        credentialFormField: {
           userNameRegistered: '',
           passWordRegistered: '',
         },
@@ -352,7 +352,7 @@ describe('DataSourceManagement: Utils.ts', () => {
           value: authTypeToBeTested,
           inputDisplay: 'some input',
         },
-        crendentialFormField: {
+        credentialFormField: {
           userNameRegistered: '',
           passWordRegistered: '',
         },
@@ -380,5 +380,71 @@ describe('DataSourceManagement: Utils.ts', () => {
 
       expect(deepEqual(registedAuthTypeCredentials, expectExtractedAuthCredentials));
     });
+
+    test('Should inherit value from registered field when credential state not have registered field', () => {
+      const authTypeToBeTested = 'Some Auth Type';
+
+      const authMethodToBeTested = {
+        name: authTypeToBeTested,
+        credentialSourceOption: {
+          value: authTypeToBeTested,
+          inputDisplay: 'some input',
+        },
+        credentialFormField: {
+          registeredField: 'some value',
+        },
+      } as AuthenticationMethod;
+
+      const mockedCredentialState = {} as { [key: string]: string };
+
+      const expectExtractedAuthCredentials = {
+        registeredField: 'some value',
+      };
+
+      const authenticationMethodRegistery = new AuthenticationMethodRegistery();
+      authenticationMethodRegistery.registerAuthenticationMethod(authMethodToBeTested);
+
+      const registedAuthTypeCredentials = extractRegisteredAuthTypeCredentials(
+        mockedCredentialState,
+        authTypeToBeTested,
+        authenticationMethodRegistery
+      );
+
+      expect(deepEqual(registedAuthTypeCredentials, expectExtractedAuthCredentials));
+    });
+
+    test('Should not inherit value from registered field when credentail state have registered field', () => {
+      const authTypeToBeTested = 'Some Auth Type';
+
+      const authMethodToBeTested = {
+        name: authTypeToBeTested,
+        credentialSourceOption: {
+          value: authTypeToBeTested,
+          inputDisplay: 'some input',
+        },
+        credentialFormField: {
+          registeredField: 'Some value',
+        },
+      } as AuthenticationMethod;
+
+      const mockedCredentialState = {
+        registeredField: 'some other values',
+      } as { [key: string]: string };
+
+      const expectExtractedAuthCredentials = {
+        registeredField: 'some other values',
+      };
+
+      const authenticationMethodRegistery = new AuthenticationMethodRegistery();
+      authenticationMethodRegistery.registerAuthenticationMethod(authMethodToBeTested);
+
+      const registedAuthTypeCredentials = extractRegisteredAuthTypeCredentials(
+        mockedCredentialState,
+        authTypeToBeTested,
+        authenticationMethodRegistery
+      );
+
+      expect(deepEqual(registedAuthTypeCredentials, expectExtractedAuthCredentials));
+    });
   });
 });
diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts
@@ -151,10 +151,11 @@ export const extractRegisteredAuthTypeCredentials = (
 ) => {
   const registeredCredentials = {} as { [key: string]: string };
   const registeredCredentialField =
-    authenticationMethodRegistery.getAuthenticationMethod(authType)?.crendentialFormField ?? {};
+    authenticationMethodRegistery.getAuthenticationMethod(authType)?.credentialFormField ?? {};
 
   Object.keys(registeredCredentialField).forEach((credentialFiled) => {
-    registeredCredentials[credentialFiled] = currentCredentialState[credentialFiled] ?? '';
+    registeredCredentials[credentialFiled] =
+      currentCredentialState[credentialFiled] ?? registeredCredentialField[credentialFiled];
   });
 
   return registeredCredentials;

diff --git a/...ns/data_source_management/public/components/validation/datasource_form_validation.test.ts b/...ns/data_source_management/public/components/validation/datasource_form_validation.test.ts
@@ -82,7 +82,7 @@ describe('DataSourceManagement: Form Validation', () => {
           inputDisplay: 'some input',
         },
         credentialForm: jest.fn(),
-        crendentialFormField: {
+        credentialFormField: {
           userNameRegistered: 'some filled in userName from registed auth credential form',
           passWordRegistered: 'some filled in password from registed auth credential form',
         },

diff --git a/src/plugins/data_source_management/public/types.ts b/src/plugins/data_source_management/public/types.ts
@@ -74,7 +74,7 @@ export const noAuthCredentialField = {};
 export const noAuthCredentialAuthMethod = {
   name: AuthType.NoAuth,
   credentialSourceOption: noAuthCredentialOption,
-  crendentialFormField: noAuthCredentialField,
+  credentialFormField: noAuthCredentialField,
 };
 
 export const usernamePasswordCredentialOption = {
@@ -92,7 +92,7 @@ export const usernamePasswordCredentialField = {
 export const usernamePasswordAuthMethod = {
   name: AuthType.UsernamePasswordType,
   credentialSourceOption: usernamePasswordCredentialOption,
-  crendentialFormField: usernamePasswordCredentialField,
+  credentialFormField: usernamePasswordCredentialField,
 };
 
 export const sigV4CredentialOption = {
@@ -127,7 +127,7 @@ export const sigV4CredentialField = {
 export const sigV4AuthMethod = {
   name: AuthType.SigV4,
   credentialSourceOption: sigV4CredentialOption,
-  crendentialFormField: sigV4CredentialField,
+  credentialFormField: sigV4CredentialField,
 };
 
 export const credentialSourceOptions = [