From 43791c2b333261fe5b64e827f0e58d95304c7fdd Mon Sep 17 00:00:00 2001 From: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com> Date: Wed, 17 Jan 2024 13:26:09 +0100 Subject: [PATCH] v14: Merge NewBackOfficeSettings into SecuritySettings. (#15586) * Merge NewBackOfficeSettings into SecuritySettings. * Apply suggestions from code review Co-authored-by: Kenn Jacobsen * Remove hardcoded callback path --------- Co-authored-by: Kenn Jacobsen --- .../BackOfficeCorsPolicyBuilderExtensions.cs | 7 +++--- .../UmbracoBuilderExtensions.cs | 7 ------ .../Security/BackOfficeApplicationManager.cs | 12 ++++----- .../Configuration/Models/SecuritySettings.cs | 12 +++++++++ .../Validation/SecuritySettingsValidator.cs | 24 ++++++++++++++++++ .../UmbracoBuilder.Configuration.cs | 3 ++- .../Configuration/NewBackOfficeSettings.cs | 12 --------- .../NewBackOfficeSettingsValidator.cs | 25 ------------------- src/Umbraco.Core/Umbraco.Core.csproj | 4 --- 9 files changed, 47 insertions(+), 59 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/Models/Validation/SecuritySettingsValidator.cs delete mode 100644 src/Umbraco.Core/Models/Configuration/NewBackOfficeSettings.cs delete mode 100644 src/Umbraco.Core/Models/Configuration/NewBackOfficeSettingsValidator.cs diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeCorsPolicyBuilderExtensions.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeCorsPolicyBuilderExtensions.cs index 93453cfeff..5baee9de6b 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeCorsPolicyBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeCorsPolicyBuilderExtensions.cs @@ -2,8 +2,8 @@ using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.DependencyInjection; -using Umbraco.Cms.Core.Models.Configuration; using Umbraco.Cms.Web.Common.ApplicationBuilder; namespace Umbraco.Cms.Api.Management.DependencyInjection; @@ -12,10 +12,9 @@ internal static class BackOfficeCorsPolicyBuilderExtensions { internal static IUmbracoBuilder AddCorsPolicy(this IUmbracoBuilder builder) { - // FIXME: Get the correct settings once NewBackOfficeSettings is merged with relevant existing settings from Core Uri? backOfficeHost = builder.Config - .GetSection($"{Constants.Configuration.ConfigPrefix}NewBackOffice") - .Get()?.BackOfficeHost; + .GetSection(Constants.Configuration.ConfigSecurity) + .Get()?.BackOfficeHost; if (backOfficeHost is null) { diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs index 938b9808f2..199e822ed5 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/UmbracoBuilderExtensions.cs @@ -1,5 +1,4 @@ using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using Umbraco.Cms.Api.Common.Configuration; using Umbraco.Cms.Api.Common.DependencyInjection; using Umbraco.Cms.Api.Management.Configuration; @@ -8,7 +7,6 @@ using Umbraco.Cms.Api.Management.Serialization; using Umbraco.Cms.Api.Management.Services; using Umbraco.Cms.Core; using Umbraco.Cms.Core.DependencyInjection; -using Umbraco.Cms.Core.Models.Configuration; using Umbraco.Cms.Web.Common.ApplicationBuilder; namespace Umbraco.Extensions; @@ -78,11 +76,6 @@ public static class UmbracoBuilderExtensions applicationBuilder => { }, applicationBuilder => applicationBuilder.UseEndpoints())); }); - - // FIXME: when this is moved to core, make the AddUmbracoOptions extension private again and remove core InternalsVisibleTo for Umbraco.Cms.Api.Management - builder.AddUmbracoOptions(); - // FIXME: remove this when NewBackOfficeSettings is moved to core - services.AddSingleton, NewBackOfficeSettingsValidator>(); } return builder; diff --git a/src/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManager.cs b/src/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManager.cs index 6137f98cac..a4d12931ae 100644 --- a/src/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManager.cs +++ b/src/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManager.cs @@ -3,7 +3,7 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Models.Configuration; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Security; @@ -14,12 +14,12 @@ public class BackOfficeApplicationManager : OpenIdDictApplicationManagerBase, IB private readonly IWebHostEnvironment _webHostEnvironment; private readonly IRuntimeState _runtimeState; private readonly Uri? _backOfficeHost; - private readonly string? _authorizeCallbackPathName; + private readonly string _authorizeCallbackPathName; public BackOfficeApplicationManager( IOpenIddictApplicationManager applicationManager, IWebHostEnvironment webHostEnvironment, - IOptions securitySettings, + IOptions securitySettings, IRuntimeState runtimeState) : base(applicationManager) { @@ -101,14 +101,14 @@ public class BackOfficeApplicationManager : OpenIdDictApplicationManagerBase, IB ClientId = Constants.OAuthClientIds.BackOffice, RedirectUris = { - CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName ?? "/umbraco") + CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName) }, Type = OpenIddictConstants.ClientTypes.Public, PostLogoutRedirectUris = { - CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName ?? "/umbraco/login"), + CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName + "/login"), // FIXME: remove when we no longer use Umbraco.Web.UI project - CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName ?? "/umbraco") + CallbackUrlFor(_backOfficeHost ?? backOfficeUrl, _authorizeCallbackPathName) }, Permissions = { diff --git a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs index f1005e5d1b..f52c41f60c 100644 --- a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs +++ b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs @@ -25,6 +25,7 @@ public class SecuritySettings internal const int StaticMemberDefaultLockoutTimeInMinutes = 30 * 24 * 60; internal const int StaticUserDefaultLockoutTimeInMinutes = 30 * 24 * 60; + internal const string StaticAuthorizeCallbackPathName = "/umbraco"; /// /// Gets or sets a value indicating whether to keep the user logged in. @@ -116,4 +117,15 @@ public class SecuritySettings /// [DefaultValue(StaticAllowConcurrentLogins)] public bool AllowConcurrentLogins { get; set; } = StaticAllowConcurrentLogins; + + /// + /// Gets or sets a value of the back-office host URI. Use this when running the back-office client and the Management API on different hosts. Leave empty when running both on the same host. + /// + public Uri? BackOfficeHost { get; set; } + + /// + /// The path to use for authorization callback. Will be appended to the BackOfficeHost. + /// + [DefaultValue(StaticAuthorizeCallbackPathName)] + public string AuthorizeCallbackPathName { get; set; } = StaticAuthorizeCallbackPathName; } diff --git a/src/Umbraco.Core/Configuration/Models/Validation/SecuritySettingsValidator.cs b/src/Umbraco.Core/Configuration/Models/Validation/SecuritySettingsValidator.cs new file mode 100644 index 0000000000..3df30bbd03 --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/Validation/SecuritySettingsValidator.cs @@ -0,0 +1,24 @@ +using Microsoft.Extensions.Options; + +namespace Umbraco.Cms.Core.Configuration.Models.Validation; + +public class SecuritySettingsValidator : ConfigurationValidatorBase, IValidateOptions +{ + public ValidateOptionsResult Validate(string? name, SecuritySettings options) + { + if (options.BackOfficeHost != null) + { + if (options.BackOfficeHost.IsAbsoluteUri == false) + { + return ValidateOptionsResult.Fail($"{nameof(SecuritySettings.BackOfficeHost)} must be an absolute URL"); + } + + if (options.BackOfficeHost.PathAndQuery != "/") + { + return ValidateOptionsResult.Fail($"{nameof(SecuritySettings.BackOfficeHost)} must not have any path or query"); + } + } + + return ValidateOptionsResult.Success; + } +} diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index ceff2cf7ce..b42aa5a0eb 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -14,7 +14,7 @@ namespace Umbraco.Cms.Core.DependencyInjection; /// public static partial class UmbracoBuilderExtensions { - internal static IUmbracoBuilder AddUmbracoOptions(this IUmbracoBuilder builder, Action>? configure = null) + private static IUmbracoBuilder AddUmbracoOptions(this IUmbracoBuilder builder, Action>? configure = null) where TOptions : class { UmbracoOptionsAttribute? umbracoOptionsAttribute = typeof(TOptions).GetCustomAttribute(); @@ -45,6 +45,7 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddSingleton, HealthChecksSettingsValidator>(); builder.Services.AddSingleton, RequestHandlerSettingsValidator>(); builder.Services.AddSingleton, UnattendedSettingsValidator>(); + builder.Services.AddSingleton, SecuritySettingsValidator>(); // Register configuration sections. builder diff --git a/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettings.cs b/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettings.cs deleted file mode 100644 index 371ddc279c..0000000000 --- a/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettings.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Umbraco.Cms.Core.Configuration.Models; - -namespace Umbraco.Cms.Core.Models.Configuration; - -// FIXME: merge this class with relevant existing settings from Core and clean up -[UmbracoOptions($"{Umbraco.Cms.Core.Constants.Configuration.ConfigPrefix}NewBackOffice")] -public class NewBackOfficeSettings -{ - public Uri? BackOfficeHost { get; set; } = null; - - public string? AuthorizeCallbackPathName { get; set; } = null; -} diff --git a/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettingsValidator.cs b/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettingsValidator.cs deleted file mode 100644 index 2845853700..0000000000 --- a/src/Umbraco.Core/Models/Configuration/NewBackOfficeSettingsValidator.cs +++ /dev/null @@ -1,25 +0,0 @@ -using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Configuration.Models.Validation; - -namespace Umbraco.Cms.Core.Models.Configuration; - -// TODO: merge this class with relevant existing settings validators from Core and clean up -public class NewBackOfficeSettingsValidator : ConfigurationValidatorBase, IValidateOptions -{ - public ValidateOptionsResult Validate(string? name, NewBackOfficeSettings options) - { - if (options.BackOfficeHost != null) - { - if (options.BackOfficeHost.IsAbsoluteUri == false) - { - return ValidateOptionsResult.Fail($"{nameof(NewBackOfficeSettings.BackOfficeHost)} must be an absolute URL"); - } - if (options.BackOfficeHost.PathAndQuery != "/") - { - return ValidateOptionsResult.Fail($"{nameof(NewBackOfficeSettings.BackOfficeHost)} must not have any path or query"); - } - } - - return ValidateOptionsResult.Success; - } -} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index e4e7d38e41..85c2b4997b 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -38,10 +38,6 @@ <_Parameter1>DynamicProxyGenAssembly2 - - - <_Parameter1>Umbraco.Cms.Api.Management -