From 88f6d09ef937c094208e10f97f0989631a3764d1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 14 May 2020 22:14:00 +1000 Subject: [PATCH 01/10] Enables the boot failed exception logic --- .../ApplicationBuilderExtensions.cs | 2 ++ .../UmbracoWebServiceCollectionExtensions.cs | 4 --- .../Middleware/BootFailedMiddleware.cs | 34 +++++++++++++++++++ .../Runtime/AspNetCoreComposer.cs | 8 +++++ src/Umbraco.Web/UmbracoInjectedModule.cs | 19 ----------- 5 files changed, 44 insertions(+), 23 deletions(-) create mode 100644 src/Umbraco.Web.Common/Middleware/BootFailedMiddleware.cs diff --git a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs index 228f133ab0..a43094a8f9 100644 --- a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs @@ -73,6 +73,8 @@ namespace Umbraco.Extensions { if (app == null) throw new ArgumentNullException(nameof(app)); + app.UseMiddleware(); + if (!app.UmbracoCanBoot()) return app; app.UseMiddleware(); diff --git a/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs index 559730bdc3..a3e9b901dc 100644 --- a/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/UmbracoWebServiceCollectionExtensions.cs @@ -34,10 +34,6 @@ namespace Umbraco.Extensions /// public static IServiceCollection AddUmbracoWebComponents(this IServiceCollection services) { - services.AddTransient(); - services.AddTransient(); - - services.TryAddSingleton(); services.ConfigureOptions(); services.TryAddEnumerable(ServiceDescriptor.Transient()); diff --git a/src/Umbraco.Web.Common/Middleware/BootFailedMiddleware.cs b/src/Umbraco.Web.Common/Middleware/BootFailedMiddleware.cs new file mode 100644 index 0000000000..685312778d --- /dev/null +++ b/src/Umbraco.Web.Common/Middleware/BootFailedMiddleware.cs @@ -0,0 +1,34 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Umbraco.Core; +using Umbraco.Core.Exceptions; + +namespace Umbraco.Web.Common.Middleware +{ + /// + /// Executes when Umbraco booting fails in order to show the problem + /// + public class BootFailedMiddleware : IMiddleware + { + private readonly IRuntimeState _runtimeState; + + public BootFailedMiddleware(IRuntimeState runtimeState) + { + _runtimeState = runtimeState; + } + + public async Task InvokeAsync(HttpContext context, RequestDelegate next) + { + if (_runtimeState.Level == RuntimeLevel.BootFailed) + { + // short circuit + BootFailedException.Rethrow(_runtimeState.BootFailedException); + } + else + { + await next(context); + } + } + } +} diff --git a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs index c2a29c63a6..a2e6ba2a02 100644 --- a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs +++ b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs @@ -18,6 +18,9 @@ using Umbraco.Web.Common.Install; using Umbraco.Extensions; using System.Linq; using Umbraco.Web.Common.Controllers; +using System; +using Umbraco.Web.Common.Middleware; +using Umbraco.Web.Common.ModelBinding; namespace Umbraco.Web.Common.Runtime { @@ -77,6 +80,11 @@ namespace Umbraco.Web.Common.Runtime composition.RegisterUnique(); + composition.RegisterUnique(); + composition.RegisterUnique(); + composition.RegisterUnique(); + + composition.RegisterUnique(); } } } diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 1df99ad45f..95adb9c64e 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -361,25 +361,6 @@ namespace Umbraco.Web /// public void Init(HttpApplication app) { - if (_runtime.Level == RuntimeLevel.BootFailed) - { - // there's nothing we can do really - app.BeginRequest += (sender, args) => - { - // would love to avoid throwing, and instead display a customized Umbraco 500 - // page - however if we don't throw here, something else might go wrong, and - // it's this later exception that would be reported. could not figure out how - // to prevent it, either with httpContext.Response.End() or .ApplicationInstance - // .CompleteRequest() - - // also, if something goes wrong with our DI setup, the logging subsystem may - // not even kick in, so here we try to give as much detail as possible - - BootFailedException.Rethrow(Current.RuntimeState.BootFailedException); - }; - return; - } - app.BeginRequest += (sender, e) => { var httpContext = ((HttpApplication) sender).Context; From 3d9603e669faf80f6f8be10d2497d115c5883c4b Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 14 May 2020 22:50:27 +1000 Subject: [PATCH 02/10] Migrates more logic from the umb module over to netcore --- .../ApplicationBuilderExtensions.cs | 51 +++++----- .../Install/InstallController.cs | 4 + .../UmbracoRequestLoggingMiddleware.cs | 3 + .../Middleware/UmbracoRequestMiddleware.cs | 93 +++++++++++++++++-- src/Umbraco.Web/UmbracoInjectedModule.cs | 93 +------------------ 5 files changed, 121 insertions(+), 123 deletions(-) diff --git a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs index a43094a8f9..bce4c2430e 100644 --- a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs @@ -27,8 +27,6 @@ namespace Umbraco.Extensions return runtime.State.Level > RuntimeLevel.BootFailed; } - - /// /// Start Umbraco /// @@ -38,27 +36,19 @@ namespace Umbraco.Extensions { if (app == null) throw new ArgumentNullException(nameof(app)); - if (app.UmbracoCanBoot()) - { - var runtime = app.ApplicationServices.GetRequiredService(); + var runtime = app.ApplicationServices.GetRequiredService(); - // Register a listener for application shutdown in order to terminate the runtime - var hostLifetime = app.ApplicationServices.GetRequiredService(); - var runtimeShutdown = new CoreRuntimeShutdown(runtime, hostLifetime); - hostLifetime.RegisterObject(runtimeShutdown); + // Register a listener for application shutdown in order to terminate the runtime + var hostLifetime = app.ApplicationServices.GetRequiredService(); + var runtimeShutdown = new CoreRuntimeShutdown(runtime, hostLifetime); + hostLifetime.RegisterObject(runtimeShutdown); - // Register our global threadabort enricher for logging - var threadAbortEnricher = app.ApplicationServices.GetRequiredService(); - LogContext.Push(threadAbortEnricher); // NOTE: We are not in a using clause because we are not removing it, it is on the global context + // Register our global threadabort enricher for logging + var threadAbortEnricher = app.ApplicationServices.GetRequiredService(); + LogContext.Push(threadAbortEnricher); // NOTE: We are not in a using clause because we are not removing it, it is on the global context - // Start the runtime! - runtime.Start(); - } - else - { - // TODO: Register simple middleware to show the error like we used to in UmbracoModule? Or maybe that's part of a UseUmbracoWebsite/backoffice type thing .. probably :) - - } + // Start the runtime! + runtime.Start(); return app; } @@ -73,15 +63,24 @@ namespace Umbraco.Extensions { if (app == null) throw new ArgumentNullException(nameof(app)); - app.UseMiddleware(); - - if (!app.UmbracoCanBoot()) return app; - - app.UseMiddleware(); - app.UseMiddleware(); + if (!app.UmbracoCanBoot()) + { + app.UseMiddleware(); + } + else + { + app.UseMiddleware(); + app.UseMiddleware(); + } + return app; } + /// + /// Adds request based serilog enrichers to the LogContext for each request + /// + /// + /// public static IApplicationBuilder UseUmbracoRequestLogging(this IApplicationBuilder app) { if (app == null) throw new ArgumentNullException(nameof(app)); diff --git a/src/Umbraco.Web.Common/Install/InstallController.cs b/src/Umbraco.Web.Common/Install/InstallController.cs index 6614f2d577..a4f659379f 100644 --- a/src/Umbraco.Web.Common/Install/InstallController.cs +++ b/src/Umbraco.Web.Common/Install/InstallController.cs @@ -92,6 +92,10 @@ namespace Umbraco.Web.Common.Install return View(); } + /// + /// Used to perform the redirect to the installer when the runtime level is or + /// + /// [HttpGet] public ActionResult Redirect() { diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs index 803eb95d62..febf939d03 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestLoggingMiddleware.cs @@ -8,6 +8,9 @@ using Umbraco.Core.Logging.Serilog.Enrichers; namespace Umbraco.Web.Common.Middleware { + /// + /// Adds request based serilog enrichers to the LogContext for each request + /// public class UmbracoRequestLoggingMiddleware : IMiddleware { private readonly HttpSessionIdEnricher _sessionIdEnricher; diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs index e1662e834f..6cf3929e06 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs @@ -6,6 +6,8 @@ using Umbraco.Web.Common.Lifetime; using Umbraco.Core; using Umbraco.Core.Logging; using System.Threading; +using Umbraco.Core.Cache; +using System.Collections.Generic; namespace Umbraco.Web.Common.Middleware { @@ -18,22 +20,26 @@ namespace Umbraco.Web.Common.Middleware private readonly ILogger _logger; private readonly IUmbracoRequestLifetimeManager _umbracoRequestLifetimeManager; private readonly IUmbracoContextFactory _umbracoContextFactory; + private readonly IRequestCache _requestCache; public UmbracoRequestMiddleware( ILogger logger, IUmbracoRequestLifetimeManager umbracoRequestLifetimeManager, - IUmbracoContextFactory umbracoContextFactory) + IUmbracoContextFactory umbracoContextFactory, + IRequestCache requestCache) { _logger = logger; _umbracoRequestLifetimeManager = umbracoRequestLifetimeManager; _umbracoContextFactory = umbracoContextFactory; + _requestCache = requestCache; } public async Task InvokeAsync(HttpContext context, RequestDelegate next) { - // do not process if client-side request + var requestUri = new Uri(context.Request.GetEncodedUrl(), UriKind.RelativeOrAbsolute); - if (new Uri(context.Request.GetEncodedUrl(), UriKind.RelativeOrAbsolute).IsClientSideRequest()) + // do not process if client-side request + if (requestUri.IsClientSideRequest()) { await next(context); return; @@ -43,6 +49,12 @@ namespace Umbraco.Web.Common.Middleware try { + if (umbracoContextReference.UmbracoContext.IsFrontEndUmbracoRequest) + { + LogHttpRequest.TryGetCurrentHttpRequestId(out var httpRequestId, _requestCache); + _logger.Verbose("Begin request [{HttpRequestId}]: {RequestUrl}", httpRequestId, requestUri); + } + try { _umbracoRequestLifetimeManager.InitRequest(context); @@ -52,15 +64,80 @@ namespace Umbraco.Web.Common.Middleware // try catch so we don't kill everything in all requests _logger.Error(ex); } - - await next(context); - - _umbracoRequestLifetimeManager.EndRequest(context); + finally + { + try + { + await next(context); + } + finally + { + _umbracoRequestLifetimeManager.EndRequest(context); + } + } } finally { - umbracoContextReference.Dispose(); + if (umbracoContextReference.UmbracoContext.IsFrontEndUmbracoRequest) + { + LogHttpRequest.TryGetCurrentHttpRequestId(out var httpRequestId, _requestCache); + _logger.Verbose("End Request [{HttpRequestId}]: {RequestUrl} ({RequestDuration}ms)", httpRequestId, requestUri, DateTime.Now.Subtract(umbracoContextReference.UmbracoContext.ObjectCreated).TotalMilliseconds); + } + + try + { + DisposeRequestCacheItems(_logger, _requestCache, requestUri); + } + finally + { + umbracoContextReference.Dispose(); + } } } + + /// + /// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request + /// + /// + /// + /// + private static void DisposeRequestCacheItems(ILogger logger, IRequestCache requestCache, Uri requestUri) + { + // do not process if client-side request + if (requestUri.IsClientSideRequest()) + return; + + //get a list of keys to dispose + var keys = new HashSet(); + foreach (var i in requestCache) + { + if (i.Value is IDisposeOnRequestEnd || i.Key is IDisposeOnRequestEnd) + { + keys.Add(i.Key); + } + } + //dispose each item and key that was found as disposable. + foreach (var k in keys) + { + try + { + requestCache.Get(k).DisposeIfDisposable(); + } + catch (Exception ex) + { + logger.Error("Could not dispose item with key " + k, ex); + } + try + { + k.DisposeIfDisposable(); + } + catch (Exception ex) + { + logger.Error("Could not dispose item key " + k, ex); + } + } + } + + } } diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index 95adb9c64e..a63e305ce2 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -210,11 +210,9 @@ namespace Umbraco.Web case RuntimeLevel.Install: case RuntimeLevel.Upgrade: - // redirect to install - ReportRuntime(level, "Umbraco must install or upgrade."); - var installPath = _uriUtility.ToAbsolute(Constants.SystemDirectories.Install); - var installUrl = $"{installPath}/?redir=true&url={HttpUtility.UrlEncode(uri.ToString())}"; - httpContext.Response.Redirect(installUrl, true); + + // NOTE: We have moved the logic that was here to netcore already + return false; // cannot serve content default: @@ -222,17 +220,6 @@ namespace Umbraco.Web } } - private static bool _reported; - private static RuntimeLevel _reportedLevel; - - private void ReportRuntime(RuntimeLevel level, string message) - { - if (_reported && _reportedLevel == level) return; - _reported = true; - _reportedLevel = level; - _logger.Warn(message); - } - // ensures Umbraco has at least one published node // if not, rewrites to splash and return false // if yes, return true @@ -309,47 +296,7 @@ namespace Umbraco.Web } - /// - /// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request - /// - /// - private void DisposeRequestCacheItems(HttpContext http, IRequestCache requestCache) - { - // do not process if client-side request - if (http.Request.Url.IsClientSideRequest()) - return; - - //get a list of keys to dispose - var keys = new HashSet(); - foreach (var i in requestCache) - { - if (i.Value is IDisposeOnRequestEnd || i.Key is IDisposeOnRequestEnd) - { - keys.Add(i.Key); - } - } - //dispose each item and key that was found as disposable. - foreach (var k in keys) - { - try - { - requestCache.Get(k).DisposeIfDisposable(); - } - catch (Exception ex) - { - _logger.Error("Could not dispose item with key " + k, ex); - } - try - { - k.DisposeIfDisposable(); - } - catch (Exception ex) - { - _logger.Error("Could not dispose item key " + k, ex); - } - } - } - + #endregion #region IHttpModule @@ -365,32 +312,9 @@ namespace Umbraco.Web { var httpContext = ((HttpApplication) sender).Context; - LogHttpRequest.TryGetCurrentHttpRequestId(out var httpRequestId, _requestCache); - - _logger.Verbose("Begin request [{HttpRequestId}]: {RequestUrl}", httpRequestId, httpContext.Request.Url); BeginRequest(new HttpContextWrapper(httpContext)); }; - //disable asp.net headers (security) - // This is the correct place to modify headers according to MS: - // https://our.umbraco.com/forum/umbraco-7/using-umbraco-7/65241-Heap-error-from-header-manipulation?p=0#comment220889 - app.PostReleaseRequestState += (sender, args) => - { - var httpContext = ((HttpApplication) sender).Context; - try - { - httpContext.Response.Headers.Remove("Server"); - //this doesn't normally work since IIS sets it but we'll keep it here anyways. - httpContext.Response.Headers.Remove("X-Powered-By"); - httpContext.Response.Headers.Remove("X-AspNet-Version"); - httpContext.Response.Headers.Remove("X-AspNetMvc-Version"); - } - catch (PlatformNotSupportedException) - { - // can't remove headers this way on IIS6 or cassini. - } - }; - app.PostAuthenticateRequest += (sender, e) => { var httpContext = ((HttpApplication) sender).Context; @@ -408,16 +332,7 @@ namespace Umbraco.Web { var httpContext = ((HttpApplication) sender).Context; - if (Current.UmbracoContext != null && Current.UmbracoContext.IsFrontEndUmbracoRequest) - { - LogHttpRequest.TryGetCurrentHttpRequestId(out var httpRequestId, _requestCache); - - _logger.Verbose("End Request [{HttpRequestId}]: {RequestUrl} ({RequestDuration}ms)", httpRequestId, httpContext.Request.Url, DateTime.Now.Subtract(Current.UmbracoContext.ObjectCreated).TotalMilliseconds); - } - UmbracoModule.OnEndRequest(this, new UmbracoRequestEventArgs(Current.UmbracoContext)); - - DisposeRequestCacheItems(httpContext, _requestCache); }; } From 95d695594a4ddd306d5438d0ef691ade32645a9e Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 19 May 2020 14:51:05 +1000 Subject: [PATCH 03/10] Ensure we capture boot failed exception earlier and ensure that composers registered to run on boot failed are executed --- .../Runtime/CoreRuntime.cs | 39 ++++++++++++------- .../ApplicationBuilderExtensions.cs | 7 ++-- ...racoInstallApplicationBuilderExtensions.cs | 2 + .../Runtime/AspNetCoreBootFailedComposer.cs | 18 +++++++++ .../Runtime/AspNetCoreComposer.cs | 1 + 5 files changed, 50 insertions(+), 17 deletions(-) create mode 100644 src/Umbraco.Web.Common/Runtime/AspNetCoreBootFailedComposer.cs diff --git a/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs b/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs index 1b400c3506..2f9766e5c2 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs @@ -185,23 +185,19 @@ namespace Umbraco.Core.Runtime // register ourselves (TODO: Should we put this in RegisterEssentials?) composition.Register(_ => this, Lifetime.Singleton); - // determine our runtime level - DetermineRuntimeLevel(databaseFactory, ProfilingLogger); - - // get composers, and compose - var composerTypes = ResolveComposerTypes(typeLoader); - - IEnumerable enableDisableAttributes; - using (ProfilingLogger.DebugDuration("Scanning enable/disable composer attributes")) + try { - enableDisableAttributes = typeLoader.GetAssemblyAttributes(typeof(EnableComposerAttribute), typeof(DisableComposerAttribute)); + // determine our runtime level + DetermineRuntimeLevel(databaseFactory, ProfilingLogger); + } + finally + { + // always run composers + RunComposers(typeLoader, composition); } - var composers = new Composers(composition, composerTypes, enableDisableAttributes, ProfilingLogger); - composers.Compose(); - - // create the factory - factory = composition.CreateFactory(); + // create the factory + factory = composition.CreateFactory(); } catch (Exception e) { @@ -286,6 +282,21 @@ namespace Umbraco.Core.Runtime }; } + private void RunComposers(TypeLoader typeLoader, Composition composition) + { + // get composers, and compose + var composerTypes = ResolveComposerTypes(typeLoader); + + IEnumerable enableDisableAttributes; + using (ProfilingLogger.DebugDuration("Scanning enable/disable composer attributes")) + { + enableDisableAttributes = typeLoader.GetAssemblyAttributes(typeof(EnableComposerAttribute), typeof(DisableComposerAttribute)); + } + + var composers = new Composers(composition, composerTypes, enableDisableAttributes, ProfilingLogger); + composers.Compose(); + } + private bool AcquireMainDom(IMainDom mainDom, IApplicationShutdownRegistry applicationShutdownRegistry) { using (var timer = ProfilingLogger.DebugDuration("Acquiring MainDom.", "Acquired.")) diff --git a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs index bce4c2430e..5a68fddb73 100644 --- a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs @@ -36,8 +36,9 @@ namespace Umbraco.Extensions { if (app == null) throw new ArgumentNullException(nameof(app)); - var runtime = app.ApplicationServices.GetRequiredService(); + if (!app.UmbracoCanBoot()) return app; + var runtime = app.ApplicationServices.GetRequiredService(); // Register a listener for application shutdown in order to terminate the runtime var hostLifetime = app.ApplicationServices.GetRequiredService(); var runtimeShutdown = new CoreRuntimeShutdown(runtime, hostLifetime); @@ -65,14 +66,14 @@ namespace Umbraco.Extensions if (!app.UmbracoCanBoot()) { - app.UseMiddleware(); + app.UseMiddleware(); } else { app.UseMiddleware(); app.UseMiddleware(); } - + return app; } diff --git a/src/Umbraco.Web.Common/Extensions/UmbracoInstallApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/UmbracoInstallApplicationBuilderExtensions.cs index 79e253e053..3350af756e 100644 --- a/src/Umbraco.Web.Common/Extensions/UmbracoInstallApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/UmbracoInstallApplicationBuilderExtensions.cs @@ -13,6 +13,8 @@ namespace Umbraco.Extensions /// public static IApplicationBuilder UseUmbracoInstaller(this IApplicationBuilder app) { + if (!app.UmbracoCanBoot()) return app; + app.UseEndpoints(endpoints => { var installerRoutes = app.ApplicationServices.GetRequiredService(); diff --git a/src/Umbraco.Web.Common/Runtime/AspNetCoreBootFailedComposer.cs b/src/Umbraco.Web.Common/Runtime/AspNetCoreBootFailedComposer.cs new file mode 100644 index 0000000000..010e6533d9 --- /dev/null +++ b/src/Umbraco.Web.Common/Runtime/AspNetCoreBootFailedComposer.cs @@ -0,0 +1,18 @@ +using Umbraco.Core; +using Umbraco.Core.Composing; +using Umbraco.Web.Common.Middleware; + +namespace Umbraco.Web.Common.Runtime +{ + /// + /// Executes if the boot fails to ensure critical services are registered + /// + [RuntimeLevel(MinLevel = RuntimeLevel.BootFailed)] + public class AspNetCoreBootFailedComposer : IComposer + { + public void Compose(Composition composition) + { + composition.RegisterUnique(); + } + } +} diff --git a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs index a2e6ba2a02..babf6cb80d 100644 --- a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs +++ b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs @@ -24,6 +24,7 @@ using Umbraco.Web.Common.ModelBinding; namespace Umbraco.Web.Common.Runtime { + /// /// Adds/replaces AspNetCore specific services /// From ea380985dbb3274c50fa6f428c3503bac3d74a49 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 20 May 2020 00:19:43 +0200 Subject: [PATCH 04/10] Moving files using MailKit nuget package (Smtp successor) as SmtpClient has been marked as obsolete by Microsoft --- .../Events/SendEmailEventArgs.cs | 0 .../Users}/EmailSender.cs | 50 ++++++++++++++++--- .../Users}/IEmailSender.cs | 0 3 files changed, 43 insertions(+), 7 deletions(-) rename src/{Umbraco.Core => Umbraco.Infrastructure}/Events/SendEmailEventArgs.cs (100%) rename src/{Umbraco.Core => Umbraco.Infrastructure/Users}/EmailSender.cs (58%) rename src/{Umbraco.Core => Umbraco.Infrastructure/Users}/IEmailSender.cs (100%) diff --git a/src/Umbraco.Core/Events/SendEmailEventArgs.cs b/src/Umbraco.Infrastructure/Events/SendEmailEventArgs.cs similarity index 100% rename from src/Umbraco.Core/Events/SendEmailEventArgs.cs rename to src/Umbraco.Infrastructure/Events/SendEmailEventArgs.cs diff --git a/src/Umbraco.Core/EmailSender.cs b/src/Umbraco.Infrastructure/Users/EmailSender.cs similarity index 58% rename from src/Umbraco.Core/EmailSender.cs rename to src/Umbraco.Infrastructure/Users/EmailSender.cs index 5cfdd765bc..62c45b8f78 100644 --- a/src/Umbraco.Core/EmailSender.cs +++ b/src/Umbraco.Infrastructure/Users/EmailSender.cs @@ -1,9 +1,12 @@ using System; using System.Net.Mail; using System.Threading.Tasks; -using Umbraco.Core.Composing; +using MailKit.Security; +using MimeKit; +using MimeKit.Text; using Umbraco.Core.Configuration; using Umbraco.Core.Events; +using SmtpClient = MailKit.Net.Smtp.SmtpClient; namespace Umbraco.Core { @@ -21,7 +24,7 @@ namespace Umbraco.Core { } - internal EmailSender(IGlobalSettings globalSettings, bool enableEvents) + public EmailSender(IGlobalSettings globalSettings, bool enableEvents) { _globalSettings = globalSettings; _enableEvents = enableEvents; @@ -45,7 +48,9 @@ namespace Umbraco.Core { using (var client = new SmtpClient()) { - client.Send(message); + client.Connect(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); + client.Send(ConstructEmailMessage(message)); + client.Disconnect(true); } } } @@ -65,14 +70,21 @@ namespace Umbraco.Core { using (var client = new SmtpClient()) { - if (client.DeliveryMethod == SmtpDeliveryMethod.Network) + var appSettingsDeliveryMethod = _globalSettings.SmtpSettings.DeliveryMethod; + var deliveryMethod = (SmtpDeliveryMethod)Enum.Parse(typeof(SmtpDeliveryMethod), appSettingsDeliveryMethod, true); + + await client.ConnectAsync(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); + + if (deliveryMethod == SmtpDeliveryMethod.Network) { - await client.SendMailAsync(message); + await client.SendAsync(ConstructEmailMessage(message)); } else { - client.Send(message); + client.Send(ConstructEmailMessage(message)); } + + await client.DisconnectAsync(true); } } } @@ -83,7 +95,7 @@ namespace Umbraco.Core /// /// We assume this is possible if either an event handler is registered or an smtp server is configured /// - internal static bool CanSendRequiredEmail(IGlobalSettings globalSettings) => EventHandlerRegistered || globalSettings.IsSmtpServerConfigured; + public static bool CanSendRequiredEmail(IGlobalSettings globalSettings) => EventHandlerRegistered || globalSettings.IsSmtpServerConfigured; /// /// returns true if an event handler has been registered @@ -103,5 +115,29 @@ namespace Umbraco.Core var handler = SendEmail; if (handler != null) handler(null, e); } + + private MimeMessage ConstructEmailMessage(MailMessage mailMessage) + { + var messageToSend = new MimeMessage + { + Subject = mailMessage.Subject + }; + + var fromEmail = mailMessage.From?.Address; + if(string.IsNullOrEmpty(fromEmail)) + fromEmail = _globalSettings.SmtpSettings.From; + + messageToSend.From.Add(new MailboxAddress(fromEmail)); + + foreach (var mailAddress in mailMessage.To) + messageToSend.To.Add(new MailboxAddress(mailAddress.Address)); + + if (mailMessage.IsBodyHtml) + messageToSend.Body = new TextPart(TextFormat.Html) { Text = mailMessage.Body }; + else + messageToSend.Body = new TextPart(TextFormat.Plain) { Text = mailMessage.Body }; + + return messageToSend; + } } } diff --git a/src/Umbraco.Core/IEmailSender.cs b/src/Umbraco.Infrastructure/Users/IEmailSender.cs similarity index 100% rename from src/Umbraco.Core/IEmailSender.cs rename to src/Umbraco.Infrastructure/Users/IEmailSender.cs From e85ccb0dabd0b488292f566afcbb1640980f5cb6 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 20 May 2020 00:20:25 +0200 Subject: [PATCH 05/10] Removing TODOs with the actual implementation --- .../Editors/AuthenticationController.cs | 21 +++++++++++++------ src/Umbraco.Web/Editors/UsersController.cs | 20 +++++++++--------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index fc34a35566..50e9921154 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Collections.Generic; +using System.Net.Mail; using System.Security.Principal; using System.Threading.Tasks; using System.Web; @@ -328,12 +329,20 @@ namespace Umbraco.Web.Editors UmbracoUserExtensions.GetUserCulture(identityUser.Culture, Services.TextService, GlobalSettings), new[] { identityUser.UserName, callbackUrl }); - // TODO: Port email service to ASP.NET Core - /*await UserManager.SendEmailAsync(identityUser.Id, - Services.TextService.Localize("login/resetPasswordEmailCopySubject", - // Ensure the culture of the found user is used for the email! - UmbracoUserExtensions.GetUserCulture(identityUser.Culture, Services.TextService, GlobalSettings)), - message);*/ + var subject = Services.TextService.Localize("login/resetPasswordEmailCopySubject", + // Ensure the culture of the found user is used for the email! + UmbracoUserExtensions.GetUserCulture(identityUser.Culture, Services.TextService, GlobalSettings)); + + var emailSender = new EmailSender(GlobalSettings, true); + var mailMessage = new MailMessage() + { + Subject = subject, + Body = message, + IsBodyHtml = true + }; + mailMessage.To.Add(user.Email); + + await emailSender.SendAsync(mailMessage); UserManager.RaiseForgotPasswordRequestedEvent(user.Id); } diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 7aa71b6e2e..029612dc90 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -4,6 +4,7 @@ using System.IO; using System.Linq; using System.Net; using System.Net.Http; +using System.Net.Mail; using System.Runtime.Serialization; using System.Security.Cryptography; using System.Threading.Tasks; @@ -500,17 +501,16 @@ namespace Umbraco.Web.Editors UmbracoUserExtensions.GetUserCulture(to.Language, Services.TextService, GlobalSettings), new[] { userDisplay.Name, from, message, inviteUri.ToString(), fromEmail }); - // TODO: Port email service to ASP.NET Core - /*await UserManager.EmailService.SendAsync( - //send the special UmbracoEmailMessage which configures it's own sender - //to allow for events to handle sending the message if no smtp is configured - new UmbracoEmailMessage(new EmailSender(GlobalSettings, true)) - { - Body = emailBody, - Destination = userDisplay.Email, - Subject = emailSubject - });*/ + var emailSender = new EmailSender(GlobalSettings, true); + var mailMessage = new MailMessage() + { + Subject = emailSubject, + Body = emailBody, + IsBodyHtml = true + }; + mailMessage.To.Add(to.Email); + await emailSender.SendAsync(mailMessage); } /// From ad31db9f4afa888a88e5617d25f94d43d856336b Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 20 May 2020 00:21:45 +0200 Subject: [PATCH 06/10] Adding DeliveryMethod option which we make use of in EmailSender.cs --- src/Umbraco.Configuration/Legacy/SmtpSettings.cs | 1 + src/Umbraco.Configuration/Models/GlobalSettings.cs | 5 +++-- src/Umbraco.Core/Configuration/ISmtpSettings.cs | 1 + .../Builders/SmtpSettingsBuilder.cs | 10 ++++++++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Configuration/Legacy/SmtpSettings.cs b/src/Umbraco.Configuration/Legacy/SmtpSettings.cs index 7e3ff80690..beb6eb9691 100644 --- a/src/Umbraco.Configuration/Legacy/SmtpSettings.cs +++ b/src/Umbraco.Configuration/Legacy/SmtpSettings.cs @@ -8,5 +8,6 @@ namespace Umbraco.Configuration public string Host { get; set; } public int Port { get; set; } public string PickupDirectoryLocation { get; set; } + public string DeliveryMethod { get; set; } } } diff --git a/src/Umbraco.Configuration/Models/GlobalSettings.cs b/src/Umbraco.Configuration/Models/GlobalSettings.cs index 4b30813bd5..646bca0f13 100644 --- a/src/Umbraco.Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Configuration/Models/GlobalSettings.cs @@ -72,10 +72,10 @@ namespace Umbraco.Configuration.Models _configuration.GetValue(Prefix + "NoNodesViewPath", "~/config/splashes/NoNodes.cshtml"); public bool IsSmtpServerConfigured => - _configuration.GetSection(Constants.Configuration.ConfigPrefix + "Smtp")?.GetChildren().Any() ?? false; + _configuration.GetSection(Constants.Configuration.ConfigGlobalPrefix + "Smtp")?.GetChildren().Any() ?? false; public ISmtpSettings SmtpSettings => - new SmtpSettingsImpl(_configuration.GetSection(Constants.Configuration.ConfigPrefix + "Smtp")); + new SmtpSettingsImpl(_configuration.GetSection(Constants.Configuration.ConfigGlobalPrefix + "Smtp")); private class SmtpSettingsImpl : ISmtpSettings { @@ -90,6 +90,7 @@ namespace Umbraco.Configuration.Models public string Host => _configurationSection.GetValue("Host"); public int Port => _configurationSection.GetValue("Port"); public string PickupDirectoryLocation => _configurationSection.GetValue("PickupDirectoryLocation"); + public string DeliveryMethod => _configurationSection.GetValue("DeliveryMethod"); } } } diff --git a/src/Umbraco.Core/Configuration/ISmtpSettings.cs b/src/Umbraco.Core/Configuration/ISmtpSettings.cs index c2fb4b2dbe..217e83455b 100644 --- a/src/Umbraco.Core/Configuration/ISmtpSettings.cs +++ b/src/Umbraco.Core/Configuration/ISmtpSettings.cs @@ -6,5 +6,6 @@ namespace Umbraco.Core.Configuration string Host { get; } int Port{ get; } string PickupDirectoryLocation { get; } + string DeliveryMethod { get; } } } diff --git a/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs b/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs index 344d7bcf87..140af5f723 100644 --- a/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs @@ -16,6 +16,7 @@ namespace Umbraco.Tests.Common.Builders private string _host; private int? _port; private string _pickupDirectoryLocation; + private string _deliveryMethod; public SmtpSettingsBuilder(TParent parentBuilder) : base(parentBuilder) { @@ -45,12 +46,19 @@ namespace Umbraco.Tests.Common.Builders return this; } + public SmtpSettingsBuilder WithDeliveryMethod(string deliveryMethod) + { + _deliveryMethod = deliveryMethod; + return this; + } + public override ISmtpSettings Build() { var from = _from ?? null; var host = _host ?? null; var port = _port ?? 25; var pickupDirectoryLocation = _pickupDirectoryLocation ?? null; + var deliveryMethod = _deliveryMethod ?? null; return new TestSmtpSettings() { @@ -58,6 +66,7 @@ namespace Umbraco.Tests.Common.Builders Host = host, Port = port, PickupDirectoryLocation = pickupDirectoryLocation, + DeliveryMethod = deliveryMethod }; } @@ -67,6 +76,7 @@ namespace Umbraco.Tests.Common.Builders public string Host { get; set; } public int Port { get; set; } public string PickupDirectoryLocation { get; set; } + public string DeliveryMethod { get; set; } } } } From 16d805e6f5b7750b5acb52fdd43d8380aae020c6 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 20 May 2020 00:24:55 +0200 Subject: [PATCH 07/10] Adding Smtp section in appsettings, registering IEmailSender as an email service and ofc the MailKit nuget in Infrastructure proj --- .../Runtime/CoreInitialComposer.cs | 1 + .../Umbraco.Infrastructure.csproj | 1 + src/Umbraco.Web.UI.NetCore/appsettings.json | 10 ++++++++++ 3 files changed, 12 insertions(+) diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index 19d3716e1c..34d69c2cd5 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -320,6 +320,7 @@ namespace Umbraco.Core.Runtime composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); + composition.RegisterUnique(); composition.RegisterUnique(); diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index db3ae1bc25..8da86df6e8 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Umbraco.Web.UI.NetCore/appsettings.json b/src/Umbraco.Web.UI.NetCore/appsettings.json index 46dc20034b..95fdaeec67 100644 --- a/src/Umbraco.Web.UI.NetCore/appsettings.json +++ b/src/Umbraco.Web.UI.NetCore/appsettings.json @@ -116,6 +116,16 @@ "Replacement": "" } ] + }, + "Global": { + "Smtp": { + "From": "noreply@example.com", + "Host": "127.0.0.1", + "Port": 25, + "UserName": "username", + "Password": "password", + "DeliveryMethod": "network" + } } } } From 65fa1efc9c7f3dd4fddc3775810a224e35651659 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 20 May 2020 00:54:28 +0200 Subject: [PATCH 08/10] Changing a var name for better reflection on the origin of its value --- src/Umbraco.Infrastructure/Users/EmailSender.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Infrastructure/Users/EmailSender.cs b/src/Umbraco.Infrastructure/Users/EmailSender.cs index 62c45b8f78..87f27e5097 100644 --- a/src/Umbraco.Infrastructure/Users/EmailSender.cs +++ b/src/Umbraco.Infrastructure/Users/EmailSender.cs @@ -70,8 +70,8 @@ namespace Umbraco.Core { using (var client = new SmtpClient()) { - var appSettingsDeliveryMethod = _globalSettings.SmtpSettings.DeliveryMethod; - var deliveryMethod = (SmtpDeliveryMethod)Enum.Parse(typeof(SmtpDeliveryMethod), appSettingsDeliveryMethod, true); + var smtpSettingsDeliveryMethod = _globalSettings.SmtpSettings.DeliveryMethod; + var deliveryMethod = (SmtpDeliveryMethod)Enum.Parse(typeof(SmtpDeliveryMethod), smtpSettingsDeliveryMethod, true); await client.ConnectAsync(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); From 2b4dfbca9d44b7415bae5d6582bec35fde37a3b5 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 20 May 2020 11:42:23 +0200 Subject: [PATCH 09/10] #8142 - Small cleanup, and bugfix --- .../Legacy/SmtpSettings.cs | 7 +++- .../Models/GlobalSettings.cs | 7 +++- .../Configuration/ISmtpSettings.cs | 6 +++- .../Events/SendEmailEventArgs.cs | 0 .../Users => Umbraco.Core}/IEmailSender.cs | 0 .../Users/EmailSender.cs | 35 +++++++------------ .../Builders/SmtpSettingsBuilder.cs | 34 ++++++++++++++---- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 1 + .../AuthenticationControllerTests.cs | 3 +- .../Web/Controllers/UsersControllerTests.cs | 15 +++++--- .../Editors/AuthenticationController.cs | 12 ++++--- src/Umbraco.Web/Editors/UsersController.cs | 12 ++++--- 12 files changed, 85 insertions(+), 47 deletions(-) rename src/{Umbraco.Infrastructure => Umbraco.Core}/Events/SendEmailEventArgs.cs (100%) rename src/{Umbraco.Infrastructure/Users => Umbraco.Core}/IEmailSender.cs (100%) diff --git a/src/Umbraco.Configuration/Legacy/SmtpSettings.cs b/src/Umbraco.Configuration/Legacy/SmtpSettings.cs index beb6eb9691..dce3d85840 100644 --- a/src/Umbraco.Configuration/Legacy/SmtpSettings.cs +++ b/src/Umbraco.Configuration/Legacy/SmtpSettings.cs @@ -1,3 +1,4 @@ +using System.Net.Mail; using Umbraco.Core.Configuration; namespace Umbraco.Configuration @@ -8,6 +9,10 @@ namespace Umbraco.Configuration public string Host { get; set; } public int Port { get; set; } public string PickupDirectoryLocation { get; set; } - public string DeliveryMethod { get; set; } + public SmtpDeliveryMethod DeliveryMethod { get; set; } + + public string Username { get; set; } + + public string Password { get; set; } } } diff --git a/src/Umbraco.Configuration/Models/GlobalSettings.cs b/src/Umbraco.Configuration/Models/GlobalSettings.cs index 646bca0f13..e4995cfb0b 100644 --- a/src/Umbraco.Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Configuration/Models/GlobalSettings.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Net.Mail; using Microsoft.Extensions.Configuration; using Umbraco.Core; using Umbraco.Core.Configuration; @@ -90,7 +91,11 @@ namespace Umbraco.Configuration.Models public string Host => _configurationSection.GetValue("Host"); public int Port => _configurationSection.GetValue("Port"); public string PickupDirectoryLocation => _configurationSection.GetValue("PickupDirectoryLocation"); - public string DeliveryMethod => _configurationSection.GetValue("DeliveryMethod"); + public SmtpDeliveryMethod DeliveryMethod => _configurationSection.GetValue("DeliveryMethod"); + + public string Username => _configurationSection.GetValue("Username"); + + public string Password => _configurationSection.GetValue("Password"); } } } diff --git a/src/Umbraco.Core/Configuration/ISmtpSettings.cs b/src/Umbraco.Core/Configuration/ISmtpSettings.cs index 217e83455b..ea42ae5567 100644 --- a/src/Umbraco.Core/Configuration/ISmtpSettings.cs +++ b/src/Umbraco.Core/Configuration/ISmtpSettings.cs @@ -1,3 +1,5 @@ +using System.Net.Mail; + namespace Umbraco.Core.Configuration { public interface ISmtpSettings @@ -6,6 +8,8 @@ namespace Umbraco.Core.Configuration string Host { get; } int Port{ get; } string PickupDirectoryLocation { get; } - string DeliveryMethod { get; } + SmtpDeliveryMethod DeliveryMethod { get; } + string Username { get; } + string Password { get; } } } diff --git a/src/Umbraco.Infrastructure/Events/SendEmailEventArgs.cs b/src/Umbraco.Core/Events/SendEmailEventArgs.cs similarity index 100% rename from src/Umbraco.Infrastructure/Events/SendEmailEventArgs.cs rename to src/Umbraco.Core/Events/SendEmailEventArgs.cs diff --git a/src/Umbraco.Infrastructure/Users/IEmailSender.cs b/src/Umbraco.Core/IEmailSender.cs similarity index 100% rename from src/Umbraco.Infrastructure/Users/IEmailSender.cs rename to src/Umbraco.Core/IEmailSender.cs diff --git a/src/Umbraco.Infrastructure/Users/EmailSender.cs b/src/Umbraco.Infrastructure/Users/EmailSender.cs index 87f27e5097..315a1748c3 100644 --- a/src/Umbraco.Infrastructure/Users/EmailSender.cs +++ b/src/Umbraco.Infrastructure/Users/EmailSender.cs @@ -1,7 +1,7 @@ using System; +using System.Linq; using System.Net.Mail; using System.Threading.Tasks; -using MailKit.Security; using MimeKit; using MimeKit.Text; using Umbraco.Core.Configuration; @@ -70,20 +70,18 @@ namespace Umbraco.Core { using (var client = new SmtpClient()) { - var smtpSettingsDeliveryMethod = _globalSettings.SmtpSettings.DeliveryMethod; - var deliveryMethod = (SmtpDeliveryMethod)Enum.Parse(typeof(SmtpDeliveryMethod), smtpSettingsDeliveryMethod, true); - await client.ConnectAsync(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); - if (deliveryMethod == SmtpDeliveryMethod.Network) + var mailMessage = ConstructEmailMessage(message); + if (_globalSettings.SmtpSettings.DeliveryMethod == SmtpDeliveryMethod.Network) { - await client.SendAsync(ConstructEmailMessage(message)); + await client.SendAsync(mailMessage); } else { - client.Send(ConstructEmailMessage(message)); + client.Send(mailMessage); } - + await client.DisconnectAsync(true); } } @@ -118,24 +116,17 @@ namespace Umbraco.Core private MimeMessage ConstructEmailMessage(MailMessage mailMessage) { - var messageToSend = new MimeMessage - { - Subject = mailMessage.Subject - }; - var fromEmail = mailMessage.From?.Address; if(string.IsNullOrEmpty(fromEmail)) fromEmail = _globalSettings.SmtpSettings.From; - - messageToSend.From.Add(new MailboxAddress(fromEmail)); - foreach (var mailAddress in mailMessage.To) - messageToSend.To.Add(new MailboxAddress(mailAddress.Address)); - - if (mailMessage.IsBodyHtml) - messageToSend.Body = new TextPart(TextFormat.Html) { Text = mailMessage.Body }; - else - messageToSend.Body = new TextPart(TextFormat.Plain) { Text = mailMessage.Body }; + var messageToSend = new MimeMessage + { + Subject = mailMessage.Subject, + From = { new MailboxAddress(fromEmail)}, + Body = new TextPart(mailMessage.IsBodyHtml ? TextFormat.Html : TextFormat.Plain) { Text = mailMessage.Body } + }; + messageToSend.To.AddRange(mailMessage.To.Select(x=>new MailboxAddress(x.Address))); return messageToSend; } diff --git a/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs b/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs index 140af5f723..bd85807203 100644 --- a/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/SmtpSettingsBuilder.cs @@ -1,4 +1,6 @@ -using Umbraco.Core.Configuration; +using System.Net.Mail; +using Umbraco.Core.Configuration; +using Umbraco.Core.Models.Membership; namespace Umbraco.Tests.Common.Builders { @@ -16,7 +18,9 @@ namespace Umbraco.Tests.Common.Builders private string _host; private int? _port; private string _pickupDirectoryLocation; - private string _deliveryMethod; + private SmtpDeliveryMethod? _deliveryMethod; + private string _username; + private string _password; public SmtpSettingsBuilder(TParent parentBuilder) : base(parentBuilder) { @@ -34,19 +38,31 @@ namespace Umbraco.Tests.Common.Builders return this; } + public SmtpSettingsBuilder WithUsername(string username) + { + _username = username; + return this; + } + public SmtpSettingsBuilder WithPost(int port) { _port = port; return this; } + public SmtpSettingsBuilder WithPassword(string password) + { + _password = password; + return this; + } + public SmtpSettingsBuilder WithPickupDirectoryLocation(string pickupDirectoryLocation) { _pickupDirectoryLocation = pickupDirectoryLocation; return this; } - public SmtpSettingsBuilder WithDeliveryMethod(string deliveryMethod) + public SmtpSettingsBuilder WithDeliveryMethod(SmtpDeliveryMethod deliveryMethod) { _deliveryMethod = deliveryMethod; return this; @@ -58,7 +74,9 @@ namespace Umbraco.Tests.Common.Builders var host = _host ?? null; var port = _port ?? 25; var pickupDirectoryLocation = _pickupDirectoryLocation ?? null; - var deliveryMethod = _deliveryMethod ?? null; + var deliveryMethod = _deliveryMethod ?? SmtpDeliveryMethod.Network; + var username = _username ?? null; + var password = _password ?? null; return new TestSmtpSettings() { @@ -66,7 +84,9 @@ namespace Umbraco.Tests.Common.Builders Host = host, Port = port, PickupDirectoryLocation = pickupDirectoryLocation, - DeliveryMethod = deliveryMethod + DeliveryMethod = deliveryMethod, + Username = username, + Password = password, }; } @@ -76,7 +96,9 @@ namespace Umbraco.Tests.Common.Builders public string Host { get; set; } public int Port { get; set; } public string PickupDirectoryLocation { get; set; } - public string DeliveryMethod { get; set; } + public SmtpDeliveryMethod DeliveryMethod { get; set; } + public string Username { get; set; } + public string Password { get; set; } } } } diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index 80f6ab9c9e..bbc869fc65 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -313,6 +313,7 @@ namespace Umbraco.Tests.Testing Composition.RegisterUnique(); Composition.RegisterUnique(); + Composition.RegisterUnique(); Composition.RegisterUnique(); Composition.RegisterUnique(); Composition.RegisterUnique(); diff --git a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs index a162b0cd48..f8f02560c4 100644 --- a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs @@ -91,7 +91,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance() + Factory.GetInstance(), + Factory.GetInstance() ); return usersController; } diff --git a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs index 4c373b2bc8..ce355180f6 100644 --- a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs @@ -103,7 +103,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance() + Factory.GetInstance(), + Factory.GetInstance() ); return usersController; @@ -176,7 +177,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance() + Factory.GetInstance(), + Factory.GetInstance() ); return usersController; } @@ -219,7 +221,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance() + Factory.GetInstance(), + Factory.GetInstance() ); return usersController; } @@ -297,7 +300,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance() + Factory.GetInstance(), + Factory.GetInstance() ); return usersController; } @@ -487,7 +491,8 @@ namespace Umbraco.Tests.Web.Controllers Factory.GetInstance(), Factory.GetInstance(), Factory.GetInstance(), - Factory.GetInstance()); + Factory.GetInstance(), + Factory.GetInstance()); var mockOwinContext = new Mock(); var mockUserManagerMarker = new Mock(); diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 50e9921154..01e13ff051 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -48,6 +48,7 @@ namespace Umbraco.Web.Editors private readonly IHostingEnvironment _hostingEnvironment; private readonly IRuntimeState _runtimeState; private readonly ISecuritySettings _securitySettings; + private readonly IEmailSender _emailSender; public AuthenticationController( IUserPasswordConfiguration passwordConfiguration, @@ -61,13 +62,15 @@ namespace Umbraco.Web.Editors IRuntimeState runtimeState, UmbracoMapper umbracoMapper, ISecuritySettings securitySettings, - IPublishedUrlProvider publishedUrlProvider) + IPublishedUrlProvider publishedUrlProvider, + IEmailSender emailSender) : base(globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoMapper, publishedUrlProvider) { _passwordConfiguration = passwordConfiguration ?? throw new ArgumentNullException(nameof(passwordConfiguration)); _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment)); _runtimeState = runtimeState ?? throw new ArgumentNullException(nameof(runtimeState)); _securitySettings = securitySettings ?? throw new ArgumentNullException(nameof(securitySettings)); + _emailSender = emailSender; } protected BackOfficeUserManager UserManager => _userManager @@ -333,16 +336,15 @@ namespace Umbraco.Web.Editors // Ensure the culture of the found user is used for the email! UmbracoUserExtensions.GetUserCulture(identityUser.Culture, Services.TextService, GlobalSettings)); - var emailSender = new EmailSender(GlobalSettings, true); var mailMessage = new MailMessage() { Subject = subject, Body = message, - IsBodyHtml = true + IsBodyHtml = true, + To = { user.Email} }; - mailMessage.To.Add(user.Email); - await emailSender.SendAsync(mailMessage); + await _emailSender.SendAsync(mailMessage); UserManager.RaiseForgotPasswordRequestedEvent(user.Id); } diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 029612dc90..3c71245a0a 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -53,6 +53,7 @@ namespace Umbraco.Web.Editors private readonly ISqlContext _sqlContext; private readonly IImageUrlGenerator _imageUrlGenerator; private readonly ISecuritySettings _securitySettings; + private readonly IEmailSender _emailSender; public UsersController( IGlobalSettings globalSettings, @@ -69,7 +70,8 @@ namespace Umbraco.Web.Editors IHostingEnvironment hostingEnvironment, IImageUrlGenerator imageUrlGenerator, IPublishedUrlProvider publishedUrlProvider, - ISecuritySettings securitySettings) + ISecuritySettings securitySettings, + IEmailSender emailSender) : base(globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, shortStringHelper, umbracoMapper, publishedUrlProvider) { _mediaFileSystem = mediaFileSystem; @@ -78,6 +80,7 @@ namespace Umbraco.Web.Editors _sqlContext = sqlContext; _imageUrlGenerator = imageUrlGenerator; _securitySettings = securitySettings; + _emailSender = emailSender; } /// @@ -501,16 +504,15 @@ namespace Umbraco.Web.Editors UmbracoUserExtensions.GetUserCulture(to.Language, Services.TextService, GlobalSettings), new[] { userDisplay.Name, from, message, inviteUri.ToString(), fromEmail }); - var emailSender = new EmailSender(GlobalSettings, true); var mailMessage = new MailMessage() { Subject = emailSubject, Body = emailBody, - IsBodyHtml = true + IsBodyHtml = true, + To = { to.Email} }; - mailMessage.To.Add(to.Email); - await emailSender.SendAsync(mailMessage); + await _emailSender.SendAsync(mailMessage); } /// From 490ae37b28571996da37cdf0f932318d317a9c21 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 20 May 2020 12:38:21 +0200 Subject: [PATCH 10/10] #8142 - Added smtp authentication if username or password provided --- src/Umbraco.Infrastructure/Users/EmailSender.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Umbraco.Infrastructure/Users/EmailSender.cs b/src/Umbraco.Infrastructure/Users/EmailSender.cs index 315a1748c3..9a2f425021 100644 --- a/src/Umbraco.Infrastructure/Users/EmailSender.cs +++ b/src/Umbraco.Infrastructure/Users/EmailSender.cs @@ -48,7 +48,15 @@ namespace Umbraco.Core { using (var client = new SmtpClient()) { + client.Connect(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); + + if (!(_globalSettings.SmtpSettings.Username is null && + _globalSettings.SmtpSettings.Password is null)) + { + client.Authenticate(_globalSettings.SmtpSettings.Username, _globalSettings.SmtpSettings.Password); + } + client.Send(ConstructEmailMessage(message)); client.Disconnect(true); } @@ -72,6 +80,12 @@ namespace Umbraco.Core { await client.ConnectAsync(_globalSettings.SmtpSettings.Host, _globalSettings.SmtpSettings.Port); + if (!(_globalSettings.SmtpSettings.Username is null && + _globalSettings.SmtpSettings.Password is null)) + { + await client.AuthenticateAsync(_globalSettings.SmtpSettings.Username, _globalSettings.SmtpSettings.Password); + } + var mailMessage = ConstructEmailMessage(message); if (_globalSettings.SmtpSettings.DeliveryMethod == SmtpDeliveryMethod.Network) {