[Core] Enable token cache encryption on MacOS #20636
Conversation
| @@ -861,7 +861,7 @@ def _create_identity_instance(cli_ctx, *args, **kwargs): | |||
| from .auth.identity import Identity | |||
|
|
|||
| # Only enable encryption for Windows (for now). | |||
There was a problem hiding this comment.
This comment probably also needs to update to mention macOS now.
By the way, does this comment actually mean to say, "only allow fallback when running on Windows ..."?
There was a problem hiding this comment.
Oh. I forgot to update the comment!
| # msal-extensions automatically computes account_name from signal_location. | ||
| # https://github.com/AzureAD/microsoft-authentication-extensions-for-python/pull/103 | ||
| logger.debug("Initializing KeychainPersistence") | ||
| return KeychainPersistence(location, service_name=KEYCHAIN_SERVICE_NAME) |
There was a problem hiding this comment.
You probably already know that, this line won't work until AzureAD/microsoft-authentication-extensions-for-python#103 being released. So, please help review and approve my PR there (otherwise I will not give an approval to your PR here :-D)
There was a problem hiding this comment.
I have installed the dev code of msal-extensions and it works perfectly.
|
Enable token cache encryption on MacOS |
|
Given we are seeing a plethora of issues with Windows DPAPI encryption (#19506, https://github.com/Azure/azure-cli/issues?q=is%3Aissue+label%3A%22common+issue%22+winerror), it may not be a good idea to rush this PR. |
Different encryption mechanisms are used on different platforms. All those DPAPI issues will not be observed on macOS platform. On the other hand, we do not know whether there would be "plethora of issues" with macOS's Keychain. @bgavrilMS may comment based on the observation of keychain on .Net side. |
|
There is a plethora of issues on Mac as well. They mainly stem from how you package your app, in this case how you package Azure CLI. If you package it as Apple like it, with bundle id and via the app store, then you'll have access to the newest KeyChain APIs, which I think work rather well (this is what MSAL iOS uses on Mac, and I discussed it with them and it looks ok). If however you don't have a bundle id, then you can't use the newest KeyChain APIs and the older ones are not as reliable. Example of "old" API: |
I understand. I am only saying if encryption problems occur on MacOS, this will heavy the burden we currently have. According to our internal email discussion "Workaround for DPAPI/KeyChain Errors": Azure/azure-powershell#14478 is one example of KeyChain not working as expected on MacOS:
|
|
Hi @rayluo @jiasli I submitted a PR in MSAL EX to use the modern OS X Keyring API, take a look and let me know if we can help move this forward AzureAD/microsoft-authentication-extensions-for-python#119 |
Description
Utilize AzureAD/microsoft-authentication-extensions-for-python#103 to enable token cache encryption on MacOS.
We don't need to provide
account_nameby ourselves. Instead,msal-extensionsautomatically computesaccount_namefromsignal_location.See code comment for more information.
⚠ After this change, user will have to re-login so that credential is stored into Keychain.
References
Related email discussion: Naming for service_name and account_name for MacOS token encryption