From be8ecb823eec46fab1a2401abb469cd8218a327b Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Thu, 27 Feb 2025 16:50:32 +0900 Subject: [PATCH] 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 } } }