Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for SMTP OAuth authentication through easier IEmailSenderClient implementation #17484

Merged
merged 10 commits into from
Nov 19, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
using Umbraco.Cms.Infrastructure.HostedServices;
using Umbraco.Cms.Infrastructure.Install;
using Umbraco.Cms.Infrastructure.Mail;
using Umbraco.Cms.Infrastructure.Mail.Interfaces;
using Umbraco.Cms.Infrastructure.Manifest;
using Umbraco.Cms.Infrastructure.Migrations;
using Umbraco.Cms.Infrastructure.Migrations.Install;
Expand Down Expand Up @@ -172,14 +173,18 @@

builder.Services.AddSingleton<IContentLastChanceFinder, ContentFinderByConfigured404>();

builder.Services.AddTransient<IEmailSenderClient, BasicSmtpEmailSenderClient>();

// replace
builder.Services.AddSingleton<IEmailSender, EmailSender>(
services => new EmailSender(
services.GetRequiredService<ILogger<EmailSender>>(),
services.GetRequiredService<IOptionsMonitor<GlobalSettings>>(),
services.GetRequiredService<IEventAggregator>(),
services.GetRequiredService<IEmailSenderClient>(),
services.GetService<INotificationHandler<SendEmailNotification>>(),
services.GetService<INotificationAsyncHandler<SendEmailNotification>>()));

Check warning on line 187 in src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (contrib)

❌ Getting worse: Large Method

AddCoreInitialServices increases from 111 to 113 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
builder.Services.AddTransient<IUserInviteSender, EmailUserInviteSender>();
builder.Services.AddTransient<IUserForgotPasswordSender, EmailUserForgotPasswordSender>();

Expand Down
50 changes: 50 additions & 0 deletions src/Umbraco.Infrastructure/Mail/BasicSmtpEmailSenderClient.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using System.Net.Mail;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Models.Email;
using Umbraco.Cms.Infrastructure.Extensions;
using Umbraco.Cms.Infrastructure.Mail.Interfaces;
using SecureSocketOptions = MailKit.Security.SecureSocketOptions;
using SmtpClient = MailKit.Net.Smtp.SmtpClient;

namespace Umbraco.Cms.Infrastructure.Mail
{
/// <summary>
/// A basic SMTP email sender client using MailKits SMTP client.
/// </summary>
public class BasicSmtpEmailSenderClient : IEmailSenderClient
{
private readonly GlobalSettings _globalSettings;
public BasicSmtpEmailSenderClient(IOptionsMonitor<GlobalSettings> globalSettings)
{
_globalSettings = globalSettings.CurrentValue;
}

public async Task SendAsync(EmailMessage message)
{
using var client = new SmtpClient();

await client.ConnectAsync(
_globalSettings.Smtp!.Host,
_globalSettings.Smtp.Port,
(SecureSocketOptions)(int)_globalSettings.Smtp.SecureSocketOptions);

if (!string.IsNullOrWhiteSpace(_globalSettings.Smtp.Username) &&
!string.IsNullOrWhiteSpace(_globalSettings.Smtp.Password))
{
await client.AuthenticateAsync(_globalSettings.Smtp.Username, _globalSettings.Smtp.Password);
}

var mimeMessage = message.ToMimeMessage(_globalSettings.Smtp!.From);

if (_globalSettings.Smtp.DeliveryMethod == SmtpDeliveryMethod.Network)
{
await client.SendAsync(mimeMessage);
}
else
{
client.Send(mimeMessage);
}
}
}
}
55 changes: 27 additions & 28 deletions src/Umbraco.Infrastructure/Mail/EmailSender.cs
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.

using System.Net.Mail;
using MailKit.Net.Smtp;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using MimeKit;
using MimeKit.IO;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Mail;
using Umbraco.Cms.Core.Models.Email;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Infrastructure.Extensions;
using SecureSocketOptions = MailKit.Security.SecureSocketOptions;
using SmtpClient = MailKit.Net.Smtp.SmtpClient;
using Umbraco.Cms.Infrastructure.Mail.Interfaces;

namespace Umbraco.Cms.Infrastructure.Mail;

Expand All @@ -28,15 +28,18 @@
private readonly ILogger<EmailSender> _logger;
private readonly bool _notificationHandlerRegistered;
private GlobalSettings _globalSettings;
private readonly IEmailSenderClient _emailSenderClient;

[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
public EmailSender(
ILogger<EmailSender> logger,
IOptionsMonitor<GlobalSettings> globalSettings,
IEventAggregator eventAggregator)
: this(logger, globalSettings, eventAggregator, null, null)
: this(logger, globalSettings, eventAggregator,null, null)
{
}

[Obsolete("Please use the non-obsolete constructor. Will be removed in V17.")]
public EmailSender(
ILogger<EmailSender> logger,
IOptionsMonitor<GlobalSettings> globalSettings,
Expand All @@ -48,6 +51,24 @@
_eventAggregator = eventAggregator;
_globalSettings = globalSettings.CurrentValue;
_notificationHandlerRegistered = handler1 is not null || handler2 is not null;
_emailSenderClient = StaticServiceProvider.Instance.GetRequiredService<IEmailSenderClient>();
globalSettings.OnChange(x => _globalSettings = x);
}

[ActivatorUtilitiesConstructor]
public EmailSender(
ILogger<EmailSender> logger,
IOptionsMonitor<GlobalSettings> globalSettings,
IEventAggregator eventAggregator,
IEmailSenderClient emailSenderClient,
INotificationHandler<SendEmailNotification>? handler1,
INotificationAsyncHandler<SendEmailNotification>? handler2)
{
_logger = logger;
_eventAggregator = eventAggregator;
_globalSettings = globalSettings.CurrentValue;
_notificationHandlerRegistered = handler1 is not null || handler2 is not null;
_emailSenderClient = emailSenderClient;

Check warning on line 71 in src/Umbraco.Infrastructure/Mail/EmailSender.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (contrib)

❌ New issue: Constructor Over-Injection

EmailSender has 6 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
globalSettings.OnChange(x => _globalSettings = x);
}

Expand Down Expand Up @@ -152,29 +173,7 @@
while (true);
}

using var client = new SmtpClient();

await client.ConnectAsync(
_globalSettings.Smtp!.Host,
_globalSettings.Smtp.Port,
(SecureSocketOptions)(int)_globalSettings.Smtp.SecureSocketOptions);

if (!string.IsNullOrWhiteSpace(_globalSettings.Smtp.Username) &&
!string.IsNullOrWhiteSpace(_globalSettings.Smtp.Password))
{
await client.AuthenticateAsync(_globalSettings.Smtp.Username, _globalSettings.Smtp.Password);
}

var mailMessage = message.ToMimeMessage(_globalSettings.Smtp.From);
if (_globalSettings.Smtp.DeliveryMethod == SmtpDeliveryMethod.Network)
{
await client.SendAsync(mailMessage);
}
else
{
client.Send(mailMessage);
}

await client.DisconnectAsync(true);
await _emailSenderClient.SendAsync(message);

Check notice on line 176 in src/Umbraco.Infrastructure/Mail/EmailSender.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (contrib)

✅ Getting better: Complex Method

SendAsyncInternal decreases in cyclomatic complexity from 16 to 13, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

}
17 changes: 17 additions & 0 deletions src/Umbraco.Infrastructure/Mail/Interfaces/IEmailSenderClient.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using Umbraco.Cms.Core.Models.Email;

namespace Umbraco.Cms.Infrastructure.Mail.Interfaces
{
/// <summary>
/// Client for sending an email from a MimeMessage
/// </summary>
public interface IEmailSenderClient
{
/// <summary>
/// Sends the email message
/// </summary>
/// <param name="message"></param>
/// <returns></returns>
public Task SendAsync(EmailMessage message);
}
}
3 changes: 2 additions & 1 deletion tests/Umbraco.Tests.UnitTests/TestHelpers/TestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Infrastructure.Mail;
using Umbraco.Cms.Infrastructure.Mail.Interfaces;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Infrastructure.Persistence.Mappers;
using Umbraco.Cms.Persistence.SqlServer.Services;
Expand Down Expand Up @@ -80,7 +81,7 @@ public static class TestHelper

public static UriUtility UriUtility => s_testHelperInternal.UriUtility;

public static IEmailSender EmailSender { get; } = new EmailSender(new NullLogger<EmailSender>(), new TestOptionsMonitor<GlobalSettings>(new GlobalSettings()), Mock.Of<IEventAggregator>());
public static IEmailSender EmailSender { get; } = new EmailSender(new NullLogger<EmailSender>(), new TestOptionsMonitor<GlobalSettings>(new GlobalSettings()), Mock.Of<IEventAggregator>(), Mock.Of<IEmailSenderClient>(), null,null);

public static ITypeFinder GetTypeFinder() => s_testHelperInternal.GetTypeFinder();

Expand Down
Loading