From a0efd86f879ef08260c36ca262613010ad89de92 Mon Sep 17 00:00:00 2001 From: Dongjiang You Date: Wed, 25 Apr 2018 17:15:13 -0700 Subject: [PATCH] Always use access token for acr login (#6211) --- .../cli/command_modules/acr/_docker_utils.py | 32 +++------- .../tests/latest/test_acr_commands_mock.py | 63 +------------------ 2 files changed, 10 insertions(+), 85 deletions(-) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py index 388fbdc0799..9ef0a649d38 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py @@ -56,33 +56,15 @@ def _get_aad_token(cli_ctx, login_server, only_refresh_token, repository=None, p from azure.cli.core._profile import Profile profile = Profile(cli_ctx=cli_ctx) - sp_id, refresh, access, tenant = profile.get_refresh_token() + creds, _, tenant = profile.get_raw_token() headers = {'Content-Type': 'application/x-www-form-urlencoded'} - if not sp_id: - if not refresh: - content = { - 'grant_type': 'access_token', - 'service': params['service'], - 'tenant': tenant, - 'access_token': access - } - else: - content = { - 'grant_type': 'access_token_refresh_token', - 'service': params['service'], - 'tenant': tenant, - 'access_token': access, - 'refresh_token': refresh - } - else: - content = { - 'grant_type': 'spn', - 'service': params['service'], - 'tenant': tenant, - 'username': sp_id, - 'password': refresh - } + content = { + 'grant_type': 'access_token', + 'service': params['service'], + 'tenant': tenant, + 'access_token': creds[1] + } response = requests.post(authhost, urlencode(content), headers=headers, verify=(not should_disable_connection_verify())) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py index 26b6ad3e669..df52c8b7cb7 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py @@ -215,11 +215,11 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g headers=_get_authorization_header('username', 'password'), verify=mock.ANY) - @mock.patch('azure.cli.core._profile.Profile.get_refresh_token', autospec=True) + @mock.patch('azure.cli.core._profile.Profile.get_raw_token', autospec=True) @mock.patch('azure.cli.command_modules.acr._docker_utils.get_registry_by_name', autospec=True) @mock.patch('requests.post', autospec=True) @mock.patch('requests.get', autospec=True) - def test_get_docker_credentials(self, mock_requests_get, mock_requests_post, mock_get_registry_by_name, mock_get_refresh_token): + def test_get_docker_credentials(self, mock_requests_get, mock_requests_post, mock_get_registry_by_name, mock_get_raw_token): cmd = mock.MagicMock() cmd.cli_ctx = TestCli() @@ -244,37 +244,8 @@ def test_get_docker_credentials(self, mock_requests_get, mock_requests_post, moc 'access_token': 'testaccesstoken'}).encode() mock_requests_post.return_value = refresh_token_response - # Set up AAD token with both refresh and access tokens - mock_get_refresh_token.return_value = None, 'aadrefreshtoken', 'aadaccesstoken', 'testtenant' - get_login_credentials(cmd.cli_ctx, 'testregistry', None, None, None) - mock_requests_get.assert_called_with('https://testregistry.azurecr.io/v2/', - verify=mock.ANY) - mock_requests_post.assert_called_with( - 'https://testregistry.azurecr.io/oauth2/exchange', - urlencode({ - 'grant_type': 'access_token_refresh_token', - 'service': 'testregistry.azurecr.io', - 'tenant': 'testtenant', - 'access_token': 'aadaccesstoken', - 'refresh_token': 'aadrefreshtoken' - }), - headers={'Content-Type': 'application/x-www-form-urlencoded'}, - verify=mock.ANY) - - get_access_credentials(cmd.cli_ctx, 'testregistry', None, None, None, 'testrepository') - mock_requests_post.assert_called_with( - 'https://testregistry.azurecr.io/oauth2/token', - urlencode({ - 'grant_type': 'refresh_token', - 'service': 'testregistry.azurecr.io', - 'scope': 'repository:testrepository:*', - 'refresh_token': 'testrefreshtoken' - }), - headers={'Content-Type': 'application/x-www-form-urlencoded'}, - verify=mock.ANY) - # Set up AAD token with only access token - mock_get_refresh_token.return_value = None, None, 'aadaccesstoken', 'testtenant' + mock_get_raw_token.return_value = ('Bearer', 'aadaccesstoken', {}), 'testsubscription', 'testtenant' get_login_credentials(cmd.cli_ctx, 'testregistry', None, None, None) mock_requests_get.assert_called_with('https://testregistry.azurecr.io/v2/', verify=mock.ANY) mock_requests_post.assert_called_with( @@ -299,31 +270,3 @@ def test_get_docker_credentials(self, mock_requests_get, mock_requests_post, moc }), headers={'Content-Type': 'application/x-www-form-urlencoded'}, verify=mock.ANY) - - # Set up AAD token with service principal - mock_get_refresh_token.return_value = 'testspid', 'testsppassword', None, 'testtenant' - get_login_credentials(cmd.cli_ctx, 'testregistry', None, None, None) - mock_requests_get.assert_called_with('https://testregistry.azurecr.io/v2/', verify=mock.ANY) - mock_requests_post.assert_called_with( - 'https://testregistry.azurecr.io/oauth2/exchange', - urlencode({ - 'grant_type': 'spn', - 'service': 'testregistry.azurecr.io', - 'tenant': 'testtenant', - 'username': 'testspid', - 'password': 'testsppassword' - }), - headers={'Content-Type': 'application/x-www-form-urlencoded'}, - verify=mock.ANY) - - get_access_credentials(cmd.cli_ctx, 'testregistry', None, None, None, 'testrepository') - mock_requests_post.assert_called_with( - 'https://testregistry.azurecr.io/oauth2/token', - urlencode({ - 'grant_type': 'refresh_token', - 'service': 'testregistry.azurecr.io', - 'scope': 'repository:testrepository:*', - 'refresh_token': 'testrefreshtoken' - }), - headers={'Content-Type': 'application/x-www-form-urlencoded'}, - verify=mock.ANY)