Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .evergreen/setup-mongodb-aws-auth-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ cd $DRIVERS_TOOLS/.evergreen/auth_aws
# Create a python virtual environment.
. ./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

cd $BEFORE

Expand Down
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "drivers-evergreen-tools"]
path = drivers-evergreen-tools
url = https://github.com/mongodb-labs/drivers-evergreen-tools.git
url = https://github.com/mongodb-labs/drivers-evergreen-tools.git
1 change: 1 addition & 0 deletions src/cmap/auth/mongo_credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface AuthMechanismProperties extends Document {
SERVICE_NAME?: string;
SERVICE_REALM?: string;
CANONICALIZE_HOST_NAME?: GSSAPICanonicalizationValue;
/** @internal */
AWS_SESSION_TOKEN?: string;
/** A user provided OIDC machine callback function. */
OIDC_CALLBACK?: OIDCCallbackFunction;
Expand Down
10 changes: 4 additions & 6 deletions src/cmap/auth/mongodb_aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,10 @@ export class MongoDBAWS extends AuthProvider {
);
}

if (!authContext.credentials.username) {
authContext.credentials = await makeTempCredentials(
authContext.credentials,
this.credentialFetcher
);
}
authContext.credentials = await makeTempCredentials(
authContext.credentials,
this.credentialFetcher
);

const { credentials } = authContext;

Expand Down
12 changes: 12 additions & 0 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,18 @@ export function parseOptions(
);
}

if (isAws) {
const { username, password } = mongoOptions.credentials;
if (username || password) {
throw new MongoParseError(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're trying to move away from using MongoParseError for options that we are able to read but that happen to be invalid (see NODE-4015), can we go with MongoAPIError for both of the new instances here?

'username and password cannot be provided when using MONGODB-AWS'
);
}
if (mongoOptions.credentials.mechanismProperties.AWS_SESSION_TOKEN) {
throw new MongoParseError('AWS_SESSION_TOKEN cannot be provided when using MONGODB-AWS');
Copy link
Contributor

Choose a reason for hiding this comment

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

can we update both of these messages to hint at the correct way to provide the credentials?

}
}

mongoOptions.credentials.validate();

// Check if the only auth related option provided was authSource, if so we can remove credentials
Expand Down
4 changes: 0 additions & 4 deletions test/integration/node-specific/examples/aws_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ const { MongoClient } = require('mongodb');
// options. Note that MongoClient now auto-connects so no need to store the connect()
// promise anywhere and reference it.
const client = new MongoClient(process.env.MONGODB_URI, {
auth: {
username: process.env.AWS_ACCESS_KEY_ID,
password: process.env.AWS_SECRET_ACCESS_KEY
},
authSource: '$external',
authMechanism: 'MONGODB-AWS'
});
Expand Down
15 changes: 15 additions & 0 deletions test/spec/auth/legacy/connection-string.json
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,21 @@
}
}
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

bookmark for corresponding spec update

"description": "should throw an exception if username provided (MONGODB-AWS) implies default mechanism)",
"uri": "mongodb://user:localhost.com/",
"valid": false
},
{
"description": "should throw an exception if username and password provided (MONGODB-AWS) implies default mechanism)",
"uri": "mongodb://user@pass:localhost.com/",
"valid": false
},
{
"description": "should throw an exception if AWS_SESSION_TOKEN provided (MONGODB-AWS) implies default mechanism)",
"uri": "mongodb://localhost/?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token",
"valid": false
},
{
"description": "should recognise the mechanism with test environment (MONGODB-OIDC)",
"uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test",
Expand Down
14 changes: 13 additions & 1 deletion test/spec/auth/legacy/connection-string.yml
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,18 @@ tests:
mechanism: MONGODB-AWS
mechanism_properties:
AWS_SESSION_TOKEN: token!@#$%^&*()_+
- description: should throw an exception if username provided (MONGODB-AWS)
Copy link
Contributor

Choose a reason for hiding this comment

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

bookmark for corresponding spec update

implies default mechanism)
uri: mongodb://user:localhost.com/
valid: false
- description: should throw an exception if username and password provided (MONGODB-AWS)
implies default mechanism)
uri: mongodb://user@pass:localhost.com/
valid: false
- description: should throw an exception if AWS_SESSION_TOKEN provided (MONGODB-AWS)
implies default mechanism)
uri: mongodb://localhost/?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token
valid: false
- description: should recognise the mechanism with test environment (MONGODB-OIDC)
uri: mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test
valid: true
Expand Down Expand Up @@ -468,4 +480,4 @@ tests:
(MONGODB-OIDC)
uri: mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s
valid: false
credential: null
credential: null
10 changes: 10 additions & 0 deletions test/unit/assorted/auth.spec.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
import { loadSpecTests } from '../../spec';
import { executeUriValidationTest } from '../../tools/uri_spec_runner';

const SKIP = [
'should throw an exception if username and no password (MONGODB-AWS)',
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect we can leave this one unskipped, since it should continue to fail (albeit for a different reason), but no strong preference since we should get the same coverage from the newly added test

'should use username and password if specified (MONGODB-AWS)',
'should use username, password and session token if specified (MONGODB-AWS)'
];

describe('Auth option spec tests (legacy)', function () {
const suites = loadSpecTests('auth', 'legacy');

for (const suite of suites) {
describe(suite.name, function () {
for (const test of suite.tests) {
it(`${test.description}`, function () {
if (SKIP.includes(test.description)) {
this.test.skipReason = `NODE-7046: ${test.description}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

bookmark for the deletion ticket (once the DRIVERS ticket is filed, we can generate the language tickets and then add the NODE ticket reference here)

this.test.skip();
}
executeUriValidationTest(test);
});
}
Expand Down