Delivery enhancements

[MattMofDoom]

@nblumhardt This is the next feature set for whenever you get around to review - mail host fallback (comma-delimited list of mail hosts) and using DNS to deliver emails directly.

It also corrects deficiencies from #79 .. delivery results are now always logged with appropriate properties for monitoring and alerting usages.

I had left the test cases in place for actually sending emails (which I'd used to validate the mail host and DNS delivery changes), but these are now removed after I merged with the upstream conflicts.

Cheers,

Matt

[MattMofDoom]

@nblumhardt I've merged the changes for delivery enhancements with the upstream changes from #80 and #75 to address conflicts 😊

[MattMofDoom]

Addressed conflicts with #82 ... Lazy SmtpOptions was already in this PR so not a big shift, and just adjusted it to match the new constructor and switch from Server to Host, with the additions for delivery enhancements.

[MattMofDoom]

Easy enough to merge the change for SMTP over TCP 465 with the rest of the handling for SecureSocketOptions in this PR. This essentially means that all enumerations from SecureSocketOptions are available within Email+, however SecureSocketOptions.Auto is only a last ditch setting if EnableSsl somehow contrived to be null.

[MattMofDoom]

@nblumhardt I mentioned this in a self review on this PR, but just a note on MailKit (and technically SmtpClient) behaviour.

If we use the UseSsl Boolean in MailKit, the SecureSocketOptions behaviour is not quite what's mentioned elsewhere. UseSsl = false actually translates to SecureSocketOptions.None, which I only know for certain because I have a mail host for test that has a certificate that offers TLS but causes certificate validation errors with default .NET app cert validation, and I'm using MailKit in other libraries with the simply UseSsl Boolean.

Hence to make this consistent with MailKit and offer more control over the behaviour, I added a Boolean option UseTlsWhenAvailable which kicks in if EnableSsl = false.

This paid off when merging your changes from #86, because it meant that it was a small tweak to this PR to be allow for the TCP 465 behaviour.

Ideally the correct default would be EnableSsl = false and UseTlsWhenAvailable = true, but it doesn't seem possible to set this with SeqApp settings. That said, the existence of the two options means that the app also now defaults to the same behaviour as EnableSsl = false in the pre-Mailkit version, but we have finer grained control.

[MattMofDoom]

Referencing my last comment -

Hence to make this consistent with MailKit and offer more control over the behaviour, I added a Boolean option UseTlsWhenAvailable which kicks in if EnableSsl = false.

Ideally the correct default would be EnableSsl = false and UseTlsWhenAvailable = true, but it doesn't seem possible to set this with SeqApp settings. That said, the existence of the two options means that the app also now defaults to the same behaviour as EnableSsl = false in the pre-Mailkit version, but we have finer grained control.

I've now spent some time reviewing this against various scenarios, and it does appear that this is required. The current Nuget release can break a number of scenarios, even when the remote certificate has been imported. This additional option at least makes it equivalent to the old SmtpClient behaviour, which would disable TLS altogether if EnableSsl was false. This is the only option for the scenarios I've encountered, if importing the cert to trusted root isn't possible - excepting the previous withdrawn PR to allow disabling certificate validation, which we agreed was not ideal.

[MattMofDoom]

Hopefully this will close #89 - see comments there.

Additional updates - updated MailKit to v2.15.0, and updated Newtonsoft.Json to the maximum tolerable version (v12.0.3).

[MattMofDoom]

Hi @liammclennan and @nblumhardt 😊

I've merged #93 into the PR. This PR already handled this case via SmtpOptions.GetSocketOptions via explicit evaluation of the new enum for #89, and I've re-checked that, so I've redirected the merged code back there!

Cheers,

Matt

[MattMofDoom]

@liammclennan @nblumhardt For your interest ... I ported improvements from this PR into my Lurgle.Alerting library, to allow multiple mail hosts, DNS delivery, and the TLS socket options to be configured. This offered further opportunity to test the functionality, and I'm overall very happy. https://mattmofdoom.com/lurglealerting-v131-multiple-mail-hosts-dns-delivery-and-fine-grained-tls-options/

[MattMofDoom]

Hi @nblumhardt @liammclennan @KodrAus

Just noting that there are a few issues that this PR and its descendant #83 would resolve - #89 and #94 for this PR, and #65 for the #83 PR.

I had taken onboard Nick's comments in #89 and implemented the appropriate enum setting to allow MailKit SecureSocketOptions to be fully controlled without multiple bools, so this also had pre-emptively addressed #93 while ensuring migration from the old EnableSsl setting could be sustained.

Understand of course that you've been busy with 2021.3 and its awesomeness. Apologies for besting a pest - just thought a heads up is worthwhile.

Cheers,

Matt

[MattMofDoom]

Updated dependencies

[MattMofDoom]

App comprising the enhancements from #81 and #83 is currently published to Nuget as https://www.nuget.org/packages/Seq.App.EmailPlus-Enhanced while this is pending review/merge.

This is working well in a production environment. More information on the features was published at https://mattmofdoom.com/seqappemailplus-enhanced-available/

[mike-cochrane]

I've been using Seq.App.EmailPlus-Enhanced and it works great, thanks @MattMofDoom.