From 7dbb5a00a21730e448b5f02a4622c0b7286ebe2e Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 22:05:24 -0700 Subject: [PATCH] add endpoint validation to create data source API (#6631) (#6646) --- .../server/data_source_service.mock.ts | 16 +++++++ src/plugins/data_source/server/plugin.ts | 25 +++++----- .../routes/fetch_data_source_metadata.test.ts | 1 - ...ource_saved_objects_client_wrapper.test.ts | 44 ++++++++++++++++- ...ata_source_saved_objects_client_wrapper.ts | 47 +++++++++++++++++-- .../data_source/server/util/constants.ts | 6 +++ 6 files changed, 120 insertions(+), 19 deletions(-) create mode 100644 src/plugins/data_source/server/data_source_service.mock.ts create mode 100644 src/plugins/data_source/server/util/constants.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 new file mode 100644 index 000000000000..58145bddb506 --- /dev/null +++ b/src/plugins/data_source/server/data_source_service.mock.ts @@ -0,0 +1,16 @@ +/* + * 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 bbf5a89d1b53..f8741d8d3f32 100644 --- a/src/plugins/data_source/server/plugin.ts +++ b/src/plugins/data_source/server/plugin.ts @@ -61,16 +61,26 @@ 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 ); @@ -104,20 +114,12 @@ 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( - dataSourceService, + dataSourceServiceSetup, cryptographyServiceSetup, this.logger, auditTrailPromise, @@ -129,14 +131,14 @@ export class DataSourcePlugin implements Plugin, authRegistryPromise: Promise, customApiSchemaRegistryPromise: Promise diff --git a/src/plugins/data_source/server/routes/fetch_data_source_metadata.test.ts b/src/plugins/data_source/server/routes/fetch_data_source_metadata.test.ts index d361b89ddff7..d018af82ffee 100644 --- a/src/plugins/data_source/server/routes/fetch_data_source_metadata.test.ts +++ b/src/plugins/data_source/server/routes/fetch_data_source_metadata.test.ts @@ -16,7 +16,6 @@ import { registerFetchDataSourceMetaDataRoute } from './fetch_data_source_metada import { AuthType } from '../../common/data_sources'; // eslint-disable-next-line @osd/eslint/no-restricted-paths import { opensearchClientMock } from '../../../../../src/core/server/opensearch/client/mocks'; -import { index } from 'mathjs'; type SetupServerReturn = UnwrapPromise>; 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 4921218a2d48..9b9fe899a7a7 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,6 +14,10 @@ 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', () => { const customAuthName = 'role_based_auth'; @@ -33,11 +37,17 @@ 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 wrapperInstance = new DataSourceSavedObjectsClientWrapper( + dataSourceServiceSetup, cryptographyMock, logger, - authRegistryPromise + authRegistryPromise, + customApiSchemaRegistryPromise ); + const mockedClient = savedObjectsClientMock.create(); const wrapperClient = wrapperInstance.wrapperFactory({ client: mockedClient, @@ -69,6 +79,9 @@ 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({ @@ -76,6 +89,7 @@ describe('DataSourceSavedObjectsClientWrapper', () => { type: AuthType.NoAuth, }, }); + await wrapperClient.create( DATA_SOURCE_SAVED_OBJECT_TYPE, mockDataSourceAttributesWithNoAuth, @@ -200,6 +214,17 @@ describe('DataSourceSavedObjectsClientWrapper', () => { ).rejects.toThrowError(`"title" attribute must be a non-empty string`); }); + it(`should throw error when title is longer than ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters`, async () => { + const mockDataSourceAttributes = attributes({ + title: 'a'.repeat(33), + }); + await expect( + wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, mockDataSourceAttributes, {}) + ).rejects.toThrowError( + `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters` + ); + }); + it('should throw error when endpoint is not valid', async () => { const mockDataSourceAttributes = attributes({ endpoint: 'asasasasas', @@ -209,6 +234,23 @@ 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 25fdf6d59d75..e56536cb2aa5 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,6 +4,7 @@ */ import { + OpenSearchClient, SavedObjectsBulkCreateObject, SavedObjectsBulkResponse, SavedObjectsBulkUpdateObject, @@ -26,6 +27,10 @@ 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'; /** * Describes the Credential Saved Objects Client Wrapper class, @@ -139,9 +144,11 @@ export class DataSourceSavedObjectsClientWrapper { }; constructor( + private dataSourcesService: DataSourceServiceSetup, private cryptography: CryptographyServiceSetup, private logger: Logger, private authRegistryPromise: Promise, + private customApiSchemaRegistryPromise: Promise, private endpointBlockedIps?: string[] ) {} @@ -252,26 +259,56 @@ export class DataSourceSavedObjectsClientWrapper { private async validateAttributes(attributes: T) { const { title, endpoint, auth } = attributes; - if (!title?.trim?.().length) { + + this.validateTitle(title); + await this.validateEndpoint(endpoint, attributes as DataSourceAttributes); + 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) { + throw SavedObjectsErrorHelpers.createBadRequestError( + `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters` + ); + } + } + + private async validateEndpoint(endpoint: string, attributes: DataSourceAttributes) { 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, + authRegistry: await this.authRegistryPromise, + customApiSchemaRegistryPromise: this.customApiSchemaRegistryPromise, + }); - if (!auth) { - throw SavedObjectsErrorHelpers.createBadRequestError('"auth" attribute is required'); - } + const dataSourceValidator = new DataSourceConnectionValidator(dataSourceClient, attributes); - await this.validateAuth(auth); + await dataSourceValidator.validate(); + } catch (err: any) { + 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) { diff --git a/src/plugins/data_source/server/util/constants.ts b/src/plugins/data_source/server/util/constants.ts new file mode 100644 index 000000000000..0914e1a25fdd --- /dev/null +++ b/src/plugins/data_source/server/util/constants.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export const DATA_SOURCE_TITLE_LENGTH_LIMIT = 32;