URL-encode client_id in Azure IMDS token request#2787
Open
ikow wants to merge 2 commits intomongodb:masterfrom
Open
URL-encode client_id in Azure IMDS token request#2787ikow wants to merge 2 commits intomongodb:masterfrom
ikow wants to merge 2 commits intomongodb:masterfrom
Conversation
The `_get_azure_response()` function constructs the Azure IMDS URL by interpolating `client_id` via f-string without URL encoding. While `resource` is already encoded (via `quote()` at the call site in `auth_oidc_shared.py`), `client_id` is not, creating an inconsistency. Apply `urllib.parse.quote()` to `client_id` before interpolation, consistent with the handling of `resource` and with the Node.js driver's use of `url.searchParams.append()` for the same parameter. Add a test to verify special characters in `client_id` are properly percent-encoded and cannot introduce additional query parameters.
|
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Apply
urllib.parse.quote()to theclient_idparameter in_get_azure_response()before interpolating it into the Azure IMDS URL, consistent with the existing treatment ofresource(which is already encoded viaquote()in_OIDCAzureCallback.__init__).Motivation
The
resourceparameter is URL-encoded at the call site (auth_oidc_shared.py:103), butclient_idis interpolated raw via f-string (_azure_helpers.py:32). This inconsistency means aclient_idcontaining URL-special characters (&,=,#, etc.) could unintentionally alter the query string structure.For comparison, the Node.js driver uses the safe
url.searchParams.append('client_id', username)API for the same operation (src/client-side-encryption/providers/azure.ts:127), which automatically handles encoding.Changes
pymongo/_azure_helpers.py: Applyquote()toclient_idbefore URL interpolationtest/test_azure_helpers.py: Addtest_client_id_is_url_encodedto verify special characters are properly percent-encodedTest
All 13 tests in
test/test_azure_helpers.pypass, including the new encoding test.