From 2f17d766be1894340637a8f2f1a7ebaf7cab2638 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 26 Jan 2022 10:57:49 +0100 Subject: [PATCH 1/3] Cherry pick Add allowlist for HelpPage --- src/Umbraco.Core/Help/HelpPageSettings.cs | 12 ++++++++++ src/Umbraco.Core/Help/IHelpPageSettings.cs | 10 ++++++++ .../Controllers/HelpController.cs | 24 +++++++++++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 src/Umbraco.Core/Help/HelpPageSettings.cs create mode 100644 src/Umbraco.Core/Help/IHelpPageSettings.cs diff --git a/src/Umbraco.Core/Help/HelpPageSettings.cs b/src/Umbraco.Core/Help/HelpPageSettings.cs new file mode 100644 index 0000000000..d2a4a3a0f5 --- /dev/null +++ b/src/Umbraco.Core/Help/HelpPageSettings.cs @@ -0,0 +1,12 @@ +using System.Configuration; + +namespace Umbraco.Core.Help +{ + public class HelpPageSettings : IHelpPageSettings + { + public string HelpPageUrlAllowList => + ConfigurationManager.AppSettings.ContainsKey(Constants.AppSettings.HelpPageUrlAllowList) + ? ConfigurationManager.AppSettings[Constants.AppSettings.HelpPageUrlAllowList] + : null; + } +} diff --git a/src/Umbraco.Core/Help/IHelpPageSettings.cs b/src/Umbraco.Core/Help/IHelpPageSettings.cs new file mode 100644 index 0000000000..5643e47a30 --- /dev/null +++ b/src/Umbraco.Core/Help/IHelpPageSettings.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Core.Help +{ + public interface IHelpPageSettings + { + /// + /// Gets the allowed addresses to retrieve data for the help page. + /// + string HelpPageUrlAllowList { get; } + } +} diff --git a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs index 3bc45703fa..ecec8f864d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs @@ -1,10 +1,11 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.Net.Http; using System.Runtime.Serialization; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Newtonsoft.Json; using Umbraco.Cms.Web.Common.Attributes; +using Umbraco.Core.Help; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Web.BackOffice.Controllers @@ -13,8 +14,10 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers public class HelpController : UmbracoAuthorizedJsonController { private readonly ILogger _logger; + private readonly IHelpPageSettings _helpPageSettings; - public HelpController(ILogger logger) + public HelpController(ILogger logger, + IHelpPageSettings helpPageSettings) { _logger = logger; } @@ -22,6 +25,12 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers private static HttpClient _httpClient; public async Task> GetContextHelpForPage(string section, string tree, string baseUrl = "https://our.umbraco.com") { + if (IsAllowedUrl(baseUrl) is false) + { + Logger.Error($"The following URL is not listed in the allowlist for HelpPage in web.config: {baseUrl}"); + throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.BadRequest, "HelpPage source not permitted")); + } + var url = string.Format(baseUrl + "/Umbraco/Documentation/Lessons/GetContextHelpDocs?sectionAlias={0}&treeAlias={1}", section, tree); try @@ -44,6 +53,17 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return new List(); } + + private bool IsAllowedUrl(string url) + { + if (string.IsNullOrEmpty(_helpPageSettings.HelpPageUrlAllowList) || + _helpPageSettings.HelpPageUrlAllowList.Contains(url)) + { + return true; + } + + return false; + } } [DataContract(Name = "HelpPage")] From 3261a6f71dd519ab0c17b9df6a1a773e044f2a87 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 26 Jan 2022 12:12:59 +0100 Subject: [PATCH 2/3] Fix up for V9 --- src/JsonSchema/AppSettings.cs | 2 ++ .../Configuration/Models/HelpPageSettings.cs | 11 ++++++ src/Umbraco.Core/Constants-Configuration.cs | 1 + .../UmbracoBuilder.Configuration.cs | 3 +- src/Umbraco.Core/Help/HelpPageSettings.cs | 12 ------- src/Umbraco.Core/Help/IHelpPageSettings.cs | 10 ------ .../Controllers/HelpController.cs | 34 +++++++++++++++---- 7 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/Models/HelpPageSettings.cs delete mode 100644 src/Umbraco.Core/Help/HelpPageSettings.cs delete mode 100644 src/Umbraco.Core/Help/IHelpPageSettings.cs diff --git a/src/JsonSchema/AppSettings.cs b/src/JsonSchema/AppSettings.cs index 62817bdec7..73c5ea18f5 100644 --- a/src/JsonSchema/AppSettings.cs +++ b/src/JsonSchema/AppSettings.cs @@ -89,6 +89,8 @@ namespace JsonSchema public LegacyPasswordMigrationSettings LegacyPasswordMigration { get; set; } public ContentDashboardSettings ContentDashboard { get; set; } + + public HelpPageSettings HelpPage { get; set; } } /// diff --git a/src/Umbraco.Core/Configuration/Models/HelpPageSettings.cs b/src/Umbraco.Core/Configuration/Models/HelpPageSettings.cs new file mode 100644 index 0000000000..3bd518b37e --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/HelpPageSettings.cs @@ -0,0 +1,11 @@ +namespace Umbraco.Cms.Core.Configuration.Models +{ + [UmbracoOptions(Constants.Configuration.ConfigHelpPage)] + public class HelpPageSettings + { + /// + /// Gets or sets the allowed addresses to retrieve data for the content dashboard. + /// + public string[] HelpPageUrlAllowList { get; set; } + } +} diff --git a/src/Umbraco.Core/Constants-Configuration.cs b/src/Umbraco.Core/Constants-Configuration.cs index ab951618e3..bdbd13b2a4 100644 --- a/src/Umbraco.Core/Constants-Configuration.cs +++ b/src/Umbraco.Core/Constants-Configuration.cs @@ -55,6 +55,7 @@ namespace Umbraco.Cms.Core public const string ConfigRichTextEditor = ConfigPrefix + "RichTextEditor"; public const string ConfigPackageMigration = ConfigPrefix + "PackageMigration"; public const string ConfigContentDashboard = ConfigPrefix + "ContentDashboard"; + public const string ConfigHelpPage = ConfigPrefix + "HelpPage"; } } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 8baf34f9cb..91e6f71415 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -87,7 +87,8 @@ namespace Umbraco.Cms.Core.DependencyInjection .AddUmbracoOptions() .AddUmbracoOptions() .AddUmbracoOptions() - .AddUmbracoOptions(); + .AddUmbracoOptions() + .AddUmbracoOptions(); builder.Services.Configure(options => options.MergeReplacements(builder.Config)); diff --git a/src/Umbraco.Core/Help/HelpPageSettings.cs b/src/Umbraco.Core/Help/HelpPageSettings.cs deleted file mode 100644 index d2a4a3a0f5..0000000000 --- a/src/Umbraco.Core/Help/HelpPageSettings.cs +++ /dev/null @@ -1,12 +0,0 @@ -using System.Configuration; - -namespace Umbraco.Core.Help -{ - public class HelpPageSettings : IHelpPageSettings - { - public string HelpPageUrlAllowList => - ConfigurationManager.AppSettings.ContainsKey(Constants.AppSettings.HelpPageUrlAllowList) - ? ConfigurationManager.AppSettings[Constants.AppSettings.HelpPageUrlAllowList] - : null; - } -} diff --git a/src/Umbraco.Core/Help/IHelpPageSettings.cs b/src/Umbraco.Core/Help/IHelpPageSettings.cs deleted file mode 100644 index 5643e47a30..0000000000 --- a/src/Umbraco.Core/Help/IHelpPageSettings.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace Umbraco.Core.Help -{ - public interface IHelpPageSettings - { - /// - /// Gets the allowed addresses to retrieve data for the help page. - /// - string HelpPageUrlAllowList { get; } - } -} diff --git a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs index ecec8f864d..dd01f9621f 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs @@ -1,11 +1,17 @@ +using System; using System.Collections.Generic; +using System.Linq; +using System.Net; using System.Net.Http; using System.Runtime.Serialization; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Newtonsoft.Json; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Web.Common.Attributes; -using Umbraco.Core.Help; +using Umbraco.Cms.Web.Common.DependencyInjection; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Web.BackOffice.Controllers @@ -14,21 +20,35 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers public class HelpController : UmbracoAuthorizedJsonController { private readonly ILogger _logger; - private readonly IHelpPageSettings _helpPageSettings; + private readonly HelpPageSettings _helpPageSettings; - public HelpController(ILogger logger, - IHelpPageSettings helpPageSettings) + [Obsolete("Use constructor that takes IOptions")] + public HelpController(ILogger logger) + : this(logger, StaticServiceProvider.Instance.GetRequiredService>()) + { + } + + [ActivatorUtilitiesConstructor] + public HelpController( + ILogger logger, + IOptions helpPageSettings) { _logger = logger; + _helpPageSettings = helpPageSettings.Value; } private static HttpClient _httpClient; + public async Task> GetContextHelpForPage(string section, string tree, string baseUrl = "https://our.umbraco.com") { if (IsAllowedUrl(baseUrl) is false) { - Logger.Error($"The following URL is not listed in the allowlist for HelpPage in web.config: {baseUrl}"); - throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.BadRequest, "HelpPage source not permitted")); + _logger.LogError($"The following URL is not listed in the allowlist for HelpPage in web.config: {baseUrl}"); + HttpContext.Response.StatusCode = (int)HttpStatusCode.BadRequest; + + // Ideally we'd want to return a BadRequestResult here, + // however, since we're not returning ActionResult this is not possible and changing it would be a breaking change. + return new List(); } var url = string.Format(baseUrl + "/Umbraco/Documentation/Lessons/GetContextHelpDocs?sectionAlias={0}&treeAlias={1}", section, tree); @@ -56,7 +76,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers private bool IsAllowedUrl(string url) { - if (string.IsNullOrEmpty(_helpPageSettings.HelpPageUrlAllowList) || + if (_helpPageSettings.HelpPageUrlAllowList is null || _helpPageSettings.HelpPageUrlAllowList.Contains(url)) { return true; From d4c43b69f77f5a7fd2a3ab45f61c85c7cfe0b5c0 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 26 Jan 2022 12:40:14 +0100 Subject: [PATCH 3/3] Updated to use IOptionsMonitor --- .../Controllers/HelpController.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs index dd01f9621f..d79001d0f8 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/HelpController.cs @@ -20,21 +20,28 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers public class HelpController : UmbracoAuthorizedJsonController { private readonly ILogger _logger; - private readonly HelpPageSettings _helpPageSettings; + private HelpPageSettings _helpPageSettings; [Obsolete("Use constructor that takes IOptions")] public HelpController(ILogger logger) - : this(logger, StaticServiceProvider.Instance.GetRequiredService>()) + : this(logger, StaticServiceProvider.Instance.GetRequiredService>()) { } [ActivatorUtilitiesConstructor] public HelpController( ILogger logger, - IOptions helpPageSettings) + IOptionsMonitor helpPageSettings) { _logger = logger; - _helpPageSettings = helpPageSettings.Value; + + ResetHelpPageSettings(helpPageSettings.CurrentValue); + helpPageSettings.OnChange(ResetHelpPageSettings); + } + + private void ResetHelpPageSettings(HelpPageSettings settings) + { + _helpPageSettings = settings; } private static HttpClient _httpClient;