Fix login after failure and 2fa too

shana · shana · commit 83c2e9f4fc66 · 2017-05-08T15:46:46.000+02:00
diff --git a/src/GitHub.Api/Authentication/IKeychain.cs b/src/GitHub.Api/Authentication/IKeychain.cs
@@ -6,12 +6,12 @@ interface IKeychain
     {
         KeychainAdapter Connect(UriString host);
         Task<KeychainAdapter> Load(UriString host);
-        Task Clear(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; }
-        Task ClearAll();
     }
 }
diff --git a/src/GitHub.Api/Authentication/ILoginManager.cs b/src/GitHub.Api/Authentication/ILoginManager.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);
     }
 }
diff --git a/src/GitHub.Api/Authentication/Keychain.cs b/src/GitHub.Api/Authentication/Keychain.cs
@@ -129,30 +129,30 @@ private void WriteCacheToDisk()
             cachePath.WriteAllText(json);
         }
 
-        public async Task Clear(UriString host)
+        /// <summary>
+        /// Call Flush() to apply these changes
+        /// </summary>
+        public void Clear(UriString host)
         {
             logger.Trace("Clear Host:{0}", host);
        
             // delete connection in the connection list
             connectionCache.Remove(host);
-            WriteCacheToDisk();
 
             // delete credential in octokit store
-            keychainAdapters.Remove(host);
-
-            // delete credential from credential manager
-            await credentialManager.Delete(host);
+            FindOrCreateAdapter(host).Clear();
         }
 
-        public async Task ClearAll()
+        /// <summary>
+        /// Call Flush() to apply these changes
+        /// </summary>
+        public void Clear()
         {
-            var uriStrings = keychainAdapters.Keys.Union(connectionCache.Keys).Distinct().ToArray();
-            logger.Trace("ClearAll Count:{0}", uriStrings.Length);
-
-            foreach (var uriString in uriStrings)
+            foreach (var k in keychainAdapters.Values.ToArray())
             {
-                await Clear(uriString);
+                k.Clear();
             }
+            connectionCache.Clear();
         }
 
         public async Task Flush(UriString host)
@@ -171,6 +171,8 @@ 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)
diff --git a/src/GitHub.Api/Authentication/KeychainAdapter.cs b/src/GitHub.Api/Authentication/KeychainAdapter.cs
@@ -19,6 +19,11 @@ public void UpdateToken(string token)
             Credential.UpdateToken(token);
         }
 
+        public void Clear()
+        {
+            OctokitCredentials = Credentials.Anonymous;
+        }
+
         /// <summary>
         /// Implementation for Octokit
         /// </summary>
diff --git a/src/GitHub.Api/Authentication/LoginManager.cs b/src/GitHub.Api/Authentication/LoginManager.cs
@@ -110,15 +110,15 @@ public async Task<LoginResultData> Login(
             {
                 logger.Warning(e, "Login LoginAttemptsExceededException: {0}", e.Message);
 
-                await keychain.Clear(host);
+                keychain.Clear(host);
                 return new LoginResultData(LoginResultCodes.LockedOut, Localization.LockedOut, host);
             }
             catch (ApiValidationException e)
             {
                 logger.Warning(e, "Login ApiValidationException: {0}", e.Message);
 
                 var message = e.ApiError.FirstErrorMessageSafe();
-                await keychain.Clear(host);
+                keychain.Clear(host);
                 return new LoginResultData(LoginResultCodes.Failed, message, host);
             }
             catch (Exception e)
@@ -134,7 +134,7 @@ public async Task<LoginResultData> Login(
                 }
                 else
                 {
-                    await keychain.Clear(host);
+                    keychain.Clear(host);
                     return new LoginResultData(LoginResultCodes.Failed, Localization.LoginFailed, host);
                 }
             }
@@ -161,11 +161,11 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
                     twofacode);
                 EnsureNonNullAuthorization(auth);
 
-                var credentialStore = await keychain.Load(host);
-                var credentials = credentialStore.Credential;
-                credentials.UpdateToken(auth.Token);
-                
-                keychain.Save(credentials);
+
+                var keychainAdapter = keychain.Connect(host);
+                keychainAdapter.UpdateToken(auth.Token);
+                await keychain.Flush(host);
+
                 return new LoginResultData(LoginResultCodes.Success, "", host);
             }
             catch (TwoFactorAuthorizationException e)
@@ -179,27 +179,27 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
                 logger.Debug(e, "2FA ApiValidationException: {0}", e.Message);
 
                 var message = e.ApiError.FirstErrorMessageSafe();
-                await keychain.Clear(host);
+                keychain.Clear(host);
                 return new LoginResultData(LoginResultCodes.Failed, message, host);
             }
             catch (Exception e)
             {
                 logger.Debug(e, "Exception: {0}", e.Message);
 
-                await keychain.Clear(host);
+                keychain.Clear(host);
                 return new LoginResultData(LoginResultCodes.Failed, e.Message, host);
             }
         }
 
         /// <inheritdoc/>
-        public async Task Logout(UriString hostAddress, IGitHubClient client)
+        public async Task Logout(UriString hostAddress)
         {
             Guard.ArgumentNotNull(hostAddress, nameof(hostAddress));
-            Guard.ArgumentNotNull(client, nameof(client));
 
             logger.Trace("Logout");
 
-            await keychain.Clear(hostAddress);
+            keychain.Clear(hostAddress);
+            await keychain.Flush(hostAddress);
         }
 
         private async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(

Original file line number	Diff line number	Diff line change
`@@ -6,12 +6,12 @@ interface IKeychain`
`6`	`6`	`{`
`7`	`7`	`KeychainAdapter Connect(UriString host);`
`8`	`8`	`Task<KeychainAdapter> Load(UriString host);`
`9`		`- Task Clear(UriString host);`
	`9`	`+ void Clear(UriString host);`
	`10`	`+ void Clear();`
`10`	`11`	`Task Flush(UriString host);`
`11`	`12`	`void UpdateToken(UriString host, string token);`
`12`	`13`	`void Save(ICredential credential);`
`13`	`14`	`void Initialize();`
`14`	`15`	`bool HasKeys { get; }`
`15`		`- Task ClearAll();`
`16`	`16`	`}`
`17`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,6 @@ interface ILoginManager`
`26`	`26`	`/// Logs out of GitHub server.`
`27`	`27`	`/// </summary>`
`28`	`28`	`/// <param name="hostAddress">The address of the server.</param>`
`29`		`- Task Logout(UriString hostAddress, IGitHubClient client);`
	`29`	`+ Task Logout(UriString hostAddress);`
`30`	`30`	`}`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -129,30 +129,30 @@ private void WriteCacheToDisk()`
`129`	`129`	`cachePath.WriteAllText(json);`
`130`	`130`	`}`
`131`	`131`
`132`		`- public async Task Clear(UriString host)`
	`132`	`+ /// <summary>`
	`133`	`+ /// Call Flush() to apply these changes`
	`134`	`+ /// </summary>`
	`135`	`+ public void Clear(UriString host)`
`133`	`136`	`{`
`134`	`137`	`logger.Trace("Clear Host:{0}", host);`
`135`	`138`
`136`	`139`	`// delete connection in the connection list`
`137`	`140`	`connectionCache.Remove(host);`
`138`		`- WriteCacheToDisk();`
`139`	`141`
`140`	`142`	`// delete credential in octokit store`
`141`		`- keychainAdapters.Remove(host);`
`142`		`-`
`143`		`- // delete credential from credential manager`
`144`		`- await credentialManager.Delete(host);`
	`143`	`+ FindOrCreateAdapter(host).Clear();`
`145`	`144`	`}`
`146`	`145`
`147`		`- public async Task ClearAll()`
	`146`	`+ /// <summary>`
	`147`	`+ /// Call Flush() to apply these changes`
	`148`	`+ /// </summary>`
	`149`	`+ public void Clear()`
`148`	`150`	`{`
`149`		`- var uriStrings = keychainAdapters.Keys.Union(connectionCache.Keys).Distinct().ToArray();`
`150`		`- logger.Trace("ClearAll Count:{0}", uriStrings.Length);`
`151`		`-`
`152`		`- foreach (var uriString in uriStrings)`
	`151`	`+ foreach (var k in keychainAdapters.Values.ToArray())`
`153`	`152`	`{`
`154`		`- await Clear(uriString);`
	`153`	`+ k.Clear();`
`155`	`154`	`}`
	`155`	`+ connectionCache.Clear();`
`156`	`156`	`}`
`157`	`157`
`158`	`158`	`public async Task Flush(UriString host)`
`@@ -171,6 +171,8 @@ public async Task Flush(UriString host)`
`171`	`171`	`}`
`172`	`172`
`173`	`173`	`// create new connection in the connection cache for this host`
	`174`	`+ if (connectionCache.ContainsKey(host))`
	`175`	`+ connectionCache.Remove(host);`
`174`	`176`	`connectionCache.Add(host, new Connection { Host = host, Username = credentialAdapter.OctokitCredentials.Login });`
`175`	`177`
`176`	`178`	`// flushes credential cache to disk (host and username only)`
Original file line number	Diff line number	Diff line change
`@@ -110,15 +110,15 @@ public async Task<LoginResultData> Login(`
`110`	`110`	`{`
`111`	`111`	`logger.Warning(e, "Login LoginAttemptsExceededException: {0}", e.Message);`
`112`	`112`
`113`		`- await keychain.Clear(host);`
	`113`	`+ keychain.Clear(host);`
`114`	`114`	`return new LoginResultData(LoginResultCodes.LockedOut, Localization.LockedOut, host);`
`115`	`115`	`}`
`116`	`116`	`catch (ApiValidationException e)`
`117`	`117`	`{`
`118`	`118`	`logger.Warning(e, "Login ApiValidationException: {0}", e.Message);`
`119`	`119`
`120`	`120`	`var message = e.ApiError.FirstErrorMessageSafe();`
`121`		`- await keychain.Clear(host);`
	`121`	`+ keychain.Clear(host);`
`122`	`122`	`return new LoginResultData(LoginResultCodes.Failed, message, host);`
`123`	`123`	`}`
`124`	`124`	`catch (Exception e)`
`@@ -134,7 +134,7 @@ public async Task<LoginResultData> Login(`
`134`	`134`	`}`
`135`	`135`	`else`
`136`	`136`	`{`
`137`		`- await keychain.Clear(host);`
	`137`	`+ keychain.Clear(host);`
`138`	`138`	`return new LoginResultData(LoginResultCodes.Failed, Localization.LoginFailed, host);`
`139`	`139`	`}`
`140`	`140`	`}`
`@@ -161,11 +161,11 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData`
`161`	`161`	`twofacode);`
`162`	`162`	`EnsureNonNullAuthorization(auth);`
`163`	`163`
`164`		`- var credentialStore = await keychain.Load(host);`
`165`		`- var credentials = credentialStore.Credential;`
`166`		`- credentials.UpdateToken(auth.Token);`
`167`		`-`
`168`		`- keychain.Save(credentials);`
	`164`	`+`
	`165`	`+ var keychainAdapter = keychain.Connect(host);`
	`166`	`+ keychainAdapter.UpdateToken(auth.Token);`
	`167`	`+ await keychain.Flush(host);`
	`168`	`+`
`169`	`169`	`return new LoginResultData(LoginResultCodes.Success, "", host);`
`170`	`170`	`}`
`171`	`171`	`catch (TwoFactorAuthorizationException e)`
`@@ -179,27 +179,27 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData`
`179`	`179`	`logger.Debug(e, "2FA ApiValidationException: {0}", e.Message);`
`180`	`180`
`181`	`181`	`var message = e.ApiError.FirstErrorMessageSafe();`
`182`		`- await keychain.Clear(host);`
	`182`	`+ keychain.Clear(host);`
`183`	`183`	`return new LoginResultData(LoginResultCodes.Failed, message, host);`
`184`	`184`	`}`
`185`	`185`	`catch (Exception e)`
`186`	`186`	`{`
`187`	`187`	`logger.Debug(e, "Exception: {0}", e.Message);`
`188`	`188`
`189`		`- await keychain.Clear(host);`
	`189`	`+ keychain.Clear(host);`
`190`	`190`	`return new LoginResultData(LoginResultCodes.Failed, e.Message, host);`
`191`	`191`	`}`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`/// <inheritdoc/>`
`195`		`- public async Task Logout(UriString hostAddress, IGitHubClient client)`
	`195`	`+ public async Task Logout(UriString hostAddress)`
`196`	`196`	`{`
`197`	`197`	`Guard.ArgumentNotNull(hostAddress, nameof(hostAddress));`
`198`		`- Guard.ArgumentNotNull(client, nameof(client));`
`199`	`198`
`200`	`199`	`logger.Trace("Logout");`
`201`	`200`
`202`		`- await keychain.Clear(hostAddress);`
	`201`	`+ keychain.Clear(hostAddress);`
	`202`	`+ await keychain.Flush(hostAddress);`
`203`	`203`	`}`
`204`	`204`
`205`	`205`	`private async Task<ApplicationAuthorization> CreateAndDeleteExistingApplicationAuthorization(`