From ef307b7c2860456405c1ee355578d3d51996677c Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Tue, 14 Apr 2020 17:17:51 +0800 Subject: [PATCH 1/7] {Core} support get msal accesstoken with adal refresh token for vm ssh feature --- src/azure-cli-core/azure/cli/core/_msal.py | 30 +++++++++++++ src/azure-cli-core/azure/cli/core/_profile.py | 44 +++++++++++++++++++ .../core/commands/ssh_credential_factory.py | 30 +++++++++++++ src/azure-cli-core/setup.py | 2 + 4 files changed, 106 insertions(+) create mode 100644 src/azure-cli-core/azure/cli/core/_msal.py create mode 100644 src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py diff --git a/src/azure-cli-core/azure/cli/core/_msal.py b/src/azure-cli-core/azure/cli/core/_msal.py new file mode 100644 index 00000000000..b4240c477cb --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/_msal.py @@ -0,0 +1,30 @@ +from knack.util import CLIError +from msal import ClientApplication + + +class SSHCertificateClientApplication(ClientApplication): + """ + This class is added only for vmssh feature. + This is a temporary solution and will deprecate after adoption to MSAL completely. + """ + def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( + self, authority, scopes, account, **kwargs): + # pylint: disable=line-too-long + return self._acquire_token_silent_by_finding_specific_refresh_token( + authority, scopes, **kwargs) + + def _acquire_token_silent_by_finding_specific_refresh_token( + self, authority, scopes, query, + rt_remover=None, break_condition=lambda response: False, **kwargs): + refresh_token = kwargs.get('refresh_token', None) + client = self._build_client(self.client_credential, authority) + if 'refresh_token' in kwargs: + kwargs.pop('refresh_token') + if 'force_refresh' in kwargs: + kwargs.pop('force_refresh') + if 'correlation_id' in kwargs: + kwargs.pop('correlation_id') + response = client.obtain_token_by_refresh_token(refresh_token, scope=scopes, **kwargs) + if "error" in response: + raise CLIError(response["error"]) + return response diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 38877ec69df..eaf90c673b6 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -525,6 +525,16 @@ def get_access_token_for_resource(self, username, tenant, resource): username, tenant, resource) return access_token + def _get_ssh_certificate_for_resource(self, tenant, modulus, exponent): + """ + This is added only for vmssh feature. + It is a temporary solution and will deprecate after adoption to MSAL completely. + """ + tenant = tenant or 'common' + _, refresh_token, _, _ = self.get_refresh_token() + _, cert, _ = self._creds_cache.retrieve_ssh_certificate_for_user(tenant, modulus, exponent, refresh_token) + return cert + @staticmethod def _try_parse_msi_account_name(account): msi_info, user = account[_USER_ENTITY].get(_ASSIGNED_IDENTITY_INFO), account[_USER_ENTITY].get(_USER_NAME) @@ -593,6 +603,17 @@ def _retrieve_tokens_from_external_tenants(): str(account[_SUBSCRIPTION_ID]), str(account[_TENANT_ID])) + def get_ssh_credentials(self, modulus, exponent): + """ + This is added only for vmssh feature. + It is a temporary solution and will deprecate after adoption to MSAL completely. + """ + account = self.get_subscription() + username = account[_USER_ENTITY][_USER_NAME] + return username, self._get_ssh_certificate_for_resource( + account[_TENANT_ID], modulus, exponent + ) + def get_refresh_token(self, resource=None, subscription=None): account = self.get_subscription(subscription) @@ -1008,6 +1029,29 @@ def retrieve_token_for_user(self, username, tenant, resource): self.persist_cached_creds() return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN], token_entry) + def retrieve_ssh_certificate_for_user(self, tenant, modulus, exponent, refresh_token): + """ + This is added only for vmssh feature. + It is a temporary solution and will deprecate after adoption to MSAL completely. + """ + from azure.cli.core._msal import SSHCertificateClientApplication + scopes = ["https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] + tenant = tenant or 'organizations' + authority = self._ctx.cloud.endpoints.active_directory + '/' + tenant + app = SSHCertificateClientApplication(_CLIENT_ID, authority=authority) + jwk = { + "kty": "RSA", + "n": modulus, + "e": exponent, + "kid": str(hash((modulus, exponent))) + } + json_jwk = json.dumps(jwk) + result = app.acquire_token_silent(scopes, None, + data={"token_type": "ssh-cert", "req_cnf": json_jwk, "key_id": jwk["kid"]}, + refresh_token=refresh_token) + + return result["token_type"], result["access_token"], result + def retrieve_token_for_service_principal(self, sp_id, resource, tenant, use_cert_sn_issuer=False): self.load_adal_token_cache() matched = [x for x in self._service_principal_creds if sp_id == x[_SERVICE_PRINCIPAL_ID]] diff --git a/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py b/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py new file mode 100644 index 00000000000..20f5ea9beb8 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py @@ -0,0 +1,30 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- +""" +This is added only for vmssh feature. +This is a temporary solution and will deprecate after adoption to MSAL completely. +The function is try to retrieve MSAL access token with ADAL refresh token. +""" +from knack import log + + +logger = log.get_logger(__name__) + + +def get_ssh_credentials(cli_ctx, modulus, exponent): + from azure.cli.core._profile import Profile + logger.debug("Getting SSH credentials") + profile = Profile(cli_ctx=cli_ctx) + + user, cert = profile.get_ssh_credentials(modulus, exponent) + return SSHCredentials(user, cert) + + +class SSHCredentials(object): + # pylint: disable=too-few-public-methods + + def __init__(self, username, cert): + self.username = username + self.certificate = "ssh-rsa-cert-v01@openssh.com " + cert diff --git a/src/azure-cli-core/setup.py b/src/azure-cli-core/setup.py index ea6d1b40b85..98d9c5f7c39 100644 --- a/src/azure-cli-core/setup.py +++ b/src/azure-cli-core/setup.py @@ -57,6 +57,8 @@ 'humanfriendly>=4.7,<9.0', 'jmespath', 'knack==0.7.0rc4', + 'msal~=1.0.0', + 'msal-extensions~=0.1.3', 'msrest>=0.4.4', 'msrestazure>=0.6.3', 'paramiko>=2.0.8,<3.0.0', From 0d909118185573e680ef753bf5eda8f5e50f68bf Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Tue, 14 Apr 2020 17:43:33 +0800 Subject: [PATCH 2/7] add License header for _msal.py --- src/azure-cli-core/azure/cli/core/_msal.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/azure-cli-core/azure/cli/core/_msal.py b/src/azure-cli-core/azure/cli/core/_msal.py index b4240c477cb..d208eee618f 100644 --- a/src/azure-cli-core/azure/cli/core/_msal.py +++ b/src/azure-cli-core/azure/cli/core/_msal.py @@ -1,3 +1,7 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- from knack.util import CLIError from msal import ClientApplication From adf6adb5c703b29bcd76f04ff40a1ae9ee55fc29 Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Wed, 22 Apr 2020 14:51:30 +0800 Subject: [PATCH 3/7] use hashlib instead of build-in hash function --- src/azure-cli-core/azure/cli/core/_profile.py | 9 ++++++++- 1 file changed, 8 insertions(+), 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 eaf90c673b6..5a8f9805f61 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -1035,15 +1035,22 @@ def retrieve_ssh_certificate_for_user(self, tenant, modulus, exponent, refresh_t It is a temporary solution and will deprecate after adoption to MSAL completely. """ from azure.cli.core._msal import SSHCertificateClientApplication + import hashlib scopes = ["https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] tenant = tenant or 'organizations' authority = self._ctx.cloud.endpoints.active_directory + '/' + tenant app = SSHCertificateClientApplication(_CLIENT_ID, authority=authority) + + key_hash = hashlib.sha256() + key_hash.update(modulus.encode('utf-8')) + key_hash.update(exponent.encode('utf-8')) + key_id = key_hash.hexdigest() + jwk = { "kty": "RSA", "n": modulus, "e": exponent, - "kid": str(hash((modulus, exponent))) + "kid": key_id } json_jwk = json.dumps(jwk) result = app.acquire_token_silent(scopes, None, From 6f12c34308f1bcc6e832175c1ed134898108c2af Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Fri, 24 Apr 2020 10:45:44 +0800 Subject: [PATCH 4/7] refactor get_msal_token function --- src/azure-cli-core/azure/cli/core/_msal.py | 2 +- src/azure-cli-core/azure/cli/core/_profile.py | 50 +++++-------------- .../core/commands/ssh_credential_factory.py | 4 +- 3 files changed, 15 insertions(+), 41 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_msal.py b/src/azure-cli-core/azure/cli/core/_msal.py index d208eee618f..0f9c4bc6214 100644 --- a/src/azure-cli-core/azure/cli/core/_msal.py +++ b/src/azure-cli-core/azure/cli/core/_msal.py @@ -6,7 +6,7 @@ from msal import ClientApplication -class SSHCertificateClientApplication(ClientApplication): +class AdalRefreshTokenBasedClientApplication(ClientApplication): """ This class is added only for vmssh feature. This is a temporary solution and will deprecate after adoption to MSAL completely. diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 5a8f9805f61..658c047bfb4 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -525,16 +525,6 @@ def get_access_token_for_resource(self, username, tenant, resource): username, tenant, resource) return access_token - def _get_ssh_certificate_for_resource(self, tenant, modulus, exponent): - """ - This is added only for vmssh feature. - It is a temporary solution and will deprecate after adoption to MSAL completely. - """ - tenant = tenant or 'common' - _, refresh_token, _, _ = self.get_refresh_token() - _, cert, _ = self._creds_cache.retrieve_ssh_certificate_for_user(tenant, modulus, exponent, refresh_token) - return cert - @staticmethod def _try_parse_msi_account_name(account): msi_info, user = account[_USER_ENTITY].get(_ASSIGNED_IDENTITY_INFO), account[_USER_ENTITY].get(_USER_NAME) @@ -603,16 +593,17 @@ def _retrieve_tokens_from_external_tenants(): str(account[_SUBSCRIPTION_ID]), str(account[_TENANT_ID])) - def get_ssh_credentials(self, modulus, exponent): + def get_msal_token(self, scopes, data): """ This is added only for vmssh feature. - It is a temporary solution and will deprecate after adoption to MSAL completely. + It is a temporary solution and will deprecate after MSAL adopted completely. """ account = self.get_subscription() username = account[_USER_ENTITY][_USER_NAME] - return username, self._get_ssh_certificate_for_resource( - account[_TENANT_ID], modulus, exponent - ) + tenant = account[_TENANT_ID] or 'common' + _, refresh_token, _, _ = self.get_refresh_token() + certificate = self._creds_cache.retrieve_msal_token(tenant, scopes, data, refresh_token) + return username, certificate def get_refresh_token(self, resource=None, subscription=None): @@ -1029,35 +1020,18 @@ def retrieve_token_for_user(self, username, tenant, resource): self.persist_cached_creds() return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN], token_entry) - def retrieve_ssh_certificate_for_user(self, tenant, modulus, exponent, refresh_token): + def retrieve_msal_token(self, tenant, scopes, data, refresh_token): """ This is added only for vmssh feature. - It is a temporary solution and will deprecate after adoption to MSAL completely. + It is a temporary solution and will deprecate after MSAL adopted completely. """ - from azure.cli.core._msal import SSHCertificateClientApplication - import hashlib - scopes = ["https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] + from azure.cli.core._msal import AdalRefreshTokenBasedClientApplication tenant = tenant or 'organizations' authority = self._ctx.cloud.endpoints.active_directory + '/' + tenant - app = SSHCertificateClientApplication(_CLIENT_ID, authority=authority) - - key_hash = hashlib.sha256() - key_hash.update(modulus.encode('utf-8')) - key_hash.update(exponent.encode('utf-8')) - key_id = key_hash.hexdigest() - - jwk = { - "kty": "RSA", - "n": modulus, - "e": exponent, - "kid": key_id - } - json_jwk = json.dumps(jwk) - result = app.acquire_token_silent(scopes, None, - data={"token_type": "ssh-cert", "req_cnf": json_jwk, "key_id": jwk["kid"]}, - refresh_token=refresh_token) + app = AdalRefreshTokenBasedClientApplication(_CLIENT_ID, authority=authority) + result = app.acquire_token_silent(scopes, None, data=data, refresh_token=refresh_token) - return result["token_type"], result["access_token"], result + return result["access_token"] def retrieve_token_for_service_principal(self, sp_id, resource, tenant, use_cert_sn_issuer=False): self.load_adal_token_cache() diff --git a/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py b/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py index 20f5ea9beb8..332f3e0bdc0 100644 --- a/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py +++ b/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py @@ -13,12 +13,12 @@ logger = log.get_logger(__name__) -def get_ssh_credentials(cli_ctx, modulus, exponent): +def get_ssh_credentials(cli_ctx, scopes, data): from azure.cli.core._profile import Profile logger.debug("Getting SSH credentials") profile = Profile(cli_ctx=cli_ctx) - user, cert = profile.get_ssh_credentials(modulus, exponent) + user, cert = profile.get_ssh_credentials(scopes, data) return SSHCredentials(user, cert) From 287c23be7a6158d78c70d1e1f4fdda51ba22ba2b Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Fri, 24 Apr 2020 11:27:47 +0800 Subject: [PATCH 5/7] delete unused code --- .../core/commands/ssh_credential_factory.py | 30 ------------------- 1 file changed, 30 deletions(-) delete mode 100644 src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py diff --git a/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py b/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py deleted file mode 100644 index 332f3e0bdc0..00000000000 --- a/src/azure-cli-core/azure/cli/core/commands/ssh_credential_factory.py +++ /dev/null @@ -1,30 +0,0 @@ -# -------------------------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- -""" -This is added only for vmssh feature. -This is a temporary solution and will deprecate after adoption to MSAL completely. -The function is try to retrieve MSAL access token with ADAL refresh token. -""" -from knack import log - - -logger = log.get_logger(__name__) - - -def get_ssh_credentials(cli_ctx, scopes, data): - from azure.cli.core._profile import Profile - logger.debug("Getting SSH credentials") - profile = Profile(cli_ctx=cli_ctx) - - user, cert = profile.get_ssh_credentials(scopes, data) - return SSHCredentials(user, cert) - - -class SSHCredentials(object): - # pylint: disable=too-few-public-methods - - def __init__(self, username, cert): - self.username = username - self.certificate = "ssh-rsa-cert-v01@openssh.com " + cert From 0aee4dcb38055433431f7c789196dc20fd8cd349 Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Fri, 24 Apr 2020 13:38:26 +0800 Subject: [PATCH 6/7] add query parameter --- src/azure-cli-core/azure/cli/core/_msal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/_msal.py b/src/azure-cli-core/azure/cli/core/_msal.py index 0f9c4bc6214..9ea58a077d1 100644 --- a/src/azure-cli-core/azure/cli/core/_msal.py +++ b/src/azure-cli-core/azure/cli/core/_msal.py @@ -15,7 +15,7 @@ def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( self, authority, scopes, account, **kwargs): # pylint: disable=line-too-long return self._acquire_token_silent_by_finding_specific_refresh_token( - authority, scopes, **kwargs) + authority, scopes, None, **kwargs) def _acquire_token_silent_by_finding_specific_refresh_token( self, authority, scopes, query, From 87a35333dc4d2aa1cee2a5bd134a59a519ce6772 Mon Sep 17 00:00:00 2001 From: Xiaojian Xu Date: Fri, 24 Apr 2020 17:20:25 +0800 Subject: [PATCH 7/7] add test for get_msal_token --- src/azure-cli-core/azure/cli/core/_msal.py | 4 +-- .../azure/cli/core/tests/test_profile.py | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_msal.py b/src/azure-cli-core/azure/cli/core/_msal.py index 9ea58a077d1..cc46e9d2d57 100644 --- a/src/azure-cli-core/azure/cli/core/_msal.py +++ b/src/azure-cli-core/azure/cli/core/_msal.py @@ -8,8 +8,8 @@ class AdalRefreshTokenBasedClientApplication(ClientApplication): """ - This class is added only for vmssh feature. - This is a temporary solution and will deprecate after adoption to MSAL completely. + This is added only for vmssh feature. + It is a temporary solution and will deprecate after MSAL adopted completely. """ def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( self, authority, scopes, account, **kwargs): 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 57292e2cef0..a1a254840e8 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 @@ -1911,6 +1911,35 @@ def test_find_using_specific_tenant(self, _get_authorization_code_mock, mock_aut self.assertEqual(all_subscriptions[0].tenant_id, token_tenant) self.assertEqual(all_subscriptions[0].home_tenant_id, home_tenant) + @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_for_user', autospec=True) + @mock.patch('azure.cli.core._msal.AdalRefreshTokenBasedClientApplication._acquire_token_silent_by_finding_specific_refresh_token', autospec=True) + def test_get_msal_token(self, mock_acquire_token, mock_retrieve_token_for_user): + """ + This is added only for vmssh feature. + It is a temporary solution and will deprecate after MSAL adopted completely. + """ + cli = DummyCli() + storage_mock = {'subscriptions': None} + profile = Profile(cli_ctx=cli, storage=storage_mock, use_global_creds_cache=False, async_persist=False) + + consolidated = profile._normalize_properties(self.user1, [self.subscription1], False) + profile._set_subscriptions(consolidated) + + some_token_type = 'Bearer' + mock_retrieve_token_for_user.return_value = (some_token_type, TestProfile.raw_token1, TestProfile.token_entry1) + mock_acquire_token.return_value = { + 'access_token': 'fake_access_token' + } + scopes = ["https://pas.windows.net/CheckMyAccess/Linux/user_impersonation"] + data = { + "token_type": "ssh-cert", + "req_cnf": "fake_jwk", + "key_id": "fake_id" + } + username, access_token = profile.get_msal_token(scopes, data) + self.assertEqual(username, self.user1) + self.assertEqual(access_token, 'fake_access_token') + class FileHandleStub(object): # pylint: disable=too-few-public-methods