yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Un-routable content should never be accessible in the delivery API (#14242)

Browse Source
This commit is contained in:
Kenn Jacobsen
2023-05-15 10:27:20 +02:00
committed by GitHub
parent 85d46c3e82
commit b32b8c2265
13 changed files with 163 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/Umbraco.Cms.Api.Delivery/Controllers/ByIdContentApiController.cs
View File

@@ -43,6 +43,12 @@ public class ByIdContentApiController : ContentApiItemControllerBase
return Unauthorized();
}
return await Task.FromResult(Ok(ApiContentResponseBuilder.Build(contentItem)));
IApiContentResponse? apiContentResponse = ApiContentResponseBuilder.Build(contentItem);
if (apiContentResponse is null)
{
return NotFound();
}
return await Task.FromResult(Ok(apiContentResponse));
}
}

3
src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs
View File

@@ -7,6 +7,7 @@ using Umbraco.Cms.Core.DeliveryApi;
using Umbraco.Cms.Core.Models.DeliveryApi;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Extensions;
using Umbraco.New.Cms.Core.Models;
namespace Umbraco.Cms.Api.Delivery.Controllers;
@@ -53,7 +54,7 @@ public class QueryContentApiController : ContentApiControllerBase
PagedModel<Guid> pagedResult = queryAttempt.Result;
IEnumerable<IPublishedContent> contentItems = ApiPublishedContentCache.GetByIds(pagedResult.Items);
IApiContentResponse[] apiContentItems = contentItems.Select(ApiContentResponseBuilder.Build).ToArray();
IApiContentResponse[] apiContentItems = contentItems.Select(ApiContentResponseBuilder.Build).WhereNotNull().ToArray();
var model = new PagedViewModel<IApiContentResponse>
{

10
src/Umbraco.Core/DeliveryApi/ApiContentBuilderBase.cs
View File

@@ -19,8 +19,14 @@ public abstract class ApiContentBuilderBase<T>
protected abstract T Create(IPublishedContent content, Guid id, string name, string contentType, IApiContentRoute route, IDictionary<string, object?> properties);
public virtual T Build(IPublishedContent content)
public virtual T? Build(IPublishedContent content)
{
IApiContentRoute? route = _apiContentRouteBuilder.Build(content);
if (route is null)
{
return default;
}
IDictionary<string, object?> properties =
_outputExpansionStrategyAccessor.TryGetValue(out IOutputExpansionStrategy? outputExpansionStrategy)
? outputExpansionStrategy.MapContentProperties(content)
@@ -31,7 +37,7 @@ public abstract class ApiContentBuilderBase<T>
content.Key,
_apiContentNameProvider.GetName(content),
content.ContentType.Alias,
_apiContentRouteBuilder.Build(content),
route,
properties);
}
}

27
src/Umbraco.Core/DeliveryApi/ApiContentResponseBuilder.cs
View File

@@ -1,5 +1,6 @@
using Umbraco.Cms.Core.Models.DeliveryApi;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;
using Umbraco.Extensions;
namespace Umbraco.Cms.Core.DeliveryApi;
@@ -14,12 +15,26 @@ public sealed class ApiContentResponseBuilder : ApiContentBuilderBase<IApiConten
protected override IApiContentResponse Create(IPublishedContent content, Guid id, string name, string contentType, IApiContentRoute route, IDictionary<string, object?> properties)
{
var cultures = content.Cultures.Values
.Where(publishedCultureInfo => publishedCultureInfo.Culture.IsNullOrWhiteSpace() == false) // filter out invariant cultures
.ToDictionary(
publishedCultureInfo => publishedCultureInfo.Culture,
publishedCultureInfo => _apiContentRouteBuilder.Build(content, publishedCultureInfo.Culture));
var routesByCulture = new Dictionary<string, IApiContentRoute>();
return new ApiContentResponse(id, name, contentType, route, properties, cultures);
foreach (PublishedCultureInfo publishedCultureInfo in content.Cultures.Values)
{
if (publishedCultureInfo.Culture.IsNullOrWhiteSpace())
{
// filter out invariant cultures
continue;
}
IApiContentRoute? cultureRoute = _apiContentRouteBuilder.Build(content, publishedCultureInfo.Culture);
if (cultureRoute == null)
{
// content is un-routable in this culture
continue;
}
routesByCulture[publishedCultureInfo.Culture] = cultureRoute;
}
return new ApiContentResponse(id, name, contentType, route, properties, routesByCulture);
}
}

12
src/Umbraco.Core/DeliveryApi/ApiContentRouteBuilder.cs
View File

@@ -27,7 +27,7 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
_globalSettings = globalSettings.Value;
}
public IApiContentRoute Build(IPublishedContent content, string? culture = null)
public IApiContentRoute? Build(IPublishedContent content, string? culture = null)
{
if (content.ItemType != PublishedItemType.Content)
{
@@ -42,11 +42,17 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
// in some scenarios the published content is actually routable, but due to the built-in handling of i.e. lacking culture setup
// the URL provider resolves the content URL as empty string or "#". since the Delivery API handles routing explicitly,
// we can perform fallback to the content route.
if (contentPath.IsNullOrWhiteSpace() || "#".Equals(contentPath))
if (IsInvalidContentPath(contentPath))
{
contentPath = _publishedSnapshotAccessor.GetRequiredPublishedSnapshot().Content?.GetRouteById(content.Id, culture) ?? contentPath;
}
// if the content path has still not been resolved as a valid path, the content is un-routable in this culture
if (IsInvalidContentPath(contentPath))
{
return null;
}
contentPath = contentPath.EnsureStartsWith("/");
if (_globalSettings.HideTopLevelNodeFromPath == false)
{
@@ -55,4 +61,6 @@ public sealed class ApiContentRouteBuilder : IApiContentRouteBuilder
return new ApiContentRoute(contentPath, new ApiContentStartItem(root.Key, rootPath));
}
private static bool IsInvalidContentPath(string path) => path.IsNullOrWhiteSpace() || "#".Equals(path);
}

2
src/Umbraco.Core/DeliveryApi/IApiContentBuilder.cs
View File

@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
public interface IApiContentBuilder
{
IApiContent Build(IPublishedContent content);
IApiContent? Build(IPublishedContent content);
}

2
src/Umbraco.Core/DeliveryApi/IApiContentResponseBuilder.cs
View File

@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
public interface IApiContentResponseBuilder
{
IApiContentResponse Build(IPublishedContent content);
IApiContentResponse? Build(IPublishedContent content);
}

2
src/Umbraco.Core/DeliveryApi/IApiContentRouteBuilder.cs
View File

@@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.DeliveryApi;
public interface IApiContentRouteBuilder
{
IApiContentRoute Build(IPublishedContent content, string? culture = null);
IApiContentRoute? Build(IPublishedContent content, string? culture = null);
}

7
src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParser.cs
View File

@@ -117,9 +117,12 @@ internal sealed partial class ApiRichTextParser : IApiRichTextParser
{
case Constants.UdiEntityType.Document:
IPublishedContent? content = publishedSnapshot.Content?.GetById(udi);
if (content != null)
IApiContentRoute? route = content != null
? _apiContentRouteBuilder.Build(content)
: null;
if (route != null)
{
attributes["route"] = _apiContentRouteBuilder.Build(content);
attributes["route"] = route;
}
break;

7
src/Umbraco.Infrastructure/PropertyEditors/ValueConverters/MultiUrlPickerValueConverter.cs
View File

@@ -177,14 +177,17 @@ public class MultiUrlPickerValueConverter : PropertyValueConverterBase, IDeliver
{
case Constants.UdiEntityType.Document:
IPublishedContent? content = publishedSnapshot.Content?.GetById(item.Udi.Guid);
return content == null
IApiContentRoute? route = content != null
? _apiContentRouteBuilder.Build(content)
: null;
return content == null || route == null
? null
: ApiLink.Content(
item.Name.IfNullOrWhiteSpace(_apiContentNameProvider.GetName(content)),
item.Target,
content.Key,
content.ContentType.Alias,
_apiContentRouteBuilder.Build(content));
route);
case Constants.UdiEntityType.Media:
IPublishedContent? media = publishedSnapshot.Media?.GetById(item.Udi.Guid);
return media == null

29
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentBuilderTests.cs
View File

@@ -1,6 +1,7 @@
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.DeliveryApi;
using Umbraco.Cms.Core.Models.DeliveryApi;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.PublishedCache;
@@ -61,10 +62,36 @@ public class ContentBuilderTests : DeliveryApiTests
var customNameProvider = new Mock<IApiContentNameProvider>();
customNameProvider.Setup(n => n.GetName(content.Object)).Returns($"Custom name for: {content.Object.Name}");
var builder = new ApiContentBuilder(customNameProvider.Object, Mock.Of<IApiContentRouteBuilder>(), CreateOutputExpansionStrategyAccessor());
var routeBuilder = new Mock<IApiContentRouteBuilder>();
routeBuilder
.Setup(r => r.Build(content.Object, It.IsAny<string?>()))
.Returns(new ApiContentRoute(content.Object.UrlSegment!, new ApiContentStartItem(Guid.NewGuid(), "/")));
var builder = new ApiContentBuilder(customNameProvider.Object, routeBuilder.Object, CreateOutputExpansionStrategyAccessor());
var result = builder.Build(content.Object);
Assert.NotNull(result);
Assert.AreEqual("Custom name for: The page", result.Name);
}
[Test]
public void ContentBuilder_ReturnsNullForUnRoutableContent()
{
var content = new Mock<IPublishedContent>();
var contentType = new Mock<IPublishedContentType>();
contentType.SetupGet(c => c.Alias).Returns("thePageType");
ConfigurePublishedContentMock(content, Guid.NewGuid(), "The page", "the-page", contentType.Object, Array.Empty<PublishedElementPropertyBase>());
var routeBuilder = new Mock<IApiContentRouteBuilder>();
routeBuilder
.Setup(r => r.Build(content.Object, It.IsAny<string?>()))
.Returns((ApiContentRoute)null);
var builder = new ApiContentBuilder(new ApiContentNameProvider(), routeBuilder.Object, CreateOutputExpansionStrategyAccessor());
var result = builder.Build(content.Object);
Assert.Null(result);
}
}

84
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/ContentRouteBuilderTests.cs
View File

@@ -2,6 +2,7 @@
using NUnit.Framework;
using Umbraco.Cms.Core.DeliveryApi;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.DeliveryApi;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.PublishedCache;
using Umbraco.Cms.Core.Routing;
@@ -21,6 +22,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
var result = builder.Build(root);
Assert.IsNotNull(result);
Assert.AreEqual("/", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root", result.StartItem.Path);
@@ -38,6 +40,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
var result = builder.Build(child);
Assert.IsNotNull(result);
Assert.AreEqual("/the-child", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root", result.StartItem.Path);
@@ -58,6 +61,7 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(hideTopLevelNodeFromPath);
var result = builder.Build(grandchild);
Assert.IsNotNull(result);
Assert.AreEqual("/the-child/the-grandchild", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root", result.StartItem.Path);
@@ -74,11 +78,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(false);
var result = builder.Build(child, "en-us");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child-en-us", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root-en-us", result.StartItem.Path);
result = builder.Build(child, "da-dk");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child-da-dk", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root-da-dk", result.StartItem.Path);
@@ -95,11 +101,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(false);
var result = builder.Build(child, "en-us");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root-en-us", result.StartItem.Path);
result = builder.Build(child, "da-dk");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root-da-dk", result.StartItem.Path);
@@ -116,11 +124,13 @@ public class ContentRouteBuilderTests : DeliveryApiTests
var builder = CreateApiContentRouteBuilder(false);
var result = builder.Build(child, "en-us");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child-en-us", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root", result.StartItem.Path);
result = builder.Build(child, "da-dk");
Assert.IsNotNull(result);
Assert.AreEqual("/the-child-da-dk", result.Path);
Assert.AreEqual(rootKey, result.StartItem.Id);
Assert.AreEqual("the-root", result.StartItem.Path);
@@ -144,36 +154,18 @@ public class ContentRouteBuilderTests : DeliveryApiTests
[TestCase("#")]
public void FallsBackToContentPathIfUrlProviderCannotResolveUrl(string resolvedUrl)
{
var publishedUrlProviderMock = new Mock<IPublishedUrlProvider>();
publishedUrlProviderMock
.Setup(p => p.GetUrl(It.IsAny<IPublishedContent>(), It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns(resolvedUrl);
var result = GetUnRoutableRoute(resolvedUrl, "/the/content/route");
Assert.IsNotNull(result);
Assert.AreEqual("/the/content/route", result.Path);
}
var publishedContentCacheMock = new Mock<IPublishedContentCache>();
publishedContentCacheMock
.Setup(c => c.GetRouteById(It.IsAny<int>(), It.IsAny<string?>()))
.Returns("/the/content/route");
var publishedSnapshotMock = new Mock<IPublishedSnapshot>();
publishedSnapshotMock
.SetupGet(s => s.Content)
.Returns(publishedContentCacheMock.Object);
var publishedSnapshot = publishedSnapshotMock.Object;
var publishedSnapshotAccessorMock = new Mock<IPublishedSnapshotAccessor>();
publishedSnapshotAccessorMock
.Setup(a => a.TryGetPublishedSnapshot(out publishedSnapshot))
.Returns(true);
var content = SetupVariantPublishedContent("The Content", Guid.NewGuid());
var builder = new ApiContentRouteBuilder(
publishedUrlProviderMock.Object,
CreateGlobalSettings(),
Mock.Of<IVariationContextAccessor>(),
publishedSnapshotAccessorMock.Object);
Assert.AreEqual("/the/content/route", builder.Build(content).Path);
[TestCase("")]
[TestCase(" ")]
[TestCase("#")]
public void YieldsNullForUnRoutableContent(string contentPath)
{
var result = GetUnRoutableRoute(contentPath, contentPath);
Assert.IsNull(result);
}
[TestCase(true)]
@@ -253,4 +245,38 @@ public class ContentRouteBuilderTests : DeliveryApiTests
CreateGlobalSettings(hideTopLevelNodeFromPath),
Mock.Of<IVariationContextAccessor>(),
Mock.Of<IPublishedSnapshotAccessor>());
private IApiContentRoute? GetUnRoutableRoute(string publishedUrl, string routeById)
{
var publishedUrlProviderMock = new Mock<IPublishedUrlProvider>();
publishedUrlProviderMock
.Setup(p => p.GetUrl(It.IsAny<IPublishedContent>(), It.IsAny<UrlMode>(), It.IsAny<string?>(), It.IsAny<Uri?>()))
.Returns(publishedUrl);
var publishedContentCacheMock = new Mock<IPublishedContentCache>();
publishedContentCacheMock
.Setup(c => c.GetRouteById(It.IsAny<int>(), It.IsAny<string?>()))
.Returns(routeById);
var publishedSnapshotMock = new Mock<IPublishedSnapshot>();
publishedSnapshotMock
.SetupGet(s => s.Content)
.Returns(publishedContentCacheMock.Object);
var publishedSnapshot = publishedSnapshotMock.Object;
var publishedSnapshotAccessorMock = new Mock<IPublishedSnapshotAccessor>();
publishedSnapshotAccessorMock
.Setup(a => a.TryGetPublishedSnapshot(out publishedSnapshot))
.Returns(true);
var content = SetupVariantPublishedContent("The Content", Guid.NewGuid());
var builder = new ApiContentRouteBuilder(
publishedUrlProviderMock.Object,
CreateGlobalSettings(),
Mock.Of<IVariationContextAccessor>(),
publishedSnapshotAccessorMock.Object);
return builder.Build(content);
}
}

21
tests/Umbraco.Tests.UnitTests/Umbraco.Core/DeliveryApi/MultiNodeTreePickerValueConverterTests.cs
View File

@@ -15,13 +15,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.DeliveryApi;
[TestFixture]
public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTests
{
private MultiNodeTreePickerValueConverter MultiNodeTreePickerValueConverter()
private MultiNodeTreePickerValueConverter MultiNodeTreePickerValueConverter(IApiContentRouteBuilder? routeBuilder = null)
{
var expansionStrategyAccessor = CreateOutputExpansionStrategyAccessor();
var contentNameProvider = new ApiContentNameProvider();
var apiUrProvider = new ApiMediaUrlProvider(PublishedUrlProvider);
var routeBuilder = new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
routeBuilder = routeBuilder ?? new ApiContentRouteBuilder(PublishedUrlProvider, CreateGlobalSettings(), Mock.Of<IVariationContextAccessor>(), Mock.Of<IPublishedSnapshotAccessor>());
return new MultiNodeTreePickerValueConverter(
PublishedSnapshotAccessor,
Mock.Of<IUmbracoContextAccessor>(),
@@ -274,4 +274,21 @@ public class MultiNodeTreePickerValueConverterTests : PropertyValueConverterTest
Assert.NotNull(result);
Assert.IsEmpty(result);
}
[Test]
public void MultiNodeTreePickerValueConverter_YieldsNothingForUnRoutableContent()
{
var publishedDataType = MultiNodePickerPublishedDataType(false, Constants.UdiEntityType.Document);
var publishedPropertyType = new Mock<IPublishedPropertyType>();
publishedPropertyType.SetupGet(p => p.DataType).Returns(publishedDataType);
// mocking the route builder will make it yield null values for all routes, so there is no need to setup anything on the mock
var routeBuilder = new Mock<IApiContentRouteBuilder>();
var valueConverter = MultiNodeTreePickerValueConverter(routeBuilder.Object);
var inter = new Udi[] { new GuidUdi(Constants.UdiEntityType.Document, PublishedContent.Key) };
var result = valueConverter.ConvertIntermediateToDeliveryApiObject(Mock.Of<IPublishedElement>(), publishedPropertyType.Object, PropertyCacheLevel.Element, inter, false) as IEnumerable<IApiContent>;
Assert.NotNull(result);
Assert.IsEmpty(result);
}
}