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

[MD] Fix server sider endpoint validation by passing in request when creating datasource client #6822

Merged
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
2 changes: 2 additions & 0 deletions changelogs/fragments/6822.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Fix endpoint validation by passing in request when creating datasource client ([#6822](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6822))
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
const customApiSchemaRegistry = new CustomApiSchemaRegistry();
const customApiSchemaRegistryPromise = Promise.resolve(customApiSchemaRegistry);
const dataSourceServiceSetup = dataSourceServiceSetupMock.create();
const requestMock = httpServerMock.createOpenSearchDashboardsRequest();
const wrapperInstance = new DataSourceSavedObjectsClientWrapper(
dataSourceServiceSetup,
cryptographyMock,
Expand All @@ -52,7 +53,7 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
const wrapperClient = wrapperInstance.wrapperFactory({
client: mockedClient,
typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
request: httpServerMock.createOpenSearchDashboardsRequest(),
request: requestMock,
});

const getSavedObject = (savedObject: Partial<SavedObject>) => {
Expand Down Expand Up @@ -204,6 +205,23 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import {
OpenSearchClient,
OpenSearchDashboardsRequest,
SavedObjectsBulkCreateObject,
SavedObjectsBulkResponse,
SavedObjectsBulkUpdateObject,
Expand Down Expand Up @@ -51,7 +52,10 @@ export class DataSourceSavedObjectsClientWrapper {
return await wrapperOptions.client.create(type, attributes, options);
}

const encryptedAttributes = await this.validateAndEncryptAttributes(attributes);
const encryptedAttributes = await this.validateAndEncryptAttributes(
attributes,
wrapperOptions.request
);

return await wrapperOptions.client.create(type, encryptedAttributes, options);
};
Expand All @@ -70,7 +74,7 @@ export class DataSourceSavedObjectsClientWrapper {

return {
...object,
attributes: await this.validateAndEncryptAttributes(attributes),
attributes: await this.validateAndEncryptAttributes(attributes, wrapperOptions.request),
};
})
);
Expand Down Expand Up @@ -152,8 +156,11 @@ export class DataSourceSavedObjectsClientWrapper {
private endpointBlockedIps?: string[]
) {}

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

const { endpoint, auth } = attributes;

Expand Down Expand Up @@ -257,11 +264,14 @@ export class DataSourceSavedObjectsClientWrapper {
}
}

private async validateAttributes<T = unknown>(attributes: T) {
private async validateAttributes<T = unknown>(
attributes: T,
request?: OpenSearchDashboardsRequest
) {
const { title, endpoint, auth } = attributes;

this.validateTitle(title);
await this.validateEndpoint(endpoint, attributes as DataSourceAttributes);
await this.validateEndpoint(endpoint, attributes as DataSourceAttributes, request);
await this.validateAuth(auth);
}

Expand All @@ -279,7 +289,11 @@ export class DataSourceSavedObjectsClientWrapper {
}
}

private async validateEndpoint(endpoint: string, attributes: DataSourceAttributes) {
private async validateEndpoint(
endpoint: string,
attributes: DataSourceAttributes,
request?: OpenSearchDashboardsRequest
) {
if (!isValidURL(endpoint, this.endpointBlockedIps)) {
throw SavedObjectsErrorHelpers.createBadRequestError(
'"endpoint" attribute is not valid or allowed'
Expand All @@ -290,6 +304,7 @@ export class DataSourceSavedObjectsClientWrapper {
savedObjects: {} as any,
cryptography: this.cryptography,
testClientDataSourceAttr: attributes as DataSourceAttributes,
request,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this.dataSourcesService.getDataSourceClient already supports this option being passed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's an optional param tho

authRegistry: await this.authRegistryPromise,
customApiSchemaRegistryPromise: this.customApiSchemaRegistryPromise,
});
Expand All @@ -298,6 +313,7 @@ export class DataSourceSavedObjectsClientWrapper {

await dataSourceValidator.validate();
} catch (err: any) {
this.logger.error(err);
throw SavedObjectsErrorHelpers.createBadRequestError(
`endpoint is not valid OpenSearch endpoint`
);
Expand Down
Loading