Skip to content

Commit

Permalink
trace2: add error tracing statements
Browse files Browse the repository at this point in the history
Add TRACE2 error tracing statements throughout GCM codebase. Note that
this is a best effort - there are certain places (most notably certain
static and unsafe methods) where statements were purposefully not added
because it is either not safe or was deemed to much lift/churn to do so.

Note that there are some instances in which a "parameterized" message is
passed to TRACE2. This is used only when PII information could be passed
into the message (e.g. through a path or remote URI) and should be
redacted for collection purposes.

Finally, there are certain instances in which we write an error with no
exception thrown. These align with the places where we are currently
writing to GCM Trace without throwing an error for the purposes of parity.
  • Loading branch information
ldennington committed Mar 21, 2023
1 parent 2399751 commit e898ae7
Show file tree
Hide file tree
Showing 35 changed files with 240 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private async Task<int> ExecuteAsync(Uri url, string userName, bool showOAuth, b

if (!viewModel.WindowResult || viewModel.SelectedMode == AuthenticationModes.None)
{
throw new Exception("User cancelled dialog.");
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
}

switch (viewModel.SelectedMode)
Expand Down
7 changes: 3 additions & 4 deletions src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Atlassian.Bitbucket.Cloud;
using GitCredentialManager;
using GitCredentialManager.Authentication;
using GitCredentialManager.Authentication.OAuth;
Expand Down Expand Up @@ -88,7 +87,7 @@ public async Task<CredentialsPromptResult> GetCredentialsAsync(Uri targetUri, st
// We need at least one mode!
if (modes == AuthenticationModes.None)
{
throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes));
throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied");
}

// Shell out to the UI helper and show the Bitbucket u/p prompt
Expand Down Expand Up @@ -128,12 +127,12 @@ public async Task<CredentialsPromptResult> GetCredentialsAsync(Uri targetUri, st
{
if (!output.TryGetValue("username", out userName))
{
throw new Exception("Missing username in response");
throw new Trace2Exception(Context.Trace2, "Missing username in response");
}

if (!output.TryGetValue("password", out password))
{
throw new Exception("Missing password in response");
throw new Trace2Exception(Context.Trace2, "Missing password in response");
}

return new CredentialsPromptResult(
Expand Down
37 changes: 25 additions & 12 deletions src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Atlassian.Bitbucket.Cloud;
Expand Down Expand Up @@ -79,7 +78,8 @@ public async Task<ICredential> GetCredentialAsync(InputArguments input)
if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http")
&& BitbucketHelper.IsBitbucketOrg(input))
{
throw new Exception("Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS.");
throw new Trace2Exception(_context.Trace2,
"Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS.");
}

var authModes = await GetSupportedAuthenticationModesAsync(input);
Expand Down Expand Up @@ -145,8 +145,9 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
var result = await _bitbucketAuth.GetCredentialsAsync(remoteUri, input.UserName, authModes);
if (result is null || result.AuthenticationMode == AuthenticationModes.None)
{
_context.Trace.WriteLine("User cancelled credential prompt");
throw new Exception("User cancelled credential prompt.");
var message = "User cancelled credential prompt";
_context.Trace.WriteLine(message);
throw new Trace2Exception(_context.Trace2, message);
}

switch (result.AuthenticationMode)
Expand Down Expand Up @@ -176,8 +177,10 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
}
catch (OAuth2Exception ex)
{
_context.Trace.WriteLine("Failed to refresh existing OAuth credential using refresh token");
var message = "Failed to refresh existing OAuth credential using refresh token";
_context.Trace.WriteLine(message);
_context.Trace.WriteException(ex);
_context.Trace2.WriteError(message);

// We failed to refresh the AT using the RT; log the refresh failure and fall through to restart
// the OAuth authentication flow
Expand Down Expand Up @@ -279,7 +282,7 @@ public async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Inpu
try
{
var authenticationMethods = await _restApiRegistry.Get(input).GetAuthenticationMethodsAsync();

var modes = AuthenticationModes.None;

if (authenticationMethods.Contains(AuthenticationMethod.BasicAuth))
Expand All @@ -298,10 +301,14 @@ public async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Inpu
}
catch (Exception ex)
{
_context.Trace.WriteLine($"Failed to query '{input.GetRemoteUri()}' for supported authentication schemes.");
var format = "Failed to query '{0}' for supported authentication schemes.";
var message = string.Format(format, input.GetRemoteUri());

_context.Trace.WriteLine(message);
_context.Trace.WriteException(ex);
_context.Trace2.WriteError(message, format);

_context.Terminal.WriteLine($"warning: failed to query '{input.GetRemoteUri()}' for supported authentication schemes.");
_context.Terminal.WriteLine($"warning: {message}");

// Fall-back to offering all modes so the user is never blocked from authenticating by at least one mode
return AuthenticationModes.All;
Expand Down Expand Up @@ -356,7 +363,8 @@ private async Task<string> ResolveOAuthUserNameAsync(InputArguments input, strin
return result.Response.UserName;
}

throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}");
throw new Trace2Exception(_context.Trace2,
$"Failed to resolve username. HTTP: {result.StatusCode}");
}

private async Task<string> ResolveBasicAuthUserNameAsync(InputArguments input, string username, string password)
Expand All @@ -367,7 +375,8 @@ private async Task<string> ResolveBasicAuthUserNameAsync(InputArguments input, s
return result.Response.UserName;
}

throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}");
throw new Trace2Exception(_context.Trace2,
$"Failed to resolve username. HTTP: {result.StatusCode}");
}

private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredential credentials, AuthenticationModes authModes)
Expand Down Expand Up @@ -404,8 +413,10 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
}
catch (Exception ex)
{
_context.Trace.WriteLine($"Failed to validate existing credentials using OAuth");
var message = "Failed to validate existing credentials using OAuth";
_context.Trace.WriteLine(message);
_context.Trace.WriteException(ex);
_context.Trace2.WriteError(message);
}
}

Expand All @@ -419,8 +430,10 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
}
catch (Exception ex)
{
_context.Trace.WriteLine($"Failed to validate existing credentials using Basic Auth");
var message = "Failed to validate existing credentials using Basic Auth";
_context.Trace.WriteLine(message);
_context.Trace.WriteException(ex);
_context.Trace2.WriteError(message);
return false;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public BitbucketRestApi(ICommandContext context)
EnsureArgument.NotNull(context, nameof(context));

_context = context;

}

public async Task<RestApiResult<IUserInfo>> GetUserInformationAsync(string userName, string password, bool isBearerToken)
Expand All @@ -35,7 +35,7 @@ public async Task<RestApiResult<IUserInfo>> GetUserInformationAsync(string userN
}

// Bitbucket Server/DC doesn't actually provide a REST API we can use to trade an access_token for the owning username,
// therefore this is always going to return a placeholder username, however this call does provide a way to validate the
// therefore this is always going to return a placeholder username, however this call does provide a way to validate the
// credentials we do have
var requestUri = new Uri(ApiUri, "api/1.0/users");
using (HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, requestUri))
Expand Down Expand Up @@ -131,9 +131,9 @@ public void Dispose()

private HttpClient HttpClient => _httpClient ??= _context.HttpClientFactory.CreateClient();

private Uri ApiUri
private Uri ApiUri
{
get
get
{
var remoteUri = _context.Settings?.RemoteUri;
if (remoteUri == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA

var msAuth = new MicrosoftAuthentication(context);

await Assert.ThrowsAsync<InvalidOperationException>(
await Assert.ThrowsAsync<Trace2InvalidOperationException>(
() => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Core.UI/Commands/CredentialsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private async Task<int> ExecuteAsync(CommandOptions options)

if (!viewModel.WindowResult)
{
throw new Exception("User cancelled dialog.");
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
}

WriteResult(
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Core.UI/Commands/DeviceCodeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private async Task<int> ExecuteAsync(string code, string url, bool noLogo)

if (!viewModel.WindowResult)
{
throw new Exception("User cancelled dialog.");
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Core.UI/Commands/OAuthCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private async Task<int> ExecuteAsync(CommandOptions options)

if (!viewModel.WindowResult)
{
throw new Exception("User cancelled dialog.");
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
}

var result = new Dictionary<string, string>();
Expand Down
16 changes: 8 additions & 8 deletions src/shared/Core/Authentication/AuthenticationBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ protected AuthenticationBase(ICommandContext context)
var process = ChildProcess.Start(Context.Trace2, procStartInfo, Trace2ProcessClass.UIHelper);
if (process is null)
{
throw new Exception($"Failed to start helper process: {path} {args}");
var format = "Failed to start helper process: {0} {1}";
var message = string.Format(format, path, args);

throw new Trace2Exception(Context.Trace2, message, format);
}

// Kill the process upon a cancellation request
Expand All @@ -69,7 +72,7 @@ protected AuthenticationBase(ICommandContext context)
errorMessage = "Unknown";
}

throw new Exception($"helper error ({exitCode}): {errorMessage}");
throw new Trace2Exception(Context.Trace2, $"helper error ({exitCode}): {errorMessage}");
}

return resultDict;
Expand All @@ -85,8 +88,7 @@ protected void ThrowIfUserInteractionDisabled()
Constants.GitConfiguration.Credential.Interactive);

Context.Trace.WriteLine($"{envName} / {cfgName} is false/never; user interactivity has been disabled.");

throw new InvalidOperationException("Cannot prompt because user interactivity has been disabled.");
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot prompt because user interactivity has been disabled.");
}
}

Expand All @@ -95,8 +97,7 @@ protected void ThrowIfGuiPromptsDisabled()
if (!Context.Settings.IsGuiPromptsEnabled)
{
Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; GUI prompts have been disabled.");

throw new InvalidOperationException("Cannot show prompt because GUI prompts have been disabled.");
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot show prompt because GUI prompts have been disabled.");
}
}

Expand All @@ -105,8 +106,7 @@ protected void ThrowIfTerminalPromptsDisabled()
if (!Context.Settings.IsTerminalPromptsEnabled)
{
Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; terminal prompts have been disabled.");

throw new InvalidOperationException("Cannot prompt because terminal prompts have been disabled.");
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot prompt because terminal prompts have been disabled.");
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/shared/Core/Authentication/BasicAuthentication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ private async Task<ICredential> GetCredentialsByUiAsync(string command, string a

if (!resultDict.TryGetValue("username", out userName))
{
throw new Exception("Missing 'username' in response");
throw new Trace2Exception(Context.Trace2, "Missing 'username' in response");
}

if (!resultDict.TryGetValue("password", out string password))
{
throw new Exception("Missing 'password' in response");
throw new Trace2Exception(Context.Trace2, "Missing 'password' in response");
}

return new GitCredential(userName, password);
Expand Down
19 changes: 13 additions & 6 deletions src/shared/Core/Authentication/MicrosoftAuthentication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,11 @@ private async Task RegisterTokenCacheAsync(IPublicClientApplication app, ITrace2
}
catch (MsalCachePersistenceException ex)
{
var message = "Cannot persist Microsoft Authentication data securely!";
Context.Streams.Error.WriteLine("warning: cannot persist Microsoft authentication token cache securely!");
Context.Trace.WriteLine("Cannot persist Microsoft Authentication data securely!");
Context.Trace.WriteLine(message);
Context.Trace.WriteException(ex);
Context.Trace2.WriteError(message);

if (PlatformUtils.IsMacOS())
{
Expand Down Expand Up @@ -498,10 +500,12 @@ private void EnsureCanUseEmbeddedWebView()
#if NETFRAMEWORK
if (!Context.SessionManager.IsDesktopSession)
{
throw new InvalidOperationException("Embedded web view is not available without a desktop session.");
throw new Trace2InvalidOperationException(Context.Trace2,
"Embedded web view is not available without a desktop session.");
}
#else
throw new InvalidOperationException("Embedded web view is not available on .NET Core.");
throw new Trace2InvalidOperationException(Context.Trace2,
"Embedded web view is not available on .NET Core.");
#endif
}

Expand All @@ -515,17 +519,20 @@ private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirec
{
if (!Context.SessionManager.IsWebBrowserAvailable)
{
throw new InvalidOperationException("System web view is not available without a way to start a browser.");
throw new Trace2InvalidOperationException(Context.Trace2,
"System web view is not available without a way to start a browser.");
}

if (!app.IsSystemWebViewAvailable)
{
throw new InvalidOperationException("System web view is not available on this platform.");
throw new Trace2InvalidOperationException(Context.Trace2,
"System web view is not available on this platform.");
}

if (!redirectUri.IsLoopback)
{
throw new InvalidOperationException("System web view is not available for this service configuration.");
throw new Trace2InvalidOperationException(Context.Trace2,
"System web view is not available for this service configuration.");
}
}

Expand Down
16 changes: 11 additions & 5 deletions src/shared/Core/Authentication/OAuth/OAuth2Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,17 +163,19 @@ public IOAuth2CodeGenerator CodeGenerator
IDictionary<string, string> redirectQueryParams = finalUri.GetQueryParameters();
if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.StateParameter, out string replyState))
{
throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response.");
throw new Trace2OAuth2Exception(_trace2, $"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response.");
}
if (!StringComparer.Ordinal.Equals(state, replyState))
{
throw new OAuth2Exception($"Invalid '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response. Does not match initial request.");
throw new Trace2OAuth2Exception(_trace2,
$"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response.");
}

// We expect to have the auth code in the response otherwise terminate the flow (we failed authentication for some reason)
if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter, out string authCode))
{
throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response.");
throw new Trace2OAuth2Exception(_trace2,
$"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response.");
}

return new OAuth2AuthorizationCodeResult(authCode, redirectUri, codeVerifier);
Expand All @@ -183,7 +185,8 @@ public async Task<OAuth2DeviceCodeResult> GetDeviceCodeAsync(IEnumerable<string>
{
if (_endpoints.DeviceAuthorizationEndpoint is null)
{
throw new InvalidOperationException("No device authorization endpoint has been configured for this client.");
throw new Trace2InvalidOperationException(_trace2,
"No device authorization endpoint has been configured for this client.");
}

string scopesStr = string.Join(" ", scopes);
Expand Down Expand Up @@ -383,10 +386,13 @@ protected Exception CreateExceptionFromResponse(string json)
{
if (TryCreateExceptionFromResponse(json, out OAuth2Exception exception))
{
_trace2.WriteError(exception.Message);
return exception;
}

return new OAuth2Exception($"Unknown OAuth error: {json}");
var format = "Unknown OAuth error: {0}";
var message = string.Format(format, json);
return new Trace2OAuth2Exception(_trace2, message, format);
}

protected static bool TryDeserializeJson<T>(string json, out T obj)
Expand Down
Loading

0 comments on commit e898ae7

Please sign in to comment.