Skip to content

[REST] Fix #14152: az rest: Accept ARM URLs without subscription ID #14370

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 5 commits into from
Jul 15, 2020
Merged

[REST] Fix #14152: az rest: Accept ARM URLs without subscription ID #14370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f13471
Accept ARM URL without subscription ID
Jul 15, 2020
b7cb921
Add comments
Jul 15, 2020
1857df2
Suppress CredScan
Jul 15, 2020
07a142a
Update src/azure-cli-core/azure/cli/core/util.py
Jul 15, 2020
9ce02ac
style
Jul 15, 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
5 changes: 2 additions & 3 deletions scripts/ci/credscan/CredScanSuppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,7 @@
},
{
"file": [
"src\\azure-cli\\azure\\cli\\command_modules\\resource\\tests\\latest\\test-largesize-parameters.json",
"src\\azure-cli\\azure\\cli\\command_modules\\resource\\tests\\latest\\recordings\\test_rest.yaml"
"src\\azure-cli\\azure\\cli\\command_modules\\resource\\tests\\latest\\test-largesize-parameters.json"
],
"_justification": "random name and value"
},
Expand All @@ -382,7 +381,7 @@
},
{
"file": [
"src/azure-cli/azure/cli/command_modules/util/tests/latest/recordings/test_rest.yaml"
"src/azure-cli/azure/cli/command_modules/util/tests/latest/recordings/test_rest_storage.yaml"
],
"_justification": "one-time sas token"
}
Expand Down
22 changes: 22 additions & 0 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 @@ -301,6 +301,28 @@ def test_send_raw_requests(self, send_mock, get_raw_token_mock):
request = send_mock.call_args.args[1]
self.assertDictEqual(dict(request.headers), expected_header_with_auth)

# Test ARM Subscriptions - List
# https://docs.microsoft.com/en-us/rest/api/resources/subscriptions/list
# /subscriptions?api-version=2020-01-01
send_raw_request(cli_ctx, 'GET', '/subscriptions?api-version=2020-01-01', body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, test_arm_endpoint.rstrip('/') + '/subscriptions?api-version=2020-01-01')
self.assertDictEqual(dict(request.headers), expected_header_with_auth)

# Test ARM Tenants - List
# https://docs.microsoft.com/en-us/rest/api/resources/tenants/list
# /tenants?api-version=2020-01-01
send_raw_request(cli_ctx, 'GET', '/tenants?api-version=2020-01-01', body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, test_arm_endpoint.rstrip('/') + '/tenants?api-version=2020-01-01')
self.assertDictEqual(dict(request.headers), expected_header_with_auth)
Comment on lines +304 to +324

@jiasli jiasli Jul 15, 2020

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.

Unit tests are added for APIs without subscription ID.


# 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,
Expand Down
30 changes: 19 additions & 11 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 @@ -735,16 +735,17 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
# Replace common tokens with real values. It is for smooth experience if users copy and paste the url from
# Azure Rest API doc
from azure.cli.core._profile import Profile
profile = Profile()
profile = Profile(cli_ctx=cli_ctx)

@jiasli jiasli Jul 15, 2020

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.

Pass the existing cli_ctx to Profile. Otherwise, Profile will create a new AzCli which is unnecessary.

azure-cli/src/azure-cli-core/azure/cli/core/_profile.py

Line 152 in fcc93da

self.cli_ctx = cli_ctx or get_default_cli()

if '{subscriptionId}' in url:
url = url.replace('{subscriptionId}', cli_ctx.data['subscription_id'] or profile.get_subscription_id())

# Prepare the Bearer token for `Authorization` header
if not skip_authorization_header and url.lower().startswith('https://'):
# Prepare `resource`
# Prepare `resource` for `get_raw_token`
if not resource:
# 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
# If url starts with ARM endpoint, like `https://management.azure.com/`,
# use `active_directory_resource_id` for resource, like `https://management.core.windows.net/`.
# This follows the same behavior as `azure.cli.core.commands.client_factory._get_mgmt_service_client`
if url.lower().startswith(endpoints.resource_manager.rstrip('/')):
resource = endpoints.active_directory_resource_id
else:
Expand All @@ -758,11 +759,15 @@ def send_raw_request(cli_ctx, method, url, headers=None, uri_parameters=None, #
resource = value
break
if resource:
# 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.
# Prepare `subscription` for `get_raw_token`
# If this is an ARM request, try to extract subscription ID from the URL.
# But there are APIs which don't require subscription ID, like /subscriptions, /tenants
# TODO: In the future when multi-tenant subscription is supported, we won't be able to uniquely identify
# the token from subscription anymore.
token_subscription = None
if url.lower().startswith(endpoints.resource_manager.rstrip('/')):
token_subscription = _extract_subscription_id(url)
if token_subscription:
Comment thread
jsntcy marked this conversation as resolved.
logger.debug('Retrieving token for resource %s, subscription %s', resource, token_subscription)
token_info, _, _ = profile.get_raw_token(resource, subscription=token_subscription)
else:
Expand Down Expand Up @@ -804,9 +809,12 @@ 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]
if match:
subscription_id = match.groups()[0]
logger.debug('Found subscription ID %s in the URL %s', subscription_id, url)
return subscription_id
logger.debug('No subscription ID specified in the URL %s', url)
return None
Comment on lines +816 to +817

@jiasli jiasli Jul 15, 2020

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.

Stop raising an exception but return None instead.



def _log_request(request):
Expand Down
Loading