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

[mjcheetham]

See the original issue: git-ecosystem/git-credential-manager#286

Logs and Network traces

Which Version of MSAL are you using ?
4.3.0

Platform
netcoreapp3.1

What authentication flow has the issue?

• Desktop / Mobile
  • Interactive - system browser

Is this a new or existing app?
b. The app is in production, I haven't upgraded MSAL, but started seeing this issue

Repro
On Linux, create a new console app, set Chromium set as your default browser and open an existing tab:

var pca = BuildPublicClientApplication();
var result = await pca.AcquireTokenInteractive().ExecuteAsync();

Expected behavior
A new tab is opened to the authentication page; nothing is written to the console.

Actual behavior
Text is written to the console.

Possible Solution
Detach/redirect stdout & err from the child process when launching a browser on Linux.

Additional context/ Logs / Screenshots
The problem appears to be the use of Process.Start when opening the URL on Linux. The .NET Core BCL uses xdg-open (amongst others) to start the new process/default browser, but it does not detach the standard in/out/err streams from the calling process. This means anything written by the browser (e.g., Chromium) or xdg-open will be written to the entry process' streams!

In GCM, who is invoked by Git, we must communicate back to Git over the standard output stream, and this noise is causing issues.

[bgavrilMS]

GCM have already fixed this issue here: https://github.com/mjcheetham/Git-Credential-Manager-Core/blob/293204e7533452f27129ebb634462c43921ba7aa/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2SystemWebBrowser.cs#L92-L124

[pmaytak]

Hi @mjcheetham
I made the related changes in PR #2507. Here's a build that includes those changes: Microsoft.Identity.Client.4.29.0-20210325.2.zip. Can you please test if it solves the issue?

I don't know if I was able to repro the exact behavior. I tried on Ubuntu with Chromium. I built my test project for Linux, then ran that executable from the terminal window.

• When I already had Chromium open, I would get this written to the terminal:
  
• When I didn't have the browser open, I would get this in the terminal:
  

Is the expectation that a new terminal window should be open for that console output? Or is this a different behavior that you're getting?

[mjcheetham]

I don't know if I was able to repro the exact behavior. I tried on Ubuntu with Chromium. I built my test project for Linux, then ran that executable from the terminal window.

• When I already had Chromium open, I would get this written to the terminal:
  
• When I didn't have the browser open, I would get this in the terminal:
  

Is the expectation that a new terminal window should be open for that console output? Or is this a different behavior that you're getting?

The expectation is that there should be nothing written to the standard output or error streams.

When the .NET application (Git Credential Manager in our case) calls Process::Start, on Linux it will fork & execve and the new process (xdg-open and friends) will inherit the stdin/out/err file descriptors. Chromium likes to write a lot of crud to output and error that is being consumed by our calling process (Git).

Redirecting the streams in .NET should cause new streams/file descriptors to be created for xdg-open. The new FDs are not connected to any console (only accessible from the process.StandardOutput/Error property in .NET/MSAL). MSAL can just ignore reading those streams.

Just getting my Linux box ready to test.

[mjcheetham]

Hi @pmaytak I've just tried the new package out, and this still does not fix the issue (see my reply on the PR).

If you're looking for a repro, here's the smallest one:

namespace TestApp
{
    class Program
    {
        public static async Task Main(string[] args)
        {
            const string clientId = "872cd9fa-d31f-45e0-9eab-6e460a02d1f1";
            string[] scopes = new { "User.Read" };
            var pca = PublicClientApplicationBuilder.Create(clientId)
                .WithRedirectUri("http://localhost")
                .Build();
            var result = await pca.AcquireTokenInteractive(scopes).ExecuteAsync();
        }
    }
}

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Identity.Client" Version="4.28.1" />
  </ItemGroup>
</Project>

$ dotnet run

(chrome:21189): Gtk-WARNING **: 13:58:53.285: Theme parsing error gtk.css:1565.23: 'font-feature-settings' is not a valid property name

(chrome:21189): Gtk-WARNING **: 13:58:53.289: Theme parsing error gtk.css:3615.25: 'font-feature-settings' is not a valid property name

(chrome:21189): Gtk-WARNING **: 13:58:53.290: Theme parsing error gtk.css:4077.23: 'font-feature-settings' is not a valid property name
Opening in existing browser session.

☝️ those messages to standard out/error should not be written.

[pmaytak]

This is included in MSAL 4.30.0 release.

cc: @mjcheetham