Fix authentication, again

Fix signin/signout not available if there's no repo. Fix signout of the
active connection. Fix mismatches between what's in the connections file
to what we're trying to signout or load the keychain for.

Author: shana

octorun/src/api.js

@@ -6,8 +6,8 @@ function ApiWrapper() {
         throw "appName missing";
     }
 
-    if (!config.user || !config.token) {
-        throw "user and/or token missing";
+    if (!config.token) {
+        throw "token missing";
     }
 
     this.octokit = octokitWrapper.createOctokit(config.appName);

octorun/src/authentication.js

@@ -1,4 +1,3 @@
-var endOfLine = require('os').EOL;
 var config = require("./configuration");
 var octokitWrapper = require("./octokit");
 

octorun/src/bin/app-login.js

@@ -1,7 +1,6 @@
 var commander = require("commander");
 var package = require('../../package.json');
 var authentication = require('../authentication');
-var endOfLine = require('os').EOL;
 var output = require('../output');
 
 commander

octorun/src/bin/app-organizations.js

@@ -1,7 +1,6 @@
 var commander = require("commander");
 var package = require('../../package.json');
 var ApiWrapper = require('../api');
-var endOfLine = require('os').EOL;
 var output = require('../output');
 
 commander

octorun/src/bin/app-publish.js

@@ -1,7 +1,6 @@
 var commander = require("commander");
 var package = require('../../package.json')
 var ApiWrapper = require('../api')
-var endOfLine = require('os').EOL;
 var output = require('../output');
 
 commander

octorun/src/bin/app-usage.js

@@ -1,9 +1,7 @@
-var commander = require("commander");
-var package = require('../../package.json')
-var config = require("../configuration");
-var endOfLine = require('os').EOL;
+var commander = require('commander');
+var package = require('../../package.json');
+var config = require('../configuration');
 var fs = require('fs');
-var util = require('util');
 var output = require('../output');
 
 commander

octorun/src/bin/app-validate.js

@@ -1,6 +1,5 @@
 var commander = require("commander");
 var package = require('../../package.json');
-var endOfLine = require('os').EOL;
 var ApiWrapper = require('../api');
 var output = require('../output');
 

octorun/src/configuration.js

@@ -3,13 +3,11 @@ require("dotenv").config({silent: true});
 var clientId = process.env.OCTOKIT_CLIENT_ID;
 var clientSecret = process.env.OCTOKIT_CLIENT_SECRET;
 var appName = process.env.OCTOKIT_USER_AGENT;
-var user = process.env.OCTORUN_USER;
 var token = process.env.OCTORUN_TOKEN;
 
 module.exports = {
     clientId: clientId,
     clientSecret: clientSecret,
     appName: appName,
-    user: user,
     token: token
 };

script

@@ -16,38 +16,41 @@ class ApiClient : IApiClient
         private readonly IKeychain keychain;
         private readonly IProcessManager processManager;
         private readonly ITaskManager taskManager;
-        private readonly NPath nodeJsExecutablePath;
-        private readonly NPath octorunScriptPath;
         private readonly ILoginManager loginManager;
+        private readonly IEnvironment environment;
 
-        public ApiClient(UriString hostUrl, IKeychain keychain, IProcessManager processManager, ITaskManager taskManager, NPath nodeJsExecutablePath, NPath octorunScriptPath)
+        public ApiClient(UriString hostUrl, IKeychain keychain, IProcessManager processManager, ITaskManager taskManager, IEnvironment environment)
         {
-            Guard.ArgumentNotNull(hostUrl, nameof(hostUrl));
             Guard.ArgumentNotNull(keychain, nameof(keychain));
 
-            HostAddress = HostAddress.Create(hostUrl);
-            OriginalUrl = hostUrl;
+            var host = String.IsNullOrEmpty(hostUrl)
+                ? UriString.ToUriString(HostAddress.GitHubDotComHostAddress.WebUri)
+                : new UriString(hostUrl.ToRepositoryUri()
+                    .GetComponents(UriComponents.SchemeAndServer, UriFormat.SafeUnescaped));
+
+            HostAddress = HostAddress.Create(host);
+            OriginalUrl = host;
             this.keychain = keychain;
             this.processManager = processManager;
             this.taskManager = taskManager;
-            this.nodeJsExecutablePath = nodeJsExecutablePath;
-            this.octorunScriptPath = octorunScriptPath;
-            loginManager = new LoginManager(keychain, processManager, taskManager, nodeJsExecutablePath, octorunScriptPath);
+            this.environment = environment;
+            loginManager = new LoginManager(keychain, processManager, taskManager, environment);
         }
 
         public ITask Logout(UriString host)
         {
             return loginManager.Logout(host);
         }
 
-        public void CreateRepository(string name, string description, bool isPrivate, Action<GitHubRepository, Exception> callback, string organization = null)
+        public void CreateRepository(string name, string description, bool isPrivate,
+            Action<GitHubRepository, Exception> callback, string organization = null)
         {
             Guard.ArgumentNotNull(callback, "callback");
 
             new FuncTask<GitHubRepository>(taskManager.Token, () =>
             {
-                var user = GetCurrentUser();
-                var keychainAdapter = keychain.Connect(OriginalUrl);
+                // this validates the user, again
+                GetCurrentUser();
 
                 var command = new StringBuilder("publish -r \"");
                 command.Append(name);
@@ -72,8 +75,7 @@ public void CreateRepository(string name, string description, bool isPrivate, Ac
                     command.Append(" -p");
                 }
 
-                var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, command.ToString(),
-                        user: user.Login, userToken: keychainAdapter.Credential.Token)
+                var octorunTask = new OctorunTask(taskManager.Token, keychain, environment, command.ToString())
                     .Configure(processManager);
 
                 var ret = octorunTask.RunSynchronously();
@@ -106,11 +108,8 @@ public void GetOrganizations(Action<Organization[]> onSuccess, Action<Exception>
             Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
             new FuncTask<Organization[]>(taskManager.Token, () =>
             {
-                var user = GetCurrentUser();
-                var keychainAdapter = keychain.Connect(OriginalUrl);
-
-                var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, "organizations",
-                        user: user.Login, userToken: keychainAdapter.Credential.Token)
+                var octorunTask = new OctorunTask(taskManager.Token, keychain, environment,
+                        "organizations")
                     .Configure(processManager);
 
                 var ret = octorunTask.RunSynchronously();
@@ -208,8 +207,7 @@ public void ContinueLogin(LoginResult loginResult, string code)
 
         private GitHubUser GetCurrentUser()
         {
-            //TODO: ONE_USER_LOGIN This assumes we only support one login
-            var keychainConnection = keychain.Connections.FirstOrDefault();
+            var keychainConnection = keychain.Connections.FirstOrDefault(x => x.Host == OriginalUrl);
             if (keychainConnection == null)
                 throw new KeychainEmptyException();
 
@@ -249,8 +247,7 @@ private GitHubUser GetValidatedGitHubUser(Connection keychainConnection, IKeycha
         {
             try
             {
-                var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, "validate",
-                        user: keychainConnection.Username, userToken: keychainAdapter.Credential.Token)
+                var octorunTask = new OctorunTask(taskManager.Token, keychain, environment, "validate")
                     .Configure(processManager);
 
                 var ret = octorunTask.RunSynchronously();

src/GitHub.Api/Application/ApiClient.cs

@@ -53,9 +53,8 @@ protected void Initialize()
 #if ENABLE_METRICS
             var metricsService = new MetricsService(ProcessManager,
                 TaskManager,
-                Environment.FileSystem,
-                Environment.NodeJsExecutablePath,
-                Environment.OctorunScriptPath);
+                Platform.Keychain,
+                Environment);
             UsageTracker.MetricsService = metricsService;
 #endif
         }

src/GitHub.Api/Application/ApplicationManagerBase.cs

@@ -6,7 +6,7 @@ namespace GitHub.Unity
     public interface IKeychain
     {
         IKeychainAdapter Connect(UriString host);
-        IKeychainAdapter Load(UriString host);
+        IKeychainAdapter Load(UriString host, bool dontClear = false);
         void Clear(UriString host, bool deleteFromCredentialManager);
         void Save(UriString host);
         void SetCredentials(ICredential credential);

src/GitHub.Api/Authentication/IKeychain.cs

@@ -99,22 +99,30 @@ public IKeychainAdapter Connect(UriString host)
             return FindOrCreateAdapter(host);
         }
 
-        public IKeychainAdapter Load(UriString host)
+        public IKeychainAdapter Load(UriString host, bool dontClear = false)
         {
             Guard.ArgumentNotNull(host, nameof(host));
 
-            var keychainAdapter = FindOrCreateAdapter(host);
-            var connection = GetConnection(host);
-
+            var keychainAdapter = Connect(host) as KeychainAdapter;
             var keychainItem = credentialManager.Load(host);
             if (keychainItem == null)
             {
-                logger.Warning("Cannot load host from Credential Manager; removing from cache");
-                Clear(host, false);
+                if (!dontClear)
+                {
+                    logger.Warning("Cannot load host from Credential Manager; removing from cache");
+                    Clear(host, false);
+                }
                 keychainAdapter = null;
             }
             else
             {
+                var connection = GetConnection(host);
+                if (connection.Username == null)
+                {
+                    connection.Username = keychainItem.Username;
+                    SaveConnectionsToDisk();
+                }
+
                 if (keychainItem.Username != connection.Username)
                 {
                     logger.Warning("Keychain Username:\"{0}\" does not match cached Username:\"{1}\"; Hopefully it works", keychainItem.Username, connection.Username);
@@ -262,18 +270,19 @@ private void RemoveCredential(UriString host, bool deleteFromCredentialManager)
         private Connection GetConnection(UriString host)
         {
             if (!connections.ContainsKey(host))
-                throw new ArgumentException($"{host} is not found", nameof(host));
+                return AddConnection(new Connection(host, null));
             return connections[host];
         }
 
-        private void AddConnection(Connection connection)
+        private Connection AddConnection(Connection connection)
         {
             // create new connection in the connection cache for this host
             if (connections.ContainsKey(connection.Host))
                 connections[connection.Host] = connection;
             else
                 connections.Add(connection.Host, connection);
             SaveConnectionsToDisk();
+            return connection;
         }
 
         private void RemoveConnection(UriString host)

src/GitHub.Api/Authentication/Keychain.cs

@@ -22,8 +22,7 @@ class LoginManager : ILoginManager
         private readonly IKeychain keychain;
         private readonly IProcessManager processManager;
         private readonly ITaskManager taskManager;
-        private readonly NPath? nodeJsExecutablePath;
-        private readonly NPath? octorunScript;
+        private readonly IEnvironment environment;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="LoginManager"/> class.
@@ -35,15 +34,14 @@ class LoginManager : ILoginManager
         /// <param name="octorunScript"></param>
         public LoginManager(
             IKeychain keychain, IProcessManager processManager, ITaskManager taskManager,
-            NPath? nodeJsExecutablePath = null, NPath? octorunScript = null)
+            IEnvironment environment)
         {
             Guard.ArgumentNotNull(keychain, nameof(keychain));
 
             this.keychain = keychain;
             this.processManager = processManager;
             this.taskManager = taskManager;
-            this.nodeJsExecutablePath = nodeJsExecutablePath;
-            this.octorunScript = octorunScript;
+            this.environment = environment;
         }
 
         /// <inheritdoc/>
@@ -146,22 +144,12 @@ private LoginResultData TryLogin(
             string code = null
         )
         {
-            if (!nodeJsExecutablePath.HasValue)
-            {
-                throw new InvalidOperationException("nodeJsExecutablePath must be set");
-            }
-
-            if (!octorunScript.HasValue)
-            {
-                throw new InvalidOperationException("octorunScript must be set");
-            }
-
             var hasTwoFactorCode = code != null;
 
             var arguments = hasTwoFactorCode ? "login --twoFactor" : "login";
-            var loginTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath.Value, octorunScript.Value,
-                arguments, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret);
-            loginTask.Configure(processManager, workingDirectory: octorunScript.Value.Parent.Parent, withInput: true);
+            var loginTask = new OctorunTask(taskManager.Token, keychain, environment,
+                arguments);
+            loginTask.Configure(processManager, withInput: true);
             loginTask.OnStartProcess += proc =>
             {
                 proc.StandardInput.WriteLine(username);
@@ -198,8 +186,8 @@ private string RetrieveUsername(LoginResultData loginResultData, string username
                 return username;
             }
 
-            var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath.Value, octorunScript.Value, "validate",
-                user: username, userToken: loginResultData.Token).Configure(processManager);
+            var octorunTask = new OctorunTask(taskManager.Token, keychain, environment, "validate")
+                .Configure(processManager);
 
             var validateResult = octorunTask.RunSynchronously();
             if (!validateResult.IsSuccess)

src/GitHub.Api/Authentication/LoginManager.cs

@@ -60,7 +60,7 @@ public ICredential Load(UriString host)
 
                 if (String.IsNullOrEmpty(kvpCreds))
                 {
-                    Logger.Error("No credentials are stored");
+                    // we didn't find credentials, stop here
                     return null;
                 }
 

src/GitHub.Api/Git/GitCredentialManager.cs

@@ -28,7 +28,8 @@ public class UriString : StringEquivalent<UriString>, IEquatable<UriString>
         public UriString(string uriString) : base(NormalizePath(uriString))
         {
             if (uriString == null || uriString.Length == 0) return;
-            if (Uri.TryCreate(uriString, UriKind.Absolute, out url))
+            if (Uri.TryCreate(uriString, UriKind.Absolute, out url)
+                || Uri.TryCreate("https://" + uriString, UriKind.Absolute, out url))
             {
                 if (!url.IsFile)
                     SetUri(url);