Skip to content

Conversation

durran
Copy link
Member

@durran durran commented Sep 28, 2025

Description

Removes the ability to provide credentials when using MONGODB-AWS authentication.

Summary of Changes

  • Updates drivers tools to get the --nouri option for aws_tester.py
  • Sets up CI environments without credentials in the MONGODB_URI
  • Skips no longer relevant legacy auth tests.
  • Adds new tests in the legacy auth tests for testing error conditions
  • Removes support for providing credentials in the URI or options when auth mech is MONGODB-AWS.
Notes for Reviewers

I made AWS_SESSION_TOKEN internal instead of removing it completely as we still use that object and property internally when fetching credentials.

What is the motivation for this change?

NODE-7046/DRIVERS-3131

Release Highlight

Explicitly Provided Credentials No Longer Accepted With MONGODB-AWS Authentication

AWS environments (such as AWS Lambda) do not have credentials that are permanent and expire within a set amount of time. Providing credentials in the URI or options would mandate that those credentials would be valid for the life of the MongoClient, which is problematic. With this change, the installed required AWS SDK will now fetch credentials using the environment, endpoints, or a custom credential provider.

This means that for AWS authentication, all client URIs MUST now be specified as:

import { MongoClient } from 'mongodb';

const client = new MongoClient('mongodb<+srv>://<host>:<port>/?authMechanism=MONGODB-AWS');

Double check the following

  • Lint is passing (npm run check:lint)
  • 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 durran changed the title feat(NODE-7046): remove AWS uri support feat(NODE-7046): remove AWS uri/options support Sep 30, 2025
@durran durran marked this pull request as ready for review September 30, 2025 16:39
@durran durran requested a review from a team as a code owner September 30, 2025 16:39
@dariakp dariakp self-assigned this Oct 1, 2025
@dariakp dariakp added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Oct 1, 2025
@durran durran requested a review from dariakp October 6, 2025 20:00
@durran durran changed the title feat(NODE-7046): remove AWS uri/options support feat(NODE-7046)!: remove AWS uri/options support Oct 7, 2025

// Get the URI for the cluster then set AWS_ACCESS_KEY_ID as the username in the
// URI and AWS_SECRET_ACCESS_KEY as the password, then set the appropriate auth
// options. Note that MongoClient now auto-connects so no need to store the connect()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to update the text in the comments to talk about doing this via env vars instead

const { username, password } = mongoOptions.credentials;
if (username || password) {
throw new MongoAPIError(
'username and password cannot be provided when using MONGODB-AWS. Credentials must be read via the AWS SDK'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'username and password cannot be provided when using MONGODB-AWS. Credentials must be read via the AWS SDK'
'username and password cannot be provided directly when using MONGODB-AWS. Credentials must be read via the AWS SDK'

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm hesitant to accept this change as the addition of the word "directly" to me indicates that a username and password could be provided in another fashion. In the case of AWS authentication username and password do not really apply in any form. Same with the comment on the other error message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Open to different phrasing, but I wanted to make sure that we differentiate between the valid and invalid manner of providing the credentials, "via the sdk" isn't really actionable for users, so perhaps something along the lines "in a manner that can be read by the AWS SDK" and/or "via environment variables or a custom credential provider". Does that make sense?

}
if (mongoOptions.credentials.mechanismProperties.AWS_SESSION_TOKEN) {
throw new MongoAPIError(
'AWS_SESSION_TOKEN cannot be provided when using MONGODB-AWS. Credentials must be read via the AWS SDK'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'AWS_SESSION_TOKEN cannot be provided when using MONGODB-AWS. Credentials must be read via the AWS SDK'
'AWS_SESSION_TOKEN cannot be provided directly when using MONGODB-AWS. Credentials must be read via the AWS SDK'

. ./activate-authawsvenv.sh
# Source the environment variables. Configure the environment and the server.
. aws_setup.sh $AWS_CREDENTIAL_TYPE
. aws_setup.sh --nouri $AWS_CREDENTIAL_TYPE
Copy link
Contributor

Choose a reason for hiding this comment

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

is there some logging we can add that confirms that the aws_tester.py is correctly running with that option set? I think the current version of the script here should be passing something in, but currently nothing is printing in the run (e.g. here)

Copy link
Member Author

Choose a reason for hiding this comment

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

We could file a ticket to update the tools script to log this, but it would be obvious if the option was set or not given the tests. In this PR if the option wasn't set the driver would be throwing an error since MONGODB_URI would have the credentials set. (But we don't want to log what our URIs are in CI or we redact them)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful to do this anyway for debugging purposes in the future - can you go ahead and file the corresponding ticket?

import { loadSpecTests } from '../../spec';
import { executeUriValidationTest } from '../../tools/uri_spec_runner';

const SKIP = [
Copy link
Contributor

@dariakp dariakp Oct 8, 2025

Choose a reason for hiding this comment

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

I don't have a good file to comment on, but I was looking for changes to the prose tests, and noticed that our prose tests don't follow the convention - the relevant tests for this are in mongodb_aws.test.ts, where I think we can delete the Case 1 block. Can we move the prose tests from that file to mongodb_aws.prose.test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Primary Review In Review with primary reviewer, not yet ready for team's eyes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants