Refactoring and commenting

BTCPayServer.Tests/GreenfieldAPITests.cs

@@ -4115,12 +4115,15 @@ namespace BTCPayServer.Tests
             };
             var actualUpdated = await adminClient.UpdateServerEmailSettings(data);
             
-            var actualUpdated2 = await adminClient.GetServerEmailSettings();
+            var finalEmailSettings = await adminClient.GetServerEmailSettings();
             // email password is masked and not returned from the server once set
             data.Password = null;
             data.PasswordSet = true;
-            Assert.Equal(JsonConvert.SerializeObject(actualUpdated2), JsonConvert.SerializeObject(data));
+            
-            Assert.Equal(JsonConvert.SerializeObject(actualUpdated2), JsonConvert.SerializeObject(actualUpdated));
+            Assert.Equal(JsonConvert.SerializeObject(finalEmailSettings), JsonConvert.SerializeObject(data));
+            Assert.Equal(JsonConvert.SerializeObject(finalEmailSettings), JsonConvert.SerializeObject(actualUpdated));
+            
+            // check that email validation works
             await AssertValidationError(new[] { nameof(EmailSettingsData.From) },
                 async () => await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData
                 {

BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs

@@ -31,28 +31,30 @@ namespace BTCPayServer.Controllers.GreenField
             _settingsRepository = settingsRepository;
         }
         
-        [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
+        private ServerEmailSettingsData ToApiModel(EmailSettings email)
-        [HttpGet("~/api/v1/server/email")]
-        public async Task<IActionResult> ServerEmailSettings()
-        {
-            var email = await _emailSenderFactory.GetSettings() ?? new EmailSettings();
-            return Ok(FromModel(email));
-        }
-
-        private ServerEmailSettingsData FromModel(EmailSettings email)
         {
             var data = email.ToData<ServerEmailSettingsData>();
             data.EnableStoresToUseServerEmailSettings = !_policiesSettings.DisableStoresToUseServerEmailSettings;
             return data;
         }
 
+        [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
+        [HttpGet("~/api/v1/server/email")]
+        public async Task<IActionResult> ServerEmailSettings()
+        {
+            var email = await _emailSenderFactory.GetSettings() ?? new EmailSettings();
+            return Ok(ToApiModel(email));
+        }
+
         [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
         [HttpPut("~/api/v1/server/email")]
         public async Task<IActionResult> ServerEmailSettings(ServerEmailSettingsData request)
         {
-            request.Validate(ModelState);
+            if (!MailboxAddressValidator.IsMailboxAddress(request.From))
-            if (!ModelState.IsValid)
+            {
+                ModelState.AddModelError(nameof(request.From), "Invalid email address");
                 return this.CreateValidationError(ModelState);
+            }
 
             if (_policiesSettings.DisableStoresToUseServerEmailSettings == request.EnableStoresToUseServerEmailSettings)
             {
@@ -65,7 +67,7 @@ namespace BTCPayServer.Controllers.GreenField
 
             // important to save as EmailSettings otherwise it won't be able to be fetched
             await _settingsRepository.UpdateSetting(settings);
-            return Ok(FromModel(settings));
+            return Ok(ToApiModel(settings));
         }
     }
 }

BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs

@@ -51,34 +51,40 @@ namespace BTCPayServer.Controllers.GreenField
             return Ok();
         }
 
+
+        private EmailSettingsData ToApiModel(Data.StoreData data)
+        {
+            var storeEmailSettings = data.GetStoreBlob().EmailSettings ?? new();
+            return storeEmailSettings.ToData<EmailSettingsData>();
+        }
+
         [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
         [HttpGet("~/api/v1/stores/{storeId}/email")]
         public IActionResult GetStoreEmailSettings()
         {
             var store = HttpContext.GetStoreData();
-            return store == null ? StoreNotFound() : Ok(FromModel(store));
+            return store == null ? StoreNotFound() : Ok(ToApiModel(store));
         }
 
         [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
         [HttpPut("~/api/v1/stores/{storeId}/email")]
         public async Task<IActionResult> UpdateStoreEmailSettings(string storeId, EmailSettingsData request)
         {
-            var store = HttpContext.GetStoreData();
+            if (!MailboxAddressValidator.IsMailboxAddress(request.From))
-			request ??= new();
+            {
-			request.Validate(this.ModelState);
+                ModelState.AddModelError(nameof(request.From), "Invalid email address");
-			if (!ModelState.IsValid)
                 return this.CreateValidationError(ModelState);
+            }
 
+            var store = HttpContext.GetStoreData();
             var blob = store.GetStoreBlob();
             var settings = EmailSettings.FromData(request, blob.EmailSettings?.Password);
             blob.EmailSettings = settings;
             if (store.SetStoreBlob(blob))
                 await _storeRepository.UpdateStore(store);
 
-            return Ok(FromModel(store));
+            return Ok(ToApiModel(store));
         }
-        private EmailSettingsData FromModel(Data.StoreData data)
-        => (data.GetStoreBlob().EmailSettings ?? new()).ToData();
 
         private IActionResult StoreNotFound()
         {

BTCPayServer/Services/Mails/EmailSettings.cs

@@ -10,32 +10,11 @@ using Newtonsoft.Json;
 
 namespace BTCPayServer.Services.Mails
 {
+    /// <summary>
+    /// Email Settings gets saved to database, and is used by both the server and store settings
+    /// </summary>
     public class EmailSettings
     {
-#nullable enable
-        public static EmailSettings FromData(EmailSettingsData data, string? existingPassword)
-        => new EmailSettings()
-        {
-            Server = data.Server,
-            Port = data.Port,
-            Login = data.Login,
-            // Retaining the password if it exists and was not provided in request
-            Password = string.IsNullOrEmpty(data.Password) ? existingPassword : data.Password,
-            From = data.From,
-            DisableCertificateCheck = data.DisableCertificateCheck
-        };
-        public EmailSettingsData ToData() => ToData<EmailSettingsData>();
-        public T ToData<T>() where T : EmailSettingsData, new()
-        => new T()
-        {
-            Server = Server,
-            Port = Port,
-            Login = Login,
-            PasswordSet = !string.IsNullOrEmpty(Password),
-            From = From,
-            DisableCertificateCheck = DisableCertificateCheck
-        };
-#nullable restore
         public string Server { get; set; }
         public int? Port { get; set; }
         public string Login { get; set; }
@@ -48,11 +27,40 @@ namespace BTCPayServer.Services.Mails
             get => !DisableCertificateCheck;
             set { DisableCertificateCheck = !value; }
         }
+        
+        // Conversion functions for mapping to and from the client EmailSettingsData model
+#nullable enable
+        public static EmailSettings FromData(EmailSettingsData data, string? existingPassword)
+            => new()
+            {
+                Server = data.Server,
+                Port = data.Port,
+                Login = data.Login,
+                // Retaining the password if it exists and was not provided in request
+                Password = string.IsNullOrEmpty(data.Password) ? existingPassword : data.Password,
+                From = data.From,
+                DisableCertificateCheck = data.DisableCertificateCheck
+            };
+        
+        public T ToData<T>() where T : EmailSettingsData, new()
+            => new T()
+            {
+                Server = Server,
+                Port = Port,
+                Login = Login,
+                PasswordSet = !string.IsNullOrEmpty(Password),
+                From = From,
+                DisableCertificateCheck = DisableCertificateCheck
+            };
+#nullable restore
+        
+        
+        // 
         public bool IsComplete()
         {
             return MailboxAddressValidator.IsMailboxAddress(From)
                 && !string.IsNullOrWhiteSpace(Server)
-                && Port is int;
+                && Port != null;
         }
 
         public void Validate(string prefixKey, ModelStateDictionary modelState)

BTCPayServer/ValidationExtensions.cs

@@ -1,14 +0,0 @@
-#nullable enable
-using BTCPayServer.Client.Models;
-
-namespace BTCPayServer
-{
-    public static class ValidationExtensions
-    {
-        public static void Validate(this EmailSettingsData request, Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary modelState)
-        {
-            if (!string.IsNullOrEmpty(request.From) && !MailboxAddressValidator.IsMailboxAddress(request.From))
-                modelState.AddModelError(nameof(request.From), "Invalid email address");
-        }
-    }
-}

BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json

@@ -221,7 +221,7 @@
                     },
                     "disableCertificateCheck": {
                         "type": "boolean",
-                        "description": "Use SSL for SMTP connection",
+                        "description": "Disable TLS certificate security checks",
                         "example": false
                     }
                 }