[Core] Fix MSAL to respect REQUESTS_CA_BUNDLE for proxy certificates#32208
[Core] Fix MSAL to respect REQUESTS_CA_BUNDLE for proxy certificates#32208ciaran-finnegan wants to merge 1 commit intoAzure:devfrom
Conversation
Problem: - When using 'az login' behind a proxy with self-signed certificates, MSAL authentication fails with SSL certificate verification errors - While other Azure CLI operations properly respect the REQUESTS_CA_BUNDLE environment variable, MSAL library doesn't use the custom CA bundle - This causes authentication to fail even when users correctly configure their proxy certificates Solution: - Added get_msal_http_client() helper function in _debug.py that creates a configured requests.Session - Updated Identity._msal_app_kwargs to include http_client parameter with proper certificate configuration - Modified ManagedIdentityCredential to use the configured HTTP client instead of creating its own - The fix ensures all MSAL-based authentication respects certificate settings consistently Testing: - Added unit test test_get_msal_http_client_respects_ca_bundle() to verify proper configuration - All existing tests pass without regression - Code style and linting checks pass This change allows users working behind corporate proxies to successfully authenticate with Azure CLI by properly configuring their custom certificate bundles via the REQUESTS_CA_BUNDLE environment variable, matching the behavior of other Azure SDK operations.
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
Thank you for your contribution @ciaran-finnegan! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes MSAL authentication to respect the REQUESTS_CA_BUNDLE environment variable for proxy certificates, ensuring consistent certificate handling across all Azure CLI operations.
- Adds
get_msal_http_client()helper function to create properly configured HTTP clients for MSAL - Updates MSAL authentication components to use the new HTTP client configuration
- Ensures MSAL respects custom CA bundles and certificate verification settings
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/azure-cli-core/azure/cli/core/_debug.py | Added get_msal_http_client() function to create configured requests.Session for MSAL |
| src/azure-cli-core/azure/cli/core/auth/identity.py | Updated _msal_app_kwargs to include configured HTTP client |
| src/azure-cli-core/azure/cli/core/auth/msal_credentials.py | Modified ManagedIdentityCredential to use configured HTTP client |
| src/azure-cli-core/azure/cli/core/tests/test_connection_verify.py | Added comprehensive test for the new HTTP client configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if should_disable_connection_verify(): | ||
| logger.warning("Connection verification disabled by environment variable %s", | ||
| DISABLE_VERIFY_VARIABLE_NAME) | ||
| os.environ[ADAL_PYTHON_SSL_NO_VERIFY] = '1' | ||
| session.verify = False |
There was a problem hiding this comment.
Setting os.environ[ADAL_PYTHON_SSL_NO_VERIFY] = '1' inside this function modifies global state as a side effect. This should be handled separately or documented clearly, as it affects the entire process and may have unintended consequences for other parts of the application.
| if original_ca_bundle: | ||
| os.environ[_debug.REQUESTS_CA_BUNDLE] = original_ca_bundle | ||
| elif _debug.REQUESTS_CA_BUNDLE in os.environ: | ||
| del os.environ[_debug.REQUESTS_CA_BUNDLE] |
There was a problem hiding this comment.
The test cleanup logic has a potential issue: if original_ca_bundle is None but the environment variable existed originally with an empty string value, it won't be restored correctly. Consider using a sentinel value or checking original_ca_bundle is not None instead of truthy evaluation.
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
|
@microsoft-github-policy-service agree I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer. |
| logger.warning("Connection verification disabled by environment variable %s", | ||
| DISABLE_VERIFY_VARIABLE_NAME) | ||
| os.environ[ADAL_PYTHON_SSL_NO_VERIFY] = '1' | ||
| session.verify = False |
There was a problem hiding this comment.
It is by design that AZURE_CLI_DISABLE_CONNECTION_VERIFICATION is not honored when calling MSAL during authentication, as disabling SSL verification puts Azure CLI at risk. Setting the root CA with REQUESTS_CA_BUNDLE is the only supported approach. (See #32207)
|
We haven't observed that MSAL doesn't respect |
Problem:
Solution:
Testing:
This change allows users working behind corporate proxies to successfully authenticate with Azure CLI by properly configuring their custom certificate bundles via the REQUESTS_CA_BUNDLE environment variable, matching the behavior of other Azure SDK operations.
Related command
Description
Testing Guide
History Notes
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.