feat(NODE-5191): OIDC Auth Updates #3637

durran · 2023-04-13T12:11:46Z

Description

Updates OIDC implementation to the latest changes in mongodb/specifications#1381

What is changing?

Adds ALLOWED_HOSTS validation for connections using callback workflows.
OIDCMechanismServerStep1 becomes IdPServerInfo, removing endpoint fields.
OIDCRequestTokenResult becomes IdPServerResponse.
OIDCCallbackContext created to pass to the request and refresh callbacks.
Request and refresh functions now only take 2 arguments.
CallbackWorkflow is refactored to split behaviour around server commands and callbacks.
CallbackLockCache implements the locking of callbacks and function hashing.
TokenResultCache is simplified and is passed function hashes.
Updates to implement all new prose tests.
Added unit tests where applicable.

Is there new documentation needed for these changes?

None, this is still considered public preview, not stable.

What is the motivation for this change?

NODE-5191/DRIVERS-2415

mongodb/specifications#1381

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

src/cmap/auth/mongodb_oidc.ts

src/utils.ts

src/connection_string.ts

src/utils.ts

durran · 2023-04-27T11:32:55Z

src/cmap/auth/mongodb_oidc/callback_workflow.ts

@@ -1,16 +1,31 @@
 import { Binary, BSON, type Document } from 'bson';


Note to reviewers. This file is probably better off reviewing as a whole instead of looking at the diff. The entire class is pretty much refactored to separate the request/refresh function logic when dealing with the token cache and the actual saslStart/saslContinue commands sent to the server. It made more sense to separate them then to have them intertwined - I think the result is easier to understand.

src/utils.ts

src/cmap/auth/mongodb_oidc.ts

src/cmap/auth/mongodb_oidc/callback_workflow.ts

addaleax

Locking looks good! 🚀

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

durran · 2023-04-28T17:19:42Z

src/cmap/auth/mongodb_oidc.ts

@@ -73,19 +81,10 @@ export class MongoDBOIDC extends AuthProvider {
   * Authenticate using OIDC
   */
  override async auth(authContext: AuthContext): Promise<void> {
-    const { connection, credentials, response, reauthenticating } = authContext;
-
-    if (response?.speculativeAuthenticate) {


Speculative auth is implemented in the the workflows.

durran · 2023-04-28T17:21:49Z

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

+/**
+ * A cache of request and refresh callbacks per server/user.
+ */
+export class CallbackLockCache {


This cache is now responsible for the hashing of the request and refresh functions, which is also then used by the TokenEntryCache. It also is responsible for wrapping the callbacks in a lock and storing them by the same cache key as the token results in the other cache. Main reasoning for not storing these in the other cache and creating a new one is that the callback functions have no expiration.

durran · 2023-04-28T17:23:08Z

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

+/**
+ * Ensure the callback is only executed one at a time.
+ */
+function withLock(callback: OIDCRequestFunction | OIDCRefreshFunction) {


This ensures a request or refresh function cannot be entered simultaneously by different code paths, by chaining on to the end of each previous promise. Inspired by those clever dev tools folks: https://github.com/mongodb-js/oidc-plugin/blob/ca6bb4fdddabfd651ea2d544b2f9bdcf2095fe71/src/util.ts#L62-L80

durran · 2023-04-28T17:23:37Z

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

+/**
+ * Get the hash string for the request and refresh functions.
+ */
+function hashFunctions(requestFn: OIDCRequestFunction, refreshFn?: OIDCRefreshFunction): string {


This is the same as before, just moved to this cache impl from the other.

durran · 2023-04-28T17:27:11Z

src/cmap/auth/mongodb_oidc/aws_service_workflow.ts

@@ -3,6 +3,9 @@ import { readFile } from 'fs/promises';
 import { MongoAWSError } from '../../../error';
 import { ServiceWorkflow } from './service_workflow';

+/** Error for when the token is missing in the environment. */
+const TOKEN_MISSING_ERROR = 'AWS_WEB_IDENTITY_TOKEN_FILE must be set in the environment.';


This and other string constants just a simple refactoring of any non-interpolated string being a constant.

src/cmap/auth/mongodb_oidc/aws_service_workflow.ts

src/cmap/auth/mongodb_oidc/callback_lock_cache.ts

test/unit/cmap/auth/mongodb_oidc/callback_lock_cache.test.ts

test/manual/mongodb_oidc.prose.test.ts

nbbeeken

just username and sleep outstanding comments

durran · 2023-05-03T20:26:50Z

Patch on latest commit without all the other clutter: https://spruce.mongodb.com/version/6452c353a4cf47cc42d9b7f5/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

addaleax reviewed Apr 13, 2023

View reviewed changes

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

src/utils.ts Outdated Show resolved Hide resolved

durran force-pushed the NODE-5191 branch 2 times, most recently from 1c249b3 to fbc8550 Compare April 14, 2023 12:48

addaleax reviewed Apr 17, 2023

View reviewed changes

src/connection_string.ts Outdated Show resolved Hide resolved

src/utils.ts Outdated Show resolved Hide resolved

src/utils.ts Outdated Show resolved Hide resolved

durran force-pushed the NODE-5191 branch 4 times, most recently from 3635712 to 4be3e9d Compare April 21, 2023 18:42

durran force-pushed the NODE-5191 branch from b501c90 to 0b7be80 Compare April 27, 2023 11:29

durran commented Apr 27, 2023

View reviewed changes

addaleax reviewed Apr 27, 2023

View reviewed changes

src/utils.ts Outdated Show resolved Hide resolved

addaleax reviewed Apr 27, 2023

View reviewed changes

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

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

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

durran force-pushed the NODE-5191 branch from 0bdfafc to 7924792 Compare April 27, 2023 20:58

addaleax reviewed Apr 28, 2023

View reviewed changes

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

durran marked this pull request as ready for review April 28, 2023 17:15

durran requested a review from addaleax April 28, 2023 17:15

addaleax reviewed Apr 28, 2023

View reviewed changes

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

durran commented Apr 28, 2023

View reviewed changes

nbbeeken self-assigned this May 2, 2023

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

nbbeeken requested changes May 2, 2023

View reviewed changes

durran added 5 commits May 3, 2023 13:41

feat(NODE-5191): update oidc objects

51532f2

feat(NODE-5191): add allowed hosts option

08823d1

test(NODE-5191): add prose spec template

a943d7b

test: adding first prose test

3383e87

test: prose tests up to aws

3608ea0

durran added 19 commits May 3, 2023 13:41

test: create new client

780e4df

chore: remove console logs

2e6d7a3

test: fix unit tests

e0899f6

fix: ts import version

87345ec

test: add case for unix sockets

f193239

feat: prevent simultaneous callback execution

ce5fcab

test: add lock prose test

8bb8ae1

chore: debug

dda9dcb

chore: lock again

98b7225

test: use lock cache

b4f2008

test: clear callback cache

1abd3b0

chore: debug

76897a4

refactor: move function hashing

8e5c113

test: throw if entered with lock

9335966

fix:lint

f28c29d

fix: change cache keys

68c52ef

fix: timers on node 14

fd5a712

fix: settimeout issues

76aa52b

fix: suggestions

5fecafc

durran force-pushed the NODE-5191 branch from d47a39e to 5fecafc Compare May 3, 2023 13:03

durran requested a review from nbbeeken May 3, 2023 13:19

nbbeeken requested changes May 3, 2023

View reviewed changes

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 3, 2023

durran added 2 commits May 3, 2023 22:16

fix: credentials defaulting

b947b11

fix: comment addressing

bc167a2

nbbeeken approved these changes May 3, 2023

View reviewed changes

nbbeeken merged commit c52a4ed into main May 4, 2023
18 of 21 checks passed

nbbeeken deleted the NODE-5191 branch May 4, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(NODE-5191): OIDC Auth Updates #3637

feat(NODE-5191): OIDC Auth Updates #3637

durran commented Apr 13, 2023 •

edited

durran Apr 27, 2023 •

edited

addaleax left a comment

durran Apr 28, 2023

durran Apr 28, 2023

durran Apr 28, 2023 •

edited

durran Apr 28, 2023

durran Apr 28, 2023

nbbeeken left a comment

durran commented May 3, 2023

		@@ -1,16 +1,31 @@
		import { Binary, BSON, type Document } from 'bson';

feat(NODE-5191): OIDC Auth Updates #3637

feat(NODE-5191): OIDC Auth Updates #3637

Conversation

durran commented Apr 13, 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

durran Apr 27, 2023 • edited

Choose a reason for hiding this comment

addaleax left a comment

Choose a reason for hiding this comment

durran Apr 28, 2023

Choose a reason for hiding this comment

durran Apr 28, 2023

Choose a reason for hiding this comment

durran Apr 28, 2023 • edited

Choose a reason for hiding this comment

durran Apr 28, 2023

Choose a reason for hiding this comment

durran Apr 28, 2023

Choose a reason for hiding this comment

nbbeeken left a comment

Choose a reason for hiding this comment

durran commented May 3, 2023

durran commented Apr 13, 2023 •

edited

durran Apr 27, 2023 •

edited

durran Apr 28, 2023 •

edited