From 6ada592ac86240c19c2cc0c402f3a242492514f4 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Thu, 30 Apr 2020 18:05:29 +0800 Subject: [PATCH 1/3] az logout --- .../azure/cli/core/_identity.py | 39 ++++++++++++++++++ src/azure-cli-core/azure/cli/core/_profile.py | 41 ++++++++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_identity.py b/src/azure-cli-core/azure/cli/core/_identity.py index 9d04a236584..6b2ef722f1c 100644 --- a/src/azure-cli-core/azure/cli/core/_identity.py +++ b/src/azure-cli-core/azure/cli/core/_identity.py @@ -216,6 +216,45 @@ def login_with_managed_identity(self, identity_id, resource): return credential, managed_identity_info + def logout_user(self, home_account_id): + from msal import PublicClientApplication + # sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py:122 + from azure.identity._internal.msal_credentials import _load_cache + cache = _load_cache() + app = PublicClientApplication(authority=self.authority, client_id=_CLIENT_ID, token_cache=cache) + + accounts = app.get_accounts() + logger.warning('Before account removal:') + logger.warning(json.dumps(accounts)) + + account = { + "environment": self.authority, + "home_account_id": home_account_id + } + app.remove_account(account) + + accounts = app.get_accounts() + logger.warning('After account removal:') + logger.warning(json.dumps(accounts)) + + def logout_all(self): + from msal import PublicClientApplication + from azure.identity._internal.msal_credentials import _load_cache + cache = _load_cache() + # TODO: Support multi-authority logout + app = PublicClientApplication(authority=self.authority, client_id=_CLIENT_ID, token_cache=cache) + + accounts = app.get_accounts() + logger.warning('Before account removal:') + logger.warning(json.dumps(accounts)) + + for account in accounts: + app.remove_account(account) + + accounts = app.get_accounts() + logger.warning('After account removal:') + logger.warning(json.dumps(accounts)) + def get_user_credential(self, home_account_id, username): auth_profile = AuthProfile(self.authority, home_account_id, self.tenant_id, username) return InteractiveBrowserCredential(profile=auth_profile, silent_auth_only=True) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 1fb7d0cc0e1..d0430486d76 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -109,6 +109,7 @@ def __init__(self, storage=None, auth_ctx_factory=None, use_global_creds_cache=T self._management_resource_uri = self.cli_ctx.cloud.endpoints.management self._ad_resource_uri = self.cli_ctx.cloud.endpoints.active_directory_resource_id + self._authority = self.cli_ctx.cloud.endpoints.active_directory.replace('https://', '') self._ad = self.cli_ctx.cloud.endpoints.active_directory self._adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) @@ -126,8 +127,8 @@ def login(self, credential=None auth_profile=None - authority = self.cli_ctx.cloud.endpoints.active_directory.replace('https://', '') - identity = Identity(authority, tenant, cred_cache=self._adal_cache) + adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) + identity = Identity(self._authority, tenant, cred_cache=self._adal_cache) if not subscription_finder: subscription_finder = SubscriptionFinder(self.cli_ctx, adal_cache=self._adal_cache) @@ -289,8 +290,8 @@ def _normalize_properties(self, user, subscriptions, is_service_principal, cert_ subscription_dict[_USER_ENTITY]['clientId'] = managed_identity_client_id # This will be deprecated and client_id will be the only persisted ID - logger.warning("assignedIdentityInfo will be deprecated in the future. Only client ID should be used.") if user_assigned_identity_id: + logger.warning("assignedIdentityInfo will be deprecated in the future. Only client ID should be used.") subscription_dict[_USER_ENTITY][_ASSIGNED_IDENTITY_INFO] = user_assigned_identity_id consolidated.append(subscription_dict) @@ -420,14 +421,42 @@ def logout(self, user_or_sp): subscriptions = self.load_cached_subscriptions(all_clouds=True) result = [x for x in subscriptions if user_or_sp.lower() == x[_USER_ENTITY][_USER_NAME].lower()] - subscriptions = [x for x in subscriptions if x not in result] + if not result: + raise CLIError("Account {} is not logged in.".format(user_or_sp)) + # Remove the account from the profile + subscriptions = [x for x in subscriptions if x not in result] self._storage[_SUBSCRIPTIONS] = subscriptions - self._creds_cache.remove_cached_creds(user_or_sp) + + # Remove the credential + account = result[0] + + #tenant=account[_TENANT_ID] + username_or_sp_id = account[_USER_ENTITY][_USER_NAME] + home_account_id = account[_USER_ENTITY].get(_USER_HOME_ACCOUNT_ID) + + identity = Identity(self._authority) + identity.logout_user(home_account_id) + + # Remove credential from the legacy cred cache + adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) + adal_cache.remove_cached_creds(user_or_sp) + + logger.warning('Account %s is logged out. To clear the cached credentials too, run' + '`az logout --username %s --clear-credential`.', user_or_sp, user_or_sp) def logout_all(self): self._storage[_SUBSCRIPTIONS] = [] - self._creds_cache.remove_all_cached_creds() + + identity = Identity(self._authority) + identity.logout_all() + + # Clear the legacy token cache + adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) + adal_cache.remove_all_cached_creds() + + logger.warning('All accounts are logged out. To clear the cached credentials too, run' + '`az account clear --clear-credentials`.') def load_cached_subscriptions(self, all_clouds=False): subscriptions = self._storage.get(_SUBSCRIPTIONS) or [] From 0d8f5d40e159055a7e7e6c853ca1e64396168845 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 11 May 2020 20:24:28 +0800 Subject: [PATCH 2/3] Support clear credential --- .../azure/cli/core/_identity.py | 44 ++++++----- src/azure-cli-core/azure/cli/core/_profile.py | 74 ++++++++++++------- .../cli/command_modules/profile/__init__.py | 13 +++- .../cli/command_modules/profile/custom.py | 8 +- 4 files changed, 90 insertions(+), 49 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_identity.py b/src/azure-cli-core/azure/cli/core/_identity.py index 6b2ef722f1c..6d4ac0be034 100644 --- a/src/azure-cli-core/azure/cli/core/_identity.py +++ b/src/azure-cli-core/azure/cli/core/_identity.py @@ -216,44 +216,52 @@ def login_with_managed_identity(self, identity_id, resource): return credential, managed_identity_info - def logout_user(self, home_account_id): + def get_user(self, user_or_sp=None): from msal import PublicClientApplication # sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py:122 from azure.identity._internal.msal_credentials import _load_cache cache = _load_cache() - app = PublicClientApplication(authority=self.authority, client_id=_CLIENT_ID, token_cache=cache) + authority = "https://{}/organizations".format(self.authority) + app = PublicClientApplication(authority=authority, client_id=_CLIENT_ID, token_cache=cache) + return app.get_accounts(user_or_sp) - accounts = app.get_accounts() - logger.warning('Before account removal:') - logger.warning(json.dumps(accounts)) + def logout_user(self, user_or_sp): + from msal import PublicClientApplication + # sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py:122 + from azure.identity._internal.msal_credentials import _load_cache + cache = _load_cache() + authority = "https://{}/organizations".format(self.authority) + app = PublicClientApplication(authority=authority, client_id=_CLIENT_ID, token_cache=cache) - account = { - "environment": self.authority, - "home_account_id": home_account_id - } - app.remove_account(account) + accounts = app.get_accounts(user_or_sp) + logger.info('Before account removal:') + logger.info(json.dumps(accounts)) - accounts = app.get_accounts() - logger.warning('After account removal:') - logger.warning(json.dumps(accounts)) + for account in accounts: + app.remove_account(account) + + accounts = app.get_accounts(user_or_sp) + logger.info('After account removal:') + logger.info(json.dumps(accounts)) def logout_all(self): from msal import PublicClientApplication from azure.identity._internal.msal_credentials import _load_cache cache = _load_cache() # TODO: Support multi-authority logout - app = PublicClientApplication(authority=self.authority, client_id=_CLIENT_ID, token_cache=cache) + authority = "https://{}/organizations".format(self.authority) + app = PublicClientApplication(authority=authority, client_id=_CLIENT_ID, token_cache=cache) accounts = app.get_accounts() - logger.warning('Before account removal:') - logger.warning(json.dumps(accounts)) + logger.info('Before account removal:') + logger.info(json.dumps(accounts)) for account in accounts: app.remove_account(account) accounts = app.get_accounts() - logger.warning('After account removal:') - logger.warning(json.dumps(accounts)) + logger.info('After account removal:') + logger.info(json.dumps(accounts)) def get_user_credential(self, home_account_id, username): auth_profile = AuthProfile(self.authority, home_account_id, self.tenant_id, username) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index d0430486d76..3b416302918 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -417,46 +417,68 @@ def set_active_subscription(self, subscription): # take id or name set_cloud_subscription(self.cli_ctx, active_cloud.name, result[0][_SUBSCRIPTION_ID]) self._storage[_SUBSCRIPTIONS] = subscriptions - def logout(self, user_or_sp): + def logout(self, user_or_sp, clear_credential): subscriptions = self.load_cached_subscriptions(all_clouds=True) result = [x for x in subscriptions if user_or_sp.lower() == x[_USER_ENTITY][_USER_NAME].lower()] - if not result: - raise CLIError("Account {} is not logged in.".format(user_or_sp)) - # Remove the account from the profile - subscriptions = [x for x in subscriptions if x not in result] - self._storage[_SUBSCRIPTIONS] = subscriptions + if result: + # Remove the account from the profile + subscriptions = [x for x in subscriptions if x not in result] + self._storage[_SUBSCRIPTIONS] = subscriptions - # Remove the credential - account = result[0] + # Always remove credential from the legacy cred cache, regardless of MSAL cache + adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) + adal_cache.remove_cached_creds(user_or_sp) - #tenant=account[_TENANT_ID] - username_or_sp_id = account[_USER_ENTITY][_USER_NAME] - home_account_id = account[_USER_ENTITY].get(_USER_HOME_ACCOUNT_ID) + logger.warning('Account %s was 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) + # Deal with MSAL cache identity = Identity(self._authority) - identity.logout_user(home_account_id) - - # Remove credential from the legacy cred cache - adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) - adal_cache.remove_cached_creds(user_or_sp) - - logger.warning('Account %s is logged out. To clear the cached credentials too, run' - '`az logout --username %s --clear-credential`.', user_or_sp, user_or_sp) + accounts = identity.get_user(user_or_sp) + if accounts: + logger.info("The credential of %s were found from MSAL encrypted cache: %s", user_or_sp, accounts) + if clear_credential: + identity.logout_user(user_or_sp) + 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. ' + 'To clear the credential, run `az logout --username %s --clear-credential`.', + user_or_sp, user_or_sp) + else: + logger.warning("The credential of %s was not found from MSAL encrypted cache.", user_or_sp) - def logout_all(self): + def logout_all(self, clear_credential): self._storage[_SUBSCRIPTIONS] = [] - identity = Identity(self._authority) - identity.logout_all() - - # Clear the legacy token cache + # Always remove credentials from the legacy cred cache, regardless of MSAL cache adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) adal_cache.remove_all_cached_creds() + logger.warning('All accounts were logged out.') - logger.warning('All accounts are logged out. To clear the cached credentials too, run' - '`az account clear --clear-credentials`.') + # Deal with MSAL cache + identity = Identity(self._authority) + accounts = identity.get_user() + if accounts: + logger.info("These credentials were found from MSAL encrypted cache: %s", accounts) + if clear_credential: + identity.logout_all() + logger.warning('All credentials store in MSAL encrypted cache were cleared.') + else: + logger.warning('These credentials are still stored in MSAL encrypted cached:') + for account in identity.get_user(): + 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.') + else: + logger.warning('No credential was not found from MSAL encrypted cache.') def load_cached_subscriptions(self, all_clouds=False): subscriptions = self._storage.get(_SUBSCRIPTIONS) or [] 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 203d504be1d..5d52720fca1 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/__init__.py +++ b/src/azure-cli/azure/cli/command_modules/profile/__init__.py @@ -43,6 +43,14 @@ def load_command_table(self, args): # pylint: disable=line-too-long def load_arguments(self, command): from azure.cli.core.api import get_subscription_id_list + from azure.cli.core.commands.parameters import get_three_state_flag + from knack.arguments import CLIArgumentType + + clear_credential_type = CLIArgumentType(options_list=['--clear-credential', '-c'], + arg_type=get_three_state_flag(), + help="Clear the credential stored in MSAL encrypted cache. " + "The user will also be logged out from other SDK tools " + "which uses Azure CLI's credential via Single Sign-On.") with self.argument_context('login') as c: c.argument('password', options_list=['--password', '-p'], help="Credentials like user password, or for a service principal, provide client secret or a pem file with key and public certificate. Will prompt if not given.") @@ -58,7 +66,8 @@ def load_arguments(self, command): c.argument('use_cert_sn_issuer', action='store_true', help='used with a service principal configured with Subject Name and Issuer Authentication in order to support automatic certificate rolls') with self.argument_context('logout') as c: - c.argument('username', help='account user, if missing, logout the current active account') + c.argument('username', options_list=['--username', '-u'], help='account user, if missing, logout the current active account') + c.argument('clear_credential', clear_credential_type) c.ignore('_subscription') # hide the global subscription parameter with self.argument_context('account') as c: @@ -77,5 +86,7 @@ def load_arguments(self, command): c.argument('resource_type', get_enum_type(cloud_resource_types), options_list=['--resource-type'], arg_group='', help='Type of well-known resource.') c.argument('tenant', options_list=['--tenant', '-t'], is_preview=True, help='Tenant ID for which the token is acquired. Only available for user and service principal account, not for MSI or Cloud Shell account') + with self.argument_context('account logout') as c: + c.argument('clear_credential', clear_credential_type) COMMAND_LOADER_CLS = ProfileCommandsLoader 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 6731aba86eb..42f992ed81c 100644 --- a/src/azure-cli/azure/cli/command_modules/profile/custom.py +++ b/src/azure-cli/azure/cli/command_modules/profile/custom.py @@ -93,12 +93,12 @@ def set_active_subscription(cmd, subscription): profile.set_active_subscription(subscription) -def account_clear(cmd): +def account_clear(cmd, clear_credential=False): """Clear all stored subscriptions. To clear individual, use 'logout'""" if in_cloud_console(): logger.warning(_CLOUD_CONSOLE_LOGOUT_WARNING) profile = Profile(cli_ctx=cmd.cli_ctx) - profile.logout_all() + profile.logout_all(clear_credential) # pylint: disable=inconsistent-return-statements @@ -175,7 +175,7 @@ def login(cmd, username=None, password=None, service_principal=None, tenant=None return all_subscriptions -def logout(cmd, username=None): +def logout(cmd, username=None, clear_credential=False): """Log out to remove access to Azure subscriptions""" if in_cloud_console(): logger.warning(_CLOUD_CONSOLE_LOGOUT_WARNING) @@ -183,7 +183,7 @@ def logout(cmd, username=None): profile = Profile(cli_ctx=cmd.cli_ctx) if not username: username = profile.get_current_account_user() - profile.logout(username) + profile.logout(username, clear_credential) def list_locations(cmd): From ecbfaf4a105cdb73b95f4b92ae8050a37496a71b Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Tue, 12 May 2020 21:44:12 +0800 Subject: [PATCH 3/3] Remove extra code --- src/azure-cli-core/azure/cli/core/_profile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 3b416302918..31abf3e4c57 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -127,7 +127,6 @@ def login(self, credential=None auth_profile=None - adal_cache = ADALCredentialCache(cli_ctx=self.cli_ctx) identity = Identity(self._authority, tenant, cred_cache=self._adal_cache) if not subscription_finder: