From 60cadb8b6d6cc9b08fbeb8ca30e1a24c13726725 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Mon, 5 Oct 2020 15:42:19 +0900 Subject: [PATCH 1/2] Do not show password in clear text in email configuration (Fix #1790) --- BTCPayServer/Controllers/ServerController.cs | 34 +++++-- .../Controllers/StoresController.Email.cs | 25 ++++- .../ServerViewModels/EmailsViewModel.cs | 11 ++- BTCPayServer/Services/Mails/EmailSettings.cs | 1 + BTCPayServer/Views/Shared/EmailsBody.cshtml | 97 +++++++++++-------- 5 files changed, 115 insertions(+), 53 deletions(-) diff --git a/BTCPayServer/Controllers/ServerController.cs b/BTCPayServer/Controllers/ServerController.cs index 24497b815..0653ac520 100644 --- a/BTCPayServer/Controllers/ServerController.cs +++ b/BTCPayServer/Controllers/ServerController.cs @@ -944,23 +944,28 @@ namespace BTCPayServer.Controllers public async Task Emails() { var data = (await _SettingsRepository.GetSettingAsync()) ?? new EmailSettings(); - return View(new EmailsViewModel() { Settings = data }); + return View(new EmailsViewModel(data)); } [Route("server/emails")] [HttpPost] public async Task Emails(EmailsViewModel model, string command) { - if (!model.Settings.IsComplete()) - { - TempData[WellKnownTempData.ErrorMessage] = "Required fields missing"; - return View(model); - } - + if (command == "Test") { try { + if (model.PasswordSet) + { + var settings = await _SettingsRepository.GetSettingAsync(); + model.Settings.Password = settings.Password; + } + if (!model.Settings.IsComplete()) + { + TempData[WellKnownTempData.ErrorMessage] = "Required fields missing"; + return View(model); + } using (var client = model.Settings.CreateSmtpClient()) using (var message = model.Settings.CreateMailMessage(new MailAddress(model.TestEmail), "BTCPay test", "BTCPay test")) { @@ -974,11 +979,24 @@ namespace BTCPayServer.Controllers } return View(model); } + else if (command == "ResetPassword") + { + var settings = await _SettingsRepository.GetSettingAsync(); + settings.Password = null; + await _SettingsRepository.UpdateSetting(model.Settings); + TempData[WellKnownTempData.SuccessMessage] = "Email server password reset"; + return RedirectToAction(nameof(Emails)); + } else // if(command == "Save") { + var oldSettings = await _SettingsRepository.GetSettingAsync(); + if (new EmailsViewModel(oldSettings).PasswordSet) + { + model.Settings.Password = oldSettings.Password; + } await _SettingsRepository.UpdateSetting(model.Settings); TempData[WellKnownTempData.SuccessMessage] = "Email settings saved"; - return View(model); + return RedirectToAction(nameof(Emails)); } } diff --git a/BTCPayServer/Controllers/StoresController.Email.cs b/BTCPayServer/Controllers/StoresController.Email.cs index 600c74db9..df5e2d5ba 100644 --- a/BTCPayServer/Controllers/StoresController.Email.cs +++ b/BTCPayServer/Controllers/StoresController.Email.cs @@ -18,7 +18,7 @@ namespace BTCPayServer.Controllers if (store == null) return NotFound(); var data = store.GetStoreBlob().EmailSettings ?? new EmailSettings(); - return View(new EmailsViewModel() { Settings = data }); + return View(new EmailsViewModel(data)); } [Route("{storeId}/emails")] @@ -32,6 +32,10 @@ namespace BTCPayServer.Controllers { try { + if (model.PasswordSet) + { + model.Settings.Password = store.GetStoreBlob().EmailSettings.Password; + } if (!model.Settings.IsComplete()) { TempData[WellKnownTempData.ErrorMessage] = "Required fields missing"; @@ -48,10 +52,26 @@ namespace BTCPayServer.Controllers } return View(model); } + else if (command == "ResetPassword") + { + var storeBlob = store.GetStoreBlob(); + storeBlob.EmailSettings.Password = null; + store.SetStoreBlob(storeBlob); + await _Repo.UpdateStore(store); + TempData[WellKnownTempData.SuccessMessage] = "Email server password reset"; + return RedirectToAction(nameof(UpdateStore), new + { + storeId + }); + } else // if(command == "Save") { - var storeBlob = store.GetStoreBlob(); + var oldPassword = storeBlob.EmailSettings?.Password; + if (new EmailsViewModel(storeBlob.EmailSettings).PasswordSet) + { + model.Settings.Password = storeBlob.EmailSettings.Password; + } storeBlob.EmailSettings = model.Settings; store.SetStoreBlob(storeBlob); await _Repo.UpdateStore(store); @@ -60,7 +80,6 @@ namespace BTCPayServer.Controllers { storeId }); - } } } diff --git a/BTCPayServer/Models/ServerViewModels/EmailsViewModel.cs b/BTCPayServer/Models/ServerViewModels/EmailsViewModel.cs index 8d63c1189..29c90df51 100644 --- a/BTCPayServer/Models/ServerViewModels/EmailsViewModel.cs +++ b/BTCPayServer/Models/ServerViewModels/EmailsViewModel.cs @@ -5,11 +5,20 @@ namespace BTCPayServer.Models.ServerViewModels { public class EmailsViewModel { + public EmailsViewModel() + { + + } + public EmailsViewModel(EmailSettings settings) + { + Settings = settings; + PasswordSet = !string.IsNullOrEmpty(settings?.Password); + } public EmailSettings Settings { get; set; } - + public bool PasswordSet { get; set; } [EmailAddress] [Display(Name = "Test Email")] public string TestEmail diff --git a/BTCPayServer/Services/Mails/EmailSettings.cs b/BTCPayServer/Services/Mails/EmailSettings.cs index 083817671..9483b2f46 100644 --- a/BTCPayServer/Services/Mails/EmailSettings.cs +++ b/BTCPayServer/Services/Mails/EmailSettings.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using System.Net; using System.Net.Mail; +using Newtonsoft.Json; namespace BTCPayServer.Services.Mails { diff --git a/BTCPayServer/Views/Shared/EmailsBody.cshtml b/BTCPayServer/Views/Shared/EmailsBody.cshtml index 246377d2e..8f6825da5 100644 --- a/BTCPayServer/Views/Shared/EmailsBody.cshtml +++ b/BTCPayServer/Views/Shared/EmailsBody.cshtml @@ -1,4 +1,4 @@ -@model BTCPayServer.Models.ServerViewModels.EmailsViewModel +@model BTCPayServer.Models.ServerViewModels.EmailsViewModel @@ -27,36 +27,36 @@
@@ -64,17 +64,17 @@ $(document).ready(function(){
- +
- +
- + Some email providers (like Gmail) don't allow you to set your display name, so this setting may not have any effect. @@ -82,28 +82,43 @@ $(document).ready(function(){
- +
- + - For many email providers (like Gmail) your login is your email address. + For many email providers (like Gmail) your login is your email address.
- - - + @if (!Model.PasswordSet) + { + + + + + } + else + { + +
+ +
+ +
+
+ }
- +
+
@@ -116,11 +131,11 @@ $(document).ready(function(){

If you want to test your settings, enter an email address here and click "Send Test Email". Your settings won't be saved, only a test email will be sent. -
+
After a successful test, you can click "Save".

- + From d8da8023c27bb393d7dfc2b19cc46add674a5cde Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Mon, 5 Oct 2020 16:39:49 +0900 Subject: [PATCH 2/2] Add tests --- BTCPayServer.Tests/SeleniumTester.cs | 5 +++ BTCPayServer.Tests/SeleniumTests.cs | 40 +++++++++++++++++++ .../Controllers/StoresController.Email.cs | 4 +- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/BTCPayServer.Tests/SeleniumTester.cs b/BTCPayServer.Tests/SeleniumTester.cs index 96c853b68..b3d36c370 100644 --- a/BTCPayServer.Tests/SeleniumTester.cs +++ b/BTCPayServer.Tests/SeleniumTester.cs @@ -384,6 +384,11 @@ namespace BTCPayServer.Tests Driver.FindElement(By.Id($"Wallet{navPages}")).Click(); } } + + public void GoToUrl(string relativeUrl) + { + Driver.Navigate().GoToUrl(new Uri(Server.PayTester.ServerUri, relativeUrl)); + } public void GoToServer(ServerNavPages navPages = ServerNavPages.Index) { diff --git a/BTCPayServer.Tests/SeleniumTests.cs b/BTCPayServer.Tests/SeleniumTests.cs index 531200616..b57efba9d 100644 --- a/BTCPayServer.Tests/SeleniumTests.cs +++ b/BTCPayServer.Tests/SeleniumTests.cs @@ -203,6 +203,46 @@ namespace BTCPayServer.Tests } } + [Fact(Timeout = TestTimeout)] + public async Task CanSetupEmailServer() + { + using (var s = SeleniumTester.Create()) + { + await s.StartAsync(); + var alice = s.RegisterNewUser(isAdmin: true); + s.Driver.Navigate().GoToUrl(s.Link("/server/emails")); + if (s.Driver.PageSource.Contains("Configured")) + { + s.Driver.FindElement(By.CssSelector("button[value=\"ResetPassword\"]")).Submit(); + s.AssertHappyMessage(); + } + CanSetupEmailCore(s); + s.CreateNewStore(); + s.GoToUrl($"stores/{s.StoreId}/emails"); + CanSetupEmailCore(s); + } + } + + private static void CanSetupEmailCore(SeleniumTester s) + { + s.Driver.FindElement(By.ClassName("dropdown-toggle")).Click(); + s.Driver.FindElement(By.ClassName("dropdown-item")).Click(); + s.Driver.FindElement(By.Id("Settings_Login")).SendKeys("test@gmail.com"); + s.Driver.FindElement(By.CssSelector("button[value=\"Save\"]")).Submit(); + s.AssertHappyMessage(); + s.Driver.FindElement(By.Id("Settings_Password")).SendKeys("mypassword"); + s.Driver.FindElement(By.CssSelector("button[value=\"Save\"]")).Submit(); + Assert.Contains("Configured", s.Driver.PageSource); + s.Driver.FindElement(By.Id("Settings_Login")).SendKeys("test_fix@gmail.com"); + s.Driver.FindElement(By.CssSelector("button[value=\"Save\"]")).Submit(); + Assert.Contains("Configured", s.Driver.PageSource); + Assert.Contains("test_fix", s.Driver.PageSource); + s.Driver.FindElement(By.CssSelector("button[value=\"ResetPassword\"]")).Submit(); + s.AssertHappyMessage(); + Assert.DoesNotContain("Configured", s.Driver.PageSource); + Assert.Contains("test_fix", s.Driver.PageSource); + } + [Fact(Timeout = TestTimeout)] public async Task CanUseDynamicDns() { diff --git a/BTCPayServer/Controllers/StoresController.Email.cs b/BTCPayServer/Controllers/StoresController.Email.cs index df5e2d5ba..1551489e2 100644 --- a/BTCPayServer/Controllers/StoresController.Email.cs +++ b/BTCPayServer/Controllers/StoresController.Email.cs @@ -59,7 +59,7 @@ namespace BTCPayServer.Controllers store.SetStoreBlob(storeBlob); await _Repo.UpdateStore(store); TempData[WellKnownTempData.SuccessMessage] = "Email server password reset"; - return RedirectToAction(nameof(UpdateStore), new + return RedirectToAction(nameof(Emails), new { storeId }); @@ -76,7 +76,7 @@ namespace BTCPayServer.Controllers store.SetStoreBlob(storeBlob); await _Repo.UpdateStore(store); TempData[WellKnownTempData.SuccessMessage] = "Email settings modified"; - return RedirectToAction(nameof(UpdateStore), new + return RedirectToAction(nameof(Emails), new { storeId });