AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
754 stars 191 forks source link

[Bug] System browser process on Linux writes junk to standard out/error #675

Open Binary-Eater opened 3 months ago

Binary-Eater commented 3 months ago

Describe the bug When using msal-python to acquire tokens interactively, the stdout/stderr of the launched browser process is not redirected (at least on Linux). This leads to the browser output interfering with the program's stdout for any programming consuming msal-python in this manner.

A similar issue was already opened and resolved for the dotnet equivalent.

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/2427

To Reproduce Steps to reproduce the behavior:

  1. Launch Chrome/Chromium before running anything
  2. Run interactive_sample.py with Chromium or Google Chrome set as the default browser on Linux. You should not need to modify the sample.

Expected behavior No prints from the launched browser program should be propogated to the user (unless the user explicitly requests to see them/configures to suppress them).

What you see instead The output Opening in existing browser session.

The MSAL Python version you are using Paste the output of this python -c "import msal; print(msal.__version__)"

❯ python -c "import msal; print(msal.__version__)"
1.24.1
rayluo commented 3 months ago

To Reproduce Steps to reproduce the behavior:

  1. Launch Chrome/Chromium before running anything

  2. Run interactive_sample.py with Chromium or Google Chrome set as the default browser on Linux

Can't reproduce. I installed a Chrome (Version 122.0.6261.94 (Official Build) (64-bit)) on my Debian-variation just now, set it as default, and then launch an equivalent script using MSAL 1.27 (the browser launching mechanism has remained the same in last many MSAL versions). See no extra output.

Expected behavior No prints from the launched browser program should be propogated to the user (unless the user explicitly requests to see them/configures to suppress them).

What you see instead The output Opening in existing browser session.

How bad was it? If it is just a one-liner "Opening in existing browser session", it sounds not that bad, even though it was not necessarily desirable.

When using msal-python to acquire tokens interactively, the stdout/stderr of the launched browser process is not redirected (at least on Linux). This leads to the browser output interfering with the program's stdout for any programming consuming msal-python in this manner.

Even if we could, where shall we redirect those stdout/stderr to? Wouldn't we still want them to be visible if a subprocess really wants to show some info/error?

Binary-Eater commented 3 months ago

Can't reproduce. I installed a Chrome (Version 122.0.6261.94 (Official Build) (64-bit)) on my Debian-variation just now, set it as default, and then launch an equivalent script using MSAL 1.27 (the browser launching mechanism has remained the same in last many MSAL versions). See no extra output.

Did you open Chrome first before starting this process? You need an existing Chrome process open to see this issue.

How bad was it? If it is just a one-liner "Opening in existing browser session", it sounds not that bad, even though it was not necessarily desirable.

It's problematic because you need to do the following in python.

https://github.com/Binary-Eater/git-credential-msal/blob/c679ff7ad2b746dcd5be8546de608584230078a6/git-credential-msal#L109-L111

The issue with the hack is it blocks off stdout redirection from the Chrome process which leads to other output on stderr (not bad but annoying/confusing to users).

Even if we could, where shall we redirect those stdout/stderr to? Wouldn't we still want them to be visible if a subprocess really wants to show some info/error?

In this case, we would have stdout, stderr file arguments. If a developer wants to directly output, they can pass sys.stdout and sys.stderr.

rayluo commented 3 months ago

Can't reproduce. I installed a Chrome (Version 122.0.6261.94 (Official Build) (64-bit)) on my Debian-variation just now, set it as default, and then launch an equivalent script using MSAL 1.27 (the browser launching mechanism has remained the same in last many MSAL versions). See no extra output.

Did you open Chrome first before starting this process? You need an existing Chrome process open to see this issue.

Yes, I double checked. I have a Chrome window opened. Still see no output on stdour/stderr. Could it be a Chrome version relative thing? Speaking about that, is Chrome's stdout/stderr behavior configurable?

Also, what Linux distro are you using? Other than WSL, MSAL is just using Python built-in webbrowser.open("url"). Not sure whether it provides much control on capturing stdout/stderr.

How bad was it? If it is just a one-liner "Opening in existing browser session", it sounds not that bad, even though it was not necessarily desirable.

It's problematic because you need to do the following in python.

https://github.com/Binary-Eater/git-credential-msal/blob/c679ff7ad2b746dcd5be8546de608584230078a6/git-credential-msal#L109-L111

The issue with the hack is it blocks off stdout redirection from the Chrome process which leads to other output on stderr (not bad but annoying/confusing to users).

That hack can be a good tactical fix, because it controls the blast radius.

BTW, you are working on a Git Credential Msal project? Sounds cool! Starred. :-) Also a FYI, I just recently start using Git Credential OAuth, and it works reasonably well.

Binary-Eater commented 3 months ago

Hi, I did not give good reproduction steps earlier.

git-credential-msal on  credential-helper-rfc [!?] via ❄️  impure (shell) 
❯ xdg-mime default chromium.desktop 'x-scheme-handler/http'

git-credential-msal on  credential-helper-rfc [!?] via ❄️  impure (shell) 
❯ xdg-mime default chromium.desktop 'x-scheme-handler/https'

git-credential-msal on  credential-helper-rfc [!?] via ❄️  impure (shell) 
❯ xdg-open https://git.beater.town
16:30:36 INFO: Opening in existing instance

You can then test with export BROWSER=xdg-open and then test the script. Apparently, Chrome/Chromium will change its output behavior if it's launched from xdg-open due to its code. There is nothing special about the invoke based on the related desktop file contents....

#!/nix/store/qbvwymv0614j5f2pfl552g8wrrs5rpbp-xdg-utils-unstable-2020-10-21/bin/xdg-open
[Desktop Entry]
Version=1.0
Terminal=false
Type=Application
Name=Microsoft Teams
Exec=/nix/store/3gpnym3j1aiys0isd6vqvzhr7fg87bv5-chromium-unwrapped-111.0.5563.110/libexec/chromium/chromium --profile-directory=Default --app-id=cifhbcnohmdccbgoicgdjpfamggdegmo
Icon=chrome-cifhbcnohmdccbgoicgdjpfamggdegmo-Default
StartupWMClass=crx_cifhbcnohmdccbgoicgdjpfamggdegmo

Sorry for missing this detail.

That hack can be a good tactical fix, because it controls the blast radius.

It has a problem when implementing a git credential helper and working with xdg-open. It leads to the following output in stderr.

cat: standard output: Bad file descriptor

BTW, you are working on a Git Credential Msal project? Sounds cool! Starred. :-) Also a FYI, I just recently start using Git Credential OAuth, and it works reasonably well.

The problem with git credential oauth is that it depends on the git server also being the identity provider. It does not work for the use case where you solely use Microsoft for the identity provider of your bare git server running cgit like the linux kernel project. More details in my blog.

https://binary-eater.github.io/posts/git_oidc/

rayluo commented 3 months ago

You can then test with export BROWSER=xdg-open and then test the script. Apparently, Chrome/Chromium will change its output behavior if it's launched from xdg-open due to its code.

Haven't fully followed your steps yet. But, a quick test indeed indicates that Chrome would output 2 dozen lines of logs into stdout/stderr, when and only when there was NO Chrome window already running. So, that is kind of the opposite of what you observed. That being said, if it is about export BROWSER=xdg-open, can you try just skip that setting completely? Python's webbrowser.open("...") is smart enough to work without BROWSER env var.

More details in my blog.

https://binary-eater.github.io/posts/git_oidc/

Thanks! I'll definitely give it a close look soon.

Binary-Eater commented 3 months ago

can you try just skip that setting completely? Python's webbrowser.open("...") is smart enough to work without BROWSER env var.

So I am just using this to manually reproduce what other people have reported to me on different Linux distributions. This is not just a one off use case since I have users for my credential helper already. Anyways, here is the full demo fwiw. I could try getting their environment details and sharing them here. I myself use Firefox.

microsoft-authentication-library-for-python on  dev [?] via 🐍 v3.11.6 via ❄️  impure (shell) 
❯ python3 ./sample/interactive_sample.py ./sample/parameters.json 
A local browser window will be open for you to sign in. CTRL+C to cancel.
17:10:46 INFO: Opening in existing instance
rayluo commented 3 months ago

So I am just using this to manually reproduce what other people have reported to me on different Linux distributions.

In that case, there are lots of setups to cater for. MSAL itself is not exactly in the browser configuration business.

Just thinking out aloud. Could your script unset the BROWSER env var (if at all possible), before calling MSAL?

Binary-Eater commented 3 months ago

So I agree configuring a browser in general is unreasonable. I think having control over the stdout and stderr of a subprocess spawned by library is reasonable design in general. I am pretty sure on their systems BROWSER is not set but xdg-open is invoked somehow.

rayluo commented 3 months ago

This is how MSAL Python currently opens a browser. It is already too complicated than we would like it to be.

If you can provide some hints or links on how to configure their stdout and stderr, I'll see what we can do here.

Binary-Eater commented 3 months ago

If you can provide some hints or links on how to configure their stdout and stderr, I'll see what we can do here.

Sure, I actually do not mind even making a PR for this. I mostly wanted to gauge interest before investing any time.

bgavrilMS commented 3 months ago

@rayluo - please remember to add "Public Client" / "Confidential Client" tags, as we monitor these bug / feature request queues separately.