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

Adds a session token to AWS Credentials #6103

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

bandinib-amzn
Copy link
Member

@bandinib-amzn bandinib-amzn commented Mar 11, 2024

Description

The new interface in the data source plugin, named registerCredentialProvider makes it easier for users to set up their own authentication methods and mechanism to fetch the credentials. This feature helps with role-based authentication, where users can create temporary AWS credentials, including session tokens. While users can design their own credential provider, they rely on the data source plugin to authenticate requests and return the client. Therefore, it's important to add support for session tokens.

Issues Resolved

Partially resolves #5838, #5696

Screenshot

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 67.13%. Comparing base (735424b) to head (ade9ac7).
Report is 2 commits behind head on main.

❗ Current head ade9ac7 differs from pull request most recent head 880c9e6. Consider uploading reports for the commit 880c9e6 to get more accurate results

Files Patch % Lines
...gins/data_source/server/client/configure_client.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6103   +/-   ##
=======================================
  Coverage   67.13%   67.13%           
=======================================
  Files        3324     3324           
  Lines       64318    64327    +9     
  Branches    10344    10345    +1     
=======================================
+ Hits        43178    43185    +7     
- Misses      18617    18618    +1     
- Partials     2523     2524    +1     
Flag Coverage Δ
Linux_1 31.66% <ø> (ø)
Linux_2 55.23% <ø> (ø)
Linux_3 44.70% <92.85%> (+<0.01%) ⬆️
Linux_4 35.08% <ø> (ø)
Windows_1 31.68% <ø> (ø)
Windows_2 55.20% <ø> (ø)
Windows_3 44.72% <92.85%> (+0.01%) ⬆️
Windows_4 35.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seraphjiang
Copy link
Member

seraphjiang commented Mar 11, 2024

@bandinib-amzn would you add a little background why we has to add session token to support AWS Credentials which is missed before.

is it a optional parameter or required?

@bandinib-amzn
Copy link
Member Author

@bandinib-amzn would you add a little background why we has to add session token to support AWS Credentials which is missed before.

is it a optional parameter or required?

Added more details in description. It is optional parameter.

@@ -199,11 +200,12 @@ const getBasicAuthClient = (
};

const getAWSClient = (credential: SigV4Content, clientOptions: ClientOptions): Client => {
const { accessKey, secretKey, region, service } = credential;
const { accessKey, secretKey, region, service, sessionToken } = credential;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would assume we need to use sts to generate the sessionToken, and since session token has limited time, we need to generate it every time we use the IAM cred, will it be addressed in a separate PR?

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, right. But we are not providing any predefined auth methods in core data source plugin which will generate session token. But user can build their own auth method in plugin or any other component where they can use sts to generate the sessionToken and register auth methods and provides credentials using registerCredentialProvider but as I said in overview, while users can design their own credential provider, they rely on the data source plugin to authenticate requests and return the client. Therefore, it's important to add support for session tokens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it, thanks!!!

Signed-off-by: Bandini Bhopi <[email protected]>
@bandinib-amzn bandinib-amzn merged commit 6d882c9 into opensearch-project:main Mar 11, 2024
54 of 55 checks passed
@bandinib-amzn bandinib-amzn deleted the aws-sigv4 branch March 11, 2024 18:18
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 11, 2024
* Adds session token for aws connection

Signed-off-by: Bandini Bhopi <[email protected]>

* Adds changelog

Signed-off-by: Bandini Bhopi <[email protected]>

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 6d882c9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
bandinib-amzn pushed a commit that referenced this pull request Mar 11, 2024
* Adds session token for aws connection

Signed-off-by: Bandini Bhopi <[email protected]>

* Adds changelog

Signed-off-by: Bandini Bhopi <[email protected]>

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 6d882c9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ZilongX pushed a commit that referenced this pull request Mar 11, 2024
* Adds session token for aws connection

Signed-off-by: Bandini Bhopi <[email protected]>

* Adds changelog

Signed-off-by: Bandini Bhopi <[email protected]>

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 6d882c9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Meta] Refactoring data source plugin to support add-on authentication method with plug-in module
4 participants