From 811c84a8450416959758239e21f5bd44b3a793c8 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sat, 6 Mar 2021 00:14:50 +0100 Subject: [PATCH] Multiple Email Address Fixes #854 Fixes #884 Fixes #951 Fixes #954 (cherry picked from commit a8b6f70be1860aa502795f0dd30299c87d54dbbe) --- .../EmailSettingsValidatorFixture.cs | 103 +++++++++++++ .../Migration/007_update_notifiarr.cs | 1 - .../Notifications/Email/Email.cs | 137 ++++++++++++++++-- .../Notifications/Email/EmailService.cs | 92 ------------ .../Notifications/Email/EmailSettings.cs | 30 ++-- 5 files changed, 251 insertions(+), 112 deletions(-) create mode 100644 src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs delete mode 100644 src/NzbDrone.Core/Notifications/Email/EmailService.cs 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..8f54b217b --- /dev/null +++ b/src/NzbDrone.Core.Test/NotificationTests/EmailTests/EmailSettingsValidatorFixture.cs @@ -0,0 +1,103 @@ +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.From = "readarr@readarr.com") + .With(s => s.To = new string[] { "readarr@readarr.com" }) + .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("readarr")] + [TestCase("readarr@readarr")] + [TestCase("readarr.com")] + public void should_not_be_valid_if_to_is_invalid(string email) + { + _emailSettings.To = new string[] { email }; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("readarr")] + [TestCase("readarr@readarr")] + [TestCase("readarr.com")] + public void should_not_be_valid_if_cc_is_invalid(string email) + { + _emailSettings.CC = new string[] { email }; + + _validator.Validate(_emailSettings).IsValid.Should().BeFalse(); + } + + [TestCase("readarr")] + [TestCase("readarr@readarr")] + [TestCase("readarr.com")] + 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/007_update_notifiarr.cs b/src/NzbDrone.Core/Datastore/Migration/007_update_notifiarr.cs index 198a3e5e5..a94067048 100644 --- a/src/NzbDrone.Core/Datastore/Migration/007_update_notifiarr.cs +++ b/src/NzbDrone.Core/Datastore/Migration/007_update_notifiarr.cs @@ -1,5 +1,4 @@ using FluentMigrator; -using Newtonsoft.Json.Linq; using NzbDrone.Core.Datastore.Migration.Framework; namespace NzbDrone.Core.Datastore.Migration diff --git a/src/NzbDrone.Core/Notifications/Email/Email.cs b/src/NzbDrone.Core/Notifications/Email/Email.cs index 96acc0779..1bd58b294 100644 --- a/src/NzbDrone.Core/Notifications/Email/Email.cs +++ b/src/NzbDrone.Core/Notifications/Email/Email.cs @@ -1,18 +1,24 @@ +using System; using System.Collections.Generic; +using System.Linq; using FluentValidation.Results; +using MailKit.Net.Smtp; +using MailKit.Security; +using MimeKit; +using NLog; using NzbDrone.Common.Extensions; namespace NzbDrone.Core.Notifications.Email { public class Email : NotificationBase { - private readonly IEmailService _emailService; + private readonly Logger _logger; public override string Name => "Email"; - public Email(IEmailService emailService) + public Email(Logger logger) { - _emailService = emailService; + _logger = logger; } public override string Link => null; @@ -21,7 +27,7 @@ namespace NzbDrone.Core.Notifications.Email { var body = $"{grabMessage.Message} sent to queue."; - _emailService.SendEmail(Settings, BOOK_GRABBED_TITLE_BRANDED, body); + SendEmail(Settings, BOOK_GRABBED_TITLE_BRANDED, body); } public override void OnReleaseImport(BookDownloadMessage message) @@ -30,31 +36,144 @@ namespace NzbDrone.Core.Notifications.Email var paths = Settings.AttachFiles ? message.BookFiles.SelectList(a => a.Path) : null; - _emailService.SendEmail(Settings, BOOK_DOWNLOADED_TITLE_BRANDED, body, false, paths); + SendEmail(Settings, BOOK_DOWNLOADED_TITLE_BRANDED, body, paths); } public override void OnHealthIssue(HealthCheck.HealthCheck message) { - _emailService.SendEmail(Settings, HEALTH_ISSUE_TITLE_BRANDED, message.Message); + SendEmail(Settings, HEALTH_ISSUE_TITLE_BRANDED, message.Message); } public override void OnDownloadFailure(DownloadFailedMessage message) { - _emailService.SendEmail(Settings, DOWNLOAD_FAILURE_TITLE_BRANDED, message.Message); + SendEmail(Settings, DOWNLOAD_FAILURE_TITLE_BRANDED, message.Message); } public override void OnImportFailure(BookDownloadMessage message) { - _emailService.SendEmail(Settings, IMPORT_FAILURE_TITLE_BRANDED, message.Message); + SendEmail(Settings, IMPORT_FAILURE_TITLE_BRANDED, message.Message); } public override ValidationResult Test() { var failures = new List(); - failures.AddIfNotNull(_emailService.Test(Settings)); + failures.AddIfNotNull(Test(Settings)); return new ValidationResult(failures); } + + public ValidationFailure Test(EmailSettings settings) + { + const string body = "Success! You have properly configured your email notification settings"; + + try + { + SendEmail(settings, "Readarr - Test Notification", body); + } + catch (Exception ex) + { + _logger.Error(ex, "Unable to send test email"); + return new ValidationFailure("Server", "Unable to send test email"); + } + + return null; + } + + private void SendEmail(EmailSettings settings, string subject, string body, bool htmlBody = false, List attachmentUrls = null) + { + var email = new MimeMessage(); + + email.From.Add(ParseAddress("From", settings.From)); + email.To.AddRange(settings.To.Select(x => ParseAddress("To", x))); + email.Cc.AddRange(settings.CC.Select(x => ParseAddress("CC", x))); + email.Bcc.AddRange(settings.Bcc.Select(x => ParseAddress("BCC", x))); + + email.Subject = subject; + email.Body = new TextPart(htmlBody ? "html" : "plain") + { + Text = body + }; + + _logger.Debug("Sending email Subject: {0}", subject); + + if (attachmentUrls != null) + { + foreach (var url in attachmentUrls) + { + email.Attachments.Add(new Attachment(url)); + } + } + + try + { + Send(email, settings); + _logger.Debug("Email sent. Subject: {0}", subject); + } + catch (Exception ex) + { + _logger.Error("Error sending email. Subject: {0}", email.Subject); + _logger.Debug(ex, ex.Message); + throw; + } + + _logger.Debug("Finished sending email. Subject: {0}", email.Subject); + } + + private void Send(MimeMessage email, EmailSettings settings) + { + using (var client = new SmtpClient()) + { + client.Timeout = 10000; + + var serverOption = SecureSocketOptions.Auto; + + if (settings.RequireEncryption) + { + if (settings.Port == 465) + { + serverOption = SecureSocketOptions.SslOnConnect; + } + else + { + serverOption = SecureSocketOptions.StartTls; + } + } + + _logger.Debug("Connecting to mail server"); + + client.Connect(settings.Server, settings.Port, serverOption); + + if (!string.IsNullOrWhiteSpace(settings.Username)) + { + _logger.Debug("Authenticating to mail server"); + + client.Authenticate(settings.Username, settings.Password); + } + + _logger.Debug("Sending to mail server"); + + client.Send(email); + + _logger.Debug("Sent to mail server, disconnecting"); + + client.Disconnect(true); + + _logger.Debug("Disconnecting from mail server"); + } + } + + private MailboxAddress ParseAddress(string type, string address) + { + try + { + return MailboxAddress.Parse(address); + } + catch (Exception ex) + { + _logger.Error(ex, "{0} email address '{1}' invalid", type, address); + throw; + } + } } } diff --git a/src/NzbDrone.Core/Notifications/Email/EmailService.cs b/src/NzbDrone.Core/Notifications/Email/EmailService.cs deleted file mode 100644 index efe26cbbe..000000000 --- a/src/NzbDrone.Core/Notifications/Email/EmailService.cs +++ /dev/null @@ -1,92 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Net.Mail; -using FluentValidation.Results; -using NLog; -using NzbDrone.Common.Http; - -namespace NzbDrone.Core.Notifications.Email -{ - public interface IEmailService - { - void SendEmail(EmailSettings settings, string subject, string body, bool htmlBody = false, List attachmentUrls = null); - ValidationFailure Test(EmailSettings settings); - } - - public class EmailService : IEmailService - { - private readonly Logger _logger; - - public EmailService(Logger logger) - { - _logger = logger; - } - - public void SendEmail(EmailSettings settings, string subject, string body, bool htmlBody = false, List attachmentUrls = null) - { - var email = new MailMessage(); - email.From = new MailAddress(settings.From); - - settings.To.ToList().ForEach(x => email.To.Add(x)); - settings.CC.ToList().ForEach(x => email.CC.Add(x)); - settings.Bcc.ToList().ForEach(x => email.Bcc.Add(x)); - - email.Subject = subject; - email.Body = body; - email.IsBodyHtml = htmlBody; - - if (attachmentUrls != null) - { - foreach (var url in attachmentUrls) - { - email.Attachments.Add(new Attachment(url)); - } - } - - BasicNetworkCredential credentials = null; - - if (!string.IsNullOrWhiteSpace(settings.Username)) - { - credentials = new BasicNetworkCredential(settings.Username, settings.Password); - } - - try - { - Send(email, settings.Server, settings.Port, settings.Ssl, credentials); - } - catch (Exception ex) - { - _logger.Error("Error sending email. Subject: {0}", email.Subject); - _logger.Debug(ex, ex.Message); - throw; - } - } - - private void Send(MailMessage email, string server, int port, bool ssl, BasicNetworkCredential credentials) - { - var smtp = new SmtpClient(server, port); - smtp.EnableSsl = ssl; - smtp.Credentials = credentials; - - smtp.Send(email); - } - - public ValidationFailure Test(EmailSettings settings) - { - const string body = "Success! You have properly configured your email notification settings"; - - try - { - SendEmail(settings, "Readarr - Test Notification", body); - } - catch (Exception ex) - { - _logger.Error(ex, "Unable to send test email"); - return new ValidationFailure("Server", "Unable to send test email"); - } - - return null; - } - } -} diff --git a/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs b/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs index 079467804..351934c0c 100644 --- a/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs +++ b/src/NzbDrone.Core/Notifications/Email/EmailSettings.cs @@ -1,4 +1,6 @@ +using System; using System.Collections.Generic; +using System.Linq; using FluentValidation; using NzbDrone.Core.Annotations; using NzbDrone.Core.ThingiProvider; @@ -13,7 +15,14 @@ namespace NzbDrone.Core.Notifications.Email RuleFor(c => c.Server).NotEmpty(); RuleFor(c => c.Port).InclusiveBetween(1, 65535); RuleFor(c => c.From).NotEmpty(); - RuleFor(c => c.To).NotEmpty(); + RuleForEach(c => c.To).EmailAddress(); + RuleForEach(c => c.CC).EmailAddress(); + RuleForEach(c => c.Bcc).EmailAddress(); + + // Only require one of three send fields to be set + RuleFor(c => c.To).NotEmpty().Unless(c => c.Bcc.Any() || c.CC.Any()); + RuleFor(c => c.CC).NotEmpty().Unless(c => c.To.Any() || c.Bcc.Any()); + RuleFor(c => c.Bcc).NotEmpty().Unless(c => c.To.Any() || c.CC.Any()); } } @@ -23,11 +32,12 @@ namespace NzbDrone.Core.Notifications.Email public EmailSettings() { - Port = 25; + Server = "smtp.gmail.com"; + Port = 587; - To = new string[] { }; - CC = new string[] { }; - Bcc = new string[] { }; + To = Array.Empty(); + CC = Array.Empty(); + Bcc = Array.Empty(); } [FieldDefinition(0, Label = "Server", HelpText = "Hostname or IP of Email server")] @@ -36,8 +46,8 @@ namespace NzbDrone.Core.Notifications.Email [FieldDefinition(1, Label = "Port")] public int Port { get; set; } - [FieldDefinition(2, Label = "SSL", Type = FieldType.Checkbox)] - public bool Ssl { 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(3, Label = "Username", Privacy = PrivacyLevel.UserName)] public string Username { get; set; } @@ -48,13 +58,13 @@ namespace NzbDrone.Core.Notifications.Email [FieldDefinition(5, Label = "From Address")] public string From { get; set; } - [FieldDefinition(6, Label = "Recipient Address(es)", HelpText = "Comma separated list of email recipients")] + [FieldDefinition(6, Label = "Recipient Address(es)", HelpText = "Comma seperated list of email recipients")] public IEnumerable To { get; set; } - [FieldDefinition(7, Label = "CC Address(es)", HelpText = "Comma separated list of email cc recipients", Advanced = true)] + [FieldDefinition(7, Label = "CC Address(es)", HelpText = "Comma seperated list of email cc recipients", Advanced = true)] public IEnumerable CC { get; set; } - [FieldDefinition(8, Label = "BCC Address(es)", HelpText = "Comma separated list of email bcc recipients", Advanced = true)] + [FieldDefinition(8, Label = "BCC Address(es)", HelpText = "Comma seperated list of email bcc recipients", Advanced = true)] public IEnumerable Bcc { get; set; } [FieldDefinition(9, Label = "Attach Books", HelpText = "Add books as an attachment on import", Type = FieldType.Checkbox)]