Merge remote-tracking branch 'remotes/origin/master' into fixes/open-source-ready

Author: StanleyGoldman

build.sh

@@ -32,7 +32,7 @@ else
 	nuget restore
 fi
 
-xbuild GitHub.Unity.sln /verbosity:normal /property:Configuration=$Configuration /target:$Target
+xbuild GitHub.Unity.sln /verbosity:normal /property:Configuration=$Configuration /target:$Target || true
 
 cp -r unity/PackageProject/Assets/Editor/GitHub ../github-unity-test/GitHubExtensionProject/Assets/Editor || true
 

src/GitHub.Api/Api/ApiClient.cs

@@ -49,6 +49,16 @@ public async void GetRepository(Action<Octokit.Repository> callback)
             callback(repo);
         }
 
+        public async void Logout(UriString host)
+        {
+            await LogoutInternal(host);
+        }
+
+        private async Task LogoutInternal(UriString host)
+        {
+            await loginManager.Logout(host);
+        }
+
         public async Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result)
         {
             Guard.ArgumentNotNull(need2faCode, "need2faCode");

src/GitHub.Api/Api/IApiClient.cs

@@ -13,5 +13,6 @@ interface IApiClient
         Task ContinueLogin(LoginResult loginResult, string code);
         Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode);
         Task<bool> ValidateCredentials();
+        void Logout(UriString host);
     }
 }

src/GitHub.Api/ApplicationManagerBase.cs

@@ -1,6 +1,8 @@
 using System;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Octokit;
 
 namespace GitHub.Unity
 {
@@ -42,8 +44,29 @@ protected void Initialize()
 
         public virtual ITask Run()
         {
+            Logger.Trace("Run");
+
             var progress = new ProgressReport();
-            return new ActionTask(SetupAndRestart(progress));
+            return new ActionTask(SetupAndRestart(progress))
+                .Then(new ActionTask(LoadKeychain()));
+        }
+
+        private async Task LoadKeychain()
+        {
+            Logger.Trace("Loading Keychain");
+
+            var firstConnection = Platform.Keychain.Connections.FirstOrDefault();
+            if (firstConnection == null)
+            {
+                Logger.Trace("No Host Found");
+            }
+            else
+            {
+                Logger.Trace("Loading Connection to Host:\"{0}\"", firstConnection);
+                var keychainAdapter = await Platform.Keychain.Load(firstConnection).SafeAwait();
+                if (keychainAdapter.OctokitCredentials == Credentials.Anonymous)
+                { }
+            }
         }
 
         protected abstract string DetermineInstallationPath();
@@ -84,6 +107,8 @@ public virtual ITask RestartRepository()
 
         private async Task SetupAndRestart(ProgressReport progress)
         {
+            Logger.Trace("SetupAndRestart");
+
             var gitSetup = new GitSetup(Environment, CancellationToken);
             var expectedPath = gitSetup.GitInstallationPath;
             var setupDone = await gitSetup.SetupIfNeeded(progress.Percentage, progress.Remaining);

src/GitHub.Api/Authentication/IKeychain.cs

@@ -1,17 +1,19 @@
-using System.Threading.Tasks;
+using System.Collections.Generic;
+using System.Threading.Tasks;
 
 namespace GitHub.Unity
 {
     interface IKeychain
     {
-        KeychainAdapter Connect(UriString host);
-        Task<KeychainAdapter> Load(UriString host);
-        void Clear(UriString host);
-        void Clear();
-        Task Flush(UriString host);
+        IKeychainAdapter Connect(UriString host);
+        Task<IKeychainAdapter> Load(UriString host);
+        Task Clear(UriString host, bool deleteFromCredentialManager);
+        Task Save(UriString host);
         void UpdateToken(UriString host, string token);
-        void Save(ICredential credential);
+        void SetCredentials(ICredential credential);
         void Initialize();
+        IList<UriString> Connections { get; }
         bool HasKeys { get; }
+        void SetToken(UriString host, string token);
     }
 }

src/GitHub.Api/Authentication/ILoginManager.cs

@@ -26,6 +26,7 @@ interface ILoginManager
         /// Logs out of GitHub server.
         /// </summary>
         /// <param name="hostAddress">The address of the server.</param>
+        /// <inheritdoc/>
         Task Logout(UriString hostAddress);
     }
 }

src/GitHub.Api/Authentication/Keychain.cs

@@ -39,28 +39,42 @@ class Keychain : IKeychain
         private Dictionary<UriString, Connection> connectionCache = new Dictionary<UriString, Connection>();
 
         // cached credentials loaded from git to pass to GitHub/ApiClient
-        private Dictionary<UriString, KeychainAdapter> keychainAdapters =
-            new Dictionary<UriString, KeychainAdapter>();
+        private readonly Dictionary<UriString, KeychainAdapter> keychainAdapters
+            = new Dictionary<UriString, KeychainAdapter>();
 
         public Keychain(IEnvironment environment, ICredentialManager credentialManager)
         {
+            Guard.ArgumentNotNull(environment, nameof(environment));
+            Guard.ArgumentNotNull(credentialManager, nameof(credentialManager));
+
+            Guard.NotNull(environment, environment.UserCachePath, nameof(environment.UserCachePath));
+
             cachePath = environment.UserCachePath.Combine(ConnectionFile);
             this.credentialManager = credentialManager;
         }
 
-        public KeychainAdapter Connect(UriString host)
+        public IKeychainAdapter Connect(UriString host)
         {
             return FindOrCreateAdapter(host);
         }
 
-        public async Task<KeychainAdapter> Load(UriString host)
+        public async Task<IKeychainAdapter> Load(UriString host)
         {
-            logger.Trace("Load Host:{0}", host);
-
             var keychainAdapter = FindOrCreateAdapter(host);
 
+            logger.Trace("Load KeychainAdapter Host:\"{0}\"", host);
             var keychainItem = await credentialManager.Load(host);
-            keychainAdapter.Set(keychainItem);
+
+            if (keychainItem == null)
+            {
+                logger.Warning("Cannot load host from credential manager; removing from cache");
+                await Clear(host, false);
+            }
+            else
+            {
+                logger.Trace("Loading KeychainItem:{0}", keychainItem.ToString());
+                keychainAdapter.Set(keychainItem);
+            }
 
             return keychainAdapter;
         }
@@ -89,6 +103,7 @@ private void ReadCacheFromDisk()
             logger.Trace("ReadCacheFromDisk Path:{0}", cachePath.ToString());
 
             ConnectionCacheItem[] connections = null;
+            
             if (cachePath.FileExists())
             {
                 var json = cachePath.ReadAllText();
@@ -122,7 +137,7 @@ private void WriteCacheToDisk()
             var connectionCacheItems =
                 connectionCache.Select(
                     pair =>
-                        new ConnectionCacheItem() {
+                        new ConnectionCacheItem {
                             Host = pair.Value.Host.ToString(),
                             Username = pair.Value.Username
                         }).ToArray();
@@ -131,35 +146,27 @@ private void WriteCacheToDisk()
             cachePath.WriteAllText(json);
         }
 
-        /// <summary>
-        /// Call Flush() to apply these changes
-        /// </summary>
-        public void Clear(UriString host)
+        public async Task Clear(UriString host, bool deleteFromCredentialManager)
         {
             logger.Trace("Clear Host:{0}", host);
-       
+
             // delete connection in the connection list
             connectionCache.Remove(host);
 
-            // delete credential in octokit store
+            //clear octokit credentials
             FindOrCreateAdapter(host).Clear();
-        }
 
-        /// <summary>
-        /// Call Flush() to apply these changes
-        /// </summary>
-        public void Clear()
-        {
-            foreach (var k in keychainAdapters.Values.ToArray())
+            WriteCacheToDisk();
+
+            if (deleteFromCredentialManager)
             {
-                k.Clear();
+                await credentialManager.Delete(host);
             }
-            connectionCache.Clear();
         }
 
-        public async Task Flush(UriString host)
+        public async Task Save(UriString host)
         {
-            logger.Trace("Flush: {0}", host);
+            logger.Trace("Save: {0}", host);
 
             KeychainAdapter credentialAdapter;
             if (!keychainAdapters.TryGetValue(host, out credentialAdapter))
@@ -175,6 +182,7 @@ public async Task Flush(UriString host)
             // create new connection in the connection cache for this host
             if (connectionCache.ContainsKey(host))
                 connectionCache.Remove(host);
+
             connectionCache.Add(host, new Connection { Host = host, Username = credentialAdapter.OctokitCredentials.Login });
 
             // flushes credential cache to disk (host and username only)
@@ -185,12 +193,20 @@ public async Task Flush(UriString host)
             await credentialManager.Save(credentialAdapter.Credential);
         }
 
-        public void Save(ICredential credential)
+        public void SetCredentials(ICredential credential)
         {
-            logger.Trace("Save Host:{0}", credential.Host);
+            logger.Trace("SetCredentials Host:{0}", credential.Host);
 
-            var credentialAdapter = FindOrCreateAdapter(credential.Host);
-            credentialAdapter.Set(credential);
+            var keychainAdapter = keychainAdapters[credential.Host];
+            keychainAdapter.Set(credential);
+        }
+
+        public void SetToken(UriString host, string token)
+        {
+            logger.Trace("SetToken Host:{0}", host);
+
+            var keychainAdapter = keychainAdapters[host];
+            keychainAdapter.UpdateToken(token);
         }
 
         public void UpdateToken(UriString host, string token)
@@ -207,6 +223,8 @@ public void UpdateToken(UriString host, string token)
             keychainItem.UpdateToken(token);
         }
 
+        public IList<UriString> Connections => connectionCache.Keys.ToArray();
+
         public bool HasKeys => connectionCache.Any();
     }
 }

src/GitHub.Api/Authentication/KeychainAdapter.cs

@@ -3,7 +3,7 @@
 
 namespace GitHub.Unity
 {
-    class KeychainAdapter : ICredentialStore
+    class KeychainAdapter : IKeychainAdapter
     {
         public Credentials OctokitCredentials { get; private set; } = Credentials.Anonymous;
         public ICredential Credential { get; private set; }
@@ -33,4 +33,10 @@ Task<Credentials> ICredentialStore.GetCredentials()
             return TaskEx.FromResult(OctokitCredentials);
         }
     }
+
+    interface IKeychainAdapter: ICredentialStore
+    {
+        Credentials OctokitCredentials { get; }
+        ICredential Credential { get; }
+    }
 }

src/GitHub.Api/Authentication/LoginManager.cs

@@ -3,6 +3,7 @@
 using System.Threading.Tasks;
 using Octokit;
 using GitHub.Unity;
+using Octokit_Extensions35;
 
 namespace GitHub.Unity
 {
@@ -70,9 +71,8 @@ public async Task<LoginResultData> Login(
 
             // Start by saving the username and password, these will be used by the `IGitHubClient`
             // until an authorization token has been created and acquired:
-            var keychainAdapter = keychain.Connect(host);
-            var keychainItem = new Credential(host, username, password);
-            keychainAdapter.Set(keychainItem);
+            keychain.Connect(host);
+            keychain.SetCredentials(new Credential(host, username, password));
 
             var newAuth = new NewAuthorization
             {
@@ -110,15 +110,15 @@ public async Task<LoginResultData> Login(
             {
                 logger.Warning(e, "Login LoginAttemptsExceededException: {0}", e.Message);
 
-                keychain.Clear(host);
+                await keychain.Clear(host, false);
                 return new LoginResultData(LoginResultCodes.LockedOut, Localization.LockedOut, host);
             }
             catch (ApiValidationException e)
             {
                 logger.Warning(e, "Login ApiValidationException: {0}", e.Message);
 
                 var message = e.ApiError.FirstErrorMessageSafe();
-                keychain.Clear(host);
+                await keychain.Clear(host, false);
                 return new LoginResultData(LoginResultCodes.Failed, message, host);
             }
             catch (Exception e)
@@ -134,13 +134,13 @@ public async Task<LoginResultData> Login(
                 }
                 else
                 {
-                    keychain.Clear(host);
+                    await keychain.Clear(host, false);
                     return new LoginResultData(LoginResultCodes.Failed, Localization.LoginFailed, host);
                 }
             }
 
-            keychainAdapter.UpdateToken(auth.Token);
-            await keychain.Flush(host);
+            keychain.SetToken(host, auth.Token);
+            await keychain.Save(host);
 
             return new LoginResultData(LoginResultCodes.Success, "Success", host);
         }
@@ -161,10 +161,8 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
                     twofacode);
                 EnsureNonNullAuthorization(auth);
 
-
-                var keychainAdapter = keychain.Connect(host);
-                keychainAdapter.UpdateToken(auth.Token);
-                await keychain.Flush(host);
+                keychain.SetToken(host, auth.Token);
+                await keychain.Save(host);
 
                 return new LoginResultData(LoginResultCodes.Success, "", host);
             }
@@ -179,14 +177,14 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
                 logger.Debug(e, "2FA ApiValidationException: {0}", e.Message);
 
                 var message = e.ApiError.FirstErrorMessageSafe();
-                keychain.Clear(host);
+                await keychain.Clear(host, false);
                 return new LoginResultData(LoginResultCodes.Failed, message, host);
             }
             catch (Exception e)
             {
                 logger.Debug(e, "Exception: {0}", e.Message);
 
-                keychain.Clear(host);
+                await keychain.Clear(host, false);
                 return new LoginResultData(LoginResultCodes.Failed, e.Message, host);
             }
         }
@@ -196,10 +194,7 @@ public async Task Logout(UriString hostAddress)
         {
             Guard.ArgumentNotNull(hostAddress, nameof(hostAddress));
 
-            logger.Trace("Logout");
-
-            keychain.Clear(hostAddress);
-            await keychain.Flush(hostAddress);
+            await new ActionTask(keychain.Clear(hostAddress, true)).StartAwait();
         }
 
         private async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(