Skip to content

Commit

Permalink
trace2: add error event (#1156)
Browse files Browse the repository at this point in the history
This series adds the `TRACE2` error event.

The first two commits are opportunistic fixes that remove unnecessary
components of `Trace2` messages. The next four commits add `Trace2` as a
dependency to certain classes where exceptions are thrown in order to
capture those exceptions with the new error event. The seventh commit
adds the error event, and eighth adds special exceptions that write to
`Trace2`. Finally, the ninth commit adds error tracing exceptions and
messages throughout the GCM codebase.
  • Loading branch information
ldennington committed Mar 21, 2023
2 parents 65e5a88 + 94f8d91 commit 784e7c7
Show file tree
Hide file tree
Showing 56 changed files with 603 additions and 305 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Atlassian.Bitbucket.Cloud;
using GitCredentialManager;
using GitCredentialManager.Authentication.OAuth;
using GitCredentialManager.Tests.Objects;
using Moq;
using Xunit;

Expand All @@ -16,7 +17,6 @@ public class BitbucketOAuth2ClientTest
{
private Mock<HttpClient> httpClient = new Mock<HttpClient>(MockBehavior.Strict);
private Mock<ISettings> settings = new Mock<ISettings>(MockBehavior.Loose);
private Mock<Trace> trace = new Mock<Trace>(MockBehavior.Loose);
private Mock<IOAuth2WebBrowser> browser = new Mock<IOAuth2WebBrowser>(MockBehavior.Strict);
private Mock<IOAuth2CodeGenerator> codeGenerator = new Mock<IOAuth2CodeGenerator>(MockBehavior.Strict);
private IEnumerable<string> scopes = new List<string>();
Expand Down Expand Up @@ -55,7 +55,7 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_RespectsClient
Uri finalCallbackUri = MockFinalCallbackUri();

Bitbucket.Cloud.BitbucketOAuth2Client client = GetBitbucketOAuth2Client();

MockGetAuthenticationCodeAsync(finalCallbackUri, clientId, client.Scopes);

MockCodeGenerator();
Expand All @@ -68,8 +68,9 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_RespectsClient
[Fact]
public async Task BitbucketOAuth2Client_GetDeviceCodeAsync()
{
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace.Object);
await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.GetDeviceCodeAsync(scopes, ct));
var trace2 = new NullTrace2();
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace2);
await Assert.ThrowsAsync<Trace2InvalidOperationException>(async () => await client.GetDeviceCodeAsync(scopes, ct));
}

[Theory]
Expand All @@ -79,7 +80,8 @@ public async Task BitbucketOAuth2Client_GetDeviceCodeAsync()
[InlineData("https", "example.com/", "john", "https://example.com/refresh_token")]
public void BitbucketOAuth2Client_GetRefreshTokenServiceName(string protocol, string host, string username, string expectedResult)
{
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace.Object);
var trace2 = new NullTrace2();
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace2);
var input = new InputArguments(new Dictionary<string, string>
{
["protocol"] = protocol,
Expand All @@ -100,7 +102,8 @@ private void VerifyAuthorizationCodeResult(OAuth2AuthorizationCodeResult result)

private Bitbucket.Cloud.BitbucketOAuth2Client GetBitbucketOAuth2Client()
{
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace.Object);
var trace2 = new NullTrace2();
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace2);
client.CodeGenerator = codeGenerator.Object;
return client;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Atlassian.Bitbucket.DataCenter;
using GitCredentialManager;
using GitCredentialManager.Authentication.OAuth;
using GitCredentialManager.Tests.Objects;
using Moq;
using Xunit;

Expand All @@ -16,7 +17,6 @@ public class BitbucketOAuth2ClientTest
{
private Mock<HttpClient> httpClient = new Mock<HttpClient>(MockBehavior.Strict);
private Mock<ISettings> settings = new Mock<ISettings>(MockBehavior.Loose);
private Mock<Trace> trace = new Mock<Trace>(MockBehavior.Loose);
private Mock<IOAuth2WebBrowser> browser = new Mock<IOAuth2WebBrowser>(MockBehavior.Strict);
private Mock<IOAuth2CodeGenerator> codeGenerator = new Mock<IOAuth2CodeGenerator>(MockBehavior.Strict);
private CancellationToken ct = new CancellationToken();
Expand Down Expand Up @@ -77,7 +77,8 @@ private void VerifyAuthorizationCodeResult(OAuth2AuthorizationCodeResult result,

private Bitbucket.DataCenter.BitbucketOAuth2Client GetBitbucketOAuth2Client()
{
var client = new Bitbucket.DataCenter.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace.Object);
var trace2 = new NullTrace2();
var client = new Bitbucket.DataCenter.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace2);
client.CodeGenerator = codeGenerator.Object;
return client;
}
Expand Down
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
5 changes: 2 additions & 3 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 @@ -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
7 changes: 6 additions & 1 deletion src/shared/Atlassian.Bitbucket/BitbucketOAuth2Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ namespace Atlassian.Bitbucket
{
public abstract class BitbucketOAuth2Client : OAuth2Client
{
public BitbucketOAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri, string clientSecret, ITrace trace) : base(httpClient, endpoints, clientId, redirectUri, clientSecret, false)
public BitbucketOAuth2Client(HttpClient httpClient,
OAuth2ServerEndpoints endpoints,
string clientId,
Uri redirectUri,
string clientSecret,
ITrace2 trace2) : base(httpClient, endpoints, clientId, trace2, redirectUri, clientSecret, false)
{
}

Expand Down
6 changes: 3 additions & 3 deletions src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ namespace Atlassian.Bitbucket.Cloud
{
public class BitbucketOAuth2Client : Bitbucket.BitbucketOAuth2Client
{
public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace trace)
public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace2 trace2)
: base(httpClient, GetEndpoints(),
GetClientId(settings), GetRedirectUri(settings), GetClientSecret(settings), trace)
GetClientId(settings), GetRedirectUri(settings), GetClientSecret(settings), trace2)
{
}

Expand Down Expand Up @@ -62,7 +62,7 @@ private static string GetClientSecret(ISettings settings)

return CloudConstants.OAuth2ClientSecret;
}

private static OAuth2ServerEndpoints GetEndpoints()
{
return new OAuth2ServerEndpoints(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ namespace Atlassian.Bitbucket.DataCenter
{
public class BitbucketOAuth2Client : Bitbucket.BitbucketOAuth2Client
{
public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace trace)
public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace2 trace2)
: base(httpClient, GetEndpoints(settings),
GetClientId(settings), GetRedirectUri(settings), GetClientSecret(settings), trace)
GetClientId(settings), GetRedirectUri(settings), GetClientSecret(settings), trace2)
{
}

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
8 changes: 5 additions & 3 deletions src/shared/Atlassian.Bitbucket/OAuth2ClientRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public class OAuth2ClientRegistry : IRegistry<BitbucketOAuth2Client>
private readonly HttpClient http;
private ISettings settings;
private readonly ITrace trace;
private readonly ITrace2 trace2;
private Cloud.BitbucketOAuth2Client cloudClient;
private DataCenter.BitbucketOAuth2Client dataCenterClient;

Expand All @@ -16,6 +17,7 @@ public OAuth2ClientRegistry(ICommandContext context)
this.http = context.HttpClientFactory.CreateClient();
this.settings = context.Settings;
this.trace = context.Trace;
this.trace2 = context.Trace2;
}

public BitbucketOAuth2Client Get(InputArguments input)
Expand All @@ -36,7 +38,7 @@ public void Dispose()
dataCenterClient = null;
}

private Cloud.BitbucketOAuth2Client CloudClient => cloudClient ??= new Cloud.BitbucketOAuth2Client(http, settings, trace);
private DataCenter.BitbucketOAuth2Client DataCenterClient => dataCenterClient ??= new DataCenter.BitbucketOAuth2Client(http, settings, trace);
private Cloud.BitbucketOAuth2Client CloudClient => cloudClient ??= new Cloud.BitbucketOAuth2Client(http, settings, trace2);
private DataCenter.BitbucketOAuth2Client DataCenterClient => dataCenterClient ??= new DataCenter.BitbucketOAuth2Client(http, settings, trace2);
}
}
}
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
Loading

0 comments on commit 784e7c7

Please sign in to comment.