Skip to content

Commit

Permalink
Minor fixes (Azure#9)
Browse files Browse the repository at this point in the history
* Only use credential cache if it is available
* Pass tenant id to Identity properly
* Use the client id and scopes properly in client factory
* Accept single string scope (not a bug but good to have)

Co-authored-by: Tibor Takacs <titakac@microsoft.com>
  • Loading branch information
tibortakacs and Tibor Takacs authored Aug 7, 2020
1 parent 095964a commit 74090f9
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
17 changes: 12 additions & 5 deletions src/azure-cli-core/azure/cli/core/_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ def __init__(self, authority=None, tenant_id=None, client_id=None, scopes=None,
self._cred_cache = kwargs.pop('cred_cache', None)
self.allow_unencrypted = kwargs.pop('allow_unencrypted', True)
self.scopes = scopes
if self.scopes and not isinstance(self.scopes, (list, tuple)):
self.scopes = (self.scopes,)
self._msal_app_instance = None
# Store for Service principal credential persistence
self._msal_secret_store = MsalSecretStore(fallback_to_plaintext=self.allow_unencrypted)
Expand Down Expand Up @@ -107,7 +109,8 @@ def login_with_interactive_browser(self):
**self.credential_kwargs)
auth_record = credential.authenticate(scopes=self.scopes)
# todo: remove after ADAL token deprecation
self._cred_cache.add_credential(credential)
if self._cred_cache:
self._cred_cache.add_credential(credential)
return credential, auth_record

def login_with_device_code(self):
Expand All @@ -127,7 +130,8 @@ def prompt_callback(verification_uri, user_code, _):

auth_record = credential.authenticate(scopes=self.scopes)
# todo: remove after ADAL token deprecation
self._cred_cache.add_credential(credential)
if self._cred_cache:
self._cred_cache.add_credential(credential)
return credential, auth_record
except ValueError as ex:
logger.debug('Device code authentication failed: %s', str(ex))
Expand All @@ -149,7 +153,8 @@ def login_with_username_password(self, username, password):
auth_record = credential.authenticate(scopes=self.scopes)

# todo: remove after ADAL token deprecation
self._cred_cache.add_credential(credential)
if self._cred_cache:
self._cred_cache.add_credential(credential)
return credential, auth_record

def login_with_service_principal_secret(self, client_id, client_secret):
Expand All @@ -160,7 +165,8 @@ def login_with_service_principal_secret(self, client_id, client_secret):
entry = sp_auth.get_entry_to_persist()
self._msal_secret_store.save_service_principal_cred(entry)
# backward compatible with ADAL, to be deprecated
self._cred_cache.save_service_principal_cred(entry)
if self._cred_cache:
self._cred_cache.save_service_principal_cred(entry)

credential = ClientSecretCredential(self.tenant_id, client_id, client_secret, authority=self.authority)
return credential
Expand All @@ -173,7 +179,8 @@ def login_with_service_principal_certificate(self, client_id, certificate_path):
entry = sp_auth.get_entry_to_persist()
self._msal_secret_store.save_service_principal_cred(entry)
# backward compatible with ADAL, to be deprecated
self._cred_cache.save_service_principal_cred(entry)
if self._cred_cache:
self._cred_cache.save_service_principal_cred(entry)

# TODO: support use_cert_sn_issuer in CertificateCredential
credential = CertificateCredential(self.tenant_id, client_id, certificate_path, authority=self.authority)
Expand Down
7 changes: 5 additions & 2 deletions src/azure-cli-core/azure/cli/core/_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ def __init__(self, cli_ctx=None, storage=None, auth_ctx_factory=None, use_global
self._adal_cache = AdalCredentialCache(cli_ctx=self.cli_ctx)
self._client_id = client_id
self._msal_scopes = scopes or adal_resource_to_msal_scopes(self._ad_resource_uri)
if self._msal_scopes and not isinstance(self._msal_scopes, (list, tuple)):
self._msal_scopes = (self._msal_scopes,)

# pylint: disable=too-many-branches,too-many-statements
def login(self,
Expand All @@ -144,7 +146,7 @@ def login(self,

credential = None
auth_record = None
identity = Identity(authority=self._authority, tenant=tenant, client_id=self._client_id,
identity = Identity(authority=self._authority, tenant_id=tenant, client_id=self._client_id,
scopes=self._msal_scopes,
allow_unencrypted=self.cli_ctx.config
.getboolean('core', 'allow_fallback_to_plaintext', fallback=True),
Expand Down Expand Up @@ -214,7 +216,8 @@ def login(self,

self._set_subscriptions(consolidated)
# todo: remove after ADAL token deprecation
self._adal_cache.persist_cached_creds()
if self._adal_cache:
self._adal_cache.persist_cached_creds()
# use deepcopy as we don't want to persist these changes to file.
return deepcopy(consolidated)

Expand Down
5 changes: 4 additions & 1 deletion src/azure-cli-core/azure/cli/core/commands/client_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ def _get_mgmt_service_client(cli_ctx,
from azure.cli.core._profile import Profile
logger.debug('Getting management service client client_type=%s', client_type.__name__)
resource = resource or cli_ctx.cloud.endpoints.active_directory_resource_id
profile = Profile(cli_ctx=cli_ctx)

client_id = kwargs.pop('client_id', None)

profile = Profile(cli_ctx=cli_ctx, scopes=scopes, client_id=client_id)
cred, subscription_id, _ = profile.get_login_credentials(subscription_id=subscription_id, resource=resource,
aux_subscriptions=aux_subscriptions,
aux_tenants=aux_tenants)
Expand Down

0 comments on commit 74090f9

Please sign in to comment.