diff --git a/src/azure-cli-core/azure/cli/core/_identity.py b/src/azure-cli-core/azure/cli/core/_identity.py index f8fc541f558..7beb05dd86d 100644 --- a/src/azure-cli-core/azure/cli/core/_identity.py +++ b/src/azure-cli-core/azure/cli/core/_identity.py @@ -17,7 +17,8 @@ ClientSecretCredential, CertificateCredential, ManagedIdentityCredential, - EnvironmentCredential + EnvironmentCredential, + TokenCachePersistenceOptions ) from ._environment import get_config_dir @@ -65,6 +66,7 @@ def __init__(self, authority=None, tenant_id=None, client_id=None, **kwargs): self._msal_app_instance = None # Store for Service principal credential persistence self._msal_secret_store = MsalSecretStore(fallback_to_plaintext=self.allow_unencrypted) + self._cache_persistence_options = TokenCachePersistenceOptions(name="azcli", allow_unencrypted_storage=True) # TODO: Allow disabling SSL verification # The underlying requests lib of MSAL has been patched with Azure Core by MsalTransportAdapter @@ -86,13 +88,19 @@ def __init__(self, authority=None, tenant_id=None, client_id=None, **kwargs): # - Access token # - Service principal secret # - Service principal certificate - # self._credential_kwargs['logging_enable'] = True + self._credential_kwargs['logging_enable'] = True + + # Make MSAL remove existing accounts on successful login. + # self._credential_kwargs['remove_existing_account'] = True + # from azure.cli.core._msal_patch import patch_token_cache_add + # patch_token_cache_add(self.msal_app.remove_account) def _load_msal_cache(self): # sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py:95 - from azure.identity._internal.persistent_cache import load_user_cache + from azure.identity._persistent_cache import _load_persistent_cache # Store for user token persistence - cache = load_user_cache(self.allow_unencrypted) + cache = _load_persistent_cache(self._cache_persistence_options) + cache._reload_if_necessary() # pylint: disable=protected-access return cache def _build_persistent_msal_app(self, authority): @@ -104,7 +112,7 @@ def _build_persistent_msal_app(self, authority): return msal_app @property - def _msal_app(self): + def msal_app(self): if not self._msal_app_instance: # Build the authority in MSAL style, like https://login.microsoftonline.com/your_tenant msal_authority = "{}/{}".format(self.authority, self.tenant_id) @@ -120,8 +128,7 @@ def login_with_interactive_browser(self, scopes=None): credential = InteractiveBrowserCredential(authority=self.authority, tenant_id=self.tenant_id, client_id=self.client_id, - enable_persistent_cache=True, - allow_unencrypted_cache=self.allow_unencrypted, + cache_persistence_options=self._cache_persistence_options, **self._credential_kwargs) auth_record = credential.authenticate(scopes=scopes) # todo: remove after ADAL token deprecation @@ -139,9 +146,8 @@ def prompt_callback(verification_uri, user_code, _): credential = DeviceCodeCredential(authority=self.authority, tenant_id=self.tenant_id, client_id=self.client_id, - enable_persistent_cache=True, prompt_callback=prompt_callback, - allow_unencrypted_cache=self.allow_unencrypted, + cache_persistence_options=self._cache_persistence_options, **self._credential_kwargs) auth_record = credential.authenticate(scopes=scopes) @@ -163,8 +169,7 @@ def login_with_username_password(self, username, password, scopes=None): client_id=self.client_id, username=username, password=password, - enable_persistent_cache=True, - allow_unencrypted_cache=self.allow_unencrypted, + cache_persistence_options=self._cache_persistence_options, **self._credential_kwargs) auth_record = credential.authenticate(scopes=scopes) @@ -305,15 +310,15 @@ def login_in_cloud_shell(self, scopes): return credential, cloud_shell_identity_info def logout_user(self, user): - accounts = self._msal_app.get_accounts(user) + accounts = self.msal_app.get_accounts(user) logger.info('Before account removal:') logger.info(json.dumps(accounts)) # `accounts` are the same user in all tenants, log out all of them for account in accounts: - self._msal_app.remove_account(account) + self.msal_app.remove_account(account) - accounts = self._msal_app.get_accounts(user) + accounts = self.msal_app.get_accounts(user) logger.info('After account removal:') logger.info(json.dumps(accounts)) @@ -323,25 +328,25 @@ def logout_sp(self, sp): def logout_all(self): # TODO: Support multi-authority logout - accounts = self._msal_app.get_accounts() + accounts = self.msal_app.get_accounts() logger.info('Before account removal:') logger.info(json.dumps(accounts)) for account in accounts: - self._msal_app.remove_account(account) + self.msal_app.remove_account(account) - accounts = self._msal_app.get_accounts() + accounts = self.msal_app.get_accounts() logger.info('After account removal:') logger.info(json.dumps(accounts)) # remove service principal secrets self._msal_secret_store.remove_all_cached_creds() def get_user(self, user=None): - accounts = self._msal_app.get_accounts(user) if user else self._msal_app.get_accounts() + accounts = self.msal_app.get_accounts(user) if user else self.msal_app.get_accounts() return accounts def get_user_credential(self, username): - accounts = self._msal_app.get_accounts(username) + accounts = self.msal_app.get_accounts(username) # TODO: Confirm with MSAL team that username can uniquely identify the account if not accounts: @@ -352,8 +357,7 @@ def get_user_credential(self, username): auth_record = AuthenticationRecord(self.tenant_id, self.client_id, self.authority, account['home_account_id'], username) return InteractiveBrowserCredential(authentication_record=auth_record, disable_automatic_authentication=True, - enable_persistent_cache=True, - allow_unencrypted_cache=self.allow_unencrypted, + cache_persistence_options=self._cache_persistence_options, **self._credential_kwargs) def get_service_principal_credential(self, client_id, use_cert_sn_issuer): @@ -427,7 +431,6 @@ def serialize_token_cache(self, path=None): "It contains login information of all logged-in users. Make sure you protect it safely.", path) cache = self._load_msal_cache() - cache._reload_if_necessary() # pylint: disable=protected-access with open(path, "w") as fd: fd.write(cache.serialize()) diff --git a/src/azure-cli-core/azure/cli/core/_msal_patch.py b/src/azure-cli-core/azure/cli/core/_msal_patch.py new file mode 100644 index 00000000000..15129d081de --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/_msal_patch.py @@ -0,0 +1,173 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +""" +A temporary workaround for MSAL limitation +https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/335 + +After a successful sign-in, if the sign-in account already exists in the token +cache, remove it first along with its tokens to prevent MSAL from returning +cached access tokens from the previous session that may have been revoked. + +Otherwise, MSAL will return revoked access tokens, resulting in 401 failure +which can't be handled by commands that don't support silent reauth. +""" + +# pylint: skip-file +# flake8: noqa + +import json +import time + +from knack.log import get_logger + +from msal.oauth2cli.oauth2 import Client +from msal.token_cache import decode_id_token, canonicalize, decode_part + +logger = get_logger(__name__) + + +def patch_token_cache_add(callback): + + def __add(self, event, now=None): + # event typically contains: client_id, scope, token_endpoint, + # response, params, data, grant_type + environment = realm = None + if "token_endpoint" in event: + _, environment, realm = canonicalize(event["token_endpoint"]) + if "environment" in event: # Always available unless in legacy test cases + environment = event["environment"] # Set by application.py + response = event.get("response", {}) + data = event.get("data", {}) + access_token = response.get("access_token") + refresh_token = response.get("refresh_token") + id_token = response.get("id_token") + id_token_claims = ( + decode_id_token(id_token, client_id=event["client_id"]) + if id_token else {}) + client_info = {} + home_account_id = None # It would remain None in client_credentials flow + if "client_info" in response: # We asked for it, and AAD will provide it + client_info = json.loads(decode_part(response["client_info"])) + home_account_id = "{uid}.{utid}".format(**client_info) + elif id_token_claims: # This would be an end user on ADFS-direct scenario + client_info["uid"] = id_token_claims.get("sub") + home_account_id = id_token_claims.get("sub") + + target = ' '.join(event.get("scope") or []) # Per schema, we don't sort it + + with self._lock: + now = int(time.time() if now is None else now) + + if client_info and not event.get("skip_account_creation"): + account = { + "home_account_id": home_account_id, + "environment": environment, + "realm": realm, + "local_account_id": id_token_claims.get( + "oid", id_token_claims.get("sub")), + "username": id_token_claims.get("preferred_username") # AAD + or id_token_claims.get("upn") # ADFS 2019 + or "", # The schema does not like null + "authority_type": + self.AuthorityType.ADFS if realm == "adfs" + else self.AuthorityType.MSSTS, + # "client_info": response.get("client_info"), # Optional + } + + logger.debug("Remove existing account %r", account) + logger.debug("Calling %r", callback) + callback(account) + self.modify(self.CredentialType.ACCOUNT, account, account) + + if id_token: + idt = { + "credential_type": self.CredentialType.ID_TOKEN, + "secret": id_token, + "home_account_id": home_account_id, + "environment": environment, + "realm": realm, + "client_id": event.get("client_id"), + # "authority": "it is optional", + } + self.modify(self.CredentialType.ID_TOKEN, idt, idt) + + if access_token: + expires_in = int( # AADv1-like endpoint returns a string + response.get("expires_in", 3599)) + ext_expires_in = int( # AADv1-like endpoint returns a string + response.get("ext_expires_in", expires_in)) + at = { + "credential_type": self.CredentialType.ACCESS_TOKEN, + "secret": access_token, + "home_account_id": home_account_id, + "environment": environment, + "client_id": event.get("client_id"), + "target": target, + "realm": realm, + "token_type": response.get("token_type", "Bearer"), + "cached_at": str(now), # Schema defines it as a string + "expires_on": str(now + expires_in), # Same here + "extended_expires_on": str(now + ext_expires_in) # Same here + } + if data.get("key_id"): # It happens in SSH-cert or POP scenario + at["key_id"] = data.get("key_id") + if "refresh_in" in response: + refresh_in = response["refresh_in"] # It is an integer + at["refresh_on"] = str(now + refresh_in) # Schema wants a string + self.modify(self.CredentialType.ACCESS_TOKEN, at, at) + + if refresh_token: + rt = { + "credential_type": self.CredentialType.REFRESH_TOKEN, + "secret": refresh_token, + "home_account_id": home_account_id, + "environment": environment, + "client_id": event.get("client_id"), + "target": target, # Optional per schema though + "last_modification_time": str(now), # Optional. Schema defines it as a string. + } + if "foci" in response: + rt["family_id"] = response["foci"] + self.modify(self.CredentialType.REFRESH_TOKEN, rt, rt) + + app_metadata = { + "client_id": event.get("client_id"), + "environment": environment, + } + if "foci" in response: + app_metadata["family_id"] = response.get("foci") + self.modify(self.CredentialType.APP_METADATA, app_metadata, app_metadata) + + def obtain_token_by_refresh_token(self, token_item, scope=None, + rt_getter=lambda token_item: token_item["refresh_token"], + on_removing_rt=None, + on_updating_rt=None, + on_obtaining_tokens=None, + **kwargs): + resp = super(Client, self).obtain_token_by_refresh_token( + rt_getter(token_item) + if not isinstance(token_item, str) else token_item, + scope=scope, + also_save_rt=on_updating_rt is False, + on_obtaining_tokens=on_obtaining_tokens, + **kwargs) + if resp.get('error') == 'invalid_grant': + (on_removing_rt or self.on_removing_rt)(token_item) # Discard old RT + RT = "refresh_token" + if on_updating_rt is not False and RT in resp: + (on_updating_rt or self.on_updating_rt)(token_item, resp[RT]) + return resp + + from unittest.mock import patch + + # Temporary patch for https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/335 + cm_add = patch('msal.token_cache.TokenCache._TokenCache__add', __add) + + # Temporary patch for https://github.com/AzureAD/microsoft-authentication-library-for-python/pull/339 + cm_obtain_token_by_refresh_token = patch('msal.oauth2cli.oauth2.Client.obtain_token_by_refresh_token', + obtain_token_by_refresh_token) + cm_add.__enter__() + cm_obtain_token_by_refresh_token.__enter__() diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 158e7a26f40..9db526f8898 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -536,24 +536,24 @@ def logout(self, user_or_sp, clear_credential): adal_cache = AdalCredentialCache() adal_cache.remove_cached_creds(user_or_sp) - logger.warning('Account %s has been logged out from Azure CLI.', user_or_sp) + logger.warning("Account '%s' has been logged out from Azure CLI.", user_or_sp) else: # https://english.stackexchange.com/questions/5302/log-in-to-or-log-into-or-login-to - logger.warning("Account %s was not logged in to Azure CLI.", user_or_sp) + logger.warning("Account '%s' was not logged in to Azure CLI.", user_or_sp) # Log out from MSAL cache identity = Identity(self._authority) accounts = identity.get_user(user_or_sp) if accounts: - logger.info("The credential of %s were found from MSAL encrypted cache.", user_or_sp) + logger.info("The credential of '%s' were found from MSAL encrypted cache.", user_or_sp) if clear_credential: identity.logout_user(user_or_sp) - logger.warning("The credential of %s were cleared from MSAL encrypted cache. This account is " + logger.warning("The credential of '%s' were cleared from MSAL encrypted cache. This account is " "also logged out from other SDK tools which use Azure CLI's credential " "via Single Sign-On.", user_or_sp) else: - logger.warning('The credential of %s is still stored in MSAL encrypted cached. Other SDK tools may use ' - 'Azure CLI\'s credential via Single Sign-On. ' + logger.warning("The credential of '%s' is still stored in MSAL encrypted cached. Other SDK tools may " + "use Azure CLI\'s credential via Single Sign-On. " 'To clear the credential, run `az logout --username %s --clear-credential`.', user_or_sp, user_or_sp) else: @@ -582,7 +582,7 @@ def logout_all(self, clear_credential): logger.warning(account['username']) logger.warning('Other SDK tools may use Azure CLI\'s credential via Single Sign-On. ' 'To clear all credentials, run `az account clear --clear-credential`. ' - 'To clear one of them, run `az logout --username USERNAME` --clear-credential.') + 'To clear one of them, run `az logout --username USERNAME --clear-credential`.') else: logger.warning('No credential was not found from MSAL encrypted cache.') @@ -888,11 +888,11 @@ def __init__(self, cli_ctx, arm_client_factory=None, **kwargs): self.authority = self.cli_ctx.cloud.endpoints.active_directory self.adal_cache = kwargs.pop("adal_cache", None) - def create_arm_client_factory(credentials): + def create_arm_client_factory(credential): if arm_client_factory: - return arm_client_factory(credentials) + return arm_client_factory(credential) from azure.cli.core.profiles import ResourceType, get_api_version - from azure.cli.core.commands.client_factory import _prepare_client_kwargs_track2 + from azure.cli.core.commands.client_factory import _prepare_mgmt_client_kwargs_track2 client_type = self._get_subscription_client_class() if client_type is None: @@ -900,11 +900,10 @@ def create_arm_client_factory(credentials): raise CLIInternalError("Unable to get '{}' in profile '{}'" .format(ResourceType.MGMT_RESOURCE_SUBSCRIPTIONS, cli_ctx.cloud.profile)) api_version = get_api_version(cli_ctx, ResourceType.MGMT_RESOURCE_SUBSCRIPTIONS) - client_kwargs = _prepare_client_kwargs_track2(cli_ctx) + client_kwargs = _prepare_mgmt_client_kwargs_track2(cli_ctx, credential) # We don't need to change credential_scopes as 'scopes' is ignored by BasicTokenCredential anyway - client = client_type(credentials, api_version=api_version, + client = client_type(credential, api_version=api_version, base_url=self.cli_ctx.cloud.endpoints.resource_manager, - credential_scopes=resource_to_scopes(self._arm_resource_id), **client_kwargs) return client diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/azure-cli-core/azure/cli/core/azclierror.py b/src/azure-cli-core/azure/cli/core/azclierror.py index 297b42bf9bb..e4390ec2b8d 100644 --- a/src/azure-cli-core/azure/cli/core/azclierror.py +++ b/src/azure-cli-core/azure/cli/core/azclierror.py @@ -25,9 +25,10 @@ class AzCLIError(CLIError): """ Base class for all the AzureCLI defined error classes. DO NOT raise this error class in your codes. """ - def __init__(self, error_msg, recommendation=None): + def __init__(self, error_msg, recommendation=None, original_error=None): # error message self.error_msg = error_msg + self.original_error = original_error # manual recommendations provided based on developers' knowledge self.recommendations = [] @@ -169,7 +170,32 @@ class BadRequestError(UserFault): class UnauthorizedError(UserFault): """ Unauthorized request: 401 error """ - pass + + def __init__(self, error_msg, recommendation=None, original_error=None): + + def _extract_claims(challenge): + # Copied from azure.mgmt.core.policies._authentication._parse_claims_challenge + from azure.mgmt.core.policies._authentication import _parse_challenges + parsed_challenges = _parse_challenges(challenge) + if len(parsed_challenges) != 1 or "claims" not in parsed_challenges[0].parameters: + # no or multiple challenges, or no claims directive + return None + + encoded_claims = parsed_challenges[0].parameters["claims"] + padding_needed = -len(encoded_claims) % 4 + return encoded_claims + "=" * padding_needed + + claims = _extract_claims(original_error.response.headers.get('WWW-Authenticate')) + + from azure.cli.core.credential import _generate_login_command, _generate_login_message + # login_command = _generate_login_command(claims=claims) + login_message = _generate_login_message(claims=claims) + + recommendation = ( + "The access token has expired or been revoked by Continuous Access Evaluation. " + "Silent re-authentication will be attempted in the future.\n{}") + recommendation = recommendation.format(login_message) + super().__init__(error_msg, recommendation=recommendation, original_error=original_error) class ForbiddenError(UserFault): @@ -259,7 +285,7 @@ class RecommendationError(ClientError): pass -class AuthenticationError(ServiceError): - """ Raised when AAD authentication fails. """ +class AuthenticationError(AzCLIError): + """ Raised when credential.get_token fails. """ # endregion diff --git a/src/azure-cli-core/azure/cli/core/azlogging.py b/src/azure-cli-core/azure/cli/core/azlogging.py index d2c3fbf1064..d30af54387a 100644 --- a/src/azure-cli-core/azure/cli/core/azlogging.py +++ b/src/azure-cli-core/azure/cli/core/azlogging.py @@ -51,12 +51,18 @@ def __init__(self, name, cli_ctx=None): def configure(self, args): super(AzCliLogging, self).configure(args) - from knack.log import CliLogLevel - if self.log_level == CliLogLevel.DEBUG: - # As azure.core.pipeline.policies.http_logging_policy is a redacted version of - # azure.core.pipeline.policies._universal, disable azure.core.pipeline.policies.http_logging_policy - # when debug log is shown. - logging.getLogger("azure.core.pipeline.policies.http_logging_policy").setLevel(logging.CRITICAL) + if self.log_level: + # When invoked by pytest, configure() is skipped and log_level will not be set. + from knack.log import CliLogLevel + if self.log_level == CliLogLevel.DEBUG: + # As azure.core.pipeline.policies.http_logging_policy is a redacted version of + # azure.core.pipeline.policies._universal, disable azure.core.pipeline.policies.http_logging_policy + # when debug log is shown. + logging.getLogger("azure.core.pipeline.policies.http_logging_policy").setLevel(logging.CRITICAL) + + if self.log_level <= CliLogLevel.WARNING: + # Disable warnings from Azure Identity + logging.getLogger("azure.identity").setLevel(logging.CRITICAL) def get_command_log_dir(self): return self.command_log_dir diff --git a/src/azure-cli-core/azure/cli/core/commands/client_factory.py b/src/azure-cli-core/azure/cli/core/commands/client_factory.py index 96e4d12fc73..ba6dbfe149a 100644 --- a/src/azure-cli-core/azure/cli/core/commands/client_factory.py +++ b/src/azure-cli-core/azure/cli/core/commands/client_factory.py @@ -117,7 +117,7 @@ def configure_common_settings(cli_ctx, client): def _prepare_client_kwargs_track2(cli_ctx): - """Prepare kwargs for Track 2 SDK client.""" + """Prepare kwargs for Track 2 data and mgmt SDK clients.""" client_kwargs = {} # Prepare connection_verify to change SSL verification behavior, used by ConnectionConfiguration @@ -160,6 +160,23 @@ def _prepare_client_kwargs_track2(cli_ctx): return client_kwargs +def _prepare_mgmt_client_kwargs_track2(cli_ctx, cred): + """Prepare kwargs for Track 2 SDK mgmt client.""" + client_kwargs = _prepare_client_kwargs_track2(cli_ctx) + + # Enable CAE support in mgmt SDK + from azure.mgmt.core.policies import ARMChallengeAuthenticationPolicy + + # Track 2 SDK maintains `scopes` and passes `scopes` to get_token. + scopes = resource_to_scopes(cli_ctx.cloud.endpoints.active_directory_resource_id) + policy = ARMChallengeAuthenticationPolicy(cred, *scopes) + + client_kwargs['credential_scopes'] = scopes + client_kwargs['authentication_policy'] = policy + + return client_kwargs + + def _get_mgmt_service_client(cli_ctx, client_type, subscription_bound=True, @@ -167,7 +184,6 @@ def _get_mgmt_service_client(cli_ctx, api_version=None, base_url_bound=True, resource=None, - credential_scopes=None, sdk_profile=None, aux_subscriptions=None, aux_tenants=None, @@ -181,8 +197,6 @@ def _get_mgmt_service_client(cli_ctx, :param api_version: :param base_url_bound: :param resource: For track 1 SDK which uses msrest and ADAL. It will be passed to get_login_credentials. - :param credential_scopes: For track 2 SDK which uses Azure Identity and MSAL. It will be passed to the client's - __init__ method. :param sdk_profile: :param aux_subscriptions: :param aux_tenants: @@ -211,11 +225,7 @@ def _get_mgmt_service_client(cli_ctx, client_kwargs.update(kwargs) if is_track2(client_type): - client_kwargs.update(_prepare_client_kwargs_track2(cli_ctx)) - # Track 2 SDK maintains `scopes` and passes `scopes` to get_token. Specify `scopes` via `credential_scopes` - # in client's __init__ method. - client_kwargs['credential_scopes'] = credential_scopes or \ - resource_to_scopes(cli_ctx.cloud.endpoints.active_directory_resource_id) + client_kwargs.update(_prepare_mgmt_client_kwargs_track2(cli_ctx, cred=cred)) if subscription_bound: client = client_type(cred, subscription_id, **client_kwargs) diff --git a/src/azure-cli-core/azure/cli/core/credential.py b/src/azure-cli-core/azure/cli/core/credential.py index b843a9303f6..c9c7d9e8c72 100644 --- a/src/azure-cli-core/azure/cli/core/credential.py +++ b/src/azure-cli-core/azure/cli/core/credential.py @@ -5,11 +5,12 @@ from typing import Tuple, List +import json import requests from azure.cli.core._identity import resource_to_scopes from azure.cli.core.util import in_cloud_console from azure.core.credentials import AccessToken -from azure.core.exceptions import ClientAuthenticationError +from azure.identity import CredentialUnavailableError, AuthenticationRequiredError from knack.log import get_logger from knack.util import CLIError @@ -44,40 +45,25 @@ def _get_token(self, scopes=None, **kwargs): token = self._credential.get_token(*scopes, **kwargs) if self._external_credentials: external_tenant_tokens = [cred.get_token(*scopes) for cred in self._external_credentials] + return token, external_tenant_tokens except CLIError as err: if in_cloud_console(): CredentialAdaptor._log_hostname() raise err - except ClientAuthenticationError as err: - # pylint: disable=no-member - if in_cloud_console(): - CredentialAdaptor._log_hostname() - - err = getattr(err, 'message', None) or '' - if 'authentication is required' in err: - raise CLIError("Authentication is migrated to Microsoft identity platform (v2.0). {}".format( - "Please run 'az login' to login." if not in_cloud_console() else '')) - 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) + except AuthenticationRequiredError as err: + err_dict = json.loads(err.response.text()) + aad_error_handler(err_dict, scopes=err.scopes, claims=err.claims) + except CredentialUnavailableError as err: + err_dict = json.loads(err.response.text()) + aad_error_handler(err_dict) except requests.exceptions.SSLError as err: from .util import SSLERROR_TEMPLATE raise CLIError(SSLERROR_TEMPLATE.format(str(err))) except requests.exceptions.ConnectionError as err: raise CLIError('Please ensure you have network connection. Error detail: ' + str(err)) - return token, external_tenant_tokens def signed_session(self, session=None): - logger.debug("CredentialAdaptor.signed_session invoked by Track 1 SDK") + logger.debug("CredentialAdaptor.get_token") session = session or requests.Session() token, external_tenant_tokens = self._get_token() header = "{} {}".format('Bearer', token.token) @@ -88,8 +74,7 @@ def signed_session(self, session=None): return session def get_token(self, *scopes, **kwargs): - # type: (*str) -> AccessToken - logger.debug("CredentialAdaptor.get_token invoked by Track 2 SDK with scopes=%r", scopes) + logger.debug("CredentialAdaptor.get_token: scopes=%r, kwargs=%r", scopes, kwargs) scopes = _normalize_scopes(scopes) token, _ = self._get_token(scopes, **kwargs) return token @@ -125,3 +110,47 @@ def _normalize_scopes(scopes): return scopes[1:] return scopes + + +def _generate_login_command(scopes=None, claims=None): + login_command = ['az login'] + + if scopes: + login_command.append('--scope {}'.format(' '.join(scopes))) + + if claims: + import base64 + try: + base64.urlsafe_b64decode(claims) + is_base64 = True + except ValueError: + is_base64 = False + + if not is_base64: + claims = base64.urlsafe_b64encode(claims.encode()).decode() + + login_command.append('--claims {}'.format(claims)) + + return ' '.join(login_command) + + +def _generate_login_message(**kwargs): + login_command = _generate_login_command(**kwargs) + login_command = 'az logout\naz login' + msg = "To re-authenticate, please {}" \ + "If the problem persists, please contact your tenant administrator.".format( + "refresh Azure Portal." if in_cloud_console() else "run:\n{}\n".format(login_command)) + + return msg + + +def aad_error_handler(error, scopes=None, claims=None): + """ Handle the error from AAD server returned by ADAL or MSAL. """ + + # 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 = error.get('error_description') + login_message = _generate_login_message(scopes=scopes, claims=claims) + + from azure.cli.core.azclierror import AuthenticationError + raise AuthenticationError(msg, recommendation=login_message) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_credential.py b/src/azure-cli-core/azure/cli/core/tests/test_credential.py new file mode 100644 index 00000000000..a2d75827517 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/tests/test_credential.py @@ -0,0 +1,30 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import unittest + +from azure.cli.core.credential import _generate_login_command + + +class TestUtils(unittest.TestCase): + def test_generate_login_command(self): + # No parameter is given + assert _generate_login_command() == 'az login' + + base64_claims = "eyJhY2Nlc3NfdG9rZW4iOnsibmJmIjp7ImVzc2VudGlhbCI6dHJ1ZSwgInZhbHVlIjoiMTYxNzE3MjE1NiJ9fX0=" + json_claims = '{"access_token":{"nbf":{"essential":true, "value":"1617172156"}}}' + expect = 'az login --claims eyJhY2Nlc3NfdG9rZW4iOnsibmJmIjp7ImVzc2VudGlhbCI6dHJ1ZSwgInZhbHVlIjoiMTYxNzE3MjE1NiJ9fX0=' + + # Base64 string is preserved + actual = _generate_login_command(claims=base64_claims) + assert actual == expect + + # JSON string is converted to base64 + actual = _generate_login_command(claims=json_claims) + assert actual == expect + + # scopes + actual = _generate_login_command(scopes=["https://management.core.windows.net//.default"]) + assert actual == 'az login --scope https://management.core.windows.net//.default' diff --git a/src/azure-cli-core/azure/cli/core/tests/test_identity.py b/src/azure-cli-core/azure/cli/core/tests/test_identity.py index a82de7f3c9f..a73e413e315 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_identity.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_identity.py @@ -57,7 +57,7 @@ def test_login_with_service_principal_certificate_cert_err(self): current_dir = os.path.dirname(os.path.realpath(__file__)) test_cert_file = os.path.join(current_dir, 'err_sp_cert.pem') # TODO: wrap exception - with self.assertRaisesRegex(ValueError, "Unable to load certificate."): + with self.assertRaisesRegex(ValueError, "Could not deserialize key data."): identity.login_with_service_principal_certificate("00000000-0000-0000-0000-000000000000", test_cert_file) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_profile.py b/src/azure-cli-core/azure/cli/core/tests/test_profile.py index 1ba1171c2e9..39b9556814e 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_profile.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_profile.py @@ -384,7 +384,7 @@ def test_login_with_service_principal_cert_sn_issuer(self, get_token_mock): self.assertEqual(output, subs) @mock.patch('azure.cli.core._profile.SubscriptionFinder._get_subscription_client_class', autospec=True) - @mock.patch.dict('os.environ') + @mock.patch.dict('os.environ', clear=True) def test_login_with_environment_credential_service_principal(self, get_client_class_mock): os.environ['AZURE_TENANT_ID'] = self.service_principal_tenant_id os.environ['AZURE_CLIENT_ID'] = self.service_principal_id @@ -1616,7 +1616,7 @@ def test_refresh_accounts_one_user_account_one_sp_account(self, app_mock, retrie cli = DummyCli() storage_mock = {'subscriptions': None} profile = Profile(cli_ctx=cli, storage=storage_mock, use_global_creds_cache=False, async_persist=False) - sp_subscription1 = SubscriptionStub('sp-sub/3', 'foo-subname', self.state1, 'foo_tenant.onmicrosoft.com') + sp_subscription1 = SubscriptionStub('sp-sub/3', 'foo-subname', self.state1, 'footenant.onmicrosoft.com') consolidated = profile._normalize_properties(self.user1, deepcopy([self.subscription1]), False, None, None) consolidated += profile._normalize_properties('http://foo', [sp_subscription1], True) profile._set_subscriptions(consolidated) diff --git a/src/azure-cli-core/azure/cli/core/util.py b/src/azure-cli-core/azure/cli/core/util.py index 50c19b33802..9761d8a13b7 100644 --- a/src/azure-cli-core/azure/cli/core/util.py +++ b/src/azure-cli-core/azure/cli/core/util.py @@ -90,7 +90,7 @@ def handle_exception(ex): # pylint: disable=too-many-locals, too-many-statement error_msg = extract_common_error_message(ex) status_code = str(getattr(ex, 'status_code', 'Unknown Code')) AzCLIErrorType = get_error_type_by_status_code(status_code) - az_error = AzCLIErrorType(error_msg) + az_error = AzCLIErrorType(error_msg, original_error=ex) elif isinstance(ex, ValidationError): az_error = azclierror.ValidationError(error_msg) @@ -103,7 +103,7 @@ def handle_exception(ex): # pylint: disable=too-many-locals, too-many-statement if extract_common_error_message(ex): error_msg = extract_common_error_message(ex) AzCLIErrorType = get_error_type_by_azure_error(ex) - az_error = AzCLIErrorType(error_msg) + az_error = AzCLIErrorType(error_msg, original_error=ex) elif isinstance(ex, AzureException): if is_azure_connection_error(error_msg): diff --git a/src/azure-cli-core/setup.py b/src/azure-cli-core/setup.py index 223a4852f19..25868da2dc7 100644 --- a/src/azure-cli-core/setup.py +++ b/src/azure-cli-core/setup.py @@ -47,13 +47,14 @@ 'argcomplete~=1.8', 'azure-cli-telemetry==1.0.6.*', 'azure-common~=1.1', - 'azure-mgmt-core>=1.2.0,<2.0.0', + 'azure-core==1.14.0b1', + 'azure-mgmt-core==1.3.0b1', 'colorama~=0.4.1', 'cryptography>=3.2,<3.4', 'humanfriendly>=4.7,<10.0', 'jmespath', 'knack==0.8.0rc2', - 'azure-identity==1.5.0b2', + 'azure-identity==1.6.0b3', # Dependencies of the vendored subscription SDK # https://github.com/Azure/azure-sdk-for-python/blob/ab12b048ddf676fe0ccec16b2167117f0609700d/sdk/resources/azure-mgmt-resource/setup.py#L82-L86 'msrest>=0.5.0', diff --git a/src/azure-cli-testsdk/azure/cli/testsdk/base.py b/src/azure-cli-testsdk/azure/cli/testsdk/base.py index 62685d62613..babec2408cc 100644 --- a/src/azure-cli-testsdk/azure/cli/testsdk/base.py +++ b/src/azure-cli-testsdk/azure/cli/testsdk/base.py @@ -214,6 +214,9 @@ def __init__(self, method_name): self.kwargs = {} self.test_resources_count = 0 + def setUp(self): + patch_main_exception_handler(self) + def cmd(self, command, checks=None, expect_failure=False): command = self._apply_kwargs(command) return execute(self.cli_ctx, command, expect_failure=expect_failure).assert_with_checks(checks) diff --git a/src/azure-cli/azure/cli/command_modules/profile/__init__.py b/src/azure-cli/azure/cli/command_modules/profile/__init__.py index 2c4e7d95ded..1dd3f5e5e86 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/__init__.py +++ b/src/azure-cli/azure/cli/command_modules/profile/__init__.py @@ -104,7 +104,7 @@ def load_arguments(self, command): c.argument('clear_credential', clear_credential_type) with self.argument_context('account export-msal-cache') as c: - c.argument('path', help='The path to export the MSAL cache.') + c.argument('path', help='The path to export the MSAL cache.', default='~/.azure/msal.cache.snapshot.json') COMMAND_LOADER_CLS = ProfileCommandsLoader diff --git a/src/azure-cli/azure/cli/command_modules/profile/_help.py b/src/azure-cli/azure/cli/command_modules/profile/_help.py index c0acf7b7007..9a31b49cbe8 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/_help.py +++ b/src/azure-cli/azure/cli/command_modules/profile/_help.py @@ -98,12 +98,12 @@ helps['account export-msal-cache'] = """ type: command -short-summary: Export MSAL cache, by default to `~/.azure/msal.cache.snapshot.json`. +short-summary: Export MSAL cache in plain text. long-summary: > - The exported cache is unencrypted. It contains login information of all logged-in users. Make sure you protect - it safely. + The exported cache is unencrypted. + It contains login information of all logged-in users. Make sure you protect it safely. - You can mount the exported MSAL cache to a container at `~/.IdentityService/msal.cache`, so that Azure CLI + You can mount the exported MSAL cache to a container at '~/.IdentityService/msal.cache', so that Azure CLI inside the container can automatically authenticate. examples: - name: Export MSAL cache to the default path. diff --git a/src/azure-cli/azure/cli/command_modules/profile/custom.py b/src/azure-cli/azure/cli/command_modules/profile/custom.py index d7c595a85a6..020d044233a 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/custom.py +++ b/src/azure-cli/azure/cli/command_modules/profile/custom.py @@ -103,7 +103,7 @@ def set_active_subscription(cmd, subscription): profile.set_active_subscription(subscription) -def account_clear(cmd, clear_credential=False): +def account_clear(cmd, clear_credential=True): """Clear all stored subscriptions. To clear individual, use 'logout'""" if in_cloud_console(): logger.warning(_CLOUD_CONSOLE_LOGOUT_WARNING) @@ -190,7 +190,7 @@ def login(cmd, username=None, password=None, service_principal=None, tenant=None return all_subscriptions -def logout(cmd, username=None, clear_credential=False): +def logout(cmd, username=None, clear_credential=True): """Log out to remove access to Azure subscriptions""" if in_cloud_console(): logger.warning(_CLOUD_CONSOLE_LOGOUT_WARNING) diff --git a/src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_auth_e2e.py b/src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_auth_e2e.py new file mode 100644 index 00000000000..22339c36910 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_auth_e2e.py @@ -0,0 +1,62 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +from time import sleep + +import jwt +from azure.cli.core.azclierror import AuthenticationError +from azure.cli.testsdk import LiveScenarioTest +from msrestazure.azure_exceptions import CloudError + +ARM_URL = "https://eastus2euap.management.azure.com/" # ARM canary +ARM_RETRY_INTERVAL = 10 + + +class CAEScenarioTest(LiveScenarioTest): + + def test_client_capabilities(self): + self.cmd('login') + + # Verify the access token has CAE enabled + out = self.cmd('account get-access-token').get_output_in_json() + access_token = out['accessToken'] + decoded = jwt.decode(access_token, verify=False, algorithms=['RS256']) + self.assertEqual(decoded['xms_cc'], ['CP1']) # xms_cc: extension microsoft client capabilities + self.assertEqual(decoded['xms_ssm'], '1') # xms_ssm: extension microsoft smart session management + + def _test_revoke_session(self, command, expected_error, checks=None): + self.test_client_capabilities() + + # Test access token is working + self.cmd(command) + + self._revoke_sign_in_sessions() + + # CAE is currently only available in canary endpoint + # with mock.patch.object(self.cli_ctx.cloud.endpoints, "resource_manager", ARM_URL): + exit_code = 0 + with self.assertRaises(expected_error) as ex: + while exit_code == 0: + exit_code = self.cmd(command).exit_code + sleep(ARM_RETRY_INTERVAL) + if checks: + checks(ex.exception) + + def test_revoke_session_track2(self): + def check_aad_error_code(ex): + self.assertIn('AADSTS50173', str(ex)) + + self._test_revoke_session("storage account list", AuthenticationError, check_aad_error_code) + + def test_revoke_session_track1(self): + def check_arm_error(ex): + self.assertEqual(ex.status_code, 401) + self.assertIsNotNone(ex.response.headers["WWW-Authenticate"]) + + self._test_revoke_session('group list', CloudError, check_arm_error) + + def _revoke_sign_in_sessions(self): + # Manually revoke sign in sessions + self.cmd('rest -m POST -u https://graph.microsoft.com/v1.0/me/revokeSignInSessions') diff --git a/src/azure-cli/requirements.py3.Darwin.txt b/src/azure-cli/requirements.py3.Darwin.txt index 6dfc3dffa5f..2c673ff2569 100644 --- a/src/azure-cli/requirements.py3.Darwin.txt +++ b/src/azure-cli/requirements.py3.Darwin.txt @@ -9,13 +9,12 @@ azure-cli-core==2.21.0.1 azure-cli-telemetry==1.0.6 azure-cli==2.21.0.1 azure-common==1.1.22 -azure-core==1.10.0 +azure-core==1.14.0b1 azure-cosmos==3.2.0 azure-datalake-store==0.0.49 azure-functions-devops-build==0.0.22 azure-graphrbac==0.60.0 -azure-identity==1.5.0b2 -azure-keyvault==1.1.0 +azure-identity==1.6.0b3 azure-keyvault-administration==4.0.0b3 azure-keyvault==1.1.0 azure-loganalytics==0.1.0 @@ -35,7 +34,7 @@ azure-mgmt-consumption==2.0.0 azure-mgmt-containerinstance==1.5.0 azure-mgmt-containerregistry==3.0.0rc17 azure-mgmt-containerservice==11.1.0 -azure-mgmt-core==1.2.1 +azure-mgmt-core==1.3.0b1 azure-mgmt-cosmosdb==3.0.0 azure-mgmt-databoxedge==0.2.0 azure-mgmt-datalake-analytics==0.2.1 diff --git a/src/azure-cli/requirements.py3.Linux.txt b/src/azure-cli/requirements.py3.Linux.txt index 3ee05b3e83f..2c673ff2569 100644 --- a/src/azure-cli/requirements.py3.Linux.txt +++ b/src/azure-cli/requirements.py3.Linux.txt @@ -9,16 +9,14 @@ azure-cli-core==2.21.0.1 azure-cli-telemetry==1.0.6 azure-cli==2.21.0.1 azure-common==1.1.22 -azure-core==1.10.0 +azure-core==1.14.0b1 azure-cosmos==3.2.0 azure-datalake-store==0.0.49 azure-functions-devops-build==0.0.22 azure-graphrbac==0.60.0 -azure-identity==1.5.0b2 -azure-keyvault==1.1.0 +azure-identity==1.6.0b3 azure-keyvault-administration==4.0.0b3 azure-keyvault==1.1.0 -azure-keyvault==1.1.0 azure-loganalytics==0.1.0 azure-mgmt-advisor==2.0.1 azure-mgmt-apimanagement==0.2.0 @@ -36,7 +34,7 @@ azure-mgmt-consumption==2.0.0 azure-mgmt-containerinstance==1.5.0 azure-mgmt-containerregistry==3.0.0rc17 azure-mgmt-containerservice==11.1.0 -azure-mgmt-core==1.2.1 +azure-mgmt-core==1.3.0b1 azure-mgmt-cosmosdb==3.0.0 azure-mgmt-databoxedge==0.2.0 azure-mgmt-datalake-analytics==0.2.1 diff --git a/src/azure-cli/requirements.py3.windows.txt b/src/azure-cli/requirements.py3.windows.txt index 14f8dbc41c3..8f4bb47d2f5 100644 --- a/src/azure-cli/requirements.py3.windows.txt +++ b/src/azure-cli/requirements.py3.windows.txt @@ -9,16 +9,14 @@ azure-cli-core==2.21.0.1 azure-cli-telemetry==1.0.6 azure-cli==2.21.0.1 azure-common==1.1.22 -azure-core==1.10.0 +azure-core==1.14.0b1 azure-cosmos==3.2.0 azure-datalake-store==0.0.49 azure-functions-devops-build==0.0.22 azure-graphrbac==0.60.0 -azure-identity==1.5.0b2 -azure-keyvault==1.1.0 +azure-identity==1.6.0b3 azure-keyvault-administration==4.0.0b3 azure-keyvault==1.1.0 -azure-keyvault==1.1.0 azure-loganalytics==0.1.0 azure-mgmt-advisor==2.0.1 azure-mgmt-apimanagement==0.2.0 @@ -36,7 +34,7 @@ azure-mgmt-consumption==2.0.0 azure-mgmt-containerinstance==1.5.0 azure-mgmt-containerregistry==3.0.0rc17 azure-mgmt-containerservice==11.1.0 -azure-mgmt-core==1.2.1 +azure-mgmt-core==1.3.0b1 azure-mgmt-cosmosdb==3.0.0 azure-mgmt-databoxedge==0.2.0 azure-mgmt-datalake-analytics==0.2.1