Skip to content

Commit

Permalink
Always use access token for acr login (Azure#6211)
Browse files Browse the repository at this point in the history
  • Loading branch information
djyou authored and yugangw-msft committed Apr 26, 2018
1 parent 8b12f72 commit a0efd86
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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(
Expand All @@ -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)

0 comments on commit a0efd86

Please sign in to comment.