diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index c4ae1782d..d039d228f 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -20,17 +20,13 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain) new GitHubClient(AppConfiguration.ProductHeader, credentialStore, hostAddress.ApiUri)); } - private static readonly Unity.ILogging logger = Unity.Logging.GetLogger(); + private static readonly ILogging logger = Logging.GetLogger(); public HostAddress HostAddress { get; } public UriString OriginalUrl { get; } private readonly IKeychain keychain; private readonly IGitHubClient githubClient; private readonly ILoginManager loginManager; - private static readonly SemaphoreSlim sem = new SemaphoreSlim(1); - - IList organizationsCache; - Octokit.User userCache; string owner; bool? isEnterprise; @@ -72,11 +68,10 @@ public async Task CreateRepository(NewRepository newRepository, Action> callback) + public async Task GetOrganizations(Action> onSuccess, Action onError = null) { - Guard.ArgumentNotNull(callback, "callback"); - var organizations = await GetOrganizationInternal(); - callback(organizations); + Guard.ArgumentNotNull(onSuccess, nameof(onSuccess)); + await GetOrganizationInternal(onSuccess, onError); } public async Task LoadKeychain(Action callback) @@ -195,10 +190,8 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func ContinueLoginAsync(LoginResult loginResult, Func> GetOrganizationInternal() + private async Task GetOrganizationInternal(Action> onSuccess, Action onError = null) { try { logger.Trace("Getting Organizations"); - if (!await LoadKeychainInternal()) - { - return new List(); - } + await ValidateKeychain(); + await ValidateCurrentUserInternal(); var organizations = await githubClient.Organization.GetAllForCurrent(); @@ -241,49 +232,51 @@ private async Task> GetOrganizationInternal() if (organizations != null) { - organizationsCache = organizations.ToArray(); + onSuccess(organizations.ToArray()); } } catch(Exception ex) { logger.Error(ex, "Error Getting Organizations"); - throw; + onError?.Invoke(ex); } - - return organizationsCache; } private async Task GetCurrentUserInternal() { try { - logger.Trace("Getting Organizations"); - - if (!await LoadKeychainInternal()) - { - return null; - } + logger.Trace("Getting Current User"); + await ValidateKeychain(); - userCache = await githubClient.User.Current(); + return await githubClient.User.Current(); } - catch(Exception ex) + catch (Exception ex) { logger.Error(ex, "Error Getting Current User"); throw; } + } + + private async Task ValidateCurrentUserInternal() + { + logger.Trace("Validating User"); + var apiUsername = await GetCurrentUserInternal(); + var cachedUsername = keychain.Connections.First().Username; - return userCache; + if (apiUsername.Name != cachedUsername) + { + throw new TokenUsernameMismatchException(); + } } private async Task LoadKeychainInternal() { - logger.Trace("LoadKeychainInternal"); - if (keychain.HasKeys) { if (!keychain.NeedsLoad) { - logger.Trace("LoadKeychainInternal: Has keys does not need load"); + logger.Trace("LoadKeychainInternal: Previously Loaded"); return true; } @@ -293,6 +286,8 @@ private async Task LoadKeychainInternal() var uriString = keychain.Connections.First().Host; var keychainAdapter = await keychain.Load(uriString); + logger.Trace("LoadKeychainInternal: Loaded"); + return keychainAdapter.OctokitCredentials != Credentials.Anonymous; } @@ -301,23 +296,48 @@ private async Task LoadKeychainInternal() return false; } - public async Task ValidateCredentials() + private async Task ValidateKeychain() { - try + if (!await LoadKeychainInternal()) { - var store = keychain.Connect(OriginalUrl); - - if (store.OctokitCredentials != Credentials.Anonymous) - { - var credential = store.Credential; - await githubClient.Authorization.CheckApplicationAuthentication(ApplicationInfo.ClientId, credential.Token); - } - } - catch - { - return false; + throw new KeychainEmptyException(); } - return true; } } + + class ApiClientException : Exception + { + public ApiClientException() + { } + + public ApiClientException(string message) : base(message) + { } + + public ApiClientException(string message, Exception innerException) : base(message, innerException) + { } + } + + class TokenUsernameMismatchException : ApiClientException + { + public TokenUsernameMismatchException() + { } + + public TokenUsernameMismatchException(string message) : base(message) + { } + + public TokenUsernameMismatchException(string message, Exception innerException) : base(message, innerException) + { } + } + + class KeychainEmptyException : ApiClientException + { + public KeychainEmptyException() + { } + + public KeychainEmptyException(string message) : base(message) + { } + + public KeychainEmptyException(string message, Exception innerException) : base(message, innerException) + { } + } } diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 1ebbcd414..95c5623ce 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -10,11 +10,10 @@ interface IApiClient HostAddress HostAddress { get; } UriString OriginalUrl { get; } Task CreateRepository(NewRepository newRepository, Action callback, string organization = null); - Task GetOrganizations(Action> callback); + Task GetOrganizations(Action> onSuccess, Action onError = null); Task Login(string username, string password, Action need2faCode, Action result); Task ContinueLogin(LoginResult loginResult, string code); Task LoginAsync(string username, string password, Func need2faCode); - Task ValidateCredentials(); Task Logout(UriString host); Task GetCurrentUser(Action callback); Task LoadKeychain(Action callback); diff --git a/src/GitHub.Api/Authentication/Keychain.cs b/src/GitHub.Api/Authentication/Keychain.cs index b531735f9..bab311239 100644 --- a/src/GitHub.Api/Authentication/Keychain.cs +++ b/src/GitHub.Api/Authentication/Keychain.cs @@ -68,7 +68,7 @@ public async Task Load(UriString host) { if (keychainItem.Username != cachedConnection.Username) { - logger.Warning("Keychain Username: {0} does not match; Hopefully it works", keychainItem.Username); + logger.Warning("Keychain Username:\"{0}\" does not match cached Username:\"{1}\"; Hopefully it works", keychainItem.Username, cachedConnection.Username); } logger.Trace("Loaded from Credential Manager Host:\"{0}\" Username:\"{1}\"", keychainItem.Host, keychainItem.Username); diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index addb84e63..f84a66858 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -122,6 +122,24 @@ private void LoadOrganizations() owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); isBusy = false; + }, exception => { + var tokenUsernameMismatchException = exception as TokenUsernameMismatchException; + if (tokenUsernameMismatchException != null) + { + //This is a specific case + + Logger.Error(exception, "Token Username Mismatch"); + return; + } + + var keychainEmptyException = exception as KeychainEmptyException; + if (keychainEmptyException != null) + { + Logger.Error(exception, "Keychain empty"); + return; + } + + Logger.Error(exception, "Unhandled Exception Type"); }); }); }