From 16baceb784faf80ae352b9b718db6bbec425a790 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 24 Jan 2024 11:37:01 +0200 Subject: [PATCH] New: Option to disable Email encryption * New: Option to disable Email encryption (cherry picked from commit 7be5732a3a6679120b0f01bd1eb1207194f57f5e) * Fix possible NullRef in Email Encryption migration (cherry picked from commit 271266b10ac51ee6dd7a7024d346b631bd5397c2) --- .../Migration/039_email_encryptionFixture.cs | 151 ++++++++++++++++++ .../EmailSettingsValidatorFixture.cs | 111 +++++++++++++ .../Migration/039_email_encryption.cs | 50 ++++++ src/NzbDrone.Core/Localization/Core/en.json | 2 + .../Notifications/Email/Email.cs | 53 +++--- .../Notifications/Email/EmailSettings.cs | 11 +- 6 files changed, 347 insertions(+), 31 deletions(-) create mode 100644 src/NzbDrone.Core.Test/Datastore/Migration/039_email_encryptionFixture.cs create mode 100644 src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs create mode 100644 src/NzbDrone.Core/Datastore/Migration/039_email_encryption.cs diff --git a/src/NzbDrone.Core.Test/Datastore/Migration/039_email_encryptionFixture.cs b/src/NzbDrone.Core.Test/Datastore/Migration/039_email_encryptionFixture.cs new file mode 100644 index 000000000..513095162 --- /dev/null +++ b/src/NzbDrone.Core.Test/Datastore/Migration/039_email_encryptionFixture.cs @@ -0,0 +1,151 @@ +using System.Collections.Generic; +using System.Linq; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Common.Serializer; +using NzbDrone.Core.Datastore.Migration; +using NzbDrone.Core.Notifications.Email; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.Datastore.Migration +{ + [TestFixture] + public class email_encryptionFixture : MigrationTest + { + [Test] + public void should_convert_do_not_require_encryption_to_auto() + { + var db = WithMigrationTestDb(c => + { + c.Insert.IntoTable("Notifications").Row(new + { + OnGrab = true, + OnHealthIssue = true, + IncludeHealthWarnings = true, + Name = "Mail Prowlarr", + Implementation = "Email", + Tags = "[]", + Settings = new EmailSettings38 + { + Server = "smtp.gmail.com", + Port = 563, + To = new List { "dont@email.me" }, + RequireEncryption = false + }.ToJson(), + ConfigContract = "EmailSettings" + }); + }); + + var items = db.Query("SELECT * FROM \"Notifications\""); + + items.Should().HaveCount(1); + items.First().Implementation.Should().Be("Email"); + items.First().ConfigContract.Should().Be("EmailSettings"); + items.First().Settings.UseEncryption.Should().Be((int)EmailEncryptionType.Preferred); + } + + [Test] + public void should_convert_require_encryption_to_always() + { + var db = WithMigrationTestDb(c => + { + c.Insert.IntoTable("Notifications").Row(new + { + OnGrab = true, + OnHealthIssue = true, + IncludeHealthWarnings = true, + Name = "Mail Prowlarr", + Implementation = "Email", + Tags = "[]", + Settings = new EmailSettings38 + { + Server = "smtp.gmail.com", + Port = 563, + To = new List { "dont@email.me" }, + RequireEncryption = true + }.ToJson(), + ConfigContract = "EmailSettings" + }); + }); + + var items = db.Query("SELECT * FROM \"Notifications\""); + + items.Should().HaveCount(1); + items.First().Implementation.Should().Be("Email"); + items.First().ConfigContract.Should().Be("EmailSettings"); + items.First().Settings.UseEncryption.Should().Be((int)EmailEncryptionType.Always); + } + + [Test] + public void should_use_defaults_when_settings_are_empty() + { + var db = WithMigrationTestDb(c => + { + c.Insert.IntoTable("Notifications").Row(new + { + OnGrab = true, + OnHealthIssue = true, + IncludeHealthWarnings = true, + Name = "Mail Prowlarr", + Implementation = "Email", + Tags = "[]", + Settings = new { }.ToJson(), + ConfigContract = "EmailSettings" + }); + }); + + var items = db.Query("SELECT * FROM \"Notifications\""); + + items.Should().HaveCount(1); + items.First().Implementation.Should().Be("Email"); + items.First().ConfigContract.Should().Be("EmailSettings"); + items.First().Settings.UseEncryption.Should().Be((int)EmailEncryptionType.Preferred); + } + } + + public class NotificationDefinition39 + { + public int Id { get; set; } + public string Implementation { get; set; } + public string ConfigContract { get; set; } + public EmailSettings39 Settings { get; set; } + public string Name { get; set; } + public bool OnGrab { get; set; } + public bool OnHealthIssue { get; set; } + public bool OnHealthRestored { get; set; } + public bool OnApplicationUpdate { get; set; } + public bool SupportsOnGrab { get; set; } + public bool IncludeManualGrabs { get; set; } + public bool SupportsOnHealthIssue { get; set; } + public bool SupportsOnHealthRestored { get; set; } + public bool IncludeHealthWarnings { get; set; } + public bool SupportsOnApplicationUpdate { get; set; } + public List Tags { get; set; } + } + + public class EmailSettings38 + { + public string Server { get; set; } + public int Port { get; set; } + public bool RequireEncryption { get; set; } + public string Username { get; set; } + public string Password { get; set; } + public string From { get; set; } + public IEnumerable To { get; set; } + public IEnumerable Cc { get; set; } + public IEnumerable Bcc { get; set; } + } + + public class EmailSettings39 + { + public string Server { get; set; } + public int Port { get; set; } + public int UseEncryption { get; set; } + public string Username { get; set; } + public string Password { get; set; } + public string From { get; set; } + public IEnumerable To { get; set; } + public IEnumerable Cc { get; set; } + public IEnumerable Bcc { get; set; } + } +} diff --git a/src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs b/src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs new file mode 100644 index 000000000..309169c69 --- /dev/null +++ b/src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs @@ -0,0 +1,111 @@ +using System; +using FizzWare.NBuilder; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.Notifications.Email; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.NotificationTests.EmailTests +{ + [TestFixture] + public class EmailSettingsValidatorFixture : CoreTest + { + private EmailSettings _emailSettings; + private TestValidator _validator; + + [SetUp] + public void Setup() + { + _validator = new TestValidator + { + v => v.RuleFor(s => s).SetValidator(Subject) + }; + + _emailSettings = Builder.CreateNew() + .With(s => s.Server = "someserver") + .With(s => s.Port = 567) + .With(s => s.UseEncryption = (int)EmailEncryptionType.Always) + .With(s => s.From = "dont@email.me") + .With(s => s.To = new string[] { "dont@email.me" }) + .Build(); + } + + [Test] + public void should_be_valid_if_all_settings_valid() + { + _validator.Validate(_emailSettings).IsValid.Should().BeTrue(); + } + + [Test] + public void should_not_be_valid_if_port_is_out_of_range() + { + _emailSettings.Port = 900000; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [Test] + public void should_not_be_valid_if_server_is_empty() + { + _emailSettings.Server = ""; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [Test] + public void should_not_be_valid_if_from_is_empty() + { + _emailSettings.From = ""; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("prowlarr")] + [TestCase("email.me")] + [Ignore("Allowed coz some email servers allow arbitrary source, we probably need to support 'Name ' syntax")] + public void should_not_be_valid_if_from_is_invalid(string email) + { + _emailSettings.From = email; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("prowlarr")] + [TestCase("email.me")] + public void should_not_be_valid_if_to_is_invalid(string email) + { + _emailSettings.To = new string[] { email }; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("prowlarr")] + [TestCase("email.me")] + public void should_not_be_valid_if_cc_is_invalid(string email) + { + _emailSettings.Cc = new string[] { email }; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("prowlarr")] + [TestCase("email.me")] + public void should_not_be_valid_if_bcc_is_invalid(string email) + { + _emailSettings.Bcc = new string[] { email }; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [Test] + public void should_not_be_valid_if_to_bcc_cc_are_all_empty() + { + _emailSettings.To = Array.Empty(); + _emailSettings.Cc = Array.Empty(); + _emailSettings.Bcc = Array.Empty(); + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + } +} diff --git a/src/NzbDrone.Core/Datastore/Migration/039_email_encryption.cs b/src/NzbDrone.Core/Datastore/Migration/039_email_encryption.cs new file mode 100644 index 000000000..f275bbd70 --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/039_email_encryption.cs @@ -0,0 +1,50 @@ +using System.Collections.Generic; +using System.Data; +using Dapper; +using FluentMigrator; +using Newtonsoft.Json.Linq; +using NzbDrone.Common.Serializer; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(039)] + public class email_encryption : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + Execute.WithConnection(ChangeEncryption); + } + + private void ChangeEncryption(IDbConnection conn, IDbTransaction tran) + { + var updated = new List(); + using (var getEmailCmd = conn.CreateCommand()) + { + getEmailCmd.Transaction = tran; + getEmailCmd.CommandText = "SELECT \"Id\", \"Settings\" FROM \"Notifications\" WHERE \"Implementation\" = 'Email'"; + + using (var reader = getEmailCmd.ExecuteReader()) + { + while (reader.Read()) + { + var id = reader.GetInt32(0); + var settings = Json.Deserialize(reader.GetString(1)); + + settings["useEncryption"] = settings.Value("requireEncryption") ?? false ? 1 : 0; + settings["requireEncryption"] = null; + + updated.Add(new + { + Settings = settings.ToJson(), + Id = id + }); + } + } + } + + var updateSql = "UPDATE \"Notifications\" SET \"Settings\" = @Settings WHERE \"Id\" = @Id"; + conn.Execute(updateSql, updated, transaction: tran); + } + } +} diff --git a/src/NzbDrone.Core/Localization/Core/en.json b/src/NzbDrone.Core/Localization/Core/en.json index 40db67dc3..782510f03 100644 --- a/src/NzbDrone.Core/Localization/Core/en.json +++ b/src/NzbDrone.Core/Localization/Core/en.json @@ -366,6 +366,8 @@ "NotificationTriggers": "Notification Triggers", "NotificationTriggersHelpText": "Select which events should trigger this notification", "Notifications": "Notifications", + "NotificationsEmailSettingsUseEncryption": "Use Encryption", + "NotificationsEmailSettingsUseEncryptionHelpText": "Whether to prefer using encryption if configured on the server, to always use encryption via SSL (Port 465 only) or StartTLS (any other port) or to never use encryption", "OAuthPopupMessage": "Pop-ups are being blocked by your browser", "Ok": "Ok", "OnApplicationUpdate": "On Application Update", diff --git a/src/NzbDrone.Core/Notifications/Email/Email.cs b/src/NzbDrone.Core/Notifications/Email/Email.cs index 6f09eec92..45719839e 100644 --- a/src/NzbDrone.Core/Notifications/Email/Email.cs +++ b/src/NzbDrone.Core/Notifications/Email/Email.cs @@ -108,47 +108,42 @@ namespace NzbDrone.Core.Notifications.Email private void Send(MimeMessage email, EmailSettings settings) { - using (var client = new SmtpClient()) - { - client.Timeout = 10000; + using var client = new SmtpClient(); + client.Timeout = 10000; - var serverOption = SecureSocketOptions.Auto; + var useEncyption = (EmailEncryptionType)settings.UseEncryption; - if (settings.RequireEncryption) - { - if (settings.Port == 465) - { - serverOption = SecureSocketOptions.SslOnConnect; - } - else - { - serverOption = SecureSocketOptions.StartTls; - } - } + var serverOption = useEncyption switch + { + EmailEncryptionType.Always => settings.Port == 465 + ? SecureSocketOptions.SslOnConnect + : SecureSocketOptions.StartTls, + EmailEncryptionType.Never => SecureSocketOptions.None, + _ => SecureSocketOptions.Auto + }; - client.ServerCertificateValidationCallback = _certificateValidationService.ShouldByPassValidationError; + client.ServerCertificateValidationCallback = _certificateValidationService.ShouldByPassValidationError; - _logger.Debug("Connecting to mail server"); + _logger.Debug("Connecting to mail server"); - client.Connect(settings.Server, settings.Port, serverOption); + client.Connect(settings.Server, settings.Port, serverOption); - if (!string.IsNullOrWhiteSpace(settings.Username)) - { - _logger.Debug("Authenticating to mail server"); + if (!string.IsNullOrWhiteSpace(settings.Username)) + { + _logger.Debug("Authenticating to mail server"); - client.Authenticate(settings.Username, settings.Password); - } + client.Authenticate(settings.Username, settings.Password); + } - _logger.Debug("Sending to mail server"); + _logger.Debug("Sending to mail server"); - client.Send(email); + client.Send(email); - _logger.Debug("Sent to mail server, disconnecting"); + _logger.Debug("Sent to mail server, disconnecting"); - client.Disconnect(true); + client.Disconnect(true); - _logger.Debug("Disconnecting from mail server"); - } + _logger.Debug("Disconnecting from mail server"); } private MailboxAddress ParseAddress(string type, string address) diff --git a/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs b/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs index ad386b57a..2ef1d9d0c 100644 --- a/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs +++ b/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs @@ -44,8 +44,8 @@ namespace NzbDrone.Core.Notifications.Email [FieldDefinition(1, Label = "Port")] public int Port { get; set; } - [FieldDefinition(2, Label = "Require Encryption", HelpText = "Require SSL (Port 465 only) or StartTLS (any other port)", Type = FieldType.Checkbox)] - public bool RequireEncryption { get; set; } + [FieldDefinition(2, Label = "NotificationsEmailSettingsUseEncryption", HelpText = "NotificationsEmailSettingsUseEncryptionHelpText", Type = FieldType.Select, SelectOptions = typeof(EmailEncryptionType))] + public int UseEncryption { get; set; } [FieldDefinition(3, Label = "Username", HelpText = "Username", Type = FieldType.Textbox, Privacy = PrivacyLevel.UserName)] public string Username { get; set; } @@ -70,4 +70,11 @@ namespace NzbDrone.Core.Notifications.Email return new NzbDroneValidationResult(Validator.Validate(this)); } } + + public enum EmailEncryptionType + { + Preferred = 0, + Always = 1, + Never = 2 + } }