Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Umbraco.Core/Configuration/Models/DeliveryApiSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,25 @@ public class DeliveryApiSettings
/// <value>
/// The content type aliases that are not to be exposed.
/// </value>
/// <remarks>
/// If <see cref="AllowedContentTypeAliases"/> is configured (non-empty), this setting is ignored.
/// </remarks>
public ISet<string> DisallowedContentTypeAliases { get; set; } = new HashSet<string>();

/// <summary>
/// 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.
/// </summary>
/// <value>
/// The content type aliases that are allowed to be exposed.
/// </value>
/// <remarks>
/// When this setting is configured (non-empty), it takes precedence over <see cref="DisallowedContentTypeAliases"/>.
/// 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 <see cref="DisallowedContentTypeAliases"/>.
/// </remarks>
public ISet<string> AllowedContentTypeAliases { get; set; } = new HashSet<string>();

/// <summary>
/// Gets or sets a value indicating whether the Delivery API should output rich text values as JSON instead of HTML.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Validator for configuration represented as <see cref="DeliveryApiSettings" />.
/// </summary>
public class DeliveryApiSettingsValidator : IValidateOptions<DeliveryApiSettings>
Comment thread
NillasKA marked this conversation as resolved.
{
private readonly ILogger<DeliveryApiSettingsValidator> _logger;

public DeliveryApiSettingsValidator(ILogger<DeliveryApiSettingsValidator> logger)
=> _logger = logger;

/// <inheritdoc />
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));
}
}
}
2 changes: 1 addition & 1 deletion src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,5 @@ public IEnumerable<IPublishedContent> GetByIds(IEnumerable<Guid> contentIds)
: null;

private bool IsAllowedContentType(IPublishedContent content)
=> _deliveryApiSettings.IsAllowedContentType(content);
=> _deliveryApiSettings.IsAllowedContentType(content.ContentType.Alias);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static IUmbracoBuilder AddConfiguration(this IUmbracoBuilder builder)
{
// Register configuration validators.
builder.Services.AddSingleton<IValidateOptions<ContentSettings>, ContentSettingsValidator>();
builder.Services.AddSingleton<IValidateOptions<DeliveryApiSettings>, DeliveryApiSettingsValidator>();
builder.Services.AddSingleton<IValidateOptions<GlobalSettings>, GlobalSettingsValidator>();
builder.Services.AddSingleton<IValidateOptions<HealthChecksSettings>, HealthChecksSettingsValidator>();
builder.Services.AddSingleton<IValidateOptions<LoggingSettings>, LoggingSettingsValidator>();
Expand Down
35 changes: 32 additions & 3 deletions src/Umbraco.Core/Extensions/DeliveryApiSettingsExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Models.PublishedContent;

namespace Umbraco.Extensions;

/// <summary>
/// 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.
/// </summary>
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);

/// <summary>
/// Determines whether a content type alias is allowed to be exposed through the Delivery API.
/// </summary>
/// <param name="settings">The Delivery API settings.</param>
/// <param name="contentTypeAlias">The content type alias to check.</param>
/// <returns>
/// <c>true</c> if the content type is allowed; otherwise, <c>false</c>.
/// </returns>
/// <remarks>
/// 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.
/// </remarks>
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);
}

// 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)
Comment thread
NillasKA marked this conversation as resolved.
=> settings.DisallowedContentTypeAliases.InvariantContains(contentTypeAlias);
=> settings.IsAllowedContentType(contentTypeAlias) is false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ILogger<DeliveryApiSettingsValidator>> _loggerMock;

[SetUp]
public void SetUp() => _loggerMock = new Mock<ILogger<DeliveryApiSettingsValidator>>();

[Test]
public void Returns_Success_For_Configuration_With_Only_AllowList()
{
var validator = new DeliveryApiSettingsValidator(_loggerMock.Object);
var options = new DeliveryApiSettings
{
AllowedContentTypeAliases = new HashSet<string> { "content1", "content2" },
};

ValidateOptionsResult result = validator.Validate("settings", options);

Assert.IsTrue(result.Succeeded);
VerifyNoWarningLogged();
}

Check warning on line 34 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/DeliveryApiSettingsValidatorTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Code Duplication

The module contains 4 functions with similar structure: Returns_Success_But_Logs_Warning_For_Overlapping_Lists,Returns_Success_For_Configuration_With_No_Overlapping_Lists,Returns_Success_For_Configuration_With_Only_AllowList,Returns_Success_For_Configuration_With_Only_DisallowList. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

[Test]
public void Returns_Success_For_Configuration_With_Only_DisallowList()
{
var validator = new DeliveryApiSettingsValidator(_loggerMock.Object);
var options = new DeliveryApiSettings
{
DisallowedContentTypeAliases = new HashSet<string> { "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<string> { "content1", "content2" },
DisallowedContentTypeAliases = new HashSet<string> { "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<string> { "content1", "content2" },
DisallowedContentTypeAliases = new HashSet<string> { "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<LogLevel>().Any(level => level == LogLevel.Warning));
}
Loading
Loading