Skip to content

Commit

Permalink
Remove endpoint validation for create data source saved object API
Browse files Browse the repository at this point in the history
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
  • Loading branch information
zhongnansu committed Jun 4, 2024
1 parent 766a39a commit 028bec3
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 137 deletions.
16 changes: 0 additions & 16 deletions src/plugins/data_source/server/data_source_service.mock.ts

This file was deleted.

24 changes: 11 additions & 13 deletions src/plugins/data_source/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,16 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
const cryptographyServiceSetup: CryptographyServiceSetup = this.cryptographyService.setup(
config
);
const dataSourceServiceSetup: DataSourceServiceSetup = await this.dataSourceService.setup(
config
);

const authRegistryPromise = core.getStartServices().then(([, , selfStart]) => {
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
);

Expand Down Expand Up @@ -114,12 +104,20 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
},
};
core.auditTrail.register(auditorFactory);
const auditTrailPromise = core.getStartServices().then(([coreStart]) => 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,
Expand All @@ -131,14 +129,14 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
const router = core.http.createRouter();
registerTestConnectionRoute(
router,
dataSourceServiceSetup,
dataSourceService,
cryptographyServiceSetup,
authRegistryPromise,
customApiSchemaRegistryPromise
);
registerFetchDataSourceMetaDataRoute(
router,
dataSourceServiceSetup,
dataSourceService,
cryptographyServiceSetup,
authRegistryPromise,
customApiSchemaRegistryPromise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ 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<ReturnType<typeof setupServer>>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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<SavedObject>) => {
Expand All @@ -80,17 +70,13 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
describe('createWithCredentialsEncryption', () => {
beforeEach(() => {
mockedClient.create.mockClear();
jest
.spyOn(DataSourceConnectionValidator.prototype, 'validate')
.mockResolvedValue(Promise.resolve() as Promise<any>);
});
it('should create data source when auth type is NO_AUTH', async () => {
const mockDataSourceAttributesWithNoAuth = attributes({
auth: {
type: AuthType.NoAuth,
},
});

await wrapperClient.create(
DATA_SOURCE_SAVED_OBJECT_TYPE,
mockDataSourceAttributesWithNoAuth,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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(), {})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
*/

import {
OpenSearchClient,
OpenSearchDashboardsRequest,
SavedObjectsBulkCreateObject,
SavedObjectsBulkResponse,
SavedObjectsBulkUpdateObject,
Expand All @@ -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';

/**
Expand All @@ -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);
};
Expand All @@ -74,7 +66,7 @@ export class DataSourceSavedObjectsClientWrapper {

return {
...object,
attributes: await this.validateAndEncryptAttributes(attributes, wrapperOptions.request),
attributes: await this.validateAndEncryptAttributes(attributes),
};
})
);
Expand Down Expand Up @@ -148,19 +140,14 @@ export class DataSourceSavedObjectsClientWrapper {
};

constructor(
private dataSourcesService: DataSourceServiceSetup,
private cryptography: CryptographyServiceSetup,
private logger: Logger,
private authRegistryPromise: Promise<IAuthenticationMethodRegistry>,
private customApiSchemaRegistryPromise: Promise<CustomApiSchemaRegistry>,
private endpointBlockedIps?: string[]
) {}

private async validateAndEncryptAttributes<T = unknown>(
attributes: T,
request?: OpenSearchDashboardsRequest
) {
await this.validateAttributes(attributes, request);
private async validateAndEncryptAttributes<T = unknown>(attributes: T) {
await this.validateAttributes(attributes);

const { endpoint, auth } = attributes;

Expand Down Expand Up @@ -264,14 +251,19 @@ export class DataSourceSavedObjectsClientWrapper {
}
}

private async validateAttributes<T = unknown>(
attributes: T,
request?: OpenSearchDashboardsRequest
) {
private async validateAttributes<T = unknown>(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);
}

Expand All @@ -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<T = unknown>(auth: T) {
if (!auth) {
throw SavedObjectsErrorHelpers.createBadRequestError('"auth" attribute is required');
}

const { type, credentials } = auth;

if (!type) {
Expand Down

0 comments on commit 028bec3

Please sign in to comment.