Merge pull request #300 from github-for-unity/fixes/api-failure-switch-auth-view

Switch to AuthView on Publish API Failure

Author: shana

src/GitHub.Api/Application/ApiClient.cs

@@ -19,17 +19,14 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain)
                 new GitHubClient(AppConfiguration.ProductHeader, credentialStore, hostAddress.ApiUri));
         }
 
-        private static readonly Unity.ILogging logger = Unity.Logging.GetLogger<ApiClient>();
+        private static readonly ILogging logger = Logging.GetLogger<ApiClient>();
         public HostAddress HostAddress { get; }
         public UriString OriginalUrl { get; }
 
         private readonly IKeychain keychain;
         private readonly IGitHubClient githubClient;
         private readonly ILoginManager loginManager;
 
-        IList<Organization> organizationsCache;
-        Octokit.User userCache;
-
         public ApiClient(UriString hostUrl, IKeychain keychain, IGitHubClient githubClient)
         {
             Guard.ArgumentNotNull(hostUrl, nameof(hostUrl));
@@ -53,7 +50,7 @@ private async Task LogoutInternal(UriString host)
             await loginManager.Logout(host);
         }
 
-        public async Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null)
+        public async Task CreateRepository(NewRepository newRepository, Action<GitHubRepository, Exception> callback, string organization = null)
         {
             Guard.ArgumentNotNull(callback, "callback");
             try
@@ -67,21 +64,27 @@ public async Task CreateRepository(NewRepository newRepository, Action<Octokit.R
             }
         }
 
-        public async Task GetOrganizations(Action<IList<Organization>> callback)
+        public async Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> 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<bool> callback)
+        public async Task ValidateCurrentUser(Action onSuccess, Action<Exception> onError = null)
         {
-            Guard.ArgumentNotNull(callback, "callback");
-            var hasLoadedKeys = await LoadKeychainInternal();
-            callback(hasLoadedKeys);
+            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
+            try
+            {
+                await ValidateCurrentUserInternal();
+                onSuccess();
+            }
+            catch (Exception e)
+            {
+                onError?.Invoke(e);
+            }
         }
 
-        public async Task GetCurrentUser(Action<Octokit.User> callback)
+        public async Task GetCurrentUser(Action<GitHubUser> callback)
         {
             Guard.ArgumentNotNull(callback, "callback");
             var user = await GetCurrentUserInternal();
@@ -184,29 +187,27 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             return result.Code == LoginResultCodes.Success;
         }
 
-        private async Task<Octokit.Repository> CreateRepositoryInternal(NewRepository newRepository, string organization)
+        private async Task<GitHubRepository> CreateRepositoryInternal(NewRepository newRepository, string organization)
         {
             try
             {
                 logger.Trace("Creating repository");
 
-                if (!await LoadKeychainInternal())
-                {
-                    throw new InvalidOperationException("The keychain did not load");
-                }
+                await ValidateKeychain();
+                await ValidateCurrentUserInternal();
 
-                Octokit.Repository repository;
+                GitHubRepository repository;
                 if (!string.IsNullOrEmpty(organization))
                 {
                     logger.Trace("Creating repository for organization");
 
-                    repository = await githubClient.Repository.Create(organization, newRepository);
+                    repository = (await githubClient.Repository.Create(organization, newRepository)).ToGitHubRepository();
                 }
                 else
                 {
                     logger.Trace("Creating repository for user");
 
-                    repository = await githubClient.Repository.Create(newRepository);
+                    repository = (await githubClient.Repository.Create(newRepository)).ToGitHubRepository();
                 }
 
                 logger.Trace("Created Repository");
@@ -219,66 +220,78 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             }
         }
 
-        private async Task<IList<Organization>> GetOrganizationInternal()
+        private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
             try
             {
                 logger.Trace("Getting Organizations");
 
-                if (!await LoadKeychainInternal())
-                {
-                    return new List<Organization>();
-                }
+                await ValidateKeychain();
+                await ValidateCurrentUserInternal();
 
                 var organizations = await githubClient.Organization.GetAllForCurrent();
 
                 logger.Trace("Obtained {0} Organizations", organizations?.Count.ToString() ?? "NULL");
 
                 if (organizations != null)
                 {
-                    organizationsCache = organizations.ToArray();
+                    var array = organizations.Select(organization => new Organization() {
+                        Name = organization.Name,
+                        Login = organization.Login
+                    }).ToArray();
+                    onSuccess(array);
                 }
             }
             catch(Exception ex)
             {
                 logger.Error(ex, "Error Getting Organizations");
-                throw;
+                onError?.Invoke(ex);
             }
-
-            return organizationsCache;
         }
 
-        private async Task<Octokit.User> GetCurrentUserInternal()
+        private async Task<GitHubUser> 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()).ToGitHubUser();
             }
-            catch(Exception ex)
+            catch (KeychainEmptyException)
+            {
+                logger.Warning("Keychain is empty");
+                throw;
+            }
+            catch (Exception ex)
             {
                 logger.Error(ex, "Error Getting Current User");
                 throw;
             }
+        }
+
+        private async Task ValidateCurrentUserInternal()
+        {
+            logger.Trace("Validating User");
 
-            return userCache;
+            var apiUser = await GetCurrentUserInternal();
+            var apiUsername = apiUser.Login;
+
+            var cachedUsername = keychain.Connections.First().Username;
+
+            if (apiUsername != cachedUsername)
+            {
+                throw new TokenUsernameMismatchException(cachedUsername, apiUsername);
+            }
         }
 
         private async Task<bool> 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;
                 }
 
@@ -288,6 +301,8 @@ private async Task<bool> LoadKeychainInternal()
                 var uriString = keychain.Connections.First().Host;
                 var keychainAdapter = await keychain.Load(uriString);
 
+                logger.Trace("LoadKeychainInternal: Loaded");
+
                 return keychainAdapter.OctokitCredentials != Credentials.Anonymous;
             }
 
@@ -296,23 +311,54 @@ private async Task<bool> LoadKeychainInternal()
             return false;
         }
 
-        public async Task<bool> 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);
-                }
+                throw new KeychainEmptyException();
             }
-            catch
-            {
-                return false;
-            }
-            return true;
         }
     }
+
+    class GitHubUser
+    {
+        public string Name { get; set; }
+        public string Login { get; set; }
+    }
+
+    class GitHubRepository
+    {
+        public string Name { get; set; }
+        public string CloneUrl { get; set; }
+    }
+
+    class ApiClientException : Exception
+    {
+        public ApiClientException()
+        { }
+
+        public ApiClientException(string message) : base(message)
+        { }
+
+        public ApiClientException(string message, Exception innerException) : base(message, innerException)
+        { }
+    }
+
+    class TokenUsernameMismatchException : ApiClientException
+    {
+        public string CachedUsername { get; }
+        public string CurrentUsername { get; }
+
+        public TokenUsernameMismatchException(string cachedUsername, string currentUsername)
+        {
+            CachedUsername = cachedUsername;
+            CurrentUsername = currentUsername;
+        }
+    }
+
+    class KeychainEmptyException : ApiClientException
+    {
+        public KeychainEmptyException()
+        { }
+    }
 }

src/GitHub.Api/Application/IApiClient.cs

@@ -9,14 +9,13 @@ interface IApiClient
     {
         HostAddress HostAddress { get; }
         UriString OriginalUrl { get; }
-        Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null);
-        Task GetOrganizations(Action<IList<Organization>> callback);
+        Task CreateRepository(NewRepository newRepository, Action<GitHubRepository, Exception> callback, string organization = null);
+        Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null);
         Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);
         Task ContinueLogin(LoginResult loginResult, string code);
         Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode);
-        Task<bool> ValidateCredentials();
         Task Logout(UriString host);
-        Task GetCurrentUser(Action<Octokit.User> callback);
-        Task LoadKeychain(Action<bool> callback);
+        Task GetCurrentUser(Action<GitHubUser> callback);
+        Task ValidateCurrentUser(Action onSuccess, Action<Exception> onError = null);
     }
 }

src/GitHub.Api/Application/OctokitExtensions.cs

@@ -0,0 +1,21 @@
+namespace GitHub.Unity
+{
+    static class OctokitExtensions
+    {
+        public static GitHubUser ToGitHubUser(this Octokit.User user)
+        {
+            return new GitHubUser() {
+                Name = user.Name,
+                Login = user.Login,
+            };
+        }
+
+        public static GitHubRepository ToGitHubRepository(this Octokit.Repository repository)
+        {
+            return new GitHubRepository {
+                Name = repository.Name,
+                CloneUrl = repository.CloneUrl
+            };
+        }
+    }
+}

src/GitHub.Api/Application/Organization.cs

@@ -0,0 +1,8 @@
+namespace GitHub.Unity
+{
+    class Organization
+    {
+        public string Name { get; set; }
+        public string Login { get; set; }
+    }
+}

src/GitHub.Api/Authentication/Keychain.cs

@@ -68,7 +68,7 @@ public async Task<IKeychainAdapter> 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); 

src/GitHub.Api/GitHub.Api.csproj

@@ -99,6 +99,8 @@
     <Compile Include="Application\ApplicationInfo.cs" />
     <Compile Include="Application\LoginResult.cs" />
     <Compile Include="Application\AppConfiguration.cs" />
+    <Compile Include="Application\OctokitExtensions.cs" />
+    <Compile Include="Application\Organization.cs" />
     <Compile Include="Extensions\ListExtensions.cs" />
     <Compile Include="Git\Tasks\GitLfsVersionTask.cs" />
     <Compile Include="Git\Tasks\GitVersionTask.cs" />

src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs

@@ -591,7 +591,6 @@ public static GUIStyle AuthHeaderBoxStyle
                 {
                     authHeaderBoxStyle = new GUIStyle(HeaderBoxStyle);
                     authHeaderBoxStyle.name = "AuthHeaderBoxStyle";
-                    authHeaderBoxStyle.padding = new RectOffset(10, 10, 0, 5);
                 }
                 return authHeaderBoxStyle;
             }