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-5035): enable OIDC authentication #3577

Merged
merged 57 commits into from Mar 3, 2023
Merged

feat(NODE-5035): enable OIDC authentication #3577

merged 57 commits into from Mar 3, 2023

Conversation

durran
Copy link
Member

@durran durran commented Feb 20, 2023 •
edited

Description

Implements OIDC auth.

Spec PR: mongodb/specifications#1365

Latest tools fix: mongodb-labs/drivers-evergreen-tools#272

What is changing?

  • Adds a new MongoDBOIDC auth provider.
  • Introduces the concept of workflows for additional cloud providers in the future.
  • Adds the spec prose tests for OIDC except reauthentication. (NODE-5036)
Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-5035/DRIVERS-2415

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 changed the title test(NODE-5035): enable OIDC authentication feat(NODE-5035): enable OIDC authentication Feb 20, 2023
@durran durran force-pushed the NODE-5035 branch 2 times, most recently from c161b8f to 1043567 Compare February 20, 2023 22:05
addaleax
addaleax reviewed Feb 20, 2023
View reviewed changes
src/cmap/auth/mongodb_oidc.ts Outdated Show resolved Hide resolved
src/cmap/auth/mongo_credentials.ts Show resolved Hide resolved
@durran durran added the wip label Feb 20, 2023
@durran durran force-pushed the NODE-5035 branch 3 times, most recently from 93376a0 to 464a587 Compare February 24, 2023 16:28
addaleax
addaleax reviewed Feb 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
@durran durran removed the wip label Feb 27, 2023
@durran durran marked this pull request as ready for review February 27, 2023 13:17
addaleax
addaleax reviewed Feb 27, 2023
View reviewed changes
Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Didn’t quite get around to the test/ changes right now, will try to do that later, but what’s in src/ looks good to me 👍

.evergreen/config.in.yml Outdated Show resolved Hide resolved
.evergreen/config.yml Outdated Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/callback_workflow.ts Outdated Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/aws_device_workflow.ts Outdated Show resolved Hide resolved
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Feb 27, 2023
W-A-James
W-A-James requested changes Feb 27, 2023
View reviewed changes
src/cmap/auth/mongodb_oidc/token_entry_cache.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.test.ts Outdated Show resolved Hide resolved
test/unit/cmap/auth/mongodb_oidc/aws_device_workflow.test.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.test.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.test.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.test.ts Outdated Show resolved Hide resolved
baileympearson
baileympearson requested changes Feb 27, 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.

I didn't look at tests here, but have some suggestions on the implementation. I like the workflow abstraction, and I love to see more usage of async-await in the driver. Nice work 🙂

src/cmap/auth/mongodb_oidc.ts 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
src/cmap/auth/mongodb_oidc/callback_workflow.ts Outdated Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/callback_workflow.ts Outdated Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/callback_workflow.ts Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/token_entry_cache.ts Outdated Show resolved Hide resolved
src/cmap/auth/mongodb_oidc/token_entry_cache.ts Outdated Show resolved Hide resolved
W-A-James
W-A-James reviewed Feb 28, 2023
View reviewed changes
src/cmap/auth/mongodb_oidc/callback_workflow.ts Outdated Show resolved Hide resolved
test/manual/mongodb_oidc.test.ts Outdated Show resolved Hide resolved
@W-A-James W-A-James self-requested a review February 28, 2023 15:16
W-A-James
W-A-James reviewed Feb 28, 2023
View reviewed changes
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Note: Serverless failures are caused by issue documented in https://jira.mongodb.org/browse/DRIVERS-2564

Filing flaky test ticket for time series unified spec tests

@W-A-James W-A-James added Primary Review In Review with primary reviewer, not yet ready for team's eyes Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Feb 28, 2023
durran and others added 26 commits March 3, 2023 15:29
@durran
refactor(NODE-5035): only implement getToken for devices
3125359
@durran
chore(NODE-5035): remove comment
9b28104
@durran
chore(NODE-5035): remove comment
cb1bd77
@durran
test(NODE-5035): use tools master again
2c87f3c
@durran
feat(NODE-5035): cleanup cache ono each request
88f9bf4
@durran
feat(NODE-5035): remove from cache when auth fails
025b546
@durran
chore(NODE-5035): add comment
1d700d6
@durran
fix(NODE-5035): change per suggestions
46bad06
@durran
fix(NODE-5035): change per suggestions
528c102
@durran
test(NODE-5035): fix unit tests
7c674d8
@durran
test(NODE-5035): fix callback tests
defde0e
@durran
fix(NODE-5035): no expiresInSeconds is immediate expiration
221c45d
@durran
chore(NODE-5035): update per comments
b2bd284
@durran
fix(NODE-5035): change default setting
5d3015f
@durran
test(NODE-5035): check for error type
28b52cd
@durran
fix: comment
427b499
@durran
chore: add suggestions
a21a271
@durran
chore: add suggestions
33253a3
@durran
chore: addressing comments
e76c781
@durran
chore: update per suggestions
5db996d
@durran
refactor(NODE-5035): device becomes service
36caf3f
@durran
feat(NODE-5035): add function identity
10bc3c3
@durran @dariakp
Update test/unit/cmap/auth/mongodb_oidc/token_entry_cache.test.ts
f38b215 
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
@durran
test: fix test
e9c43e7
@durran
fix: change service name to provider name
5028985
@durran
test: fix unit test
182ef41
@dariakp dariakp merged commit 35879f6 into main Mar 3, 2023
@dariakp dariakp deleted the NODE-5035 branch March 3, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax left review comments

@nbbeeken nbbeeken nbbeeken left review comments

@dariakp dariakp dariakp approved these changes

@baileympearson baileympearson baileympearson left review comments

@W-A-James W-A-James W-A-James left review comments

Assignees
No one assigned
Labels
Team Review Needs review from team
Projects
None yet
Milestone
No milestone
6 participants
@durran @addaleax @nbbeeken @baileympearson @W-A-James @dariakp