-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Core] PREVIEW: Support Web Account Manager (WAM) login on Windows #23828
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.
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
Conversation
| "flow with `az login --use-device-code`.", | ||
| self._msal_app.authority.authorization_endpoint) | ||
| elif ui == 'broker': | ||
| logger.warning("Please select the account you want to log in with.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to refine the warning message.
|
|
||
| return Identity(*args, encrypt=encrypt, use_msal_http_cache=use_msal_http_cache, **kwargs) | ||
| # PREVIEW: On Windows, use core.allow_broker=true to use broker (WAM) for authentication. | ||
| allow_broker = cli_ctx.config.getboolean('core', 'allow_broker', fallback=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to align the config name with Azure PowerShell. @Jacekey23 @chasewilson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, Azure PowerShell will release separate versions for WAM, instead of using a config option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, WAM will be enabled by default in year 2023. By then, should all linux users manually set core.allow_broker=False to avoid failure? Can we add --allow-broker parameter for az login to distinguish WAM and traditional browser way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we don't have the users manually adjust anything to avoid failure. That's not a great experience if the default login is an error. Is it possible to set the configuration appropriately from an environment variable? eg in PowerShell $IsLinux or $IsMacOS.
As for if we want to add a parameter like --allow-broker, we kind of run into the same issue. If we want to have the broker to be the default flow, we would need to set --allow-broker as a default parameter and only if it's Windows. Meaning we are dealing with the same issue.
Open to more discussion around this as it is a big change to how we handle the flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to align the config name with Azure PowerShell. @Jacekey23 @chasewilson
Could you point me to a complete list of our configurations? Even if it's just the code location where the input is managed. I'd just like to look at all of our configurations and make sure we're following a good format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, WAM will be enabled by default in year 2023. By then, should all linux users manually set
core.allow_broker=Falseto avoid failure? Can we add--allow-brokerparameter foraz loginto distinguish WAM and traditional browser way?
Ideally, we don't have the users manually adjust anything to avoid failure. That's not a great experience if the default login is an error. Is it possible to set the configuration appropriately from an environment variable? eg in PowerShell
$IsLinuxor$IsMacOS.As for if we want to add a parameter like
--allow-broker, we kind of run into the same issue. If we want to have the broker to be the default flow, we would need to set--allow-brokeras a default parameter and only if it's Windows. Meaning we are dealing with the same issue.Open to more discussion around this as it is a big change to how we handle the flow.
Those are all good points. MSAL Python foresaw this difficulty, so, one of the design goal is to save app developers from needing to conditionally enable/disable broker behavior. Our model is, the app developer only needs to opt-in when their app meets those prerequisites, and then MSAL Python will take care of those conditions such as the platforms. By the way, that is also why the parameter was named "allow_broker" rather than "enable_broker": after the app allows it, the MSAL will decide whether to actually use it. For example, currently MSAL Python effectively ignores allow_broker when current app is running on non-Windows. (You can try test Azure CLI dev branch on Linux to see how allow_broker=True is a no-op.)
In particular, please do not add "--allow-broker" as a parameter to "az login". The reason being, even though the broker UI is only visible/tangible during an "az login", the broker mechanism is holistic. It will also affect silent token refresh. So, it has to be a global toggle switch.
|
WAM support |
rayluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with one more suggestion below.
Related command
az loginDescription
Close #21201
Require AzureAD/microsoft-authentication-library-for-python#415
Rework #22774
Testing Guide
On Windows, users can opt in WAM login using:
Then run
Instead of launching a web browser, an account selector will be shown to ask the user to select a Windows account:
History Notes
[Core] PREVIEW: Support Web Account Manager (WAM) login on Windows. To opt in, run
az config set core.allow_broker=trueAdditional information
WAM will be enabled by default in year 2023.