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.

Sign up for GitHub

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

Jump to bottom

Fix SmtpService to validate email address #12253

Merged
merged 5 commits into from
Sep 1, 2022
Merged

Fix SmtpService to validate email address #12253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
381f675
Fix #12252
MikeAlhayek Aug 23, 2022
0fc229a
Update AdminController
MikeAlhayek Aug 23, 2022
ce9e051
Undo the AdminController change
MikeAlhayek Aug 23, 2022
3ac3a18
Fix ReplayTo settings
MikeAlhayek Aug 23, 2022
99f6170
Add generic error message
MikeAlhayek Aug 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/OrchardCore.Modules/OrchardCore.Email/ViewModels/SmtpSettingsViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Localization;

namespace OrchardCore.Email.ViewModels
{
public class SmtpSettingsViewModel
public class SmtpSettingsViewModel : IValidatableObject
{
[Required(AllowEmptyStrings = false)]
public string To { get; set; }
Expand All @@ -18,5 +22,16 @@ public class SmtpSettingsViewModel
public string Subject { get; set; }

public string Body { get; set; }

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
var emailAddressValidator = validationContext.GetService<IEmailAddressValidator>();
var S = validationContext.GetService<IStringLocalizer<SmtpSettingsViewModel>>();

if (!String.IsNullOrWhiteSpace(Sender) && !emailAddressValidator.Validate(Sender))
{
yield return new ValidationResult(S["Invalid Email."], new[] { nameof(Sender) });
}
}
}
}
151 changes: 97 additions & 54 deletions src/OrchardCore/OrchardCore.Email.Core/Services/SmtpService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
Expand Down Expand Up @@ -71,7 +72,14 @@ public async Task<SmtpResult> SendAsync(MailMessage message)
message.From = senderAddress;
}

var mimeMessage = FromMailMessage(message);
var errors = new List<LocalizedString>();

var mimeMessage = FromMailMessage(message, errors);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if errors should be part of FromMailMessage method? may be just create IsValid method separate from FromMailMessage method

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ns8482e this is how the class was designed. All we did here is create more checks to ensure we report proper error instead of throwing exception


if (errors.Count > 0)
{
return SmtpResult.Failed(errors.ToArray());
}

if (mimeMessage.From.Count == 0 && mimeMessage.Cc.Count == 0 && mimeMessage.Bcc.Count == 0)
{
Expand All @@ -94,15 +102,15 @@ public async Task<SmtpResult> SendAsync(MailMessage message)
}
catch (Exception ex)
{
result = SmtpResult.Failed(S["An error occurred while sending an email: '{0}'", ex.Message]);
result = SmtpResult.Failed(S["An error occurred while sending an email: '{0}'", ex.Message]);
}

result.Response = response;

return result;
}

private MimeMessage FromMailMessage(MailMessage message)
private MimeMessage FromMailMessage(MailMessage message, IList<LocalizedString> errors)
{
var submitterAddress = String.IsNullOrWhiteSpace(message.Sender)
? _options.DefaultSender
Expand All @@ -112,42 +120,78 @@ private MimeMessage FromMailMessage(MailMessage message)

if (!String.IsNullOrEmpty(submitterAddress))
{
mimeMessage.Sender = MailboxAddress.Parse(submitterAddress);
if (MailboxAddress.TryParse(submitterAddress, out var mailBox))
{
mimeMessage.Sender = mailBox;

}
else
{
errors.Add(S["Invalid email address: '{0}'", submitterAddress]);
}
}

if (!string.IsNullOrWhiteSpace(message.From))
if (!String.IsNullOrWhiteSpace(message.From))
{
foreach (var address in message.From.Split(EmailsSeparator, StringSplitOptions.RemoveEmptyEntries))
foreach (var address in message.From.Split(EmailsSeparator, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
{
mimeMessage.From.Add(MailboxAddress.Parse(address));
if (MailboxAddress.TryParse(address, out var mailBox))
{
mimeMessage.From.Add(mailBox);
}
else
{
errors.Add(S["Invalid email address: '{0}'", address]);
}
}
}

if (!string.IsNullOrWhiteSpace(message.To))
if (!String.IsNullOrWhiteSpace(message.To))
{
foreach (var address in message.To.Split(EmailsSeparator, StringSplitOptions.RemoveEmptyEntries))
foreach (var address in message.To.Split(EmailsSeparator, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
{
mimeMessage.To.Add(MailboxAddress.Parse(address));
if (MailboxAddress.TryParse(address, out var mailBox))
{
mimeMessage.To.Add(mailBox);
}
else
{
errors.Add(S["Invalid email address: '{0}'", address]);
}
}
}

if (!string.IsNullOrWhiteSpace(message.Cc))
if (!String.IsNullOrWhiteSpace(message.Cc))
{
foreach (var address in message.Cc.Split(EmailsSeparator, StringSplitOptions.RemoveEmptyEntries))
foreach (var address in message.Cc.Split(EmailsSeparator, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
{
mimeMessage.Cc.Add(MailboxAddress.Parse(address));
if (MailboxAddress.TryParse(address, out var mailBox))
{
mimeMessage.Cc.Add(mailBox);
}
else
{
errors.Add(S["Invalid email address: '{0}'", address]);
}
}
}

if (!string.IsNullOrWhiteSpace(message.Bcc))
if (!String.IsNullOrWhiteSpace(message.Bcc))
{
foreach (var address in message.Bcc.Split(EmailsSeparator, StringSplitOptions.RemoveEmptyEntries))
foreach (var address in message.Bcc.Split(EmailsSeparator, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
{
mimeMessage.Bcc.Add(MailboxAddress.Parse(address));
if (MailboxAddress.TryParse(address, out var mailBox))
{
mimeMessage.Bcc.Add(mailBox);
}
else
{
errors.Add(S["Invalid email address: '{0}'", address]);
}
}
}

if (string.IsNullOrWhiteSpace(message.ReplyTo))
if (String.IsNullOrWhiteSpace(message.ReplyTo))
{
foreach (var address in mimeMessage.From)
{
Expand All @@ -158,7 +202,14 @@ private MimeMessage FromMailMessage(MailMessage message)
{
foreach (var address in message.ReplyTo.Split(EmailsSeparator, StringSplitOptions.RemoveEmptyEntries))
{
mimeMessage.ReplyTo.Add(MailboxAddress.Parse(address));
if (MailboxAddress.TryParse(address, out var mailBox))
{
mimeMessage.ReplyTo.Add(mailBox);
}
else
{
errors.Add(S["Invalid email address: '{0}'", address]);
}
}
}

Expand Down Expand Up @@ -223,57 +274,49 @@ private async Task<string> SendOnlineMessageAsync(MimeMessage message)

if (!_options.AutoSelectEncryption)
{
switch (_options.EncryptionMethod)
secureSocketOptions = _options.EncryptionMethod switch
{
case SmtpEncryptionMethod.None:
secureSocketOptions = SecureSocketOptions.None;
break;
case SmtpEncryptionMethod.SSLTLS:
secureSocketOptions = SecureSocketOptions.SslOnConnect;
break;
case SmtpEncryptionMethod.STARTTLS:
secureSocketOptions = SecureSocketOptions.StartTls;
break;
default:
break;
}
SmtpEncryptionMethod.None => SecureSocketOptions.None,
SmtpEncryptionMethod.SSLTLS => SecureSocketOptions.SslOnConnect,
SmtpEncryptionMethod.STARTTLS => SecureSocketOptions.StartTls,
_ => SecureSocketOptions.Auto,
};
}

using (var client = new SmtpClient())
{
client.ServerCertificateValidationCallback = CertificateValidationCallback;
using var client = new SmtpClient();

await OnMessageSendingAsync(client, message);
client.ServerCertificateValidationCallback = CertificateValidationCallback;

await client.ConnectAsync(_options.Host, _options.Port, secureSocketOptions);
await OnMessageSendingAsync(client, message);

if (_options.RequireCredentials)
await client.ConnectAsync(_options.Host, _options.Port, secureSocketOptions);

if (_options.RequireCredentials)
{
if (_options.UseDefaultCredentials)
{
if (_options.UseDefaultCredentials)
{
// There's no notion of 'UseDefaultCredentials' in MailKit, so empty credentials is passed in
await client.AuthenticateAsync(String.Empty, String.Empty);
}
else if (!String.IsNullOrWhiteSpace(_options.UserName))
{
await client.AuthenticateAsync(_options.UserName, _options.Password);
}
// There's no notion of 'UseDefaultCredentials' in MailKit, so empty credentials is passed in
await client.AuthenticateAsync(String.Empty, String.Empty);
}

if (!String.IsNullOrEmpty( _options.ProxyHost))
else if (!String.IsNullOrWhiteSpace(_options.UserName))
{
client.ProxyClient = new Socks5Client(_options.ProxyHost, _options.ProxyPort);
await client.AuthenticateAsync(_options.UserName, _options.Password);
}
}

if (!String.IsNullOrEmpty(_options.ProxyHost))
{
client.ProxyClient = new Socks5Client(_options.ProxyHost, _options.ProxyPort);
}

var response = await client.SendAsync(message);
var response = await client.SendAsync(message);

await client.DisconnectAsync(true);
await client.DisconnectAsync(true);

return response;
}
return response;
}

private async Task SendOfflineMessage(MimeMessage message, string pickupDirectory)
private static async Task SendOfflineMessage(MimeMessage message, string pickupDirectory)
{
var mailPath = Path.Combine(pickupDirectory, Guid.NewGuid().ToString() + EmailExtension);
await message.WriteToAsync(mailPath, CancellationToken.None);
Expand Down