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.

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

DRIVERS-2416 OIDC: Automatic token acquisition for Azure Identity Provider #1421

Closed
wants to merge 5 commits into from

Conversation

blink1073
Copy link
Member

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 durran self-requested a review May 18, 2023 11:49
@blink1073 blink1073 marked this pull request as ready for review May 22, 2023 21:28
@blink1073 blink1073 requested review from a team as code owners May 22, 2023 21:28
@blink1073 blink1073 requested review from katcharov and JamesKovacs and removed request for a team May 22, 2023 21:28
obtain an access token, with the one exception of using the TOKEN_AUDIENCE as
the ``resource`` parameter. The azure credentials MUST be cached by
TOKEN_AUDIENCE, and expire within 5 minutes of the time given by the
``expires_in`` parameter of the IMDS response.
Copy link
Member

@durran durran May 25, 2023

Choose a reason for hiding this comment

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

I think we should call out that the token audience is in the format api://${TOKEN_AUDIENCE}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be more accurate to say that the audience itself is an encoded URI, most likely an Application ID URI of the form app://<app id>.

Drivers MUST be able to authenticate using the "azure" provider workflow, using
an Azure VM provisioned using the helper scripts in Drivers Evergreen Tools.
These tests will most likely need to be run in a separate test file from the
rest of the tests, to avoid needing to skip multiple tests.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's worth noting here that the URI for all tests is the same, and must be equal to:

mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:azure,TOKEN_AUDIENCE:api://${AZUREOIDC_CLIENTID}

Then each individual test can reuse the same URI. Thoughts? I did the same as Python and just set it as the MONGODB_URI and had all the clients use that.

@blink1073 blink1073 marked this pull request as draft June 21, 2023 14:51
- Create a client with a url of the form ``mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:azure,TOKEN_AUDIENCE:<foo>``.
- Assert that a ``find`` operation succeeds.
- Close the client.
- Assert that the Azure OIDC cache has one entry.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not combine the Main Cache Not Used and Azure Cache is Used tests? Clear both caches, perform the operation, and check that the main cache remains empty and the Azure one has a single entry.

Copy link
Contributor

@JamesKovacs JamesKovacs left a comment

Choose a reason for hiding this comment

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

LGTM

@blink1073
Copy link
Member Author

Closing since this will have to be re-worked to use the machine callback.

@blink1073 blink1073 closed this Dec 1, 2023
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.

None yet

3 participants