-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Using msal token cache to fetch refresh token inside proxy command for az cli version >= 2.30 #4194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
398bd9f
c296d6c
8c84d56
aafc9ef
83582a2
b4a0319
0039e5f
96bd30f
29cbd57
96cb409
27679bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| import stat | ||
| import platform | ||
| from azure.core.exceptions import ClientAuthenticationError | ||
| from msal_extensions.token_cache import PersistedTokenCache | ||
| import yaml | ||
| import requests | ||
| import urllib.request | ||
|
|
@@ -25,6 +26,7 @@ | |
| from knack.prompting import prompt_y_n | ||
| from knack.prompting import NoTTYException | ||
| from azure.cli.core.commands.client_factory import get_subscription_id | ||
| from msal import PublicClientApplication, ConfidentialClientApplication | ||
| from azure.cli.core._profile import Profile | ||
| from azure.cli.core.util import sdk_no_wait | ||
| from azure.cli.core import telemetry | ||
|
|
@@ -40,9 +42,11 @@ | |
| from azext_connectedk8s._client_factory import get_graph_client_service_principals | ||
| import azext_connectedk8s._constants as consts | ||
| import azext_connectedk8s._utils as utils | ||
| from azext_connectedk8s._utils import az_cli | ||
| from glob import glob | ||
| from .vendored_sdks.models import ConnectedCluster, ConnectedClusterIdentity, ListClusterUserCredentialProperties | ||
| from threading import Timer, Thread | ||
| import msal_extensions | ||
| import sys | ||
| import hashlib | ||
| import re | ||
|
|
@@ -1635,12 +1639,16 @@ def client_side_proxy_wrapper(cmd, | |
| requestUri = f'{consts.CSP_Storage_Url}/{consts.RELEASE_DATE_WINDOWS}/arcProxy{operating_system}{consts.CLIENT_PROXY_VERSION}.exe' | ||
| older_version_string = f'.clientproxy\\arcProxy{operating_system}*.exe' | ||
| creds_string = r'.azure\accessTokens.json' | ||
| msal_token_cache_user = r'.azure\msal_token_cache.bin' | ||
| msal_token_cache_spn = r'.azure\service_principal_entries.bin' | ||
|
|
||
| elif(operating_system == 'Linux' or operating_system == 'Darwin'): | ||
| install_location_string = f'.clientproxy/arcProxy{operating_system}{consts.CLIENT_PROXY_VERSION}' | ||
| requestUri = f'{consts.CSP_Storage_Url}/{consts.RELEASE_DATE_LINUX}/arcProxy{operating_system}{consts.CLIENT_PROXY_VERSION}' | ||
| older_version_string = f'.clientproxy/arcProxy{operating_system}*' | ||
| creds_string = r'.azure/accessTokens.json' | ||
| msal_token_cache_user = r'.azure/msal_token_cache.bin' | ||
| msal_token_cache_spn = r'.azure/service_principal_entries.bin' | ||
|
|
||
| else: | ||
| telemetry.set_exception(exception='Unsupported OS', fault_type=consts.Unsupported_Fault_Type, | ||
|
|
@@ -1738,21 +1746,40 @@ def client_side_proxy_wrapper(cmd, | |
| raise FileOperationError("Failed to load credentials." + str(e)) | ||
|
|
||
| user_name = account['user']['name'] | ||
| use_msal_cache = utils.use_msal_cache() | ||
| if not use_msal_cache: | ||
| if user_type == 'user': | ||
| key = 'userId' | ||
| key2 = 'refreshToken' | ||
| else: | ||
| key = 'servicePrincipalId' | ||
| key2 = 'accessToken' | ||
|
|
||
| if user_type == 'user': | ||
| key = 'userId' | ||
| key2 = 'refreshToken' | ||
| else: | ||
| key = 'servicePrincipalId' | ||
| key2 = 'accessToken' | ||
|
|
||
| for i in range(len(creds_list)): | ||
| creds_obj = creds_list[i] | ||
|
|
||
| if key in creds_obj and creds_obj[key] == user_name: | ||
| creds = creds_obj[key2] | ||
| break | ||
| for i in range(len(creds_list)): | ||
| creds_obj = creds_list[i] | ||
|
|
||
| if key in creds_obj and creds_obj[key] == user_name: | ||
| creds = creds_obj[key2] | ||
| break | ||
| else: | ||
| if user_type == "user": | ||
| response_user_objectid = az_cli("ad signed-in-user show --query objectId -o tsv") | ||
| token_cache_location = os.path.expanduser(os.path.join('~', msal_token_cache_user)) | ||
| persistence = msal_extensions.FilePersistenceWithDataProtection(token_cache_location) | ||
| token_cache = msal_extensions.PersistedTokenCache(persistence) | ||
| token_cache._reload_if_necessary() | ||
| home_account_id = response_user_objectid + "." + tenantId | ||
| owned_by_home_account = {"home_account_id": home_account_id} | ||
| creds_info = token_cache.find(PersistedTokenCache.CredentialType.REFRESH_TOKEN, query=owned_by_home_account) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ray Luo (@rayluo), I think this is another instance where people are hacking MSAL cache to get the refresh token. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is really unfortunate. siriteja (@sirireddy12) , why is refresh token (RT) needed? Can the usage pattern be replaced by periodically requesting an access token, possibly via Azure CLI? (Jiashuo Li (@jiasli) , does FYI: Not only the token cache helpers are considered internal, MSAL would probably NOT save RT in token cache in near future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ray Luo (@rayluo) Refresh token is needed to retrieve PoP token. Does PoP implementation support available in MSAL python which az extension can utilize? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
MSAL Python does not currently support PoP. However, I got an impression that the PoP implementation might be similar to another feature named "Ssh Cert" which MSAL Python does support, and then there is also an az extension "ssh" already handles the key management and feeds signed content to MSAL. You may take a look and see whether that pattern could be repurposed for PoP token. (Oh, I did not notice your alias, krdhruva :-). The content above was the idea that I already mentioned in our email conversation.) |
||
| creds = creds_info[0]['secret'] | ||
| else: | ||
| token_cache_location = os.path.expanduser(os.path.join('~', msal_token_cache_spn)) | ||
| persistence = msal_extensions.FilePersistenceWithDataProtection(token_cache_location) | ||
| token_cache = msal_extensions.PersistedTokenCache(persistence) | ||
| token_cache._reload_if_necessary() | ||
| token_cache_string = token_cache.serialize() | ||
| cache_list = json.loads(token_cache_string) | ||
| creds = cache_list[0]['client_secret'] | ||
| if creds == '': | ||
| telemetry.set_exception(exception='Credentials of user not found.', fault_type=consts.Creds_NotFound_Fault_Type, | ||
| summary='Unable to find creds of user') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DON'T DO THIS. THIS IS NOT SUPPORTED. See Azure/azure-cli#19853 (comment)