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

[Multiple Datasource] Handle form values(request payload) if the selected type is available in the authentication registry. #6049

Merged
Show file tree
Hide file tree
Changes from 8 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Dynamic Configurations] Add support for dynamic application configurations ([#5855](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5855))
- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949))
- [Multiple Datasource] Refactoring create and edit form to use authentication registry ([#6002](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6002))
- [Multiple Datasource] Send registered authentication credentials during the dataSource creation and editing processes ([#6049](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6049))
Copy link
Member

Choose a reason for hiding this comment

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

I would say something simpler, e.g. "Handle form values(request payload) if the selected type is available in the authentication registry." or something like that. Same for PR title.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment~
Will using the suggested title.


### 🐛 Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,13 @@ describe('Datasource Management: Create Datasource form with registered Auth Typ
const mockSubmitHandler = jest.fn();
const mockTestConnectionHandler = jest.fn();
const mockCancelHandler = jest.fn();
const changeTextFieldValue = (testSubjId: string, value: string) => {
component.find(testSubjId).last().simulate('change', {
target: {
value,
},
});
};

test('should call registered crendential form at the first round when registered method is at the first place and username & password disabled', () => {
const mockCredentialForm = jest.fn();
Expand Down Expand Up @@ -517,4 +524,49 @@ describe('Datasource Management: Create Datasource form with registered Auth Typ
expect(mockCredentialForm).not.toHaveBeenCalled();
});
});

test('should create data source with registered Auth when all fields are valid', () => {
const mockCredentialForm = jest.fn();
const authMethodToBeTested = {
name: 'Some Auth Type',
credentialSourceOption: {
value: 'Some Auth Type',
inputDisplay: 'some input',
},
credentialForm: mockCredentialForm,
crendentialFormField: {
userNameRegistered: 'some filled in userName from registed auth credential form',
passWordRegistered: 'some filled in password from registed auth credential form',
},
} as AuthenticationMethod;

const mockedContext = mockManagementPlugin.createDataSourceManagementContext();
mockedContext.authenticationMethodRegistery = new AuthenticationMethodRegistery();
mockedContext.authenticationMethodRegistery.registerAuthenticationMethod(authMethodToBeTested);

component = mount(
wrapWithIntl(
<CreateDataSourceForm
handleTestConnection={mockTestConnectionHandler}
handleSubmit={mockSubmitHandler}
handleCancel={mockCancelHandler}
existingDatasourceNamesList={['dup20']}
/>
),
{
wrappingComponent: OpenSearchDashboardsContextProvider,
wrappingComponentProps: {
services: mockedContext,
},
}
);

changeTextFieldValue(titleIdentifier, 'test');
changeTextFieldValue(descriptionIdentifier, 'test');
changeTextFieldValue(endpointIdentifier, 'https://test.com');

findTestSubject(component, 'createDataSourceButton').simulate('click');

expect(mockSubmitHandler).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ import {
isTitleValid,
performDataSourceFormValidation,
} from '../../../validation';
import { getDefaultAuthMethod, isValidUrl } from '../../../utils';
import {
extractRegisteredAuthTypeCredentials,
getDefaultAuthMethod,
isValidUrl,
} from '../../../utils';

export interface CreateDataSourceProps {
existingDatasourceNamesList: string[];
Expand All @@ -55,8 +59,12 @@ export interface CreateDataSourceState {
description: string;
endpoint: string;
auth: {
type: AuthType;
credentials: UsernamePasswordTypedContent | SigV4Content | undefined;
type: AuthType | string;
credentials:
| UsernamePasswordTypedContent
| SigV4Content
| { [key: string]: string }
xinruiba marked this conversation as resolved.
Show resolved Hide resolved
| undefined;
};
}

Expand Down Expand Up @@ -102,7 +110,12 @@ export class CreateDataSourceForm extends React.Component<
/* Validations */

isFormValid = () => {
return performDataSourceFormValidation(this.state, this.props.existingDatasourceNamesList, '');
return performDataSourceFormValidation(
this.state,
this.props.existingDatasourceNamesList,
'',
this.authenticationMethodRegistery
);
};

/* Events */
Expand Down Expand Up @@ -298,22 +311,29 @@ export class CreateDataSourceForm extends React.Component<

getFormValues = (): DataSourceAttributes => {
let credentials = this.state.auth.credentials;
if (this.state.auth.type === AuthType.UsernamePasswordType) {
const authType = this.state.auth.type;

if (authType === AuthType.NoAuth) {
credentials = {};
} else if (authType === AuthType.UsernamePasswordType) {
credentials = {
username: this.state.auth.credentials.username,
password: this.state.auth.credentials.password,
} as UsernamePasswordTypedContent;
}
if (this.state.auth.type === AuthType.SigV4) {
} else if (authType === AuthType.SigV4) {
credentials = {
region: this.state.auth.credentials.region,
accessKey: this.state.auth.credentials.accessKey,
secretKey: this.state.auth.credentials.secretKey,
service: this.state.auth.credentials.service || SigV4ServiceName.OpenSearch,
} as SigV4Content;
}
if (this.state.auth.type === AuthType.NoAuth) {
credentials = {};
} else {
const currentCredentials = (credentials ?? {}) as { [key: string]: string };
credentials = extractRegisteredAuthTypeCredentials(
currentCredentials,
authType,
this.authenticationMethodRegistery
);
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
mockManagementPlugin,
existingDatasourceNamesList,
mockDataSourceAttributesWithNoAuth,
mockDataSourceAttributesWithRegisteredAuth,
} from '../../../../mocks';
import { OpenSearchDashboardsContextProvider } from '../../../../../../opensearch_dashboards_react/public';
import { EditDataSourceForm } from './edit_data_source_form';
Expand Down Expand Up @@ -344,17 +345,33 @@ describe('Datasource Management: Edit Datasource Form', () => {

describe('With Registered Authentication', () => {
let component: ReactWrapper<any, Readonly<{}>, React.Component<{}, {}, any>>;
const mockCredentialForm = jest.fn();
const updateInputFieldAndBlur = (
comp: ReactWrapper<any, Readonly<{}>, React.Component<{}, {}, any>>,
fieldName: string,
updatedValue: string,
isTestSubj?: boolean
) => {
const field = isTestSubj ? comp.find(fieldName) : comp.find({ name: fieldName });
act(() => {
field.last().simulate('change', { target: { value: updatedValue } });
});
comp.update();
act(() => {
field.last().simulate('focus').simulate('blur');
});
comp.update();
};

test('should call registered crendential form', () => {
const mockedCredentialForm = jest.fn();
const authTypeToBeTested = 'Some Auth Type';
const authMethodToBeTest = {
name: authTypeToBeTested,
credentialSourceOption: {
value: authTypeToBeTested,
inputDisplay: 'some input',
},
credentialForm: mockCredentialForm,
credentialForm: mockedCredentialForm,
} as AuthenticationMethod;

const mockedContext = mockManagementPlugin.createDataSourceManagementContext();
Expand All @@ -380,6 +397,69 @@ describe('With Registered Authentication', () => {
}
);

expect(mockCredentialForm).toHaveBeenCalled();
expect(mockedCredentialForm).toHaveBeenCalled();
});

test('should update the form with registered auth type on click save changes', async () => {
const mockedCredentialForm = jest.fn();
const mockedSubmitHandler = jest.fn();
const authMethodToBeTest = {
name: 'Some Auth Type',
credentialSourceOption: {
value: 'Some Auth Type',
inputDisplay: 'some input',
},
credentialForm: mockedCredentialForm,
crendentialFormField: {},
} as AuthenticationMethod;

const mockedContext = mockManagementPlugin.createDataSourceManagementContext();
mockedContext.authenticationMethodRegistery = new AuthenticationMethodRegistery();
mockedContext.authenticationMethodRegistery.registerAuthenticationMethod(authMethodToBeTest);

component = mount(
wrapWithIntl(
<EditDataSourceForm
existingDataSource={mockDataSourceAttributesWithRegisteredAuth}
existingDatasourceNamesList={existingDatasourceNamesList}
onDeleteDataSource={jest.fn()}
handleSubmit={mockedSubmitHandler}
handleTestConnection={jest.fn()}
displayToastMessage={jest.fn()}
/>
),
{
wrappingComponent: OpenSearchDashboardsContextProvider,
wrappingComponentProps: {
services: mockedContext,
},
}
);

await new Promise((resolve) =>
setTimeout(() => {
updateInputFieldAndBlur(component, descriptionFieldIdentifier, '');
expect(
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

why do we need //@ts-ignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, will remove it.

component.find(descriptionFormRowIdentifier).first().props().isInvalid
).toBeUndefined();
xinruiba marked this conversation as resolved.
Show resolved Hide resolved
resolve();
}, 100)
);
await new Promise((resolve) =>
setTimeout(() => {
/* Updated description*/
updateInputFieldAndBlur(component, descriptionFieldIdentifier, 'testDescription');
expect(
// @ts-ignore
component.find(descriptionFormRowIdentifier).first().props().isInvalid
).toBeUndefined();
xinruiba marked this conversation as resolved.
Show resolved Hide resolved

expect(component.find('[data-test-subj="datasource-edit-saveButton"]').exists()).toBe(true);
component.find('[data-test-subj="datasource-edit-saveButton"]').first().simulate('click');
expect(mockedSubmitHandler).toHaveBeenCalled();
resolve();
}, 100)
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
} from '../../../validation';
import { UpdatePasswordModal } from '../update_password_modal';
import { UpdateAwsCredentialModal } from '../update_aws_credential_modal';
import { getDefaultAuthMethod } from '../../../utils';
import { extractRegisteredAuthTypeCredentials, getDefaultAuthMethod } from '../../../utils';

export interface EditDataSourceProps {
existingDataSource: DataSourceAttributes;
Expand All @@ -60,8 +60,12 @@ export interface EditDataSourceState {
description: string;
endpoint: string;
auth: {
type: AuthType;
credentials: UsernamePasswordTypedContent | SigV4Content;
type: AuthType | string;
credentials:
| UsernamePasswordTypedContent
| SigV4Content
| { [key: string]: string }
| undefined;
BionIT marked this conversation as resolved.
Show resolved Hide resolved
};
showUpdatePasswordModal: boolean;
showUpdateAwsCredentialModal: boolean;
Expand Down Expand Up @@ -155,7 +159,8 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
return performDataSourceFormValidation(
this.state,
this.props.existingDatasourceNamesList,
this.props.existingDataSource.title
this.props.existingDataSource.title,
this.authenticationMethodRegistery
);
};

Expand Down Expand Up @@ -362,6 +367,14 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
delete formValues.auth.credentials?.password;
break;
default:
const currentCredentials = (this.state.auth.credentials ?? {}) as {
[key: string]: string;
};
formValues.auth.credentials = extractRegisteredAuthTypeCredentials(
currentCredentials,
this.state.auth.type,
this.authenticationMethodRegistery
);
break;
}

Expand Down
Loading
Loading