From 44cabc3c9285bc63286bf153b6fe8a4a7dfe5a94 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 7 Nov 2016 11:01:55 -0800 Subject: [PATCH 1/8] expose refresh token retrieving mechanism in profile --- src/azure-cli-core/azure/cli/core/_profile.py | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 4282214aa9b..8f4b17c3b70 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -38,6 +38,7 @@ _STATE = 'state' _USER_TYPE = 'type' _USER = 'user' +_USER_AUTHORITY = '_authority' _SERVICE_PRINCIPAL = 'servicePrincipal' _SERVICE_PRINCIPAL_ID = 'servicePrincipalId' _SERVICE_PRINCIPAL_TENANT = 'servicePrincipalTenant' @@ -46,6 +47,7 @@ #This could mean either real access token, or client secret of a service principal #This naming is no good, but can't change because xplat-cli does so. _ACCESS_TOKEN = 'accessToken' +_REFRESH_TOKEN = 'refreshToken' TOKEN_FIELDS_EXCLUDED_FROM_PERSISTENCE = ['familyName', 'givenName', @@ -282,6 +284,21 @@ def get_expanded_subscription_info(self, subscription_id=None, name=None, passwo result['endpoints'] = CLOUD.endpoints return result + def get_refresh_credentials(self, resource=CLOUD.endpoints.management, + subscription_id=None): + account = self.get_subscription(subscription_id) + user_type = account[_USER_ENTITY][_USER_TYPE] + username_or_sp_id = account[_USER_ENTITY][_USER_NAME] + + if user_type == _USER: + refresh_object = self._creds_cache.retrieve_token_entry_for_user( + username_or_sp_id, account[_TENANT_ID], resource)[_REFRESH_TOKEN] + else: + refresh_object = self._creds_cache \ + .retrieve_cred_for_service_principal(username_or_sp_id) + + return refresh_object + def get_installation_id(self): installation_id = self._storage.get(_INSTALLATION_ID) if not installation_id: @@ -393,7 +410,7 @@ def persist_cached_creds(self): self.adal_token_cache.has_state_changed = False - def retrieve_token_for_user(self, username, tenant, resource): + def retrieve_token_entry_for_user(self, username, tenant, resource): authority = get_authority_url(tenant) context = self._auth_ctx_factory(authority, cache=self.adal_token_cache) token_entry = context.acquire_token(resource, username, _CLIENT_ID) @@ -402,19 +419,29 @@ def retrieve_token_for_user(self, username, tenant, resource): if self.adal_token_cache.has_state_changed: self.persist_cached_creds() + + return token_entry + + def retrieve_token_for_user(self, username, tenant, resource): + token_entry = self.retrieve_token_entry_for_user(username, tenant, resource) return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN]) def retrieve_token_for_service_principal(self, sp_id, resource): + cred = self.retrieve_cred_for_service_principal(sp_id) + authority_url = get_authority_url(cred[0]) + context = self._auth_ctx_factory(authority_url, None) + token_entry = context.acquire_token_with_client_credentials(resource, + cred[1], + cred[2]) + return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN]) + + def retrieve_cred_for_service_principal(self, sp_id): matched = [x for x in self._service_principal_creds if sp_id == x[_SERVICE_PRINCIPAL_ID]] if not matched: raise CLIError("Please run 'az account set' to select active account.") cred = matched[0] - authority_url = get_authority_url(cred[_SERVICE_PRINCIPAL_TENANT]) - context = self._auth_ctx_factory(authority_url, None) - token_entry = context.acquire_token_with_client_credentials(resource, - sp_id, - cred[_ACCESS_TOKEN]) - return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN]) + # (Tenant, Username, Password) + return (cred[_SERVICE_PRINCIPAL_TENANT], sp_id, cred[_ACCESS_TOKEN]) def retrieve_secret_of_service_principal(self, sp_id): matched = [x for x in self._service_principal_creds if sp_id == x[_SERVICE_PRINCIPAL_ID]] From 8021dda62af2c1bcba9060621fe214479a4d4af7 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 7 Nov 2016 15:01:38 -0800 Subject: [PATCH 2/8] added unit tests for refresh token operations --- src/azure-cli-core/azure/cli/core/_profile.py | 1 - .../azure/cli/core/tests/test_profile.py | 108 ++++++++++++++++-- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 8f4b17c3b70..46431b7483a 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -38,7 +38,6 @@ _STATE = 'state' _USER_TYPE = 'type' _USER = 'user' -_USER_AUTHORITY = '_authority' _SERVICE_PRINCIPAL = 'servicePrincipal' _SERVICE_PRINCIPAL_ID = 'servicePrincipalId' _SERVICE_PRINCIPAL_TENANT = 'servicePrincipalTenant' 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 609e6b3f294..6a8528c00c5 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 @@ -26,6 +26,7 @@ def setUpClass(cls): cls.state1, cls.tenant_id) cls.raw_token1 = 'some...secrets' + cls.refresh_token1 = 'faked123' cls.token_entry1 = { "_clientId": "04b07795-8ddb-461a-bbee-02f9e1bf7b46", "resource": "https://management.core.windows.net/", @@ -35,7 +36,7 @@ def setUpClass(cls): "identityProvider": "live.com", "_authority": "https://login.microsoftonline.com/common", "isMRRT": True, - "refreshToken": "faked123", + "refreshToken": cls.refresh_token1, "accessToken": cls.raw_token1, "userId": cls.user1 } @@ -272,6 +273,7 @@ def test_load_cached_tokens(self, mock_read_file): }) self.assertEqual(len(matched), 1) self.assertEqual(matched[0]['accessToken'], self.raw_token1) + self.assertEqual(matched[0]['refreshToken'], self.refresh_token1) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_for_user', autospec=True) @@ -301,6 +303,27 @@ def test_get_login_credentials(self, mock_get_token, mock_read_cred_file): 'https://management.core.windows.net/') self.assertEqual(mock_get_token.call_count, 1) + @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) + @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_entry_for_user', autospec=True) + def test_get_refresh_credentials(self, mock_get_token_entry, mock_read_cred_file): + mock_read_cred_file.return_value = [Test_Profile.token_entry1] + mock_get_token_entry.return_value = Test_Profile.token_entry1 + #setup + storage_mock = {'subscriptions': None} + profile = Profile(storage_mock) + consolidated = Profile._normalize_properties(self.user1, + [self.subscription1], + False) + profile._set_subscriptions(consolidated) + #action + refresh_object = profile.get_refresh_credentials() + + self.assertEqual(refresh_object, self.refresh_token1) + self.assertEqual(mock_read_cred_file.call_count, 1) + mock_get_token_entry.assert_called_once_with(mock.ANY, self.user1, self.tenant_id, + 'https://management.core.windows.net/') + self.assertEqual(mock_get_token_entry.call_count, 1) + @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_for_user', autospec=True) def test_get_login_credentials_for_graph_client(self, mock_get_token, mock_read_cred_file): @@ -322,6 +345,49 @@ def test_get_login_credentials_for_graph_client(self, mock_get_token, mock_read_ 'https://graph.windows.net/') self.assertEqual(tenant_id, self.tenant_id) + @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) + @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_entry_for_user', autospec=True) + def test_get_refresh_credentials_for_graph_client(self, mock_get_token_entry, mock_read_cred_file): + mock_read_cred_file.return_value = [Test_Profile.token_entry1] + mock_get_token_entry.return_value = Test_Profile.token_entry1 + #setup + storage_mock = {'subscriptions': None} + profile = Profile(storage_mock) + consolidated = Profile._normalize_properties(self.user1, [self.subscription1], + False) + profile._set_subscriptions(consolidated) + #action + refresh_object = profile.get_refresh_credentials( + resource=CLOUD.endpoints.active_directory_graph_resource_id) + + #verify + mock_get_token_entry.assert_called_once_with(mock.ANY, self.user1, self.tenant_id, + 'https://graph.windows.net/') + self.assertEqual(refresh_object, Test_Profile.refresh_token1) + + @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) + def test_get_refresh_credentials_for_spn(self, mock_read_cred_file): + sp_user = Test_Profile.user1 + sp_tenant = "mytenant" + sp_secret = "mysecret" + test_sp = { + "servicePrincipalId": sp_user, + "servicePrincipalTenant": sp_tenant, + "accessToken": sp_secret + } + mock_read_cred_file.return_value = [test_sp] + #setup + storage_mock = {'subscriptions': None} + profile = Profile(storage_mock) + consolidated = Profile._normalize_properties(self.user1, [self.subscription1], + True) + profile._set_subscriptions(consolidated) + #action + refresh_object = profile.get_refresh_credentials() + #verify + self.assertEqual(refresh_object, (sp_tenant, sp_user, sp_secret)) + self.assertEqual(mock_read_cred_file.call_count, 1) + @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @mock.patch('azure.cli.core._profile.CredsCache.persist_cached_creds', autospec=True) def test_logout(self, mock_persist_creds, mock_read_cred_file): @@ -431,10 +497,13 @@ def test_find_subscriptions_from_service_principal_id(self, mock_auth_context): @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) def test_credscache_load_tokens_and_sp_creds(self, mock_read_file): + sp_id = "myapp" + sp_tenant = "mytenant" + sp_secret = "Secret" test_sp = { - "servicePrincipalId": "myapp", - "servicePrincipalTenant": "mytenant", - "accessToken": "Secret" + "servicePrincipalId": sp_id, + "servicePrincipalTenant": sp_tenant, + "accessToken": sp_secret } mock_read_file.return_value = [self.token_entry1, test_sp] @@ -445,20 +514,29 @@ def test_credscache_load_tokens_and_sp_creds(self, mock_read_file): token_entries = [entry for _, entry in creds_cache.adal_token_cache.read_items()] self.assertEqual(token_entries, [self.token_entry1]) self.assertEqual(creds_cache._service_principal_creds, [test_sp]) + self.assertEqual(creds_cache.retrieve_cred_for_service_principal(sp_id), + (sp_tenant, sp_id, sp_secret)) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @mock.patch('os.fdopen', autospec=True) @mock.patch('os.open', autospec=True) def test_credscache_add_new_sp_creds(self, _, mock_open_for_write, mock_read_file): + sp1_id = "myapp" + sp1_tenant = "mytenant" + sp1_secret = "Secret" + + sp2_id = "myapp2" + sp2_tenant = "mytenant2" + sp2_secret = "Secret2" test_sp = { - "servicePrincipalId": "myapp", - "servicePrincipalTenant": "mytenant", - "accessToken": "Secret" + "servicePrincipalId": sp1_id, + "servicePrincipalTenant": sp1_tenant, + "accessToken": sp1_secret } test_sp2 = { - "servicePrincipalId": "myapp2", - "servicePrincipalTenant": "mytenant2", - "accessToken": "Secret2" + "servicePrincipalId": sp2_id, + "servicePrincipalTenant": sp2_tenant, + "accessToken": sp2_secret } mock_open_for_write.return_value = FileHandleStub() mock_read_file.return_value = [self.token_entry1, test_sp] @@ -474,6 +552,11 @@ def test_credscache_add_new_sp_creds(self, _, mock_open_for_write, mock_read_fil token_entries = [entry for _, entry in creds_cache.adal_token_cache.read_items()] self.assertEqual(token_entries, [self.token_entry1]) self.assertEqual(creds_cache._service_principal_creds, [test_sp, test_sp2]) + self.assertEqual(creds_cache.retrieve_cred_for_service_principal(sp1_id), + (sp1_tenant, sp1_id, sp1_secret)) + self.assertEqual(creds_cache.retrieve_cred_for_service_principal(sp2_id), + (sp2_tenant, sp2_id, sp2_secret)) + mock_open_for_write.assert_called_with(mock.ANY, 'w+') @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @@ -512,6 +595,7 @@ def test_credscache_remove_creds(self, _, mock_open_for_write, mock_read_file): def test_credscache_new_token_added_by_adal(self, mock_adal_auth_context, _, mock_open_for_write, mock_read_file): # pylint: disable=line-too-long token_entry2 = { "accessToken": "new token", + "refreshToken": "refresh token", "tokenType": "Bearer", "userId": self.user1 } @@ -531,14 +615,18 @@ def get_auth_context(authority, **kwargs): # pylint: disable=unused-argument mgmt_resource = 'https://management.core.windows.net/' token_type, token = creds_cache.retrieve_token_for_user(self.user1, self.tenant_id, mgmt_resource) + mock_adal_auth_context.acquire_token.assert_called_once_with( 'https://management.core.windows.net/', self.user1, mock.ANY) + token_entry = creds_cache.retrieve_token_entry_for_user(self.user1, self.tenant_id, + mgmt_resource) #assert mock_open_for_write.assert_called_with(mock.ANY, 'w+') self.assertEqual(token, 'new token') + self.assertEqual(token_entry, token_entry2) self.assertEqual(token_type, token_entry2['tokenType']) class FileHandleStub(object): # pylint: disable=too-few-public-methods From 5c8a1a2c4b6968dc5b2960bb8e53d3382301f0df Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 7 Nov 2016 15:22:05 -0800 Subject: [PATCH 3/8] add acr login --- .../azure/cli/command_modules/acr/_help.py | 8 +++ .../azure/cli/command_modules/acr/_utils.py | 51 +++++++++++++++++++ .../azure/cli/command_modules/acr/custom.py | 10 +++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py index 4e8603ad258..c72c8d4441e 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py @@ -44,6 +44,14 @@ az acr create -n myRegistry -g myResourceGroup -l southcentralus --storage-account-name myStorageAccount """ +helps['acr login'] = """ + type: command + examples: + - name: Log into a registry + text: + az acr login -n myRegistry + """ + helps['acr update'] = """ type: command short-summary: Updates a container registry. diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py index 84b87f439fd..d08bd1fea55 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py @@ -5,6 +5,14 @@ from azure.cli.core._util import CLIError from azure.cli.core.commands.parameters import get_resources_in_subscription +from azure.cli.core._profile import Profile + +from subprocess import call +from base64 import b64encode +from json import loads + +from msrest.service_client import ServiceClient +from msrest import Configuration from ._constants import ( ACR_RESOURCE_PROVIDER, @@ -18,6 +26,7 @@ get_acr_api_version ) + def _arm_get_resource_by_name(resource_name, resource_type): '''Returns the ARM resource in the current subscription with resource_name. :param str resource_name: The name of resource @@ -84,6 +93,48 @@ def get_access_key_by_storage_account_name(storage_account_name, resource_group_ return client.list_keys(resource_group_name, storage_account_name).keys[0].value #pylint: disable=no-member +def docker_login_to_registry(registry): + '''Logs in the Docker client to a registry. + :param Registry registry: the registry to log in to + ''' + def generate_value(value): + yield value.encode("utf-8") + + profile = Profile() + credentials, subscription_id, tenant = profile.get_login_credentials() + refresh = profile.get_refresh_credentials() + + # when token authorization is supported by the registry + # this URI should be retrieved from the authentication challenge + config = Configuration('http://172.30.168.178:44376/') + client = ServiceClient(credentials, config) + client.add_header('Content-Type', 'application/x-www-form-urlencoded') + + request = client.post('/auth/exchange') + + if isinstance(refresh, str): + content = 'resource_id={}&user_type=user&tenant={}&refresh_token={}'.format( + registry.id, tenant, refresh) + else: + content = 'resource_id={}&user_type=spn&tenant={}&username={}&password={}'.format( + registry.id, tenant, refresh[1], refresh[2]) + + # todo turn on verify after publishing! + response = client.send(request, None, generate_value(content), verify=False) + + if response.status_code not in [200]: + raise CLIError("Access to repository was denied.") + + refresh_token = loads(response.content.decode("utf-8"))["refresh_token"] + + print(registry) + + # this URL should be gotten from the RP once it supports it + service_name = registry.name + "-exp.azurecr.io" + service_name = "10.0.75.1:5000" + + call(["docker", "login", service_name, "--username", "00000000-0000-0000-0000-000000000000", "--password", refresh_token]) + def arm_deploy_template(resource_group_name, registry_name, location, diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py index f188abdb482..6e663c7567b 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py @@ -17,7 +17,8 @@ from ._utils import ( get_access_key_by_storage_account_name, get_resource_group_name_by_registry_name, - arm_deploy_template + arm_deploy_template, + docker_login_to_registry ) import azure.cli.core._logging as _logging @@ -116,6 +117,13 @@ def acr_show(registry_name, resource_group_name=None): return client.get_properties(resource_group_name, registry_name) +def acr_login(registry_name): + '''Login to a container registry through Docker. + :param str registry_name: The name of container registry + ''' + props = acr_show(registry_name) + docker_login_to_registry(props) + def acr_update_get(client, registry_name, resource_group_name=None): From b43850858dbadd0f3dcff149c5c42fd1adf662ff Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 22 Nov 2016 11:32:19 -0800 Subject: [PATCH 4/8] add the acr login command --- .../azure/cli/command_modules/acr/_utils.py | 88 ++++++++++--------- .../azure/cli/command_modules/acr/commands.py | 1 + .../azure/cli/command_modules/acr/custom.py | 7 +- 3 files changed, 50 insertions(+), 46 deletions(-) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py index d08bd1fea55..d490184ad15 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py @@ -7,13 +7,13 @@ from azure.cli.core.commands.parameters import get_resources_in_subscription from azure.cli.core._profile import Profile +from urllib.parse import urlencode, urlparse, urlunparse +import requests.api + from subprocess import call from base64 import b64encode from json import loads -from msrest.service_client import ServiceClient -from msrest import Configuration - from ._constants import ( ACR_RESOURCE_PROVIDER, ACR_RESOURCE_TYPE, @@ -36,15 +36,11 @@ def _arm_get_resource_by_name(resource_name, resource_type): elements = [item for item in result if item.name.lower() == resource_name.lower()] if len(elements) == 0: - raise CLIError( - 'No resource with type {} can be found with name: {}'.format( - resource_type, resource_name)) + raise CLIError('No resource with type {} can be found with name: {}'.format(resource_type, resource_name)) elif len(elements) == 1: return elements[0] else: - raise CLIError( - 'More than one resources with type {} are found with name: {}'.format( - resource_type, resource_name)) + raise CLIError('More than one resources with type {} are found with name: {}'.format(resource_type, resource_name)) def get_resource_group_name_by_resource_id(resource_id): '''Returns the resource group name from parsing the resource id. @@ -92,10 +88,9 @@ def get_access_key_by_storage_account_name(storage_account_name, resource_group_ client = get_storage_service_client().storage_accounts return client.list_keys(resource_group_name, storage_account_name).keys[0].value #pylint: disable=no-member - -def docker_login_to_registry(registry): +def docker_login_to_registry(registry_url): '''Logs in the Docker client to a registry. - :param Registry registry: the registry to log in to + :param str registry: the registry to log in to ''' def generate_value(value): yield value.encode("utf-8") @@ -103,37 +98,51 @@ def generate_value(value): profile = Profile() credentials, subscription_id, tenant = profile.get_login_credentials() refresh = profile.get_refresh_credentials() + base_endpoint = 'http://' + registry_url.rstrip('/'); + + challenge = requests.get(base_endpoint + '/v2/'); + if challenge.status_code not in [401] or 'WWW-Authenticate' not in challenge.headers: + raise CLIError('Registry did not issue a challenge.') - # when token authorization is supported by the registry - # this URI should be retrieved from the authentication challenge - config = Configuration('http://172.30.168.178:44376/') - client = ServiceClient(credentials, config) - client.add_header('Content-Type', 'application/x-www-form-urlencoded') + authenticate = challenge.headers['WWW-Authenticate'] + print(authenticate) - request = client.post('/auth/exchange') + tokens = authenticate.split(' ', 2) + if len(tokens) < 2 or tokens[0].lower() != 'bearer': + raise CLIError('Registry does not support AAD login.') + params = { y[0]: y[1].strip('"') for y in (x.strip().split('=', 2) for x in tokens[1].split(',')) } + if 'realm' not in params or 'service' not in params: + raise CLIError('Registry does not support AAD login.') + + authurl = urlparse(params['realm']) + authhost = urlunparse((authurl[0], authurl[1], '/oauth2/exchange', '', '', '')) + + headers = { 'Content-Type': 'application/x-www-form-urlencoded' } if isinstance(refresh, str): - content = 'resource_id={}&user_type=user&tenant={}&refresh_token={}'.format( - registry.id, tenant, refresh) + content = { + 'service': params['service'], + 'user_type': 'user', + 'tenant': tenant, + 'refresh_token': refresh + } else: - content = 'resource_id={}&user_type=spn&tenant={}&username={}&password={}'.format( - registry.id, tenant, refresh[1], refresh[2]) + content = { + 'service': params['service'], + 'user_type': 'spn', + 'tenant': tenant, + 'username': refresh[1], + 'password': refresh[2] + } - # todo turn on verify after publishing! - response = client.send(request, None, generate_value(content), verify=False) + response = requests.post(authhost, urlencode(content), headers=headers) if response.status_code not in [200]: - raise CLIError("Access to repository was denied.") + raise CLIError("Access to repository was denied. Response code: {}".format(response.status_code)) refresh_token = loads(response.content.decode("utf-8"))["refresh_token"] - print(registry) - - # this URL should be gotten from the RP once it supports it - service_name = registry.name + "-exp.azurecr.io" - service_name = "10.0.75.1:5000" - - call(["docker", "login", service_name, "--username", "00000000-0000-0000-0000-000000000000", "--password", refresh_token]) + call(["docker", "login", registry_url, "--username", "00000000-0000-0000-0000-000000000000", "--password", refresh_token]) def arm_deploy_template(resource_group_name, registry_name, @@ -157,8 +166,7 @@ def arm_deploy_template(resource_group_name, template = get_file_json(file_path) properties = DeploymentProperties(template=template, parameters=parameters, mode='incremental') - return _arm_deploy_template( - get_arm_service_client().deployments, resource_group_name, properties) + return _arm_deploy_template(get_arm_service_client().deployments, resource_group_name, properties) def _arm_deploy_template(deployments_client, resource_group_name, @@ -173,19 +181,15 @@ def _arm_deploy_template(deployments_client, if index == 0: deployment_name = ACR_RESOURCE_PROVIDER elif index > 9: # Just a number to avoid infinite loops - raise CLIError( - 'The resource group {} has too many deployments'.format(resource_group_name)) + raise CLIError('The resource group {} has too many deployments'.format(resource_group_name)) else: deployment_name = ACR_RESOURCE_PROVIDER + '_' + str(index) try: - deployments_client.validate( - resource_group_name, deployment_name, properties) - return deployments_client.create_or_update( - resource_group_name, deployment_name, properties) + deployments_client.validate(resource_group_name, deployment_name, properties) + return deployments_client.create_or_update(resource_group_name, deployment_name, properties) except: #pylint: disable=bare-except - return _arm_deploy_template( - deployments_client, resource_group_name, properties, index + 1) + return _arm_deploy_template(deployments_client, resource_group_name, properties, index + 1) def _parameters(registry_name, location, diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/commands.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/commands.py index 7e3ed543959..41148c4f53c 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/commands.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/commands.py @@ -19,6 +19,7 @@ cli_command(__name__, 'acr create', 'azure.cli.command_modules.acr.custom#acr_create', table_transformer=output_format) cli_command(__name__, 'acr delete', 'azure.cli.command_modules.acr.custom#acr_delete', table_transformer=output_format) cli_command(__name__, 'acr show', 'azure.cli.command_modules.acr.custom#acr_show', table_transformer=output_format) +cli_command(__name__, 'acr login', 'azure.cli.command_modules.acr.custom#acr_login') cli_generic_update_command(__name__, 'acr update', 'azure.cli.command_modules.acr.custom#acr_update_get', diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py index 6e663c7567b..950c0dae59a 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py @@ -117,12 +117,11 @@ def acr_show(registry_name, resource_group_name=None): return client.get_properties(resource_group_name, registry_name) -def acr_login(registry_name): +def acr_login(registry_url): '''Login to a container registry through Docker. - :param str registry_name: The name of container registry + :param str registry_url: The url of container registry ''' - props = acr_show(registry_name) - docker_login_to_registry(props) + docker_login_to_registry(registry_url) def acr_update_get(client, registry_name, From e6954ea453e3121c60197ed901d25292a2d692f6 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 22 Nov 2016 11:37:24 -0800 Subject: [PATCH 5/8] remove print in docker login utility --- .../azure-cli-acr/azure/cli/command_modules/acr/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py index d490184ad15..48033184259 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py @@ -88,6 +88,7 @@ def get_access_key_by_storage_account_name(storage_account_name, resource_group_ client = get_storage_service_client().storage_accounts return client.list_keys(resource_group_name, storage_account_name).keys[0].value #pylint: disable=no-member + def docker_login_to_registry(registry_url): '''Logs in the Docker client to a registry. :param str registry: the registry to log in to @@ -105,7 +106,6 @@ def generate_value(value): raise CLIError('Registry did not issue a challenge.') authenticate = challenge.headers['WWW-Authenticate'] - print(authenticate) tokens = authenticate.split(' ', 2) if len(tokens) < 2 or tokens[0].lower() != 'bearer': From 06df765721f172de20a38a03555872d5c7623f11 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 22 Nov 2016 11:51:04 -0800 Subject: [PATCH 6/8] fix linter errors --- .../azure/cli/core/tests/test_profile.py | 10 ++-- .../azure/cli/command_modules/acr/_utils.py | 56 ++++++++++--------- 2 files changed, 37 insertions(+), 29 deletions(-) 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 6a8528c00c5..fec4cc793bd 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 @@ -321,7 +321,7 @@ def test_get_refresh_credentials(self, mock_get_token_entry, mock_read_cred_file self.assertEqual(refresh_object, self.refresh_token1) self.assertEqual(mock_read_cred_file.call_count, 1) mock_get_token_entry.assert_called_once_with(mock.ANY, self.user1, self.tenant_id, - 'https://management.core.windows.net/') + 'https://management.core.windows.net/') self.assertEqual(mock_get_token_entry.call_count, 1) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @@ -347,7 +347,9 @@ def test_get_login_credentials_for_graph_client(self, mock_get_token, mock_read_ @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @mock.patch('azure.cli.core._profile.CredsCache.retrieve_token_entry_for_user', autospec=True) - def test_get_refresh_credentials_for_graph_client(self, mock_get_token_entry, mock_read_cred_file): + def test_get_refresh_credentials_for_graph_client(self, + mock_get_token_entry, + mock_read_cred_file): mock_read_cred_file.return_value = [Test_Profile.token_entry1] mock_get_token_entry.return_value = Test_Profile.token_entry1 #setup @@ -362,7 +364,7 @@ def test_get_refresh_credentials_for_graph_client(self, mock_get_token_entry, mo #verify mock_get_token_entry.assert_called_once_with(mock.ANY, self.user1, self.tenant_id, - 'https://graph.windows.net/') + 'https://graph.windows.net/') self.assertEqual(refresh_object, Test_Profile.refresh_token1) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) @@ -622,7 +624,7 @@ def get_auth_context(authority, **kwargs): # pylint: disable=unused-argument mock.ANY) token_entry = creds_cache.retrieve_token_entry_for_user(self.user1, self.tenant_id, - mgmt_resource) + mgmt_resource) #assert mock_open_for_write.assert_called_with(mock.ANY, 'w+') self.assertEqual(token, 'new token') diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py index 48033184259..c2fb1da6b7d 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py @@ -3,16 +3,14 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from azure.cli.core._util import CLIError -from azure.cli.core.commands.parameters import get_resources_in_subscription -from azure.cli.core._profile import Profile - from urllib.parse import urlencode, urlparse, urlunparse -import requests.api - from subprocess import call -from base64 import b64encode from json import loads +import requests + +from azure.cli.core._util import CLIError +from azure.cli.core.commands.parameters import get_resources_in_subscription +from azure.cli.core._profile import Profile from ._constants import ( ACR_RESOURCE_PROVIDER, @@ -26,7 +24,6 @@ get_acr_api_version ) - def _arm_get_resource_by_name(resource_name, resource_type): '''Returns the ARM resource in the current subscription with resource_name. :param str resource_name: The name of resource @@ -36,11 +33,15 @@ def _arm_get_resource_by_name(resource_name, resource_type): elements = [item for item in result if item.name.lower() == resource_name.lower()] if len(elements) == 0: - raise CLIError('No resource with type {} can be found with name: {}'.format(resource_type, resource_name)) + raise CLIError( + 'No resource with type {} can be found with name: {}'.format( + resource_type, resource_name)) elif len(elements) == 1: return elements[0] else: - raise CLIError('More than one resources with type {} are found with name: {}'.format(resource_type, resource_name)) + raise CLIError( + 'More than one resources with type {} are found with name: {}'.format( + resource_type, resource_name)) def get_resource_group_name_by_resource_id(resource_id): '''Returns the resource group name from parsing the resource id. @@ -93,15 +94,12 @@ def docker_login_to_registry(registry_url): '''Logs in the Docker client to a registry. :param str registry: the registry to log in to ''' - def generate_value(value): - yield value.encode("utf-8") - profile = Profile() - credentials, subscription_id, tenant = profile.get_login_credentials() + _, _, tenant = profile.get_login_credentials() refresh = profile.get_refresh_credentials() - base_endpoint = 'http://' + registry_url.rstrip('/'); + base_endpoint = 'http://' + registry_url.rstrip('/') - challenge = requests.get(base_endpoint + '/v2/'); + challenge = requests.get(base_endpoint + '/v2/') if challenge.status_code not in [401] or 'WWW-Authenticate' not in challenge.headers: raise CLIError('Registry did not issue a challenge.') @@ -111,14 +109,15 @@ def generate_value(value): if len(tokens) < 2 or tokens[0].lower() != 'bearer': raise CLIError('Registry does not support AAD login.') - params = { y[0]: y[1].strip('"') for y in (x.strip().split('=', 2) for x in tokens[1].split(',')) } + params = {y[0]: y[1].strip('"') for y in + (x.strip().split('=', 2) for x in tokens[1].split(','))} if 'realm' not in params or 'service' not in params: raise CLIError('Registry does not support AAD login.') authurl = urlparse(params['realm']) authhost = urlunparse((authurl[0], authurl[1], '/oauth2/exchange', '', '', '')) - headers = { 'Content-Type': 'application/x-www-form-urlencoded' } + headers = {'Content-Type': 'application/x-www-form-urlencoded'} if isinstance(refresh, str): content = { 'service': params['service'], @@ -138,11 +137,13 @@ def generate_value(value): response = requests.post(authhost, urlencode(content), headers=headers) if response.status_code not in [200]: - raise CLIError("Access to repository was denied. Response code: {}".format(response.status_code)) + raise CLIError( + "Access to repository was denied. Response code: {}".format(response.status_code)) refresh_token = loads(response.content.decode("utf-8"))["refresh_token"] - call(["docker", "login", registry_url, "--username", "00000000-0000-0000-0000-000000000000", "--password", refresh_token]) + call(["docker", "login", registry_url, "--username", + "00000000-0000-0000-0000-000000000000", "--password", refresh_token]) def arm_deploy_template(resource_group_name, registry_name, @@ -166,7 +167,8 @@ def arm_deploy_template(resource_group_name, template = get_file_json(file_path) properties = DeploymentProperties(template=template, parameters=parameters, mode='incremental') - return _arm_deploy_template(get_arm_service_client().deployments, resource_group_name, properties) + return _arm_deploy_template( + get_arm_service_client().deployments, resource_group_name, properties) def _arm_deploy_template(deployments_client, resource_group_name, @@ -181,15 +183,19 @@ def _arm_deploy_template(deployments_client, if index == 0: deployment_name = ACR_RESOURCE_PROVIDER elif index > 9: # Just a number to avoid infinite loops - raise CLIError('The resource group {} has too many deployments'.format(resource_group_name)) + raise CLIError( + 'The resource group {} has too many deployments'.format(resource_group_name)) else: deployment_name = ACR_RESOURCE_PROVIDER + '_' + str(index) try: - deployments_client.validate(resource_group_name, deployment_name, properties) - return deployments_client.create_or_update(resource_group_name, deployment_name, properties) + deployments_client.validate( + resource_group_name, deployment_name, properties) + return deployments_client.create_or_update( + resource_group_name, deployment_name, properties) except: #pylint: disable=bare-except - return _arm_deploy_template(deployments_client, resource_group_name, properties, index + 1) + return _arm_deploy_template( + deployments_client, resource_group_name, properties, index + 1) def _parameters(registry_name, location, From aa7f5a13d39f9a1e43fa0c2ce68e951403829438 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 28 Nov 2016 13:52:16 -0800 Subject: [PATCH 7/8] switch protocol to https and tweak error message --- .../azure-cli-acr/azure/cli/command_modules/acr/_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py index c2fb1da6b7d..f62c35ab3a9 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_utils.py @@ -97,7 +97,7 @@ def docker_login_to_registry(registry_url): profile = Profile() _, _, tenant = profile.get_login_credentials() refresh = profile.get_refresh_credentials() - base_endpoint = 'http://' + registry_url.rstrip('/') + base_endpoint = 'https://' + registry_url.rstrip('/') challenge = requests.get(base_endpoint + '/v2/') if challenge.status_code not in [401] or 'WWW-Authenticate' not in challenge.headers: @@ -138,7 +138,7 @@ def docker_login_to_registry(registry_url): if response.status_code not in [200]: raise CLIError( - "Access to repository was denied. Response code: {}".format(response.status_code)) + "Access to registry was denied. Response code: {}".format(response.status_code)) refresh_token = loads(response.content.decode("utf-8"))["refresh_token"] From fa95c751b97cf065c8865958dacf30b794b9ce84 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Mon, 28 Nov 2016 13:58:01 -0800 Subject: [PATCH 8/8] update help and readme for az login --- src/command_modules/azure-cli-acr/README.rst | 14 ++++++++++++++ .../azure/cli/command_modules/acr/_help.py | 17 +++++++++-------- .../azure/cli/command_modules/acr/_params.py | 4 ++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/command_modules/azure-cli-acr/README.rst b/src/command_modules/azure-cli-acr/README.rst index 7f090ed256d..bd32643d6b4 100644 --- a/src/command_modules/azure-cli-acr/README.rst +++ b/src/command_modules/azure-cli-acr/README.rst @@ -152,6 +152,20 @@ List repositories in a given container registry List repositories in a given container registry with credentials az acr repository list -n myRegistry -u myUsername -p myPassword +Login to a container registry +------------- +:: + + Command + az acr login: Login to a container registry through Docker. + + Arguments + --registry-url -u [Required]: The login server of the container registry. + + Examples + Login to a container registry + az acr login -u myregistry.azurecr.io + Show tags of a given repository in a given container registry ------------- :: diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py index c72c8d4441e..7599c76db71 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_help.py @@ -44,14 +44,6 @@ az acr create -n myRegistry -g myResourceGroup -l southcentralus --storage-account-name myStorageAccount """ -helps['acr login'] = """ - type: command - examples: - - name: Log into a registry - text: - az acr login -n myRegistry - """ - helps['acr update'] = """ type: command short-summary: Updates a container registry. @@ -67,6 +59,15 @@ az acr update -n myRegistry --admin-enabled true """ +helps['acr login'] = """ + type: command + short-summary: Login to a container registry through Docker. + examples: + - name: Login to a registry + text: + az acr login -u myregistry.azurecr.io + """ + helps['acr repository list'] = """ type: command examples: diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_params.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_params.py index e35e85ffbfd..2090f768466 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_params.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_params.py @@ -41,6 +41,10 @@ options_list=('--password', '-p'), help='The password used to log into a container registry') +register_cli_argument('acr', 'registry_url', + options_list=('--registry-url', '-u'), + help='The login server of the container registry') + register_cli_argument('acr create', 'registry_name', completer=None) register_cli_argument('acr create', 'resource_group_name', validator=validate_resource_group_name)