Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Sep 4, 2019

Fix #8840: Convert tenant domain name to GUID if it is not.

Copy link
Contributor

@yugangw-msft yugangw-msft Sep 5, 2019

Choose a reason for hiding this comment

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

Please handle the case when a proxy is involved, check out the code.
Also like I mentioned endpoint like login.microsoftonline.com should come from current cloud configuration, again code is here.
Last, can we do the conversion during az login which would benefit other commands ?

Choose a reason for hiding this comment

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

+1 for doing the conversion at 'az login' level if possible.

Copy link
Member Author

@jiasli jiasli Sep 16, 2019

Choose a reason for hiding this comment

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

I have moved the domain resolution logic to az login. @yugangw-msft, please kindly review. Thanks!
az login --service-principal -u {} -p {} -t {}.onmicrosoft.com shows warning 'Resolve tenant domain name %s to GUID %s'.
az keyvault create --name {} -g {} should be good.

uuid.UUID(guid)
return True
except ValueError:
pass
Copy link
Contributor

@yugangw-msft yugangw-msft Sep 16, 2019

Choose a reason for hiding this comment

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

nit: you can return False here

Copy link
Member Author

@jiasli jiasli Sep 16, 2019

Choose a reason for hiding this comment

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

Good point! All _is_guids are fixed.

@yugangw-msft
Copy link
Contributor

This is a user facing change, please add a releasing note

@jiasli jiasli requested a review from yugangw-msft September 16, 2019 14:10
Copy link

@rajarshi-singh rajarshi-singh left a comment

Choose a reason for hiding this comment

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

Thanks again for taking care of this. :)

if not _is_guid(namespace.tenant):
import requests
active_directory_endpoint = cmd.cli_ctx.cloud.endpoints.active_directory
url = '{}/{}/.well-known/openid-configuration'.format(active_directory_endpoint, namespace.tenant)
Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants