From 4806ffd0c9ad93bf677b8b8e608c8cd67708c402 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 24 Feb 2021 15:01:36 +0800 Subject: [PATCH 1/6] Show original ADAL error --- src/azure-cli-core/azure/cli/core/_profile.py | 27 ++++++++------- .../azure/cli/core/adal_authentication.py | 33 ++++++++++--------- .../azure/cli/core/azclierror.py | 4 +++ 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 7a060f7667a..6da52428e97 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -674,17 +674,22 @@ def get_raw_token(self, resource=None, subscription=None, tenant=None): creds = self._get_token_from_cloud_shell(resource) else: tenant_dest = tenant if tenant else account[_TENANT_ID] - if user_type == _USER: - # User - creds = self._creds_cache.retrieve_token_for_user(username_or_sp_id, - tenant_dest, resource) - else: - # Service Principal - use_cert_sn_issuer = bool(account[_USER_ENTITY].get(_SERVICE_PRINCIPAL_CERT_SN_ISSUER_AUTH)) - creds = self._creds_cache.retrieve_token_for_service_principal(username_or_sp_id, - resource, - tenant_dest, - use_cert_sn_issuer) + import adal + try: + if user_type == _USER: + # User + creds = self._creds_cache.retrieve_token_for_user(username_or_sp_id, + tenant_dest, resource) + else: + # Service Principal + use_cert_sn_issuer = bool(account[_USER_ENTITY].get(_SERVICE_PRINCIPAL_CERT_SN_ISSUER_AUTH)) + creds = self._creds_cache.retrieve_token_for_service_principal(username_or_sp_id, + resource, + tenant_dest, + use_cert_sn_issuer) + except adal.AdalError as ex: + from azure.cli.core.adal_authentication import aad_exception_handler + aad_exception_handler(ex) return (creds, None if tenant else str(account[_SUBSCRIPTION_ID]), str(tenant if tenant else account[_TENANT_ID])) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 1ba6d504928..01085fd4499 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -43,24 +43,9 @@ def _get_token(self, sdk_resource=None): AdalAuthentication._log_hostname() raise err except adal.AdalError as err: - # pylint: disable=no-member if in_cloud_console(): AdalAuthentication._log_hostname() - - err = (getattr(err, 'error_response', None) or {}).get('error_description') or str(err) - if 'AADSTS70008' in err: # all errors starting with 70008 should be creds expiration related - raise CLIError("Credentials have expired due to inactivity. {}".format( - "Please run 'az login'" if not in_cloud_console() else '')) - if 'AADSTS50079' in err: - raise CLIError("Configuration of your account was changed. {}".format( - "Please run 'az login'" if not in_cloud_console() else '')) - if 'AADSTS50173' in err: - raise CLIError("The credential data used by CLI has been expired because you might have changed or " - "reset the password. {}".format( - "Please clear browser's cookies and run 'az login'" - if not in_cloud_console() else '')) - - raise CLIError(err) + aad_exception_handler(err) except requests.exceptions.SSLError as err: from .util import SSLERROR_TEMPLATE raise CLIError(SSLERROR_TEMPLATE.format(str(err))) @@ -246,3 +231,19 @@ def _timestamp(dt): # https://docs.python.org/3/library/unittest.mock-examples.html#partial-mocking # https://williambert.online/2011/07/how-to-unit-testing-in-django-with-mocking-and-patching/ return dt.timestamp() + + +def aad_exception_handler(ex): + login_message = ("To re-authenticate, please {}. If the problem persists, " + "please contact your tenant administrator." + .format("refresh Azure Portal" if in_cloud_console() else "run `az login`")) + + # 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 + try: + msg = ex.error_response['error_description'] + except (KeyError, AttributeError): + msg = str(ex) + + from azure.cli.core.azclierror import AuthenticationError + raise AuthenticationError(msg, login_message) from ex diff --git a/src/azure-cli-core/azure/cli/core/azclierror.py b/src/azure-cli-core/azure/cli/core/azclierror.py index 246e5919a57..5749d000847 100644 --- a/src/azure-cli-core/azure/cli/core/azclierror.py +++ b/src/azure-cli-core/azure/cli/core/azclierror.py @@ -260,4 +260,8 @@ class RecommendationError(ClientError): """ The client error raised by `az next`. It is needed in `az next` to skip error records. """ pass + +class AuthenticationError(ServiceError): + """ Raised when AAD authentication fails. """ + # endregion From 46393a585ada71d76bd074ad8d1a5cca8710617b Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 24 Feb 2021 17:25:41 +0800 Subject: [PATCH 2/6] handle msal error --- src/azure-cli-core/azure/cli/core/_profile.py | 4 ++++ .../azure/cli/core/adal_authentication.py | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 6da52428e97..7180219af07 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -23,6 +23,7 @@ from azure.cli.core.util import get_file_json, in_cloud_console, open_page_in_browser, can_launch_browser,\ is_windows, is_wsl from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription +from azure.cli.core.adal_authentication import aad_exception_handler logger = get_logger(__name__) @@ -631,6 +632,9 @@ def get_msal_token(self, scopes, data): authority = posixpath.join(self.cli_ctx.cloud.endpoints.active_directory, tenant) app = ClientApplication(_CLIENT_ID, authority=authority) result = app.acquire_token_by_refresh_token(refresh_token, scopes, data=data) + + if 'error' in result: + aad_exception_handler(result) return username, result["access_token"] def get_refresh_token(self, resource=None, diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 01085fd4499..d3fc2126d5f 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -45,7 +45,10 @@ def _get_token(self, sdk_resource=None): except adal.AdalError as err: if in_cloud_console(): AdalAuthentication._log_hostname() - aad_exception_handler(err) + try: + aad_exception_handler(err.error_response) + except AttributeError: + raise CLIError(str(err)) except requests.exceptions.SSLError as err: from .util import SSLERROR_TEMPLATE raise CLIError(SSLERROR_TEMPLATE.format(str(err))) @@ -233,17 +236,14 @@ def _timestamp(dt): return dt.timestamp() -def aad_exception_handler(ex): +def aad_exception_handler(ex: dict): login_message = ("To re-authenticate, please {}. If the problem persists, " "please contact your tenant administrator." .format("refresh Azure Portal" if in_cloud_console() else "run `az login`")) # 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 - try: - msg = ex.error_response['error_description'] - except (KeyError, AttributeError): - msg = str(ex) + msg = ex.get('error_description') from azure.cli.core.azclierror import AuthenticationError - raise AuthenticationError(msg, login_message) from ex + raise AuthenticationError(msg, login_message) From 8a964aeddc8afd189524ee7bdf0271658147dd52 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 24 Feb 2021 18:56:11 +0800 Subject: [PATCH 3/6] adal_exception_handler --- src/azure-cli-core/azure/cli/core/_profile.py | 4 ++-- .../azure/cli/core/adal_authentication.py | 20 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 7180219af07..dd5305a5a9c 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -23,7 +23,7 @@ from azure.cli.core.util import get_file_json, in_cloud_console, open_page_in_browser, can_launch_browser,\ is_windows, is_wsl from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription -from azure.cli.core.adal_authentication import aad_exception_handler +from azure.cli.core.adal_authentication import aad_exception_handler, adal_exception_handler logger = get_logger(__name__) @@ -693,7 +693,7 @@ def get_raw_token(self, resource=None, subscription=None, tenant=None): use_cert_sn_issuer) except adal.AdalError as ex: from azure.cli.core.adal_authentication import aad_exception_handler - aad_exception_handler(ex) + adal_exception_handler(ex) return (creds, None if tenant else str(account[_SUBSCRIPTION_ID]), str(tenant if tenant else account[_TENANT_ID])) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index d3fc2126d5f..771b0e72661 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -45,10 +45,7 @@ def _get_token(self, sdk_resource=None): except adal.AdalError as err: if in_cloud_console(): AdalAuthentication._log_hostname() - try: - aad_exception_handler(err.error_response) - except AttributeError: - raise CLIError(str(err)) + adal_exception_handler(err) except requests.exceptions.SSLError as err: from .util import SSLERROR_TEMPLATE raise CLIError(SSLERROR_TEMPLATE.format(str(err))) @@ -236,14 +233,25 @@ def _timestamp(dt): return dt.timestamp() -def aad_exception_handler(ex: dict): +def aad_exception_handler(error: dict): + """ Handle the error from AAD server returned by ADAL or MSAL. """ login_message = ("To re-authenticate, please {}. If the problem persists, " "please contact your tenant administrator." .format("refresh Azure Portal" if in_cloud_console() else "run `az login`")) # 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 - msg = ex.get('error_description') + msg = error.get('error_description') from azure.cli.core.azclierror import AuthenticationError raise AuthenticationError(msg, login_message) + + +def adal_exception_handler(err: adal.AdalError): + """ Handle AdalError. """ + try: + aad_exception_handler(err.error_response) + except AttributeError: + # In case of AdalError created as + # AdalError('More than one token matches the criteria. The result is ambiguous.') + raise CLIError(str(err)) From 837cad96be0297cc2724c5eb720fb5879a6c5d54 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Wed, 24 Feb 2021 18:58:58 +0800 Subject: [PATCH 4/6] adal_exception_handler --- src/azure-cli-core/azure/cli/core/_profile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index dd5305a5a9c..598b4131d2f 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -23,7 +23,6 @@ from azure.cli.core.util import get_file_json, in_cloud_console, open_page_in_browser, can_launch_browser,\ is_windows, is_wsl from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription -from azure.cli.core.adal_authentication import aad_exception_handler, adal_exception_handler logger = get_logger(__name__) @@ -634,6 +633,7 @@ def get_msal_token(self, scopes, data): result = app.acquire_token_by_refresh_token(refresh_token, scopes, data=data) if 'error' in result: + from azure.cli.core.adal_authentication import aad_exception_handler aad_exception_handler(result) return username, result["access_token"] @@ -692,7 +692,7 @@ def get_raw_token(self, resource=None, subscription=None, tenant=None): tenant_dest, use_cert_sn_issuer) except adal.AdalError as ex: - from azure.cli.core.adal_authentication import aad_exception_handler + from azure.cli.core.adal_authentication import adal_exception_handler adal_exception_handler(ex) return (creds, None if tenant else str(account[_SUBSCRIPTION_ID]), From 820b83360566e1849ffbe1cf689c025fc9250157 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Thu, 25 Feb 2021 18:09:59 +0800 Subject: [PATCH 5/6] Rename to error_handler --- src/azure-cli-core/azure/cli/core/_profile.py | 8 ++++---- src/azure-cli-core/azure/cli/core/adal_authentication.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 598b4131d2f..ff2da30d551 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -633,8 +633,8 @@ def get_msal_token(self, scopes, data): result = app.acquire_token_by_refresh_token(refresh_token, scopes, data=data) if 'error' in result: - from azure.cli.core.adal_authentication import aad_exception_handler - aad_exception_handler(result) + from azure.cli.core.adal_authentication import aad_error_handler + aad_error_handler(result) return username, result["access_token"] def get_refresh_token(self, resource=None, @@ -692,8 +692,8 @@ def get_raw_token(self, resource=None, subscription=None, tenant=None): tenant_dest, use_cert_sn_issuer) except adal.AdalError as ex: - from azure.cli.core.adal_authentication import adal_exception_handler - adal_exception_handler(ex) + from azure.cli.core.adal_authentication import adal_error_handler + adal_error_handler(ex) return (creds, None if tenant else str(account[_SUBSCRIPTION_ID]), str(tenant if tenant else account[_TENANT_ID])) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 771b0e72661..91ad81a8af2 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -45,7 +45,7 @@ def _get_token(self, sdk_resource=None): except adal.AdalError as err: if in_cloud_console(): AdalAuthentication._log_hostname() - adal_exception_handler(err) + adal_error_handler(err) except requests.exceptions.SSLError as err: from .util import SSLERROR_TEMPLATE raise CLIError(SSLERROR_TEMPLATE.format(str(err))) @@ -233,7 +233,7 @@ def _timestamp(dt): return dt.timestamp() -def aad_exception_handler(error: dict): +def aad_error_handler(error: dict): """ Handle the error from AAD server returned by ADAL or MSAL. """ login_message = ("To re-authenticate, please {}. If the problem persists, " "please contact your tenant administrator." @@ -247,10 +247,10 @@ def aad_exception_handler(error: dict): raise AuthenticationError(msg, login_message) -def adal_exception_handler(err: adal.AdalError): +def adal_error_handler(err: adal.AdalError): """ Handle AdalError. """ try: - aad_exception_handler(err.error_response) + aad_error_handler(err.error_response) except AttributeError: # In case of AdalError created as # AdalError('More than one token matches the criteria. The result is ambiguous.') From c62df4d79ac61fc40a857897a5b6e2d83ed17830 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Tue, 2 Mar 2021 10:54:08 +0800 Subject: [PATCH 6/6] Raise UnknownError --- src/azure-cli-core/azure/cli/core/adal_authentication.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 91ad81a8af2..f14276d1d2f 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -254,4 +254,6 @@ def adal_error_handler(err: adal.AdalError): except AttributeError: # In case of AdalError created as # AdalError('More than one token matches the criteria. The result is ambiguous.') - raise CLIError(str(err)) + # https://github.com/Azure/azure-cli/issues/15320 + from azure.cli.core.azclierror import UnknownError + raise UnknownError(str(err), recommendation="Please run `az account clear`, then `az login`.")