From 26fa50ca66af0d882b45ee41fc45f691e655907b Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 11 Sep 2017 19:53:32 -0400 Subject: [PATCH 01/30] Functionality to swith to authentication view on api failure --- src/GitHub.Api/Application/ApiClient.cs | 30 ++++++++++++------- src/GitHub.Api/Application/IApiClient.cs | 2 +- .../Editor/GitHub.Unity/UI/PublishView.cs | 4 +++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index c4ae1782d..c8a8db630 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -27,7 +27,6 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain) private readonly IKeychain keychain; private readonly IGitHubClient githubClient; private readonly ILoginManager loginManager; - private static readonly SemaphoreSlim sem = new SemaphoreSlim(1); IList organizationsCache; Octokit.User userCache; @@ -72,11 +71,10 @@ public async Task CreateRepository(NewRepository newRepository, Action> callback) + public async Task GetOrganizations(Action> onSuccess, Action onNeedsAuth = null, Action onError = null) { - Guard.ArgumentNotNull(callback, "callback"); - var organizations = await GetOrganizationInternal(); - callback(organizations); + Guard.ArgumentNotNull(onSuccess, "callback"); + await GetOrganizationInternal(onSuccess, onNeedsAuth, onError); } public async Task LoadKeychain(Action callback) @@ -224,7 +222,7 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func> GetOrganizationInternal() + private async Task GetOrganizationInternal(Action> onSuccess, Action onNeedsAuth = null, Action onError = null) { try { @@ -232,7 +230,8 @@ private async Task> GetOrganizationInternal() if (!await LoadKeychainInternal()) { - return new List(); + onSuccess(new List()); + return; } var organizations = await githubClient.Organization.GetAllForCurrent(); @@ -244,13 +243,22 @@ private async Task> GetOrganizationInternal() organizationsCache = organizations.ToArray(); } } - catch(Exception ex) + catch (LoginAttemptsExceededException) { - logger.Error(ex, "Error Getting Organizations"); - throw; + logger.Warning("Authentication Failed; Clearing Keychain"); + + var uriString = keychain.Hosts.First(); + await keychain.Clear(uriString, false); + + onNeedsAuth?.Invoke(); + } + catch (Exception ex) + { + logger.Error(ex, "Error Getting Organizations: {0}", ex.GetType().Name); + onError?.Invoke(ex); } - return organizationsCache; + onSuccess(organizationsCache); } private async Task GetCurrentUserInternal() diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 1ebbcd414..7d897c61f 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -10,7 +10,7 @@ interface IApiClient HostAddress HostAddress { get; } UriString OriginalUrl { get; } Task CreateRepository(NewRepository newRepository, Action callback, string organization = null); - Task GetOrganizations(Action> callback); + Task GetOrganizations(Action> onSuccess, Action onNeedsAuth = null, Action onError = null); Task Login(string username, string password, Action need2faCode, Action result); Task ContinueLogin(LoginResult loginResult, string code); Task LoginAsync(string username, string password, Func need2faCode); diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index addb84e63..0d644d333 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -122,6 +122,10 @@ private void LoadOrganizations() owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); isBusy = false; + }, () => { + PopupWindow.Open(PopupWindow.PopupViewType.AuthenticationView); + }, exception => { + }); }); } From 90d1f0c52ad17e6b220876f4c5c4b269a7893b71 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 11 Sep 2017 19:59:59 -0400 Subject: [PATCH 02/30] Adding an additional exception --- src/GitHub.Api/Application/ApiClient.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index c8a8db630..a32a41aef 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -243,9 +243,18 @@ private async Task GetOrganizationInternal(Action> onSuccess organizationsCache = organizations.ToArray(); } } - catch (LoginAttemptsExceededException) + catch (AuthorizationException ex) { - logger.Warning("Authentication Failed; Clearing Keychain"); + logger.Warning("Authentication Failed; {0}; Clearing Keychain", ex.GetType().Name); + + var uriString = keychain.Hosts.First(); + await keychain.Clear(uriString, false); + + onNeedsAuth?.Invoke(); + } + catch (LoginAttemptsExceededException ex) + { + logger.Warning("Authentication Failed; {0}; Clearing Keychain", ex.GetType().Name); var uriString = keychain.Hosts.First(); await keychain.Clear(uriString, false); From 553977c16c491274723decc54a5b2fb5bb72256a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 11 Oct 2017 17:07:14 -0400 Subject: [PATCH 03/30] Throwing an exception when the cached username does not match the one we get from the api --- src/GitHub.Api/Application/ApiClient.cs | 60 +++++++++++++---------- src/GitHub.Api/Application/IApiClient.cs | 1 - src/GitHub.Api/Authentication/Keychain.cs | 2 +- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index c4ae1782d..d5bad7799 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -195,10 +195,8 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func> GetOrganizationInternal() { logger.Trace("Getting Organizations"); - if (!await LoadKeychainInternal()) - { - return new List(); - } + await ValidateKeychain(); + await ValidateCurrentUserInternal(); var organizations = await githubClient.Organization.GetAllForCurrent(); @@ -257,16 +253,16 @@ private async Task> GetOrganizationInternal() { try { - logger.Trace("Getting Organizations"); + logger.Trace("Getting Current User"); if (!await LoadKeychainInternal()) { - return null; + throw new ApplicationException("Error Loading Keychain"); } userCache = await githubClient.User.Current(); } - catch(Exception ex) + catch (Exception ex) { logger.Error(ex, "Error Getting Current User"); throw; @@ -275,6 +271,29 @@ private async Task> GetOrganizationInternal() return userCache; } + private async Task ValidateCurrentUserInternal() + { + Octokit.User apiUsername; + string cachedUsername; + + try + { + logger.Trace("Validating User"); + apiUsername = await GetCurrentUserInternal(); + cachedUsername = keychain.Connections.First().Username; + } + catch (Exception ex) + { + logger.Error(ex, "Error Validating User"); + throw; + } + + if (apiUsername.Name != cachedUsername) + { + throw new ApplicationException("Credentialed user does not match API"); + } + } + private async Task LoadKeychainInternal() { logger.Trace("LoadKeychainInternal"); @@ -283,7 +302,7 @@ private async Task LoadKeychainInternal() { if (!keychain.NeedsLoad) { - logger.Trace("LoadKeychainInternal: Has keys does not need load"); + logger.Trace("LoadKeychainInternal: Previously Loaded"); return true; } @@ -301,23 +320,12 @@ private async Task LoadKeychainInternal() return false; } - public async Task ValidateCredentials() + private async Task ValidateKeychain() { - try + if (!await LoadKeychainInternal()) { - var store = keychain.Connect(OriginalUrl); - - if (store.OctokitCredentials != Credentials.Anonymous) - { - var credential = store.Credential; - await githubClient.Authorization.CheckApplicationAuthentication(ApplicationInfo.ClientId, credential.Token); - } - } - catch - { - return false; + throw new ApplicationException("Error Loading Keychain"); } - return true; } } } diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 1ebbcd414..78b72e280 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -14,7 +14,6 @@ interface IApiClient Task Login(string username, string password, Action need2faCode, Action result); Task ContinueLogin(LoginResult loginResult, string code); Task LoginAsync(string username, string password, Func need2faCode); - Task ValidateCredentials(); Task Logout(UriString host); Task GetCurrentUser(Action callback); Task LoadKeychain(Action callback); diff --git a/src/GitHub.Api/Authentication/Keychain.cs b/src/GitHub.Api/Authentication/Keychain.cs index b531735f9..bab311239 100644 --- a/src/GitHub.Api/Authentication/Keychain.cs +++ b/src/GitHub.Api/Authentication/Keychain.cs @@ -68,7 +68,7 @@ public async Task Load(UriString host) { if (keychainItem.Username != cachedConnection.Username) { - logger.Warning("Keychain Username: {0} does not match; Hopefully it works", keychainItem.Username); + logger.Warning("Keychain Username:\"{0}\" does not match cached Username:\"{1}\"; Hopefully it works", keychainItem.Username, cachedConnection.Username); } logger.Trace("Loaded from Credential Manager Host:\"{0}\" Username:\"{1}\"", keychainItem.Host, keychainItem.Username); From 7568a55cc677f9c74aa2977a57a3f4b23a61fb10 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 12 Oct 2017 10:47:56 -0400 Subject: [PATCH 04/30] Validating the user and throwing custom exceptions --- src/GitHub.Api/Application/ApiClient.cs | 92 +++++++++++-------- src/GitHub.Api/Application/IApiClient.cs | 2 +- .../Editor/GitHub.Unity/UI/PublishView.cs | 18 ++++ 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index d5bad7799..d039d228f 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -20,17 +20,13 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain) new GitHubClient(AppConfiguration.ProductHeader, credentialStore, hostAddress.ApiUri)); } - private static readonly Unity.ILogging logger = Unity.Logging.GetLogger(); + private static readonly ILogging logger = Logging.GetLogger(); public HostAddress HostAddress { get; } public UriString OriginalUrl { get; } private readonly IKeychain keychain; private readonly IGitHubClient githubClient; private readonly ILoginManager loginManager; - private static readonly SemaphoreSlim sem = new SemaphoreSlim(1); - - IList organizationsCache; - Octokit.User userCache; string owner; bool? isEnterprise; @@ -72,11 +68,10 @@ public async Task CreateRepository(NewRepository newRepository, Action> callback) + public async Task GetOrganizations(Action> onSuccess, Action onError = null) { - Guard.ArgumentNotNull(callback, "callback"); - var organizations = await GetOrganizationInternal(); - callback(organizations); + Guard.ArgumentNotNull(onSuccess, nameof(onSuccess)); + await GetOrganizationInternal(onSuccess, onError); } public async Task LoadKeychain(Action callback) @@ -222,7 +217,7 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func> GetOrganizationInternal() + private async Task GetOrganizationInternal(Action> onSuccess, Action onError = null) { try { @@ -237,16 +232,14 @@ private async Task> GetOrganizationInternal() if (organizations != null) { - organizationsCache = organizations.ToArray(); + onSuccess(organizations.ToArray()); } } catch(Exception ex) { logger.Error(ex, "Error Getting Organizations"); - throw; + onError?.Invoke(ex); } - - return organizationsCache; } private async Task GetCurrentUserInternal() @@ -254,50 +247,31 @@ private async Task> GetOrganizationInternal() try { logger.Trace("Getting Current User"); + await ValidateKeychain(); - if (!await LoadKeychainInternal()) - { - throw new ApplicationException("Error Loading Keychain"); - } - - userCache = await githubClient.User.Current(); + return await githubClient.User.Current(); } catch (Exception ex) { logger.Error(ex, "Error Getting Current User"); throw; } - - return userCache; } private async Task ValidateCurrentUserInternal() { - Octokit.User apiUsername; - string cachedUsername; - - try - { - logger.Trace("Validating User"); - apiUsername = await GetCurrentUserInternal(); - cachedUsername = keychain.Connections.First().Username; - } - catch (Exception ex) - { - logger.Error(ex, "Error Validating User"); - throw; - } + logger.Trace("Validating User"); + var apiUsername = await GetCurrentUserInternal(); + var cachedUsername = keychain.Connections.First().Username; if (apiUsername.Name != cachedUsername) { - throw new ApplicationException("Credentialed user does not match API"); + throw new TokenUsernameMismatchException(); } } private async Task LoadKeychainInternal() { - logger.Trace("LoadKeychainInternal"); - if (keychain.HasKeys) { if (!keychain.NeedsLoad) @@ -312,6 +286,8 @@ private async Task LoadKeychainInternal() var uriString = keychain.Connections.First().Host; var keychainAdapter = await keychain.Load(uriString); + logger.Trace("LoadKeychainInternal: Loaded"); + return keychainAdapter.OctokitCredentials != Credentials.Anonymous; } @@ -324,8 +300,44 @@ private async Task ValidateKeychain() { if (!await LoadKeychainInternal()) { - throw new ApplicationException("Error Loading Keychain"); + throw new KeychainEmptyException(); } } } + + class ApiClientException : Exception + { + public ApiClientException() + { } + + public ApiClientException(string message) : base(message) + { } + + public ApiClientException(string message, Exception innerException) : base(message, innerException) + { } + } + + class TokenUsernameMismatchException : ApiClientException + { + public TokenUsernameMismatchException() + { } + + public TokenUsernameMismatchException(string message) : base(message) + { } + + public TokenUsernameMismatchException(string message, Exception innerException) : base(message, innerException) + { } + } + + class KeychainEmptyException : ApiClientException + { + public KeychainEmptyException() + { } + + public KeychainEmptyException(string message) : base(message) + { } + + public KeychainEmptyException(string message, Exception innerException) : base(message, innerException) + { } + } } diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 78b72e280..95c5623ce 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -10,7 +10,7 @@ interface IApiClient HostAddress HostAddress { get; } UriString OriginalUrl { get; } Task CreateRepository(NewRepository newRepository, Action callback, string organization = null); - Task GetOrganizations(Action> callback); + Task GetOrganizations(Action> onSuccess, Action onError = null); Task Login(string username, string password, Action need2faCode, Action result); Task ContinueLogin(LoginResult loginResult, string code); Task LoginAsync(string username, string password, Func need2faCode); diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index addb84e63..f84a66858 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -122,6 +122,24 @@ private void LoadOrganizations() owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); isBusy = false; + }, exception => { + var tokenUsernameMismatchException = exception as TokenUsernameMismatchException; + if (tokenUsernameMismatchException != null) + { + //This is a specific case + + Logger.Error(exception, "Token Username Mismatch"); + return; + } + + var keychainEmptyException = exception as KeychainEmptyException; + if (keychainEmptyException != null) + { + Logger.Error(exception, "Keychain empty"); + return; + } + + Logger.Error(exception, "Unhandled Exception Type"); }); }); } From 682fb99b903a0b447d44c9fa300e6caac542f6a3 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 12 Oct 2017 11:32:02 -0400 Subject: [PATCH 05/30] Using custom exceptions and popping up dialog to proceed or switch accounts --- src/GitHub.Api/Application/ApiClient.cs | 36 +++--- src/GitHub.Api/Application/IApiClient.cs | 1 - .../Editor/GitHub.Unity/UI/HistoryView.cs | 10 +- .../Editor/GitHub.Unity/UI/PublishView.cs | 108 +++++++++--------- 4 files changed, 68 insertions(+), 87 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index d039d228f..2667ffa8e 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -74,13 +74,6 @@ public async Task GetOrganizations(Action> onSuccess, Action await GetOrganizationInternal(onSuccess, onError); } - public async Task LoadKeychain(Action callback) - { - Guard.ArgumentNotNull(callback, "callback"); - var hasLoadedKeys = await LoadKeychainInternal(); - callback(hasLoadedKeys); - } - public async Task GetCurrentUser(Action callback) { Guard.ArgumentNotNull(callback, "callback"); @@ -261,12 +254,15 @@ private async Task GetOrganizationInternal(Action> onSuccess private async Task ValidateCurrentUserInternal() { logger.Trace("Validating User"); - var apiUsername = await GetCurrentUserInternal(); + + var apiUser = await GetCurrentUserInternal(); + var apiUsername = apiUser.Login; + var cachedUsername = keychain.Connections.First().Username; - if (apiUsername.Name != cachedUsername) + if (apiUsername != cachedUsername) { - throw new TokenUsernameMismatchException(); + throw new TokenUsernameMismatchException(cachedUsername, apiUsername); } } @@ -319,25 +315,19 @@ public ApiClientException(string message, Exception innerException) : base(messa class TokenUsernameMismatchException : ApiClientException { - public TokenUsernameMismatchException() - { } - - public TokenUsernameMismatchException(string message) : base(message) - { } + public string CachedUsername { get; } + public string CurrentUsername { get; } - public TokenUsernameMismatchException(string message, Exception innerException) : base(message, innerException) - { } + public TokenUsernameMismatchException(string cachedUsername, string currentUsername) + { + CachedUsername = cachedUsername; + CurrentUsername = currentUsername; + } } class KeychainEmptyException : ApiClientException { public KeychainEmptyException() { } - - public KeychainEmptyException(string message) : base(message) - { } - - public KeychainEmptyException(string message, Exception innerException) : base(message, innerException) - { } } } diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 95c5623ce..2714275f4 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -16,6 +16,5 @@ interface IApiClient Task LoginAsync(string username, string password, Func need2faCode); Task Logout(UriString host); Task GetCurrentUser(Action callback); - Task LoadKeychain(Action callback); } } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs index f2cfcf278..d2a526d6b 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs @@ -284,15 +284,11 @@ public void OnEmbeddedGUI() else { // Publishing a repo - EditorGUI.BeginDisabledGroup(!Platform.Keychain.Connections.Any()); + var publishedClicked = GUILayout.Button(PublishButton, Styles.HistoryToolbarButtonStyle); + if (publishedClicked) { - var publishedClicked = GUILayout.Button(PublishButton, Styles.HistoryToolbarButtonStyle); - if (publishedClicked) - { - PopupWindow.Open(PopupWindow.PopupViewType.PublishView); - } + PopupWindow.Open(PopupWindow.PopupViewType.PublishView); } - EditorGUI.EndDisabledGroup(); } } GUILayout.EndHorizontal(); diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index f4424c6a3..ab5c11353 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -21,6 +21,10 @@ class PublishView : Subview private const string RepositoryNameLabel = "Repository Name"; private const string DescriptionLabel = "Description"; private const string CreatePrivateRepositoryLabel = "Create as a private repository"; + private const string AuthenticationChangedMessageFormat = "You were authenticated as \"{0}\", but you are now authenticated as \"{1}\". Would you like to proceed or logout?"; + private const string AuthenticationChangedTitle = "Authentication Changed"; + private const string AuthenticationChangedProceed = "Proceed"; + private const string AuthenticationChangedLogout = "Logout"; [SerializeField] private string username; [SerializeField] private string[] owners = { OwnersDefaultText }; @@ -33,7 +37,7 @@ class PublishView : Subview [NonSerialized] private IApiClient client; [NonSerialized] private bool isBusy; [NonSerialized] private string error; - [NonSerialized] private bool organizationsNeedLoading; + [NonSerialized] private bool ownersNeedLoading; public IApiClient Client { @@ -62,7 +66,7 @@ public IApiClient Client public override void OnEnable() { base.OnEnable(); - organizationsNeedLoading = organizations == null && !isBusy; + ownersNeedLoading = organizations == null && !isBusy; } public override void OnDataUpdate() @@ -73,10 +77,10 @@ public override void OnDataUpdate() private void MaybeUpdateData() { - if (organizationsNeedLoading) + if (ownersNeedLoading) { - organizationsNeedLoading = false; - LoadOrganizations(); + ownersNeedLoading = false; + LoadOwners(); } } @@ -87,75 +91,67 @@ public override void InitializeView(IView parent) Size = viewSize; } - private void LoadOrganizations() + private void LoadOwners() { var keychainConnections = Platform.Keychain.Connections; //TODO: ONE_USER_LOGIN This assumes only ever one user can login - if (keychainConnections.Any()) - { - try - { - Logger.Trace("Loading Keychain"); - - isBusy = true; - Client.LoadKeychain(hasKeys => { - if (!hasKeys) - { - Logger.Warning("Unable to get current user"); - isBusy = false; - return; - } + isBusy = true; - //TODO: ONE_USER_LOGIN This assumes only ever one user can login - username = keychainConnections.First().Username; + //TODO: ONE_USER_LOGIN This assumes only ever one user can login + username = keychainConnections.First().Username; - Logger.Trace("Loading Organizations"); + Logger.Trace("Loading Owners"); - Client.GetOrganizations(orgs => { - organizations = orgs; - Logger.Trace("Loaded {0} organizations", organizations.Count); + Client.GetOrganizations(orgs => + { + organizations = orgs; + Logger.Trace("Loaded {0} Owners", organizations.Count); - var organizationLogins = organizations - .OrderBy(organization => organization.Login).Select(organization => organization.Login); + var organizationLogins = organizations + .OrderBy(organization => organization.Login) + .Select(organization => organization.Login); - owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); + owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); - isBusy = false; - }, exception => { + isBusy = false; + }, exception => + { + isBusy = false; - //PopupWindow.Open(PopupWindow.PopupViewType.AuthenticationView); + var tokenUsernameMismatchException = exception as TokenUsernameMismatchException; + if (tokenUsernameMismatchException != null) + { + Logger.Trace("Token Username Mismatch"); - var tokenUsernameMismatchException = exception as TokenUsernameMismatchException; - if (tokenUsernameMismatchException != null) - { - //This is a specific case + var shouldProceed = EditorUtility.DisplayDialog(AuthenticationChangedTitle, + string.Format(AuthenticationChangedMessageFormat, + tokenUsernameMismatchException.CachedUsername, + tokenUsernameMismatchException.CurrentUsername), AuthenticationChangedProceed, AuthenticationChangedLogout); - Logger.Error(exception, "Token Username Mismatch"); - return; - } + if (shouldProceed) + { + //Proceed as current user - var keychainEmptyException = exception as KeychainEmptyException; - if (keychainEmptyException != null) - { - Logger.Error(exception, "Keychain empty"); - return; - } + } + else + { + //Logout current user and try again - Logger.Error(exception, "Unhandled Exception Type"); - }); - }); + } + return; } - catch (Exception e) + + var keychainEmptyException = exception as KeychainEmptyException; + if (keychainEmptyException != null) { - Logger.Error(e, "Error PopulateView & GetOrganizations"); - isBusy = false; + Logger.Trace("Keychain empty"); + PopupWindow.Open(PopupWindow.PopupViewType.AuthenticationView); + return; } - } - else - { - Logger.Warning("No Keychain connections to use"); - } + + Logger.Error(exception, "Unhandled Exception"); + }); } public override void OnGUI() From b0cc6969ad5529399c0ade9d9a38023fe80fc068 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 12 Oct 2017 16:12:42 -0400 Subject: [PATCH 06/30] Initial routine in PopupWindow to handle authentication before presenting specific views --- src/GitHub.Api/Application/ApiClient.cs | 14 +++ src/GitHub.Api/Application/IApiClient.cs | 1 + .../Editor/GitHub.Unity/UI/HistoryView.cs | 2 +- .../Editor/GitHub.Unity/UI/PopupWindow.cs | 90 ++++++++++++++++--- .../Editor/GitHub.Unity/UI/PublishView.cs | 2 +- .../Assets/Editor/GitHub.Unity/UI/Window.cs | 2 +- 6 files changed, 98 insertions(+), 13 deletions(-) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index 2667ffa8e..25b744cf2 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -74,6 +74,20 @@ public async Task GetOrganizations(Action> onSuccess, Action await GetOrganizationInternal(onSuccess, onError); } + public async Task ValidateCurrentUser(Action onSuccess, Action onError = null) + { + Guard.ArgumentNotNull(onSuccess, nameof(onSuccess)); + try + { + await ValidateCurrentUserInternal(); + onSuccess(); + } + catch (Exception e) + { + onError?.Invoke(e); + } + } + public async Task GetCurrentUser(Action callback) { Guard.ArgumentNotNull(callback, "callback"); diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 2714275f4..77819e3f5 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -16,5 +16,6 @@ interface IApiClient Task LoginAsync(string username, string password, Func need2faCode); Task Logout(UriString host); Task GetCurrentUser(Action callback); + Task ValidateCurrentUser(Action onSuccess, Action onError = null); } } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs index d2a526d6b..d9bdf4c4e 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/HistoryView.cs @@ -287,7 +287,7 @@ public void OnEmbeddedGUI() var publishedClicked = GUILayout.Button(PublishButton, Styles.HistoryToolbarButtonStyle); if (publishedClicked) { - PopupWindow.Open(PopupWindow.PopupViewType.PublishView); + PopupWindow.OpenWindow(PopupWindow.PopupViewType.PublishView); } } } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 9ee629ca9..d66ea6a08 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -20,32 +20,100 @@ public enum PopupViewType [SerializeField] private PublishView publishView; [SerializeField] private LoadingView loadingView; + [NonSerialized] private IApiClient client; + public event Action OnClose; [MenuItem("GitHub/Authenticate")] public static void Launch() { - Open(PopupViewType.AuthenticationView); + OpenWindow(PopupViewType.AuthenticationView); } - public static PopupWindow Open(PopupViewType popupViewType, Action onClose = null) + public static PopupWindow OpenWindow(PopupViewType popupViewType, Action onClose = null) { var popupWindow = GetWindow(true); - popupWindow.OnClose.SafeInvoke(false); + popupWindow.Open(popupViewType, onClose); + + return popupWindow; + } + + private void Open(PopupViewType popupViewType, Action onClose) + { + OnClose.SafeInvoke(false); + OnClose = null; + + Logger.Trace("OpenView: {0}", popupViewType.ToString()); + + var viewNeedsAuthentication = popupViewType == PopupViewType.PublishView; + if (viewNeedsAuthentication) + { + Logger.Trace("Validating to open view"); + + Client.ValidateCurrentUser(() => { + + Logger.Trace("User validated opening view"); + + OpenInternal(popupViewType, onClose); + + }, exception => { + + Logger.Trace("User required validation opening AuthenticationView"); + + Open(PopupViewType.AuthenticationView, completedAuthentication => { + if (completedAuthentication) + { + Logger.Trace("User completed validation opening view: {0}", popupViewType.ToString()); + + Open(popupViewType, onClose); + } + }); + }); + } + else + { + OpenInternal(popupViewType, onClose); + } + } + + private void OpenInternal(PopupViewType popupViewType, Action onClose) + { if (onClose != null) { - popupWindow.OnClose += onClose; + OnClose += onClose; } - popupWindow.ActiveViewType = popupViewType; - popupWindow.titleContent = new GUIContent(popupWindow.ActiveView.Title, Styles.SmallLogo); - popupWindow.OnEnable(); - popupWindow.Show(); - popupWindow.Refresh(); + ActiveViewType = popupViewType; + titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); + OnEnable(); + Show(); + Refresh(); + } - return popupWindow; + public IApiClient Client + { + get + { + if (client == null) + { + var repository = Environment.Repository; + UriString host; + if (repository != null && !string.IsNullOrEmpty(repository.CloneUrl)) + { + host = repository.CloneUrl.ToRepositoryUrl(); + } + else + { + host = UriString.ToUriString(HostAddress.GitHubDotComHostAddress.WebUri); + } + + client = ApiClient.Create(host, Platform.Keychain); + } + + return client; + } } public override void Initialize(IApplicationManager applicationManager) @@ -59,6 +127,8 @@ public override void Initialize(IApplicationManager applicationManager) publishView.InitializeView(this); authenticationView.InitializeView(this); loadingView.InitializeView(this); + + titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); } public override void OnEnable() diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index ab5c11353..e0a799e77 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -146,7 +146,7 @@ private void LoadOwners() if (keychainEmptyException != null) { Logger.Trace("Keychain empty"); - PopupWindow.Open(PopupWindow.PopupViewType.AuthenticationView); + PopupWindow.OpenWindow(PopupWindow.PopupViewType.AuthenticationView); return; } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs index cf9da47f6..325ca22cc 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs @@ -351,7 +351,7 @@ private void DoAccountDropdown() private void SignIn(object obj) { - PopupWindow.Open(PopupWindow.PopupViewType.AuthenticationView); + PopupWindow.OpenWindow(PopupWindow.PopupViewType.AuthenticationView); } private void GoToProfile(object obj) From b0845413304ed5d853dc978e3f8b48b7aac4576f Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 12 Oct 2017 16:35:31 -0400 Subject: [PATCH 07/30] Attempting to fix the close operation --- .../Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index d66ea6a08..ac2fb4d6f 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -44,6 +44,11 @@ private void Open(PopupViewType popupViewType, Action onClose) OnClose.SafeInvoke(false); OnClose = null; + onClose = onClose ?? (b => { + Logger.Trace("Closing Window"); + //Close(); + }); + Logger.Trace("OpenView: {0}", popupViewType.ToString()); var viewNeedsAuthentication = popupViewType == PopupViewType.PublishView; @@ -172,7 +177,6 @@ public override void Finish(bool result) { OnClose.SafeInvoke(result); OnClose = null; - Close(); base.Finish(result); } From e9f7f09ed60522fa5821ef0f260fd17d2401a74b Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 12 Oct 2017 16:42:59 -0400 Subject: [PATCH 08/30] Attempting to close properly after publish --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index ac2fb4d6f..642eeb023 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -46,7 +46,7 @@ private void Open(PopupViewType popupViewType, Action onClose) onClose = onClose ?? (b => { Logger.Trace("Closing Window"); - //Close(); + Close(); }); Logger.Trace("OpenView: {0}", popupViewType.ToString()); From 79507a3b1d2d09afe52c11246740ae233c8f07e4 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 13 Oct 2017 10:02:00 -0400 Subject: [PATCH 09/30] Controlling the close action with a flag --- .../Editor/GitHub.Unity/UI/PopupWindow.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 642eeb023..36a9ac7b6 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -14,8 +14,8 @@ public enum PopupViewType AuthenticationView } + [SerializeField] private bool shouldCloseOnFinish; [SerializeField] private PopupViewType activeViewType; - [SerializeField] private AuthenticationView authenticationView; [SerializeField] private PublishView publishView; [SerializeField] private LoadingView loadingView; @@ -44,11 +44,6 @@ private void Open(PopupViewType popupViewType, Action onClose) OnClose.SafeInvoke(false); OnClose = null; - onClose = onClose ?? (b => { - Logger.Trace("Closing Window"); - Close(); - }); - Logger.Trace("OpenView: {0}", popupViewType.ToString()); var viewNeedsAuthentication = popupViewType == PopupViewType.PublishView; @@ -61,12 +56,13 @@ private void Open(PopupViewType popupViewType, Action onClose) Logger.Trace("User validated opening view"); OpenInternal(popupViewType, onClose); + shouldCloseOnFinish = true; }, exception => { Logger.Trace("User required validation opening AuthenticationView"); - Open(PopupViewType.AuthenticationView, completedAuthentication => { + OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { if (completedAuthentication) { @@ -75,11 +71,13 @@ private void Open(PopupViewType popupViewType, Action onClose) Open(popupViewType, onClose); } }); + shouldCloseOnFinish = false; }); } else { OpenInternal(popupViewType, onClose); + shouldCloseOnFinish = true; } } @@ -177,6 +175,13 @@ public override void Finish(bool result) { OnClose.SafeInvoke(result); OnClose = null; + + if (shouldCloseOnFinish) + { + shouldCloseOnFinish = false; + Close(); + } + base.Finish(result); } From 380ede928dd1bde5cb2819cd838c6a4bb768c211 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 13 Oct 2017 10:44:19 -0400 Subject: [PATCH 10/30] Displaying a message on the Authentication window and setting the cached username --- .../GitHub.Unity/UI/AuthenticationView.cs | 23 +++++++++++++++ .../Editor/GitHub.Unity/UI/PopupWindow.cs | 29 +++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index d20ab8fa7..3917a905e 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -23,6 +23,7 @@ class AuthenticationView : Subview [SerializeField] private Vector2 scroll; [SerializeField] private string username = ""; [SerializeField] private string two2fa = ""; + [SerializeField] private string message; [NonSerialized] private bool need2fa; [NonSerialized] private bool isBusy; @@ -98,6 +99,16 @@ public override void OnGUI() GUILayout.EndScrollView(); } + public void SetMessage(string value) + { + message = value; + } + + public void SetUsername(string value) + { + username = value; + } + private void HandleEnterPressed() { if (Event.current.type != EventType.KeyDown) @@ -112,6 +123,8 @@ private void OnGUILogin() { EditorGUI.BeginDisabledGroup(isBusy); { + ShowMessage(); + GUILayout.Space(3); GUILayout.BeginHorizontal(); { @@ -217,6 +230,16 @@ private void DoResult(bool success, string msg) } } + private void ShowMessage() + { + if (message != null) + { + GUILayout.Space(Styles.BaseSpacing + 3); + GUILayout.Label(message, Styles.CenteredLabel); + GUILayout.Space(Styles.BaseSpacing + 3); + } + } + private void ShowErrorMessage() { if (errorMessage != null) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 36a9ac7b6..e0fa8581f 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -59,11 +59,30 @@ private void Open(PopupViewType popupViewType, Action onClose) shouldCloseOnFinish = true; }, exception => { - Logger.Trace("User required validation opening AuthenticationView"); - OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { + string message = null; + string username = null; + + var usernameMismatchException = exception as TokenUsernameMismatchException; + if (usernameMismatchException != null) + { + message = "Your credentials need to be refreshed"; + username = usernameMismatchException.CachedUsername; + } + + var keychainEmptyException = exception as KeychainEmptyException; + if (keychainEmptyException != null) + { + message = "We need you to authenticate first"; + } + if (usernameMismatchException == null && keychainEmptyException == null) + { + message = "There was an error validating your account"; + } + + OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { if (completedAuthentication) { Logger.Trace("User completed validation opening view: {0}", popupViewType.ToString()); @@ -71,7 +90,13 @@ private void Open(PopupViewType popupViewType, Action onClose) Open(popupViewType, onClose); } }); + shouldCloseOnFinish = false; + authenticationView.SetMessage(message); + if (username != null) + { + authenticationView.SetUsername(username); + } }); } else From 5aa80925d6fc7b421ed67bc6c9c92ca756b0679c Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 13 Oct 2017 13:42:26 -0400 Subject: [PATCH 11/30] Clearing the message at the right time --- .../Editor/GitHub.Unity/UI/AuthenticationView.cs | 16 +++++++++++++--- .../Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 3 +++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index 3917a905e..d9de5a8bc 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -21,15 +21,15 @@ class AuthenticationView : Subview private const string TwofaButton = "Verify"; [SerializeField] private Vector2 scroll; - [SerializeField] private string username = ""; - [SerializeField] private string two2fa = ""; + [SerializeField] private string username = string.Empty; + [SerializeField] private string two2fa = string.Empty; [SerializeField] private string message; [NonSerialized] private bool need2fa; [NonSerialized] private bool isBusy; [NonSerialized] private string errorMessage; [NonSerialized] private bool enterPressed; - [NonSerialized] private string password = ""; + [NonSerialized] private string password = string.Empty; [NonSerialized] private AuthenticationService authenticationService; @@ -104,11 +104,21 @@ public void SetMessage(string value) message = value; } + public void ClearMessage() + { + message = null; + } + public void SetUsername(string value) { username = value; } + public void ClearUsername() + { + username = string.Empty; + } + private void HandleEnterPressed() { if (Event.current.type != EventType.KeyDown) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index e0fa8581f..a3731aec6 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -83,6 +83,9 @@ private void Open(PopupViewType popupViewType, Action onClose) } OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { + authenticationView.ClearMessage(); + authenticationView.ClearUsername(); + if (completedAuthentication) { Logger.Trace("User completed validation opening view: {0}", popupViewType.ToString()); From 494f552b6b6b05edcb8e58bb07bf4c075997b136 Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Tue, 17 Oct 2017 14:55:50 -0700 Subject: [PATCH 12/30] Bring back auth header styles Use default padding settings --- .../Assets/Editor/GitHub.Unity/Misc/Styles.cs | 1 - .../Assets/Editor/GitHub.Unity/UI/PublishView.cs | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs index a4c57790a..aac5f334f 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/Misc/Styles.cs @@ -578,7 +578,6 @@ public static GUIStyle AuthHeaderBoxStyle { authHeaderBoxStyle = new GUIStyle(HeaderBoxStyle); authHeaderBoxStyle.name = "AuthHeaderBoxStyle"; - authHeaderBoxStyle.padding = new RectOffset(10, 10, 0, 5); } return authHeaderBoxStyle; } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index 1940b728c..27f933f05 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -157,7 +157,11 @@ private void LoadOwners() public override void OnGUI() { - GUILayout.Label("Publish to GitHub", EditorStyles.boldLabel); + GUILayout.BeginHorizontal(Styles.AuthHeaderBoxStyle); + { + GUILayout.Label("Publish to GitHub", EditorStyles.boldLabel); + } + GUILayout.EndHorizontal(); EditorGUI.BeginDisabledGroup(isBusy); { From 23498ceda96dff73ad6ae3b3e40e564035ac46af Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 19 Oct 2017 14:01:39 -0400 Subject: [PATCH 13/30] Turning messages into constants; Changing account refresh message --- .../Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index a3731aec6..bda14bab9 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -7,6 +7,10 @@ namespace GitHub.Unity [Serializable] class PopupWindow : BaseWindow { + private const string CredentialsNeedRefreshMessage = "We've detected that your stored credentials are out of sync with your current user. This can happen if you have signed in to git outside of Unity. Sign in again to refresh your credentials."; + private const string NeedAuthenticationMessage = "We need you to authenticate first"; + private const string AccountValidationErrorMessage = "There was an error validating your account"; + public enum PopupViewType { None, @@ -67,19 +71,19 @@ private void Open(PopupViewType popupViewType, Action onClose) var usernameMismatchException = exception as TokenUsernameMismatchException; if (usernameMismatchException != null) { - message = "Your credentials need to be refreshed"; + message = CredentialsNeedRefreshMessage; username = usernameMismatchException.CachedUsername; } var keychainEmptyException = exception as KeychainEmptyException; if (keychainEmptyException != null) { - message = "We need you to authenticate first"; + message = NeedAuthenticationMessage; } if (usernameMismatchException == null && keychainEmptyException == null) { - message = "There was an error validating your account"; + message = AccountValidationErrorMessage; } OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { From e00c98ca1b927a3e325b67463b790365efca0249 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 19 Oct 2017 14:04:40 -0400 Subject: [PATCH 14/30] Returning the exception message --- .../Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index bda14bab9..7851acd2c 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -9,7 +9,6 @@ class PopupWindow : BaseWindow { private const string CredentialsNeedRefreshMessage = "We've detected that your stored credentials are out of sync with your current user. This can happen if you have signed in to git outside of Unity. Sign in again to refresh your credentials."; private const string NeedAuthenticationMessage = "We need you to authenticate first"; - private const string AccountValidationErrorMessage = "There was an error validating your account"; public enum PopupViewType { @@ -83,7 +82,7 @@ private void Open(PopupViewType popupViewType, Action onClose) if (usernameMismatchException == null && keychainEmptyException == null) { - message = AccountValidationErrorMessage; + message = exception.Message; } OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { From 85969ada182024cd7bfd7e6de97481d0ba691934 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 23 Oct 2017 13:58:34 -0400 Subject: [PATCH 15/30] Preventing the exposure of Octokit objects to the codebase --- src/GitHub.Api/Application/ApiClient.cs | 38 +++++++++++++------ src/GitHub.Api/Application/IApiClient.cs | 6 +-- .../Application/OctokitExtensions.cs | 21 ++++++++++ src/GitHub.Api/Application/Organization.cs | 8 ++++ src/GitHub.Api/GitHub.Api.csproj | 2 + .../Editor/GitHub.Unity/UI/PublishView.cs | 14 +++---- 6 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 src/GitHub.Api/Application/OctokitExtensions.cs create mode 100644 src/GitHub.Api/Application/Organization.cs diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index 415872b07..83b6603a1 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -50,7 +50,7 @@ private async Task LogoutInternal(UriString host) await loginManager.Logout(host); } - public async Task CreateRepository(NewRepository newRepository, Action callback, string organization = null) + public async Task CreateRepository(NewRepository newRepository, Action callback, string organization = null) { Guard.ArgumentNotNull(callback, "callback"); try @@ -64,7 +64,7 @@ public async Task CreateRepository(NewRepository newRepository, Action> onSuccess, Action onError = null) + public async Task GetOrganizations(Action onSuccess, Action onError = null) { Guard.ArgumentNotNull(onSuccess, nameof(onSuccess)); await GetOrganizationInternal(onSuccess, onError); @@ -84,7 +84,7 @@ public async Task ValidateCurrentUser(Action onSuccess, Action onErro } } - public async Task GetCurrentUser(Action callback) + public async Task GetCurrentUser(Action callback) { Guard.ArgumentNotNull(callback, "callback"); var user = await GetCurrentUserInternal(); @@ -187,7 +187,7 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func CreateRepositoryInternal(NewRepository newRepository, string organization) + private async Task CreateRepositoryInternal(NewRepository newRepository, string organization) { try { @@ -196,18 +196,18 @@ public async Task ContinueLoginAsync(LoginResult loginResult, Func ContinueLoginAsync(LoginResult loginResult, Func> onSuccess, Action onError = null) + private async Task GetOrganizationInternal(Action onSuccess, Action onError = null) { try { @@ -235,7 +235,11 @@ private async Task GetOrganizationInternal(Action> onSuccess if (organizations != null) { - onSuccess(organizations.ToArray()); + var array = organizations.Select(organization => new Organization() { + Name = organization.Name, + Login = organization.Login + }).ToArray(); + onSuccess(array); } } catch(Exception ex) @@ -245,14 +249,14 @@ private async Task GetOrganizationInternal(Action> onSuccess } } - private async Task GetCurrentUserInternal() + private async Task GetCurrentUserInternal() { try { logger.Trace("Getting Current User"); await ValidateKeychain(); - return await githubClient.User.Current(); + return (await githubClient.User.Current()).ToGitHubUser(); } catch (Exception ex) { @@ -311,6 +315,18 @@ private async Task ValidateKeychain() } } + class GitHubUser + { + public string Name { get; set; } + public string Login { get; set; } + } + + class GitHubRepository + { + public string Name { get; set; } + public string CloneUrl { get; set; } + } + class ApiClientException : Exception { public ApiClientException() diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs index 77819e3f5..d4a87e3e2 100644 --- a/src/GitHub.Api/Application/IApiClient.cs +++ b/src/GitHub.Api/Application/IApiClient.cs @@ -9,13 +9,13 @@ interface IApiClient { HostAddress HostAddress { get; } UriString OriginalUrl { get; } - Task CreateRepository(NewRepository newRepository, Action callback, string organization = null); - Task GetOrganizations(Action> onSuccess, Action onError = null); + Task CreateRepository(NewRepository newRepository, Action callback, string organization = null); + Task GetOrganizations(Action onSuccess, Action onError = null); Task Login(string username, string password, Action need2faCode, Action result); Task ContinueLogin(LoginResult loginResult, string code); Task LoginAsync(string username, string password, Func need2faCode); Task Logout(UriString host); - Task GetCurrentUser(Action callback); + Task GetCurrentUser(Action callback); Task ValidateCurrentUser(Action onSuccess, Action onError = null); } } diff --git a/src/GitHub.Api/Application/OctokitExtensions.cs b/src/GitHub.Api/Application/OctokitExtensions.cs new file mode 100644 index 000000000..c53101c74 --- /dev/null +++ b/src/GitHub.Api/Application/OctokitExtensions.cs @@ -0,0 +1,21 @@ +namespace GitHub.Unity +{ + static class OctokitExtensions + { + public static GitHubUser ToGitHubUser(this Octokit.User user) + { + return new GitHubUser() { + Name = user.Name, + Login = user.Login, + }; + } + + public static GitHubRepository ToGitHubRepository(this Octokit.Repository repository) + { + return new GitHubRepository { + Name = repository.Name, + CloneUrl = repository.CloneUrl + }; + } + } +} \ No newline at end of file diff --git a/src/GitHub.Api/Application/Organization.cs b/src/GitHub.Api/Application/Organization.cs new file mode 100644 index 000000000..e78849dd6 --- /dev/null +++ b/src/GitHub.Api/Application/Organization.cs @@ -0,0 +1,8 @@ +namespace GitHub.Unity +{ + class Organization + { + public string Name { get; set; } + public string Login { get; set; } + } +} \ No newline at end of file diff --git a/src/GitHub.Api/GitHub.Api.csproj b/src/GitHub.Api/GitHub.Api.csproj index a87766cee..87f8e2c30 100644 --- a/src/GitHub.Api/GitHub.Api.csproj +++ b/src/GitHub.Api/GitHub.Api.csproj @@ -99,6 +99,8 @@ + + diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index 27f933f05..61d19e8c8 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -29,7 +29,7 @@ class PublishView : Subview [SerializeField] private string username; [SerializeField] private string[] owners = { OwnersDefaultText }; - [SerializeField] private IList organizations; + [SerializeField] private string[] publishOwners; [SerializeField] private int selectedOwner; [SerializeField] private string repoName = String.Empty; [SerializeField] private string repoDescription = ""; @@ -67,7 +67,7 @@ public IApiClient Client public override void OnEnable() { base.OnEnable(); - ownersNeedLoading = organizations == null && !isBusy; + ownersNeedLoading = publishOwners == null && !isBusy; } public override void OnDataUpdate() @@ -106,14 +106,14 @@ private void LoadOwners() Client.GetOrganizations(orgs => { - organizations = orgs; - Logger.Trace("Loaded {0} Owners", organizations.Count); + Logger.Trace("Loaded {0} Owners", orgs.Length); - var organizationLogins = organizations + publishOwners = orgs .OrderBy(organization => organization.Login) - .Select(organization => organization.Login); + .Select(organization => organization.Login) + .ToArray(); - owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray(); + owners = new[] { OwnersDefaultText, username }.Union(publishOwners).ToArray(); isBusy = false; }, exception => From 6482545048a412c80481d46d7343ce8f95a2f57d Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Thu, 26 Oct 2017 08:55:34 -0700 Subject: [PATCH 16/30] :fire: extra cruft from auth view --- .../GitHub.Unity/UI/AuthenticationView.cs | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index d9de5a8bc..4c33e1b3f 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -59,28 +59,11 @@ public override void OnGUI() scroll = GUILayout.BeginScrollView(scroll); { - Rect authHeader = EditorGUILayout.BeginHorizontal(Styles.AuthHeaderBoxStyle); + GUILayout.BeginHorizontal(Styles.AuthHeaderBoxStyle); { - GUILayout.BeginVertical(GUILayout.Width(16)); - { - GUILayout.Space(9); - GUILayout.Label(Styles.BigLogo, GUILayout.Height(20), GUILayout.Width(20)); - } - GUILayout.EndVertical(); - - GUILayout.BeginVertical(); - { - GUILayout.Space(11); - GUILayout.Label(AuthTitle, Styles.HeaderRepoLabelStyle); - } - GUILayout.EndVertical(); + GUILayout.Label(AuthTitle, Styles.HeaderRepoLabelStyle); } - GUILayout.EndHorizontal(); - EditorGUI.DrawRect( - new Rect(authHeader.x, authHeader.yMax, authHeader.xMax, 1), - new Color(0.455F, 0.455F, 0.455F, 1F) - ); GUILayout.BeginVertical(Styles.GenericBoxStyle); { From 58b48aa871a9fef9d5c45b474b51c7b00e18193c Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Thu, 26 Oct 2017 09:00:08 -0700 Subject: [PATCH 17/30] Use a HelpBox instead --- .../Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index 4c33e1b3f..2da6d2209 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -227,9 +227,7 @@ private void ShowMessage() { if (message != null) { - GUILayout.Space(Styles.BaseSpacing + 3); - GUILayout.Label(message, Styles.CenteredLabel); - GUILayout.Space(Styles.BaseSpacing + 3); + EditorGUILayout.HelpBox(message, MessageType.Warning); } } @@ -237,7 +235,7 @@ private void ShowErrorMessage() { if (errorMessage != null) { - GUILayout.Label(errorMessage, Styles.ErrorLabel); + EditorGUILayout.HelpBox(errorMessage, MessageType.Error); } } From 20e137a3eaee4fef6d2df215fa7dabad157e680d Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Thu, 26 Oct 2017 14:55:56 -0700 Subject: [PATCH 18/30] Adjust some spacing --- .../Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index 2da6d2209..d669d738d 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -118,20 +118,24 @@ private void OnGUILogin() { ShowMessage(); - GUILayout.Space(3); + EditorGUILayout.Space(); + GUILayout.BeginHorizontal(); { username = EditorGUILayout.TextField(UsernameLabel ,username, Styles.TextFieldStyle); } GUILayout.EndHorizontal(); - GUILayout.Space(Styles.BaseSpacing); + EditorGUILayout.Space(); + GUILayout.BeginHorizontal(); { password = EditorGUILayout.PasswordField(PasswordLabel, password, Styles.TextFieldStyle); } GUILayout.EndHorizontal(); + EditorGUILayout.Space(); + ShowErrorMessage(); GUILayout.Space(Styles.BaseSpacing + 3); From 8b260dd85e1d42f362ea7f7cffb19660272842f0 Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Fri, 27 Oct 2017 09:26:36 -0700 Subject: [PATCH 19/30] Remove unneeded spacing --- .../GitHub.Unity/UI/AuthenticationView.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index d669d738d..0bb15fb25 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -65,7 +65,7 @@ public override void OnGUI() } GUILayout.EndHorizontal(); - GUILayout.BeginVertical(Styles.GenericBoxStyle); + GUILayout.BeginVertical(); { if (!need2fa) { @@ -119,7 +119,7 @@ private void OnGUILogin() ShowMessage(); EditorGUILayout.Space(); - + GUILayout.BeginHorizontal(); { username = EditorGUILayout.TextField(UsernameLabel ,username, Styles.TextFieldStyle); @@ -163,17 +163,11 @@ private void OnGUI2FA() EditorGUI.BeginDisabledGroup(isBusy); { - GUILayout.Space(Styles.BaseSpacing); - GUILayout.BeginHorizontal(); - { - two2fa = EditorGUILayout.TextField(TwofaLabel, two2fa, Styles.TextFieldStyle); - } - GUILayout.EndHorizontal(); - - GUILayout.Space(Styles.BaseSpacing); + EditorGUILayout.Space(); + two2fa = EditorGUILayout.TextField(TwofaLabel, two2fa, Styles.TextFieldStyle); + EditorGUILayout.Space(); ShowErrorMessage(); - GUILayout.Space(Styles.BaseSpacing); GUILayout.BeginHorizontal(); { GUILayout.FlexibleSpace(); @@ -193,7 +187,7 @@ private void OnGUI2FA() } GUILayout.EndHorizontal(); - GUILayout.Space(Styles.BaseSpacing); + EditorGUILayout.Space(); } EditorGUI.EndDisabledGroup(); } From 58cb0521896e3223312715526e6a180678422dc1 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Wed, 1 Nov 2017 21:09:00 -0700 Subject: [PATCH 20/30] AuthenticationView can handle its own message logic --- .../GitHub.Unity/UI/AuthenticationView.cs | 47 ++++++++----------- .../Editor/GitHub.Unity/UI/PopupWindow.cs | 37 +-------------- 2 files changed, 21 insertions(+), 63 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index 0bb15fb25..467ab768b 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -9,6 +9,8 @@ class AuthenticationView : Subview { private static readonly Vector2 viewSize = new Vector2(290, 290); + private const string CredentialsNeedRefreshMessage = "We've detected that your stored credentials are out of sync with your current user. This can happen if you have signed in to git outside of Unity. Sign in again to refresh your credentials."; + private const string NeedAuthenticationMessage = "We need you to authenticate first"; private const string WindowTitle = "Authenticate"; private const string UsernameLabel = "Username"; private const string PasswordLabel = "Password"; @@ -41,14 +43,25 @@ public override void InitializeView(IView parent) Size = viewSize; } - public override void OnEnable() + public void Initialize(Exception exception) { - base.OnEnable(); - } + var usernameMismatchException = exception as TokenUsernameMismatchException; + if (usernameMismatchException != null) + { + message = CredentialsNeedRefreshMessage; + username = usernameMismatchException.CachedUsername; + } - public override void OnDisable() - { - base.OnDisable(); + var keychainEmptyException = exception as KeychainEmptyException; + if (keychainEmptyException != null) + { + message = NeedAuthenticationMessage; + } + + if (usernameMismatchException == null && keychainEmptyException == null) + { + message = exception.Message; + } } public override void OnGUI() @@ -81,27 +94,7 @@ public override void OnGUI() } GUILayout.EndScrollView(); } - - public void SetMessage(string value) - { - message = value; - } - - public void ClearMessage() - { - message = null; - } - - public void SetUsername(string value) - { - username = value; - } - - public void ClearUsername() - { - username = string.Empty; - } - + private void HandleEnterPressed() { if (Event.current.type != EventType.KeyDown) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 80a626517..86a62cb75 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -7,9 +7,6 @@ namespace GitHub.Unity [Serializable] class PopupWindow : BaseWindow { - private const string CredentialsNeedRefreshMessage = "We've detected that your stored credentials are out of sync with your current user. This can happen if you have signed in to git outside of Unity. Sign in again to refresh your credentials."; - private const string NeedAuthenticationMessage = "We need you to authenticate first"; - public enum PopupViewType { None, @@ -63,32 +60,8 @@ private void Open(PopupViewType popupViewType, Action onClose) }, exception => { Logger.Trace("User required validation opening AuthenticationView"); - - string message = null; - string username = null; - - var usernameMismatchException = exception as TokenUsernameMismatchException; - if (usernameMismatchException != null) - { - message = CredentialsNeedRefreshMessage; - username = usernameMismatchException.CachedUsername; - } - - var keychainEmptyException = exception as KeychainEmptyException; - if (keychainEmptyException != null) - { - message = NeedAuthenticationMessage; - } - - if (usernameMismatchException == null && keychainEmptyException == null) - { - message = exception.Message; - } - + authenticationView.Initialize(exception); OpenInternal(PopupViewType.AuthenticationView, completedAuthentication => { - authenticationView.ClearMessage(); - authenticationView.ClearUsername(); - if (completedAuthentication) { Logger.Trace("User completed validation opening view: {0}", popupViewType.ToString()); @@ -98,11 +71,6 @@ private void Open(PopupViewType popupViewType, Action onClose) }); shouldCloseOnFinish = false; - authenticationView.SetMessage(message); - if (username != null) - { - authenticationView.SetUsername(username); - } }); } else @@ -120,10 +88,7 @@ private void OpenInternal(PopupViewType popupViewType, Action onClose) } ActiveViewType = popupViewType; - titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); - OnEnable(); Show(); - Refresh(); } public IApiClient Client From 3709d5220b6ff8503f8e83850b95f864b0d63f03 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Wed, 1 Nov 2017 21:35:32 -0700 Subject: [PATCH 21/30] Resetting the UI when going back or finishing --- .../GitHub.Unity/UI/AuthenticationView.cs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs index 467ab768b..c7795c05a 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/AuthenticationView.cs @@ -26,10 +26,10 @@ class AuthenticationView : Subview [SerializeField] private string username = string.Empty; [SerializeField] private string two2fa = string.Empty; [SerializeField] private string message; + [SerializeField] private string errorMessage; + [SerializeField] private bool need2fa; - [NonSerialized] private bool need2fa; [NonSerialized] private bool isBusy; - [NonSerialized] private string errorMessage; [NonSerialized] private bool enterPressed; [NonSerialized] private string password = string.Empty; [NonSerialized] private AuthenticationService authenticationService; @@ -39,6 +39,7 @@ public override void InitializeView(IView parent) { base.InitializeView(parent); need2fa = isBusy = false; + message = errorMessage = null; Title = WindowTitle; Size = viewSize; } @@ -167,8 +168,7 @@ private void OnGUI2FA() if (GUILayout.Button(BackButton)) { GUI.FocusControl(null); - need2fa = false; - Redraw(); + Clear(); } if (GUILayout.Button(TwofaButton) || (!isBusy && enterPressed)) @@ -189,7 +189,7 @@ private void OnGUI2FA() private void DoRequire2fa(string msg) { - Logger.Trace("Strating 2FA - Message:\"{0}\"", msg); + Logger.Trace("Starting 2FA - Message:\"{0}\"", msg); need2fa = true; errorMessage = msg; @@ -197,19 +197,28 @@ private void DoRequire2fa(string msg) Redraw(); } + private void Clear() + { + need2fa = false; + errorMessage = null; + isBusy = false; + Redraw(); + } + private void DoResult(bool success, string msg) { Logger.Trace("DoResult - Success:{0} Message:\"{1}\"", success, msg); - errorMessage = msg; isBusy = false; if (success == true) { + Clear(); Finish(true); } else { + errorMessage = msg; Redraw(); } } From 474dcc71a8d44a5d436320aacb225ebcb7581f9a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 15:13:25 -0400 Subject: [PATCH 22/30] Changing the title content when the view changes --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 86a62cb75..fb9df1cd9 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -88,6 +88,7 @@ private void OpenInternal(PopupViewType popupViewType, Action onClose) } ActiveViewType = popupViewType; + titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); Show(); } From 16031fe2f55a7639be283a348207af4677d6d558 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 15:48:46 -0400 Subject: [PATCH 23/30] Firing OnEnable when we switch the ActiveView --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index fb9df1cd9..c682e0860 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -88,6 +88,7 @@ private void OpenInternal(PopupViewType popupViewType, Action onClose) } ActiveViewType = popupViewType; + ActiveView.OnEnable(); titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); Show(); } From d0c64268dfd4a8b9aa80a82c98df523f61247599 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 15:48:59 -0400 Subject: [PATCH 24/30] Calling Redraw when we switch the ActiveView --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index c682e0860..68b32ff23 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -91,6 +91,7 @@ private void OpenInternal(PopupViewType popupViewType, Action onClose) ActiveView.OnEnable(); titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); Show(); + Redraw(); } public IApiClient Client From 8c6cdfac35860e7ee9181767b53ca13aebe4892a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 15:53:05 -0400 Subject: [PATCH 25/30] Redraw after organizations are loaded --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index 16c3aca36..0e95e4e73 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -117,6 +117,8 @@ private void LoadOwners() owners = new[] { OwnersDefaultText, username }.Union(publishOwners).ToArray(); isBusy = false; + + Redraw(); }, exception => { isBusy = false; From b108e06d1d4c482aef37a1e96ece7c504dfd05ca Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 16:18:00 -0400 Subject: [PATCH 26/30] Changing the log output for KeychainEmptyException --- src/GitHub.Api/Application/ApiClient.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs index 83b6603a1..d456e35af 100644 --- a/src/GitHub.Api/Application/ApiClient.cs +++ b/src/GitHub.Api/Application/ApiClient.cs @@ -258,6 +258,11 @@ private async Task GetCurrentUserInternal() return (await githubClient.User.Current()).ToGitHubUser(); } + catch (KeychainEmptyException) + { + logger.Warning("Keychain is empty"); + throw; + } catch (Exception ex) { logger.Error(ex, "Error Getting Current User"); From 5e87e9a49f4a74ecc1c91289fb21d4a4ad84c832 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 16:39:23 -0400 Subject: [PATCH 27/30] Removing unused code --- .../Editor/GitHub.Unity/UI/PublishView.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index 0e95e4e73..0c3cbad01 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -123,29 +123,6 @@ private void LoadOwners() { isBusy = false; - var tokenUsernameMismatchException = exception as TokenUsernameMismatchException; - if (tokenUsernameMismatchException != null) - { - Logger.Trace("Token Username Mismatch"); - - var shouldProceed = EditorUtility.DisplayDialog(AuthenticationChangedTitle, - string.Format(AuthenticationChangedMessageFormat, - tokenUsernameMismatchException.CachedUsername, - tokenUsernameMismatchException.CurrentUsername), AuthenticationChangedProceed, AuthenticationChangedLogout); - - if (shouldProceed) - { - //Proceed as current user - - } - else - { - //Logout current user and try again - - } - return; - } - var keychainEmptyException = exception as KeychainEmptyException; if (keychainEmptyException != null) { From 684659deb74f3db26293842663b9b3b1be3725eb Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 2 Nov 2017 16:40:43 -0400 Subject: [PATCH 28/30] Removing some unused constant strings --- .../Assets/Editor/GitHub.Unity/UI/PublishView.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs index 0c3cbad01..99ab79516 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs @@ -12,7 +12,6 @@ class PublishView : Subview private static readonly Vector2 viewSize = new Vector2(400, 350); private const string WindowTitle = "Publish"; - private const string Header = "Publish this repository to GitHub"; private const string PrivateRepoMessage = "You choose who can see and commit to this repository"; private const string PublicRepoMessage = "Anyone can see this repository. You choose who can commit"; private const string PublishViewCreateButton = "Publish"; @@ -23,10 +22,6 @@ class PublishView : Subview private const string CreatePrivateRepositoryLabel = "Make repository private"; private const string PublishLimitPrivateRepositoriesError = "You are currently at your limit of private repositories"; private const string PublishToGithubLabel = "Publish to GitHub"; - private const string AuthenticationChangedMessageFormat = "You were authenticated as \"{0}\", but you are now authenticated as \"{1}\". Would you like to proceed or logout?"; - private const string AuthenticationChangedTitle = "Authentication Changed"; - private const string AuthenticationChangedProceed = "Proceed"; - private const string AuthenticationChangedLogout = "Logout"; [SerializeField] private string username; [SerializeField] private string[] owners = { OwnersDefaultText }; From 4009c5513da13270c48261892f4c2378290ff4a7 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Fri, 3 Nov 2017 15:28:46 -0700 Subject: [PATCH 29/30] Doing the same view switching logic that Window has --- .../Editor/GitHub.Unity/UI/PopupWindow.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index 68b32ff23..fae687bea 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -87,11 +87,23 @@ private void OpenInternal(PopupViewType popupViewType, Action onClose) OnClose += onClose; } + var fromView = ActiveView; ActiveViewType = popupViewType; - ActiveView.OnEnable(); - titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); + SwitchView(fromView, ActiveView); Show(); - Redraw(); + } + + private void SwitchView(Subview fromView, Subview toView) + { + GUI.FocusControl(null); + + if (fromView != null) + fromView.OnDisable(); + toView.OnEnable(); + titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); + + // this triggers a repaint + Repaint(); } public IApiClient Client From 1fb203456ca53183e3bccf374db92846ba61a189 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Fri, 3 Nov 2017 15:48:36 -0700 Subject: [PATCH 30/30] Relayout code --- .../Editor/GitHub.Unity/UI/PopupWindow.cs | 154 +++++++++--------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs index fae687bea..b7c860686 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PopupWindow.cs @@ -14,13 +14,13 @@ public enum PopupViewType AuthenticationView, } - [SerializeField] private bool shouldCloseOnFinish; + [NonSerialized] private IApiClient client; + [SerializeField] private PopupViewType activeViewType; [SerializeField] private AuthenticationView authenticationView; - [SerializeField] private PublishView publishView; [SerializeField] private LoadingView loadingView; - - [NonSerialized] private IApiClient client; + [SerializeField] private PublishView publishView; + [SerializeField] private bool shouldCloseOnFinish; public event Action OnClose; @@ -39,6 +39,79 @@ public static PopupWindow OpenWindow(PopupViewType popupViewType, Action o return popupWindow; } + public override void Initialize(IApplicationManager applicationManager) + { + base.Initialize(applicationManager); + + publishView = publishView ?? new PublishView(); + authenticationView = authenticationView ?? new AuthenticationView(); + loadingView = loadingView ?? new LoadingView(); + + publishView.InitializeView(this); + authenticationView.InitializeView(this); + loadingView.InitializeView(this); + + titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); + } + + public override void OnEnable() + { + base.OnEnable(); + minSize = maxSize = ActiveView.Size; + ActiveView.OnEnable(); + } + + public override void OnDisable() + { + base.OnDisable(); + ActiveView.OnDisable(); + } + + public override void OnDataUpdate() + { + base.OnDataUpdate(); + ActiveView.OnDataUpdate(); + } + + public override void OnUI() + { + base.OnUI(); + ActiveView.OnGUI(); + } + + public override void Refresh() + { + base.Refresh(); + ActiveView.Refresh(); + } + + public override void OnSelectionChange() + { + base.OnSelectionChange(); + ActiveView.OnSelectionChange(); + } + + public override void Finish(bool result) + { + OnClose.SafeInvoke(result); + OnClose = null; + + if (shouldCloseOnFinish) + { + shouldCloseOnFinish = false; + Close(); + } + + base.Finish(result); + } + + public override void OnDestroy() + { + base.OnDestroy(); + OnClose.SafeInvoke(false); + OnClose = null; + } + private void Open(PopupViewType popupViewType, Action onClose) { OnClose.SafeInvoke(false); @@ -130,79 +203,6 @@ public IApiClient Client } } - public override void Initialize(IApplicationManager applicationManager) - { - base.Initialize(applicationManager); - - publishView = publishView ?? new PublishView(); - authenticationView = authenticationView ?? new AuthenticationView(); - loadingView = loadingView ?? new LoadingView(); - - publishView.InitializeView(this); - authenticationView.InitializeView(this); - loadingView.InitializeView(this); - - titleContent = new GUIContent(ActiveView.Title, Styles.SmallLogo); - } - - public override void OnEnable() - { - base.OnEnable(); - minSize = maxSize = ActiveView.Size; - ActiveView.OnEnable(); - } - - public override void OnDisable() - { - base.OnDisable(); - ActiveView.OnDisable(); - } - - public override void OnDataUpdate() - { - base.OnDataUpdate(); - ActiveView.OnDataUpdate(); - } - - public override void OnUI() - { - base.OnUI(); - ActiveView.OnGUI(); - } - - public override void Refresh() - { - base.Refresh(); - ActiveView.Refresh(); - } - - public override void OnSelectionChange() - { - base.OnSelectionChange(); - ActiveView.OnSelectionChange(); - } - - public override void Finish(bool result) - { - OnClose.SafeInvoke(result); - OnClose = null; - - if (shouldCloseOnFinish) - { - shouldCloseOnFinish = false; - Close(); - } - - base.Finish(result); - } - - public override void OnDestroy() - { - base.OnDestroy(); - OnClose.SafeInvoke(false); - OnClose = null; - } - private Subview ActiveView { get