Conversation
|
/azp run MSAL-Python-SDL-CI |
fengga
left a comment
There was a problem hiding this comment.
Thanks, this PR looks good to me. And please make sure get an approval from Ray.
rayluo
left a comment
There was a problem hiding this comment.
Thanks for a clean PR! Implementation wise, it looks good. I added some suggestions above, mostly in terms of our workflow. Please make corresponding changes and then wait for the PyMsalRuntime release.
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
rayluo
left a comment
There was a problem hiding this comment.
We will also need to change the precise dependency version error message here and a approximate version hint there
updated |
|
@DharshanBJ, I would love to see this wrapped up. I arrived here after trying to debug errors with using msal on WSL. This PR worked "as-advertised" and made my life a whole lot easier. Before your PR, the following snippet kept returning a browser window with Testing with app = PublicClientApplication(
<client_id>,
authority=<authority>,
enable_broker_on_linux=True,
)
app.acquire_token_interactive(...) |
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
Co-authored-by: Ray Luo <rayluo@microsoft.com>
rayluo
left a comment
There was a problem hiding this comment.
Thanks for your effort all this time! Approving now, and I'll follow up with the merge and then the release shortly after.
|
Tested the code in Azure/azure-cli#31169, but can't get it working. The behavior is different from |
jiasli
left a comment
There was a problem hiding this comment.
Doesn't launch account selection window.
Please upgrade to WSL 2.4.13+
@DharshanBJ , can you investigate this one? |
Checked with jiasli, the issue is due to not having WSL 2.4.13+, since the user is not able to acquire the token interactively(due to not having the account picker changes), the error message "Authentication failed against tenant 72f988bf-86f1-41af-91ab-2d7cd011db47 'Microsoft': Can't find token from MSAL cache." is for that same reason. |
|
I checked update in both Windows Store and Windows Update, but WSL is still on an old version
https://learn.microsoft.com/en-us/windows/wsl/install should contain a section on "How to update WSL". Also, we can't force or expect the users to update WSL to the latest version manually. It is better to have WSL updated by Windows Store or Windows Update automatically. Besides this, the interface of this functionality looks good to me. |
| # | ||
| # We need pymsalruntime.CallbackData introduced in PyMsalRuntime 0.14 | ||
| pymsalruntime>=0.14,<0.18; python_version>='3.6' and platform_system=='Windows' | ||
| pymsalruntime>=0.14,<0.19; python_version>='3.6' and platform_system=='Windows' |
There was a problem hiding this comment.
I cannot use pymsalruntime 0.18.1 (Azure/azure-cli#31563) right now as this line has not been released.
There was a problem hiding this comment.
True. Please subscribe/watch this repo to receive its release notification. We aim for each of the Azure CLI code freeze (minus 2 weeks) as checkpoint. Until then, you would have to manually install from MSAL's dev branch and test.
No description provided.