Merge branch 'master' into enhancements/more-metrics

StanleyGoldman · StanleyGoldman · commit 4addc6c99d12 · 2018-04-23T10:06:25.000-04:00
diff --git a/.gitignore b/.gitignore
@@ -6,4 +6,5 @@ _NCrunch_GitHub.Unity
 .DS_Store
 build/
 TestResult.xml
-submodules/
+submodules/
+*.stackdump
diff --git a/common/SolutionInfo.cs b/common/SolutionInfo.cs
@@ -31,6 +31,6 @@
 namespace System
 {
     internal static class AssemblyVersionInformation {
-        internal const string Version = "0.31.8";
+        internal const string Version = "0.32.0";
     }
 }
diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs
@@ -33,7 +33,7 @@ public ApiClient(UriString hostUrl, IKeychain keychain, IProcessManager processM
             this.taskManager = taskManager;
             this.nodeJsExecutablePath = nodeJsExecutablePath;
             this.octorunScriptPath = octorunScriptPath;
-            loginManager = new LoginManager(keychain, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret,
+            loginManager = new LoginManager(keychain,
                 processManager: processManager,
                 taskManager: taskManager,
                 nodeJsExecutablePath: nodeJsExecutablePath,
@@ -131,55 +131,6 @@ public async Task ContinueLogin(LoginResult loginResult, string code)
             loginResult.Callback(result.Code == LoginResultCodes.Success, result.Message);
         }
 
-        public async Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode)
-        {
-            Guard.ArgumentNotNull(need2faCode, "need2faCode");
-
-            LoginResultData res = null;
-            try
-            {
-                res = await loginManager.Login(OriginalUrl, username, password);
-            }
-            catch (Exception)
-            {
-                return false;
-            }
-
-            if (res.Code == LoginResultCodes.CodeRequired)
-            {
-                var resultCache = new LoginResult(res, null, null);
-                var code = need2faCode(resultCache);
-                return await ContinueLoginAsync(resultCache, need2faCode, code);
-            }
-            else
-            {
-                return res.Code == LoginResultCodes.Success;
-            }
-        }
-
-        public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginResult, string> need2faCode, string code)
-        {
-            LoginResultData result = null;
-            try
-            {
-                result = await loginManager.ContinueLogin(loginResult.Data, code);
-            }
-            catch (Exception)
-            {
-                return false;
-            }
-
-            if (result.Code == LoginResultCodes.CodeFailed)
-            {
-                var resultCache = new LoginResult(result, null, null);
-                code = need2faCode(resultCache);
-                if (String.IsNullOrEmpty(code))
-                    return false;
-                return await ContinueLoginAsync(resultCache, need2faCode, code);
-            }
-            return result.Code == LoginResultCodes.Success;
-        }
-
         private async Task<GitHubUser> GetCurrentUser()
         {
             //TODO: ONE_USER_LOGIN This assumes we only support one login
diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs
@@ -12,7 +12,6 @@ Task CreateRepository(string name, string description, bool isPrivate,
         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 Logout(UriString host);
         Task GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null);
     }
diff --git a/src/GitHub.Api/Authentication/Credential.cs b/src/GitHub.Api/Authentication/Credential.cs
@@ -16,9 +16,10 @@ public Credential(UriString host, string username, string token)
             this.Token = token;
         }
 
-        public void UpdateToken(string token)
+        public void UpdateToken(string token, string username)
         {
             this.Token = token;
+            this.Username = username;
         }
 
         public UriString Host { get; private set; }
diff --git a/src/GitHub.Api/Authentication/ICredentialManager.cs b/src/GitHub.Api/Authentication/ICredentialManager.cs
@@ -8,7 +8,7 @@ public interface ICredential : IDisposable
         UriString Host { get; }
         string Username { get; }
         string Token { get; }
-        void UpdateToken(string token);
+        void UpdateToken(string token, string username);
     }
 
     public interface ICredentialManager
diff --git a/src/GitHub.Api/Authentication/IKeychain.cs b/src/GitHub.Api/Authentication/IKeychain.cs
@@ -10,13 +10,12 @@ public interface IKeychain
         Task<IKeychainAdapter> Load(UriString host);
         Task Clear(UriString host, bool deleteFromCredentialManager);
         Task Save(UriString host);
-        void UpdateToken(UriString host, string token);
         void SetCredentials(ICredential credential);
         void Initialize();
         Connection[] Connections { get; }
         IList<UriString> Hosts { get; }
         bool HasKeys { get; }
-        void SetToken(UriString host, string token);
+        void SetToken(UriString host, string token, string username);
 
         event Action ConnectionsChanged;
     }
diff --git a/src/GitHub.Api/Authentication/Keychain.cs b/src/GitHub.Api/Authentication/Keychain.cs
@@ -95,11 +95,15 @@ public Keychain(IEnvironment environment, ICredentialManager credentialManager)
 
         public IKeychainAdapter Connect(UriString host)
         {
+            Guard.ArgumentNotNull(host, nameof(host));
+
             return FindOrCreateAdapter(host);
         }
 
         public async Task<IKeychainAdapter> Load(UriString host)
         {
+            Guard.ArgumentNotNull(host, nameof(host));
+
             var keychainAdapter = FindOrCreateAdapter(host);
             var connection = GetConnection(host);
 
@@ -145,6 +149,9 @@ public void Initialize()
         public async Task Clear(UriString host, bool deleteFromCredentialManager)
         {
             //logger.Trace("Clear Host:{0}", host);
+
+            Guard.ArgumentNotNull(host, nameof(host));
+        
             //clear octokit credentials
             await RemoveCredential(host, deleteFromCredentialManager);
             RemoveConnection(host);
@@ -153,30 +160,33 @@ public async Task Clear(UriString host, bool deleteFromCredentialManager)
         public async Task Save(UriString host)
         {
             //logger.Trace("Save: {0}", host);
+
+            Guard.ArgumentNotNull(host, nameof(host));
+
             var keychainAdapter = await AddCredential(host);
             AddConnection(new Connection(host, keychainAdapter.Credential.Username));
         }
 
         public void SetCredentials(ICredential credential)
         {
             //logger.Trace("SetCredentials Host:{0}", credential.Host);
+
+            Guard.ArgumentNotNull(credential, nameof(credential));
+
             var keychainAdapter = GetKeychainAdapter(credential.Host);
             keychainAdapter.Set(credential);
         }
 
-        public void SetToken(UriString host, string token)
+        public void SetToken(UriString host, string token, string username)
         {
             //logger.Trace("SetToken Host:{0}", host);
-            var keychainAdapter = GetKeychainAdapter(host);
-            keychainAdapter.UpdateToken(token);
-        }
 
-        public void UpdateToken(UriString host, string token)
-        {
-            //logger.Trace("UpdateToken Host:{0}", host);
+            Guard.ArgumentNotNull(host, nameof(host));
+            Guard.ArgumentNotNull(token, nameof(token));
+            Guard.ArgumentNotNull(username, nameof(username));
+
             var keychainAdapter = GetKeychainAdapter(host);
-            var keychainItem = keychainAdapter.Credential;
-            keychainItem.UpdateToken(token);
+            keychainAdapter.UpdateToken(token, username);
         }
 
         private void LoadConnectionsFromDisk()
@@ -290,15 +300,6 @@ private void RemoveConnection(UriString host)
             }
         }
 
-        private void RemoveAllConnections()
-        {
-            if (connections.Count > 0)
-            {
-                connections.Clear();
-                SaveConnectionsToDisk();
-            }
-        }
-
         private void UpdateConnections(Connection[] conns)
         {
             var updated = false;
diff --git a/src/GitHub.Api/Authentication/KeychainAdapter.cs b/src/GitHub.Api/Authentication/KeychainAdapter.cs
@@ -9,9 +9,9 @@ public void Set(ICredential credential)
             Credential = credential;
         }
 
-        public void UpdateToken(string token)
+        public void UpdateToken(string token, string username)
         {
-            Credential.UpdateToken(token);
+            Credential.UpdateToken(token, username);
         }
 
         public void Clear()
diff --git a/src/GitHub.Api/Authentication/LoginManager.cs b/src/GitHub.Api/Authentication/LoginManager.cs
@@ -1,6 +1,4 @@
 ﻿using System;
-using System.Linq;
-using System.Net;
 using System.Threading.Tasks;
 using GitHub.Logging;
 
@@ -23,8 +21,6 @@ class LoginManager : ILoginManager
         private readonly ILogging logger = LogHelper.GetLogger<LoginManager>();
 
         private readonly IKeychain keychain;
-        private readonly string clientId;
-        private readonly string clientSecret;
         private readonly IProcessManager processManager;
         private readonly ITaskManager taskManager;
         private readonly NPath? nodeJsExecutablePath;
@@ -34,25 +30,17 @@ class LoginManager : ILoginManager
         /// Initializes a new instance of the <see cref="LoginManager"/> class.
         /// </summary>
         /// <param name="keychain"></param>
-        /// <param name="clientId">The application's client API ID.</param>
-        /// <param name="clientSecret">The application's client API secret.</param>
         /// <param name="processManager"></param>
         /// <param name="taskManager"></param>
         /// <param name="nodeJsExecutablePath"></param>
         /// <param name="octorunScript"></param>
         public LoginManager(
-            IKeychain keychain,
-            string clientId,
-            string clientSecret,
-            IProcessManager processManager = null, ITaskManager taskManager = null, NPath? nodeJsExecutablePath = null, NPath? octorunScript = null)
+            IKeychain keychain, IProcessManager processManager = null, ITaskManager taskManager = null,
+            NPath? nodeJsExecutablePath = null, NPath? octorunScript = null)
         {
             Guard.ArgumentNotNull(keychain, nameof(keychain));
-            Guard.ArgumentNotNullOrWhiteSpace(clientId, nameof(clientId));
-            Guard.ArgumentNotNullOrWhiteSpace(clientSecret, nameof(clientSecret));
 
             this.keychain = keychain;
-            this.clientId = clientId;
-            this.clientSecret = clientSecret;
             this.processManager = processManager;
             this.taskManager = taskManager;
             this.nodeJsExecutablePath = nodeJsExecutablePath;
@@ -84,8 +72,13 @@ public async Task<LoginResultData> Login(
                         throw new InvalidOperationException("Returned token is null or empty");
                     }
 
-                    keychain.SetToken(host, loginResultData.Token);
-                    await keychain.Save(host);
+                    username = await RetrieveUsername(loginResultData, username);
+                    keychain.SetToken(host, loginResultData.Token, username);
+
+                    if (loginResultData.Code == LoginResultCodes.Success)
+                    {
+                        await keychain.Save(host);
+                    }
 
                     return loginResultData;
                 }
@@ -119,7 +112,8 @@ public async Task<LoginResultData> ContinueLogin(LoginResultData loginResultData
                         throw new InvalidOperationException("Returned token is null or empty");
                     }
 
-                    keychain.SetToken(host, loginResultData.Token);
+                    username = await RetrieveUsername(loginResultData, username);
+                    keychain.SetToken(host, loginResultData.Token, username);
                     await keychain.Save(host);
 
                     return loginResultData;
@@ -195,6 +189,25 @@ private async Task<LoginResultData> TryLogin(
 
             return new LoginResultData(LoginResultCodes.Failed, ret.GetApiErrorMessage() ?? "Failed.", host);
         }
+
+        private async Task<string> RetrieveUsername(LoginResultData loginResultData, string username)
+        {
+            if (!username.Contains("@"))
+            {
+                return username;
+            }
+
+            var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath.Value, octorunScript.Value, "validate",
+                user: username, userToken: loginResultData.Token).Configure(processManager);
+
+            var validateResult = await octorunTask.StartAsAsync();
+            if (!validateResult.IsSuccess)
+            {
+                throw new InvalidOperationException("Authentication validation failed");
+            }
+
+            return validateResult.Output[1];
+        }
     }
 
     class LoginResultData
diff --git a/src/GitHub.Api/Git/Tasks/GitCommitTask.cs b/src/GitHub.Api/Git/Tasks/GitCommitTask.cs
@@ -6,19 +6,37 @@ namespace GitHub.Unity
     class GitCommitTask : ProcessTask<string>
     {
         private const string TaskName = "git commit";
+
+        private readonly string message;
+        private readonly string body;
         private readonly string arguments;
 
+        private NPath tempFile;
+
         public GitCommitTask(string message, string body,
             CancellationToken token, IOutputProcessor<string> processor = null)
             : base(token, processor ?? new SimpleOutputProcessor())
         {
             Guard.ArgumentNotNullOrWhiteSpace(message, "message");
 
+            this.message = message;
+            this.body = body ?? string.Empty;
+
             Name = TaskName;
-            arguments = "-c i18n.commitencoding=utf8 commit ";
-            arguments += String.Format(" -m \"{0}\"", message);
-            if (!String.IsNullOrEmpty(body))
-                arguments += String.Format(" -m \"{0}\"", body);
+            tempFile = NPath.GetTempFilename("GitCommitTask");
+            arguments = $"-c i18n.commitencoding=utf8 commit --file \"{tempFile}\"";
+        }
+
+        protected override void RaiseOnStart()
+        {
+            base.RaiseOnStart();
+            tempFile.WriteAllLines(new [] { message, Environment.NewLine, body });
+        }
+
+        protected override void RaiseOnEnd()
+        {
+            tempFile.DeleteIfExists();
+            base.RaiseOnEnd();
         }
 
         public override string ProcessArguments { get { return arguments; } }
diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs
diff --git a/src/tests/UnitTests/Authentication/KeychainTests.cs b/src/tests/UnitTests/Authentication/KeychainTests.cs
diff --git a/vim.exe.stackdump b/vim.exe.stackdump

Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,6 @@`
`31`	`31`	`namespace System`
`32`	`32`	`{`
`33`	`33`	`internal static class AssemblyVersionInformation {`
`34`		`- internal const string Version = "0.31.8";`
	`34`	`+ internal const string Version = "0.32.0";`
`35`	`35`	`}`
`36`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,6 @@ Task CreateRepository(string name, string description, bool isPrivate,`
`12`	`12`	`Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null);`
`13`	`13`	`Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);`
`14`	`14`	`Task ContinueLogin(LoginResult loginResult, string code);`
`15`		`- Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode);`
`16`	`15`	`Task Logout(UriString host);`
`17`	`16`	`Task GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null);`
`18`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,9 +16,10 @@ public Credential(UriString host, string username, string token)`
`16`	`16`	`this.Token = token;`
`17`	`17`	`}`
`18`	`18`
`19`		`- public void UpdateToken(string token)`
	`19`	`+ public void UpdateToken(string token, string username)`
`20`	`20`	`{`
`21`	`21`	`this.Token = token;`
	`22`	`+ this.Username = username;`
`22`	`23`	`}`
`23`	`24`
`24`	`25`	`public UriString Host { get; private set; }`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ public interface ICredential : IDisposable`
`8`	`8`	`UriString Host { get; }`
`9`	`9`	`string Username { get; }`
`10`	`10`	`string Token { get; }`
`11`		`- void UpdateToken(string token);`
	`11`	`+ void UpdateToken(string token, string username);`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`public interface ICredentialManager`
Original file line number	Diff line number	Diff line change
`@@ -9,9 +9,9 @@ public void Set(ICredential credential)`
`9`	`9`	`Credential = credential;`
`10`	`10`	`}`
`11`	`11`
`12`		`- public void UpdateToken(string token)`
	`12`	`+ public void UpdateToken(string token, string username)`
`13`	`13`	`{`
`14`		`- Credential.UpdateToken(token);`
	`14`	`+ Credential.UpdateToken(token, username);`
`15`	`15`	`}`
`16`	`16`
`17`	`17`	`public void Clear()`