login: use browser for interactive login #6593
Conversation
yugangw-msft
force-pushed
the
superlogin3
branch
from
541dccb
to
028443e
Compare
yugangw-msft
force-pushed
the
superlogin3
branch
from
078c6f4
to
dedce32
Compare
Codecov Report
@@ Coverage Diff @@
## dev #6593 +/- ##
===================================
Coverage 0% 0%
===================================
Files 11 11
Lines 145 145
Branches 11 11
===================================
Misses 145 145Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Mostly questions. Would like @troydai to take a look at the web server stuff.
| 2.0.39 | ||
| ++++++ | ||
| * Minor fixes | ||
| * authentication: support authorization code flow for interactive login |
There was a problem hiding this comment.
I would make this more clear:
* `login`: Improve flow when using interactive login. The browser will not be opened for you.
Question, does the auth code auto-populate the field?
There was a problem hiding this comment.
this is a new flow and device code is no longer involved
| # pylint: disable=line-too-long | ||
|
|
||
| # backup greeting pages if official ones from "docs.microsoft.com" are unavailable | ||
| ok_html = ('<!DOCTYPE html>' |
There was a problem hiding this comment.
I'd recommend extracting this out into a separate HTML file for easier maintenance.
| # handle 2 things: | ||
| # a. On OSX sierra, 'python -m webbrowser -t <url>' emits out "execution error: <url> doesn't | ||
| # understand the "open location" message" | ||
| # b. Python 2.x can't sniff out the default browser |
There was a problem hiding this comment.
This code is addressing these issues, or these need to be addressed?
There was a problem hiding this comment.
It is addressing these issues. The code was moved over from the appservice module since core module also needs to launch a browser now.
| raise CLIError("usage error: '--identity' is not applicable with other arguments") | ||
| if any([password, service_principal, username, identity]) and use_device_code: | ||
| raise CLIError("usage error: '--use-device-code' is not applicable with other arguments") |
There was a problem hiding this comment.
Why do we need a new parameter for this? If you log in interactively why wouldn't you always want to use the device code?
There was a problem hiding this comment.
it is just for safeness that in case people still like to use the old device code. I have updated the argument description of --use-device-code to clarify this.
yugangw-msft
force-pushed
the
superlogin3
branch
from
cdac916
to
6638707
Compare
yugangw-msft
force-pushed
the
superlogin3
branch
from
6638707
to
8c3da6a
Compare
| @@ -49,7 +49,7 @@ def load_arguments(self, command): | |||
| c.argument('identity', options_list=('-i', '--identity'), action='store_true', help="Log in using the Virtual Machine's identity", arg_group='Managed Service Identity') | |||
| c.argument('identity_port', type=int, help="the port to retrieve tokens for login. Default: 50342", arg_group='Managed Service Identity') | |||
| c.argument('use_device_code', action='store_true', | |||
| help='Login through device code, useful in mobile devices or when browsers are unavailable, e.g. in remote SSH sessions or Cloud Shell') | |||
| help="Use CLI's old authentication flow based on device dode. CLI will also use this if it can't launch a browser in your behalf, e.g. in remote SSH or Cloud Shell") | |||
There was a problem hiding this comment.
typo: "device code" instead of "device dode".
By default, if browser is available, CLI will use it for login. No more device code copy & paste.
The device code login is still available, through
az login --use-device-codein case desktop is unavailable say SSH session, cloud shell, etc. CLI will also pick this up if it can't launch the browser in such situations.this will address #5413
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.