From 312c4ef05aed17a955da51f835b710f8b767da51 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 10 Dec 2025 13:16:32 +0100 Subject: [PATCH 1/6] Adding allow content type alias settings and validator --- .../Models/DeliveryApiSettings.cs | 17 +++ .../DeliveryApiSettingsValidator.cs | 47 +++++++ .../UmbracoBuilder.Configuration.cs | 1 + .../DeliveryApiSettingsExtensions.cs | 24 +++- .../DeliveryApi/PublishedContentCacheTests.cs | 116 +++++++++++++++++- 5 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidator.cs diff --git a/src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs b/src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs index 797a055912a1..80353edcdd2b 100644 --- a/src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs @@ -42,8 +42,25 @@ public class DeliveryApiSettings /// /// The content type aliases that are not to be exposed. /// + /// + /// If is configured (non-empty), this setting is ignored. + /// public ISet DisallowedContentTypeAliases { get; set; } = new HashSet(); + /// + /// Gets or sets the aliases of the content types that are exclusively allowed to be exposed through the Delivery API. + /// When configured, only content of these types will be returned from Delivery API endpoints and added to the query index. + /// + /// + /// The content type aliases that are allowed to be exposed. + /// + /// + /// When this setting is configured (non-empty), it takes precedence over . + /// If a content type alias appears in both lists, the allow list wins and the content type will be exposed. + /// If this setting is empty, all content types are allowed except those in . + /// + public ISet AllowedContentTypeAliases { get; set; } = new HashSet(); + /// /// Gets or sets a value indicating whether the Delivery API should output rich text values as JSON instead of HTML. /// diff --git a/src/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidator.cs b/src/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidator.cs new file mode 100644 index 000000000000..05074690c006 --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidator.cs @@ -0,0 +1,47 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.Configuration.Models.Validation; + +/// +/// Validator for configuration represented as . +/// +public class DeliveryApiSettingsValidator : IValidateOptions +{ + private readonly ILogger _logger; + + public DeliveryApiSettingsValidator(ILogger logger) + => _logger = logger; + + /// + public ValidateOptionsResult Validate(string? name, DeliveryApiSettings options) + { + ValidateContentTypeAliasOverlap(options); + + return ValidateOptionsResult.Success; + } + + private void ValidateContentTypeAliasOverlap(DeliveryApiSettings options) + { + if (options.AllowedContentTypeAliases.Count == 0 || options.DisallowedContentTypeAliases.Count == 0) + { + return; + } + + var overlappingAliases = options.AllowedContentTypeAliases + .Where(alias => options.DisallowedContentTypeAliases.InvariantContains(alias)) + .ToArray(); + + if (overlappingAliases.Length > 0) + { + _logger.LogWarning( + "Delivery API configuration contains content type aliases that appear in both AllowedContentTypeAliases and DisallowedContentTypeAliases: {Aliases}. " + + "The allow list takes precedence, so these content types will be allowed. Consider removing them from the disallow list to avoid confusion.", + string.Join(", ", overlappingAliases)); + } + } +} diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index e56a4cef2780..db436b867064 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -41,6 +41,7 @@ public static IUmbracoBuilder AddConfiguration(this IUmbracoBuilder builder) { // Register configuration validators. builder.Services.AddSingleton, ContentSettingsValidator>(); + builder.Services.AddSingleton, DeliveryApiSettingsValidator>(); builder.Services.AddSingleton, GlobalSettingsValidator>(); builder.Services.AddSingleton, HealthChecksSettingsValidator>(); builder.Services.AddSingleton, LoggingSettingsValidator>(); diff --git a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs index f0e35f0d6e9e..8485371f932e 100644 --- a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs +++ b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs @@ -11,8 +11,30 @@ public static bool IsAllowedContentType(this DeliveryApiSettings settings, IPubl public static bool IsDisallowedContentType(this DeliveryApiSettings settings, IPublishedContent content) => settings.IsDisallowedContentType(content.ContentType.Alias); + /// + /// Determines whether a content type alias is allowed to be exposed through the Delivery API. + /// + /// The Delivery API settings. + /// The content type alias to check. + /// + /// true if the content type is allowed; otherwise, false. + /// + /// + /// If the allow list is configured (non-empty), only content types in the allow list are permitted. + /// The allow list takes precedence - if a content type is in both allow and disallow lists, it is allowed. + /// If the allow list is empty, all content types are allowed except those in the disallow list. + /// public static bool IsAllowedContentType(this DeliveryApiSettings settings, string contentTypeAlias) - => settings.IsDisallowedContentType(contentTypeAlias) is false; + { + // If allow list is configured, it takes precedence + if (settings.AllowedContentTypeAliases.Count > 0) + { + return settings.AllowedContentTypeAliases.InvariantContains(contentTypeAlias); + } + + // Fall back to disallow list behavior + return settings.IsDisallowedContentType(contentTypeAlias) is false; + } public static bool IsDisallowedContentType(this DeliveryApiSettings settings, string contentTypeAlias) => settings.DisallowedContentTypeAliases.InvariantContains(contentTypeAlias); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs index 405ff1416211..e30850befaea 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs @@ -239,6 +239,117 @@ public void PublishedContentCache_DenyListIsCaseInsensitive() Assert.IsNull(content); } + [TestCase(true)] + [TestCase(false)] + public void PublishedContentCache_GetById_SupportsAllowList(bool allowed) + { + var allowList = allowed ? new[] { "theOtherContentType" } : new[] { "someOtherType" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetById(_contentTwoId); + + if (allowed) + { + Assert.IsNotNull(content); + } + else + { + Assert.IsNull(content); + } + } + + [TestCase(true)] + [TestCase(false)] + public void PublishedContentCache_GetByRoute_SupportsAllowList(bool allowed) + { + var allowList = allowed ? new[] { "theContentType" } : new[] { "someOtherType" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetByRoute("/content-one"); + + if (allowed) + { + Assert.IsNotNull(content); + } + else + { + Assert.IsNull(content); + } + } + + [TestCase("theContentType")] + [TestCase("theOtherContentType")] + public void PublishedContentCache_GetByIds_SupportsAllowList(string allowedContentType) + { + var allowList = new[] { allowedContentType }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); + + Assert.AreEqual(1, content.Length); + if (allowedContentType == "theContentType") + { + Assert.AreEqual(_contentOneId, content.First().Key); + } + else + { + Assert.AreEqual(_contentTwoId, content.First().Key); + } + } + + [Test] + public void PublishedContentCache_GetByIds_AllowListCanAllowMultipleContentTypes() + { + var allowList = new[] { "theContentType", "theOtherContentType" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); + Assert.AreEqual(2, content.Length); + } + + [Test] + public void PublishedContentCache_AllowListIsCaseInsensitive() + { + var allowList = new[] { "THEcontentTYPE" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetByRoute("/content-one"); + Assert.IsNotNull(content); + } + + [Test] + public void PublishedContentCache_AllowListTakesPrecedenceOverDenyList() + { + var denyList = new[] { "theContentType" }; + var allowList = new[] { "theContentType" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var content = publishedContentCache.GetByRoute("/content-one"); + Assert.IsNotNull(content); + } + + [Test] + public void PublishedContentCache_AllowListIgnoresDenyListCompletely() + { + var denyList = new[] { "theOtherContentType" }; + var allowList = new[] { "theContentType" }; + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + + var contentOne = publishedContentCache.GetByRoute("/content-one"); + Assert.IsNotNull(contentOne); + + var contentTwo = publishedContentCache.GetById(_contentTwoId); + Assert.IsNull(contentTwo); + } + + [Test] + public void PublishedContentCache_EmptyAllowListFallsBackToDenyList() + { + var denyList = new[] { "theContentType" }; + string[] allowList = Array.Empty(); + var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + + var contentOne = publishedContentCache.GetByRoute("/content-one"); + Assert.IsNull(contentOne); + + var contentTwo = publishedContentCache.GetById(_contentTwoId); + Assert.IsNotNull(contentTwo); + } + private IVariationContextAccessor CreateVariationContextAccessor(string? culture = null) { var mock = new Mock(); @@ -253,11 +364,12 @@ private IRequestPreviewService CreateRequestPreviewService(bool isPreview = fals return previewServiceMock.Object; } - private IOptionsMonitor CreateDeliveryApiSettings(string[]? disallowedContentTypeAliases = null) + private IOptionsMonitor CreateDeliveryApiSettings(string[]? disallowedContentTypeAliases = null, string[]? allowedContentTypeAliases = null) { var deliveryApiSettings = new DeliveryApiSettings { - DisallowedContentTypeAliases = new HashSet(disallowedContentTypeAliases ?? Array.Empty()) + DisallowedContentTypeAliases = new HashSet(disallowedContentTypeAliases ?? Array.Empty()), + AllowedContentTypeAliases = new HashSet(allowedContentTypeAliases ?? Array.Empty()), }; var deliveryApiOptionsMonitorMock = new Mock>(); deliveryApiOptionsMonitorMock.SetupGet(s => s.CurrentValue).Returns(deliveryApiSettings); From da2cfd0d17763fa96ce1bd19d79846719f3661c8 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 10 Dec 2025 15:37:31 +0100 Subject: [PATCH 2/6] Creating private helper method for tests --- .../DeliveryApi/PublishedContentCacheTests.cs | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs index e30850befaea..48013afa160d 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs @@ -81,7 +81,7 @@ public void Setup() [Test] public void PublishedContentCache_CanGetById() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings()); var content = publishedContentCache.GetById(_contentOneId); Assert.IsNotNull(content); Assert.AreEqual(_contentOneId, content.Key); @@ -92,7 +92,7 @@ public void PublishedContentCache_CanGetById() [Test] public void PublishedContentCache_CanGetByRoute() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings()); var content = publishedContentCache.GetByRoute("/content-two"); Assert.IsNotNull(content); Assert.AreEqual(_contentTwoId, content.Key); @@ -103,7 +103,7 @@ public void PublishedContentCache_CanGetByRoute() [Test] public void PublishedContentCache_CanGetByRoute_WithStartNodeIdPrefix() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings()); var content = publishedContentCache.GetByRoute("1234/content-three"); Assert.IsNotNull(content); Assert.AreEqual(_contentThreeId, content.Key); @@ -114,7 +114,7 @@ public void PublishedContentCache_CanGetByRoute_WithStartNodeIdPrefix() [Test] public void PublishedContentCache_CanGetByIds() { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings()); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(2, content.Length); Assert.AreEqual(_contentOneId, content.First().Key); @@ -126,7 +126,7 @@ public void PublishedContentCache_CanGetByIds() public void PublishedContentCache_GetById_SupportsDenyList(bool denied) { var denyList = denied ? new[] { "theOtherContentType" } : null; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetById(_contentTwoId); if (denied) @@ -144,7 +144,7 @@ public void PublishedContentCache_GetById_SupportsDenyList(bool denied) public void PublishedContentCache_GetByRoute_SupportsDenyList(bool denied) { var denyList = denied ? new[] { "theContentType" } : null; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetByRoute("/content-one"); if (denied) @@ -162,7 +162,7 @@ public void PublishedContentCache_GetByRoute_SupportsDenyList(bool denied) public void PublishedContentCache_GetByIds_SupportsDenyList(string deniedContentType) { var denyList = new[] { deniedContentType }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(1, content.Length); @@ -180,7 +180,7 @@ public void PublishedContentCache_GetByIds_SupportsDenyList(string deniedContent public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDenyList() { var denyList = new[] { "theContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetById(_contentTwoId); Assert.IsNotNull(content); Assert.AreEqual(_contentTwoId, content.Key); @@ -192,7 +192,7 @@ public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDenyL public void PublishedContentCache_GetByRoute_CanRetrieveContentTypesOutsideTheDenyList() { var denyList = new[] { "theOtherContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNotNull(content); Assert.AreEqual(_contentOneId, content.Key); @@ -204,7 +204,7 @@ public void PublishedContentCache_GetByRoute_CanRetrieveContentTypesOutsideTheDe [TestCase("da-DK")] public void PublishedContentCache_GetByRoute_CanRetrieveForCulture(string culture) { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor(culture)); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(), culture); var content = publishedContentCache.GetByRoute("/content-four"); Assert.IsNotNull(content); Assert.AreEqual(_contentFourId, content.Key); @@ -216,7 +216,7 @@ public void PublishedContentCache_GetByRoute_CanRetrieveForCulture(string cultur [TestCase(null)] public void PublishedContentCache_GetByRoute_CannotRetrieveForMissingOrUnknownCulture(string? culture) { - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor(culture)); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(), culture); var content = publishedContentCache.GetByRoute("/content-four"); Assert.IsNull(content); } @@ -225,7 +225,7 @@ public void PublishedContentCache_GetByRoute_CannotRetrieveForMissingOrUnknownCu public void PublishedContentCache_GetByIds_CanDenyAllRequestedContent() { var denyList = new[] { "theContentType", "theOtherContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.IsEmpty(content); } @@ -234,7 +234,7 @@ public void PublishedContentCache_GetByIds_CanDenyAllRequestedContent() public void PublishedContentCache_DenyListIsCaseInsensitive() { var denyList = new[] { "THEcontentTYPE" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNull(content); } @@ -244,7 +244,7 @@ public void PublishedContentCache_DenyListIsCaseInsensitive() public void PublishedContentCache_GetById_SupportsAllowList(bool allowed) { var allowList = allowed ? new[] { "theOtherContentType" } : new[] { "someOtherType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(allowedContentTypeAliases: allowList)); var content = publishedContentCache.GetById(_contentTwoId); if (allowed) @@ -262,7 +262,7 @@ public void PublishedContentCache_GetById_SupportsAllowList(bool allowed) public void PublishedContentCache_GetByRoute_SupportsAllowList(bool allowed) { var allowList = allowed ? new[] { "theContentType" } : new[] { "someOtherType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(allowedContentTypeAliases: allowList)); var content = publishedContentCache.GetByRoute("/content-one"); if (allowed) @@ -280,7 +280,7 @@ public void PublishedContentCache_GetByRoute_SupportsAllowList(bool allowed) public void PublishedContentCache_GetByIds_SupportsAllowList(string allowedContentType) { var allowList = new[] { allowedContentType }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(allowedContentTypeAliases: allowList)); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(1, content.Length); @@ -298,7 +298,7 @@ public void PublishedContentCache_GetByIds_SupportsAllowList(string allowedConte public void PublishedContentCache_GetByIds_AllowListCanAllowMultipleContentTypes() { var allowList = new[] { "theContentType", "theOtherContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(allowedContentTypeAliases: allowList)); var content = publishedContentCache.GetByIds(new[] { _contentOneId, _contentTwoId }).ToArray(); Assert.AreEqual(2, content.Length); } @@ -307,7 +307,7 @@ public void PublishedContentCache_GetByIds_AllowListCanAllowMultipleContentTypes public void PublishedContentCache_AllowListIsCaseInsensitive() { var allowList = new[] { "THEcontentTYPE" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(allowedContentTypeAliases: allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(allowedContentTypeAliases: allowList)); var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNotNull(content); } @@ -317,7 +317,7 @@ public void PublishedContentCache_AllowListTakesPrecedenceOverDenyList() { var denyList = new[] { "theContentType" }; var allowList = new[] { "theContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList, allowList)); var content = publishedContentCache.GetByRoute("/content-one"); Assert.IsNotNull(content); } @@ -327,7 +327,7 @@ public void PublishedContentCache_AllowListIgnoresDenyListCompletely() { var denyList = new[] { "theOtherContentType" }; var allowList = new[] { "theContentType" }; - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList, allowList)); var contentOne = publishedContentCache.GetByRoute("/content-one"); Assert.IsNotNull(contentOne); @@ -341,7 +341,7 @@ public void PublishedContentCache_EmptyAllowListFallsBackToDenyList() { var denyList = new[] { "theContentType" }; string[] allowList = Array.Empty(); - var publishedContentCache = new ApiPublishedContentCache(CreateRequestPreviewService(), CreateDeliveryApiSettings(denyList, allowList), CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor()); + var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList, allowList)); var contentOne = publishedContentCache.GetByRoute("/content-one"); Assert.IsNull(contentOne); @@ -376,5 +376,8 @@ private IOptionsMonitor CreateDeliveryApiSettings(string[]? return deliveryApiOptionsMonitorMock.Object; } + private ApiPublishedContentCache CreateApiPublishedContentCache(IOptionsMonitor deliveryApiSettings, string? culture = null) + => new(CreateRequestPreviewService(), deliveryApiSettings, CreateApiDocumentUrlService(), _contentCache, CreateVariationContextAccessor(culture)); + private IApiDocumentUrlService CreateApiDocumentUrlService() => new ApiDocumentUrlService(_documentUrlService); } From 0de2194b89af1a392f35b07d7c18b8a64f055ec4 Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 10 Dec 2025 15:37:48 +0100 Subject: [PATCH 3/6] Revisiting logic for disallow types --- .../Extensions/DeliveryApiSettingsExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs index 8485371f932e..9906a3c6c5d3 100644 --- a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs +++ b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs @@ -32,10 +32,10 @@ public static bool IsAllowedContentType(this DeliveryApiSettings settings, strin return settings.AllowedContentTypeAliases.InvariantContains(contentTypeAlias); } - // Fall back to disallow list behavior - return settings.IsDisallowedContentType(contentTypeAlias) is false; + // Otherwise the content type is allowed if it's not in the disallow list + return settings.DisallowedContentTypeAliases.InvariantContains(contentTypeAlias) is false; } public static bool IsDisallowedContentType(this DeliveryApiSettings settings, string contentTypeAlias) - => settings.DisallowedContentTypeAliases.InvariantContains(contentTypeAlias); + => settings.IsAllowedContentType(contentTypeAlias) is false; } From d25d8477175fb5d7858e22efc67d9e15bee0122e Mon Sep 17 00:00:00 2001 From: NillasKA Date: Wed, 10 Dec 2025 15:52:41 +0100 Subject: [PATCH 4/6] Adding tests for validator --- .../DeliveryApiSettingsValidatorTests.cs | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidatorTests.cs diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidatorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidatorTests.cs new file mode 100644 index 000000000000..5cd3ab6ef532 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidatorTests.cs @@ -0,0 +1,101 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Configuration.Models.Validation; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models.Validation; + +[TestFixture] +public class DeliveryApiSettingsValidatorTests +{ + private Mock> _loggerMock; + + [SetUp] + public void SetUp() => _loggerMock = new Mock>(); + + [Test] + public void Returns_Success_For_Configuration_With_Only_AllowList() + { + var validator = new DeliveryApiSettingsValidator(_loggerMock.Object); + var options = new DeliveryApiSettings + { + AllowedContentTypeAliases = new HashSet { "content1", "content2" }, + }; + + ValidateOptionsResult result = validator.Validate("settings", options); + + Assert.IsTrue(result.Succeeded); + VerifyNoWarningLogged(); + } + + [Test] + public void Returns_Success_For_Configuration_With_Only_DisallowList() + { + var validator = new DeliveryApiSettingsValidator(_loggerMock.Object); + var options = new DeliveryApiSettings + { + DisallowedContentTypeAliases = new HashSet { "content1", "content2" }, + }; + + ValidateOptionsResult result = validator.Validate("settings", options); + + Assert.IsTrue(result.Succeeded); + VerifyNoWarningLogged(); + } + + [Test] + public void Returns_Success_For_Configuration_With_No_Overlapping_Lists() + { + var validator = new DeliveryApiSettingsValidator(_loggerMock.Object); + var options = new DeliveryApiSettings + { + AllowedContentTypeAliases = new HashSet { "content1", "content2" }, + DisallowedContentTypeAliases = new HashSet { "content3", "content4" }, + }; + + ValidateOptionsResult result = validator.Validate("settings", options); + + Assert.IsTrue(result.Succeeded); + VerifyNoWarningLogged(); + } + + [Test] + public void Returns_Success_But_Logs_Warning_For_Overlapping_Lists() + { + var validator = new DeliveryApiSettingsValidator(_loggerMock.Object); + var options = new DeliveryApiSettings + { + AllowedContentTypeAliases = new HashSet { "content1", "content2" }, + DisallowedContentTypeAliases = new HashSet { "content1", "content3" }, + }; + + ValidateOptionsResult result = validator.Validate("settings", options); + + Assert.IsTrue(result.Succeeded); + + VerifyWarningLogged(); + } + + private void VerifyWarningLogged() + { + var warningCount = GetWarningLogCount(); + Assert.AreEqual(1, warningCount); + } + + private void VerifyNoWarningLogged() + { + var warningCount = GetWarningLogCount(); + Assert.AreEqual(0, warningCount); + } + + private int GetWarningLogCount() => + _loggerMock.Invocations + .Count(invocation => + invocation.Method.Name == nameof(ILogger.Log) && + invocation.Arguments.OfType().Any(level => level == LogLevel.Warning)); +} From 605fe4b3e595e6d12cf882ba3815e54c92fc23be Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 10 Dec 2025 16:57:38 +0100 Subject: [PATCH 5/6] Obsolete unnecessary methods and overloads. --- .../DeliveryApi/ApiPublishedContentCache.cs | 2 +- .../Extensions/DeliveryApiSettingsExtensions.cs | 13 ++++++++++--- .../DeliveryApiContentIndexValueSetBuilder.cs | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs index dd91b4ced111..0e48262e733e 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs @@ -129,5 +129,5 @@ public IEnumerable GetByIds(IEnumerable contentIds) : null; private bool IsAllowedContentType(IPublishedContent content) - => _deliveryApiSettings.IsAllowedContentType(content); + => _deliveryApiSettings.IsAllowedContentType(content.ContentType.Alias); } diff --git a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs index 9906a3c6c5d3..2f49086cea57 100644 --- a/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs +++ b/src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs @@ -1,13 +1,19 @@ -using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models.PublishedContent; namespace Umbraco.Extensions; +/// +/// Provides extension methods for determining whether content types or content items are allowed to be exposed through +/// the Delivery API based on the configured allow and disallow lists. +/// public static class DeliveryApiSettingsExtensions { + [Obsolete("Please use the overload of IsAllowedContentType taking a content type alias. Scheduled for removal in Umbraco 19.")] public static bool IsAllowedContentType(this DeliveryApiSettings settings, IPublishedContent content) => settings.IsAllowedContentType(content.ContentType.Alias); + [Obsolete("Please use IsAllowedContentType and negate the result instead. Scheduled for removal in Umbraco 19.")] public static bool IsDisallowedContentType(this DeliveryApiSettings settings, IPublishedContent content) => settings.IsDisallowedContentType(content.ContentType.Alias); @@ -26,16 +32,17 @@ public static bool IsDisallowedContentType(this DeliveryApiSettings settings, IP /// public static bool IsAllowedContentType(this DeliveryApiSettings settings, string contentTypeAlias) { - // If allow list is configured, it takes precedence + // If allow list is configured, it takes precedence. if (settings.AllowedContentTypeAliases.Count > 0) { return settings.AllowedContentTypeAliases.InvariantContains(contentTypeAlias); } - // Otherwise the content type is allowed if it's not in the disallow list + // Otherwise the content type is allowed if it's not in the disallow list. return settings.DisallowedContentTypeAliases.InvariantContains(contentTypeAlias) is false; } + [Obsolete("Please use IsAllowedContentType and negate the result instead. Scheduled for removal in Umbraco 19.")] public static bool IsDisallowedContentType(this DeliveryApiSettings settings, string contentTypeAlias) => settings.IsAllowedContentType(contentTypeAlias) is false; } diff --git a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs index 7c76eaddd3b1..0da2e86ee7f1 100644 --- a/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs +++ b/src/Umbraco.Infrastructure/Examine/DeliveryApiContentIndexValueSetBuilder.cs @@ -204,7 +204,7 @@ private bool CanIndex(IContent content) } // is the content type allowed in the index? - if (_deliveryApiSettings.IsDisallowedContentType(content.ContentType.Alias)) + if (_deliveryApiSettings.IsAllowedContentType(content.ContentType.Alias) is false) { return false; } From 4a412cd6a14f58ffc62430311477bf07f37c8a6f Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 10 Dec 2025 17:06:01 +0100 Subject: [PATCH 6/6] Fix warning and update naming in tests. --- .../DeliveryApi/PublishedContentCacheTests.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs index 48013afa160d..f4f176755d1b 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/PublishedContentCacheTests.cs @@ -24,7 +24,7 @@ public class PublishedContentCacheTests : DeliveryApiTests private IDocumentUrlService _documentUrlService; [SetUp] - public void Setup() + public override void Setup() { var contentTypeOneMock = new Mock(); contentTypeOneMock.SetupGet(m => m.Alias).Returns("theContentType"); @@ -123,7 +123,7 @@ public void PublishedContentCache_CanGetByIds() [TestCase(true)] [TestCase(false)] - public void PublishedContentCache_GetById_SupportsDenyList(bool denied) + public void PublishedContentCache_GetById_SupportsDisallowList(bool denied) { var denyList = denied ? new[] { "theOtherContentType" } : null; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -141,7 +141,7 @@ public void PublishedContentCache_GetById_SupportsDenyList(bool denied) [TestCase(true)] [TestCase(false)] - public void PublishedContentCache_GetByRoute_SupportsDenyList(bool denied) + public void PublishedContentCache_GetByRoute_SupportsDisallowList(bool denied) { var denyList = denied ? new[] { "theContentType" } : null; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -159,7 +159,7 @@ public void PublishedContentCache_GetByRoute_SupportsDenyList(bool denied) [TestCase("theContentType")] [TestCase("theOtherContentType")] - public void PublishedContentCache_GetByIds_SupportsDenyList(string deniedContentType) + public void PublishedContentCache_GetByIds_SupportsDisallowList(string deniedContentType) { var denyList = new[] { deniedContentType }; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -177,7 +177,7 @@ public void PublishedContentCache_GetByIds_SupportsDenyList(string deniedContent } [Test] - public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDenyList() + public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDisallowList() { var denyList = new[] { "theContentType" }; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -189,7 +189,7 @@ public void PublishedContentCache_GetById_CanRetrieveContentTypesOutsideTheDenyL } [Test] - public void PublishedContentCache_GetByRoute_CanRetrieveContentTypesOutsideTheDenyList() + public void PublishedContentCache_GetByRoute_CanRetrieveContentTypesOutsideTheDisallowList() { var denyList = new[] { "theOtherContentType" }; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -231,7 +231,7 @@ public void PublishedContentCache_GetByIds_CanDenyAllRequestedContent() } [Test] - public void PublishedContentCache_DenyListIsCaseInsensitive() + public void PublishedContentCache_DisallowListIsCaseInsensitive() { var denyList = new[] { "THEcontentTYPE" }; var publishedContentCache = CreateApiPublishedContentCache(CreateDeliveryApiSettings(denyList)); @@ -313,7 +313,7 @@ public void PublishedContentCache_AllowListIsCaseInsensitive() } [Test] - public void PublishedContentCache_AllowListTakesPrecedenceOverDenyList() + public void PublishedContentCache_AllowListTakesPrecedenceOverDisallowList() { var denyList = new[] { "theContentType" }; var allowList = new[] { "theContentType" }; @@ -323,7 +323,7 @@ public void PublishedContentCache_AllowListTakesPrecedenceOverDenyList() } [Test] - public void PublishedContentCache_AllowListIgnoresDenyListCompletely() + public void PublishedContentCache_AllowListIgnoresDisallowListCompletely() { var denyList = new[] { "theOtherContentType" }; var allowList = new[] { "theContentType" }; @@ -337,7 +337,7 @@ public void PublishedContentCache_AllowListIgnoresDenyListCompletely() } [Test] - public void PublishedContentCache_EmptyAllowListFallsBackToDenyList() + public void PublishedContentCache_EmptyAllowListFallsBackToDisallowList() { var denyList = new[] { "theContentType" }; string[] allowList = Array.Empty();