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..55f23740d 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -4104,24 +4104,29 @@ namespace BTCPayServer.Tests var admin = tester.NewAccount(); await admin.GrantAccessAsync(true); var adminClient = await admin.CreateClient(Policies.Unrestricted); - var data = new EmailSettingsData + // validate that clear email settings will not throw an error + await adminClient.UpdateServerEmailSettings(new ServerEmailSettingsData()); + + 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 finalEmailSettings = 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(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 { @@ -4131,6 +4136,11 @@ namespace BTCPayServer.Tests // 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)] @@ -4142,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 { @@ -4157,13 +4167,20 @@ 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, 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)); } [Fact(Timeout = 60 * 2 * 1000)] 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()) diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldServerEmailController.cs index 6e4e64466..c1fd39144 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 { @@ -28,62 +30,44 @@ namespace BTCPayServer.Controllers.GreenField _policiesSettings = policiesSettings; _settingsRepository = settingsRepository; } + + 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(); - 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(ToApiModel(email)); } [Authorize(Policy = Policies.CanModifyServerSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [HttpPut("~/api/v1/server/email")] public async Task ServerEmailSettings(ServerEmailSettingsData request) { + 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) { _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(ToApiModel(settings)); } } } diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs index f350a86b6..409e0a8dd 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreEmailController.cs @@ -51,53 +51,41 @@ 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, 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); + 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(); - - // 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; + return Ok(ToApiModel(store)); } + private IActionResult StoreNotFound() { return this.CreateAPIError(404, "store-not-found", "The store was not found"); 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/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/Controllers/UIServerController.cs b/BTCPayServer/Controllers/UIServerController.cs index 32c3deb00..3c206ca94 100644 --- a/BTCPayServer/Controllers/UIServerController.cs +++ b/BTCPayServer/Controllers/UIServerController.cs @@ -1256,7 +1256,7 @@ namespace BTCPayServer.Controllers return RedirectToAction(nameof(Emails)); } - // save + // 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"]); diff --git a/BTCPayServer/Services/Mails/EmailSettings.cs b/BTCPayServer/Services/Mails/EmailSettings.cs index ed62a4d4d..47246f349 100644 --- a/BTCPayServer/Services/Mails/EmailSettings.cs +++ b/BTCPayServer/Services/Mails/EmailSettings.cs @@ -6,16 +6,62 @@ 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 + /// + /// Email Settings gets saved to database, and is used by both the server and store settings + /// + public class EmailSettings { + 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; } + } + + // 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, + // 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 + }; + + 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/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..081183575 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": "Disable TLS certificate security checks", + "example": false } } }