[Core] Add announcement for WAM-based login#25416
Conversation
…te and opt-in instructions
|
@jiasli for awareness |
| <p>You can close this window, or we will redirect you to the <a href="https://docs.microsoft.com/cli/azure/">Azure CLI documentation</a> in 10 seconds.</p> | ||
| <h3>You have logged into Microsoft Azure!</h3> | ||
| <p>You can close this window, or we will redirect you to the <a href="https://docs.microsoft.com/cli/azure/">Azure CLI documentation</a> in 2 minutes.</p> | ||
| <h3>Announcements</h3> |
There was a problem hiding this comment.
This is a clever usage of MSAL Python's success_template feature. Good job. I guess MSAL shall better officially document this parameter/behavior, then. :-)
There was a problem hiding this comment.
One more thing. Currently, the broker is only available on Windows. Shall we keep using the old success template when Azure CLI is running on non-Windows platform, and only use the new one on Windows?
The conditional, per-platform guidance is inevitable in the long run, because we will probably need that pattern in the near future when the broker on other platforms becomes available.
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
dcaro
left a comment
There was a problem hiding this comment.
Overall looking good. See my comments.
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
| az config set core.allow_broker=true<br> | ||
| az account clear<br> | ||
| az login |
There was a problem hiding this comment.
Out of curiosity, @jiasli, is the "az account clear" necessary? My understanding is, after enabling broker, the subsequent "az foo bar" may fail and then Azure CLI's preexisting error message will guide end user to do an "az login" anyway.
There was a problem hiding this comment.
after enabling broker, the subsequent "az foo bar" may fail and then Azure CLI's preexisting error message will guide end user to do an "az login" anyway.
Relying on the error impairs user experience, right. 🙂
My consideration is that we want users to run az account clear so that the existing MSAL cache is purged, and make sure no credentials are left behind.
src/azure-cli-core/azure/cli/core/auth/landing_pages/success.html
Outdated
Show resolved
Hide resolved
Co-authored-by: Damien Caro <dcaro@microsoft.com>
Co-authored-by: Damien Caro <dcaro@microsoft.com>

Related command
az loginDescription
This PR updates the login success landing page to include an announcement for the date on which WAM broker will be used by default for Azure CLI login, alongside instructions for opt-in.
Testing Guide
N/A
History Notes
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.