Migrate to MailKit from System.Net.Mail

[meetmandeep]

Closes #3974
Possibly Closes #3768 (Need someone to test with Office 365)

Summary

Migrated to MailKit from System.Net.Mail

Uses the new MailAttachment wrapper class instead of relying on System.Net.Mail

Deprecation & Backwards Compatible

Few methods have been marked as obsolte with scheduled removal in v11; these methods are using the System.Net.Mail Attachment. New methods have been introduced with MailAttachment parameter as alternatives. Backwards compatibility supported.

Missing Functionality

I couldn't find the equivalent functionality in MailKit for following:

mailMessage.SubjectEncoding = mailInfo.BodyEncoding;
mailMessage.BodyEncoding = mailInfo.BodyEncoding;

My understanding is that MailKit automatically encodes both Subject/Body as needed.

smtpClient.ServicePoint.MaxIdleTime = Host.SMTPMaxIdleTime;
smtpClient.ServicePoint.ConnectionLimit = Host.SMTPConnectionLimit;

I couldn't find any relevant properties in MailKit. We do have UI settings for these 2 items so need to make a decision on that also.

Testing

We really need to test this extensively since sending mail is a critical component of the platform.

I've tested using Mailtrap using the following test code for both new and backwards compatible functionality.

 try
                {
                    using (var sr = new StreamReader(this.Page.MapPath("Robots.txt")))
                    {
                        using (MemoryStream ms = new MemoryStream())
                        {
                            sr.BaseStream.CopyTo(ms);
                            sr.BaseStream.Seek(0, SeekOrigin.Begin);

                            Attachment attach1 = new Attachment(this.Page.MapPath("403-3.gif"));
                            Attachment attach2 = new Attachment(sr.BaseStream,"Robots.txt");

                            MailAttachment mattach1 = new MailAttachment(this.Page.MapPath("403-3.gif"));
                            MailAttachment mattach2 = new MailAttachment("Robots.txt", ms.ToArray());

                            List<Attachment> attachments = new List<Attachment>();
                            attachments.Add(attach1);
                            attachments.Add(attach2);

                            List<MailAttachment> mattachments = new List<MailAttachment>();
                            mattachments.Add(mattach1);
                            mattachments.Add(mattach2);

                            Mail.SendEmail("user1@test.com", "user2@test.com", "Test Email 1", "Test Body 1");
                            Mail.SendEmail("user1@test.com", "sender@test.com", user2@test.com", "Test Email 2", "Test Body 2");
                            Mail.SendEmail("user1@test.com", "sender@test.com", "user2@test.com", "Test Email 3", "Test Body 3", attachments);
                            Mail.SendEmail("user1@test.com", "sender@test.com", "user2@test.com", "Test Email 4", "Test Body 4", mattachments);
                        }
                    }
                }
                catch (Exception ex)
                {
                    string mes = ex.Message;
                }

[SkyeHoefling]

@meetmandeep thanks for contributing this PR 🎉

I was wondering if we could update this PR to add Dependency Injection support for emails instead of just swapping out the implementation in the existing DNN Provider Framework. This will make it easier for developers to use their own implementation in the future (which is going to be pretty rare). I think it would be a big win for the platform to have this critical piece of the puzzle with Dependency Injection Support.

This is only a recommendation as a non-maintainer, please only do this if you agree and the maintainers agree

What I'm thinking and only do this if you agree and the maintainers agree

Update the current EmailProvider to be an IEmailService that is registered with the Dependency Injection container. We can then define a contract of methods that we expect all email services to implement. All interfaces would be included in a namespace in the Abstractions project such as DotNetNuke.Abstraction.Mail.

Once this is done we would use your implementation as the registered implementation, but it allows a framework for developers to swap out for their own implementation as long as they implement the contract

[mitchelsellers]

@ahoefling I'm all for DI support, but I think it would need to be done at a different area than this.

The provider model is our extension point for configuration allowing an implementation to be switched out. (Authentication Providers, Permission Providers, etc.). I would see DI usage here being for the consumers to get away from Mail.Mail static members, which would be a different effort, although a good one.

[mitchelsellers]

Upon further review of this PR, this introduces a breaking change that would be in violation of our release rules, by removing a supported authentication method for users. (This requires notice and inclusion in a major release.)

The preferred approach would be to leave the existing Core provider as-is, and introduce MailKit as a new provider that could be selected by end-users. This should be a small change in theory.

@meetmandeep feel free to reach out to me directly tomorrow on this.

[valadas]

We need to rebase this to resolve the conflicts, @bdukes if you decide to handle this I would like to learn how to do it properly, ping me up.

[bdukes]

@dnnsoftware/approvers this PR is ready for review now. Does anyone have an Office365 SMTP server to test with? There were notes about testing with a null and empty from name, as well as a comma-separated to email list, @meetmandeep did you test those scenarios, or are we still needing verification? Thanks!

[mitchelsellers]

Can we quickly add the license information for these new packages to the /licenses folder to make sure we follow that pattern

[bdukes]

I've added license files, let me know if you need any adjustments @mitchelsellers

[mitchelsellers]

This looks good, I'll try to test with an O365 account later today as well

[valadas]

Can we add a merge config file so that we have both providers available in the web config and users can easily just change the defaultProvider value ?

<mail defaultProvider="CoreMailProvider">
      <providers>
        <clear />
        <add name="CoreMailProvider" type="DotNetNuke.Services.Mail.CoreMailProvider, DotNetNuke" hideCoreSettings="False" />
+        <add name="MailKitMailProvider" type="DotNetNuke.Services.Mail.MailKitMailProvider, DotNetNuke" hideCoreSettings="False" />
      </providers>
    </mail>

[bdukes]

I've added the config change

[mitchelsellers]

Thanks @bdukes

[valadas]

Looks good to me