From 34e27b79ad6b0ce021ee3fdc72625f34d74b0e07 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Fri, 8 Apr 2022 19:39:18 +0800 Subject: [PATCH 1/7] PoC --- .../cli/core/auth/adal_authentication.py | 19 ++++++++++++--- .../cli/core/auth/msal_authentication.py | 23 +++---------------- .../azure/cli/core/auth/util.py | 17 ++++++++++++++ 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py index 9cc060bbcf8..98bb7661ed8 100644 --- a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py @@ -7,6 +7,7 @@ from knack.log import get_logger from msrestazure.azure_active_directory import MSIAuthentication +from azure.cli.core.util import in_cloud_console from .util import _normalize_scopes, scopes_to_resource, AccessToken logger = get_logger(__name__) @@ -18,9 +19,21 @@ class MSIAuthenticationWrapper(MSIAuthentication): def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument logger.debug("MSIAuthenticationWrapper.get_token invoked by Track 2 SDK with scopes=%s", scopes) + # In Cloud Shell, we use + # - msrestazure to get access token + # - MSAL to get VM SSH certificate if 'data' in kwargs: - from azure.cli.core.azclierror import AuthenticationError - raise AuthenticationError("VM SSH currently doesn't support managed identity or Cloud Shell.") + if in_cloud_console(): + import msal + from .util import check_result, build_sdk_access_token + app = msal.PublicClientApplication(None) + result = app.acquire_token_silent(scopes, account=msal.application.CLOUD_SHELL_ACCOUNT, + data=kwargs['data']) + check_result(result) + return build_sdk_access_token(result) + else: + from azure.cli.core.azclierror import AuthenticationError + raise AuthenticationError("VM SSH currently doesn't support managed identity.") resource = scopes_to_resource(_normalize_scopes(scopes)) if resource: @@ -55,7 +68,7 @@ def set_token(self): import traceback from azure.cli.core.azclierror import AzureConnectionError, AzureResponseError try: - super(MSIAuthenticationWrapper, self).set_token() + super().set_token() except requests.exceptions.ConnectionError as err: logger.debug('throw requests.exceptions.ConnectionError when doing MSIAuthentication: \n%s', traceback.format_exc()) diff --git a/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py index fe13ee3e4d4..fe57d065200 100644 --- a/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py @@ -14,7 +14,7 @@ from knack.util import CLIError from msal import PublicClientApplication, ConfidentialClientApplication -from .util import check_result, AccessToken +from .util import check_result, build_sdk_access_token # OAuth 2.0 client credentials flow parameter # https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-client-creds-grant-flow @@ -87,7 +87,7 @@ def get_token(self, *scopes, **kwargs): # launch browser, but show the error message and `az login` command instead. else: raise - return _build_sdk_access_token(result) + return build_sdk_access_token(result) class ServicePrincipalCredential(ConfidentialClientApplication): @@ -130,21 +130,4 @@ def get_token(self, *scopes, **kwargs): if not result: result = self.acquire_token_for_client(scopes, **kwargs) check_result(result) - return _build_sdk_access_token(result) - - -def _build_sdk_access_token(token_entry): - import time - request_time = int(time.time()) - - # MSAL token entry sample: - # { - # 'access_token': 'eyJ0eXAiOiJKV...', - # 'token_type': 'Bearer', - # 'expires_in': 1618 - # } - - # Importing azure.core.credentials.AccessToken is expensive. - # This can slow down commands that doesn't need azure.core, like `az account get-access-token`. - # So We define our own AccessToken. - return AccessToken(token_entry["access_token"], request_time + token_entry["expires_in"]) + return build_sdk_access_token(result) diff --git a/src/azure-cli-core/azure/cli/core/auth/util.py b/src/azure-cli-core/azure/cli/core/auth/util.py index 0186cdc3ad2..45a49581bf0 100644 --- a/src/azure-cli-core/azure/cli/core/auth/util.py +++ b/src/azure-cli-core/azure/cli/core/auth/util.py @@ -139,6 +139,23 @@ def check_result(result, **kwargs): return None +def build_sdk_access_token(token_entry): + import time + request_time = int(time.time()) + + # MSAL token entry sample: + # { + # 'access_token': 'eyJ0eXAiOiJKV...', + # 'token_type': 'Bearer', + # 'expires_in': 1618 + # } + + # Importing azure.core.credentials.AccessToken is expensive. + # This can slow down commands that doesn't need azure.core, like `az account get-access-token`. + # So We define our own AccessToken. + return AccessToken(token_entry["access_token"], request_time + token_entry["expires_in"]) + + def decode_access_token(access_token): # Decode the access token. We can do the same with https://jwt.ms from msal.oauth2cli.oidc import decode_part From 07294da36a869c54a1b24a1c1c0b1e91824466cb Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 21 Apr 2022 00:00:43 +0000 Subject: [PATCH 2/7] Tested with MSAL Python wip branch on Cloud Shell --- .../azure/cli/core/auth/adal_authentication.py | 8 ++++++-- src/azure-cli-core/setup.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py index 98bb7661ed8..6a0c543f60b 100644 --- a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py @@ -26,8 +26,12 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument if in_cloud_console(): import msal from .util import check_result, build_sdk_access_token - app = msal.PublicClientApplication(None) - result = app.acquire_token_silent(scopes, account=msal.application.CLOUD_SHELL_ACCOUNT, + app = msal.PublicClientApplication( + "placeholder", # It needs a string placeholder + #token_cache=..., # TODO: This PoC does not currently maintain a token cache + ) + result = app.acquire_token_silent(list(scopes), + app.get_accounts(username=msal.CURRENT_USER)[0], data=kwargs['data']) check_result(result) return build_sdk_access_token(result) diff --git a/src/azure-cli-core/setup.py b/src/azure-cli-core/setup.py index 75f88539069..88013bc35be 100644 --- a/src/azure-cli-core/setup.py +++ b/src/azure-cli-core/setup.py @@ -52,7 +52,7 @@ 'jmespath', 'knack~=0.9.0', 'msal-extensions>=0.3.1,<0.4', - 'msal>=1.17.0,<2.0.0', + 'msal @ git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@cloudshell-imds', # TBD: 'msal>=1.18.0,<2.0.0', 'msrestazure~=0.6.4', 'packaging>=20.9,<22.0', 'paramiko>=2.0.8,<3.0.0', From 6810cdbbd999d3cc4f883b50187fce0d4a0182da Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 26 Apr 2022 16:21:32 -0700 Subject: [PATCH 3/7] Pulling dependency from its github feature branch --- scripts/release/debian/build.sh | 3 +++ src/azure-cli/requirements.py3.Linux.txt | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/release/debian/build.sh b/scripts/release/debian/build.sh index eb99705b3a6..8f3dce28cdd 100755 --- a/scripts/release/debian/build.sh +++ b/scripts/release/debian/build.sh @@ -23,6 +23,9 @@ apt-get update # uuid-dev is used to build _uuid module: https://github.com/python/cpython/pull/3796 apt-get install -y libssl-dev libffi-dev python3-dev debhelper zlib1g-dev uuid-dev apt-get install -y wget +# Git is not strictly necessary, but it would allow building an experimental package +# with dependency which is currently only available in its git repo feature branch. +apt-get install -y git # Download Python source code PYTHON_SRC_DIR=$(mktemp -d) diff --git a/src/azure-cli/requirements.py3.Linux.txt b/src/azure-cli/requirements.py3.Linux.txt index 646befac045..9d4bd4c5c83 100644 --- a/src/azure-cli/requirements.py3.Linux.txt +++ b/src/azure-cli/requirements.py3.Linux.txt @@ -112,7 +112,8 @@ jsondiff==1.3.0 knack==0.9.0 MarkupSafe==1.1.1 msal-extensions==0.3.1 -msal==1.17.0 +#msal==1.17.0 +git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@cloudshell-imds#egg=msal msrest==0.6.21 msrestazure==0.6.4 oauthlib==3.0.1 From 9bcef339fb5543014d7b5dc0d371510c79b2ed63 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 13 May 2022 03:31:43 +0000 Subject: [PATCH 4/7] Adopt latest MSAL Python changes --- .../azure/cli/core/auth/adal_authentication.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py index 6a0c543f60b..6f524d17ecf 100644 --- a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py @@ -27,12 +27,11 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument import msal from .util import check_result, build_sdk_access_token app = msal.PublicClientApplication( - "placeholder", # It needs a string placeholder - #token_cache=..., # TODO: This PoC does not currently maintain a token cache + "04b07795-8ddb-461a-bbee-02f9e1bf7b46", # Use a real client_id, so that cache would work + #token_cache=..., # TODO: This PoC does not currently maintain a token cache; + # Ideally we should reuse the real MSAL app object which has cache configured. ) - result = app.acquire_token_silent(list(scopes), - app.get_accounts(username=msal.CURRENT_USER)[0], - data=kwargs['data']) + result = app.acquire_token_interactive(list(scopes), prompt="none", data=kwargs["data"]) check_result(result) return build_sdk_access_token(result) else: From fdeb53f469746b8870079deea0ae063845ca01ea Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Wed, 18 May 2022 23:09:30 +0800 Subject: [PATCH 5/7] error handling --- .../cli/core/auth/adal_authentication.py | 19 +++++++------- .../azure/cli/core/auth/credential_adaptor.py | 2 +- .../cli/core/auth/msal_authentication.py | 2 +- .../azure/cli/core/auth/util.py | 26 +++++++++---------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py index 6f524d17ecf..f96dab82c2f 100644 --- a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py @@ -7,7 +7,6 @@ from knack.log import get_logger from msrestazure.azure_active_directory import MSIAuthentication -from azure.cli.core.util import in_cloud_console from .util import _normalize_scopes, scopes_to_resource, AccessToken logger = get_logger(__name__) @@ -17,27 +16,29 @@ class MSIAuthenticationWrapper(MSIAuthentication): # This method is exposed for Azure Core. Add *scopes, **kwargs to fit azure.core requirement # pylint: disable=line-too-long def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument - logger.debug("MSIAuthenticationWrapper.get_token invoked by Track 2 SDK with scopes=%s", scopes) + logger.debug("MSIAuthenticationWrapper.get_token: scopes=%r, kwargs=%r", scopes, kwargs) - # In Cloud Shell, we use - # - msrestazure to get access token - # - MSAL to get VM SSH certificate if 'data' in kwargs: + from azure.cli.core.util import in_cloud_console if in_cloud_console(): + # Use MSAL to get VM SSH certificate import msal from .util import check_result, build_sdk_access_token + from .identity import AZURE_CLI_CLIENT_ID app = msal.PublicClientApplication( - "04b07795-8ddb-461a-bbee-02f9e1bf7b46", # Use a real client_id, so that cache would work - #token_cache=..., # TODO: This PoC does not currently maintain a token cache; - # Ideally we should reuse the real MSAL app object which has cache configured. + AZURE_CLI_CLIENT_ID, # Use a real client_id, so that cache would work + # TODO: This PoC does not currently maintain a token cache; + # Ideally we should reuse the real MSAL app object which has cache configured. + # token_cache=..., ) result = app.acquire_token_interactive(list(scopes), prompt="none", data=kwargs["data"]) - check_result(result) + check_result(result, scopes=scopes) return build_sdk_access_token(result) else: from azure.cli.core.azclierror import AuthenticationError raise AuthenticationError("VM SSH currently doesn't support managed identity.") + # Use msrestazure to get access token resource = scopes_to_resource(_normalize_scopes(scopes)) if resource: # If available, use resource provided by SDK diff --git a/src/azure-cli-core/azure/cli/core/auth/credential_adaptor.py b/src/azure-cli-core/azure/cli/core/auth/credential_adaptor.py index 59e6ad3a426..782c8d02fea 100644 --- a/src/azure-cli-core/azure/cli/core/auth/credential_adaptor.py +++ b/src/azure-cli-core/azure/cli/core/auth/credential_adaptor.py @@ -44,7 +44,7 @@ def _get_token(self, scopes=None, **kwargs): raise CLIError(SSLERROR_TEMPLATE.format(str(err))) def signed_session(self, session=None): - logger.debug("CredentialAdaptor.get_token") + logger.debug("CredentialAdaptor.signed_session") session = session or requests.Session() token, external_tenant_tokens = self._get_token() header = "{} {}".format('Bearer', token.token) diff --git a/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py index fe57d065200..330f83ac412 100644 --- a/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/msal_authentication.py @@ -6,7 +6,7 @@ """ Credentials defined in this module are alternative implementations of credentials provided by Azure Identity. -These credentials implements azure.core.credentials.TokenCredential by exposing get_token method for Track 2 +These credentials implement azure.core.credentials.TokenCredential by exposing get_token method for Track 2 SDK invocation. """ diff --git a/src/azure-cli-core/azure/cli/core/auth/util.py b/src/azure-cli-core/azure/cli/core/auth/util.py index 45a49581bf0..53030e7bee8 100644 --- a/src/azure-cli-core/azure/cli/core/auth/util.py +++ b/src/azure-cli-core/azure/cli/core/auth/util.py @@ -20,17 +20,27 @@ def aad_error_handler(error, **kwargs): # https://docs.microsoft.com/en-us/azure/active-directory/develop/reference-aadsts-error-codes # Search for an error code at https://login.microsoftonline.com/error + # To trigger this function for testing, simply provide an invalid scope: + # az account get-access-token --scope https://my-invalid-scope + from azure.cli.core.util import in_cloud_console if in_cloud_console(): import socket logger.warning("A Cloud Shell credential problem occurred. When you report the issue with the error " "below, please mention the hostname '%s'", socket.gethostname()) - msg = error.get('error_description') - login_message = _generate_login_message(**kwargs) + error_description = error.get('error_description') + + # Build recommendation message + login_command = _generate_login_command(**kwargs) + login_message = ( + # Cloud Shell uses IMDS-like interface for implicit login. If getting token/cert failed, + # we let the user explicitly log in to AAD with MSAL. + "Please explicitly log in with:\n{}" if error.get('error') == 'broker_error' + else "To re-authenticate, please run:\n{}").format(login_command) from azure.cli.core.azclierror import AuthenticationError - raise AuthenticationError(msg, recommendation=login_message) + raise AuthenticationError(error_description, recommendation=login_message) def _generate_login_command(scopes=None): @@ -43,16 +53,6 @@ def _generate_login_command(scopes=None): return ' '.join(login_command) -def _generate_login_message(**kwargs): - from azure.cli.core.util import in_cloud_console - login_command = _generate_login_command(**kwargs) - - msg = "To re-authenticate, please {}" .format( - "refresh Azure Portal." if in_cloud_console() else "run:\n{}".format(login_command)) - - return msg - - def resource_to_scopes(resource): """Convert the ADAL resource ID to MSAL scopes by appending the /.default suffix and return a list. For example: From 2d13106f61b535142dafb8f6e343064e57584ba7 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Wed, 18 May 2022 23:38:09 +0800 Subject: [PATCH 6/7] pylint --- .../azure/cli/core/auth/adal_authentication.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py index f96dab82c2f..d37a85ec99b 100644 --- a/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/auth/adal_authentication.py @@ -30,13 +30,13 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument # TODO: This PoC does not currently maintain a token cache; # Ideally we should reuse the real MSAL app object which has cache configured. # token_cache=..., - ) + ) result = app.acquire_token_interactive(list(scopes), prompt="none", data=kwargs["data"]) check_result(result, scopes=scopes) return build_sdk_access_token(result) - else: - from azure.cli.core.azclierror import AuthenticationError - raise AuthenticationError("VM SSH currently doesn't support managed identity.") + + from azure.cli.core.azclierror import AuthenticationError + raise AuthenticationError("VM SSH currently doesn't support managed identity.") # Use msrestazure to get access token resource = scopes_to_resource(_normalize_scopes(scopes)) From 4bfa3036edbc52547fe8ad0b6f37e0725418c652 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Thu, 19 May 2022 16:39:41 +0800 Subject: [PATCH 7/7] msal==1.18.0b1 --- src/azure-cli-core/setup.py | 2 +- src/azure-cli/requirements.py3.Darwin.txt | 2 +- src/azure-cli/requirements.py3.Linux.txt | 3 +-- src/azure-cli/requirements.py3.windows.txt | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/azure-cli-core/setup.py b/src/azure-cli-core/setup.py index 0fffb7c1a9d..e54f09f91f3 100644 --- a/src/azure-cli-core/setup.py +++ b/src/azure-cli-core/setup.py @@ -52,7 +52,7 @@ 'jmespath', 'knack~=0.9.0', 'msal-extensions~=1.0.0', - 'msal @ git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@cloudshell-imds', # TBD: 'msal>=1.18.0,<2.0.0', + 'msal==1.18.0b1', 'msrestazure~=0.6.4', 'packaging>=20.9,<22.0', 'paramiko>=2.0.8,<3.0.0', diff --git a/src/azure-cli/requirements.py3.Darwin.txt b/src/azure-cli/requirements.py3.Darwin.txt index 746e2854177..aeb8b8508b1 100644 --- a/src/azure-cli/requirements.py3.Darwin.txt +++ b/src/azure-cli/requirements.py3.Darwin.txt @@ -110,7 +110,7 @@ jsondiff==1.3.0 knack==0.9.0 MarkupSafe==1.1.1 msal-extensions==1.0.0 -msal==1.17.0 +msal==1.18.0b1 msrest==0.6.21 msrestazure==0.6.4 oauthlib==3.0.1 diff --git a/src/azure-cli/requirements.py3.Linux.txt b/src/azure-cli/requirements.py3.Linux.txt index 1c23919eeca..6388bc7ff5c 100644 --- a/src/azure-cli/requirements.py3.Linux.txt +++ b/src/azure-cli/requirements.py3.Linux.txt @@ -111,8 +111,7 @@ jsondiff==1.3.0 knack==0.9.0 MarkupSafe==1.1.1 msal-extensions==1.0.0 -#msal==1.17.0 -git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@cloudshell-imds#egg=msal +msal==1.18.0b1 msrest==0.6.21 msrestazure==0.6.4 oauthlib==3.0.1 diff --git a/src/azure-cli/requirements.py3.windows.txt b/src/azure-cli/requirements.py3.windows.txt index 4e79e9c6999..e8acc08b997 100644 --- a/src/azure-cli/requirements.py3.windows.txt +++ b/src/azure-cli/requirements.py3.windows.txt @@ -110,7 +110,7 @@ jsondiff==1.3.0 knack==0.9.0 MarkupSafe==1.1.1 msal-extensions==1.0.0 -msal==1.17.0 +msal==1.18.0b1 msrest==0.6.21 msrestazure==0.6.4 oauthlib==3.0.1