From 2e4b31d8f6a8d956d88de492d78b2ede8652a515 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 1 Mar 2021 10:07:20 +0100 Subject: [PATCH 1/3] https://dev.azure.com/umbraco/D-Team%20Tracker/_workitems/edit/10710 No not use IBackOfficeSecurity in UmbracoContext. Not we use IHttpContextAccessor to get the info about whether the current user is null or not. It is expected to be null in background jobs. --- .../Objects/TestUmbracoContextFactory.cs | 6 +----- .../UmbracoContext/UmbracoContext.cs | 13 ++++++------- .../UmbracoContext/UmbracoContextFactory.cs | 12 ++++++------ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs index 15c2e91bc5..4c0578c0be 100644 --- a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs +++ b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs @@ -8,7 +8,6 @@ using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Tests.Common; using Umbraco.Cms.Web.Common.AspNetCore; @@ -57,9 +56,6 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects IHostingEnvironment hostingEnvironment = TestHelper.GetHostingEnvironment(); - var backofficeSecurityAccessorMock = new Mock(); - backofficeSecurityAccessorMock.Setup(x => x.BackOfficeSecurity).Returns(Mock.Of()); - var umbracoContextFactory = new UmbracoContextFactory( umbracoContextAccessor, snapshotService.Object, @@ -70,7 +66,7 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects new UriUtility(hostingEnvironment), new AspNetCoreCookieManager(httpContextAccessor), Mock.Of(), - backofficeSecurityAccessorMock.Object); + httpContextAccessor); return umbracoContextFactory; } diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs index c31fe4dd3e..901b3d613c 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs @@ -1,10 +1,10 @@ using System; +using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Extensions; @@ -19,10 +19,10 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private readonly UriUtility _uriUtility; private readonly ICookieManager _cookieManager; private readonly IRequestAccessor _requestAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly Lazy _publishedSnapshot; private string _previewToken; private bool? _previewing; - private readonly IBackOfficeSecurity _backofficeSecurity; private readonly UmbracoRequestPaths _umbracoRequestPaths; private Uri _originalRequestUrl; private Uri _cleanedUmbracoUrl; @@ -33,13 +33,13 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext // warn: does *not* manage setting any IUmbracoContextAccessor internal UmbracoContext( IPublishedSnapshotService publishedSnapshotService, - IBackOfficeSecurity backofficeSecurity, UmbracoRequestPaths umbracoRequestPaths, IHostingEnvironment hostingEnvironment, IVariationContextAccessor variationContextAccessor, UriUtility uriUtility, ICookieManager cookieManager, - IRequestAccessor requestAccessor) + IRequestAccessor requestAccessor, + IHttpContextAccessor httpContextAccessor) { if (publishedSnapshotService == null) { @@ -51,10 +51,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _hostingEnvironment = hostingEnvironment; _cookieManager = cookieManager; _requestAccessor = requestAccessor; - + _httpContextAccessor = httpContextAccessor; ObjectCreated = DateTime.Now; UmbracoRequestId = Guid.NewGuid(); - _backofficeSecurity = backofficeSecurity ?? throw new ArgumentNullException(nameof(backofficeSecurity)); _umbracoRequestPaths = umbracoRequestPaths; // beware - we cannot expect a current user here, so detecting preview mode must be a lazy thing @@ -143,7 +142,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext Uri requestUrl = _requestAccessor.GetRequestUrl(); if (requestUrl != null && _umbracoRequestPaths.IsBackOfficeRequest(requestUrl.AbsolutePath) == false - && _backofficeSecurity.CurrentUser != null) + && _httpContextAccessor.HttpContext?.GetCurrentIdentity() != null) { var previewToken = _cookieManager.GetCookieValue(Core.Constants.Web.PreviewCookieName); // may be null or empty _previewToken = previewToken.IsNullOrWhiteSpace() ? null : previewToken; diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs index 8d199febd0..fb94139144 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs @@ -1,10 +1,10 @@ using System; +using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; namespace Umbraco.Cms.Web.Common.UmbracoContext @@ -23,7 +23,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private readonly IHostingEnvironment _hostingEnvironment; private readonly ICookieManager _cookieManager; private readonly IRequestAccessor _requestAccessor; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly UriUtility _uriUtility; /// @@ -39,7 +39,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext UriUtility uriUtility, ICookieManager cookieManager, IRequestAccessor requestAccessor, - IBackOfficeSecurityAccessor backofficeSecurityAccessor) + IHttpContextAccessor httpContextAccessor) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _publishedSnapshotService = publishedSnapshotService ?? throw new ArgumentNullException(nameof(publishedSnapshotService)); @@ -50,7 +50,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _uriUtility = uriUtility ?? throw new ArgumentNullException(nameof(uriUtility)); _cookieManager = cookieManager ?? throw new ArgumentNullException(nameof(cookieManager)); _requestAccessor = requestAccessor ?? throw new ArgumentNullException(nameof(requestAccessor)); - _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); + _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); } private IUmbracoContext CreateUmbracoContext() @@ -75,13 +75,13 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext return new UmbracoContext( _publishedSnapshotService, - _backofficeSecurityAccessor.BackOfficeSecurity, _umbracoRequestPaths, _hostingEnvironment, _variationContextAccessor, _uriUtility, _cookieManager, - _requestAccessor); + _requestAccessor, + _httpContextAccessor); } /// From 1ef60a7c7dd5d63ad7b4805fc23f9b127d2777f0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 4 Mar 2021 16:44:09 +1100 Subject: [PATCH 2/3] removes unneeded ctor dependency on UmbracoContext --- .../Objects/TestUmbracoContextFactory.cs | 1 - .../Extensions/HttpContextExtensions.cs | 18 ++++++++++++++++-- .../UmbracoContext/UmbracoContext.cs | 19 ++++++++++--------- .../UmbracoContext/UmbracoContextFactory.cs | 5 ----- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs index 4c0578c0be..73a429753c 100644 --- a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs +++ b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs @@ -65,7 +65,6 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects hostingEnvironment, new UriUtility(hostingEnvironment), new AspNetCoreCookieManager(httpContextAccessor), - Mock.Of(), httpContextAccessor); return umbracoContextFactory; diff --git a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs index 9513eb2ec2..fbb9e77770 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs @@ -1,13 +1,27 @@ -using System; +using System; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; -using Umbraco.Cms.Core.Security; namespace Umbraco.Extensions { public static class HttpContextExtensions { + /// + /// Get the value in the request form or query string for the key + /// + public static string GetRequestValue(this HttpContext context, string key) + { + HttpRequest request = context.Request; + if (!request.HasFormContentType) + { + return request.Query[key]; + } + + string value = request.Form[key]; + return value ?? request.Query[key]; + } + public static void SetPrincipalForRequest(this HttpContext context, ClaimsPrincipal principal) { context.User = principal; diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs index 901b3d613c..9589c9e545 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs @@ -1,5 +1,6 @@ using System; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models.PublishedContent; @@ -18,12 +19,12 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private readonly IHostingEnvironment _hostingEnvironment; private readonly UriUtility _uriUtility; private readonly ICookieManager _cookieManager; - private readonly IRequestAccessor _requestAccessor; private readonly IHttpContextAccessor _httpContextAccessor; private readonly Lazy _publishedSnapshot; private string _previewToken; private bool? _previewing; private readonly UmbracoRequestPaths _umbracoRequestPaths; + private Uri _requestUrl; private Uri _originalRequestUrl; private Uri _cleanedUmbracoUrl; @@ -38,7 +39,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext IVariationContextAccessor variationContextAccessor, UriUtility uriUtility, ICookieManager cookieManager, - IRequestAccessor requestAccessor, IHttpContextAccessor httpContextAccessor) { if (publishedSnapshotService == null) @@ -50,7 +50,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _uriUtility = uriUtility; _hostingEnvironment = hostingEnvironment; _cookieManager = cookieManager; - _requestAccessor = requestAccessor; _httpContextAccessor = httpContextAccessor; ObjectCreated = DateTime.Now; UmbracoRequestId = Guid.NewGuid(); @@ -71,6 +70,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext /// internal Guid UmbracoRequestId { get; } + // lazily get/create a Uri for the current request + private Uri RequestUrl => _requestUrl ?? (_requestUrl = new Uri(_httpContextAccessor.HttpContext.Request.GetEncodedUrl())); + /// // set the urls lazily, no need to allocate until they are needed... // NOTE: The request will not be available during app startup so we can only set this to an absolute URL of localhost, this @@ -78,7 +80,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext // 'could' still generate URLs during startup BUT any domain driven URL generation will not work because it is NOT possible to get // the current domain during application startup. // see: http://issues.umbraco.org/issue/U4-1890 - public Uri OriginalRequestUrl => _originalRequestUrl ?? (_originalRequestUrl = _requestAccessor.GetRequestUrl() ?? new Uri("http://localhost")); + public Uri OriginalRequestUrl => _originalRequestUrl ?? (_originalRequestUrl = RequestUrl ?? new Uri("http://localhost")); /// // set the urls lazily, no need to allocate until they are needed... @@ -105,8 +107,8 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext /// public bool IsDebug => // NOTE: the request can be null during app startup! _hostingEnvironment.IsDebugMode - && (string.IsNullOrEmpty(_requestAccessor.GetRequestValue("umbdebugshowtrace")) == false - || string.IsNullOrEmpty(_requestAccessor.GetRequestValue("umbdebug")) == false + && (string.IsNullOrEmpty(_httpContextAccessor.HttpContext.GetRequestValue("umbdebugshowtrace")) == false + || string.IsNullOrEmpty(_httpContextAccessor.HttpContext.GetRequestValue("umbdebug")) == false || string.IsNullOrEmpty(_cookieManager.GetCookieValue("UMB-DEBUG")) == false); /// @@ -139,9 +141,8 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private void DetectPreviewMode() { - Uri requestUrl = _requestAccessor.GetRequestUrl(); - if (requestUrl != null - && _umbracoRequestPaths.IsBackOfficeRequest(requestUrl.AbsolutePath) == false + if (RequestUrl != null + && _umbracoRequestPaths.IsBackOfficeRequest(RequestUrl.AbsolutePath) == false && _httpContextAccessor.HttpContext?.GetCurrentIdentity() != null) { var previewToken = _cookieManager.GetCookieValue(Core.Constants.Web.PreviewCookieName); // may be null or empty diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs index fb94139144..b41d96e0d0 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs @@ -18,11 +18,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private readonly IPublishedSnapshotService _publishedSnapshotService; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IDefaultCultureAccessor _defaultCultureAccessor; - private readonly UmbracoRequestPaths _umbracoRequestPaths; private readonly IHostingEnvironment _hostingEnvironment; private readonly ICookieManager _cookieManager; - private readonly IRequestAccessor _requestAccessor; private readonly IHttpContextAccessor _httpContextAccessor; private readonly UriUtility _uriUtility; @@ -38,7 +36,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext IHostingEnvironment hostingEnvironment, UriUtility uriUtility, ICookieManager cookieManager, - IRequestAccessor requestAccessor, IHttpContextAccessor httpContextAccessor) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); @@ -49,7 +46,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment)); _uriUtility = uriUtility ?? throw new ArgumentNullException(nameof(uriUtility)); _cookieManager = cookieManager ?? throw new ArgumentNullException(nameof(cookieManager)); - _requestAccessor = requestAccessor ?? throw new ArgumentNullException(nameof(requestAccessor)); _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); } @@ -80,7 +76,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _variationContextAccessor, _uriUtility, _cookieManager, - _requestAccessor, _httpContextAccessor); } From 6d293c6ffd783ffe8ec8a2bcb510fb398523bf6c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 8 Mar 2021 21:37:31 +0100 Subject: [PATCH 3/3] Fixed null ref exception --- src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs index 9589c9e545..83821cbab5 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs @@ -71,7 +71,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext internal Guid UmbracoRequestId { get; } // lazily get/create a Uri for the current request - private Uri RequestUrl => _requestUrl ?? (_requestUrl = new Uri(_httpContextAccessor.HttpContext.Request.GetEncodedUrl())); + private Uri RequestUrl => _requestUrl ??= _httpContextAccessor.HttpContext is null + ? null + : new Uri(_httpContextAccessor.HttpContext.Request.GetEncodedUrl()); /// // set the urls lazily, no need to allocate until they are needed...