-
Notifications
You must be signed in to change notification settings - Fork 246
feat(DRIVERS-2903): use custom aws configuration #1743
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
Conversation
e4b7aa5
to
00dbb01
Compare
55956fe
to
51081d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few quick edits while I perused this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this needs to be supported in FLE as well, I think we need to update the FLE spec and specify how to configure the "aws" kms provider. Consider adding a similar property with same semantics to AWSKMSOptions
in https://github.com/mongodb/specifications/blob/master/source/client-side-encryption/client-side-encryption.md#kmsproviders.
(While in some drivers you might be able to apply an auth mechanism property to FLE, in Java that isn't possible as auth mechanism properties are associated with a credential, not the MongoClient, and the credential of the MongoClient might not even be an AWS credential (or could be a different one).
@kevinAlbs Do you have thoughts on this? Would there be a case where auto-encryption could want a different AWS credential provider than the associated MongoClient. (With ClientEncryption this is already possible since it has its own MongoClient) |
@durran What about users who aren't using AWS authentication for their clients but want to use AWS automatic KMS credential fetching with a custom provider? I don't think Node would support that without a provider option for KMS specifically. |
1 similar comment
@durran What about users who aren't using AWS authentication for their clients but want to use AWS automatic KMS credential fetching with a custom provider? I don't think Node would support that without a provider option for KMS specifically. |
That's a good point and I think a valid use case. I'll update everything accordingly. |
cc @kevinAlbs since this affects Client Encryption options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, two minor clarifying comments. I'm not an auth or FLE spec owner though.
Pinging @jyemin as Java has an interest here. @kevinAlbs as I've added FLE options for the custom credential providers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how we will define the types in the Java driver, as applications are allowed to use either AWS SDK v1 or v2, and those dependencies are optional so the types can't be part of the API. But we will figure out a way to make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Suggested wording changes, but nothing behavior changing.
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Co-authored-by: Kevin Albertson <[email protected]>
Adds section to auth spec and CSFLE spec on allowing user provided custom credential providers for AWS and notes on testing.
Node implementation: mongodb/node-mongodb-native#4373
clusters, and serverless).