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

[FEATURE] Add child client support AwsSigv4 #696

Closed
bandinib-amzn opened this issue Jan 16, 2024 · 7 comments
Closed

[FEATURE] Add child client support AwsSigv4 #696

bandinib-amzn opened this issue Jan 16, 2024 · 7 comments
Assignees

Comments

@bandinib-amzn
Copy link
Member

Currently we have child client support for Basic Authentication. Similarly we should have child client support for AwsSigv4.

@dblock
Copy link
Member

dblock commented Jan 17, 2024

Do you mean this code isn't propagating sigv4 options?

child(opts) {

@bandinib-amzn
Copy link
Member Author

Yes. Below lines of code will only work for Basic Auth.

opensearch-js/index.js

Lines 269 to 271 in 5ecad79

if (options.auth !== undefined) {
options.headers = prepareHeaders(options.headers, options.auth);
}

I think type of auth should also support static IAM credentials authentication methods (accesskey and secret key)

auth?: BasicAuth;

For basic auth, prepareHeaders method updates header of child client with nee credentials. Similarly we can signed the request with credentials in case of IAM credentials.

options.headers = prepareHeaders(options.headers, options.auth);

function prepareHeaders(headers = {}, auth) {
if (auth != null && headers.authorization == null) {
/* istanbul ignore else */
if (auth.username && auth.password) {
headers.authorization =
'Basic ' + Buffer.from(`${auth.username}:${auth.password}`).toString('base64');
}
}
return headers;

This is just off the top of my head. But we can build PoC and propose the solution.

@dblock
Copy link
Member

dblock commented Jan 17, 2024

When is child even used and why is it not a full, deep clone of the client instance?

@dblock
Copy link
Member

dblock commented Jan 17, 2024

@bandinib-amzn Want to help? Try to implement a unit test for your scenario alone and maybe a fix?

@bandinib-amzn
Copy link
Member Author

@bandinib-amzn Want to help? Try to implement a unit test for your scenario alone and maybe a fix?

Sure. I can give a try.

@bandinib-amzn
Copy link
Member Author

When is child even used and why is it not a full, deep clone of the client instance?

It is being used in multi data source plugin. We discovered this issue when we wanted to implement similar in AWSSigV4 authentication method.
https://github.com/opensearch-project/OpenSearch-Dashboards/blob/e83b7ee42831d7e62b81ad906b998ff035355640/src/plugins/data_source/server/client/configure_client.ts#L145-L160

@bandinib-amzn
Copy link
Member Author

@bandinib-amzn Want to help? Try to implement a unit test for your scenario alone and maybe a fix?

Hi @dblock. I raised PR. Can you please take a look when you get a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants