Merge pull request #310 from github/shana/thinking-about-auth

Fixing authentication issues

Author: StanleyGoldman

src/GitHub.Api/Api/ApiClient.cs

@@ -7,32 +7,39 @@ namespace GitHub.Unity
 {
     class ApiClient : IApiClient
     {
+        public static IApiClient Create(UriString repositoryUrl, IKeychain keychain, IAppConfiguration appConfiguration)
+        {
+            var credentialStore = keychain.Connect(repositoryUrl);
+            var hostAddress = HostAddress.Create(repositoryUrl);
+
+            return new ApiClient(repositoryUrl, keychain,
+                new GitHubClient(appConfiguration.ProductHeader, credentialStore, hostAddress.ApiUri));
+        }
+
         private static readonly Unity.ILogging logger = Unity.Logging.GetLogger<ApiClient>();
         public HostAddress HostAddress { get; }
         public UriString OriginalUrl { get; }
 
+        private readonly IKeychain keychain;
         private readonly IGitHubClient githubClient;
-        private readonly IGitHubClient githubAppClient;
-        private readonly ICredentialManager credentialManager;
         private readonly ILoginManager loginManager;
         private static readonly SemaphoreSlim sem = new SemaphoreSlim(1);
 
         Octokit.Repository repositoryCache = new Octokit.Repository();
         string owner;
         bool? isEnterprise;
 
-        public ApiClient(UriString hostUrl, ICredentialManager credentialManager, IGitHubClient githubClient, IGitHubClient githubAppClient)
+        public ApiClient(UriString hostUrl, IKeychain keychain, IGitHubClient githubClient)
         {
             Guard.ArgumentNotNull(hostUrl, nameof(hostUrl));
-            Guard.ArgumentNotNull(credentialManager, nameof(credentialManager));
+            Guard.ArgumentNotNull(keychain, nameof(keychain));
             Guard.ArgumentNotNull(githubClient, nameof(githubClient));
 
             HostAddress = HostAddress.Create(hostUrl);
             OriginalUrl = hostUrl;
+            this.keychain = keychain;
             this.githubClient = githubClient;
-            this.githubAppClient = githubAppClient;
-            this.credentialManager = credentialManager;
-            loginManager = new LoginManager(credentialManager, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret);
+            loginManager = new LoginManager(keychain, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret);
         }
 
         public async void GetRepository(Action<Octokit.Repository> callback)
@@ -54,6 +61,7 @@ public async Task Login(string username, string password, Action<LoginResult> ne
             }
             catch (Exception ex)
             {
+                logger.Warning(ex);
                 result(false, ex.Message);
                 return;
             }
@@ -176,10 +184,12 @@ public async Task<bool> ValidateCredentials()
         {
             try
             {
-                var credential = await credentialManager.Load(OriginalUrl);
-                if (credential != null)
+                var store = keychain.Connect(OriginalUrl);
+
+                if (store.OctokitCredentials != Credentials.Anonymous)
                 {
-                    await githubAppClient.Authorization.CheckApplicationAuthentication(ApplicationInfo.ClientId, credential.Token);
+                    var credential = store.Credential;
+                    await githubClient.Authorization.CheckApplicationAuthentication(ApplicationInfo.ClientId, credential.Token);
                 }
             }
             catch

src/GitHub.Api/Api/ApiClientFactory.cs

@@ -8,6 +8,7 @@ class ApplicationManagerBase : IApplicationManager
     {
         protected static readonly ILogging logger = Logging.GetLogger<IApplicationManager>();
 
+        private AppConfiguration appConfiguration;
         private RepositoryLocator repositoryLocator;
         private RepositoryManager repositoryManager;
 
@@ -30,8 +31,7 @@ protected void Initialize(IUIDispatcher uiDispatcher)
 
             Platform = new Platform(Environment, FileSystem, uiDispatcher);
             ProcessManager = new ProcessManager(Environment, Platform.GitEnvironment, CancellationToken);
-            Platform.Initialize(ProcessManager);
-            ApiClientFactory.Instance = new ApiClientFactory(new AppConfiguration(), Platform.CredentialManager);
+            Platform.Initialize(Environment, ProcessManager);
         }
 
         public virtual Task Run()
@@ -178,6 +178,14 @@ public void Dispose()
             Dispose(true);
         }
 
+        public AppConfiguration AppConfiguration
+        {
+            get
+            {
+                return appConfiguration ?? (appConfiguration = new AppConfiguration());
+            }
+        }
+
         public virtual IEnvironment Environment { get; set; }
         public IFileSystem FileSystem { get; protected set; }
         public IPlatform Platform { get; protected set; }

src/GitHub.Api/Api/IApiClientFactory.cs

@@ -0,0 +1,17 @@
+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);
+        void UpdateToken(UriString host, string token);
+        void Save(ICredential credential);
+        void Initialize();
+        bool HasKeys { get; }
+    }
+}

src/GitHub.Api/ApplicationManagerBase.cs

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

src/GitHub.Api/Authentication/AppCredentialStore.cs

@@ -5,11 +5,12 @@ namespace GitHub.Unity
 {
     interface IPlatform
     {
-        Task<IPlatform> Initialize(IProcessManager processManager);
+        Task<IPlatform> Initialize(IEnvironment environment, IProcessManager processManager);
         IProcessEnvironment GitEnvironment { get; }
         ICredentialManager CredentialManager { get; }
         IEnvironment Environment { get; }
         IProcessManager ProcessManager { get; }
         IUIDispatcher UIDispatcher { get; }
+        IKeychain Keychain { get; }
     }
 }