From 284c395171e390f52b9fbdef612f1109a1db12f6 Mon Sep 17 00:00:00 2001 From: Matteo Cominetti Date: Mon, 13 Feb 2023 11:27:50 +0000 Subject: [PATCH] Matteo/core/account flow (#2140) * fix: prevents multiple account flows running at the same time, properly reports error * fix(dui): prevent multiple add account flows * fix(core): better disposal of listener * fix(core): prevent double error reporting * chore: comment cleanup --- Core/Core/Credentials/AccountManager.cs | 168 ++++++++++-------- DesktopUI2/DesktopUI2/Utils.cs | 5 +- .../DesktopUI2/ViewModels/HomeViewModel.cs | 4 +- .../DesktopUI2/Views/Pages/HomeView.xaml | 1 + 4 files changed, 99 insertions(+), 79 deletions(-) diff --git a/Core/Core/Credentials/AccountManager.cs b/Core/Core/Credentials/AccountManager.cs index 957744fa9d..15c65fbad3 100644 --- a/Core/Core/Credentials/AccountManager.cs +++ b/Core/Core/Credentials/AccountManager.cs @@ -27,6 +27,7 @@ namespace Speckle.Core.Credentials public static class AccountManager { private static SQLiteTransport AccountStorage = new SQLiteTransport(scope: "Accounts"); + private static bool _isAddingAccount = false; /// /// Gets the basic information about a server. @@ -335,9 +336,24 @@ public static void ChangeDefaultAccount(string id) public static async Task AddAccount(string server = "") { Log.Debug("Starting to add account for {serverUrl}", server); + + if (!HttpListener.IsSupported) + { + Log.Error( + "HttpListener not supported" + ); + throw new Exception("Your operating system is not supported"); + } + + //prevent launching this flow multiple times + if (_isAddingAccount) + return; + + _isAddingAccount = true; server = server.TrimEnd(new[] { '/' }); - if (string.IsNullOrEmpty(server)) { + if (string.IsNullOrEmpty(server)) + { server = GetDefaultServerUrl(); Log.Debug("Changed server url to the default usl {serverUrl}", server); } @@ -349,99 +365,101 @@ public static async Task AddAccount(string server = "") new ProcessStartInfo($"{server}/authn/verify/sca/{challenge}") { UseShellExecute = true } ); - HttpListener listener = new HttpListener(); - - //does nothing? - var timeout = TimeSpan.FromMinutes(2); - //listener.TimeoutManager.HeaderWait = timeout; - //listener.TimeoutManager.EntityBody = timeout; - //listener.TimeoutManager.IdleConnection = timeout; + //timeout for the task + var timeout = TimeSpan.FromMinutes(1); + var listener = new HttpListener(); var task = Task.Run(() => { - try - { - if (!HttpListener.IsSupported) - { - Log.Warning( - "Windows XP SP2 or Server 2003 is required to use the HttpListener class." - ); - return; - } - listener = new HttpListener(); - var localUrl = "http://localhost:29363/"; - listener.Prefixes.Add(localUrl); - - listener.Start(); - Log.Debug("Listening for auth redirects on {localUrl}", localUrl); - // Note: The GetContext method blocks while waiting for a request. - HttpListenerContext context = listener.GetContext(); - HttpListenerRequest request = context.Request; - HttpListenerResponse response = context.Response; - - accessCode = request.QueryString["access_code"]; - Log.Debug("Got access code {accessCode}", accessCode); - var message = ""; - if (accessCode != null) - { - message = - "Yay!

You can close this window now."; - } - else - { - message = "Oups, something went wrong...!"; - } + var localUrl = "http://localhost:29363/"; + listener.Prefixes.Add(localUrl); + listener.Start(); + Log.Debug("Listening for auth redirects on {localUrl}", localUrl); + // Note: The GetContext method blocks while waiting for a request. + HttpListenerContext context = listener.GetContext(); + HttpListenerRequest request = context.Request; + HttpListenerResponse response = context.Response; + + accessCode = request.QueryString["access_code"]; + Log.Debug("Got access code {accessCode}", accessCode); + var message = ""; + if (accessCode != null) + message = "Success!

You can close this window now."; + else + message = "Oups, something went wrong...!"; + + var responseString = + $"
{message}"; + byte[] buffer = System.Text.Encoding.UTF8.GetBytes(responseString); + response.ContentLength64 = buffer.Length; + System.IO.Stream output = response.OutputStream; + output.Write(buffer, 0, buffer.Length); + output.Close(); + Log.Debug("Processed finished processing the access code."); + listener.Stop(); + listener.Close(); + }); - var responseString = - $"
{message}"; + var completedTask = await Task.WhenAny(task, Task.Delay(timeout)); - byte[] buffer = System.Text.Encoding.UTF8.GetBytes(responseString); - response.ContentLength64 = buffer.Length; - System.IO.Stream output = response.OutputStream; - output.Write(buffer, 0, buffer.Length); - output.Close(); - Log.Debug("Processed finished processing the access code."); - listener.Stop(); - } - catch (Exception ex) - { - Log.Error(ex, "Getting access code flow failed with {exceptionMessage}", ex.Message); - } - }); + //ensure the listener is closed even if the task has timed out or failed + if (listener.IsListening) + listener.Abort(); - //Timeout - if (await Task.WhenAny(task, Task.Delay(timeout)) == task) + //check if timed out or not + if (completedTask == task) { - Log.Information("Local auth flow completed within the timeout window. Access code is {accessCode}", accessCode); + if (task.IsFaulted) + { + Log.Error(task.Exception, "Getting access code flow failed with {exceptionMessage}", task.Exception.Message); + _isAddingAccount = false; + throw new Exception($"Auth flow failed: {task.Exception.Message}", task.Exception); + } + // task completed within timeout + Log.Information("Local auth flow completed successfully within the timeout window. Access code is {accessCode}", accessCode); } else { - // nada + // task timed out Log.Warning("Local auth flow failed to complete within the timeout window. Access code is {accessCode}", accessCode); + _isAddingAccount = false; + throw new Exception("Local auth flow failed to complete within the timeout window"); } if (string.IsNullOrEmpty(accessCode)) - return; - - var tokenResponse = (await GetToken(accessCode, challenge, server)); + { + _isAddingAccount = false; + Log.Warning("Access code is invalid {accessCode}", accessCode); + throw new Exception("Access code is ivalid"); + } + try + { + var tokenResponse = await GetToken(accessCode, challenge, server); + var userResponse = await GetUserServerInfo(tokenResponse.token, server); - var userResponse = await GetUserServerInfo(tokenResponse.token, server); + var account = new Account() + { + token = tokenResponse.token, + refreshToken = tokenResponse.refreshToken, + isDefault = GetAccounts().Count() == 0, + serverInfo = userResponse.serverInfo, + userInfo = userResponse.user + }; + Log.Information("Successfully created account for {serverUrl}", server); + account.serverInfo.url = server; - var account = new Account() + //if the account already exists it will not be added again + AccountStorage.SaveObject(account.id, JsonConvert.SerializeObject(account)); + } + catch (Exception ex) { - token = tokenResponse.token, - refreshToken = tokenResponse.refreshToken, - isDefault = GetAccounts().Count() == 0, - serverInfo = userResponse.serverInfo, - userInfo = userResponse.user - }; - Log.Information("Successfully created account for {serverUrl}", server); - account.serverInfo.url = server; - - //if the account already exists it will not be added again - AccountStorage.SaveObject(account.id, JsonConvert.SerializeObject(account)); + Log.Error(ex, "Error adding the account"); + throw new Exception("Something went wrong adding the account, please try again"); + } + _isAddingAccount = false; + } private static async Task GetToken( diff --git a/DesktopUI2/DesktopUI2/Utils.cs b/DesktopUI2/DesktopUI2/Utils.cs index 2de6beb143..bc49e38f2e 100644 --- a/DesktopUI2/DesktopUI2/Utils.cs +++ b/DesktopUI2/DesktopUI2/Utils.cs @@ -251,10 +251,9 @@ public static async Task AddAccountCommand() Dialogs.ShowDialog("Error", "Invalid URL", Material.Dialog.Icons.DialogIconKind.Error); else { + Analytics.TrackEvent(Analytics.Events.DUIAction, new Dictionary() { { "name", "Account Add" } }); try { - Analytics.TrackEvent(Analytics.Events.DUIAction, new Dictionary() { { "name", "Account Add" } }); - await AccountManager.AddAccount(result); await Task.Delay(1000); @@ -262,7 +261,7 @@ public static async Task AddAccountCommand() } catch (Exception e) { - new SpeckleException("Could not Add Account in AccountManager", e, true, Sentry.SentryLevel.Error); + //errors already handled in AddAccount MainUserControl.NotificationManager.Show(new PopUpNotificationViewModel() { diff --git a/DesktopUI2/DesktopUI2/ViewModels/HomeViewModel.cs b/DesktopUI2/DesktopUI2/ViewModels/HomeViewModel.cs index 1c71596a09..c9bf3eccbb 100644 --- a/DesktopUI2/DesktopUI2/ViewModels/HomeViewModel.cs +++ b/DesktopUI2/DesktopUI2/ViewModels/HomeViewModel.cs @@ -13,6 +13,7 @@ using Material.Styles.Themes; using Material.Styles.Themes.Base; using ReactiveUI; +using Serilog; using Speckle.Core.Api; using Speckle.Core.Credentials; using Speckle.Core.Helpers; @@ -29,7 +30,6 @@ using System.Threading; using System.Threading.Tasks; using Stream = Speckle.Core.Api.Stream; -using Serilog; namespace DesktopUI2.ViewModels { @@ -644,7 +644,9 @@ private void RemoveSavedStream(string stateId) public async void AddAccountCommand() { + InProgress = true; await Utils.AddAccountCommand(); + InProgress = false; } public async void RemoveAccountCommand(Account account) { diff --git a/DesktopUI2/DesktopUI2/Views/Pages/HomeView.xaml b/DesktopUI2/DesktopUI2/Views/Pages/HomeView.xaml index 0c5ce85851..2a0efe4cf6 100644 --- a/DesktopUI2/DesktopUI2/Views/Pages/HomeView.xaml +++ b/DesktopUI2/DesktopUI2/Views/Pages/HomeView.xaml @@ -76,6 +76,7 @@