From de8545456daed05fdd19ef80ccf5bdf53f21df81 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 17 Sep 2025 09:38:12 +0200 Subject: [PATCH] Improvement - Content type filters : Add Validation for allowed children and root (#19903) * Validate content type filter restrictions when creating content at root. * Validate content type filter restrictions when creating content as child. * Update tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Create.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../ContentBlueprintEditingService.cs | 6 +- .../Services/ContentEditingService.cs | 7 +- .../Services/ContentEditingServiceBase.cs | 53 +++++++-- .../ContentEditingServiceWithSortingBase.cs | 7 +- .../Services/MediaEditingService.cs | 7 +- .../Services/MemberContentEditingService.cs | 8 +- .../ContentEditingServiceTests.Create.cs | 111 +++++++++++++++++- 7 files changed, 176 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentBlueprintEditingService.cs b/src/Umbraco.Core/Services/ContentBlueprintEditingService.cs index d2e68ad327..f40aafc301 100644 --- a/src/Umbraco.Core/Services/ContentBlueprintEditingService.cs +++ b/src/Umbraco.Core/Services/ContentBlueprintEditingService.cs @@ -7,6 +7,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -27,8 +28,9 @@ internal sealed class ContentBlueprintEditingService IContentValidationService validationService, IContentBlueprintContainerService containerService, IOptionsMonitor optionsMonitor, - IRelationService relationService) - : base(contentService, contentTypeService, propertyEditorCollection, dataTypeService, logger, scopeProvider, userIdKeyResolver, validationService, optionsMonitor, relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) + : base(contentService, contentTypeService, propertyEditorCollection, dataTypeService, logger, scopeProvider, userIdKeyResolver, validationService, optionsMonitor, relationService, contentTypeFilters) => _containerService = containerService; public Task GetAsync(Guid key) diff --git a/src/Umbraco.Core/Services/ContentEditingService.cs b/src/Umbraco.Core/Services/ContentEditingService.cs index bd32e3d04a..00b3dddd25 100644 --- a/src/Umbraco.Core/Services/ContentEditingService.cs +++ b/src/Umbraco.Core/Services/ContentEditingService.cs @@ -6,6 +6,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Extensions; @@ -36,7 +37,8 @@ internal sealed class ContentEditingService ILocalizationService localizationService, ILanguageService languageService, IOptionsMonitor optionsMonitor, - IRelationService relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) : base( contentService, contentTypeService, @@ -48,7 +50,8 @@ internal sealed class ContentEditingService contentValidationService, treeEntitySortingService, optionsMonitor, - relationService) + relationService, + contentTypeFilters) { _propertyEditorCollection = propertyEditorCollection; _templateService = templateService; diff --git a/src/Umbraco.Core/Services/ContentEditingServiceBase.cs b/src/Umbraco.Core/Services/ContentEditingServiceBase.cs index 2f746d9a6f..edfde776e2 100644 --- a/src/Umbraco.Core/Services/ContentEditingServiceBase.cs +++ b/src/Umbraco.Core/Services/ContentEditingServiceBase.cs @@ -6,6 +6,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Editors; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Extensions; @@ -23,6 +24,7 @@ internal abstract class ContentEditingServiceBase _validationService; private readonly IRelationService _relationService; + private readonly ContentTypeFilterCollection _contentTypeFilters; protected ContentEditingServiceBase( TContentService contentService, @@ -34,7 +36,8 @@ internal abstract class ContentEditingServiceBase validationService, IOptionsMonitor optionsMonitor, - IRelationService relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) { _propertyEditorCollection = propertyEditorCollection; _dataTypeService = dataTypeService; @@ -51,6 +54,7 @@ internal abstract class ContentEditingServiceBase TryGetAndValidateParentIdAsync(Guid? parentKey, TContentType contentType) + protected virtual async Task<(int? ParentId, ContentEditingOperationStatus OperationStatus)> TryGetAndValidateParentIdAsync(Guid? parentKey, TContentType contentType) { TContent? parent = parentKey.HasValue ? ContentService.GetById(parentKey.Value) @@ -381,32 +385,63 @@ internal abstract class ContentEditingServiceBase((null, ContentEditingOperationStatus.ParentNotFound)); + return (null, ContentEditingOperationStatus.ParentNotFound); } - if (parent == null && contentType.AllowedAsRoot == false) + if (parent == null && + (contentType.AllowedAsRoot == false || + + // We could have a content type filter registered that prevents the content from being created at the root level, + // even if it's allowed in the content type definition. + await IsAllowedAtRootByContentTypeFilters(contentType) == false)) { - return Task.FromResult<(int?, ContentEditingOperationStatus)>((null, ContentEditingOperationStatus.NotAllowed)); + return (null, ContentEditingOperationStatus.NotAllowed); } if (parent != null) { if (parent.Trashed) { - return Task.FromResult<(int?, ContentEditingOperationStatus)>((null, ContentEditingOperationStatus.InTrash)); + return (null, ContentEditingOperationStatus.InTrash); } TContentType? parentContentType = ContentTypeService.Get(parent.ContentType.Key); Guid[] allowedContentTypeKeys = parentContentType?.AllowedContentTypes?.Select(c => c.Key).ToArray() ?? Array.Empty(); - if (allowedContentTypeKeys.Contains(contentType.Key) == false) + if (allowedContentTypeKeys.Contains(contentType.Key) == false || + + // We could have a content type filter registered that prevents the content from being created as a child, + // even if it's allowed in the content type definition. + await IsAllowedAsChildByContentTypeFilters(contentType, parentContentType!.Key, parent.Key) == false) { - return Task.FromResult<(int?, ContentEditingOperationStatus)>((null, ContentEditingOperationStatus.NotAllowed)); + return (null, ContentEditingOperationStatus.NotAllowed); } } - return Task.FromResult<(int?, ContentEditingOperationStatus)>((parent?.Id ?? Constants.System.Root, ContentEditingOperationStatus.Success)); + return (parent?.Id ?? Constants.System.Root, ContentEditingOperationStatus.Success); + } + + private async Task IsAllowedAtRootByContentTypeFilters(TContentType contentType) + { + IEnumerable filteredContentTypes = [contentType]; + foreach (IContentTypeFilter filter in _contentTypeFilters) + { + filteredContentTypes = await filter.FilterAllowedAtRootAsync(filteredContentTypes); + } + + return filteredContentTypes.Any(); + } + + private async Task IsAllowedAsChildByContentTypeFilters(TContentType contentType, Guid parentContentTypeKey, Guid parentKey) + { + IEnumerable filteredContentTypes = [new ContentTypeSort(contentType.Key, contentType.SortOrder, contentType.Alias)]; + foreach (IContentTypeFilter filter in _contentTypeFilters) + { + filteredContentTypes = await filter.FilterAllowedChildrenAsync(filteredContentTypes, parentContentTypeKey, parentKey); + } + + return filteredContentTypes.Any(); } private void UpdateNames(ContentEditingModelBase contentEditingModelBase, TContent content, TContentType contentType) diff --git a/src/Umbraco.Core/Services/ContentEditingServiceWithSortingBase.cs b/src/Umbraco.Core/Services/ContentEditingServiceWithSortingBase.cs index 530f56e19c..56dcba4613 100644 --- a/src/Umbraco.Core/Services/ContentEditingServiceWithSortingBase.cs +++ b/src/Umbraco.Core/Services/ContentEditingServiceWithSortingBase.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -30,7 +31,8 @@ internal abstract class ContentEditingServiceWithSortingBase validationService, ITreeEntitySortingService treeEntitySortingService, IOptionsMonitor optionsMonitor, - IRelationService relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) : base( contentService, contentTypeService, @@ -41,7 +43,8 @@ internal abstract class ContentEditingServiceWithSortingBase optionsMonitor, - IRelationService relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) : base( contentService, contentTypeService, @@ -37,7 +39,8 @@ internal sealed class MediaEditingService mediaValidationService, treeEntitySortingService, optionsMonitor, - relationService) + relationService, + contentTypeFilters) => _logger = logger; public Task GetAsync(Guid key) diff --git a/src/Umbraco.Core/Services/MemberContentEditingService.cs b/src/Umbraco.Core/Services/MemberContentEditingService.cs index 720ad80d62..8ec58ae343 100644 --- a/src/Umbraco.Core/Services/MemberContentEditingService.cs +++ b/src/Umbraco.Core/Services/MemberContentEditingService.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Extensions; @@ -7,6 +7,7 @@ using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -28,8 +29,9 @@ internal sealed class MemberContentEditingService IMemberValidationService memberValidationService, IUserService userService, IOptionsMonitor optionsMonitor, - IRelationService relationService) - : base(contentService, contentTypeService, propertyEditorCollection, dataTypeService, logger, scopeProvider, userIdKeyResolver, memberValidationService, optionsMonitor, relationService) + IRelationService relationService, + ContentTypeFilterCollection contentTypeFilters) + : base(contentService, contentTypeService, propertyEditorCollection, dataTypeService, logger, scopeProvider, userIdKeyResolver, memberValidationService, optionsMonitor, relationService, contentTypeFilters) { _logger = logger; _userService = userService; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Create.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Create.cs index 7d0a12110a..a341c371ea 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Create.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.Create.cs @@ -1,12 +1,14 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.PropertyEditors; +using Umbraco.Cms.Core.Services.Filters; using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Integration.Attributes; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; @@ -15,6 +17,58 @@ public partial class ContentEditingServiceTests [TestCase(true)] [TestCase(false)] public async Task Can_Create_At_Root(bool allowedAtRoot) + => await Test_Can_Create_At_Root(allowedAtRoot, allowedAtRoot); + + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureContentTypeFilterToAllowTextPageAtRoot))] + public async Task Can_Create_At_Root_With_Content_Type_Filter() => + + // Verifies that when allowed at root, the content can be created if not filtered out by a content type filter. + await Test_Can_Create_At_Root(true, true); + + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureContentTypeFilterToDisallowTextPageAtRoot))] + public async Task Cannot_Create_At_Root_With_Content_Type_Filter() => + + // Verifies that when allowed at root, the content cannot be created if filtered out by a content type filter. + await Test_Can_Create_At_Root(true, false); + + public static void ConfigureContentTypeFilterToAllowTextPageAtRoot(IUmbracoBuilder builder) + => builder.ContentTypeFilters() + .Append(); + + public static void ConfigureContentTypeFilterToDisallowTextPageAtRoot(IUmbracoBuilder builder) + => builder.ContentTypeFilters() + .Append(); + + private class ContentTypeFilterForAllowedTextPageAtRoot : ContentTypeFilterForTextPageAtRoot + { + public ContentTypeFilterForAllowedTextPageAtRoot() + : base(true) + { + } + } + + private class ContentTypeFilterForDisallowedTextPageAtRoot : ContentTypeFilterForTextPageAtRoot + { + public ContentTypeFilterForDisallowedTextPageAtRoot() + : base(false) + { + } + } + + private abstract class ContentTypeFilterForTextPageAtRoot : IContentTypeFilter + { + private readonly bool _allowed; + + protected ContentTypeFilterForTextPageAtRoot(bool allowed) => _allowed = allowed; + + public Task> FilterAllowedAtRootAsync(IEnumerable contentTypes) + where TItem : IContentTypeComposition + => Task.FromResult(contentTypes.Where(x => (_allowed && x.Alias == "textPage") || (!_allowed && x.Alias != "textPage"))); + } + + private async Task Test_Can_Create_At_Root(bool allowedAtRoot, bool expectSuccess) { var template = TemplateBuilder.CreateTextPageTemplate(); await TemplateService.CreateAsync(template, Constants.Security.SuperUserKey); @@ -41,7 +95,7 @@ public partial class ContentEditingServiceTests var result = await ContentEditingService.CreateAsync(createModel, Constants.Security.SuperUserKey); - if (allowedAtRoot) + if (expectSuccess) { Assert.IsTrue(result.Success); Assert.AreEqual(ContentEditingOperationStatus.Success, result.Status); @@ -72,6 +126,57 @@ public partial class ContentEditingServiceTests [TestCase(true)] [TestCase(false)] public async Task Can_Create_As_Child(bool allowedAsChild) + => await Test_Can_Create_As_Child(allowedAsChild, allowedAsChild); + + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureContentTypeFilterToAllowTextPageAsChild))] + public async Task Can_Create_As_Child_With_Content_Type_Filter() => + + // Verifies that when allowed as a child, the content can be created if not filtered out by a content type filter. + await Test_Can_Create_As_Child(true, true); + + [Test] + [ConfigureBuilder(ActionName = nameof(ConfigureContentTypeFilterToDisallowTextPageAsChild))] + public async Task Cannot_Create_As_Child_With_Content_Type_Filter() => + + // Verifies that when allowed as a child, the content cannot be created if filtered out by a content type filter. + await Test_Can_Create_As_Child(true, false); + + public static void ConfigureContentTypeFilterToAllowTextPageAsChild(IUmbracoBuilder builder) + => builder.ContentTypeFilters() + .Append(); + + public static void ConfigureContentTypeFilterToDisallowTextPageAsChild(IUmbracoBuilder builder) + => builder.ContentTypeFilters() + .Append(); + + private class ContentTypeFilterForAllowedTextPageAsChild : ContentTypeFilterForTextPageAsChild + { + public ContentTypeFilterForAllowedTextPageAsChild() + : base(true) + { + } + } + + private class ContentTypeFilterForDisallowedTextPageAsChild : ContentTypeFilterForTextPageAsChild + { + public ContentTypeFilterForDisallowedTextPageAsChild() + : base(false) + { + } + } + + private abstract class ContentTypeFilterForTextPageAsChild : IContentTypeFilter + { + private readonly bool _allowed; + + protected ContentTypeFilterForTextPageAsChild(bool allowed) => _allowed = allowed; + + public Task> FilterAllowedChildrenAsync(IEnumerable contentTypes, Guid parentContentTypeKey, Guid? parentContentKey) + => Task.FromResult(contentTypes.Where(x => (_allowed && x.Alias == "textPage") || (!_allowed && x.Alias != "textPage"))); + } + + private async Task Test_Can_Create_As_Child(bool allowedAsChild, bool expectSuccess) { var template = TemplateBuilder.CreateTextPageTemplate(); await TemplateService.CreateAsync(template, Constants.Security.SuperUserKey); @@ -121,7 +226,7 @@ public partial class ContentEditingServiceTests var result = await ContentEditingService.CreateAsync(createModel, Constants.Security.SuperUserKey); - if (allowedAsChild) + if (expectSuccess) { Assert.IsTrue(result.Success); Assert.AreEqual(ContentEditingOperationStatus.Success, result.Status);