From d621a5d2f32454f206cc09937e484e823dab05f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Gon=C3=A7alves?= <cgoncalves@bitwarden.com>
Date: Mon, 11 Jul 2022 18:02:11 +0100
Subject: [PATCH] [PS 920] Fix selfhosted url validations (#1967)

* PS-920 - Added feedback to user when saving bad formed URLs
* Added feedback to user when trying to perform login with bad formed URL

* PS-920 - Refactor to use AsyncCommand
*(missing file from previous commit)

* PS-920 - Fixed whitespace formatting

* PS-920 - Removed unused method

* PS-920 - Fixed validation
* Added comment for hard coded string

* PS-920 - Removed unused properties
* Fixed url validations
* Refactored method to local function

* PS-920 - Added exception handling and logging
* Added generic error message string to AppResources
---
 src/App/Pages/Accounts/EnvironmentPage.xaml   |  2 +-
 .../Pages/Accounts/EnvironmentPage.xaml.cs    |  8 -----
 .../Accounts/EnvironmentPageViewModel.cs      | 34 +++++++++++++++++--
 src/App/Resources/AppResources.Designer.cs    | 14 +++++++-
 src/App/Resources/AppResources.resx           |  8 ++++-
 src/Core/Services/ApiService.cs               | 12 +++++++
 6 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/src/App/Pages/Accounts/EnvironmentPage.xaml b/src/App/Pages/Accounts/EnvironmentPage.xaml
index 5a7abb783..cabf07ed7 100644
--- a/src/App/Pages/Accounts/EnvironmentPage.xaml
+++ b/src/App/Pages/Accounts/EnvironmentPage.xaml
@@ -14,7 +14,7 @@
 
     <ContentPage.ToolbarItems>
         <ToolbarItem Text="{u:I18n Cancel}" Clicked="Close_Clicked" Order="Primary" Priority="-1" />
-        <ToolbarItem Text="{u:I18n Save}" Clicked="Submit_Clicked" />
+        <ToolbarItem Text="{u:I18n Save}" Command="{Binding SubmitCommand}" />
     </ContentPage.ToolbarItems>
 
     <ScrollView>
diff --git a/src/App/Pages/Accounts/EnvironmentPage.xaml.cs b/src/App/Pages/Accounts/EnvironmentPage.xaml.cs
index b57d2f440..baa2dd1c0 100644
--- a/src/App/Pages/Accounts/EnvironmentPage.xaml.cs
+++ b/src/App/Pages/Accounts/EnvironmentPage.xaml.cs
@@ -36,14 +36,6 @@ namespace Bit.App.Pages
             };
         }
 
-        private async void Submit_Clicked(object sender, EventArgs e)
-        {
-            if (DoOnce())
-            {
-                await _vm.SubmitAsync();
-            }
-        }
-
         private async Task SubmitSuccessAsync()
         {
             _platformUtilsService.ShowToast("success", null, AppResources.EnvironmentSaved);
diff --git a/src/App/Pages/Accounts/EnvironmentPageViewModel.cs b/src/App/Pages/Accounts/EnvironmentPageViewModel.cs
index b25113c41..07ce99a45 100644
--- a/src/App/Pages/Accounts/EnvironmentPageViewModel.cs
+++ b/src/App/Pages/Accounts/EnvironmentPageViewModel.cs
@@ -1,15 +1,17 @@
 using System;
 using System.Threading.Tasks;
+using System.Windows.Input;
 using Bit.App.Resources;
 using Bit.Core.Abstractions;
 using Bit.Core.Utilities;
-using Xamarin.Forms;
+using Xamarin.CommunityToolkit.ObjectModel;
 
 namespace Bit.App.Pages
 {
     public class EnvironmentPageViewModel : BaseViewModel
     {
         private readonly IEnvironmentService _environmentService;
+        readonly LazyResolve<ILogger> _logger = new LazyResolve<ILogger>("logger");
 
         public EnvironmentPageViewModel()
         {
@@ -22,10 +24,10 @@ namespace Bit.App.Pages
             IdentityUrl = _environmentService.IdentityUrl;
             IconsUrl = _environmentService.IconsUrl;
             NotificationsUrls = _environmentService.NotificationsUrl;
-            SubmitCommand = new Command(async () => await SubmitAsync());
+            SubmitCommand = new AsyncCommand(SubmitAsync, onException: ex => OnSubmitException(ex), allowsMultipleExecutions: false);
         }
 
-        public Command SubmitCommand { get; }
+        public ICommand SubmitCommand { get; }
         public string BaseUrl { get; set; }
         public string ApiUrl { get; set; }
         public string IdentityUrl { get; set; }
@@ -37,6 +39,12 @@ namespace Bit.App.Pages
 
         public async Task SubmitAsync()
         {
+            if (!ValidateUrls())
+            {
+                await Page.DisplayAlert(AppResources.AnErrorHasOccurred, AppResources.EnvironmentPageUrlsError, AppResources.Ok);
+                return;
+            }
+
             var resUrls = await _environmentService.SetUrlsAsync(new Core.Models.Data.EnvironmentUrlData
             {
                 Base = BaseUrl,
@@ -57,5 +65,25 @@ namespace Bit.App.Pages
 
             SubmitSuccessAction?.Invoke();
         }
+
+        public bool ValidateUrls()
+        {
+            bool IsUrlValid(string url)
+            {
+                return string.IsNullOrEmpty(url) || Uri.IsWellFormedUriString(url, UriKind.Absolute);
+            }
+
+            return IsUrlValid(BaseUrl)
+                && IsUrlValid(ApiUrl)
+                && IsUrlValid(IdentityUrl)
+                && IsUrlValid(WebVaultUrl)
+                && IsUrlValid(IconsUrl);
+        }
+
+        private void OnSubmitException(Exception ex)
+        {
+            _logger.Value.Exception(ex);
+            Page.DisplayAlert(AppResources.AnErrorHasOccurred, AppResources.GenericErrorMessage, AppResources.Ok);
+        }
     }
 }
diff --git a/src/App/Resources/AppResources.Designer.cs b/src/App/Resources/AppResources.Designer.cs
index e25fcdcdb..09f2c3f98 100644
--- a/src/App/Resources/AppResources.Designer.cs
+++ b/src/App/Resources/AppResources.Designer.cs
@@ -1,4 +1,4 @@
-//------------------------------------------------------------------------------
+//------------------------------------------------------------------------------
 // <auto-generated>
 //     This code was generated by a tool.
 //     Runtime Version:4.0.30319.42000
@@ -4044,5 +4044,17 @@ namespace Bit.App.Resources {
                 return ResourceManager.GetString("NeverLockWarning", resourceCulture);
             }
         }
+        
+        public static string EnvironmentPageUrlsError {
+            get {
+                return ResourceManager.GetString("EnvironmentPageUrlsError", resourceCulture);
+            }
+        }
+        
+        public static string GenericErrorMessage {
+            get {
+                return ResourceManager.GetString("GenericErrorMessage", resourceCulture);
+            }
+        }
     }
 }
diff --git a/src/App/Resources/AppResources.resx b/src/App/Resources/AppResources.resx
index a04488167..58cc1f7c9 100644
--- a/src/App/Resources/AppResources.resx
+++ b/src/App/Resources/AppResources.resx
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <root>
   <!-- 
     Microsoft ResX Schema 
@@ -2257,4 +2257,10 @@
   <data name="NeverLockWarning" xml:space="preserve">
     <value>Setting your lock options to “Never” keeps your vault available to anyone with access to your device. If you use this option, you should ensure that you keep your device properly protected.</value>
   </data>
+  <data name="EnvironmentPageUrlsError" xml:space="preserve">
+    <value>One or more of the URLs entered are invalid. Please revise it and try to save again.</value>
+  </data>
+  <data name="GenericErrorMessage" xml:space="preserve">
+    <value>We were unable to process your request. Please try again or contact us.</value>
+  </data>
 </root>
diff --git a/src/Core/Services/ApiService.cs b/src/Core/Services/ApiService.cs
index ad462c058..d29e26bca 100644
--- a/src/Core/Services/ApiService.cs
+++ b/src/Core/Services/ApiService.cs
@@ -589,7 +589,19 @@ namespace Bit.Core.Services
             {
                 requestMessage.Version = new Version(1, 0);
                 requestMessage.Method = method;
+
+                if (!Uri.IsWellFormedUriString(ApiBaseUrl, UriKind.Absolute))
+                {
+                    throw new ApiException(new ErrorResponse
+                    {
+                        StatusCode = HttpStatusCode.BadGateway,
+                        //Note: This message is hardcoded until AppResources.resx gets moved into Core.csproj
+                        Message = "One or more URLs saved in the Settings are incorrect. Please revise it and try to log in again."
+                    });
+                }
+
                 requestMessage.RequestUri = new Uri(string.Concat(ApiBaseUrl, path));
+
                 if (body != null)
                 {
                     var bodyType = body.GetType();