[Identity] Update client_credential dict#46801
Draft
pvaneck wants to merge 1 commit intoAzure:mainfrom
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates azure-identity certificate-based credential handling to ensure the client_credential dictionary passed to MSAL uses a string (str) for private_key, aligning with MSAL’s documented expectations and addressing issue #36578.
Changes:
- Convert
client_credential["private_key"]from PEM bytes to a UTF-8 string before passing to MSAL. - Broaden
AadClientCertificateto acceptpem_bytes/passwordas eitherbytesorstr(encodingstrto UTF-8 internally). - Add/adjust tests to validate
private_keyis astrand update expectations forpassphrasewhen a string password is provided.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sdk/identity/azure-identity/azure/identity/_credentials/certificate.py | Decodes PEM bytes to str for private_key in the MSAL client credential dict; refactors password handling. |
| sdk/identity/azure-identity/azure/identity/_internal/aadclient_certificate.py | Expands accepted input types (bytes/str) for PEM and password and normalizes to bytes for cryptography. |
| sdk/identity/azure-identity/tests/test_obo.py | Updates assertions to expect string passphrase for string password input and validates private_key is a str. |
| sdk/identity/azure-identity/tests/test_certificate_credential.py | Adds a new test for get_client_credential output types (but currently contains a syntax-breaking indentation issue). |
Comments suppressed due to low confidence (1)
sdk/identity/azure-identity/azure/identity/_credentials/certificate.py:141
- The docstring for
get_client_credentialstill documentspasswordasbytes, but the function signature now acceptsstr | bytes. Please update the:paramentries to match the accepted types (and/or adjust the wording to clarify encoding behavior), so generated docs stay accurate.
"""Load a certificate from a filesystem path or bytes, return it as a dict suitable for msal.ClientApplication.
:param str certificate_path: Path to a PEM or PKCS12 certificate file.
:param bytes password: The certificate's password, if any.
:param bytes certificate_data: The PEM or PKCS12 certificate's bytes.
:param bool send_certificate_chain: Whether to send the certificate chain. Defaults to False.
Ensure that that private_key is passed as a string to MSAL, following their documented expectations. Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
3343423 to
e5a3c2a
Compare
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
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.
Ensure that private_key is passed as a string to MSAL, following their documented expectations.
Closes: #36578