Skip to content
Merged
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
27 changes: 13 additions & 14 deletions src/azure-cli-core/azure/cli/core/_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,9 @@ def login(self,
self._set_subscriptions(consolidated)
return deepcopy(consolidated)

def login_with_managed_identity(self, client_id=None, object_id=None, resource_id=None,
allow_no_subscriptions=None):
if _use_msal_managed_identity(self.cli_ctx):
return self.login_with_managed_identity_msal(
client_id=client_id, object_id=object_id, resource_id=resource_id,
allow_no_subscriptions=allow_no_subscriptions)

def login_with_managed_identity_msrestazure(self, client_id=None, object_id=None, resource_id=None,
allow_no_subscriptions=None):
# Old way of using msrestazure for managed identity
import jwt
from azure.cli.core.auth.adal_authentication import MSIAuthenticationWrapper
resource = self.cli_ctx.cloud.endpoints.active_directory_resource_id
Expand Down Expand Up @@ -274,8 +270,13 @@ def login_with_managed_identity(self, client_id=None, object_id=None, resource_i
self._set_subscriptions(consolidated)
return deepcopy(consolidated)

def login_with_managed_identity_msal(self, client_id=None, object_id=None, resource_id=None,
allow_no_subscriptions=None):
def login_with_managed_identity(self, client_id=None, object_id=None, resource_id=None,
allow_no_subscriptions=None):
if not _use_msal_managed_identity(self.cli_ctx):
return self.login_with_managed_identity_msrestazure(
client_id=client_id, object_id=object_id, resource_id=resource_id,
allow_no_subscriptions=allow_no_subscriptions)

import jwt
from .auth.constants import ACCESS_TOKEN

Expand Down Expand Up @@ -986,10 +987,8 @@ def _create_identity_instance(cli_ctx, authority, tenant_id=None, client_id=None


def _use_msal_managed_identity(cli_ctx):
# This indicates an Azure Arc-enabled server
from msal.managed_identity import get_managed_identity_source, AZURE_ARC
from azure.cli.core.telemetry import set_use_msal_managed_identity
# PREVIEW: Use core.use_msal_managed_identity=true to enable managed identity authentication with MSAL
use_msal_managed_identity = cli_ctx.config.getboolean('core', 'use_msal_managed_identity', fallback=False)
# Use core.use_msal_managed_identity=false to use the old msrestazure implementation
use_msal_managed_identity = cli_ctx.config.getboolean('core', 'use_msal_managed_identity', fallback=True)
set_use_msal_managed_identity(use_msal_managed_identity)
return use_msal_managed_identity or get_managed_identity_source() == AZURE_ARC
return use_msal_managed_identity
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer necessary to detect Azure Arc, as MSAL managed identity is now the default.

Copy link
Contributor

@bebound bebound Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused.
Previously, this function always returns True in AZURE_ARC. Now it can return False with use_msal_managed_identity=false. Is this expected?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Hang. AZURE_ARC could fail if customer accidently set use_msal_managed_identity=false because it's not supported with msrestazure

Copy link
Member Author

@jiasli jiasli Apr 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detecting Azure Arc will slow down CLI in other environments.

use_msal_managed_identity=false will be a hidden interface to revert to the old msrestazure implementation only when there are issues reported. Azure Arc users will not / need not to set it at all.

23 changes: 10 additions & 13 deletions src/azure-cli-core/azure/cli/core/tests/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ def test_login_in_cloud_shell(self, cloud_shell_credential_mock, create_subscrip

@mock.patch('requests.get', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_login_with_mi_system_assigned(self, create_subscription_client_mock, mock_get):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand Down Expand Up @@ -569,6 +570,7 @@ def test_login_with_mi_system_assigned(self, create_subscription_client_mock, mo

@mock.patch('requests.get', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_login_with_mi_no_subscriptions(self, create_subscription_client_mock, mock_get):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = []
Expand Down Expand Up @@ -604,6 +606,7 @@ def test_login_with_mi_no_subscriptions(self, create_subscription_client_mock, m

@mock.patch('requests.get', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_login_with_mi_user_assigned_client_id(self, create_subscription_client_mock, mock_get):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand Down Expand Up @@ -638,6 +641,7 @@ def test_login_with_mi_user_assigned_client_id(self, create_subscription_client_

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_login_with_mi_user_assigned_object_id(self, create_subscription_client_mock,
mock_msi_auth):
mock_subscription_client = mock.MagicMock()
Expand Down Expand Up @@ -678,6 +682,7 @@ def set_token(self):

@mock.patch('requests.get', autospec=True)
@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_login_with_mi_user_assigned_resource_id(self, create_subscription_client_mock,
mock_get):

Expand Down Expand Up @@ -711,7 +716,6 @@ def test_login_with_mi_user_assigned_resource_id(self, create_subscription_clien

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_login_with_mi_system_assigned_msal(self, create_subscription_client_mock):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand Down Expand Up @@ -739,7 +743,6 @@ def test_login_with_mi_system_assigned_msal(self, create_subscription_client_moc

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_login_with_mi_system_assigned_no_subscriptions_msal(self, create_subscription_client_mock):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = []
Expand Down Expand Up @@ -769,7 +772,6 @@ def test_login_with_mi_system_assigned_no_subscriptions_msal(self, create_subscr

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_login_with_mi_user_assigned_client_id_msal(self, create_subscription_client_mock):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand Down Expand Up @@ -798,7 +800,6 @@ def test_login_with_mi_user_assigned_client_id_msal(self, create_subscription_cl

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_login_with_mi_user_assigned_object_id_msal(self, create_subscription_client_mock):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand All @@ -822,7 +823,6 @@ def test_login_with_mi_user_assigned_object_id_msal(self, create_subscription_cl

@mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_login_with_mi_user_assigned_resource_id_msal(self, create_subscription_client_mock):
mock_subscription_client = mock.MagicMock()
mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)]
Expand Down Expand Up @@ -1189,6 +1189,7 @@ def test_get_login_credentials_aux_tenants(self, get_user_credential_mock):
aux_tenants=[test_tenant_id2])

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_get_login_credentials_mi_system_assigned(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties('systemAssignedIdentity',
Expand All @@ -1208,6 +1209,7 @@ def test_get_login_credentials_mi_system_assigned(self):
self.assertTrue(cred.token_read_count)

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_get_login_credentials_mi_user_assigned_with_client_id(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
test_client_id = '12345678-38d6-4fb2-bad9-b7b93a3e8888'
Expand All @@ -1229,6 +1231,7 @@ def test_get_login_credentials_mi_user_assigned_with_client_id(self):
self.assertTrue(cred.client_id, test_client_id)

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_get_login_credentials_mi_user_assigned_with_object_id(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
test_object_id = '12345678-38d6-4fb2-bad9-b7b93a3e9999'
Expand All @@ -1250,6 +1253,7 @@ def test_get_login_credentials_mi_user_assigned_with_object_id(self):
self.assertTrue(cred.object_id, test_object_id)

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_get_login_credentials_mi_user_assigned_with_res_id(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
test_res_id = ('/subscriptions/{}/resourceGroups/r1/providers/Microsoft.ManagedIdentity/'
Expand All @@ -1272,7 +1276,6 @@ def test_get_login_credentials_mi_user_assigned_with_res_id(self):
self.assertTrue(cred.msi_res_id, test_res_id)

@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_login_credentials_mi_system_assigned_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties('systemAssignedIdentity',
Expand All @@ -1289,7 +1292,6 @@ def test_get_login_credentials_mi_system_assigned_msal(self):
assert cred._credential.resource_id is None

@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_login_credentials_mi_user_assigned_client_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand All @@ -1308,7 +1310,6 @@ def test_get_login_credentials_mi_user_assigned_client_id_msal(self):
assert cred._credential.resource_id is None

@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_login_credentials_mi_user_assigned_object_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand All @@ -1327,7 +1328,6 @@ def test_get_login_credentials_mi_user_assigned_object_id_msal(self):
assert cred._credential.resource_id is None

@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_login_credentials_mi_user_assigned_resource_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand Down Expand Up @@ -1434,6 +1434,7 @@ def test_get_raw_token_for_sp(self, get_service_principal_credential_mock):
self.assertEqual(tenant, self.tenant_id)

@mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', autospec=True)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'false'})
def test_get_raw_token_mi_system_assigned(self, mock_msi_auth):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties('systemAssignedIdentity',
Expand Down Expand Up @@ -1473,7 +1474,6 @@ def mi_auth_factory(*args, **kwargs):

@mock.patch('azure.cli.core.auth.util._now_timestamp', new=_now_timestamp_mock)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_raw_token_mi_system_assigned_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties('systemAssignedIdentity',
Expand Down Expand Up @@ -1508,7 +1508,6 @@ def test_get_raw_token_mi_system_assigned_msal(self):

@mock.patch('azure.cli.core.auth.util._now_timestamp', new=_now_timestamp_mock)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_raw_token_mi_user_assigned_client_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand Down Expand Up @@ -1540,7 +1539,6 @@ def test_get_raw_token_mi_user_assigned_client_id_msal(self):

@mock.patch('azure.cli.core.auth.util._now_timestamp', new=_now_timestamp_mock)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_raw_token_mi_user_assigned_object_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand Down Expand Up @@ -1572,7 +1570,6 @@ def test_get_raw_token_mi_user_assigned_object_id_msal(self):

@mock.patch('azure.cli.core.auth.util._now_timestamp', new=_now_timestamp_mock)
@mock.patch('azure.cli.core.auth.msal_credentials.ManagedIdentityCredential', ManagedIdentityCredentialStub)
@mock.patch.dict('os.environ', {'AZURE_CORE_USE_MSAL_MANAGED_IDENTITY': 'true'})
def test_get_raw_token_mi_user_assigned_resource_id_msal(self):
profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None})
consolidated = profile._normalize_properties(
Expand Down