Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Manual Backport 2.x] Refactor getClient and getLegacyClient to use authentication method registry (#5881) #5940

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data_source/common/data_sources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes {
credentials: UsernamePasswordTypedContent | SigV4Content | undefined | AuthTypeContent;
};
lastUpdatedTime?: string;
name: AuthType | string;
}

export interface AuthTypeContent {
Expand All @@ -30,6 +31,7 @@ export interface SigV4Content extends SavedObjectAttributes {
secretKey: string;
region: string;
service?: SigV4ServiceName;
sessionToken?: string;
}

export interface UsernamePasswordTypedContent extends SavedObjectAttributes {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<IAuthenticationMethodRegistery>);

export const authenticationMethodRegisteryMock = { create };
2 changes: 2 additions & 0 deletions src/plugins/data_source/server/auth_registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ export {
IAuthenticationMethodRegistery,
AuthenticationMethodRegistery,
} from './authentication_methods_registry';

export { authenticationMethodRegisteryMock } from './authentication_methods_registry.mock';
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
56 changes: 54 additions & 2 deletions src/plugins/data_source/server/client/configure_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,24 @@ 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';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
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';

Expand All @@ -40,13 +48,15 @@ describe('configureClient', () => {
let usernamePasswordAuthContent: UsernamePasswordTypedContent;
let sigV4AuthContent: SigV4Content;
let customApiSchemaRegistry: CustomApiSchemaRegistry;
let authenticationMethodRegistery: jest.Mocked<IAuthenticationMethodRegistery>;

beforeEach(() => {
dsClient = opensearchClientMock.createInternalClient();
logger = loggingSystemMock.createLogger();
savedObjectsMock = savedObjectsClientMock.create();
cryptographyMock = cryptographyServiceSetupMock.create();
customApiSchemaRegistry = new CustomApiSchemaRegistry();
authenticationMethodRegistery = authenticationMethodRegisteryMock.create();

config = {
enabled: true,
Expand Down Expand Up @@ -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);
});
});
48 changes: 38 additions & 10 deletions src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 (
{
Expand All @@ -35,6 +37,8 @@ export const configureClient = async (
cryptography,
testClientDataSourceAttr,
customApiSchemaRegistryPromise,
request,
authRegistry,
}: DataSourceClientParams,
openSearchClientPoolSetup: OpenSearchClientPoolSetup,
config: DataSourcePluginConfigType,
Expand Down Expand Up @@ -80,6 +84,8 @@ export const configureClient = async (
cryptography,
rootClient,
dataSourceId,
request,
authRegistry,
requireDecryption
);
} catch (error: any) {
Expand All @@ -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
*/
Expand All @@ -112,15 +120,31 @@ const getQueryClient = async (
cryptography?: CryptographyServiceSetup,
rootClient?: Client,
dataSourceId?: string,
request?: OpenSearchDashboardsRequest,
authRegistry?: IAuthenticationMethodRegistery,
requireDecryption: boolean = true
): Promise<Client> => {
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);
Expand All @@ -129,21 +153,25 @@ 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);

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -29,6 +37,7 @@ describe('configureLegacyClient', () => {
let configOptions: ConfigOptions;
let dataSourceAttr: DataSourceAttributes;
let sigV4AuthContent: SigV4Content;
let authenticationMethodRegistery: jest.Mocked<IAuthenticationMethodRegistery>;

let mockOpenSearchClientInstance: {
close: jest.Mock;
Expand All @@ -48,6 +57,7 @@ describe('configureLegacyClient', () => {
logger = loggingSystemMock.createLogger();
savedObjectsMock = savedObjectsClientMock.create();
cryptographyMock = cryptographyServiceSetupMock.create();
authenticationMethodRegistery = authenticationMethodRegisteryMock.create();
config = {
enabled: true,
clientPool: {
Expand Down Expand Up @@ -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);
});
});
Loading
Loading