From 6148336d04e0132b535c3e8199a7cd2d618ac8de Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Mar 2021 12:51:07 +1100 Subject: [PATCH] Adds new event so we know when umbraco routes a value, ensure the IUmbracoWebsiteSecurity is initialized for front-end requests, cleans up some of the routing middleware, adds lots of notes --- .../Events/UmbracoRequestBegin.cs | 2 +- .../Events/UmbracoRoutedRequest.cs | 33 +++++++++++++ src/Umbraco.Core/HybridAccessorBase.cs | 45 ++++++++++++------ .../HybridEventMessagesAccessor.cs | 4 +- .../HybridVariationContextAccessor.cs | 5 +- .../HybridBackofficeSecurityAccessor.cs | 5 +- .../HybridUmbracoWebsiteSecurityAccessor.cs | 5 +- .../IBackOfficeSecurityFactory.cs} | 2 +- .../Security/IUmbracoWebsiteSecurity.cs | 19 ++------ .../Web/HybridUmbracoContextAccessor.cs | 5 +- .../HostedServices/ScheduledPublishing.cs | 7 ++- .../UmbracoTestServerTestBase.cs | 6 ++- .../Testing/UmbracoIntegrationTest.cs | 1 + .../Filters/ContentModelValidatorTests.cs | 2 +- .../ScheduledPublishingTests.cs | 1 + .../UmbracoBuilderExtensions.cs | 2 + .../Middleware/UmbracoRequestMiddleware.cs | 30 ++++++------ .../Security/BackofficeSecurityFactory.cs | 10 ++-- .../Security/UmbracoWebsiteSecurity.cs | 9 ++-- .../Security/UmbracoWebsiteSecurityFactory.cs | 46 +++++++++++++++++++ .../Controllers/UmbLoginController.cs | 12 +++-- .../UmbracoBuilderExtensions.cs | 4 +- .../Routing/UmbracoRouteValueTransformer.cs | 10 +++- 23 files changed, 180 insertions(+), 85 deletions(-) create mode 100644 src/Umbraco.Core/Events/UmbracoRoutedRequest.cs rename src/Umbraco.Core/{IBackofficeSecurityFactory.cs => Security/IBackOfficeSecurityFactory.cs} (91%) rename src/{Umbraco.Web.Website => Umbraco.Web.Common}/Security/UmbracoWebsiteSecurity.cs (97%) create mode 100644 src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs diff --git a/src/Umbraco.Core/Events/UmbracoRequestBegin.cs b/src/Umbraco.Core/Events/UmbracoRequestBegin.cs index ffb55938b3..00eb41df96 100644 --- a/src/Umbraco.Core/Events/UmbracoRequestBegin.cs +++ b/src/Umbraco.Core/Events/UmbracoRequestBegin.cs @@ -1,4 +1,4 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using Umbraco.Cms.Core.Web; diff --git a/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs b/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs new file mode 100644 index 0000000000..dd2b4d0d58 --- /dev/null +++ b/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs @@ -0,0 +1,33 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; +using Umbraco.Cms.Core.Web; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.Events +{ + /// + /// Notification raised when Umbraco routes a front-end request. + /// + public class UmbracoRoutedRequest : INotification + { + /// + /// Initializes a new instance of the class. + /// + public UmbracoRoutedRequest(IUmbracoContext umbracoContext) + { + if (!umbracoContext.IsFrontEndUmbracoRequest()) + { + throw new InvalidOperationException($"{nameof(UmbracoRoutedRequest)} is only valid for Umbraco front-end requests"); + } + + UmbracoContext = umbracoContext; + } + + /// + /// Gets the + /// + public IUmbracoContext UmbracoContext { get; } + } +} diff --git a/src/Umbraco.Core/HybridAccessorBase.cs b/src/Umbraco.Core/HybridAccessorBase.cs index ae3b4471e9..9843efdfe1 100644 --- a/src/Umbraco.Core/HybridAccessorBase.cs +++ b/src/Umbraco.Core/HybridAccessorBase.cs @@ -1,4 +1,4 @@ -using System; +using System; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Scoping; @@ -18,12 +18,15 @@ namespace Umbraco.Cms.Core { private readonly IRequestCache _requestCache; + // TODO: Do they need to be static?? These are singleton instances IMO they shouldn't be static // ReSharper disable StaticMemberInGenericType - private static readonly object Locker = new object(); - private static bool _registered; + private static readonly object s_locker = new object(); + private static bool s_registered; // ReSharper restore StaticMemberInGenericType - protected abstract string ItemKey { get; } + private string _itemKey; + + protected string ItemKey => _itemKey ?? (_itemKey = GetType().FullName); // read // http://blog.stephencleary.com/2013/04/implicit-async-context-asynclocal.html @@ -43,34 +46,42 @@ namespace Umbraco.Cms.Core private T NonContextValue { get => CallContext.GetData(ItemKey); - set - { - CallContext.SetData(ItemKey, value); - } + set => CallContext.SetData(ItemKey, value); } protected HybridAccessorBase(IRequestCache requestCache) { _requestCache = requestCache ?? throw new ArgumentNullException(nameof(requestCache)); - lock (Locker) + lock (s_locker) { // register the itemKey once with SafeCallContext - if (_registered) return; - _registered = true; + if (s_registered) + { + return; + } + + s_registered = true; } // ReSharper disable once VirtualMemberCallInConstructor var itemKey = ItemKey; // virtual SafeCallContext.Register(() => { - var value = CallContext.GetData(itemKey); + T value = CallContext.GetData(itemKey); return value; }, o => { - if (o == null) return; - var value = o as T; - if (value == null) throw new ArgumentException($"Expected type {typeof(T).FullName}, got {o.GetType().FullName}", nameof(o)); + if (o == null) + { + return; + } + + if (!(o is T value)) + { + throw new ArgumentException($"Expected type {typeof(T).FullName}, got {o.GetType().FullName}", nameof(o)); + } + CallContext.SetData(itemKey, value); }); } @@ -93,9 +104,13 @@ namespace Umbraco.Cms.Core NonContextValue = value; } else if (value == null) + { _requestCache.Remove(ItemKey); + } else + { _requestCache.Set(ItemKey, value); + } } } } diff --git a/src/Umbraco.Core/HybridEventMessagesAccessor.cs b/src/Umbraco.Core/HybridEventMessagesAccessor.cs index 6f4d33a307..df6a34aae8 100644 --- a/src/Umbraco.Core/HybridEventMessagesAccessor.cs +++ b/src/Umbraco.Core/HybridEventMessagesAccessor.cs @@ -1,12 +1,10 @@ -using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Events; namespace Umbraco.Cms.Core { public class HybridEventMessagesAccessor : HybridAccessorBase, IEventMessagesAccessor { - protected override string ItemKey => "Umbraco.Core.Events.HybridEventMessagesAccessor"; - public HybridEventMessagesAccessor(IRequestCache requestCache) : base(requestCache) { } diff --git a/src/Umbraco.Core/Models/PublishedContent/HybridVariationContextAccessor.cs b/src/Umbraco.Core/Models/PublishedContent/HybridVariationContextAccessor.cs index c412a4de3a..9c50b60ac1 100644 --- a/src/Umbraco.Core/Models/PublishedContent/HybridVariationContextAccessor.cs +++ b/src/Umbraco.Core/Models/PublishedContent/HybridVariationContextAccessor.cs @@ -1,4 +1,4 @@ -using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Cache; namespace Umbraco.Cms.Core.Models.PublishedContent { @@ -11,9 +11,6 @@ namespace Umbraco.Cms.Core.Models.PublishedContent : base(requestCache) { } - /// - protected override string ItemKey => "Umbraco.Web.HybridVariationContextAccessor"; - /// /// Gets or sets the object. /// diff --git a/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs b/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs index 990715ce39..924f0a31a6 100644 --- a/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs +++ b/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs @@ -1,4 +1,4 @@ -using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Cache; namespace Umbraco.Cms.Core.Security { @@ -11,9 +11,6 @@ namespace Umbraco.Cms.Core.Security : base(requestCache) { } - /// - protected override string ItemKey => "Umbraco.Web.HybridBackofficeSecurityAccessor"; - /// /// Gets or sets the object. /// diff --git a/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs index cb986588d3..3145f400d1 100644 --- a/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs +++ b/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs @@ -1,4 +1,4 @@ -using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Cache; namespace Umbraco.Cms.Core.Security { @@ -12,9 +12,6 @@ namespace Umbraco.Cms.Core.Security : base(requestCache) { } - /// - protected override string ItemKey => "Umbraco.Web.HybridUmbracoWebsiteSecurityAccessor"; - /// /// Gets or sets the object. /// diff --git a/src/Umbraco.Core/IBackofficeSecurityFactory.cs b/src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs similarity index 91% rename from src/Umbraco.Core/IBackofficeSecurityFactory.cs rename to src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs index ac7c875f16..423332ed42 100644 --- a/src/Umbraco.Core/IBackofficeSecurityFactory.cs +++ b/src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs @@ -1,4 +1,4 @@ -namespace Umbraco.Cms.Core +namespace Umbraco.Core.Security { /// /// Creates and manages instances. diff --git a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs index 86dbb9683e..10fb9b5f2c 100644 --- a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs +++ b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs @@ -1,9 +1,10 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.Threading.Tasks; using Umbraco.Cms.Core.Models.Security; namespace Umbraco.Cms.Core.Security { + // TODO: I think we can kill this whole thing, the logic can just be in the controllers public interface IUmbracoWebsiteSecurity { /// @@ -35,18 +36,10 @@ namespace Umbraco.Cms.Core.Security /// Result of update profile operation. Task UpdateMemberProfileAsync(ProfileModel model); - /// - /// A helper method to perform the validation and logging in of a member. - /// - /// The username. - /// The password. - /// Result of login operation. + // TODO: Kill this, we will just use the MemberManager / MemberSignInManager Task LoginAsync(string username, string password); - /// - /// Check if a member is logged in - /// - /// True if logged in, false if not. + // TODO: Kill this, we will just use the MemberManager bool IsLoggedIn(); /// @@ -55,9 +48,7 @@ namespace Umbraco.Cms.Core.Security /// Instance of Task GetCurrentLoginStatusAsync(); - /// - /// Logs out the current member. - /// + // TODO: Kill this, we will just use the MemberManager Task LogOutAsync(); /// diff --git a/src/Umbraco.Core/Web/HybridUmbracoContextAccessor.cs b/src/Umbraco.Core/Web/HybridUmbracoContextAccessor.cs index a266c07769..503bb25b57 100644 --- a/src/Umbraco.Core/Web/HybridUmbracoContextAccessor.cs +++ b/src/Umbraco.Core/Web/HybridUmbracoContextAccessor.cs @@ -1,4 +1,4 @@ -using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Cache; namespace Umbraco.Cms.Core.Web { @@ -14,9 +14,6 @@ namespace Umbraco.Cms.Core.Web : base(requestCache) { } - /// - protected override string ItemKey => "Umbraco.Web.HybridUmbracoContextAccessor"; - /// /// Gets or sets the object. /// diff --git a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs index bc81bfabcf..f5f5ee22a2 100644 --- a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs +++ b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs @@ -12,6 +12,7 @@ using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Core.Web; +using Umbraco.Core.Security; namespace Umbraco.Cms.Infrastructure.HostedServices { @@ -106,7 +107,11 @@ namespace Umbraco.Cms.Infrastructure.HostedServices // - batched messenger should not depend on a current HttpContext // but then what should be its "scope"? could we attach it to scopes? // - and we should definitively *not* have to flush it here (should be auto) - // + + // TODO: This dependency chain is broken and needs to be fixed. + // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null + // This is a very ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the + // way it was intended. _backofficeSecurityFactory.EnsureBackOfficeSecurity(); using UmbracoContextReference contextReference = _umbracoContextFactory.EnsureUmbracoContext(); try diff --git a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs index caed95ae52..60a48f3f4c 100644 --- a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs +++ b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs @@ -14,7 +14,6 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using NUnit.Framework; -using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Composing; using Umbraco.Cms.Core.DependencyInjection; @@ -25,6 +24,7 @@ using Umbraco.Cms.Tests.Integration.Testing; using Umbraco.Cms.Web.BackOffice.Controllers; using Umbraco.Cms.Web.Common.Controllers; using Umbraco.Cms.Web.Website.Controllers; +using Umbraco.Core.Security; using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; @@ -122,6 +122,10 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest } }; + // TODO: This dependency chain is broken and needs to be fixed. + // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null + // This is a very ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the + // way it was intended. backofficeSecurityFactory.EnsureBackOfficeSecurity(); umbracoContextFactory.EnsureUmbracoContext(); diff --git a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index eab085f67e..d3625d109a 100644 --- a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -36,6 +36,7 @@ using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.DependencyInjection; using Umbraco.Cms.Tests.Integration.Extensions; using Umbraco.Cms.Tests.Integration.Implementations; +using Umbraco.Core.Security; using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; diff --git a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs index 5cf0dc7b28..3e54039714 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs @@ -11,7 +11,6 @@ using Microsoft.Extensions.Logging; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using NUnit.Framework; -using Umbraco.Cms.Core; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; @@ -25,6 +24,7 @@ using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; using Umbraco.Cms.Web.BackOffice.Filters; using Umbraco.Cms.Web.BackOffice.ModelBinders; +using Umbraco.Core.Security; using Umbraco.Extensions; using DataType = Umbraco.Cms.Core.Models.DataType; diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs index 62ee1fdd29..3782a8aece 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs @@ -13,6 +13,7 @@ using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure; using Umbraco.Cms.Infrastructure.HostedServices; +using Umbraco.Core.Security; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices { diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index 9e2e79cb9b..1d636706f7 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -53,6 +53,7 @@ using Umbraco.Cms.Web.Common.Routing; using Umbraco.Cms.Web.Common.Security; using Umbraco.Cms.Web.Common.Templates; using Umbraco.Cms.Web.Common.UmbracoContext; +using Umbraco.Core.Security; using IHostingEnvironment = Umbraco.Cms.Core.Hosting.IHostingEnvironment; namespace Umbraco.Extensions @@ -263,6 +264,7 @@ namespace Umbraco.Extensions builder.Services.AddUnique(); builder.Services.AddUnique(); builder.Services.AddUnique(); + builder.AddNotificationHandler(); builder.Services.AddUnique(); var umbracoApiControllerTypes = builder.TypeLoader.GetUmbracoApiControllers().ToList(); diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs index 6fdd41a757..2515948bea 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs @@ -10,9 +10,11 @@ using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Logging; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.PublishedCache; using Umbraco.Cms.Web.Common.Profiler; +using Umbraco.Core.Security; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Middleware @@ -83,27 +85,26 @@ namespace Umbraco.Cms.Web.Common.Middleware EnsureContentCacheInitialized(); - _backofficeSecurityFactory.EnsureBackOfficeSecurity(); // Needs to be before UmbracoContext, TODO: Why? + // TODO: This dependency chain is broken and needs to be fixed. + // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null + // This is ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the + // way it was intended. + _backofficeSecurityFactory.EnsureBackOfficeSecurity(); UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(); Uri currentApplicationUrl = GetApplicationUrlFromCurrentRequest(context.Request); _hostingEnvironment.EnsureApplicationMainUrl(currentApplicationUrl); - - bool isFrontEndRequest = umbracoContextReference.UmbracoContext.IsFrontEndUmbracoRequest(); - var pathAndQuery = context.Request.GetEncodedPathAndQuery(); try { - if (isFrontEndRequest) - { - LogHttpRequest.TryGetCurrentHttpRequestId(out Guid httpRequestId, _requestCache); - _logger.LogTrace("Begin request [{HttpRequestId}]: {RequestUrl}", httpRequestId, pathAndQuery); - } + // Verbose log start of every request + LogHttpRequest.TryGetCurrentHttpRequestId(out Guid httpRequestId, _requestCache); + _logger.LogTrace("Begin request [{HttpRequestId}]: {RequestUrl}", httpRequestId, pathAndQuery); try - { + { await _eventAggregator.PublishAsync(new UmbracoRequestBegin(umbracoContextReference.UmbracoContext)); } catch (Exception ex) @@ -126,11 +127,10 @@ namespace Umbraco.Cms.Web.Common.Middleware } finally { - if (isFrontEndRequest) - { - LogHttpRequest.TryGetCurrentHttpRequestId(out Guid httpRequestId, _requestCache); - _logger.LogTrace("End Request [{HttpRequestId}]: {RequestUrl} ({RequestDuration}ms)", httpRequestId, pathAndQuery, DateTime.Now.Subtract(umbracoContextReference.UmbracoContext.ObjectCreated).TotalMilliseconds); - } + // Verbose log end of every request (in v8 we didn't log the end request of ALL requests, only the front-end which was + // strange since we always logged the beginning, so now we just log start/end of all requests) + LogHttpRequest.TryGetCurrentHttpRequestId(out Guid httpRequestId, _requestCache); + _logger.LogTrace("End Request [{HttpRequestId}]: {RequestUrl} ({RequestDuration}ms)", httpRequestId, pathAndQuery, DateTime.Now.Subtract(umbracoContextReference.UmbracoContext.ObjectCreated).TotalMilliseconds); try { diff --git a/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs b/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs index 41e7f6d816..eda7f4b98e 100644 --- a/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs +++ b/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs @@ -1,11 +1,11 @@ -using Microsoft.AspNetCore.Http; -using Umbraco.Cms.Core; +using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; +using Umbraco.Core.Security; namespace Umbraco.Cms.Web.Common.Security { - // TODO: This is only for the back office, does it need to be in common? + // TODO: This is only for the back office, does it need to be in common? YES currently UmbracoContext has an transitive dependency on this which needs to be fixed/reviewed. public class BackOfficeSecurityFactory: IBackOfficeSecurityFactory { @@ -14,11 +14,11 @@ namespace Umbraco.Cms.Web.Common.Security private readonly IHttpContextAccessor _httpContextAccessor; public BackOfficeSecurityFactory( - IBackOfficeSecurityAccessor backofficeSecurityAccessor, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IUserService userService, IHttpContextAccessor httpContextAccessor) { - _backOfficeSecurityAccessor = backofficeSecurityAccessor; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _userService = userService; _httpContextAccessor = httpContextAccessor; } diff --git a/src/Umbraco.Web.Website/Security/UmbracoWebsiteSecurity.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs similarity index 97% rename from src/Umbraco.Web.Website/Security/UmbracoWebsiteSecurity.cs rename to src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs index c878730d90..5a67f6f484 100644 --- a/src/Umbraco.Web.Website/Security/UmbracoWebsiteSecurity.cs +++ b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -11,9 +11,8 @@ using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; -using Constants = Umbraco.Cms.Core.Constants; -namespace Umbraco.Cms.Web.Website.Security +namespace Umbraco.Cms.Web.Common.Security { public class UmbracoWebsiteSecurity : IUmbracoWebsiteSecurity { @@ -36,7 +35,7 @@ namespace Umbraco.Cms.Web.Website.Security /// public RegisterModel CreateRegistrationModel(string memberTypeAlias = null) { - var providedOrDefaultMemberTypeAlias = memberTypeAlias ?? Constants.Conventions.MemberTypes.DefaultAlias; + var providedOrDefaultMemberTypeAlias = memberTypeAlias ?? Core.Constants.Conventions.MemberTypes.DefaultAlias; var memberType = _memberTypeService.Get(providedOrDefaultMemberTypeAlias); if (memberType == null) { @@ -114,7 +113,7 @@ namespace Umbraco.Cms.Web.Website.Security public Task RegisterMemberAsync(RegisterModel model, bool logMemberIn = true) { - throw new System.NotImplementedException(); + throw new NotImplementedException(); } /// diff --git a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs new file mode 100644 index 0000000000..ec256a86cb --- /dev/null +++ b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs @@ -0,0 +1,46 @@ +using Microsoft.AspNetCore.Http; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; + +namespace Umbraco.Cms.Web.Common.Security +{ + /// + /// Ensures that the is populated on a front-end request + /// + internal sealed class UmbracoWebsiteSecurityFactory : INotificationHandler + { + private readonly IUmbracoWebsiteSecurityAccessor _umbracoWebsiteSecurityAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IMemberService _memberService; + private readonly IMemberTypeService _memberTypeService; + private readonly IShortStringHelper _shortStringHelper; + + public UmbracoWebsiteSecurityFactory( + IUmbracoWebsiteSecurityAccessor umbracoWebsiteSecurityAccessor, + IHttpContextAccessor httpContextAccessor, + IMemberService memberService, + IMemberTypeService memberTypeService, + IShortStringHelper shortStringHelper) + { + _umbracoWebsiteSecurityAccessor = umbracoWebsiteSecurityAccessor; + _httpContextAccessor = httpContextAccessor; + _memberService = memberService; + _memberTypeService = memberTypeService; + _shortStringHelper = shortStringHelper; + } + + public void Handle(UmbracoRoutedRequest notification) + { + if (_umbracoWebsiteSecurityAccessor.WebsiteSecurity is null) + { + _umbracoWebsiteSecurityAccessor.WebsiteSecurity = new UmbracoWebsiteSecurity( + _httpContextAccessor, + _memberService, + _memberTypeService, + _shortStringHelper); + } + } + } +} diff --git a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs index 996103f9e5..93b1f23b76 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs @@ -18,8 +18,13 @@ namespace Umbraco.Cms.Web.Website.Controllers { private readonly IUmbracoWebsiteSecurityAccessor _websiteSecurityAccessor; - public UmbLoginController(IUmbracoContextAccessor umbracoContextAccessor, IUmbracoDatabaseFactory databaseFactory, - ServiceContext services, AppCaches appCaches, IProfilingLogger profilingLogger, IPublishedUrlProvider publishedUrlProvider, + public UmbLoginController( + IUmbracoContextAccessor umbracoContextAccessor, + IUmbracoDatabaseFactory databaseFactory, + ServiceContext services, + AppCaches appCaches, + IProfilingLogger profilingLogger, + IPublishedUrlProvider publishedUrlProvider, IUmbracoWebsiteSecurityAccessor websiteSecurityAccessor) : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider) { @@ -36,9 +41,6 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } - // TODO: This is supposed to be for members! not users - //throw new NotImplementedException("Implement this for members"); - if (await _websiteSecurityAccessor.WebsiteSecurity.LoginAsync(model.Username, model.Password) == false) { // Don't add a field level error, just model level. diff --git a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs index 3823cde255..58074ac9a5 100644 --- a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs @@ -2,8 +2,10 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.DependencyInjection; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Infrastructure.DependencyInjection; using Umbraco.Cms.Web.Common.Routing; +using Umbraco.Cms.Web.Common.Security; using Umbraco.Cms.Web.Website.Collections; using Umbraco.Cms.Web.Website.Controllers; using Umbraco.Cms.Web.Website.Routing; @@ -40,7 +42,7 @@ namespace Umbraco.Extensions builder.Services.AddSingleton(); builder.Services.AddSingleton(); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder .AddDistributedCache() diff --git a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs index d0e5d4c72a..13e70897ab 100644 --- a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs +++ b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs @@ -13,6 +13,7 @@ using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Services; @@ -48,6 +49,7 @@ namespace Umbraco.Cms.Web.Website.Routing private readonly IRoutableDocumentFilter _routableDocumentFilter; private readonly IDataProtectionProvider _dataProtectionProvider; private readonly IControllerActionSearcher _controllerActionSearcher; + private readonly IEventAggregator _eventAggregator; /// /// Initializes a new instance of the class. @@ -62,7 +64,8 @@ namespace Umbraco.Cms.Web.Website.Routing IUmbracoRouteValuesFactory routeValuesFactory, IRoutableDocumentFilter routableDocumentFilter, IDataProtectionProvider dataProtectionProvider, - IControllerActionSearcher controllerActionSearcher) + IControllerActionSearcher controllerActionSearcher, + IEventAggregator eventAggregator) { if (globalSettings is null) { @@ -79,6 +82,7 @@ namespace Umbraco.Cms.Web.Website.Routing _routableDocumentFilter = routableDocumentFilter ?? throw new ArgumentNullException(nameof(routableDocumentFilter)); _dataProtectionProvider = dataProtectionProvider; _controllerActionSearcher = controllerActionSearcher; + _eventAggregator = eventAggregator; } /// @@ -117,6 +121,10 @@ namespace Umbraco.Cms.Web.Website.Routing // Store the route values as a httpcontext feature httpContext.Features.Set(umbracoRouteValues); + // publish an event that we've routed a request + // TODO: does this occur on 404 or have we already returned? + await _eventAggregator.PublishAsync(new UmbracoRoutedRequest(_umbracoContextAccessor.UmbracoContext)); + // Need to check if there is form data being posted back to an Umbraco URL PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values); if (postedInfo != null)