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

feat(NODE-6094) RFC: Allow the Mongo-AWS creds provider to handle assuming an AWS IAM role. #4081

Closed

Conversation

jwhitaker-gridcog
Copy link

@jwhitaker-gridcog jwhitaker-gridcog commented Apr 17, 2024

Description

What is changing?

The MONGODB-AWS credential provider will now check if you pass a var AWS_ROLE_ARN as part of your auth mechanism properties. If you do, it will assume that role for you using the AWS SDK's credential provider, which will handle session expiry and re-authentication.

Is there new documentation needed for these changes?

Yes.

What is the motivation for this change?

In our org, we're using the mongo creds provider with an assumed role. We are assuming the role ourselves and passing the resulting key, secret, and session token to the mongo connector. This works fine...

... until the STS session expires.

At this point we need to implement full expiry and reconnection logic ourselves. This sucks - it's finicky, and the code to handle it already exists in both this repository and in the aws-sdk, probably more reliably than we'll ever manage.
It's also annoying because it has to exist outside of the connection pool, and must kill the whole pool. This is the sort of thing a pool should manage for you.

Because of this, we'd really like if the Mongo connector could handle assuming a role for the user. That way we don't need to do all this invalidation + reconnection stuff ourselves, and it lives inside the pool where it belongs. As a selling point to you guys, it actually lives in the aws sdks, not mongo. There is no reconnection logic added as part of this PR as none is needed with this approach.

I haven't set up automated tests with this approach yet, I'd ask for a go/no-go before going to that effort :)

Thanks for your consideration!

Code notes

Most of the changes are just to pass through the authmechanismproperties to where they are needed. This is the first commit.

You'll see the second commit, actually doing the role assumption, is quite small.

Release Highlight

Fill in title or leave empty for no highlight

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

@durran
Copy link
Member

durran commented Apr 17, 2024

Hi @jwhitaker-gridcog thanks for the PR. We'll discuss and then update you here.

@aditi-khare-mongoDB
Copy link
Contributor

Hi @jwhitaker-gridcog, thanks again for submitting this PR!

We will be closing this PR. Since AuthMechanismProperties type is part of the drivers specification, it can't be changed without a specifications change (see here). I've filed a follow-up ticket NODE-6141 to handle this issue in a spec compliant way and the specifications owners have been notified of this issue, as well, to see if any what changes should be made drivers-wide.

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