Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(NODE-5036): reauthenticate OIDC and retry #3589

Merged
merged 17 commits into from
Mar 17, 2023
Merged

feat(NODE-5036): reauthenticate OIDC and retry #3589

merged 17 commits into from
Mar 17, 2023

Conversation

durran
Copy link
Member

@durran durran commented Mar 3, 2023 •
edited
Loading

Description

Enables reauthentication on the same connection with retry.

What is changing?

  • Adds the ability to retry a command on the same connection when getting a reauthentication error.
  • Adds reauthentication prose tests.
  • Adds reauthentication unified auth tests.
  • Marks OIDC as experimental
Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-5036

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 durran marked this pull request as ready for review March 3, 2023 20:21
@durran durran force-pushed the NODE-5036-rebased branch from dbdd899 to 3849e0e Compare March 6, 2023 15:03
@baileympearson baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Mar 9, 2023
baileympearson
baileympearson requested changes Mar 9, 2023
View reviewed changes
src/cmap/connection_pool.ts Outdated Show resolved Hide resolved
src/cmap/connection_pool.ts Outdated Show resolved Hide resolved
src/cmap/connection_pool.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/workflow.ts Show resolved Hide resolved
@durran durran requested a review from baileympearson March 16, 2023 16:30
@durran durran force-pushed the NODE-5036-rebased branch 2 times, most recently from f6d8d63 to 0e6e306 Compare March 16, 2023 17:31
baileympearson
baileympearson requested changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

just putting some review up (still need to work out CI).

src/cmap/connection_pool.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Show resolved Hide resolved
@durran durran force-pushed the NODE-5036-rebased branch from 0e6e306 to 36c5df0 Compare March 16, 2023 17:45
@durran
Copy link
Member Author

durran commented Mar 16, 2023

#3589 (comment)

NODE-5122

@durran durran force-pushed the NODE-5036-rebased branch from bca6508 to f319067 Compare March 17, 2023 13:40
@durran
Copy link
Member Author

durran commented Mar 17, 2023

https://spruce.mongodb.com/task/mongo_node_driver_next_rhel80_large_node_latest_test_auth_oidc_patch_764972247737ddc868a213619522dfd8abba4c27_6414640f562343536ab6c6bb_23_03_17_12_59_20/tests?execution=5&sortBy=STATUS&sortDir=ASC

@durran durran requested a review from baileympearson March 17, 2023 13:40
@nbbeeken nbbeeken assigned nbbeeken and unassigned baileympearson Mar 17, 2023
nbbeeken
nbbeeken requested changes Mar 17, 2023
View reviewed changes
src/cmap/auth/mongo_credentials.ts Outdated Show resolved Hide resolved
src/cmap/connection_pool.ts Outdated Show resolved Hide resolved
src/cmap/connection_pool.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@nbbeeken 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 Mar 17, 2023
@durran durran requested a review from nbbeeken March 17, 2023 17:07
@dariakp dariakp self-requested a review March 17, 2023 17:53
dariakp
dariakp requested changes Mar 17, 2023
View reviewed changes
src/cmap/auth/auth_provider.ts Show resolved Hide resolved
src/cmap/auth/auth_provider.ts Outdated Show resolved Hide resolved
src/cmap/connection_pool.ts Show resolved Hide resolved
test/manual/mongodb_oidc.prose.test.ts Outdated Show resolved Hide resolved
tsdoc.json Outdated Show resolved Hide resolved
@durran durran requested a review from dariakp March 17, 2023 18:45
@nbbeeken nbbeeken merged commit a41846d into main Mar 17, 2023
@nbbeeken nbbeeken deleted the NODE-5036-rebased branch March 17, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dariakp dariakp dariakp approved these changes

@nbbeeken nbbeeken nbbeeken approved these changes

@baileympearson baileympearson Awaiting requested review from baileympearson

Assignees

@nbbeeken nbbeeken

Labels
Team Review Needs review from team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@durran @nbbeeken @baileympearson @dariakp