Skip to content

Implement OIDC SASL mechanism in sync #1107

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.

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

Merged
merged 19 commits into from
Jun 5, 2023

Conversation

katcharov
Copy link
Collaborator

@katcharov katcharov commented Apr 11, 2023

JAVA-4980

For design, see https://docs.google.com/document/d/1TJCIKdmQYjKjsPuh-M42BlcNjhqpgsayJ36cfAiV8y0/edit?pli=1#heading=h.lzf8zc90rp5u

In draft pending the following

  • resolution of TODOs, some of which are pending spec decisions; these are all related to tests
  • disabling of oidc prose tests

@stIncMale
Copy link
Member

A few risks associated with this work:

  1. Our "connection checkout timeout" (ConnectionPoolSettings.getMaxWaitTime) is documented as "The maximum time that a thread may wait for a connection to become available." However, it is not clear what is meant here by "available". Do we mean available for executing a user-requested operation, or do we mean "available" as in "there are no connections available for the pool to either provide or create"? Our implementation (and, highly likely, all other drivers) uses the second interpretation, i.e., the actual checkout duration is not limited by ConnectionPoolSettings.getMaxWaitTime, because opening a connection is not limited by it, and opening a connection includes handshaking, which in turn, includes authenticating. The latter may take up to about 5 minutes as a result of this work (this was in principle possible even before the work). If authentication takes significant time, this behavior has a chance to surprise our users.
  2. We have Authenticator.authenticateAsync for implementing asynchronous authentication for the reactive API. However, we use synchronous SalsClient underneath, which is blocking in case of the GSSAPIAuthenticator, and will be blocking (up to about 5 minutes) in the OIDC authenticator. The team has discussed this and decided that we will continue doing blocking auth-related actions when implementing the reactive driver API until someone complaints. If that happens, we may always convert a blocking synchronous API into an asynchronous one by executing the blocking auth-related methods in a single (per MongoClient) internal dedicated thread, as long as those methods don't need to be called concurrently with each other by a single MongoClient (the methods we identified don't need to be called concurrently).

@katcharov katcharov changed the title Implement OIDC SASL mechanism Implement OIDC SASL mechanism in sync May 19, 2023
@jyemin jyemin requested a review from stIncMale May 22, 2023 14:29
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The comments below are a result of partially reviewing the changes.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

Submitting more results of a partial review.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

More results of a partial review.

@katcharov katcharov force-pushed the JAVA-4757 branch 4 times, most recently from 035843c to c64bae2 Compare May 26, 2023 21:05
@katcharov katcharov marked this pull request as ready for review May 30, 2023 16:35
@katcharov katcharov changed the base branch from master to feature-oidc June 2, 2023 20:26
Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>
@katcharov katcharov merged commit b03bfbc into mongodb:feature-oidc Jun 5, 2023
@katcharov katcharov deleted the JAVA-4757 branch June 5, 2023 15:52
katcharov added a commit that referenced this pull request Nov 9, 2023
katcharov added a commit that referenced this pull request Mar 4, 2024
katcharov added a commit that referenced this pull request Apr 8, 2024
katcharov added a commit that referenced this pull request Apr 30, 2024
* Implement OIDC SASL mechanism in sync (#1107)

JAVA-4980

* Implement OIDC auth for async (#1131)

JAVA-4981

* Remove non-machine workflow (#1259)

JAVA-5077

* Add Human OIDC Workflow (#1316)

JAVA-5328

* OIDC Add remaining environments (azure, gcp), evergreen testing, API naming updates (#1371)

JAVA-5353
JAVA-5395
JAVA-4834
JAVA-4932

Co-authored-by: Valentin Kovalenko <valentin.kovalenko@mongodb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants