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

fix(NODE-6051): only provide expected allowed keys to libmongocrypt after fetching aws kms credentials #4057

Merged
merged 9 commits into from
Apr 4, 2024

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Mar 26, 2024

Description

What is changing?

  • AWS KMS credentials are now fetched using the same logic as AWS authentication (the AWSSDKTemporaryCredentialProvider).
  • Only expected keys (sessionToken,accessKeyId, secretAccessKey) are returned to libmongocrypt after fetching kms credentials

I also fixed some unit test skipping logic on Node18+, which previously only skipped on Node 18 and 20. Now it skips on Node18+.

This PR includes changes from #4064 to get a greener CI.

Is there new documentation needed for these changes?

Nope.

What is the motivation for this change?

Release Highlight

AWS credentials with expirations no longer throw when using on-demand AWS KMS credentials

In addition to letting users provide KMS credentials manually, client-side encryption supports fetching AWS KMS credentials on-demand using the AWS SDK. However, AWS credential mechanisms that returned access keys with expiration timestamps caused the driver to throw an error.

The driver will no longer throw an error when receiving an expiration token from the AWS SDK.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson changed the title signing commit object fix(NODE-6051): cache AWS credential provider during KMS refresh and only provide allowed keys to libmongocrypt Mar 26, 2024
@baileympearson baileympearson changed the title fix(NODE-6051): cache AWS credential provider during KMS refresh and only provide allowed keys to libmongocrypt fix(NODE-6051): only provide expected allowed keys to libmongocrypt after fetching aws kms credentials Mar 29, 2024
@baileympearson baileympearson marked this pull request as ready for review April 2, 2024 14:06
@W-A-James W-A-James self-assigned this Apr 2, 2024
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Apr 2, 2024
@W-A-James W-A-James self-requested a review April 2, 2024 19:05
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

LGTM apart from skip reason change and update to highlight

test/unit/connection_string.spec.test.ts Outdated Show resolved Hide resolved
Co-authored-by: Warren James <warren.james.dev@gmail.com>
@W-A-James W-A-James added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Apr 3, 2024
Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB left a comment

Choose a reason for hiding this comment

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

LGTM!

@W-A-James W-A-James merged commit c604e74 into mongodb:main Apr 4, 2024
18 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants