From be8ecb823eec46fab1a2401abb469cd8218a327b Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 27 Feb 2025 16:50:32 +0900 Subject: [PATCH 1/7] Refactor email endpoints --- .../BTCPayServerClient.ServerEmail.cs | 4 +- .../Models/EmailSettingsData.cs | 13 +-- BTCPayServer.Tests/GreenfieldAPITests.cs | 16 +-- .../GreenfieldServerEmailController.cs | 58 ++++------ .../GreenfieldStoreEmailController.cs | 38 ++----- .../GreenField/LocalBTCPayServerClient.cs | 2 +- BTCPayServer/Services/Mails/EmailSettings.cs | 39 ++++++- BTCPayServer/ValidationExtensions.cs | 14 +++ .../v1/swagger.template.serveremail.json | 57 ++++------ .../v1/swagger.template.stores-email.json | 103 +++++++++++------- 10 files changed, 185 insertions(+), 159 deletions(-) create mode 100644 BTCPayServer/ValidationExtensions.cs diff --git a/BTCPayServer.Client/BTCPayServerClient.ServerEmail.cs b/BTCPayServer.Client/BTCPayServerClient.ServerEmail.cs index 4e0ec3bf3..c6fc01c0c 100644 --- a/BTCPayServer.Client/BTCPayServerClient.ServerEmail.cs +++ b/BTCPayServer.Client/BTCPayServerClient.ServerEmail.cs @@ -13,8 +13,8 @@ public partial class BTCPayServerClient return await SendHttpRequest("api/v1/server/email", null, HttpMethod.Get, token); } - public virtual async Task UpdateServerEmailSettings(ServerEmailSettingsData request, CancellationToken token = default) + public virtual async Task UpdateServerEmailSettings(ServerEmailSettingsData request, CancellationToken token = default) { - return await SendHttpRequest("api/v1/server/email", request, HttpMethod.Put, token); + return await SendHttpRequest("api/v1/server/email", request, HttpMethod.Put, token); } } diff --git a/BTCPayServer.Client/Models/EmailSettingsData.cs b/BTCPayServer.Client/Models/EmailSettingsData.cs index 2bd82bc45..b4cf8ec23 100644 --- a/BTCPayServer.Client/Models/EmailSettingsData.cs +++ b/BTCPayServer.Client/Models/EmailSettingsData.cs @@ -1,4 +1,6 @@ +using System.Collections.Generic; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; namespace BTCPayServer.Client.Models; @@ -7,14 +9,11 @@ public class EmailSettingsData public string Server { get; set; } public int? Port { get; set; } public string Login { get; set; } + [JsonProperty(DefaultValueHandling = DefaultValueHandling.Ignore)] public string Password { get; set; } + public bool? PasswordSet { get; set; } public string From { get; set; } public bool DisableCertificateCheck { get; set; } - - [JsonIgnore] - public bool EnabledCertificateCheck - { - get => !DisableCertificateCheck; - set { DisableCertificateCheck = !value; } - } + [JsonExtensionData] + public IDictionary AdditionalData { get; set; } = new Dictionary(); } diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 5e7508fb4..d2bc35c93 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -4104,24 +4104,23 @@ namespace BTCPayServer.Tests var admin = tester.NewAccount(); await admin.GrantAccessAsync(true); var adminClient = await admin.CreateClient(Policies.Unrestricted); - var data = new EmailSettingsData + var data = new ServerEmailSettingsData { From = "admin@admin.com", Login = "admin@admin.com", Password = "admin@admin.com", Port = 1234, Server = "admin.com", - }; - var serverEmailSettings = new ServerEmailSettingsData - { EnableStoresToUseServerEmailSettings = false }; - await adminClient.UpdateServerEmailSettings(serverEmailSettings); + var actualUpdated = await adminClient.UpdateServerEmailSettings(data); - var s = await adminClient.GetServerEmailSettings(); + var actualUpdated2 = await adminClient.GetServerEmailSettings(); // email password is masked and not returned from the server once set - serverEmailSettings.Password = null; - Assert.Equal(JsonConvert.SerializeObject(s), JsonConvert.SerializeObject(serverEmailSettings)); + data.Password = null; + data.PasswordSet = true; + Assert.Equal(JsonConvert.SerializeObject(actualUpdated2), JsonConvert.SerializeObject(data)); + Assert.Equal(JsonConvert.SerializeObject(actualUpdated2), JsonConvert.SerializeObject(actualUpdated)); await AssertValidationError(new[] { nameof(EmailSettingsData.From) }, async () => await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData { @@ -4157,6 +4156,7 @@ namespace BTCPayServer.Tests var s = await adminClient.GetStoreEmailSettings(admin.StoreId); // email password is masked and not returned from the server once set data.Password = null; + data.PasswordSet = true; Assert.Equal(JsonConvert.SerializeObject(s), JsonConvert.SerializeObject(data)); await AssertValidationError(new[] { nameof(EmailSettingsData.From) }, async () => await adminClient.UpdateStoreEmailSettings(admin.StoreId, diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs index 6e4e64466..a0621cdb5 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs @@ -1,4 +1,5 @@ #nullable enable +using System; using System.Threading.Tasks; using BTCPayServer.Abstractions.Constants; using BTCPayServer.Abstractions.Extensions; @@ -10,6 +11,7 @@ using BTCPayServer.Services.Mails; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Mvc; +using Newtonsoft.Json.Linq; namespace BTCPayServer.Controllers.GreenField { @@ -34,56 +36,36 @@ namespace BTCPayServer.Controllers.GreenField public async Task ServerEmailSettings() { var email = await _emailSenderFactory.GetSettings() ?? new EmailSettings(); - var model = new ServerEmailSettingsData - { - EnableStoresToUseServerEmailSettings = !_policiesSettings.DisableStoresToUseServerEmailSettings, - From = email.From, - Server = email.Server, - Port = email.Port, - Login = email.Login, - DisableCertificateCheck = email.DisableCertificateCheck, - // Password is not returned - Password = null - }; - return Ok(model); + return Ok(FromModel(email)); + } + + private ServerEmailSettingsData FromModel(EmailSettings email) + { + var data = email.ToData(); + data.EnableStoresToUseServerEmailSettings = !_policiesSettings.DisableStoresToUseServerEmailSettings; + return data; } [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpPut("~/api/v1/server/email")] public async Task ServerEmailSettings(ServerEmailSettingsData request) { + request.Validate(ModelState); + if (!ModelState.IsValid) + return this.CreateValidationError(ModelState); + if (_policiesSettings.DisableStoresToUseServerEmailSettings == request.EnableStoresToUseServerEmailSettings) { _policiesSettings.DisableStoresToUseServerEmailSettings = !request.EnableStoresToUseServerEmailSettings; await _settingsRepository.UpdateSetting(_policiesSettings); } - - // save - if (request.From is not null && !MailboxAddressValidator.IsMailboxAddress(request.From)) - { - request.AddModelError(e => e.From, - "Invalid email address", this); - return this.CreateValidationError(ModelState); - } - - var oldSettings = await _emailSenderFactory.GetSettings() ?? new EmailSettings(); - // retaining the password if it exists and was not provided in request - if (string.IsNullOrEmpty(request.Password) && - !string.IsNullOrEmpty(oldSettings?.Password)) - request.Password = oldSettings.Password; - + + var settings = await _emailSenderFactory.GetSettings(); + settings = EmailSettings.FromData(request, settings?.Password); + // important to save as EmailSettings otherwise it won't be able to be fetched - await _settingsRepository.UpdateSetting(new EmailSettings - { - Server = request.Server, - Port = request.Port, - Login = request.Login, - Password = request.Password, - From = request.From, - DisableCertificateCheck = request.DisableCertificateCheck - }); - - return Ok(true); + await _settingsRepository.UpdateSetting(settings); + return Ok(FromModel(settings)); } } } diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs index f350a86b6..c2be0bec2 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs @@ -61,43 +61,25 @@ namespace BTCPayServer.Controllers.GreenField [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpPut("~/api/v1/stores/{storeId}/email")] - public async Task UpdateStoreEmailSettings(string storeId, EmailSettings request) + public async Task UpdateStoreEmailSettings(string storeId, EmailSettingsData request) { var store = HttpContext.GetStoreData(); - if (store == null) - { - return StoreNotFound(); - } - - if (!string.IsNullOrEmpty(request.From) && !MailboxAddressValidator.IsMailboxAddress(request.From)) - { - request.AddModelError(e => e.From, - "Invalid email address", this); - return this.CreateValidationError(ModelState); - } + request ??= new(); + request.Validate(this.ModelState); + if (!ModelState.IsValid) + return this.CreateValidationError(ModelState); var blob = store.GetStoreBlob(); - - // retaining the password if it exists and was not provided in request - if (string.IsNullOrEmpty(request.Password) && blob.EmailSettings?.Password != null) - request.Password = blob.EmailSettings?.Password; - - blob.EmailSettings = request; + var settings = EmailSettings.FromData(request, blob.EmailSettings?.Password); + blob.EmailSettings = settings; if (store.SetStoreBlob(blob)) - { await _storeRepository.UpdateStore(store); - } return Ok(FromModel(store)); } - private EmailSettings FromModel(Data.StoreData data) - { - var emailSettings = data.GetStoreBlob().EmailSettings; - if (emailSettings == null) - return new EmailSettings(); - emailSettings.Password = null; - return emailSettings; - } + private EmailSettingsData FromModel(Data.StoreData data) + => (data.GetStoreBlob().EmailSettings ?? new()).ToData(); + private IActionResult StoreNotFound() { return this.CreateAPIError(404, "store-not-found", "The store was not found"); diff --git a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs index 95e2c30b2..56cdcf2b0 100644 --- a/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs +++ b/BTCPayServer/Controllers/GreenField/LocalBTCPayServerClient.cs @@ -977,7 +977,7 @@ namespace BTCPayServer.Controllers.Greenfield { return GetFromActionResult( await GetController().UpdateStoreEmailSettings(storeId, - JObject.FromObject(request).ToObject())); + JObject.FromObject(request).ToObject())); } public override async Task GetUsers(CancellationToken token = default) diff --git a/BTCPayServer/Services/Mails/EmailSettings.cs b/BTCPayServer/Services/Mails/EmailSettings.cs index ed62a4d4d..c961d2b8f 100644 --- a/BTCPayServer/Services/Mails/EmailSettings.cs +++ b/BTCPayServer/Services/Mails/EmailSettings.cs @@ -6,11 +6,48 @@ using BTCPayServer.Validation; using MailKit.Net.Smtp; using Microsoft.AspNetCore.Mvc.ModelBinding; using MimeKit; +using Newtonsoft.Json; namespace BTCPayServer.Services.Mails { - public class EmailSettings : EmailSettingsData + 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(); + public T ToData() 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; } + public string Password { get; set; } + public string From { get; set; } + public bool DisableCertificateCheck { get; set; } + [JsonIgnore] + public bool EnabledCertificateCheck + { + get => !DisableCertificateCheck; + set { DisableCertificateCheck = !value; } + } public bool IsComplete() { return MailboxAddressValidator.IsMailboxAddress(From) diff --git a/BTCPayServer/ValidationExtensions.cs b/BTCPayServer/ValidationExtensions.cs new file mode 100644 index 000000000..1ed105edd --- /dev/null +++ b/BTCPayServer/ValidationExtensions.cs @@ -0,0 +1,14 @@ +#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"); + } + } +} diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.serveremail.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.serveremail.json index a4d774cbb..924f62500 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.serveremail.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.serveremail.json @@ -12,7 +12,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ServerEmailSettingsData" + "$ref": "#/components/schemas/GetServerEmailSettings" } } } @@ -38,14 +38,21 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ServerEmailSettingsData" + "$ref": "#/components/schemas/UpdateServerEmailSettings" } } } }, "responses": { "200": { - "description": "Email settings updated successfully" + "description": "Server email settings", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetServerEmailSettings" + } + } + } }, "400": { "description": "Invalid request or email format", @@ -72,9 +79,9 @@ }, "components": { "schemas": { - "ServerEmailSettingsData": { + "UpdateServerEmailSettings": { "allOf": [ - { "$ref": "#/components/schemas/EmailSettings" }, + { "$ref": "#/components/schemas/UpdateEmailSettings" }, { "type": "object", "properties": { @@ -86,35 +93,19 @@ } ] }, - "EmailSettings": { - "type": "object", - "properties": { - "from": { - "type": "string", - "description": "The sender email address" - }, - "server": { - "type": "string", - "description": "SMTP server host" - }, - "port": { - "type": "integer", - "description": "SMTP server port" - }, - "login": { - "type": "string", - "description": "SMTP username" - }, - "password": { - "type": "string", - "description": "SMTP password, masked in responses and retained if not updated", - "nullable": true - }, - "disableCertificateCheck": { - "type": "boolean", - "description": "Use SSL for SMTP connection" + "GetServerEmailSettings": { + "allOf": [ + { "$ref": "#/components/schemas/GetEmailSettings" }, + { + "type": "object", + "properties": { + "enableStoresToUseServerEmailSettings": { + "type": "boolean", + "description": "Indicates if stores can use server email settings" + } + } } - } + ] } } }, diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json index fdfc42dd1..732d7ebe4 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json +++ b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json @@ -19,7 +19,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/EmailSettingsData" + "$ref": "#/components/schemas/GetEmailSettings" } } } @@ -57,7 +57,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/EmailSettingsData" + "$ref": "#/components/schemas/UpdateEmailSettings" } } }, @@ -70,7 +70,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/EmailSettingsData" + "$ref": "#/components/schemas/GetEmailSettings" } } } @@ -117,7 +117,22 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/EmailData" + "type": "object", + "additionalProperties": false, + "properties": { + "email": { + "type": "string", + "description": "Email of the recipient" + }, + "subject": { + "type": "string", + "description": "Subject of the email" + }, + "body": { + "type": "string", + "description": "Body of the email to send as plain text." + } + } } } }, @@ -151,57 +166,63 @@ }, "components": { "schemas": { - "EmailData": { - "type": "object", - "additionalProperties": false, - "properties": { - "email": { - "type": "string", - "description": "Email of the recipient" - }, - "subject": { - "type": "string", - "description": "Subject of the email" - }, - "body": { - "type": "string", - "description": "Body of the email to send as plain text." + "UpdateEmailSettings": { + "allOf": [ + { "$ref": "#/components/schemas/EmailSettingsBase" }, + { + "type": "object", + "properties": { + "password": { + "type": "string", + "description": "SMTP password. Keep null or empty to not update it.", + "nullable": true, + "example": "MyS3cr3t" + } + } } - } + ] }, - "EmailSettingsData": { + "GetEmailSettings": { + "allOf": [ + { "$ref": "#/components/schemas/EmailSettingsBase" }, + { + "type": "object", + "properties": { + "passwordSet": { + "type": "boolean", + "description": "`true` if the password has been set." + } + } + } + ] + }, + "EmailSettingsBase": { "type": "object", - "additionalProperties": false, "properties": { + "from": { + "type": "string", + "description": "The sender email address", + "example": "sender@gmail.com" + }, "server": { "type": "string", - "description": "Smtp server host" + "description": "SMTP server host", + "example": "smtp.gmail.com" }, "port": { - "type": "number", - "description": "Smtp server port" + "type": "integer", + "description": "SMTP server port", + "example": 587 }, "login": { "type": "string", - "description": "Smtp server username" - }, - "password": { - "type": "string", - "description": "Smtp server password" - }, - "from": { - "type": "string", - "format": "email", - "description": "Email to send from" - }, - "fromDisplay": { - "type": "string", - "description": "The name of the sender" + "description": "SMTP username", + "example": "John.Smith" }, "disableCertificateCheck": { "type": "boolean", - "default": false, - "description": "Disable TLS certificate security checks" + "description": "Use SSL for SMTP connection", + "example": false } } } From 8094800cfc331a5f1e1f13dd2237fb45b7e1851e Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:11:22 -0600 Subject: [PATCH 2/7] IsMailboxAddress already checks for null, simplifying code --- .../Controllers/GreenField/GreenfieldUsersController.cs | 8 +++----- BTCPayServer/Controllers/UIServerController.cs | 2 +- BTCPayServer/Controllers/UIStoresController.Email.cs | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldUsersController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldUsersController.cs index 94f9ad474..37cb9ed25 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldUsersController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldUsersController.cs @@ -306,14 +306,12 @@ namespace BTCPayServer.Controllers.Greenfield { if (request.Email is null) ModelState.AddModelError(nameof(request.Email), "Email is missing"); - if (!string.IsNullOrEmpty(request.Email) && !MailboxAddressValidator.IsMailboxAddress(request.Email)) - { + if (!MailboxAddressValidator.IsMailboxAddress(request.Email)) ModelState.AddModelError(nameof(request.Email), "Invalid email"); - } + if (!ModelState.IsValid) - { return this.CreateValidationError(ModelState); - } + if (User.Identity is null) throw new JsonHttpException(this.StatusCode(401)); var anyAdmin = (await _userManager.GetUsersInRoleAsync(Roles.ServerAdmin)).Any(); diff --git a/BTCPayServer/Controllers/UIServerController.cs b/BTCPayServer/Controllers/UIServerController.cs index 32c3deb00..4f35a74e7 100644 --- a/BTCPayServer/Controllers/UIServerController.cs +++ b/BTCPayServer/Controllers/UIServerController.cs @@ -1257,7 +1257,7 @@ namespace BTCPayServer.Controllers } // save - if (model.Settings.From is not null && !MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) + if (!MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) { ModelState.AddModelError("Settings.From", StringLocalizer["Invalid email"]); return View(model); diff --git a/BTCPayServer/Controllers/UIStoresController.Email.cs b/BTCPayServer/Controllers/UIStoresController.Email.cs index 27d649a5f..10d9253ba 100644 --- a/BTCPayServer/Controllers/UIStoresController.Email.cs +++ b/BTCPayServer/Controllers/UIStoresController.Email.cs @@ -209,7 +209,7 @@ public partial class UIStoresController if (model.IsCustomSMTP) { model.Settings.Validate("Settings.", ModelState); - if (model.Settings.From is not null && !MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) + if (!MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) { ModelState.AddModelError("Settings.From", StringLocalizer["Invalid email"]); } From 746a8cf6e1f66983122322ceec3f14e292944839 Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:22:06 -0600 Subject: [PATCH 3/7] Getting fast tests to pass --- BTCPayServer.Tests/ThirdPartyTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/BTCPayServer.Tests/ThirdPartyTests.cs b/BTCPayServer.Tests/ThirdPartyTests.cs index 948073a98..0114a96cb 100644 --- a/BTCPayServer.Tests/ThirdPartyTests.cs +++ b/BTCPayServer.Tests/ThirdPartyTests.cs @@ -285,7 +285,8 @@ namespace BTCPayServer.Tests "https://support.bitpay.com", "https://www.coingecko.com", // unhappy service "https://www.wasabiwallet.io", // Banning US, CI unhappy - "https://fullynoded.app" // Sometimes DNS doesn't work + "https://fullynoded.app", // Sometimes DNS doesn't work + "https://hrf.org" // Started returning Forbidden }; foreach (var match in regex.Matches(text).OfType()) From 18852af241307c3e44279b4e20ac5ab2652c5be5 Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:22:19 -0600 Subject: [PATCH 4/7] Refactoring and commenting --- BTCPayServer.Tests/GreenfieldAPITests.cs | 9 ++- .../GreenfieldServerEmailController.cs | 28 ++++----- .../GreenfieldStoreEmailController.cs | 24 +++++--- BTCPayServer/Services/Mails/EmailSettings.cs | 58 +++++++++++-------- BTCPayServer/ValidationExtensions.cs | 14 ----- .../v1/swagger.template.stores-email.json | 2 +- 6 files changed, 70 insertions(+), 65 deletions(-) delete mode 100644 BTCPayServer/ValidationExtensions.cs diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index d2bc35c93..ba28e14ef 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/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 { diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs index a0621cdb5..eb8ce55a5 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs @@ -30,29 +30,31 @@ namespace BTCPayServer.Controllers.GreenField _policiesSettings = policiesSettings; _settingsRepository = settingsRepository; } - - [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] - [HttpGet("~/api/v1/server/email")] - public async Task ServerEmailSettings() - { - var email = await _emailSenderFactory.GetSettings() ?? new EmailSettings(); - return Ok(FromModel(email)); - } - - private ServerEmailSettingsData FromModel(EmailSettings email) + + private ServerEmailSettingsData ToApiModel(EmailSettings email) { var data = email.ToData(); data.EnableStoresToUseServerEmailSettings = !_policiesSettings.DisableStoresToUseServerEmailSettings; return data; } + [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] + [HttpGet("~/api/v1/server/email")] + public async Task 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 ServerEmailSettings(ServerEmailSettingsData request) { - request.Validate(ModelState); - if (!ModelState.IsValid) + if (!MailboxAddressValidator.IsMailboxAddress(request.From)) + { + 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)); } } } diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs index c2be0bec2..2ca96fd26 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs +++ b/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(); + } + [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 UpdateStoreEmailSettings(string storeId, EmailSettingsData request) { - var store = HttpContext.GetStoreData(); - request ??= new(); - request.Validate(this.ModelState); - if (!ModelState.IsValid) - return this.CreateValidationError(ModelState); + if (!MailboxAddressValidator.IsMailboxAddress(request.From)) + { + ModelState.AddModelError(nameof(request.From), "Invalid email address"); + 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() { diff --git a/BTCPayServer/Services/Mails/EmailSettings.cs b/BTCPayServer/Services/Mails/EmailSettings.cs index c961d2b8f..24d7f0dea 100644 --- a/BTCPayServer/Services/Mails/EmailSettings.cs +++ b/BTCPayServer/Services/Mails/EmailSettings.cs @@ -10,32 +10,11 @@ using Newtonsoft.Json; namespace BTCPayServer.Services.Mails { + /// + /// Email Settings gets saved to database, and is used by both the server and store settings + /// 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(); - public T ToData() 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() 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) diff --git a/BTCPayServer/ValidationExtensions.cs b/BTCPayServer/ValidationExtensions.cs deleted file mode 100644 index 1ed105edd..000000000 --- a/BTCPayServer/ValidationExtensions.cs +++ /dev/null @@ -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"); - } - } -} diff --git a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json b/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json index 732d7ebe4..081183575 100644 --- a/BTCPayServer/wwwroot/swagger/v1/swagger.template.stores-email.json +++ b/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 } } From 1da2cedffd1c802af4c197200e85f154bf2f1457 Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 22:14:06 -0600 Subject: [PATCH 5/7] Reverting changes, validate email only if it was provided --- .../GreenField/GreenfieldServerEmailController.cs | 6 +++--- .../GreenField/GreenfieldStoreEmailController.cs | 6 +++--- BTCPayServer/Controllers/UIServerController.cs | 4 ++-- BTCPayServer/Controllers/UIStoresController.Email.cs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs index eb8ce55a5..c1fd39144 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs @@ -50,11 +50,11 @@ namespace BTCPayServer.Controllers.GreenField [HttpPut("~/api/v1/server/email")] public async Task ServerEmailSettings(ServerEmailSettingsData request) { - if (!MailboxAddressValidator.IsMailboxAddress(request.From)) - { + if (!string.IsNullOrWhiteSpace(request.From) && !MailboxAddressValidator.IsMailboxAddress(request.From)) ModelState.AddModelError(nameof(request.From), "Invalid email address"); + + if (!ModelState.IsValid) return this.CreateValidationError(ModelState); - } if (_policiesSettings.DisableStoresToUseServerEmailSettings == request.EnableStoresToUseServerEmailSettings) { diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs index 2ca96fd26..409e0a8dd 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs @@ -70,11 +70,11 @@ namespace BTCPayServer.Controllers.GreenField [HttpPut("~/api/v1/stores/{storeId}/email")] public async Task UpdateStoreEmailSettings(string storeId, EmailSettingsData request) { - if (!MailboxAddressValidator.IsMailboxAddress(request.From)) - { + if (!string.IsNullOrWhiteSpace(request.From) && !MailboxAddressValidator.IsMailboxAddress(request.From)) ModelState.AddModelError(nameof(request.From), "Invalid email address"); + + if (!ModelState.IsValid) return this.CreateValidationError(ModelState); - } var store = HttpContext.GetStoreData(); var blob = store.GetStoreBlob(); diff --git a/BTCPayServer/Controllers/UIServerController.cs b/BTCPayServer/Controllers/UIServerController.cs index 4f35a74e7..3c206ca94 100644 --- a/BTCPayServer/Controllers/UIServerController.cs +++ b/BTCPayServer/Controllers/UIServerController.cs @@ -1256,8 +1256,8 @@ namespace BTCPayServer.Controllers return RedirectToAction(nameof(Emails)); } - // save - if (!MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) + // save if user provided valid email; this will also clear settings if no model.Settings.From + if (model.Settings.From is not null && !MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) { ModelState.AddModelError("Settings.From", StringLocalizer["Invalid email"]); return View(model); diff --git a/BTCPayServer/Controllers/UIStoresController.Email.cs b/BTCPayServer/Controllers/UIStoresController.Email.cs index 10d9253ba..27d649a5f 100644 --- a/BTCPayServer/Controllers/UIStoresController.Email.cs +++ b/BTCPayServer/Controllers/UIStoresController.Email.cs @@ -209,7 +209,7 @@ public partial class UIStoresController if (model.IsCustomSMTP) { model.Settings.Validate("Settings.", ModelState); - if (!MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) + if (model.Settings.From is not null && !MailboxAddressValidator.IsMailboxAddress(model.Settings.From)) { ModelState.AddModelError("Settings.From", StringLocalizer["Invalid email"]); } From 70c2fc0a9ec9076b80c4d4ce7c92acf2918483fb Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 22:14:27 -0600 Subject: [PATCH 6/7] Fixing bug on password not being cleared --- BTCPayServer.Tests/GreenfieldAPITests.cs | 19 ++++++++++++++++--- BTCPayServer/Services/Mails/EmailSettings.cs | 5 +++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index ba28e14ef..94a393f43 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -4104,6 +4104,9 @@ namespace BTCPayServer.Tests var admin = tester.NewAccount(); await admin.GrantAccessAsync(true); var adminClient = await admin.CreateClient(Policies.Unrestricted); + // validate that clear email settings will not throw an error + await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData()); + var data = new ServerEmailSettingsData { From = "admin@admin.com", @@ -4129,6 +4132,11 @@ namespace BTCPayServer.Tests { From = "invalid" })); + + // check that clear server email settings works + await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData()); + var clearedSettings = await adminClient.GetServerEmailSettings(); + Assert.Equal(JsonConvert.SerializeObject(new EmailSettingsData { PasswordSet = false }), JsonConvert.SerializeObject(clearedSettings)); // NOTE: This email test fails silently in EmailSender.cs#31, can't test, but leaving for the future as reminder //await adminClient.SendEmail(admin.StoreId, @@ -4144,8 +4152,8 @@ namespace BTCPayServer.Tests var admin = tester.NewAccount(); await admin.GrantAccessAsync(true); var adminClient = await admin.CreateClient(Policies.Unrestricted); - await adminClient.UpdateStoreEmailSettings(admin.StoreId, - new EmailSettingsData()); + // validate that clear email settings will not throw an error + await adminClient.UpdateStoreEmailSettings(admin.StoreId, new EmailSettingsData()); var data = new EmailSettingsData { @@ -4164,7 +4172,12 @@ namespace BTCPayServer.Tests await AssertValidationError(new[] { nameof(EmailSettingsData.From) }, async () => await adminClient.UpdateStoreEmailSettings(admin.StoreId, new EmailSettingsData { From = "invalid" })); - + + // clear store email settings + await adminClient.UpdateStoreEmailSettings(admin.StoreId, new EmailSettingsData()); + var clearedSettings = await adminClient.GetStoreEmailSettings(admin.StoreId); + Assert.Equal(JsonConvert.SerializeObject(new EmailSettingsData { PasswordSet = false }), JsonConvert.SerializeObject(clearedSettings)); + await adminClient.SendEmail(admin.StoreId, new SendEmailRequest { Body = "lol", Subject = "subj", Email = "to@example.org" }); } diff --git a/BTCPayServer/Services/Mails/EmailSettings.cs b/BTCPayServer/Services/Mails/EmailSettings.cs index 24d7f0dea..47246f349 100644 --- a/BTCPayServer/Services/Mails/EmailSettings.cs +++ b/BTCPayServer/Services/Mails/EmailSettings.cs @@ -36,8 +36,9 @@ namespace BTCPayServer.Services.Mails 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, + // If login was provided but password was not, use the existing password from database + Password = !string.IsNullOrEmpty(data.Login) && string.IsNullOrEmpty(data.Password) ? + existingPassword : data.Password, From = data.From, DisableCertificateCheck = data.DisableCertificateCheck }; From 2d21e3b26bebcd9bce4a92ee0618af375c8b8b84 Mon Sep 17 00:00:00 2001 From: rockstardev <5191402+rockstardev@users.noreply.github.com> Date: Wed, 5 Mar 2025 22:34:59 -0600 Subject: [PATCH 7/7] Final fixes --- BTCPayServer.Tests/GreenfieldAPITests.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 94a393f43..55f23740d 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -4132,15 +4132,15 @@ namespace BTCPayServer.Tests { From = "invalid" })); - - // check that clear server email settings works - await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData()); - var clearedSettings = await adminClient.GetServerEmailSettings(); - Assert.Equal(JsonConvert.SerializeObject(new EmailSettingsData { PasswordSet = false }), JsonConvert.SerializeObject(clearedSettings)); // NOTE: This email test fails silently in EmailSender.cs#31, can't test, but leaving for the future as reminder //await adminClient.SendEmail(admin.StoreId, // new SendEmailRequest { Body = "lol", Subject = "subj", Email = "to@example.org" }); + + // check that clear server email settings works + await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData()); + var clearedSettings = await adminClient.GetServerEmailSettings(); + Assert.Equal(JsonConvert.SerializeObject(new ServerEmailSettingsData { PasswordSet = false }), JsonConvert.SerializeObject(clearedSettings)); } [Fact(Timeout = 60 * 2 * 1000)] @@ -4173,13 +4173,14 @@ namespace BTCPayServer.Tests async () => await adminClient.UpdateStoreEmailSettings(admin.StoreId, new EmailSettingsData { From = "invalid" })); + // send test email + await adminClient.SendEmail(admin.StoreId, + new SendEmailRequest { Body = "lol", Subject = "subj", Email = "to@example.org" }); + // clear store email settings await adminClient.UpdateStoreEmailSettings(admin.StoreId, new EmailSettingsData()); var clearedSettings = await adminClient.GetStoreEmailSettings(admin.StoreId); Assert.Equal(JsonConvert.SerializeObject(new EmailSettingsData { PasswordSet = false }), JsonConvert.SerializeObject(clearedSettings)); - - await adminClient.SendEmail(admin.StoreId, - new SendEmailRequest { Body = "lol", Subject = "subj", Email = "to@example.org" }); } [Fact(Timeout = 60 * 2 * 1000)]