DRIVERS-3207: Custom AWS credential providers execute earlier #1838
Conversation
durran
force-pushed
the
DRIVERS-3207
branch
2 times, most recently
from
174d5cb to
b2ae9b3
Compare
| If the driver supports a custom AWS credential provider, it MUST verify the custom provider was used when testing. This | ||
| may be via a custom function or object that wraps the calls to the custom provider and asserts that it was called at | ||
| least once. | ||
| ## Testing custom credential providers |
There was a problem hiding this comment.
Do we have any tests that test the precedence that drivers use when fetching credentials?a
There was a problem hiding this comment.
We do not, as our drivers tools scripts only set up one type of credential per test, not multiples.
source/auth/auth.md
Outdated
| 1. A custom AWS credential provider if the driver supports it. | ||
| 2. The URI | ||
| 3. Environment variables |
There was a problem hiding this comment.
I would expect that the URI takes precedence over any dynamic credential loading mechanism and that the custom credential provider should be the first place we look for credentials, if they're not explicitly provided.
Thoughts?
There was a problem hiding this comment.
I think that is the fundamental question posed in this spec change (the DRIVERS ticket AC leaves this open ended) - it may be helpful to have this conversation in slack with the auth spec owners;
notably, for encryption, the URI isn't relevant, but this spec change as a whole would affect automatic credential fetching - should we add a test there, too? (e.g., see Case 15 and 26)
There was a problem hiding this comment.
I'm not too concerned about it here, since we won't even look at the URI after DRIVERS-3131 is implemented. This is all assuming in this case that the auth mechanism is MONGODB-AWS - I still think even before DRIVERS-3131 if a custom provider is supplied, that it should take preference over all, since it's provided as an option to the client and not a URI option, and we've set the standard that programmatic options always take preference over URI options.
There was a problem hiding this comment.
We do have a standard that equivalent MongoClient options take precedence over URI options, but we don't have precedent for a MongoClient option to take precedence over a related but different URI option. The current behavior is such that if credentials are provided in the URI, we do not attempt to fetch credentials. While it is true that Node will drop support for explicitly specified credentials, other drivers might not (they need to wait for a major release) and so we do need to consider explicitly provided credentials here.
My thought is that we should preserve the existing behavior: explicitly provided credentials first, then attempt to fetch credentials. If fetching credentials, use the custom provider first.
There was a problem hiding this comment.
Ok I've moved URI options back to #1 now.
source/auth/auth.md
Outdated
| 1. A custom AWS credential provider if the driver supports it. | ||
| 2. The URI | ||
| 3. Environment variables |
There was a problem hiding this comment.
I think that is the fundamental question posed in this spec change (the DRIVERS ticket AC leaves this open ended) - it may be helpful to have this conversation in slack with the auth spec owners;
notably, for encryption, the URI isn't relevant, but this spec change as a whole would affect automatic credential fetching - should we add a test there, too? (e.g., see Case 15 and 26)
| 3. A custom AWS credential provider if the driver supports it. | ||
| 1. A custom AWS credential provider if the driver supports it. | ||
| 2. The URI | ||
| 3. Environment variables | ||
| 4. Using `AssumeRoleWithWebIdentity` if `AWS_WEB_IDENTITY_TOKEN_FILE` and `AWS_ROLE_ARN` are set. | ||
| 5. The ECS endpoint if `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` is set. Otherwise, the EC2 endpoint. | ||
|
|
There was a problem hiding this comment.
Let's make sure to remember to update the changelog
|
I have added a prose test for FLE in case 26. Case 15 I don't believe is relevant as it pertains to automatic credential fetching, which means nothing is provided. Even with the changes in this PR the order on automatic credential fetching is the same. |
|
Added link. |
durran
changed the title
Updates the AWS auth spec to require drivers that implement custom AWS credential providers to use the custom provider before any other method.
Updates test wording to include the extra scenario for testing a custom provider when explicit credentials are also provided.
Node implementation: mongodb/node-mongodb-native#4656
Please complete the following before merging:
clusters).