feat(NODE-4929): Add OIDC Azure workflow #3670

durran · 2023-05-12T20:27:31Z

Description

Adds the ability for OIDC auth to use the Azure Identity Provider to get an access token.

What is changing?

Adds an AzureServiceWorkflow that gets a token from the Azure Instance Metadata Service
Adds an AzureTokenCache for caching Azure tokens separate from the callback cache.
Adds a new auth mechanism property TOKEN_AUDIENCE, that indicates the resource to query for the token.
Adds new Azure OIDC auth prose tests.

Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-4929 / DRIVERS-2416

Spec PR: mongodb/specifications#1421

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 · 2023-05-22T22:22:43Z

src/cmap/auth/mongodb_aws.ts

@@ -253,61 +251,3 @@ function deriveRegion(host: string) {

  return parts[1];
 }
-


Moving this all out of the AWS module into utils as Azure will reuse this as well as GCP.

durran · 2023-05-22T22:24:03Z

src/cmap/auth/mongodb_oidc/azure_service_workflow.ts

+const TOKEN_AUDIENCE_MISSING_ERROR = 'TOKEN_AUDIENCE must be set in the environment.';
+
+/** Base URL for getting Azure tokens. */
+const AZURE_BASE_URL =


Once we bring CSFLE into the driver we can move these constants into a common Azure shared module.

Sonuds good, wanna add a bullet to the FLE ticket calling that out, just something simple as "consolidate constants" would make us remember to go looking for dupes.

We also have an azure token cache in FLE ( cc: @baileympearson ) allegedly there could be a way to combine these as well.

I've updated https://jira.mongodb.org/browse/NODE-5283 AC to note this now.

durran · 2023-05-22T22:25:29Z

src/cmap/auth/mongodb_oidc/cache.ts

+/**
+ * An entry in a cache that can expire in a certain amount of time.
+ */
+export abstract class ExpiringCacheEntry {


Creating this class as we now have 2 caches that have expiring entries of the same expiration time. (Azure, Callbacks)

durran · 2023-05-22T22:26:56Z

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

  /**
   * Get the callbacks for the connection and credentials. If an entry does not
   * exist a new one will get set.
   */
-  getCallbacks(connection: Connection, credentials: MongoCredentials): CallbacksEntry {
+  getEntry(connection: Connection, credentials: MongoCredentials): CallbacksEntry {


Cleaning up the cache interfaces to all use the "entry" terminology.

durran · 2023-05-25T20:40:54Z

.evergreen/run-oidc-tests.sh

+  MONGODB_URI="${MONGODB_URI}/?authMechanism=MONGODB-OIDC"
+  MONGODB_URI="${MONGODB_URI}&authMechanismProperties=PROVIDER_NAME:azure"
+  export MONGODB_URI="${MONGODB_URI},TOKEN_AUDIENCE:api%3A%2F%2F${AZUREOIDC_CLIENTID}"
+  npm run check:oidc-azure


Azure needs to run its prose tests in a separate environment so they are split out.

With the GCP/Azure KMS, the tests live in the integration folder but only run on the specialized hosts/environment due to beforeEach skipping. It seems like we're approaching the specialized environment stuff in different ways, I don't feel strongly but should we do away with the manual/ dir (for this work) in favor of encoding things into mocha?

I've moved to test/integration/auth and put the skip logic in now.

durran · 2023-05-25T20:42:47Z

src/cmap/auth/mongodb_oidc/token_entry_cache.ts

    this.tokenResult = tokenResult;
    this.serverInfo = serverInfo;
-    this.expiration = expiration;


This moved to ExpiringCacheEntry

durran · 2023-05-25T20:44:31Z

src/connection_string.ts

@@ -203,7 +203,7 @@ function getUIntFromOptions(name: string, value: unknown): number {
 function* entriesFromString(value: string): Generator<[string, string]> {
  const keyValuePairs = value.split(',');
  for (const keyValue of keyValuePairs) {
-    const [key, value] = keyValue.split(':');
+    const [key, value] = keyValue.split(/:(.*)/);


This change is to handle authMechanismProperties such as TOKEN_AUDIENCE:api://foo where we need the entire string after the first ":"

… TIL that .split() inserts capture groups into the split result! :)

Yeah that came in handy. :)

addaleax · 2023-05-26T11:01:31Z

src/cmap/auth/mongo_credentials.ts

@@ -30,6 +31,7 @@ function getDefaultAuthMechanism(hello?: Document): AuthMechanism {
  return AuthMechanism.MONGODB_CR;
 }

+const ALLOWED_PROVIDER_NAMES = ['aws', 'azure'];


Suggested change

const ALLOWED_PROVIDER_NAMES = ['aws', 'azure'];

const ALLOWED_PROVIDER_NAMES: AuthMechanismProperties['PROVIDER_NAME'][] = ['aws', 'azure'];

(tiny suggestion for type safety)

Added suggestion.

.evergreen/config.in.yml

.evergreen/generate_evergreen_tasks.js

nbbeeken · 2023-05-26T18:02:03Z

.evergreen/run-oidc-tests.sh

+  MONGODB_URI="${MONGODB_URI}/?authMechanism=MONGODB-OIDC"
+  MONGODB_URI="${MONGODB_URI}&authMechanismProperties=PROVIDER_NAME:azure"
+  export MONGODB_URI="${MONGODB_URI},TOKEN_AUDIENCE:api%3A%2F%2F${AZUREOIDC_CLIENTID}"
+  npm run check:oidc-azure


With the GCP/Azure KMS, the tests live in the integration folder but only run on the specialized hosts/environment due to beforeEach skipping. It seems like we're approaching the specialized environment stuff in different ways, I don't feel strongly but should we do away with the manual/ dir (for this work) in favor of encoding things into mocha?

nbbeeken · 2023-05-26T18:03:31Z

src/cmap/auth/mongodb_oidc/azure_service_workflow.ts

+const TOKEN_AUDIENCE_MISSING_ERROR = 'TOKEN_AUDIENCE must be set in the environment.';
+
+/** Base URL for getting Azure tokens. */
+const AZURE_BASE_URL =


Sonuds good, wanna add a bullet to the FLE ticket calling that out, just something simple as "consolidate constants" would make us remember to go looking for dupes.

We also have an azure token cache in FLE ( cc: @baileympearson ) allegedly there could be a way to combine these as well.

src/cmap/auth/mongodb_oidc/azure_service_workflow.ts

nbbeeken · 2023-05-26T18:21:52Z

src/cmap/auth/mongodb_oidc/cache.ts

+/**
+ * An OIDC token cache.
+ */
+export interface Cache {
+  /**
+   * Implement the cache key for the token.
+   */
+  cacheKey(address: string, username: string, callbackHash: string): string;
+}


Seems like the abstract class already covers enforcing the cacheKey method exists on all subclasses, what is this providing?

Abstract class is now called Cache and this interface is removed.

test/manual/mongodb_oidc_azure.prose.test.ts

src/cmap/auth/mongodb_oidc/cache.ts

test/integration/auth/mongodb_oidc_azure.prose.test.ts

durran mentioned this pull request May 12, 2023

PYTHON-3467 OIDC: Automatic token acquisition for Azure Identity Provider mongodb/mongo-python-driver#1209

Closed

durran force-pushed the NODE-4929 branch 5 times, most recently from 948b7bb to 5c5318e Compare May 22, 2023 21:11

durran commented May 22, 2023

View reviewed changes

durran force-pushed the NODE-4929 branch from 5c6d31c to 12e8f8b Compare May 25, 2023 20:28

durran commented May 25, 2023

View reviewed changes

durran marked this pull request as ready for review May 25, 2023 21:19

addaleax reviewed May 26, 2023

View reviewed changes

nbbeeken requested changes May 26, 2023

View reviewed changes

durran force-pushed the NODE-4929 branch from 4f867bb to 19ddbed Compare May 30, 2023 13:28

durran requested a review from nbbeeken May 30, 2023 20:02

durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label May 31, 2023

durran assigned nbbeeken May 31, 2023

nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels May 31, 2023

durran force-pushed the NODE-4929 branch from 968ee46 to c40e801 Compare May 31, 2023 18:28

nbbeeken previously approved these changes May 31, 2023

View reviewed changes

durran force-pushed the NODE-4929 branch from c40e801 to 24b52a4 Compare May 31, 2023 19:29

W-A-James self-requested a review June 2, 2023 13:30

W-A-James reviewed Jun 2, 2023

View reviewed changes

src/cmap/auth/mongodb_oidc/cache.ts Outdated Show resolved Hide resolved

durran dismissed nbbeeken’s stale review via 413333b June 2, 2023 14:57

durran force-pushed the NODE-4929 branch from 24b52a4 to 413333b Compare June 2, 2023 14:57

durran added 16 commits June 6, 2023 14:47

test: more debug

c71adf1

test: move validation

de3d9f0

test: await fail point

29288c6

test: monitor commands

fb316d7

chore: remove console logging

25ec3da

test: fix script

a32bd39

test: fix unit test

fce0ccd

fix: more mech properties type safety

d160ac5

test: use drivers tools master

95e17e7

test: update per suggestions

0b4e3b6

test: fix prose test

506e6b6

test: fix ubuntu distro

29ed901

test: run correct script

23be573

test: use auth dir

236a64c

test: use subprocess

384eaa1

chore: update comment

0064fdd

durran dismissed W-A-James’s stale review via 0064fdd June 6, 2023 12:47

durran force-pushed the NODE-4929 branch from 60dfa59 to 0064fdd Compare June 6, 2023 12:47

durran requested a review from nbbeeken June 6, 2023 12:47

nbbeeken requested changes Jun 6, 2023

View reviewed changes

test/integration/auth/mongodb_oidc_azure.prose.test.ts Outdated Show resolved Hide resolved

fix: lint

d901e52

durran requested a review from nbbeeken June 6, 2023 14:51

nbbeeken merged commit b3482f3 into main Jun 6, 2023
17 of 26 checks passed

nbbeeken deleted the NODE-4929 branch June 6, 2023 19:52

This was referenced Jun 8, 2023

chore(main): release 5.7.0 [skip-ci] #3716

Closed

chore(main): release 5.7.0 [skip-ci] #3718

Closed

github-actions bot mentioned this pull request Jun 21, 2023

chore(main): release 5.7.0 [skip-ci] #3735

Merged

malikj2000 pushed a commit that referenced this pull request Jun 29, 2023

feat(NODE-4929): Add OIDC Azure workflow (#3670)

448148b

This was referenced Aug 21, 2023

chore(5.x): release 5.8.0 [skip-ci] #3825

Merged

chore(main): release 6.0.0 [skip-ci] #3762

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(NODE-4929): Add OIDC Azure workflow #3670

feat(NODE-4929): Add OIDC Azure workflow #3670

durran commented May 12, 2023 •

edited

durran May 22, 2023

durran May 22, 2023

nbbeeken May 26, 2023

durran May 31, 2023

durran May 22, 2023

durran May 22, 2023

durran May 25, 2023

nbbeeken May 26, 2023

durran May 30, 2023

durran May 25, 2023

durran May 25, 2023

addaleax May 26, 2023

durran May 30, 2023

addaleax May 26, 2023

durran May 26, 2023

nbbeeken May 26, 2023

nbbeeken May 26, 2023

nbbeeken May 26, 2023

durran May 31, 2023

		@@ -253,61 +251,3 @@ function deriveRegion(host: string) {

		return parts[1];
		}

	const ALLOWED_PROVIDER_NAMES = ['aws', 'azure'];
	const ALLOWED_PROVIDER_NAMES: AuthMechanismProperties['PROVIDER_NAME'][] = ['aws', 'azure'];

feat(NODE-4929): Add OIDC Azure workflow #3670

feat(NODE-4929): Add OIDC Azure workflow #3670

Conversation

durran commented May 12, 2023 • edited

Description

What is changing?

Is there new documentation needed for these changes?

What is the motivation for this change?

Double check the following

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durran commented May 12, 2023 •

edited