From 47bd3917ed6bcdcfc384c9e625bc387ce6c8def9 Mon Sep 17 00:00:00 2001 From: Zhongnan Su Date: Wed, 5 Jun 2024 11:49:47 -0700 Subject: [PATCH] [MD]Remove endpoint validation for create data source saved object API (#6899) * Remove endpoint validation for create data source saved object API Signed-off-by: Zhongnan Su * Changeset file for PR #6899 created/updated --------- Signed-off-by: Zhongnan Su Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/6899.yml | 2 + .../server/data_source_service.mock.ts | 16 ----- src/plugins/data_source/server/plugin.ts | 24 +++---- ...ource_saved_objects_client_wrapper.test.ts | 52 +------------- ...ata_source_saved_objects_client_wrapper.ts | 68 ++++--------------- 5 files changed, 29 insertions(+), 133 deletions(-) create mode 100644 changelogs/fragments/6899.yml delete mode 100644 src/plugins/data_source/server/data_source_service.mock.ts diff --git a/changelogs/fragments/6899.yml b/changelogs/fragments/6899.yml new file mode 100644 index 000000000000..fe930956760c --- /dev/null +++ b/changelogs/fragments/6899.yml @@ -0,0 +1,2 @@ +feat: +- Remove endpoint validation for create data source saved object API ([#6899](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6899)) \ No newline at end of file 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 { @@ -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..fc49983fed39 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,58 +251,31 @@ 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); + this.validateEndpoint(endpoint); await this.validateAuth(auth); } - private validateTitle(title: string) { - if (!title.trim().length) { - throw SavedObjectsErrorHelpers.createBadRequestError( - '"title" attribute must be a non-empty string' - ); - } - - if (title.length > DATA_SOURCE_TITLE_LENGTH_LIMIT) { + private validateEndpoint(endpoint: string) { + if (!isValidURL(endpoint, this.endpointBlockedIps)) { throw SavedObjectsErrorHelpers.createBadRequestError( - `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters` + '"endpoint" attribute is not valid or allowed' ); } } - private async validateEndpoint( - endpoint: string, - attributes: DataSourceAttributes, - request?: OpenSearchDashboardsRequest - ) { - if (!isValidURL(endpoint, this.endpointBlockedIps)) { + private validateTitle(title: string) { + if (!title.trim().length) { throw SavedObjectsErrorHelpers.createBadRequestError( - '"endpoint" attribute is not valid or allowed' + '"title" attribute must be a non-empty string' ); } - 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); + if (title.length > DATA_SOURCE_TITLE_LENGTH_LIMIT) { throw SavedObjectsErrorHelpers.createBadRequestError( - `endpoint is not valid OpenSearch endpoint` + `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters` ); } }