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

Fixes 10730 - Route hijacking with public access

Browse Source
This commit is contained in:
Shannon
2021-09-21 10:49:45 -06:00
parent 1b0911fd06
commit c4fdf808d3
6 changed files with 76 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformerTests.cs
View File

@@ -48,6 +48,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Routing
IPublishedRouter router = null,
IUmbracoRouteValuesFactory routeValuesFactory = null)
{
var publicAccessRequestHandler = new Mock<IPublicAccessRequestHandler>();
publicAccessRequestHandler.Setup(x => x.RewriteForPublishedContentAccessAsync(It.IsAny<HttpContext>(), It.IsAny<UmbracoRouteValues>()))
.Returns((HttpContext ctx, UmbracoRouteValues routeVals) => Task.FromResult(routeVals));
var transformer = new UmbracoRouteValueTransformer(
new NullLogger<UmbracoRouteValueTransformer>(),
ctx,
@@ -59,7 +63,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Routing
filter ?? Mock.Of<IRoutableDocumentFilter>(x => x.IsDocumentRequest(It.IsAny<string>()) == true),
Mock.Of<IDataProtectionProvider>(),
Mock.Of<IControllerActionSearcher>(),
Mock.Of<IEventAggregator>());
Mock.Of<IEventAggregator>(),
publicAccessRequestHandler.Object);
return transformer;
}
@@ -155,7 +160,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Routing
}
[Test]
public async Task Assigns_UmbracoRouteValues_To_HttpContext_Feature()
public async Task Null_When_No_Content_On_PublishedRequest()
{
IUmbracoContext umbracoContext = GetUmbracoContext(true);
IPublishedRequest request = Mock.Of<IPublishedRequest>();
@@ -168,6 +173,24 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Routing
var httpContext = new DefaultHttpContext();
RouteValueDictionary result = await transformer.TransformAsync(httpContext, new RouteValueDictionary());
UmbracoRouteValues routeVals = httpContext.Features.Get<UmbracoRouteValues>();
Assert.IsNull(routeVals);
}
[Test]
public async Task Assigns_UmbracoRouteValues_To_HttpContext_Feature()
{
IUmbracoContext umbracoContext = GetUmbracoContext(true);
IPublishedRequest request = Mock.Of<IPublishedRequest>(x => x.PublishedContent == Mock.Of<IPublishedContent>());
UmbracoRouteValueTransformer transformer = GetTransformerWithRunState(
Mock.Of<IUmbracoContextAccessor>(x => x.TryGetUmbracoContext(out umbracoContext)),
router: GetRouter(request),
routeValuesFactory: GetRouteValuesFactory(request));
var httpContext = new DefaultHttpContext();
RouteValueDictionary result = await transformer.TransformAsync(httpContext, new RouteValueDictionary());
UmbracoRouteValues routeVals = httpContext.Features.Get<UmbracoRouteValues>();
Assert.IsNotNull(routeVals);
Assert.AreEqual(routeVals.PublishedRequest, umbracoContext.PublishedRequest);

4
src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs
View File

@@ -8,8 +8,6 @@ using Umbraco.Cms.Infrastructure.DependencyInjection;
using Umbraco.Cms.Web.Common.Middleware;
using Umbraco.Cms.Web.Common.Routing;
using Umbraco.Cms.Web.Website.Collections;
using Umbraco.Cms.Web.Website.Controllers;
using Umbraco.Cms.Web.Website.Middleware;
using Umbraco.Cms.Web.Website.Models;
using Umbraco.Cms.Web.Website.Routing;
using Umbraco.Cms.Web.Website.ViewEngines;
@@ -51,7 +49,7 @@ namespace Umbraco.Extensions
builder.Services.AddSingleton<MemberModelBuilderFactory>();
builder.Services.AddSingleton<PublicAccessMiddleware>();
builder.Services.AddSingleton<IPublicAccessRequestHandler, PublicAccessRequestHandler>();
builder.Services.AddSingleton<BasicAuthenticationMiddleware>();
builder

2
src/Umbraco.Web.Website/Extensions/UmbracoApplicationBuilder.Website.cs
View File

@@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Web.Common.ApplicationBuilder;
using Umbraco.Cms.Web.Common.Middleware;
using Umbraco.Cms.Web.Website.Middleware;
using Umbraco.Cms.Web.Website.Routing;
namespace Umbraco.Extensions
@@ -20,7 +19,6 @@ namespace Umbraco.Extensions
/// <returns></returns>
public static IUmbracoApplicationBuilderContext UseWebsite(this IUmbracoApplicationBuilderContext builder)
{
builder.AppBuilder.UseMiddleware<PublicAccessMiddleware>();
builder.AppBuilder.UseMiddleware<BasicAuthenticationMiddleware>();
return builder;
}

18
src/Umbraco.Web.Website/Routing/IPublicAccessRequestHandler.cs Normal file
View File

@@ -0,0 +1,18 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Umbraco.Cms.Web.Common.Routing;
namespace Umbraco.Cms.Web.Website.Routing
{
public interface IPublicAccessRequestHandler
{
/// <summary>
/// Ensures that access to current node is permitted.
/// </summary>
/// <param name="httpContext"></param>
/// <param name="routeValues">The current route values</param>
/// <returns>Updated route values if public access changes the rendered content, else the original route values.</returns>
/// <remarks>Redirecting to a different site root and/or culture will not pick the new site root nor the new culture.</remarks>
Task<UmbracoRouteValues> RewriteForPublishedContentAccessAsync(HttpContext httpContext, UmbracoRouteValues routeValues);
}
}

46
src/Umbraco.Web.Website/Middleware/PublicAccessMiddleware.cs → src/Umbraco.Web.Website/Routing/PublicAccessRequestHandler.cs
View File

@@ -10,22 +10,21 @@ using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.Routing;
using Umbraco.Cms.Web.Website.Routing;
using Umbraco.Extensions;
namespace Umbraco.Cms.Web.Website.Middleware
namespace Umbraco.Cms.Web.Website.Routing
{
public class PublicAccessMiddleware : IMiddleware
public class PublicAccessRequestHandler : IPublicAccessRequestHandler
{
private readonly ILogger<PublicAccessMiddleware> _logger;
private readonly ILogger<PublicAccessRequestHandler> _logger;
private readonly IPublicAccessService _publicAccessService;
private readonly IPublicAccessChecker _publicAccessChecker;
private readonly IUmbracoContextAccessor _umbracoContextAccessor;
private readonly IUmbracoRouteValuesFactory _umbracoRouteValuesFactory;
private readonly IPublishedRouter _publishedRouter;
public PublicAccessMiddleware(
ILogger<PublicAccessMiddleware> logger,
public PublicAccessRequestHandler(
ILogger<PublicAccessRequestHandler> logger,
IPublicAccessService publicAccessService,
IPublicAccessChecker publicAccessChecker,
IUmbracoContextAccessor umbracoContextAccessor,
@@ -40,23 +39,8 @@ namespace Umbraco.Cms.Web.Website.Middleware
_publishedRouter = publishedRouter;
}
public async Task InvokeAsync(HttpContext context, RequestDelegate next)
{
UmbracoRouteValues umbracoRouteValues = context.Features.Get<UmbracoRouteValues>();
if (umbracoRouteValues != null)
{
await EnsurePublishedContentAccess(context, umbracoRouteValues);
}
await next(context);
}
/// <summary>
/// Ensures that access to current node is permitted.
/// </summary>
/// <remarks>Redirecting to a different site root and/or culture will not pick the new site root nor the new culture.</remarks>
private async Task EnsurePublishedContentAccess(HttpContext httpContext, UmbracoRouteValues routeValues)
/// <inheritdoc />
public async Task<UmbracoRouteValues> RewriteForPublishedContentAccessAsync(HttpContext httpContext, UmbracoRouteValues routeValues)
{
// because these might loop, we have to have some sort of infinite loop detection
int i = 0;
@@ -64,13 +48,13 @@ namespace Umbraco.Cms.Web.Website.Middleware
PublicAccessStatus publicAccessStatus = PublicAccessStatus.AccessAccepted;
do
{
_logger.LogDebug(nameof(EnsurePublishedContentAccess) + ": Loop {LoopCounter}", i);
_logger.LogDebug(nameof(RewriteForPublishedContentAccessAsync) + ": Loop {LoopCounter}", i);
IPublishedContent publishedContent = routeValues.PublishedRequest?.PublishedContent;
if (publishedContent == null)
{
return;
return routeValues;
}
var path = publishedContent.Path;
@@ -117,8 +101,10 @@ namespace Umbraco.Cms.Web.Website.Middleware
if (i == maxLoop)
{
_logger.LogDebug(nameof(EnsurePublishedContentAccess) + ": Looks like we are running into an infinite loop, abort");
_logger.LogDebug(nameof(RewriteForPublishedContentAccessAsync) + ": Looks like we are running into an infinite loop, abort");
}
return routeValues;
}
@@ -139,17 +125,11 @@ namespace Umbraco.Cms.Web.Website.Middleware
// we need to change the content item that is getting rendered so we have to re-create UmbracoRouteValues.
UmbracoRouteValues updatedRouteValues = await _umbracoRouteValuesFactory.CreateAsync(httpContext, reRouted);
// Update the feature
httpContext.Features.Set(updatedRouteValues);
return updatedRouteValues;
}
else
{
_logger.LogWarning("Public Access rule has a redirect node set to itself, nothing can be routed.");
// Update the feature to nothing - cannot continue
httpContext.Features.Set<UmbracoRouteValues>(null);
return null;
throw new InvalidOperationException("Public Access rule has a redirect node set to itself, nothing can be routed.");
}
}
}

32
src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs
View File

@@ -52,6 +52,7 @@ namespace Umbraco.Cms.Web.Website.Routing
private readonly IDataProtectionProvider _dataProtectionProvider;
private readonly IControllerActionSearcher _controllerActionSearcher;
private readonly IEventAggregator _eventAggregator;
private readonly IPublicAccessRequestHandler _publicAccessRequestHandler;
/// <summary>
/// Initializes a new instance of the <see cref="UmbracoRouteValueTransformer"/> class.
@@ -67,7 +68,8 @@ namespace Umbraco.Cms.Web.Website.Routing
IRoutableDocumentFilter routableDocumentFilter,
IDataProtectionProvider dataProtectionProvider,
IControllerActionSearcher controllerActionSearcher,
IEventAggregator eventAggregator)
IEventAggregator eventAggregator,
IPublicAccessRequestHandler publicAccessRequestHandler)
{
if (globalSettings is null)
{
@@ -85,6 +87,7 @@ namespace Umbraco.Cms.Web.Website.Routing
_dataProtectionProvider = dataProtectionProvider;
_controllerActionSearcher = controllerActionSearcher;
_eventAggregator = eventAggregator;
_publicAccessRequestHandler = publicAccessRequestHandler;
}
/// <inheritdoc/>
@@ -125,20 +128,10 @@ namespace Umbraco.Cms.Web.Website.Routing
};
}
IPublishedRequest publishedRequest = await RouteRequestAsync(httpContext, umbracoContext);
IPublishedRequest publishedRequest = await RouteRequestAsync(umbracoContext);
umbracoRouteValues = await _routeValuesFactory.CreateAsync(httpContext, publishedRequest);
// Store the route values as a httpcontext feature
httpContext.Features.Set(umbracoRouteValues);
// Need to check if there is form data being posted back to an Umbraco URL
PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values);
if (postedInfo != null)
{
return HandlePostedValues(postedInfo, httpContext);
}
if (!umbracoRouteValues?.PublishedRequest?.HasPublishedContent() ?? false)
{
// No content was found, not by any registered 404 handlers and
@@ -149,6 +142,19 @@ namespace Umbraco.Cms.Web.Website.Routing
return null;
}
// now we need to do some public access checks
umbracoRouteValues = await _publicAccessRequestHandler.RewriteForPublishedContentAccessAsync(httpContext, umbracoRouteValues);
// Store the route values as a httpcontext feature
httpContext.Features.Set(umbracoRouteValues);
// Need to check if there is form data being posted back to an Umbraco URL
PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values);
if (postedInfo != null)
{
return HandlePostedValues(postedInfo, httpContext);
}
// See https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.routing.dynamicroutevaluetransformer.transformasync?view=aspnetcore-5.0#Microsoft_AspNetCore_Mvc_Routing_DynamicRouteValueTransformer_TransformAsync_Microsoft_AspNetCore_Http_HttpContext_Microsoft_AspNetCore_Routing_RouteValueDictionary_
// We should apparenlty not be modified these values.
// So we create new ones.
@@ -164,7 +170,7 @@ namespace Umbraco.Cms.Web.Website.Routing
return newValues;
}
private async Task<IPublishedRequest> RouteRequestAsync(HttpContext httpContext, IUmbracoContext umbracoContext)
private async Task<IPublishedRequest> RouteRequestAsync(IUmbracoContext umbracoContext)
{
// ok, process