DRIVERS-2672 Add OIDC machine workflow spec. #1471

matthewdale · 2023-10-22T04:08:10Z

DRIVERS-2672

Please complete the following before merging:

Update changelog.
Make sure there are generated JSON files from the YAML test files.
Test changes in at least one language driver.
Test these changes against all server versions and topologies (including standalone, replica set, sharded clusters, and serverless).

durran

Initial comments.

source/auth/auth.rst

source/auth/tests/legacy/connection-string.yml

source/auth/tests/mongodb-oidc.rst

source/auth/auth.rst

source/auth/tests/mongodb-oidc.rst

source/auth/auth.rst

source/auth/tests/mongodb-oidc.rst

source/auth/tests/legacy/connection-string.yml

source/auth/auth.rst

source/auth/tests/mongodb-oidc.rst

alcaeus

Unified test format changes look good. Can you please add tests for authMechanism similar to the invalid/runOnRequirement-auth-type test?

source/auth/auth.rst

matthewdale · 2024-01-22T19:22:41Z

@alcaeus Added an invalid test for the type of authMechanism.

alcaeus

Unified test format changes LGTM.

Co-authored-by: Jeff Yemin <jeff.yemin@mongodb.com>

…ional PR feedback.

sanych-sun · 2024-01-25T01:19:39Z

source/auth/tests/legacy/connection-string.yml

-    (MONGODB-OIDC)
-  uri: mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:unexisted
+- description: should throw an exception custom callback is chosen but no callback is provided (MONGODB-OIDC)
+  uri: mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:custom


I suppose "custom" is not allowed value for PROVIDER_NAME. As far as I remember there is only "aws" allowed so far.

You're right, that test is vestigial from an earlier version of the spec. I've removed this test case because it's no longer relevant.

sanych-sun · 2024-01-25T01:20:47Z

source/auth/tests/legacy/connection-string.yml

-  uri: mongodb://localhost/?authMechanism=MONGODB-OIDC
+      PROVIDER_NAME: aws
+- description: should ignore username and password if specified for aws provider (MONGODB-OIDC)
+  uri: mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:aws


PROVIDER_NAME and callbacks are mutual exclusive. Callback parameter probably has to be removed from this test.

You're correct. The callback legacy test case parameter was never documented and is not supported by most drivers, so I intended to remove it from all test cases here, but accidentally left that one in. I've updated that test case to assert that providing a password causes a validation error, which matches the spec.

jyemin

One suggestion for a wording change that I leave to your discretion.

LGTM!

source/auth/auth.rst

…a fields in prose tests.

blink1073

LGTM!

Outdated

JamesKovacs

LGTM. Check to see if the noted prose test is truly a duplicate of the unified test or not.

JamesKovacs · 2024-01-31T19:00:51Z

source/auth/tests/mongodb-oidc.rst

+
+- Create a ``MongoClient`` configured with an OIDC callback and auth mechanism
+  property ``PROVIDER_NAME:aws``.
+- Assert it returns a client configuration error.


This prose test is already covered by the unified connection string tests above. Isn't this an unnecessary duplicate?

The updated Auth connection string tests don't support passing an OIDC callback, so I believe this prose test covers a case that the Auth connection string tests can't. The previous Auth connection string OIDC tests did include a "callback" param, but it was not documented in the test format README, was only implemented in 1 or 2 drivers, and was confusing with the updated OIDC callback API, so I decided to replace it with a prose test instead.

durran self-requested a review October 30, 2023 14:02

durran requested changes Oct 31, 2023

View reviewed changes

blink1073 mentioned this pull request Nov 1, 2023

DRIVERS-2616 OIDC-SASL Follow-Up #1460

Closed

4 tasks

addaleax reviewed Nov 3, 2023

View reviewed changes

jyemin reviewed Nov 3, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

blink1073 reviewed Nov 6, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

katcharov previously requested changes Nov 6, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

addaleax reviewed Nov 7, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

durran mentioned this pull request Nov 7, 2023

feat(NODE-5464): OIDC machine and callback workflow mongodb/node-mongodb-native#3912

Merged

5 tasks

durran requested changes Nov 8, 2023

View reviewed changes

source/auth/tests/mongodb-oidc.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

durran requested changes Nov 9, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

matthewdale force-pushed the drivers2672-oidc-machine branch from f805caf to 246d124 Compare November 9, 2023 22:08

matthewdale mentioned this pull request Nov 9, 2023

DRIVERS-2672 Update unified test format to support OIDC tests. #1468

Closed

4 tasks

katcharov reviewed Nov 9, 2023

View reviewed changes

source/auth/tests/mongodb-oidc.rst Show resolved Hide resolved

katcharov reviewed Nov 9, 2023

View reviewed changes

source/auth/tests/mongodb-oidc.rst Outdated Show resolved Hide resolved

katcharov mentioned this pull request Nov 15, 2023

Remove non-machine workflow mongodb/mongo-java-driver#1259

Merged

blink1073 mentioned this pull request Nov 16, 2023

DRIVERS-2616 OIDC-SASL Follow-Up #1422

Closed

4 tasks

matthewdale marked this pull request as ready for review December 7, 2023 11:05

matthewdale requested review from a team as code owners December 7, 2023 11:05

matthewdale requested review from alcaeus and JamesKovacs and removed request for a team December 7, 2023 11:05

durran reviewed Dec 7, 2023

View reviewed changes

source/auth/tests/legacy/connection-string.yml Outdated Show resolved Hide resolved

blink1073 reviewed Dec 8, 2023

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/tests/mongodb-oidc.rst Outdated Show resolved Hide resolved

source/auth/tests/mongodb-oidc.rst Outdated Show resolved Hide resolved

blink1073 added 3 commits January 3, 2024 21:57

DRIVERS-2616 OIDC-SASL Follow-Up

fbc3d72

finish refactor

f01c678

syntax

62130cf

matthewdale added 2 commits January 18, 2024 09:10

Merge branch 'master' into drivers2672-oidc-machine

9ef2650

Fix master merge.

bb09469

alcaeus reviewed Jan 19, 2024

View reviewed changes

Complete OIDC prose tests. PR feedback.

f2135be

jyemin reviewed Jan 20, 2024

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

durran approved these changes Jan 22, 2024

View reviewed changes

Add type test for authMechanism runOnRequirement.

9b6bd5f

matthewdale requested review from alcaeus and katcharov January 22, 2024 19:22

alcaeus approved these changes Jan 23, 2024

View reviewed changes

matthewdale and others added 2 commits January 23, 2024 16:53

Update source/auth/auth.rst

11357db

Co-authored-by: Jeff Yemin <jeff.yemin@mongodb.com>

Allow drivers to use either a unified or separate callback API. Addit…

357cad9

…ional PR feedback.

sanych-sun reviewed Jan 25, 2024

View reviewed changes

Update callback naming. Fix auth connection string tests.

09554a6

matthewdale requested review from jyemin and sanych-sun January 25, 2024 07:56

jyemin approved these changes Jan 25, 2024

View reviewed changes

source/auth/auth.rst Outdated Show resolved Hide resolved

source/auth/auth.rst Outdated Show resolved Hide resolved

matthewdale added 7 commits January 26, 2024 00:51

Simplify speculative auth. Clarify cache behaviors. PR feedback.

37102c5

Fix prose and spec tests. Add to prose test local testing guide.

03a0f8b

Clarify config of retryReads in prose tests. Remove checking for extr…

95395e2

…a fields in prose tests.

Bump unified test format to 1.19

f41bb37

Merge branch 'master' into drivers2672-oidc-machine

6b07e16

Correct Unified Test Format spec current schema version.

353fa83

Include all changes from unified test spec 1.18 in 1.19.

4203a6d

blink1073 approved these changes Jan 31, 2024

View reviewed changes

JamesKovacs approved these changes Jan 31, 2024

View reviewed changes

matthewdale merged commit a288630 into mongodb:master Jan 31, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRIVERS-2672 Add OIDC machine workflow spec. #1471

DRIVERS-2672 Add OIDC machine workflow spec. #1471

matthewdale commented Oct 22, 2023 •

edited

Loading

durran left a comment

alcaeus left a comment

matthewdale commented Jan 22, 2024

alcaeus left a comment

sanych-sun Jan 25, 2024

matthewdale Jan 25, 2024

sanych-sun Jan 25, 2024

matthewdale Jan 25, 2024

jyemin left a comment

blink1073 left a comment

JamesKovacs left a comment

JamesKovacs Jan 31, 2024

matthewdale Jan 31, 2024

DRIVERS-2672 Add OIDC machine workflow spec. #1471

DRIVERS-2672 Add OIDC machine workflow spec. #1471

Conversation

matthewdale commented Oct 22, 2023 • edited Loading

durran left a comment

Choose a reason for hiding this comment

alcaeus left a comment

Choose a reason for hiding this comment

matthewdale commented Jan 22, 2024

alcaeus left a comment

Choose a reason for hiding this comment

sanych-sun Jan 25, 2024

Choose a reason for hiding this comment

matthewdale Jan 25, 2024

Choose a reason for hiding this comment

sanych-sun Jan 25, 2024

Choose a reason for hiding this comment

matthewdale Jan 25, 2024

Choose a reason for hiding this comment

jyemin left a comment

Choose a reason for hiding this comment

blink1073 left a comment

Choose a reason for hiding this comment

JamesKovacs left a comment

Choose a reason for hiding this comment

JamesKovacs Jan 31, 2024

Choose a reason for hiding this comment

matthewdale Jan 31, 2024

Choose a reason for hiding this comment

matthewdale commented Oct 22, 2023 •

edited

Loading