Skip to content

Commit

Permalink
oauth: disconnect streams from xdg-open
Browse files Browse the repository at this point in the history
When using the `Process` class to open the user's default browser on
Linux, utilities like `xdg-open` are used. Some of these utilities
do not disconnect child processes from our standard input/output/error
streams. At the same time, browsers like Chromium like to write to
stdout and stderr, which gets fed back to Git or the user's terminal,
respectively. The latter looks messy, and the former causes Git to fail.

On Linux, we instead manually locate a suitable 'shell execute' utility
and launch them directly - this way we can redirect the standard
output/error streams.

For Windows and macOS, this is not an issue and we continue to use the
Framework code to do 'shell execute'.
  • Loading branch information
mjcheetham committed Sep 25, 2020
1 parent 71cf87a commit ad0c2ec
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public async Task<OAuth2TokenResult> CreateOAuthCredentialsAsync(Uri targetUri)
FailureResponseHtmlFormat = BitbucketResources.AuthenticationResponseFailureHtmlFormat
};

var browser = new OAuth2SystemWebBrowser(browserOptions);
var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
var authCodeResult = await oauthClient.GetAuthorizationCodeAsync(Scopes, browser, CancellationToken.None);

return await oauthClient.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);
Expand Down
2 changes: 1 addition & 1 deletion src/shared/GitHub/GitHubAuthentication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public async Task<OAuth2TokenResult> GetOAuthTokenAsync(Uri targetUri, IEnumerab
SuccessResponseHtml = GitHubResources.AuthenticationResponseSuccessHtml,
FailureResponseHtmlFormat = GitHubResources.AuthenticationResponseFailureHtmlFormat
};
var browser = new OAuth2SystemWebBrowser(browserOptions);
var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);

// Write message to the terminal (if any is attached) for some feedback that we're waiting for a web response
Context.Terminal.WriteLine("info: please complete authentication in your browser...");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,15 @@ public class OAuth2WebBrowserOptions

public class OAuth2SystemWebBrowser : IOAuth2WebBrowser
{
private readonly IEnvironment _environment;
private readonly OAuth2WebBrowserOptions _options;

public OAuth2SystemWebBrowser(OAuth2WebBrowserOptions options)
public OAuth2SystemWebBrowser(IEnvironment environment, OAuth2WebBrowserOptions options)
{
EnsureArgument.NotNull(environment, nameof(environment));
EnsureArgument.NotNull(options, nameof(options));

_environment = environment;
_options = options;
}

Expand Down Expand Up @@ -75,18 +80,56 @@ public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redi

private void OpenDefaultBrowser(Uri uri)
{
if (!StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttp, uri.Scheme) &&
!StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttps, uri.Scheme))
if (!uri.Scheme.Equals(Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase) &&
!uri.Scheme.Equals(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
{
throw new ArgumentException("Can only open HTTP/HTTPS URIs", nameof(uri));
}

var pci = new ProcessStartInfo(uri.ToString())
string url = uri.ToString();

ProcessStartInfo psi = null;
if (PlatformUtils.IsLinux())
{
UseShellExecute = true
};
// On Linux, 'shell execute' utilities like xdg-open launch a process without
// detaching from the standard in/out descriptors. Some applications (like
// Chromium) write messages to stdout, which is currently hooked up and being
// consumed by Git, and cause errors.
//
// Sadly, the Framework does not allow us to redirect standard streams if we
// set ProcessStartInfo::UseShellExecute = true, so we must manually launch
// these utilities and redirect the standard streams manually.
//
// We try and use the same 'shell execute' utilities as the Framework does,
// searching for them in the same order until we find one.
foreach (string shellExec in new[] { "xdg-open", "gnome-open", "kfmclient" })
{
if (_environment.TryLocateExecutable(shellExec, out string shellExecPath))
{
psi = new ProcessStartInfo(shellExecPath, url)
{
RedirectStandardOutput = true,
RedirectStandardError = true
};

// We found a way to open the URI; stop searching!
break;
}
}

if (psi is null)
{
throw new Exception("Failed to locate a utility to launch the default web browser.");
}
}
else
{
// On Windows and macOS, `ShellExecute` and `/usr/bin/open` disconnect the child process
// from our standard in/out streams, so we can just use the Framework to do this.
psi = new ProcessStartInfo(url) {UseShellExecute = true};
}

Process.Start(pci);
Process.Start(psi);
}

private async Task<Uri> InterceptRequestsAsync(Uri listenUri, CancellationToken ct)
Expand Down
26 changes: 0 additions & 26 deletions src/shared/Microsoft.Git.CredentialManager/BrowserHelper.cs

This file was deleted.

0 comments on commit ad0c2ec

Please sign in to comment.