Skip to content
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

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

Open
Binary-Eater opened this issue Mar 4, 2024 · 11 comments

Comments

@Binary-Eater
Copy link

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.

AzureAD/microsoft-authentication-library-for-dotnet#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
Copy link
Collaborator

rayluo commented Mar 4, 2024

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
Copy link
Author

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
Copy link
Collaborator

rayluo commented Mar 4, 2024

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
Copy link
Author

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
Copy link
Collaborator

rayluo commented Mar 5, 2024

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
Copy link
Author

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
Copy link
Collaborator

rayluo commented Mar 5, 2024

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
Copy link
Author

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
Copy link
Collaborator

rayluo commented Mar 5, 2024

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
Copy link
Author

Binary-Eater commented Mar 5, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants