From 7435f610b78ec9d996bb0ade069dedc9f5474dfa Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Thu, 5 Nov 2020 15:18:50 +0800 Subject: [PATCH 1/3] Empty scopes --- .../azure/cli/core/adal_authentication.py | 28 +++++++++++++++-- .../core/tests/test_adal_authentication.py | 30 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/azure-cli-core/azure/cli/core/tests/test_adal_authentication.py diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index a4496b05d7a..32a03f004ae 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -75,7 +75,7 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument if len(scopes) > 1: scopes = scopes[1:] - _, token, full_token, _ = self._get_token(scopes_to_resource(scopes)) + _, token, full_token, _ = self._get_token(_try_scopes_to_resource(scopes)) try: return AccessToken(token, int(full_token['expiresIn'] + time.time())) except KeyError: # needed to deal with differing unserialized MSI token payload @@ -106,7 +106,9 @@ class MSIAuthenticationWrapper(MSIAuthentication): # This method is exposed for Azure Core. Add *scopes, **kwargs to fit azure.core requirement def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument logger.debug("MSIAuthenticationWrapper.get_token invoked by Track 2 SDK with scopes=%s", scopes) - self.resource = scopes_to_resource(scopes) + resource = _try_scopes_to_resource(scopes) + if resource: + self.resource = resource self.set_token() return AccessToken(self.token['access_token'], int(self.token['expires_on'])) @@ -135,3 +137,25 @@ def set_token(self): def signed_session(self, session=None): logger.debug("MSIAuthenticationWrapper.signed_session invoked by Track 1 SDK") super().signed_session(session) + + +def _try_scopes_to_resource(scopes): + """Wrap scopes_to_resource to workaround some SDK issues.""" + + # Track 2 SDKs generated before https://github.com/Azure/autorest.python/pull/239 don't maintain + # credential_scopes and call `get_token` with empty scopes. + # As a workaround, return None so that the CLI-managed resource is used. + if not scopes: + logger.debug("No scope is provided by the SDK, use the CLI-managed resource.") + return None + + # Track 2 SDKs generated before https://github.com/Azure/autorest.python/pull/745 extend default + # credential_scopes with custom credential_scopes. Instead, credential_scopes should be replaced by + # custom credential_scopes. https://github.com/Azure/azure-sdk-for-python/issues/12947 + # As a workaround, remove the first one if there are multiple scopes provided. + if len(scopes) > 1: + logger.debug("Multiple scopes are provided by the SDK, discarding the first one: %s.", scopes[0]) + return scopes_to_resource(scopes[1:]) + + # Exactly only one scope is provided + return scopes_to_resource(scopes) diff --git a/src/azure-cli-core/azure/cli/core/tests/test_adal_authentication.py b/src/azure-cli-core/azure/cli/core/tests/test_adal_authentication.py new file mode 100644 index 00000000000..be660f2e751 --- /dev/null +++ b/src/azure-cli-core/azure/cli/core/tests/test_adal_authentication.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. +# -------------------------------------------------------------------------------------------- + +# pylint: disable=line-too-long +import unittest +from azure.cli.core.adal_authentication import _try_scopes_to_resource + + +class TestUtils(unittest.TestCase): + + def test_try_scopes_to_resource(self): + # Test no scopes + self.assertIsNone(_try_scopes_to_resource(())) + self.assertIsNone(_try_scopes_to_resource([])) + self.assertIsNone(_try_scopes_to_resource(None)) + + # Test multiple scopes, with the first one discarded + resource = _try_scopes_to_resource(("https://management.core.windows.net//.default", + "https://management.core.chinacloudapi.cn//.default")) + self.assertEqual(resource, "https://management.core.chinacloudapi.cn/") + + # Test single scopes (the correct usage) + resource = _try_scopes_to_resource(("https://management.core.chinacloudapi.cn//.default",)) + self.assertEqual(resource, "https://management.core.chinacloudapi.cn/") + + +if __name__ == '__main__': + unittest.main() From 6ba05d12c238876465b9a880fa4ff7b11010832c Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Thu, 5 Nov 2020 15:47:47 +0800 Subject: [PATCH 2/3] Remove useless code --- src/azure-cli-core/azure/cli/core/adal_authentication.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 32a03f004ae..16239c8fe29 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -68,13 +68,6 @@ def _get_token(self, sdk_resource=None): def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument logger.debug("AdalAuthentication.get_token invoked by Track 2 SDK with scopes=%s", scopes) - # Deal with an old Track 2 SDK issue where the default credential_scopes is extended with - # custom credential_scopes. Instead, credential_scopes should be replaced by custom credential_scopes. - # https://github.com/Azure/azure-sdk-for-python/issues/12947 - # We simply remove the first one if there are multiple scopes provided. - if len(scopes) > 1: - scopes = scopes[1:] - _, token, full_token, _ = self._get_token(_try_scopes_to_resource(scopes)) try: return AccessToken(token, int(full_token['expiresIn'] + time.time())) @@ -154,7 +147,7 @@ def _try_scopes_to_resource(scopes): # custom credential_scopes. https://github.com/Azure/azure-sdk-for-python/issues/12947 # As a workaround, remove the first one if there are multiple scopes provided. if len(scopes) > 1: - logger.debug("Multiple scopes are provided by the SDK, discarding the first one: %s.", scopes[0]) + logger.debug("Multiple scopes are provided by the SDK, discarding the first one: %s", scopes[0]) return scopes_to_resource(scopes[1:]) # Exactly only one scope is provided From 1b2786784b57a9f3de12a15de92804f47f11ad0e Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Thu, 5 Nov 2020 17:05:19 +0800 Subject: [PATCH 3/3] Add comment --- src/azure-cli-core/azure/cli/core/adal_authentication.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/azure-cli-core/azure/cli/core/adal_authentication.py b/src/azure-cli-core/azure/cli/core/adal_authentication.py index 16239c8fe29..4d42159d5f9 100644 --- a/src/azure-cli-core/azure/cli/core/adal_authentication.py +++ b/src/azure-cli-core/azure/cli/core/adal_authentication.py @@ -101,6 +101,7 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument logger.debug("MSIAuthenticationWrapper.get_token invoked by Track 2 SDK with scopes=%s", scopes) resource = _try_scopes_to_resource(scopes) if resource: + # If available, use resource provided by SDK self.resource = resource self.set_token() return AccessToken(self.token['access_token'], int(self.token['expires_on']))