Skip to content

{REST} Only extract subscription ID for ARM requests #13782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jiasli merged 3 commits into from
Jun 8, 2020
Merged

{REST} Only extract subscription ID for ARM requests #13782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79176d8
Only extract subscription ID for ARM requests
Jun 2, 2020
3191131
Update util.py
Jun 5, 2020
934c93d
Update CODEOWNERS
Jun 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/tools/ @haroldrandom @fengzhou-msft
/scripts/ @haroldrandom @fengzhou-msft
/src/azure-cli-testsdk/ @bim-msft @MyronFanQiu @haroldrandom
/src/azure-cli-core/ @jiasli @Juliehzl @haroldrandom @fengzhou-msft @qianwens @arrownj
/src/azure-cli-core/ @jiasli @Juliehzl @haroldrandom @fengzhou-msft @qianwens @arrownj @MyronFanQiu
/src/azure-cli/azure/cli/command_modules/acr/ @djyou @fengzhou-msft
/src/azure-cli/azure/cli/command_modules/acs/ @rjtsdl @arrownj
/src/azure-cli/azure/cli/command_modules/advisor/ @Prasanna-Padmanabhan
Expand Down
47 changes: 30 additions & 17 deletions src/azure-cli-core/azure/cli/core/tests/test_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ def test_send_raw_requests(self, send_mock, get_raw_token_mock):
}
test_arm_active_directory_resource_id = 'https://management.core.windows.net/'
test_arm_endpoint = 'https://management.azure.com/'
arm_resource_id = '/subscriptions/01/resourcegroups/02?api-version=2019-07-01'
subscription_id = '00000001-0000-0000-0000-000000000000'
arm_resource_id = '/subscriptions/{}/resourcegroups/02?api-version=2019-07-01'.format(subscription_id)
full_arm_rest_url = test_arm_endpoint.rstrip('/') + arm_resource_id
test_body = '{"b1": "v1"}'

Expand Down Expand Up @@ -296,49 +297,61 @@ def test_send_raw_requests(self, send_mock, get_raw_token_mock):
body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=None)
get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=subscription_id)
request = send_mock.call_args.args[1]
self.assertDictEqual(dict(request.headers), expected_header_with_auth)

# Test ARM resource ID /subscriptions/01/resourcegroups/02?api-version=2019-07-01
# Test ARM resource ID
# /subscriptions/00000001-0000-0000-0000-000000000000/resourcegroups/02?api-version=2019-07-01
send_raw_request(cli_ctx, 'GET', arm_resource_id, body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=None)
get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=subscription_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, 'https://management.azure.com/subscriptions/01/resourcegroups/02?api-version=2019-07-01')
self.assertEqual(request.url, full_arm_rest_url)
self.assertDictEqual(dict(request.headers), expected_header_with_auth)

# Test full ARM URL https://management.azure.com/subscriptions/01/resourcegroups/02?api-version=2019-07-01
# Test full ARM URL
# https://management.azure.com/subscriptions/00000001-0000-0000-0000-000000000000/resourcegroups/02?api-version=2019-07-01
send_raw_request(cli_ctx, 'GET', full_arm_rest_url)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=None)
get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=subscription_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, 'https://management.azure.com/subscriptions/01/resourcegroups/02?api-version=2019-07-01')
self.assertEqual(request.url, full_arm_rest_url)

# Test full ARM URL with port https://management.azure.com:443/subscriptions/01/resourcegroups/02?api-version=2019-07-01
# Test full ARM URL with port
# https://management.azure.com:443/subscriptions/00000001-0000-0000-0000-000000000000/resourcegroups/02?api-version=2019-07-01
test_arm_endpoint_with_port = 'https://management.azure.com:443/'
full_arm_rest_url_with_port = test_arm_endpoint_with_port.rstrip('/') + arm_resource_id
send_raw_request(cli_ctx, 'GET', full_arm_rest_url_with_port)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=None)
get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=subscription_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, 'https://management.azure.com:443/subscriptions/01/resourcegroups/02?api-version=2019-07-01')
self.assertEqual(request.url, 'https://management.azure.com:443/subscriptions/00000001-0000-0000-0000-000000000000/resourcegroups/02?api-version=2019-07-01')

# Test non-ARM API, such as MS Graph API https://graph.microsoft.com/beta/appRoleAssignments/01
send_raw_request(cli_ctx, 'PATCH', 'https://graph.microsoft.com/beta/appRoleAssignments/01',
body=test_body, generated_client_request_id_name=None)
# Test non-ARM APIs

get_raw_token_mock.assert_called_with(mock.ANY, 'https://graph.microsoft.com/', subscription=None)
# Test AD Graph API https://graph.windows.net/
url = 'https://graph.windows.net/00000002-0000-0000-0000-000000000000/applications/00000003-0000-0000-0000-000000000000?api-version=1.6'
send_raw_request(cli_ctx, 'PATCH', url, body=test_body, generated_client_request_id_name=None)
get_raw_token_mock.assert_called_with(mock.ANY, 'https://graph.windows.net/')
request = send_mock.call_args.args[1]
self.assertEqual(request.method, 'PATCH')
self.assertEqual(request.url, 'https://graph.microsoft.com/beta/appRoleAssignments/01')
self.assertEqual(request.url, url)

# Test MS Graph API https://graph.microsoft.com/beta/appRoleAssignments/01
url = 'https://graph.microsoft.com/beta/appRoleAssignments/01'
send_raw_request(cli_ctx, 'PATCH', url, body=test_body, generated_client_request_id_name=None)
get_raw_token_mock.assert_called_with(mock.ANY, 'https://graph.microsoft.com/')
request = send_mock.call_args.args[1]
self.assertEqual(request.method, 'PATCH')
self.assertEqual(request.url, url)

# Test custom case-insensitive User-Agent
with mock.patch.dict('os.environ', {'AZURE_HTTP_USER_AGENT': "env-ua"}):
send_raw_request(cli_ctx, 'GET', full_arm_rest_url, headers={'user-agent=ARG-UA'})

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=None)
get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id, subscription=subscription_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.headers['User-Agent'], get_az_user_agent() + ' env-ua ARG-UA')

Expand Down
37 changes: 23 additions & 14 deletions src/azure-cli-core/azure/cli/core/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,11 +726,12 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
result[key] = value
uri_parameters = result or None

endpoints = cli_ctx.cloud.endpoints
# If url is an ARM resource ID, like /subscriptions/xxx/resourcegroups/xxx?api-version=2019-07-01,
# default to Azure Resource Manager.
# https://management.azure.com/ + subscriptions/xxx/resourcegroups/xxx?api-version=2019-07-01
# https://management.azure.com + /subscriptions/xxx/resourcegroups/xxx?api-version=2019-07-01
if '://' not in url:
url = cli_ctx.cloud.endpoints.resource_manager + url.lstrip('/')
url = endpoints.resource_manager.rstrip('/') + url

@mmyyrroonn mmyyrroonn Jun 4, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this change necessary?

@jiasli jiasli Jun 5, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Only for consistency and keep the resource ID intact.


# Replace common tokens with real values. It is for smooth experience if users copy and paste the url from
# Azure Rest API doc
Expand All @@ -739,18 +740,9 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
if '{subscriptionId}' in url:
url = url.replace('{subscriptionId}', cli_ctx.data['subscription_id'] or profile.get_subscription_id())

token_subscription = None
_subscription_regexes = [re.compile('https://management.azure.com/subscriptions/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})'),
re.compile('https://graph.windows.net/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})')]
for regex in _subscription_regexes:
match = regex.match(url)
if match:
token_subscription = match.groups()[0]
logger.debug('Retrieve token from subscription %s', token_subscription)

if not skip_authorization_header and url.lower().startswith('https://'):
# Prepare `resource`
if not resource:
endpoints = cli_ctx.cloud.endpoints
# If url starts with ARM endpoint, like https://management.azure.com/,
# use active_directory_resource_id for resource.
# This follows the same behavior as azure.cli.core.commands.client_factory._get_mgmt_service_client
Expand All @@ -767,8 +759,16 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
resource = value
break
if resource:
token_info, _, _ = profile.get_raw_token(resource, subscription=token_subscription)
logger.debug('Retrievd AAD token for resource: %s', resource or 'ARM')
# If this is an ARM request, extract subscription ID from the URL.
# In the future when multi-tenant subscription is supported, we won't be able to uniquely identity the token
# from subscription anymore.
if url.lower().startswith(endpoints.resource_manager.rstrip('/')):
token_subscription = _extract_subscription_id(url)
logger.debug('Retrieving token for resource %s, subscription %s', resource, token_subscription)
token_info, _, _ = profile.get_raw_token(resource, subscription=token_subscription)
else:
logger.debug('Retrieving token for resource %s', resource)
token_info, _, _ = profile.get_raw_token(resource)
token_type, token, _ = token_info
headers = headers or {}
headers['Authorization'] = '{} {}'.format(token_type, token)
Expand Down Expand Up @@ -801,6 +801,15 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
return r


def _extract_subscription_id(url):
"""Extract the subscription ID from an ARM request URL."""
subscription_regex = '/subscriptions/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})'
match = re.search(subscription_regex, url, re.IGNORECASE)
if not match:
raise CLIError('No subscription ID specified in the URL')
return match.groups()[0]


def _log_request(request):
"""Log a client request. Copied from msrest
https://github.com/Azure/msrest-for-python/blob/3653d29fc44da408898b07c710290a83d196b777/msrest/http_logger.py#L39
Expand Down