Loading the Keychain before it gets used in the api

Author: StanleyGoldman

src/GitHub.Api/Application/ApiClient.cs

@@ -29,7 +29,6 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain)
         private readonly ILoginManager loginManager;
         private static readonly SemaphoreSlim sem = new SemaphoreSlim(1);
 
-        Octokit.Repository repositoryCache = new Octokit.Repository();
         IList<Organization> organizationsCache;
         Octokit.User userCache;
 
@@ -49,13 +48,6 @@ public ApiClient(UriString hostUrl, IKeychain keychain, IGitHubClient githubClie
             loginManager = new LoginManager(keychain, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret);
         }
 
-        public async Task GetRepository(Action<Octokit.Repository> callback)
-        {
-            Guard.ArgumentNotNull(callback, "callback");
-            var repo = await GetRepositoryInternal();
-            callback(repo);
-        }
-
         public async Task Logout(UriString host)
         {
             await LogoutInternal(host);
@@ -182,45 +174,18 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             return result.Code == LoginResultCodes.Success;
         }
 
-        private async Task<Octokit.Repository> GetRepositoryInternal()
+        private async Task CreateRepositoryInternal(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization)
         {
             try
             {
-                if (owner == null)
+                logger.Trace("Creating repository");
+
+                if (!await EnsureKeychainLoaded())
                 {
-                    var ownerLogin = OriginalUrl.Owner;
-                    var repositoryName = OriginalUrl.RepositoryName;
-
-                    if (ownerLogin != null && repositoryName != null)
-                    {
-                        var repo = await githubClient.Repository.Get(ownerLogin, repositoryName);
-                        if (repo != null)
-                        {
-                            repositoryCache = repo;
-                        }
-                        owner = ownerLogin;
-                    }
+                    callback(null, new Exception("Keychain Not Loaded"));
+                    return;
                 }
-            }
-            // it'll throw if it's private or an enterprise instance requiring authentication
-            catch (ApiException apiex)
-            {
-                if (!HostAddress.IsGitHubDotCom(OriginalUrl.ToRepositoryUri()))
-                    isEnterprise = apiex.IsGitHubApiException();
-            }
-            catch {}
-            finally
-            {
-                sem.Release();
-            }
 
-            return repositoryCache;
-        }
-
-        private async Task CreateRepositoryInternal(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization)
-        {
-            try
-            {
                 Octokit.Repository repository;
                 if (!string.IsNullOrEmpty(organization))
                 {
@@ -252,6 +217,11 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             {
                 logger.Trace("Getting Organizations");
 
+                if (!await EnsureKeychainLoaded())
+                {
+                    return null;
+                }
+
                 var organizations = await githubClient.Organization.GetAllForCurrent();
 
                 logger.Trace("Obtained {0} Organizations", organizations?.Count.ToString() ?? "NULL");
@@ -276,6 +246,11 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             {
                 logger.Trace("Getting Organizations");
 
+                if (!await EnsureKeychainLoaded())
+                {
+                    return null;
+                }
+
                 userCache = await githubClient.User.Current();
             }
             catch(Exception ex)
@@ -287,6 +262,31 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             return userCache;
         }
 
+        private async Task<bool> EnsureKeychainLoaded()
+        {
+            logger.Trace("EnsureKeychainLoaded");
+
+            if (keychain.HasKeys)
+            {
+                if (!keychain.NeedsLoad)
+                {
+                    logger.Trace("EnsureKeychainLoaded: Has keys does not need load");
+                    return true;
+                }
+
+                logger.Trace("EnsureKeychainLoaded: Loading");
+
+                var uriString = keychain.Connections.First().Host;
+                var keychainAdapter = await keychain.Load(uriString);
+
+                return keychainAdapter.OctokitCredentials != Credentials.Anonymous;
+            }
+
+            logger.Trace("EnsureKeychainLoaded: No keys to load");
+
+            return false;
+        }
+
         public async Task<bool> ValidateCredentials()
         {
             try

src/GitHub.Api/Application/ApplicationManagerBase.cs

@@ -69,8 +69,6 @@ public virtual async Task Run(bool firstRun)
 
             RestartRepository();
             InitializeUI();
-
-            new ActionTask(new Task(() => LoadKeychain().Start())).Start();
         }
 
         public ITask InitializeRepository()
@@ -125,22 +123,6 @@ public void RestartRepository()
             }
         }
 
-        private async Task LoadKeychain()
-        {
-            Logger.Trace("Loading Keychain");
-
-            var firstConnection = Platform.Keychain.Hosts.FirstOrDefault();
-            if (firstConnection == null)
-            {
-                Logger.Trace("No Host Found");
-            }
-            else
-            {
-                Logger.Trace("Loading Connection to Host:\"{0}\"", firstConnection);
-                await Platform.Keychain.Load(firstConnection).SafeAwait();
-            }
-        }
-
         private async Task<NPath> DetermineGitExecutablePath(ProgressReport progress = null)
         {
             var gitExecutablePath = SystemSettings.Get(Constants.GitInstallPathKey)?.ToNPath();

src/GitHub.Api/Application/IApiClient.cs

@@ -9,7 +9,6 @@ interface IApiClient
     {
         HostAddress HostAddress { get; }
         UriString OriginalUrl { get; }
-        Task GetRepository(Action<Octokit.Repository> callback);
         Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null);
         Task GetOrganizations(Action<IList<Organization>> callback);
         Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);

src/GitHub.Api/Authentication/IKeychain.cs

@@ -15,6 +15,7 @@ interface IKeychain
         Connection[] Connections { get; }
         IList<UriString> Hosts { get; }
         bool HasKeys { get; }
+        bool NeedsLoad { get; }
         void SetToken(UriString host, string token);
     }
 }

src/GitHub.Api/Authentication/Keychain.cs

@@ -230,5 +230,7 @@ public void UpdateToken(UriString host, string token)
         public IList<UriString> Hosts => connectionCache.Keys.ToArray();
 
         public bool HasKeys => connectionCache.Any();
+
+        public bool NeedsLoad => HasKeys && FindOrCreateAdapter(connectionCache.First().Value.Host).OctokitCredentials == Credentials.Anonymous;
     }
 }