From 028bec3fe59c62737ccbec42e062aee005def1fc Mon Sep 17 00:00:00 2001 From: Zhongnan Su Date: Tue, 4 Jun 2024 00:22:21 +0000 Subject: [PATCH] Remove endpoint validation for create data source saved object API Signed-off-by: Zhongnan Su --- .../server/data_source_service.mock.ts | 16 ---- src/plugins/data_source/server/plugin.ts | 24 +++--- .../routes/fetch_data_source_metadata.test.ts | 1 + ...ource_saved_objects_client_wrapper.test.ts | 52 +------------ ...ata_source_saved_objects_client_wrapper.ts | 73 ++++--------------- 5 files changed, 29 insertions(+), 137 deletions(-) delete mode 100644 src/plugins/data_source/server/data_source_service.mock.ts diff --git a/src/plugins/data_source/server/data_source_service.mock.ts b/src/plugins/data_source/server/data_source_service.mock.ts deleted file mode 100644 index 58145bddb506..000000000000 --- a/src/plugins/data_source/server/data_source_service.mock.ts +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ -// eslint-disable-next-line @osd/eslint/no-restricted-paths -import { opensearchClientMock } from '../../../../src/core/server/opensearch/client/mocks'; -import { DataSourceServiceSetup } from './data_source_service'; - -const dataSourceClient = opensearchClientMock.createInternalClient(); -const create = () => - (({ - getDataSourceClient: jest.fn(() => Promise.resolve(dataSourceClient)), - getDataSourceLegacyClient: jest.fn(), - } as unknown) as jest.Mocked); - -export const dataSourceServiceSetupMock = { create }; diff --git a/src/plugins/data_source/server/plugin.ts b/src/plugins/data_source/server/plugin.ts index 229adf2bbe7b..bbf5a89d1b53 100644 --- a/src/plugins/data_source/server/plugin.ts +++ b/src/plugins/data_source/server/plugin.ts @@ -61,26 +61,16 @@ export class DataSourcePlugin implements Plugin { const dataSourcePluginStart = selfStart as DataSourcePluginStart; return dataSourcePluginStart.getAuthenticationMethodRegistry(); }); - const auditTrailPromise = core.getStartServices().then(([coreStart]) => coreStart.auditTrail); - const customApiSchemaRegistryPromise = core.getStartServices().then(([, , selfStart]) => { - const dataSourcePluginStart = selfStart as DataSourcePluginStart; - return dataSourcePluginStart.getCustomApiSchemaRegistry(); - }); const dataSourceSavedObjectsClientWrapper = new DataSourceSavedObjectsClientWrapper( - dataSourceServiceSetup, cryptographyServiceSetup, this.logger.get('data-source-saved-objects-client-wrapper-factory'), authRegistryPromise, - customApiSchemaRegistryPromise, config.endpointDeniedIPs ); @@ -114,12 +104,20 @@ export class DataSourcePlugin implements Plugin coreStart.auditTrail); + + const dataSourceService: DataSourceServiceSetup = await this.dataSourceService.setup(config); + + const customApiSchemaRegistryPromise = core.getStartServices().then(([, , selfStart]) => { + const dataSourcePluginStart = selfStart as DataSourcePluginStart; + return dataSourcePluginStart.getCustomApiSchemaRegistry(); + }); // Register data source plugin context to route handler context core.http.registerRouteHandlerContext( 'dataSource', this.createDataSourceRouteHandlerContext( - dataSourceServiceSetup, + dataSourceService, cryptographyServiceSetup, this.logger, auditTrailPromise, @@ -131,14 +129,14 @@ export class DataSourcePlugin implements Plugin>; diff --git a/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts b/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts index d3d082a73391..4bb84825d703 100644 --- a/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts +++ b/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts @@ -14,9 +14,6 @@ import { AuthType } from '../../common/data_sources'; import { cryptographyServiceSetupMock } from '../cryptography_service.mocks'; import { DataSourceSavedObjectsClientWrapper } from './data_source_saved_objects_client_wrapper'; import { SavedObject } from 'opensearch-dashboards/public'; -import { dataSourceServiceSetupMock } from '../data_source_service.mock'; -import { CustomApiSchemaRegistry } from '../schema_registry'; -import { DataSourceConnectionValidator } from '../routes/data_source_connection_validator'; import { DATA_SOURCE_TITLE_LENGTH_LIMIT } from '../util/constants'; describe('DataSourceSavedObjectsClientWrapper', () => { @@ -37,23 +34,16 @@ describe('DataSourceSavedObjectsClientWrapper', () => { const cryptographyMock = cryptographyServiceSetupMock.create(); const logger = loggingSystemMock.createLogger(); const authRegistryPromise = Promise.resolve(authRegistry); - const customApiSchemaRegistry = new CustomApiSchemaRegistry(); - const customApiSchemaRegistryPromise = Promise.resolve(customApiSchemaRegistry); - const dataSourceServiceSetup = dataSourceServiceSetupMock.create(); - const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); const wrapperInstance = new DataSourceSavedObjectsClientWrapper( - dataSourceServiceSetup, cryptographyMock, logger, - authRegistryPromise, - customApiSchemaRegistryPromise + authRegistryPromise ); - const mockedClient = savedObjectsClientMock.create(); const wrapperClient = wrapperInstance.wrapperFactory({ client: mockedClient, typeRegistry: requestHandlerContext.savedObjects.typeRegistry, - request: requestMock, + request: httpServerMock.createOpenSearchDashboardsRequest(), }); const getSavedObject = (savedObject: Partial) => { @@ -80,9 +70,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => { describe('createWithCredentialsEncryption', () => { beforeEach(() => { mockedClient.create.mockClear(); - jest - .spyOn(DataSourceConnectionValidator.prototype, 'validate') - .mockResolvedValue(Promise.resolve() as Promise); }); it('should create data source when auth type is NO_AUTH', async () => { const mockDataSourceAttributesWithNoAuth = attributes({ @@ -90,7 +77,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => { type: AuthType.NoAuth, }, }); - await wrapperClient.create( DATA_SOURCE_SAVED_OBJECT_TYPE, mockDataSourceAttributesWithNoAuth, @@ -205,23 +191,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => { ).rejects.toThrowError(`Invalid auth type: 'not_in_registry': Bad Request`); }); - it('endpoint validator datasource client should be created with request as param', async () => { - const mockDataSourceAttributesWithNoAuth = attributes({ - auth: { - type: AuthType.NoAuth, - }, - }); - - await wrapperClient.create( - DATA_SOURCE_SAVED_OBJECT_TYPE, - mockDataSourceAttributesWithNoAuth, - {} - ); - expect(dataSourceServiceSetup.getDataSourceClient).toBeCalledWith( - expect.objectContaining({ request: requestMock }) - ); - }); - describe('createWithCredentialsEncryption: Error handling', () => { it('should throw error when title is empty', async () => { const mockDataSourceAttributes = attributes({ @@ -252,23 +221,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => { ).rejects.toThrowError(`"endpoint" attribute is not valid or allowed`); }); - it('should throw error when endpoint is not valid OpenSearch endpoint', async () => { - const mockDataSourceAttributes = attributes({ - auth: { - type: AuthType.NoAuth, - }, - }); - jest - .spyOn(DataSourceConnectionValidator.prototype, 'validate') - .mockImplementationOnce(() => { - throw new Error(); - }); - - await expect( - wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, mockDataSourceAttributes, {}) - ).rejects.toThrowError(`endpoint is not valid OpenSearch endpoint: Bad Request`); - }); - it('should throw error when auth is not present', async () => { await expect( wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes(), {}) diff --git a/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts b/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts index 35990f05777f..f69b20331a1c 100644 --- a/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts +++ b/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts @@ -4,8 +4,6 @@ */ import { - OpenSearchClient, - OpenSearchDashboardsRequest, SavedObjectsBulkCreateObject, SavedObjectsBulkResponse, SavedObjectsBulkUpdateObject, @@ -28,9 +26,6 @@ import { import { EncryptionContext, CryptographyServiceSetup } from '../cryptography_service'; import { isValidURL } from '../util/endpoint_validator'; import { IAuthenticationMethodRegistry } from '../auth_registry'; -import { DataSourceServiceSetup } from '../data_source_service'; -import { CustomApiSchemaRegistry } from '../schema_registry'; -import { DataSourceConnectionValidator } from '../routes/data_source_connection_validator'; import { DATA_SOURCE_TITLE_LENGTH_LIMIT } from '../util/constants'; /** @@ -52,10 +47,7 @@ export class DataSourceSavedObjectsClientWrapper { return await wrapperOptions.client.create(type, attributes, options); } - const encryptedAttributes = await this.validateAndEncryptAttributes( - attributes, - wrapperOptions.request - ); + const encryptedAttributes = await this.validateAndEncryptAttributes(attributes); return await wrapperOptions.client.create(type, encryptedAttributes, options); }; @@ -74,7 +66,7 @@ export class DataSourceSavedObjectsClientWrapper { return { ...object, - attributes: await this.validateAndEncryptAttributes(attributes, wrapperOptions.request), + attributes: await this.validateAndEncryptAttributes(attributes), }; }) ); @@ -148,19 +140,14 @@ export class DataSourceSavedObjectsClientWrapper { }; constructor( - private dataSourcesService: DataSourceServiceSetup, private cryptography: CryptographyServiceSetup, private logger: Logger, private authRegistryPromise: Promise, - private customApiSchemaRegistryPromise: Promise, private endpointBlockedIps?: string[] ) {} - private async validateAndEncryptAttributes( - attributes: T, - request?: OpenSearchDashboardsRequest - ) { - await this.validateAttributes(attributes, request); + private async validateAndEncryptAttributes(attributes: T) { + await this.validateAttributes(attributes); const { endpoint, auth } = attributes; @@ -264,14 +251,19 @@ export class DataSourceSavedObjectsClientWrapper { } } - private async validateAttributes( - attributes: T, - request?: OpenSearchDashboardsRequest - ) { + private async validateAttributes(attributes: T) { const { title, endpoint, auth } = attributes; - this.validateTitle(title); - await this.validateEndpoint(endpoint, attributes as DataSourceAttributes, request); + if (!isValidURL(endpoint, this.endpointBlockedIps)) { + throw SavedObjectsErrorHelpers.createBadRequestError( + '"endpoint" attribute is not valid or allowed' + ); + } + + if (!auth) { + throw SavedObjectsErrorHelpers.createBadRequestError('"auth" attribute is required'); + } + await this.validateAuth(auth); } @@ -289,42 +281,7 @@ export class DataSourceSavedObjectsClientWrapper { } } - private async validateEndpoint( - endpoint: string, - attributes: DataSourceAttributes, - request?: OpenSearchDashboardsRequest - ) { - if (!isValidURL(endpoint, this.endpointBlockedIps)) { - throw SavedObjectsErrorHelpers.createBadRequestError( - '"endpoint" attribute is not valid or allowed' - ); - } - try { - const dataSourceClient: OpenSearchClient = await this.dataSourcesService.getDataSourceClient({ - savedObjects: {} as any, - cryptography: this.cryptography, - testClientDataSourceAttr: attributes as DataSourceAttributes, - request, - authRegistry: await this.authRegistryPromise, - customApiSchemaRegistryPromise: this.customApiSchemaRegistryPromise, - }); - - const dataSourceValidator = new DataSourceConnectionValidator(dataSourceClient, attributes); - - await dataSourceValidator.validate(); - } catch (err: any) { - this.logger.error(err); - throw SavedObjectsErrorHelpers.createBadRequestError( - `endpoint is not valid OpenSearch endpoint` - ); - } - } - private async validateAuth(auth: T) { - if (!auth) { - throw SavedObjectsErrorHelpers.createBadRequestError('"auth" attribute is required'); - } - const { type, credentials } = auth; if (!type) {