Skip to content
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

[Core] Add workaround for cross-tenant authentication with Track 2 SDKs #16797

Merged
merged 4 commits into from
Apr 9, 2021
Merged

[Core] Add workaround for cross-tenant authentication with Track 2 SDKs #16797

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88738f2
cross-tenant authentication
jiasli Feb 3, 2021
b6cedae
Skip get_all_tokens for managed identity
jiasli Feb 22, 2021
b79feff
Merge branch 'dev' into cross-tenant
jiasli Mar 1, 2021
6071c37
Update src/azure-cli-core/azure/cli/core/commands/client_factory.py
jiasli Apr 6, 2021
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
21 changes: 12 additions & 9 deletions src/azure-cli-core/azure/cli/core/adal_authentication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def _get_token(self, sdk_resource=None):
"""
external_tenant_tokens = None
try:
scheme, token, full_token = self._token_retriever(sdk_resource)
scheme, token, token_entry = self._token_retriever(sdk_resource)
Copy link
Member

Choose a reason for hiding this comment

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

Is it just a rename operation? Does the object structure change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. No.

if self._external_tenant_token_retriever:
external_tenant_tokens = self._external_tenant_token_retriever(sdk_resource)
except CLIError as err:
Expand Down Expand Up @@ -67,17 +67,20 @@ def _get_token(self, sdk_resource=None):
except requests.exceptions.ConnectionError as err:
raise CLIError('Please ensure you have network connection. Error detail: ' + str(err))

return scheme, token, full_token, external_tenant_tokens
# scheme: str. The token scheme. Should always be 'Bearer'.
# token: str. The raw access token.
# token_entry: dict. The full token entry.
# external_tenant_tokens: [(scheme: str, token: str, token_entry: dict), ...]
return scheme, token, token_entry, external_tenant_tokens

def get_all_tokens(self, *scopes):
scheme, token, full_token, external_tenant_tokens = self._get_token(_try_scopes_to_resource(scopes))
return scheme, token, full_token, external_tenant_tokens
return self._get_token(_try_scopes_to_resource(scopes))

# This method is exposed for Azure Core.
def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument
logger.debug("AdalAuthentication.get_token invoked by Track 2 SDK with scopes=%s", scopes)

_, token, full_token, _ = self._get_token(_try_scopes_to_resource(scopes))
_, token, token_entry, _ = self._get_token(_try_scopes_to_resource(scopes))

# NEVER use expiresIn (expires_in) as the token is cached and expiresIn will be already out-of date
# when being retrieved.
Expand Down Expand Up @@ -107,10 +110,10 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument
# "_clientId": "22800c35-46c2-4210-b8a7-d8c3ec3b526f",
# "_authority": "https://login.microsoftonline.com/54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"
# }
if 'expiresOn' in full_token:
if 'expiresOn' in token_entry:
import datetime
expires_on_timestamp = int(_timestamp(
datetime.datetime.strptime(full_token['expiresOn'], '%Y-%m-%d %H:%M:%S.%f')))
datetime.datetime.strptime(token_entry['expiresOn'], '%Y-%m-%d %H:%M:%S.%f')))
return AccessToken(token, expires_on_timestamp)

# Cloud Shell (Managed Identity) token entry sample:
Expand All @@ -123,8 +126,8 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument
# "resource": "https://management.core.windows.net/",
# "token_type": "Bearer"
# }
if 'expires_on' in full_token:
return AccessToken(token, int(full_token['expires_on']))
if 'expires_on' in token_entry:
return AccessToken(token, int(token_entry['expires_on']))

from azure.cli.core.azclierror import CLIInternalError
raise CLIInternalError("No expiresOn or expires_on is available in the token entry.")
Expand Down
13 changes: 13 additions & 0 deletions src/azure-cli-core/azure/cli/core/commands/client_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,19 @@ def _get_mgmt_service_client(cli_ctx,
client_kwargs.update(_prepare_client_kwargs_track2(cli_ctx))
client_kwargs['credential_scopes'] = resource_to_scopes(resource)

# Track 2 currently lacks the ability to take external credentials.
# https://github.com/Azure/azure-sdk-for-python/issues/8313
# As a temporary workaround, manually add external tokens to 'x-ms-authorization-auxiliary' header.
# https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/authenticate-multi-tenant
if aux_subscriptions or aux_tenants:
_, _, _, external_tenant_tokens = cred.get_all_tokens(*resource_to_scopes(resource))
Copy link
Member

Choose a reason for hiding this comment

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

What is *

Copy link
Member Author

@jiasli jiasli Apr 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

See More on Defining Functions and Unpacking Argument Lists.

jiasli marked this conversation as resolved.
Show resolved Hide resolved

# external_tenant_tokens can be [] if no external tenant is involved.
if external_tenant_tokens:
# Hard-code scheme to 'Bearer' as _BearerTokenCredentialPolicyBase._update_headers does.
client_kwargs['headers']['x-ms-authorization-auxiliary'] = \
', '.join("Bearer {}".format(t[1]) for t in external_tenant_tokens)
Copy link
Member

Choose a reason for hiding this comment

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

Can both , and ; be used as the delimiter for each token?

Copy link
Member Author

Choose a reason for hiding this comment

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

I simply don't know. We need to do some test.

The exiting code

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

Line 142 in d07ee66

aux_tokens = ';'.join(['{} {}'.format(scheme2, tokens2) for scheme2, tokens2, _ in external_tenant_tokens])

contradicts the document Authenticate requests across tenants

Bearer <auxiliary-token1>, EncryptedBearer <auxiliary-token2>, Bearer <auxiliary-token3>

That's why this PR is still marked as a draft. 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

According to my experiment of ARM deployment, the correct delimiter should be , .
When the delimiter is ;, the service returns the following error:

{"error":{"code":"InvalidAuxiliaryTokens","message":"Authentication failed for auxiliary token: 'The auxiliary tokens should have proper format \"'scheme' 'value'\". The '1' auxiliary token(s) were not found in proper format. Invalid tokens are in the response header.'"}}

Copy link
Member Author

Choose a reason for hiding this comment

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

True! Also verified with:

> az rest --url "https://management.azure.com/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourcegroups?api-version=2020-10-01" --headers "x-ms-authorization-auxiliary=Bearer token1; Bearer token2" --verbose
Request URL: 'https://management.azure.com/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourcegroups?api-version=2020-10-01'
Request method: 'GET'
Request headers:
    'Authorization': 'Bearer eyJ0eXAiOiJKV...'
    'x-ms-authorization-auxiliary': 'Bearer token1; Bearer token2'    
    ...

Response status: 401
Response headers:
    'WWW-Authenticate': 'Bearer authorization_uri="https://login.windows.net/54826b22-38d6-4fb2-bad9-b7b93a3e9c5a", error="invalid_token", error_description="Invalid auxiliary tokens \'Bearer token1; Bearer token2\'."'
    ...
Response content:
{"error":{"code":"InvalidAuxiliaryTokens","message":"Authentication failed for auxiliary token: 'The auxiliary tokens should have proper format \"'scheme' 'value'\". The '1' auxiliary token(s) were not found in proper format. Invalid tokens are in the response header.'"}}
Unauthorized({"error":{"code":"InvalidAuxiliaryTokens","message":"Authentication failed for auxiliary token: 'The auxiliary tokens should have proper format \"'scheme' 'value'\". The '1' auxiliary token(s) were not found in proper format. Invalid tokens are in the response header.'"}})

This means the doc is right and the current CLI code is wrong.


if subscription_bound:
client = client_type(cred, subscription_id, **client_kwargs)
else:
Expand Down
21 changes: 1 addition & 20 deletions src/azure-cli/azure/cli/command_modules/vm/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3281,31 +3281,13 @@ def create_image_version(cmd, resource_group_name, gallery_name, gallery_image_n
# print(target_regions)
from msrestazure.tools import resource_id, is_valid_resource_id
from azure.cli.core.commands.client_factory import get_subscription_id
from azure.cli.core._profile import Profile

ImageVersionPublishingProfile, GalleryArtifactSource, ManagedArtifact, ImageVersion, TargetRegion = cmd.get_models(
'GalleryImageVersionPublishingProfile', 'GalleryArtifactSource', 'ManagedArtifact', 'GalleryImageVersion',
'TargetRegion')
aux_subscriptions = _get_image_version_aux_subscription(managed_image, os_snapshot, data_snapshots)
client = _compute_client_factory(cmd.cli_ctx, aux_subscriptions=aux_subscriptions)

# Auxiliary tokens, pass it to init or operation
external_bearer_token = None
if aux_subscriptions:
profile = Profile(cli_ctx=cmd.cli_ctx)
resource = cmd.cli_ctx.cloud.endpoints.active_directory_resource_id
cred, _, _ = profile.get_login_credentials(resource=resource,
aux_subscriptions=aux_subscriptions)
_, _, _, external_tokens = cred.get_all_tokens('https://management.azure.com/.default')
Copy link
Member Author

Choose a reason for hiding this comment

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

By moving this to core, it now works for sovereign cloud (solves #15750 (comment)).

if external_tokens:
external_token = external_tokens[0]
if len(external_token) >= 2:
external_bearer_token = external_token[0] + ' ' + external_token[1]
else:
logger.warning('Getting external tokens failed.')
else:
logger.warning('Getting external tokens failed.')

location = location or _get_resource_group_location(cmd.cli_ctx, resource_group_name)
end_of_life_date = fix_gallery_image_date_info(end_of_life_date)
if managed_image and not is_valid_resource_id(managed_image):
Expand Down Expand Up @@ -3362,8 +3344,7 @@ def create_image_version(cmd, resource_group_name, gallery_name, gallery_image_n
gallery_name=gallery_name,
gallery_image_name=gallery_image_name,
gallery_image_version_name=gallery_image_version,
gallery_image_version=image_version,
headers={'x-ms-authorization-auxiliary': external_bearer_token}
Copy link
Member

Choose a reason for hiding this comment

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

You deleted lots of code in vm/custom.py and didn't add anything. How are external tokens handled now? Is another PR required to fix it?

Copy link
Member Author

Choose a reason for hiding this comment

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

You deleted lots of code in vm/custom.py and didn't add anything.

The logic is not deleted, but moved to _get_mgmt_service_client.

How are external tokens handled now?

External tokens are handled automatically when aux_subscriptions is provided to _get_mgmt_service_client.

client = _compute_client_factory(cmd.cli_ctx, aux_subscriptions=aux_subscriptions)

Is another PR required to fix it?

It already works. No other PR needed.

gallery_image_version=image_version
)


Expand Down