Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Profile] BREAKING CHANGE: az login: Drop --username for managed identity authentication #31015

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 8 additions & 47 deletions src/azure-cli-core/azure/cli/core/_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@

_AZ_LOGIN_MESSAGE = "Please run 'az login' to setup account."

MANAGED_IDENTITY_ID_WARNING = (
"Passing the managed identity ID with --username is deprecated and will be removed in a future release. "
"Please use --client-id, --object-id or --resource-id instead."
)


def load_subscriptions(cli_ctx, all_clouds=False, refresh=False):
profile = Profile(cli_ctx=cli_ctx)
Expand Down Expand Up @@ -225,23 +220,22 @@ def login(self,
self._set_subscriptions(consolidated)
return deepcopy(consolidated)

def login_with_managed_identity(self, identity_id=None, client_id=None, object_id=None, resource_id=None,
def login_with_managed_identity(self, client_id=None, object_id=None, resource_id=None,
allow_no_subscriptions=None):
if _on_azure_arc():
return self.login_with_managed_identity_azure_arc(
identity_id=identity_id, allow_no_subscriptions=allow_no_subscriptions)
return self.login_with_managed_identity_azure_arc(allow_no_subscriptions=allow_no_subscriptions)

import jwt
from azure.mgmt.core.tools import is_valid_resource_id
from azure.cli.core.auth.adal_authentication import MSIAuthenticationWrapper
resource = self.cli_ctx.cloud.endpoints.active_directory_resource_id

id_arg_count = len([arg for arg in (client_id, object_id, resource_id, identity_id) if arg])
id_arg_count = len([arg for arg in (client_id, object_id, resource_id) if arg])
if id_arg_count > 1:
raise CLIError('Usage error: Provide only one of --client-id, --object-id, --resource-id, or --username.')
raise CLIError('Usage error: Provide only one of --client-id, --object-id, --resource-id.')

if id_arg_count == 0:
identity_type = MsiAccountTypes.system_assigned
identity_id = None
msi_creds = MSIAuthenticationWrapper(resource=resource)
elif client_id:
identity_type = MsiAccountTypes.user_assigned_client_id
Expand All @@ -255,38 +249,6 @@ def login_with_managed_identity(self, identity_id=None, client_id=None, object_i
identity_type = MsiAccountTypes.user_assigned_resource_id
identity_id = resource_id
msi_creds = MSIAuthenticationWrapper(resource=resource, msi_res_id=resource_id)
# The old way of re-using the same --username for 3 types of ID
elif identity_id:
logger.warning(MANAGED_IDENTITY_ID_WARNING)
if is_valid_resource_id(identity_id):
msi_creds = MSIAuthenticationWrapper(resource=resource, msi_res_id=identity_id)
identity_type = MsiAccountTypes.user_assigned_resource_id
else:
authenticated = False
from azure.cli.core.azclierror import AzureResponseError
try:
msi_creds = MSIAuthenticationWrapper(resource=resource, client_id=identity_id)
identity_type = MsiAccountTypes.user_assigned_client_id
authenticated = True
except AzureResponseError as ex:
if 'http error: 400, reason: Bad Request' in ex.error_msg:
logger.info('Sniff: not an MSI client id')
else:
raise

if not authenticated:
try:
identity_type = MsiAccountTypes.user_assigned_object_id
msi_creds = MSIAuthenticationWrapper(resource=resource, object_id=identity_id)
authenticated = True
except AzureResponseError as ex:
if 'http error: 400, reason: Bad Request' in ex.error_msg:
logger.info('Sniff: not an MSI object id')
else:
raise

if not authenticated:
raise CLIError('Failed to connect to MSI, check your managed service identity id.')

token_entry = msi_creds.token
token = token_entry['access_token']
Expand All @@ -310,9 +272,8 @@ def login_with_managed_identity(self, identity_id=None, client_id=None, object_i
self._set_subscriptions(consolidated)
return deepcopy(consolidated)

def login_with_managed_identity_azure_arc(self, identity_id=None, allow_no_subscriptions=None):
def login_with_managed_identity_azure_arc(self, allow_no_subscriptions=None):
import jwt
identity_type = MsiAccountTypes.system_assigned
from .auth.msal_credentials import ManagedIdentityCredential
from .auth.constants import ACCESS_TOKEN

Expand All @@ -324,8 +285,8 @@ def login_with_managed_identity_azure_arc(self, identity_id=None, allow_no_subsc

subscription_finder = SubscriptionFinder(self.cli_ctx)
subscriptions = subscription_finder.find_using_specific_tenant(tenant, cred)
base_name = ('{}-{}'.format(identity_type, identity_id) if identity_id else identity_type)
user = _USER_ASSIGNED_IDENTITY if identity_id else _SYSTEM_ASSIGNED_IDENTITY
base_name = MsiAccountTypes.system_assigned
user = _SYSTEM_ASSIGNED_IDENTITY
if not subscriptions:
if allow_no_subscriptions:
subscriptions = self._build_tenant_level_accounts([tenant])
Expand Down
29 changes: 0 additions & 29 deletions src/azure-cli-core/azure/cli/core/tests/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,19 +614,6 @@ def test_login_with_mi_user_assigned_client_id(self, create_subscription_client_
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(s['user']['assignedIdentityInfo'], 'MSIClient-{}'.format(test_client_id))

# Old way of using identity_id
subscriptions = profile.login_with_managed_identity(identity_id=test_client_id)

self.assertEqual(len(subscriptions), 1)
s = subscriptions[0]
self.assertEqual(s['name'], self.display_name1)
self.assertEqual(s['id'], self.id1.split('/')[-1])
self.assertEqual(s['tenantId'], self.test_mi_tenant)

self.assertEqual(s['user']['name'], 'userAssignedIdentity')
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(s['user']['assignedIdentityInfo'], 'MSIClient-{}'.format(test_client_id))

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
def test_login_with_mi_user_assigned_object_id(self, create_subscription_client_mock,
Expand Down Expand Up @@ -667,14 +654,6 @@ def set_token(self):
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(s['user']['assignedIdentityInfo'], 'MSIObject-{}'.format(test_object_id))

# Old way of using identity_id
subscriptions = profile.login_with_managed_identity(identity_id=test_object_id)

s = subscriptions[0]
self.assertEqual(s['user']['name'], 'userAssignedIdentity')
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(s['user']['assignedIdentityInfo'], 'MSIObject-{}'.format(test_object_id))

@mock.patch('requests.get', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
def test_login_with_mi_user_assigned_resource_id(self, create_subscription_client_mock,
Expand Down Expand Up @@ -708,14 +687,6 @@ def test_login_with_mi_user_assigned_resource_id(self, create_subscription_clien
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(subscriptions[0]['user']['assignedIdentityInfo'], 'MSIResource-{}'.format(test_res_id))

# Old way of using identity_id
subscriptions = profile.login_with_managed_identity(identity_id=test_res_id)

s = subscriptions[0]
self.assertEqual(s['user']['name'], 'userAssignedIdentity')
self.assertEqual(s['user']['type'], 'servicePrincipal')
self.assertEqual(subscriptions[0]['user']['assignedIdentityInfo'], 'MSIResource-{}'.format(test_res_id))

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.identity.Identity.get_user_credential', autospec=True)
@mock.patch('azure.cli.core.auth.identity.Identity.login_with_auth_code', autospec=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def load_arguments(self, command):

with self.argument_context('login') as c:
c.argument('username', options_list=['--username', '-u'],
help='User name, service principal client ID, or managed identity ID.')
help='User name or service principal client ID.')
c.argument('password', options_list=['--password', '-p'],
help='User password or service principal secret. Will prompt if not given.')
c.argument('tenant', options_list=['--tenant', '-t'], validator=validate_tenant,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def login(cmd, username=None, password=None, tenant=None, scopes=None, allow_no_
if in_cloud_console():
return profile.login_in_cloud_shell()
return profile.login_with_managed_identity(
identity_id=username, client_id=client_id, object_id=object_id, resource_id=resource_id,
client_id=client_id, object_id=object_id, resource_id=resource_id,
allow_no_subscriptions=allow_no_subscriptions)
if in_cloud_console(): # tell users they might not need login
logger.warning(_CLOUD_CONSOLE_LOGIN_WARNING)
Expand Down