MSAL credentials send requests through Requests

[chlowell]

There are code paths through MSAL which wrapping credentials (e.g. InteractiveBrowserCredential) don't intercept. For example, MSAL applications create a new HTTP client (via a private method) every time they redeem a refresh token rather than reuse the clients they create at construction (and which we replace with an adapter).

Some MSAL application code paths also directly call into requests. For example, a public application's get_accounts method may send instance discovery requests.

[chlowell]

MSAL Python 1.3.0 adds an http_client parameter to ClientApplication. This should enable channeling all MSAL's requests into our pipeline.

[jiasli]

I think the current patching logic is not enough:

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_internal/msal_transport_adapter.py

Line 69 in b299320

self._patch = mock.patch("msal.authority.requests", self)

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py

Line 136 in b15cede

app.client.session = self._adapter

MSAL still uses requests for this line:

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py

Line 274 in b15cede

result = app.acquire_token_silent_with_error(list(scopes), account=account, **kwargs)

When creating a credential with connection_verify=False (#11944), the callback shows that requests is still directly used by MSAL (without RequestsTransport in between), thus failing to apply connection_verify:

azure-sdk-for-python/sdk/core/azure-core/azure/core/pipeline/transport/_requests_basic.py

Line 166 in 0e82fae

class RequestsTransport(HttpTransport):

Traceback (most recent call last):
  File "d:\cli\azure-sdk-for-python\sdk\identity\azure-identity\azure\identity\_internal\exception_wrapper.py", line 18, in wrapper
    return fn(*args, **kwargs)
  File "d:\cli\azure-sdk-for-python\sdk\identity\azure-identity\azure\identity\_internal\msal_credentials.py", line 274, in _acquire_token_silent
    result = app.acquire_token_silent_with_error(list(scopes), account=account, **kwargs)
  File "D:\cli\microsoft-authentication-library-for-python\msal\application.py", line 546, in acquire_token_silent_with_error
    result = self._acquire_token_silent_from_cache_and_possibly_refresh_it(
  File "D:\cli\microsoft-authentication-library-for-python\msal\application.py", line 608, in _acquire_token_silent_from_cache_and_possibly_refresh_it
    return self._acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family(
  File "D:\cli\microsoft-authentication-library-for-python\msal\application.py", line 639, in _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family
    last_resp = at = self._acquire_token_silent_by_finding_specific_refresh_token(
  File "D:\cli\microsoft-authentication-library-for-python\msal\application.py", line 670, in _acquire_token_silent_by_finding_specific_refresh_token
    response = client.obtain_token_by_refresh_token(
  File "D:\cli\microsoft-authentication-library-for-python\msal\oauth2cli\oauth2.py", line 521, in obtain_token_by_refresh_token
    resp = super(Client, self).obtain_token_by_refresh_token(
  File "D:\cli\microsoft-authentication-library-for-python\msal\oauth2cli\oauth2.py", line 242, in obtain_token_by_refresh_token
    return self._obtain_token("refresh_token", data=data, **kwargs)
  File "D:\cli\microsoft-authentication-library-for-python\msal\oauth2cli\oidc.py", line 83, in _obtain_token
    ret = super(Client, self)._obtain_token(grant_type, *args, **kwargs)
  File "D:\cli\microsoft-authentication-library-for-python\msal\oauth2cli\oauth2.py", line 462, in _obtain_token
    resp = super(Client, self)._obtain_token(
  File "D:\cli\microsoft-authentication-library-for-python\msal\oauth2cli\oauth2.py", line 214, in _obtain_token
    resp = (post or self._http_client.post)(
  File "D:\cli\env38\lib\site-packages\requests\sessions.py", line 581, in post
    return self.request('POST', url, data=data, json=json, **kwargs)
  File "D:\cli\env38\lib\site-packages\requests\sessions.py", line 533, in request
    resp = self.send(prep, **send_kwargs)
  File "D:\cli\env38\lib\site-packages\requests\sessions.py", line 646, in send
    r = adapter.send(request, **kwargs)
  File "D:\cli\env38\lib\site-packages\requests\adapters.py", line 514, in send
    raise SSLError(e, request=request)
requests.exceptions.SSLError: HTTPSConnectionPool(host='login.microsoftonline.com', port=443): Max retries exceeded with url: /organizations/oauth2/v2.0/token (Caused by SSLError(SSLError("bad handshake: Error([('SSL routines', 'tls_process_server_certificate', 'certificate verify failed')])")))

[jiasli]

This is because the client created at msal.application.ClientApplication.__init__

        self.client = self._build_client(client_credential, self.authority)

is not reused at msal.application.ClientApplication._acquire_token_silent_by_finding_specific_refresh_token

        client = self._build_client(self.client_credential, authority)

So the monkey patch doesn't work for this local client:

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py

Line 136 in b15cede

app.client.session = self._adapter

Possible solutions:

1. Use http_client when creating ClientApplication, since when creating Client, self._build_client uses self.http_client:

           return Client(
               ...
               http_client=self.http_client,

   @chlowell
2. MSAL can reuse self.client instead of creating a new one. @rayluo

[rayluo]

I think the current patching logic is not enough:

...

Possible solutions:

1. Use http_client when creating ClientApplication, since when creating Client, self._build_client uses self.http_client:

           return Client(
               ...
               http_client=self.http_client,

   @chlowell

2. MSAL can reuse self.client instead of creating a new one. @rayluo

Unfortunately MSAL has some weird internal logic that we would need to create another client on-the-fly. But it should not be an issue, because that new client would automatically reuse the same self.http_client under the hood.

Besides, @chlowell is already in the process of unpatching msal (#11892), so this will be fixed there.