From 48c981d31f49e7b692c5ca78d251b1081bf41665 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 7 Sep 2022 14:38:54 +0200 Subject: [PATCH] Hotfix: Move allow edit invariant from non default setting to content settings (#12960) * Use ContentSettings instead of SecuritySettings for AllowEditInvariantFromNonDefault * Make it backwards compatible (cherry picked from commit 3846c75cc6d30930cbde9cf46b66d383ce116f5d) --- .../Configuration/Models/ContentSettings.cs | 7 ++ .../Configuration/Models/SecuritySettings.cs | 11 +-- .../UmbracoBuilder.Configuration.cs | 15 ++++ .../DependencyInjection/UmbracoBuilder.cs | 2 +- .../Models/Mapping/ContentVariantMapper.cs | 29 ++++++-- .../Services/CultureImpactFactory.cs | 22 ++++-- .../Controllers/BackOfficeServerVariables.cs | 2 +- .../Umbraco.Core/Models/CultureImpactTests.cs | 70 ++++++++++--------- 8 files changed, 105 insertions(+), 53 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs index f0532a7203..f4f3040b79 100644 --- a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs @@ -157,6 +157,7 @@ public class ContentSettings internal const bool StaticHideBackOfficeLogo = false; internal const bool StaticDisableDeleteWhenReferenced = false; internal const bool StaticDisableUnpublishWhenReferenced = false; + internal const bool StaticAllowEditInvariantFromNonDefault = false; /// /// Gets or sets a value for the content notification settings. @@ -242,4 +243,10 @@ public class ContentSettings /// Get or sets the model representing the global content version cleanup policy /// public ContentVersionCleanupPolicySettings ContentVersionCleanupPolicy { get; set; } = new(); + + /// + /// Gets or sets a value indicating whether to allow editing invariant properties from a non-default language variation. + /// + [DefaultValue(StaticAllowEditInvariantFromNonDefault)] + public bool AllowEditInvariantFromNonDefault { get; set; } = StaticAllowEditInvariantFromNonDefault; } diff --git a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs index 586b3955c2..708f9b98c2 100644 --- a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs +++ b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs @@ -86,9 +86,10 @@ public class SecuritySettings [DefaultValue(StaticUserBypassTwoFactorForExternalLogins)] public bool UserBypassTwoFactorForExternalLogins { get; set; } = StaticUserBypassTwoFactorForExternalLogins; - /// - /// Gets or sets a value indicating whether to allow editing invariant properties from a non-default language variation. - /// - [DefaultValue(StaticAllowEditInvariantFromNonDefault)] - public bool AllowEditInvariantFromNonDefault { get; set; } = StaticAllowEditInvariantFromNonDefault; + /// + /// Gets or sets a value indicating whether to allow editing invariant properties from a non-default language variation. + /// + [Obsolete("Use ContentSettings.AllowEditFromInvariant instead")] + [DefaultValue(StaticAllowEditInvariantFromNonDefault)] + public bool AllowEditInvariantFromNonDefault { get; set; } = StaticAllowEditInvariantFromNonDefault; } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 90e2e49c94..6efd096c68 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -1,4 +1,5 @@ using System.Reflection; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration; @@ -104,6 +105,20 @@ public static partial class UmbracoBuilderExtensions builder.Services.Configure(options => options.MergeReplacements(builder.Config)); + // TODO: Remove this in V12 + // This is to make the move of the AllowEditInvariantFromNonDefault setting from SecuritySettings to ContentSettings backwards compatible + // If there is a value in security settings, but no value in content setting we'll use that value, otherwise content settings always wins. + builder.Services.Configure(settings => + { + var securitySettingsValue = builder.Config.GetSection($"{Constants.Configuration.ConfigSecurity}").GetValue(nameof(SecuritySettings.AllowEditInvariantFromNonDefault)); + var contentSettingsValue = builder.Config.GetSection($"{Constants.Configuration.ConfigContent}").GetValue(nameof(ContentSettings.AllowEditInvariantFromNonDefault)); + + if (securitySettingsValue is not null && contentSettingsValue is null) + { + settings.AllowEditInvariantFromNonDefault = securitySettingsValue.Value; + } + }); + return builder; } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index ff2d4a1f1e..4bfe7fd7bd 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -324,7 +324,7 @@ namespace Umbraco.Cms.Core.DependencyInjection Services.AddUnique(); Services.AddUnique(); - Services.AddUnique(); + Services.AddUnique(provider => new CultureImpactFactory(provider.GetRequiredService>())); } } } diff --git a/src/Umbraco.Core/Models/Mapping/ContentVariantMapper.cs b/src/Umbraco.Core/Models/Mapping/ContentVariantMapper.cs index 5441320b0f..91bd8c3589 100644 --- a/src/Umbraco.Core/Models/Mapping/ContentVariantMapper.cs +++ b/src/Umbraco.Core/Models/Mapping/ContentVariantMapper.cs @@ -20,7 +20,7 @@ public class ContentVariantMapper private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly IContentService _contentService; private readonly IUserService _userService; - private SecuritySettings _securitySettings; + private ContentSettings _contentSettings; public ContentVariantMapper( ILocalizationService localizationService, @@ -28,17 +28,36 @@ public class ContentVariantMapper IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IContentService contentService, IUserService userService, - IOptionsMonitor securitySettings) + IOptionsMonitor contentSettings) { _localizationService = localizationService ?? throw new ArgumentNullException(nameof(localizationService)); _localizedTextService = localizedTextService ?? throw new ArgumentNullException(nameof(localizedTextService)); _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _contentService = contentService; _userService = userService; - _securitySettings = securitySettings.CurrentValue; - securitySettings.OnChange(settings => _securitySettings = settings); + _contentSettings = contentSettings.CurrentValue; + contentSettings.OnChange(settings => _contentSettings = settings); } + [Obsolete("Use constructor that takes all parameters instead")] + public ContentVariantMapper( + ILocalizationService localizationService, + ILocalizedTextService localizedTextService, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IContentService contentService, + IUserService userService, + IOptionsMonitor securitySettings) + : this( + localizationService, + localizedTextService, + backOfficeSecurityAccessor, + contentService, + userService, + StaticServiceProvider.Instance.GetRequiredService>()) + { + } + + [Obsolete("Use constructor that takes all parameters instead")] public ContentVariantMapper(ILocalizationService localizationService, ILocalizedTextService localizedTextService) : this( localizationService, @@ -244,7 +263,7 @@ public class ContentVariantMapper if (variantDisplay.Language is null) { var defaultLanguageId = _localizationService.GetDefaultLanguageId(); - if (_securitySettings.AllowEditInvariantFromNonDefault || (defaultLanguageId.HasValue && group.HasAccessToLanguage(defaultLanguageId.Value))) + if (_contentSettings.AllowEditInvariantFromNonDefault || (defaultLanguageId.HasValue && group.HasAccessToLanguage(defaultLanguageId.Value))) { hasAccess = true; } diff --git a/src/Umbraco.Core/Services/CultureImpactFactory.cs b/src/Umbraco.Core/Services/CultureImpactFactory.cs index c520f95d0e..a05a030d1b 100644 --- a/src/Umbraco.Core/Services/CultureImpactFactory.cs +++ b/src/Umbraco.Core/Services/CultureImpactFactory.cs @@ -1,25 +1,33 @@ -using Microsoft.Extensions.Options; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Core.Services; public class CultureImpactFactory : ICultureImpactFactory { - private SecuritySettings _securitySettings; + private ContentSettings _contentSettings; - public CultureImpactFactory(IOptionsMonitor securitySettings) + public CultureImpactFactory(IOptionsMonitor contentSettings) { - _securitySettings = securitySettings.CurrentValue; + _contentSettings = contentSettings.CurrentValue; - securitySettings.OnChange(x => _securitySettings = x); + contentSettings.OnChange(x => _contentSettings = x); + } + + [Obsolete("Use constructor that takes IOptionsMonitor instead. Scheduled for removal in V12")] + public CultureImpactFactory(IOptionsMonitor securitySettings) + : this(StaticServiceProvider.Instance.GetRequiredService>()) + { } /// public CultureImpact? Create(string? culture, bool isDefault, IContent content) { - TryCreate(culture, isDefault, content.ContentType.Variations, true, _securitySettings.AllowEditInvariantFromNonDefault, out CultureImpact? impact); + TryCreate(culture, isDefault, content.ContentType.Variations, true, _contentSettings.AllowEditInvariantFromNonDefault, out CultureImpact? impact); return impact; } @@ -48,7 +56,7 @@ public class CultureImpactFactory : ICultureImpactFactory throw new ArgumentException("Culture \"*\" is not explicit."); } - return new CultureImpact(culture, isDefault, _securitySettings.AllowEditInvariantFromNonDefault); + return new CultureImpact(culture, isDefault, _contentSettings.AllowEditInvariantFromNonDefault); } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs index 15deef1ad2..6471b5b2ae 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs @@ -569,7 +569,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers {"minimumPasswordNonAlphaNum", _memberPasswordConfigurationSettings.GetMinNonAlphaNumericChars()}, {"sanitizeTinyMce", _globalSettings.SanitizeTinyMce}, {"dataTypesCanBeChanged", _dataTypesSettings.CanBeChanged.ToString()}, - {"allowEditInvariantFromNonDefault", _securitySettings.AllowEditInvariantFromNonDefault}, + {"allowEditInvariantFromNonDefault", _contentSettings.AllowEditInvariantFromNonDefault}, } }, { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/CultureImpactTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/CultureImpactTests.cs index 2439c71a8a..0ce0f73271 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/CultureImpactTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Models/CultureImpactTests.cs @@ -13,24 +13,25 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Models; [TestFixture] public class CultureImpactTests { - private CultureImpactFactory BasicImpactFactory => createCultureImpactService(); + private CultureImpactFactory BasicImpactFactory => createCultureImpactService(); [Test] public void Get_Culture_For_Invariant_Errors() { - var result = BasicImpactFactory.GetCultureForInvariantErrors( + var result = BasicImpactFactory.GetCultureForInvariantErrors( Mock.Of(x => x.Published == true), new[] { "en-US", "fr-FR" }, "en-US"); Assert.AreEqual("en-US", result); // default culture is being saved so use it - result = BasicImpactFactory.GetCultureForInvariantErrors( + result = BasicImpactFactory.GetCultureForInvariantErrors( Mock.Of(x => x.Published == false), new[] { "fr-FR" }, "en-US"); - Assert.AreEqual("fr-FR", result); // default culture not being saved with not published version, use the first culture being saved + Assert.AreEqual("fr-FR", + result); // default culture not being saved with not published version, use the first culture being saved - result = BasicImpactFactory.GetCultureForInvariantErrors( + result = BasicImpactFactory.GetCultureForInvariantErrors( Mock.Of(x => x.Published == true), new[] { "fr-FR" }, "en-US"); @@ -70,7 +71,7 @@ public class CultureImpactTests [Test] public void Explicit_Default_Culture() { - var impact = BasicImpactFactory.ImpactExplicit("en-US", true); + var impact = BasicImpactFactory.ImpactExplicit("en-US", true); Assert.AreEqual(impact.Culture, "en-US"); @@ -85,7 +86,7 @@ public class CultureImpactTests [Test] public void Explicit_NonDefault_Culture() { - var impact = BasicImpactFactory.ImpactExplicit("en-US", false); + var impact = BasicImpactFactory.ImpactExplicit("en-US", false); Assert.AreEqual(impact.Culture, "en-US"); @@ -100,10 +101,11 @@ public class CultureImpactTests [Test] public void TryCreate_Explicit_Default_Culture() { - var success = BasicImpactFactory.TryCreate("en-US", true, ContentVariation.Culture, false, false, out var impact); + var success = + BasicImpactFactory.TryCreate("en-US", true, ContentVariation.Culture, false, false, out var impact); Assert.IsTrue(success); - Assert.IsNotNull(impact); + Assert.IsNotNull(impact); Assert.AreEqual(impact.Culture, "en-US"); Assert.IsTrue(impact.ImpactsInvariantProperties); @@ -117,10 +119,11 @@ public class CultureImpactTests [Test] public void TryCreate_Explicit_NonDefault_Culture() { - var success = BasicImpactFactory.TryCreate("en-US", false, ContentVariation.Culture, false, false, out var impact); + var success = + BasicImpactFactory.TryCreate("en-US", false, ContentVariation.Culture, false, false, out var impact); Assert.IsTrue(success); - Assert.IsNotNull(impact); + Assert.IsNotNull(impact); Assert.AreEqual(impact.Culture, "en-US"); Assert.IsFalse(impact.ImpactsInvariantProperties); @@ -137,10 +140,10 @@ public class CultureImpactTests var success = BasicImpactFactory.TryCreate("*", false, ContentVariation.Nothing, false, false, out var impact); Assert.IsTrue(success); - Assert.IsNotNull(impact); + Assert.IsNotNull(impact); Assert.AreEqual(impact.Culture, null); - Assert.AreSame(BasicImpactFactory.ImpactInvariant(), impact); + Assert.AreSame(BasicImpactFactory.ImpactInvariant(), impact); } [Test] @@ -149,10 +152,10 @@ public class CultureImpactTests var success = BasicImpactFactory.TryCreate("*", false, ContentVariation.Culture, false, false, out var impact); Assert.IsTrue(success); - Assert.IsNotNull(impact); + Assert.IsNotNull(impact); Assert.AreEqual(impact.Culture, "*"); - Assert.AreSame(BasicImpactFactory.ImpactAll(), impact); + Assert.AreSame(BasicImpactFactory.ImpactAll(), impact); } [Test] @@ -168,28 +171,27 @@ public class CultureImpactTests var success = BasicImpactFactory.TryCreate(null, false, ContentVariation.Nothing, false, false, out var impact); Assert.IsTrue(success); - Assert.AreSame(BasicImpactFactory.ImpactInvariant(), impact); - } - - [Test] - [TestCase(true)] - [TestCase(false)] - public void Edit_Invariant_From_Non_Default_Impacts_Invariant_Properties(bool allowEditInvariantFromNonDefault) - { - var sut = createCultureImpactService(new SecuritySettings { AllowEditInvariantFromNonDefault = allowEditInvariantFromNonDefault }); - var impact = sut.ImpactExplicit("da", false); - - Assert.AreEqual(allowEditInvariantFromNonDefault, impact.ImpactsAlsoInvariantProperties); + Assert.AreSame(BasicImpactFactory.ImpactInvariant(), impact); } - private CultureImpactFactory createCultureImpactService(SecuritySettings securitySettings = null) + [Test] + [TestCase(true)] + [TestCase(false)] + public void Edit_Invariant_From_Non_Default_Impacts_Invariant_Properties(bool allowEditInvariantFromNonDefault) + { + var sut = createCultureImpactService(new ContentSettings { - securitySettings ??= new SecuritySettings - { - AllowEditInvariantFromNonDefault = false, - }; + AllowEditInvariantFromNonDefault = allowEditInvariantFromNonDefault + }); + var impact = sut.ImpactExplicit("da", false); - return new CultureImpactFactory(new TestOptionsMonitor(securitySettings)); - } + Assert.AreEqual(allowEditInvariantFromNonDefault, impact.ImpactsAlsoInvariantProperties); + } + private CultureImpactFactory createCultureImpactService(ContentSettings contentSettings = null) + { + contentSettings ??= new ContentSettings { AllowEditInvariantFromNonDefault = false, }; + + return new CultureImpactFactory(new TestOptionsMonitor(contentSettings)); + } }