[MD] Credentials security redesign (opensearch-project#2253)

1. Data model changes for data source saved object 2. Server side changes for data source saved object a. Implement data_source_saved_objects_client_wrapper to integrate with CryptographyClient for password encryption / decryption. b. Change data_source_service to fetch credentials directly from data source (still decrypt via CryptographyClient) c. Fix unit tests accordingly Signed-off-by: Louis Chu <[email protected]> Signed-off-by: Kristen Tian <[email protected]>
kristenTian · Sep 14, 2022 · beb02f7 · beb02f7
1 parent 1847ed7
commit beb02f7
Show file tree

Hide file tree

Showing 12 changed files with 330 additions and 407 deletions.
diff --git a/src/plugins/data_source/common/credentials/index.ts b/src/plugins/data_source/common/credentials/index.ts
diff --git a/src/plugins/data_source/common/credentials/types.ts b/src/plugins/data_source/common/credentials/types.ts
diff --git a/src/plugins/data_source/common/data_sources/types.ts b/src/plugins/data_source/common/data_sources/types.ts
@@ -7,6 +7,20 @@ import { SavedObjectAttributes } from 'src/core/types';
 
 export interface DataSourceAttributes extends SavedObjectAttributes {
   title: string;
+  description?: string;
   endpoint: string;
-  noAuth: boolean;
+  auth: {
+    type: AuthType;
+    credentials: UsernamePasswordTypedContent | undefined;
+  };
+}
+
+export interface UsernamePasswordTypedContent extends SavedObjectAttributes {
+  username: string;
+  password: string;
+}
+
+export enum AuthType {
+  NoAuth = 'no_auth',
+  UsernamePasswordType = 'username_password',
 }
diff --git a/src/plugins/data_source/common/index.ts b/src/plugins/data_source/common/index.ts
@@ -6,11 +6,3 @@
 export const PLUGIN_ID = 'dataSource';
 export const PLUGIN_NAME = 'data_source';
 export const DATA_SOURCE_SAVED_OBJECT_TYPE = 'data-source';
-export const CREDENTIAL_SAVED_OBJECT_TYPE = 'credential';
-
-export {
-  CredentialMaterialsType,
-  CredentialSavedObjectAttributes,
-  CredentialMaterials,
-  UsernamePasswordTypedContent,
-} from './credentials';
diff --git a/src/plugins/data_source/public/index.ts b/src/plugins/data_source/public/index.ts
@@ -12,11 +12,3 @@ export function plugin() {
 }
 
 export { DataSourcePublicPluginSetup, DataSourcePublicPluginStart } from './types';
-
-export {
-  CredentialMaterialsType,
-  CredentialSavedObjectAttributes,
-  CredentialMaterials,
-  UsernamePasswordTypedContent,
-  CREDENTIAL_SAVED_OBJECT_TYPE,
-} from '../common';
diff --git a/src/plugins/data_source/server/client/configure_client.test.ts b/src/plugins/data_source/server/client/configure_client.test.ts
@@ -5,12 +5,8 @@
 
 import { SavedObjectsClientContract } from '../../../../core/server';
 import { loggingSystemMock, savedObjectsClientMock } from '../../../../core/server/mocks';
-import { DATA_SOURCE_SAVED_OBJECT_TYPE, CREDENTIAL_SAVED_OBJECT_TYPE } from '../../common';
-import {
-  CredentialMaterialsType,
-  CredentialSavedObjectAttributes,
-} from '../../common/credentials/types';
-import { DataSourceAttributes } from '../../common/data_sources';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common';
+import { DataSourceAttributes, AuthType } from '../../common/data_sources/types';
 import { DataSourcePluginConfigType } from '../../config';
 import { ClientMock, parseClientOptionsMock } from './configure_client.test.mocks';
 import { OpenSearchClientPoolSetup } from './client_pool';
@@ -21,7 +17,6 @@ import { opensearchClientMock } from '../../../../core/server/opensearch/client/
 import { CryptographyClient } from '../cryptography';
 
 const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8';
-const CREDENETIAL_ID = 'a54dsaadasfasfwe22d23d23d2453df3';
 const cryptoClient = new CryptographyClient('test', 'test', new Array(32).fill(0));
 
 // TODO: improve UT
@@ -54,38 +49,26 @@ describe('configureClient', () => {
     dataSourceAttr = {
       title: 'title',
       endpoint: 'http://localhost',
-      noAuth: false,
+      auth: {
+        type: AuthType.UsernamePasswordType,
+        credentials: {
+          username: 'username',
+          password: 'password',
+        },
+      },
     } as DataSourceAttributes;
 
     clientPoolSetup = {
       getClientFromPool: jest.fn(),
       addClientToPool: jest.fn(),
     };
 
-    const crendentialAttr = {
-      title: 'cred',
-      credentialMaterials: {
-        credentialMaterialsType: CredentialMaterialsType.UsernamePasswordType,
-        credentialMaterialsContent: {
-          username: 'username',
-          password: 'password',
-        },
-      },
-    } as CredentialSavedObjectAttributes;
-
-    savedObjectsMock.get
-      .mockResolvedValueOnce({
-        id: DATA_SOURCE_ID,
-        type: DATA_SOURCE_SAVED_OBJECT_TYPE,
-        attributes: dataSourceAttr,
-        references: [{ name: 'user', type: CREDENTIAL_SAVED_OBJECT_TYPE, id: CREDENETIAL_ID }],
-      })
-      .mockResolvedValueOnce({
-        id: CREDENETIAL_ID,
-        type: CREDENTIAL_SAVED_OBJECT_TYPE,
-        attributes: crendentialAttr,
-        references: [],
-      });
+    savedObjectsMock.get.mockResolvedValueOnce({
+      id: DATA_SOURCE_ID,
+      type: DATA_SOURCE_SAVED_OBJECT_TYPE,
+      attributes: dataSourceAttr,
+      references: [],
+    });
 
     ClientMock.mockImplementation(() => {
       return dsClient;
@@ -96,11 +79,16 @@ describe('configureClient', () => {
     ClientMock.mockReset();
   });
 
-  test('configure client with noAuth == true, will call new Client() to create client', async () => {
+  test('configure client with auth.type == no_auth, will call new Client() to create client', async () => {
     savedObjectsMock.get.mockReset().mockResolvedValueOnce({
       id: DATA_SOURCE_ID,
       type: DATA_SOURCE_SAVED_OBJECT_TYPE,
-      attributes: { ...dataSourceAttr, noAuth: true },
+      attributes: {
+        ...dataSourceAttr,
+        auth: {
+          type: AuthType.NoAuth,
+        },
+      },
       references: [],
     });
 
@@ -122,7 +110,7 @@ describe('configureClient', () => {
     expect(client).toBe(dsClient.child.mock.results[0].value);
   });
 
-  test('configure client with noAuth == false, will first call decrypt()', async () => {
+  test('configure client with auth.type == username_password, will first call decrypt()', async () => {
     const spy = jest.spyOn(cryptoClient, 'decodeAndDecrypt').mockResolvedValue('password');
 
     const client = await configureClient(
@@ -135,7 +123,7 @@ describe('configureClient', () => {
     );
 
     expect(ClientMock).toHaveBeenCalledTimes(1);
-    expect(savedObjectsMock.get).toHaveBeenCalledTimes(2);
+    expect(savedObjectsMock.get).toHaveBeenCalledTimes(1);
     expect(spy).toHaveBeenCalledTimes(1);
     expect(client).toBe(dsClient.child.mock.results[0].value);
   });

diff --git a/src/plugins/data_source/server/client/configure_client.ts b/src/plugins/data_source/server/client/configure_client.ts
@@ -10,12 +10,13 @@ import {
   SavedObjectsClientContract,
   SavedObjectsErrorHelpers,
 } from '../../../../../src/core/server';
-import { DATA_SOURCE_SAVED_OBJECT_TYPE, CREDENTIAL_SAVED_OBJECT_TYPE } from '../../common';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common';
+
 import {
-  CredentialSavedObjectAttributes,
+  AuthType,
+  DataSourceAttributes,
   UsernamePasswordTypedContent,
-} from '../../common/credentials/types';
-import { DataSourceAttributes } from '../../common/data_sources';
+} from '../../common/data_sources';
 import { DataSourcePluginConfigType } from '../../config';
 import { CryptographyClient } from '../cryptography';
 import { parseClientOptions } from './client_config';
@@ -32,7 +33,7 @@ export const configureClient = async (
   const dataSource = await getDataSource(dataSourceId, savedObjects);
   const rootClient = getRootClient(dataSource.attributes, config, openSearchClientPoolSetup);
 
-  return getQueryClient(rootClient, dataSource, savedObjects, cryptographyClient);
+  return getQueryClient(rootClient, dataSource, cryptographyClient);
 };
 
 export const getDataSource = async (
@@ -52,19 +53,11 @@ export const getDataSource = async (
 };
 
 export const getCredential = async (
-  credentialId: string,
-  savedObjects: SavedObjectsClientContract,
+  dataSource: SavedObject<DataSourceAttributes>,
   cryptographyClient: CryptographyClient
 ): Promise<UsernamePasswordTypedContent> => {
   try {
-    const credentialSavedObject = await savedObjects.get<CredentialSavedObjectAttributes>(
-      CREDENTIAL_SAVED_OBJECT_TYPE,
-      credentialId
-    );
-    const {
-      username,
-      password,
-    } = credentialSavedObject.attributes.credentialMaterials.credentialMaterialsContent;
+    const { username, password } = dataSource.attributes.auth.credentials!;
     const decodedPassword = await cryptographyClient.decodeAndDecrypt(password);
     const credential = {
       username,
@@ -83,23 +76,19 @@ export const getCredential = async (
  *
  * @param rootClient root client for the connection with given data source endpoint.
  * @param dataSource data source saved object
- * @param savedObjects scoped saved object client
+ * @param cryptographyClient cryptography client for password encryption / decrpytion
  * @returns child client.
  */
 const getQueryClient = async (
   rootClient: Client,
   dataSource: SavedObject<DataSourceAttributes>,
-  savedObjects: SavedObjectsClientContract,
   cryptographyClient: CryptographyClient
 ): Promise<Client> => {
-  if (dataSource.attributes.noAuth) {
+  if (AuthType.NoAuth === dataSource.attributes.auth.type) {
     return rootClient.child();
   } else {
-    const credential = await getCredential(
-      dataSource.references[0].id,
-      savedObjects,
-      cryptographyClient
-    );
+    const credential = await getCredential(dataSource, cryptographyClient);
+
     return getBasicAuthClient(rootClient, credential);
   }
 };

diff --git a/src/plugins/data_source/server/plugin.ts b/src/plugins/data_source/server/plugin.ts
@@ -23,8 +23,10 @@ import { DataSourcePluginConfigType } from '../config';
 import { LoggingAuditor } from './audit/logging_auditor';
 import { CryptographyClient } from './cryptography';
 import { DataSourceService, DataSourceServiceSetup } from './data_source_service';
-import { credential, CredentialSavedObjectsClientWrapper, dataSource } from './saved_objects';
+import { DataSourceSavedObjectsClientWrapper, dataSource } from './saved_objects';
 import { DataSourcePluginSetup, DataSourcePluginStart } from './types';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../common';
+
 // eslint-disable-next-line @osd/eslint/no-restricted-paths
 import { ensureRawRequest } from '../../../../src/core/server/http/router';
 export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourcePluginStart> {
@@ -41,9 +43,6 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
   public async setup(core: CoreSetup) {
     this.logger.debug('data_source: Setup');
 
-    // Register credential saved object type
-    core.savedObjects.registerType(credential);
-
     // Register data source saved object type
     core.savedObjects.registerType(dataSource);
 
@@ -52,21 +51,21 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
     // Fetch configs used to create credential saved objects client wrapper
     const { wrappingKeyName, wrappingKeyNamespace, wrappingKey } = config.encryption;
 
-    // Create credential saved objects client wrapper
+    // Create data source saved objects client wrapper
     const cryptographyClient = new CryptographyClient(
       wrappingKeyName,
       wrappingKeyNamespace,
       wrappingKey
     );
-    const credentialSavedObjectsClientWrapper = new CredentialSavedObjectsClientWrapper(
+    const dataSourceSavedObjectsClientWrapper = new DataSourceSavedObjectsClientWrapper(
       cryptographyClient
     );
 
-    // Add credential saved objects client wrapper factory
+    // Add data source saved objects client wrapper factory
     core.savedObjects.addClientWrapper(
       1,
-      'credential',
-      credentialSavedObjectsClientWrapper.wrapperFactory
+      DATA_SOURCE_SAVED_OBJECT_TYPE,
+      dataSourceSavedObjectsClientWrapper.wrapperFactory
     );
 
     const dataSourceService: DataSourceServiceSetup = await this.dataSourceService.setup(config);