AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
792 stars 195 forks source link

[Bug] Updates to msal/application.py may now be out-of-sync with msal_extensions #745

Open kcphila opened 4 days ago

kcphila commented 4 days ago

Describe the bug

I just upgraded the MSAL Python package and all of our silent authentication routines started breaking - these authenticate manually and cache the access token for future calls. Interactive authentication works and the token cache appears to save the the account and token correctly, but when the repeat call is made with silent it does not find an account and fails.

I have diagnosed the error and traced it back to this commit that changes self.token_cache.find to self.token_cache.search in several places.

It appears that upgraded to a MSAL version at or after this point requires an update of msal_extensions, upgrading from msal_extensions-1.0 .0 to msal_extensions-1.2.0 seems to work correctly, however, it was not obvious that this was the issue because both find and search are valid methods on the msal_extensions 1.0.0 token cache, but find finds the previously cached accounts and search does not.

msal_extensions understandably is not a requirement for the MSAL package, but I would recommend creating some type of link to ensure the versions are in sync when there are breaking changes (especially since the commit history suggests you maintain/contribute to both).

And if that's not doable, perhaps anyone else who runs into this and searches will find this bug report :)

To Reproduce Steps to reproduce the behavior:

  1. pip install msal_extensions==1.0.0
  2. pip install --upgrade msal
  3. Run a script or test that authenticates interactively and caches the result with an msal_extensions.PersistedTokenCache
  4. Run a script or test that authenticates silently and points to the same cache.

Expected behavior 4 should succeed

What you see instead

This fails because {msalapp}.get_accounts() returns nothing due to the search call.

The MSAL Python version you are using

1.31.0

Additional context

Steps above are fixed if (1) is changed to pip install msal_extensions>=1.2.0

rayluo commented 3 days ago

Hi @kcphila , we feel sorry that you run into this known issue.

It appears that upgraded to a MSAL version at or after this point requires an update of msal_extensions, upgrading from msal_extensions-1.0 .0 to msal_extensions-1.2.0 seems to work correctly, however, it was not obvious

You clearly already did your homework and found the solution, which is upgrading msal_extensions to 1.2.0+. Quoted from MSAL Extensions 1.2.0's release note: "Switch to MSAL 1.29+'s TokenCache.search()".

msal_extensions understandably is not a requirement for the MSAL package, but I would recommend creating some type of link to ensure the versions are in sync when there are breaking changes

Yes, that is indeed the tricky part. msal_extensions is not part of msal's dependency, but I suppose we can introduce a new optional dependency such that pip install msal[extensions] will install the proper version of Extensions. I discussed that with @jiasli before, but the decision at that time was to avoid a cyclic reference. But perhaps we shall revisit that decision, @jiasli ? I tried it in my local proof-of-concept and it would work.

kcphila commented 3 days ago

Thanks, @rayluo. I'll leave it to you all. ButI will way in with an opinion :) - the semantics of msal[extensions] does make a lot sense to me and would provide an easy way to keep the versions in sync in the requirements files of tools that use msal. I also build a lot of middle-level infrastructure for our data team, and so I think it would be more conceptually obvious for them than having msal and msal_extensions as listed separate.