From 14084adeae203a3f3342cab81cd3c75ec16e2ac4 Mon Sep 17 00:00:00 2001 From: Jiashuo Li <4003950+jiasli@users.noreply.github.com> Date: Wed, 20 Nov 2024 20:06:59 +0800 Subject: [PATCH] [Profile] Drop support for old-style managed identity account (#30321) --- src/azure-cli-core/azure/cli/core/_profile.py | 25 ++-- .../azure/cli/core/tests/test_profile.py | 130 ++++++++---------- 2 files changed, 74 insertions(+), 81 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 3e178c10aaa..5fbab701539 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -377,7 +377,7 @@ def get_login_credentials(self, resource=None, subscription_id=None, aux_subscri account = self.get_subscription(subscription_id) - managed_identity_type, managed_identity_id = Profile._try_parse_msi_account_name(account) + managed_identity_type, managed_identity_id = Profile._parse_managed_identity_account(account) if in_cloud_console() and account[_USER_ENTITY].get(_CLOUD_SHELL_ID): # Cloud Shell @@ -436,7 +436,7 @@ def get_raw_token(self, resource=None, scopes=None, subscription=None, tenant=No account = self.get_subscription(subscription) - managed_identity_type, managed_identity_id = Profile._try_parse_msi_account_name(account) + managed_identity_type, managed_identity_id = Profile._parse_managed_identity_account(account) if in_cloud_console() and account[_USER_ENTITY].get(_CLOUD_SHELL_ID): # Cloud Shell @@ -642,15 +642,18 @@ def get_subscription_id(self, subscription=None): # take id or name return self.get_subscription(subscription)[_SUBSCRIPTION_ID] @staticmethod - def _try_parse_msi_account_name(account): - msi_info, user = account[_USER_ENTITY].get(_ASSIGNED_IDENTITY_INFO), account[_USER_ENTITY].get(_USER_NAME) - - if user in [_SYSTEM_ASSIGNED_IDENTITY, _USER_ASSIGNED_IDENTITY]: - if not msi_info: - msi_info = account[_SUBSCRIPTION_NAME] # fall back to old persisting way - parts = msi_info.split('-', 1) - if parts[0] in MsiAccountTypes.valid_msi_account_types(): - return parts[0], (None if len(parts) <= 1 else parts[1]) + def _parse_managed_identity_account(account): + user_name = account[_USER_ENTITY][_USER_NAME] + if user_name == _SYSTEM_ASSIGNED_IDENTITY: + # The account contains: + # "assignedIdentityInfo": "MSI", + # "name": "systemAssignedIdentity", + return MsiAccountTypes.system_assigned, None + if user_name == _USER_ASSIGNED_IDENTITY: + # The account contains: + # "assignedIdentityInfo": "MSIClient-xxx"/"MSIObject-xxx"/"MSIResource-xxx", + # "name": "userAssignedIdentity", + return tuple(account[_USER_ENTITY][_ASSIGNED_IDENTITY_INFO].split('-', maxsplit=1)) return None, None def _create_credential(self, account, tenant_id=None, client_id=None): 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 508e55d20d0..d9825916d71 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 @@ -63,7 +63,7 @@ class MSRestAzureAuthStub: def __init__(self, *args, **kwargs): self._token = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token, + 'access_token': TestProfile.test_mi_access_token, 'expires_on': MOCK_EXPIRES_ON_STR } self.set_token_invoked_count = 0 @@ -254,8 +254,15 @@ def setUpClass(cls): }) # A random GUID generated by uuid.uuid4() - cls.test_msi_tenant = 'b6f04d88-9bff-45da-a9b4-a0b6d3cb1b2a' - cls.test_msi_access_token = _build_test_jwt({'tid': cls.test_msi_tenant}) + cls.test_mi_tenant = 'b6f04d88-9bff-45da-a9b4-a0b6d3cb1b2a' + cls.test_mi_access_token = _build_test_jwt({'tid': cls.test_mi_tenant}) + cls.test_mi_subscription_id = '796071f3-b997-48d8-87f8-dbc5869bd9c5' + cls.test_mi_subscription_resource_id = '/subscriptions/{}'.format(cls.test_mi_subscription_id) + cls.test_mi_subscription_name = 'MI-DEV-INC' + cls.test_mi_subscription = SubscriptionStub(cls.test_mi_subscription_resource_id, + cls.test_mi_subscription_name, + cls.state1, + cls.test_mi_tenant) cls.msal_accounts = [ { @@ -491,7 +498,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) - def test_find_subscriptions_in_vm_with_msi_system_assigned(self, create_subscription_client_mock, mock_get): + def test_find_subscriptions_in_vm_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)] create_subscription_client_mock.return_value = mock_subscription_client @@ -502,7 +509,7 @@ def test_find_subscriptions_in_vm_with_msi_system_assigned(self, create_subscrip test_token_entry = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token + 'access_token': TestProfile.test_mi_access_token } encoded_test_token = json.dumps(test_token_entry).encode() good_response = mock.MagicMock() @@ -520,11 +527,11 @@ def test_find_subscriptions_in_vm_with_msi_system_assigned(self, create_subscrip self.assertEqual(s['user']['assignedIdentityInfo'], 'MSI') self.assertEqual(s['name'], self.display_name1) self.assertEqual(s['id'], self.id1.split('/')[-1]) - self.assertEqual(s['tenantId'], self.test_msi_tenant) + self.assertEqual(s['tenantId'], self.test_mi_tenant) @mock.patch('requests.get', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True) - def test_find_subscriptions_in_vm_with_msi_no_subscriptions(self, create_subscription_client_mock, mock_get): + def test_find_subscriptions_in_vm_with_mi_no_subscriptions(self, create_subscription_client_mock, mock_get): mock_subscription_client = mock.MagicMock() mock_subscription_client.subscriptions.list.return_value = [] create_subscription_client_mock.return_value = mock_subscription_client @@ -535,7 +542,7 @@ def test_find_subscriptions_in_vm_with_msi_no_subscriptions(self, create_subscri test_token_entry = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token + 'access_token': TestProfile.test_mi_access_token } encoded_test_token = json.dumps(test_token_entry).encode() good_response = mock.MagicMock() @@ -550,8 +557,8 @@ def test_find_subscriptions_in_vm_with_msi_no_subscriptions(self, create_subscri s = subscriptions[0] self.assertEqual(s['name'], 'N/A(tenant level account)') - self.assertEqual(s['id'], self.test_msi_tenant) - self.assertEqual(s['tenantId'], self.test_msi_tenant) + self.assertEqual(s['id'], self.test_mi_tenant) + self.assertEqual(s['tenantId'], self.test_mi_tenant) self.assertEqual(s['user']['name'], 'systemAssignedIdentity') self.assertEqual(s['user']['type'], 'servicePrincipal') @@ -559,7 +566,8 @@ def test_find_subscriptions_in_vm_with_msi_no_subscriptions(self, create_subscri @mock.patch('requests.get', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True) - def test_find_subscriptions_in_vm_with_msi_user_assigned_with_client_id(self, create_subscription_client_mock, mock_get): + def test_find_subscriptions_in_vm_with_mi_user_assigned_with_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)] create_subscription_client_mock.return_value = mock_subscription_client @@ -570,7 +578,7 @@ def test_find_subscriptions_in_vm_with_msi_user_assigned_with_client_id(self, cr test_token_entry = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token + 'access_token': TestProfile.test_mi_access_token } test_client_id = '54826b22-38d6-4fb2-bad9-b7b93a3e9999' encoded_test_token = json.dumps(test_token_entry).encode() @@ -585,7 +593,7 @@ def test_find_subscriptions_in_vm_with_msi_user_assigned_with_client_id(self, cr s = subscriptions[0] self.assertEqual(s['name'], self.display_name1) self.assertEqual(s['id'], self.id1.split('/')[-1]) - self.assertEqual(s['tenantId'], self.test_msi_tenant) + self.assertEqual(s['tenantId'], self.test_mi_tenant) self.assertEqual(s['user']['name'], 'userAssignedIdentity') self.assertEqual(s['user']['type'], 'servicePrincipal') @@ -593,8 +601,8 @@ def test_find_subscriptions_in_vm_with_msi_user_assigned_with_client_id(self, cr @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_find_subscriptions_in_vm_with_msi_user_assigned_with_object_id(self, create_subscription_client_mock, - mock_msi_auth): + def test_find_subscriptions_in_vm_with_mi_user_assigned_with_object_id(self, create_subscription_client_mock, + mock_msi_auth): mock_subscription_client = mock.MagicMock() mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)] create_subscription_client_mock.return_value = mock_subscription_client @@ -613,7 +621,7 @@ def set_token(self): if self.object_id: self.token = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token + 'access_token': TestProfile.test_mi_access_token } else: raise AzureResponseError('Failed to connect to MSI. Please make sure MSI is configured correctly.\n' @@ -633,8 +641,8 @@ def set_token(self): @mock.patch('requests.get', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder._create_subscription_client', autospec=True) - def test_find_subscriptions_in_vm_with_msi_user_assigned_with_res_id(self, create_subscription_client_mock, - mock_get): + def test_find_subscriptions_in_vm_with_mi_user_assigned_with_res_id(self, create_subscription_client_mock, + mock_get): mock_subscription_client = mock.MagicMock() mock_subscription_client.subscriptions.list.return_value = [deepcopy(self.subscription1_raw)] @@ -646,7 +654,7 @@ def test_find_subscriptions_in_vm_with_msi_user_assigned_with_res_id(self, creat test_token_entry = { 'token_type': 'Bearer', - 'access_token': TestProfile.test_msi_access_token + 'access_token': TestProfile.test_mi_access_token } test_res_id = ('/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourcegroups/g1/' 'providers/Microsoft.ManagedIdentity/userAssignedIdentities/id1') @@ -1009,22 +1017,17 @@ 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) - def test_get_login_credentials_msi_system_assigned(self): - - # setup an existing msi subscription + def test_get_login_credentials_mi_system_assigned(self): profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None}) - test_subscription_id = '12345678-1bf0-4dda-aec3-cb9272f09590' - test_tenant_id = '12345678-38d6-4fb2-bad9-b7b93a3e1234' - test_user = 'systemAssignedIdentity' - msi_subscription = SubscriptionStub('/subscriptions/' + test_subscription_id, 'MSI', self.state1, test_tenant_id) - consolidated = profile._normalize_properties(test_user, - [msi_subscription], - True) + consolidated = profile._normalize_properties('systemAssignedIdentity', + [deepcopy(self.test_mi_subscription)], + True, + user_assigned_identity_id="MSI") profile._set_subscriptions(consolidated) cred, subscription_id, _ = profile.get_login_credentials() - self.assertEqual(subscription_id, test_subscription_id) + self.assertEqual(subscription_id, self.test_mi_subscription_id) # sniff test the msi_auth object cred.set_token() @@ -1033,20 +1036,18 @@ def test_get_login_credentials_msi_system_assigned(self): self.assertTrue(cred.token_read_count) @mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub) - def test_get_login_credentials_msi_user_assigned_with_client_id(self): - # setup an existing msi subscription + def test_get_login_credentials_mi_user_assigned_with_client_id(self): profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None}) - test_subscription_id = '12345678-1bf0-4dda-aec3-cb9272f09590' - test_tenant_id = '12345678-38d6-4fb2-bad9-b7b93a3e1234' - test_user = 'userAssignedIdentity' test_client_id = '12345678-38d6-4fb2-bad9-b7b93a3e8888' - msi_subscription = SubscriptionStub('/subscriptions/' + test_subscription_id, 'MSIClient-{}'.format(test_client_id), self.state1, test_tenant_id) - consolidated = profile._normalize_properties(test_user, [msi_subscription], True) + consolidated = profile._normalize_properties('userAssignedIdentity', + [deepcopy(self.test_mi_subscription)], + True, + user_assigned_identity_id='MSIClient-{}'.format(test_client_id)) profile._set_subscriptions(consolidated, secondary_key_name='name') cred, subscription_id, _ = profile.get_login_credentials() - self.assertEqual(subscription_id, test_subscription_id) + self.assertEqual(subscription_id, self.test_mi_subscription_id) # sniff test the msi_auth object cred.set_token() @@ -1056,21 +1057,18 @@ def test_get_login_credentials_msi_user_assigned_with_client_id(self): self.assertTrue(cred.client_id, test_client_id) @mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub) - def test_get_login_credentials_msi_user_assigned_with_object_id(self): - - # setup an existing msi subscription + def test_get_login_credentials_mi_user_assigned_with_object_id(self): profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None}) - test_subscription_id = '12345678-1bf0-4dda-aec3-cb9272f09590' test_object_id = '12345678-38d6-4fb2-bad9-b7b93a3e9999' - msi_subscription = SubscriptionStub('/subscriptions/12345678-1bf0-4dda-aec3-cb9272f09590', - 'MSIObject-{}'.format(test_object_id), - self.state1, '12345678-38d6-4fb2-bad9-b7b93a3e1234') - consolidated = profile._normalize_properties('userAssignedIdentity', [msi_subscription], True) + consolidated = profile._normalize_properties('userAssignedIdentity', + [deepcopy(self.test_mi_subscription)], + True, + user_assigned_identity_id='MSIObject-{}'.format(test_object_id)) profile._set_subscriptions(consolidated, secondary_key_name='name') cred, subscription_id, _ = profile.get_login_credentials() - self.assertEqual(subscription_id, test_subscription_id) + self.assertEqual(subscription_id, self.test_mi_subscription_id) # sniff test the msi_auth object cred.set_token() @@ -1080,21 +1078,19 @@ def test_get_login_credentials_msi_user_assigned_with_object_id(self): self.assertTrue(cred.object_id, test_object_id) @mock.patch('azure.cli.core.auth.adal_authentication.MSIAuthenticationWrapper', MSRestAzureAuthStub) - def test_get_login_credentials_msi_user_assigned_with_res_id(self): - # setup an existing msi subscription + def test_get_login_credentials_mi_user_assigned_with_res_id(self): profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None}) - test_subscription_id = '12345678-1bf0-4dda-aec3-cb9272f09590' test_res_id = ('/subscriptions/{}/resourceGroups/r1/providers/Microsoft.ManagedIdentity/' - 'userAssignedIdentities/id1').format(test_subscription_id) - msi_subscription = SubscriptionStub('/subscriptions/{}'.format(test_subscription_id), - 'MSIResource-{}'.format(test_res_id), - self.state1, '12345678-38d6-4fb2-bad9-b7b93a3e1234') - consolidated = profile._normalize_properties('userAssignedIdentity', [msi_subscription], True) + 'userAssignedIdentities/id1').format(self.test_mi_subscription_id) + consolidated = profile._normalize_properties('userAssignedIdentity', + [deepcopy(self.test_mi_subscription)], + True, + user_assigned_identity_id='MSIResource-{}'.format(test_res_id)) profile._set_subscriptions(consolidated, secondary_key_name='name') cred, subscription_id, _ = profile.get_login_credentials() - self.assertEqual(subscription_id, test_subscription_id) + self.assertEqual(subscription_id, self.test_mi_subscription_id) # sniff test the msi_auth object cred.set_token() @@ -1191,17 +1187,12 @@ 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) - def test_get_raw_token_msi_system_assigned(self, mock_msi_auth): - # setup an existing msi subscription + def test_get_raw_token_mi_system_assigned(self, mock_msi_auth): profile = Profile(cli_ctx=DummyCli(), storage={'subscriptions': None}) - test_subscription_id = '12345678-1bf0-4dda-aec3-cb9272f09590' - test_tenant_id = '12345678-38d6-4fb2-bad9-b7b93a3e1234' - test_user = 'systemAssignedIdentity' - msi_subscription = SubscriptionStub('/subscriptions/' + test_subscription_id, - 'MSI', self.state1, test_tenant_id) - consolidated = profile._normalize_properties(test_user, - [msi_subscription], - True) + consolidated = profile._normalize_properties('systemAssignedIdentity', + [deepcopy(self.test_mi_subscription)], + True, + user_assigned_identity_id='MSI') profile._set_subscriptions(consolidated) mi_auth_instance = None @@ -1220,15 +1211,14 @@ def mi_auth_factory(*args, **kwargs): assert mi_auth_instance.resource == self.adal_resource assert list(mi_auth_instance.get_token_scopes) == self.msal_scopes - self.assertEqual(subscription_id, test_subscription_id) self.assertEqual(cred[0], 'Bearer') - self.assertEqual(cred[1], TestProfile.test_msi_access_token) + self.assertEqual(cred[1], self.test_mi_access_token) # Make sure expires_on and expiresOn are set self.assertEqual(cred[2]['expires_on'], MOCK_EXPIRES_ON_INT) self.assertEqual(cred[2]['expiresOn'], MOCK_EXPIRES_ON_DATETIME) - self.assertEqual(subscription_id, test_subscription_id) - self.assertEqual(tenant_id, test_tenant_id) + self.assertEqual(subscription_id, self.test_mi_subscription_id) + self.assertEqual(tenant_id, self.test_mi_tenant) # verify tenant shouldn't be specified for MSI account with self.assertRaisesRegex(CLIError, "Tenant shouldn't be specified"):