From 8cc8736abd8f088f8662fd5f067d7436ec34f3f3 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 16 Feb 2023 10:29:17 +0100 Subject: [PATCH 1/4] Updated miniprofiler and added a few configurations --- .../UmbracoBuilderExtensions.cs | 17 +++---- .../Profiler/ConfigureMiniProfilerOptions.cs | 44 +++++++++++++++++++ .../Profiler/WebProfiler.cs | 24 ++++++++-- .../Services/UserGroupServiceTests.cs | 32 ++++++++++++++ 4 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index dc94fa3dc841..e44a7449aa5a 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -109,7 +109,7 @@ public static IUmbracoBuilder AddUmbraco( services.ConfigureOptions(); services.ConfigureOptions(); - IProfiler profiler = GetWebProfiler(config); + IProfiler profiler = GetWebProfiler(config, httpContextAccessor); services.AddLogger(webHostEnvironment, config); @@ -200,15 +200,8 @@ public static IUmbracoBuilder AddUmbracoProfiler(this IUmbracoBuilder builder) { builder.Services.AddSingleton(); - builder.Services.AddMiniProfiler(options => - { - // WebProfiler determine and start profiling. We should not use the MiniProfilerMiddleware to also profile - options.ShouldProfile = request => false; - - // this is a default path and by default it performs a 'contains' check which will match our content controller - // (and probably other requests) and ignore them. - options.IgnoredPaths.Remove("/content/"); - }); + builder.Services.AddMiniProfiler(); + builder.Services.ConfigureOptions(); builder.AddNotificationHandler(); return builder; @@ -385,7 +378,7 @@ public static IUmbracoBuilder AddWebServer(this IUmbracoBuilder builder) return builder; } - private static IProfiler GetWebProfiler(IConfiguration config) + private static IProfiler GetWebProfiler(IConfiguration config, IHttpContextAccessor httpContextAccessor) { var isDebug = config.GetValue($"{Constants.Configuration.ConfigHosting}:Debug"); @@ -397,7 +390,7 @@ private static IProfiler GetWebProfiler(IConfiguration config) return new NoopProfiler(); } - var webProfiler = new WebProfiler(); + var webProfiler = new WebProfiler(httpContextAccessor); webProfiler.StartBoot(); return webProfiler; diff --git a/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs new file mode 100644 index 000000000000..1851fbfbbd6e --- /dev/null +++ b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs @@ -0,0 +1,44 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; +using StackExchange.Profiling; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Security; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Web.Common.Profiler; + +internal sealed class ConfigureMiniProfilerOptions : IConfigureOptions +{ + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + private readonly string _backOfficePath; + + public ConfigureMiniProfilerOptions( + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IOptions globalSettings, + IHostingEnvironment hostingEnvironment) + { + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + _backOfficePath = globalSettings.Value.GetBackOfficePath(hostingEnvironment); + } + + public void Configure(MiniProfilerOptions options) + { + options.RouteBasePath = WebPath.Combine(_backOfficePath, "profiler"); + // WebProfiler determine and start profiling. We should not use the MiniProfilerMiddleware to also profile + options.ShouldProfile = request => false; + + // this is a default path and by default it performs a 'contains' check which will match our content controller + // (and probably other requests) and ignore them. + options.IgnoredPaths.Remove("/content/"); + options.IgnoredPaths.Add(WebPath.Combine(_backOfficePath, "swagger")); + options.IgnoredPaths.Add(WebPath.Combine(options.RouteBasePath, "results-list")); + options.IgnoredPaths.Add(WebPath.Combine(options.RouteBasePath, "results")); + + options.ResultsAuthorize = IsBackofficeUserAuthorized; + options.ResultsListAuthorize = IsBackofficeUserAuthorized; + } + + private bool IsBackofficeUserAuthorized(HttpRequest request) => true;//_backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is not null; +} diff --git a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs index 9608bad71521..50a0de19a9c9 100644 --- a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs +++ b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs @@ -1,6 +1,7 @@ using System.Net; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using StackExchange.Profiling; @@ -14,6 +15,14 @@ namespace Umbraco.Cms.Web.Common.Profiler; public class WebProfiler : IProfiler { + private readonly IHttpContextAccessor _httpContextAccessor; + + public WebProfiler(IHttpContextAccessor httpContextAccessor) + { + _httpContextAccessor = httpContextAccessor; + } + + public static readonly AsyncLocal MiniProfilerContext = new(x => { _ = x; @@ -24,11 +33,12 @@ public class WebProfiler : IProfiler private int _first; private MiniProfiler? _startupProfiler; - public IDisposable? Step(string name) => MiniProfiler.Current?.Step(name); + public IDisposable? Step(string name) => + MiniProfiler.Current?.Step(name); public void Start() { - MiniProfiler.StartNew(); + MiniProfiler.StartNew(_httpContextAccessor.HttpContext?.Request.Path); MiniProfilerContext.Value = MiniProfiler.Current; } @@ -75,7 +85,6 @@ public void UmbracoApplicationEndRequest(HttpContext context, RuntimeLevel runti { AddSubProfiler(_startupProfiler); } - _startupProfiler = null; } @@ -102,13 +111,20 @@ public void UmbracoApplicationEndRequest(HttpContext context, RuntimeLevel runti private static ICookieManager GetCookieManager(HttpContext context) => context.RequestServices.GetRequiredService(); - private static bool ShouldProfile(HttpRequest request) + private bool ShouldProfile(HttpRequest request) { if (request.IsClientSideRequest()) { return false; } + var miniprofilerOptions = _httpContextAccessor.HttpContext?.RequestServices?.GetService>(); + if (miniprofilerOptions is not null && miniprofilerOptions.Value.IgnoredPaths.Contains(request.Path)) + { + return false; + } + + if (bool.TryParse(request.Query["umbDebug"], out var umbDebug)) { return umbDebug; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs new file mode 100644 index 000000000000..964979ca02a6 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs @@ -0,0 +1,32 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; + + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +public class UserGroupServiceTests : UmbracoIntegrationTest +{ + private IUserGroupService UserGroupService => GetRequiredService(); + private IShortStringHelper ShortStringHelper => GetRequiredService(); + + [Test] + public async Task Cannot_create_user_group_with_name_equals_null() + { + + var userGroup = new UserGroup(ShortStringHelper) + { + Name = null + }; + + var result = await UserGroupService.CreateAsync(userGroup, Constants.Security.SuperUserId); + + Assert.IsFalse(result.Success); + } +} From a83b0b9d77242f789cdae97173511d6744af73b8 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 16 Feb 2023 11:04:59 +0100 Subject: [PATCH 2/4] added fixme --- src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs index 1851fbfbbd6e..e541a339345f 100644 --- a/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs +++ b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs @@ -40,5 +40,5 @@ public void Configure(MiniProfilerOptions options) options.ResultsListAuthorize = IsBackofficeUserAuthorized; } - private bool IsBackofficeUserAuthorized(HttpRequest request) => true;//_backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is not null; + private bool IsBackofficeUserAuthorized(HttpRequest request) => true;// FIXME when we can get current backoffice user, _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is not null; } From ba09c9eb55afff060954834c58c62b55c8cc2696 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 16 Feb 2023 11:08:32 +0100 Subject: [PATCH 3/4] Remove file that should not have been committed --- .../Services/UserGroupServiceTests.cs | 32 ------------------- 1 file changed, 32 deletions(-) delete mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs deleted file mode 100644 index 964979ca02a6..000000000000 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs +++ /dev/null @@ -1,32 +0,0 @@ -using NUnit.Framework; -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Models.Membership; -using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Core.Strings; -using Umbraco.Cms.Tests.Common.Testing; -using Umbraco.Cms.Tests.Integration.Testing; - -namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; - - -[TestFixture] -[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] -public class UserGroupServiceTests : UmbracoIntegrationTest -{ - private IUserGroupService UserGroupService => GetRequiredService(); - private IShortStringHelper ShortStringHelper => GetRequiredService(); - - [Test] - public async Task Cannot_create_user_group_with_name_equals_null() - { - - var userGroup = new UserGroup(ShortStringHelper) - { - Name = null - }; - - var result = await UserGroupService.CreateAsync(userGroup, Constants.Security.SuperUserId); - - Assert.IsFalse(result.Success); - } -} From 457cc10e79ddb0e27b727307c83accb2e55eb335 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 16 Feb 2023 11:21:40 +0100 Subject: [PATCH 4/4] added ignore list. We check the entire path and ignore client requests anyway --- .../Profiler/ConfigureMiniProfilerOptions.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs index e541a339345f..4239ba1737f3 100644 --- a/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs +++ b/src/Umbraco.Web.Common/Profiler/ConfigureMiniProfilerOptions.cs @@ -29,11 +29,10 @@ public void Configure(MiniProfilerOptions options) // WebProfiler determine and start profiling. We should not use the MiniProfilerMiddleware to also profile options.ShouldProfile = request => false; - // this is a default path and by default it performs a 'contains' check which will match our content controller - // (and probably other requests) and ignore them. - options.IgnoredPaths.Remove("/content/"); + options.IgnoredPaths.Clear(); options.IgnoredPaths.Add(WebPath.Combine(_backOfficePath, "swagger")); options.IgnoredPaths.Add(WebPath.Combine(options.RouteBasePath, "results-list")); + options.IgnoredPaths.Add(WebPath.Combine(options.RouteBasePath, "results-index")); options.IgnoredPaths.Add(WebPath.Combine(options.RouteBasePath, "results")); options.ResultsAuthorize = IsBackofficeUserAuthorized;