Fix for issue 13017 - BeginUmbracoForm doesn't work with custom umbraco routes

[justin-nevitech]

Summary

This fixes #13017 (#13017) which is an issue displaying a custom Umbraco route and a virtual page controller after submitting a form to a surface controller.

Description

When using a custom Umbraco route and a virtual page controller which submits a form using a surface controller, the surface controller action method is called but control is not returned to the virtual page afterwards resulting in a 404 error. This is because the surface controller is invoked before the action filter for the virtual page is called which means the route values (and indeed the content for the published request) is not setup correctly to return to the virtual page after the surface controller action is executed. It also means the current page is not set in the surface controller for the virtual page.

When using a virtual page controller, the route values and the content (via FindContent) are executed as part of an action filter that is called when the virtual page controller action is invoked. This code is never called after a form has been submitted via a surface controller so a 404 is displayed as no Umbraco content can be found at that URL.

The fix for this is to ensure the virtual page controller route values and content are setup before the form is handled so control can be correctly returned to the virtual page route via RedirectToCurrentPage and also so the CurrentPage correctly resolves to the content that has been returned via FindContent.

I have moved some of the code and logic that was previously in the UmbracoVirtualPageFilterAttribute class into a separate re-usable UmbracoVirtualPageRoute class which also gets called from UmbracoRouteValueTransformer prior to a surface controller form being handled. I have deliberately left the existing virtual page action filter in place and the IVirtualPageController interface as it was so this does not break any existing code or routing.

Testing

To test, create a home page (alias homePage) and a hijacked page (alias hijackedPage) and allow the hijacked page under the home page (no other properties are required). Create a home page node and underneath that a hijacked node and add the following code/templates:

Demo.cs

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Infrastructure.Persistence;
using Umbraco.Cms.Web.Common;
using Umbraco.Cms.Web.Common.ApplicationBuilder;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Website.Controllers;

namespace Umbraco.Cms.Web.UI
{
    public class DemoComposer : IComposer
    {
        public void Compose(IUmbracoBuilder builder)
        {
            builder.Services.Configure<UmbracoPipelineOptions>(options =>
            {
                options.AddFilter(new UmbracoPipelineFilter(nameof(DemoPageController))
                {
                    Endpoints = app => app.UseEndpoints(endpoints =>
                    {
                        endpoints.MapControllerRoute(
                            "DemoPageController",
                            "demo",
                            new { Controller = "DemoPage", Action = "Index" });

                        endpoints.MapControllerRoute(
                            "TestController",
                            "/test/",
                            new { Controller = "Test", Action = "Index" })
                                .ForUmbracoPage((actionExecutingContext) =>
                                {
                                    var umbracoHelper = actionExecutingContext.HttpContext.RequestServices.GetService<UmbracoHelper>();
                                    return umbracoHelper?.ContentAtRoot().First()!;
                                });
                    })
                });
            });
        }
    }

    public class TestController : UmbracoPageController
    {

        public TestController(
                    ILogger<UmbracoPageController> logger,
                    ICompositeViewEngine compositeViewEngine)
                    : base(logger, compositeViewEngine)
        { }

        [HttpGet]
        public IActionResult Index()
        {
            return View("HomePage", CurrentPage);
        }
    }

    public class DemoPageController : UmbracoPageController, IVirtualPageController
    {
        private readonly UmbracoHelper _umbracoHelper;

        public DemoPageController(
            ILogger<UmbracoPageController> logger,
            ICompositeViewEngine compositeViewEngine,
            UmbracoHelper umbracoHelper) : base(logger, compositeViewEngine) =>
            _umbracoHelper = umbracoHelper;

        public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext) =>
            _umbracoHelper.ContentAtRoot().First();

        [HttpGet]
        public IActionResult Index() =>
            View("~/Views/HomePage.cshtml", CurrentPage);
    }

    public class DemoSurfaceController : SurfaceController
    {
        public DemoSurfaceController(
            IUmbracoContextAccessor umbracoContextAccessor,
            IUmbracoDatabaseFactory databaseFactory,
            ServiceContext services,
            AppCaches appCaches,
            IProfilingLogger profilingLogger,
            IPublishedUrlProvider publishedUrlProvider)
            : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider)
        { }

        //public IActionResult Handle(string name) =>
        //    Ok(new { name });

        [HttpPost]
        public IActionResult HandleCurrentPage(string name)
        {
            var currentPage = CurrentPage;
            ViewData["name"] = name;
            TempData["name"] = name;
            return CurrentUmbracoPage();
        }

        [HttpPost]
        public IActionResult HandleRedirectToCurrentPage(string name)
        {
            var currentPage = CurrentPage;
            ViewData["name"] = name;
            TempData["name"] = name;
            return RedirectToCurrentUmbracoPage();
        }

        [HttpPost]
        public IActionResult HandleRedirectToCurrentUrl(string name)
        {
            var currentPage = CurrentPage;
            ViewData["name"] = name;
            TempData["name"] = name;
            return RedirectToCurrentUmbracoUrl();
        }
    }

    public class HijackedPageController : RenderController
    {
        public HijackedPageController(ILogger<HijackedPageController> logger, ICompositeViewEngine compositeViewEngine, IUmbracoContextAccessor umbracoContextAccessor)
            : base(logger, compositeViewEngine, umbracoContextAccessor)
        {
        }

        public override IActionResult Index()
        {
            ViewData["Hijacked"] = "Yes!";

            return CurrentTemplate(CurrentPage);
        }
    }
}

HomePage.cshtml

@using Umbraco.Cms.Web.Common.PublishedModels;
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<ContentModels.HomePage>
@using ContentModels = Umbraco.Cms.Web.Common.PublishedModels;
@{
    Layout = null;
}

<h1>@Model.Name</h1>

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleCurrentPage"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Current Page</button>
}

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleRedirectToCurrentPage"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Redirect To Current Page</button>
}

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleRedirectToCurrentUrl"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Redirect To Current URL</button>
}

<p>View Data: @ViewData["name"]</p>

<p>Temp Data: @TempData["name"]</p>

HijackedPage.cshtml

@using Umbraco.Cms.Web.Common.PublishedModels;
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<ContentModels.HijackedPage>
@using ContentModels = Umbraco.Cms.Web.Common.PublishedModels;
@{
	Layout = null;
}

<h1>@Model.Name</h1>

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleCurrentPage"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Current Page</button>
}

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleRedirectToCurrentPage"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Redirect To Current Page</button>
}

@using (Html.BeginUmbracoForm<DemoSurfaceController>("HandleRedirectToCurrentUrl"))
{
    @Html.TextBox("name")
    <button type="submit">Handle Redirect To Current URL</button>
}

<p>Hijacked: @ViewData["Hijacked"]</p>

<p>View Data: @ViewData["name"]</p>

<p>Temp Data: @TempData["name"]</p>

Tests

Scenario 1 - Home Page (this page has a normal render controller)

1. Navigate to /
2. Enter a name into the top text field and click Current Umbraco Page - The form should submit and retain the model, view data and temp data.
3. Enter a name into the middle text field and click Redirect to Current Page - The form should submit but only the temp data should be retained (as expected after a redirect).
4. Enter a name into the bottom text field and click Redirect to Current URL - The form should submit but only the temp data should be retained (as expected after a redirect).

Scenario 2 - Hijacked Page (this page has a hijacked page controller)

1. Navigate to /hijacked
2. Enter a name into the top text field and click Current Umbraco Page - The form should submit and retain the model, view data and temp data.
3. Enter a name into the middle text field and click Redirect to Current Page - The form should submit but only the temp data should be retained (as expected after a redirect).
4. Enter a name into the bottom text field and click Redirect to Current URL - The form should submit but only the temp data should be retained (as expected after a redirect).

Scenario 3 - Virtual Page (this is a virtual page controller and a custom route)

1. Navigate to /demo
2. Enter a name into the top text field and click Current Umbraco Page - The form should submit and retain the model, view data and temp data.
3. Enter a name into the middle text field and click Redirect to Current Page - The form should submit but only the temp data should be retained (as expected after a redirect). Note, this will submit and redirect to the home page as expected (as that is the URL for the home page).
4. Navigate back to /demo.
5. Enter a name into the bottom text field and click Redirect to Current URL - The form should submit but only the temp data should be retained (as expected after a redirect).

Scenario 4 - Virtual Page (this is a custom route finder delegate using a custom route)

1. Navigate to /test
2. Enter a name into the top text field and click Current Umbraco Page - The form should submit and retain the model, view data and temp data.
3. Enter a name into the middle text field and click Redirect to Current Page - The form should submit but only the temp data should be retained (as expected after a redirect). Note, this will submit and redirect to the home page as expected (as that is the URL for the home page).
4. Navigate back to /test.
5. Enter a name into the bottom text field and click Redirect to Current URL - The form should submit but only the temp data should be retained (as expected after a redirect).

All automated tests have been run and pass.

[github-actions]

Hi there @justin-nevitech, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[umbrabot]

Hi there @justin-nevitech!

First of all: A big #H5YR for making an Umbraco related contribution during Hacktoberfest! We are very thankful for the huge amount of PRs submitted, and all the amazing work you've been doing 🥇

Due to the amazing work you and others in the community have been doing, we've had a bit of a hard time keeping up. 😅 While all of the PRs for Hacktoberfest might not have been merged yet, you still qualify for receiving some Umbraco swag, congratulations! 🎉

In the spirit of Hacktoberfest we've prepared some exclusive Umbraco swag for all our contributors - including you!

As an alternative choice this year, you can opt-out of receiving anything and ask us to help improve the planet instead by planting a tree on your behalf. 🌳

Receive your swag or plant a tree! 👈 Please follow this link to fill out and submit the form, before December 18th, 2022, 09:00 AM UTC.

Following this date we'll be sending out all the swag, but please note that it might not reach your doorstep for a few months, so please bear with us and be patient 🙏

The only thing left to say is thank you so much for participating in Hacktoberfest! We really appreciate the help!

Kind regards,
The various Umbraco teams.

[bergmania]

Hi @justin-nevitech ..

Thanks a lot for this detailed PR..

I when through the test scenarios and all seems to work as expected.

I have fixed the breaking changes in constructors

[bergmania]

Cherry picked for v11/dev and v10/dev 💪

[jeroenmink2]

I think this is causing an error when integrating an Umbraco Form in the view that's served by a custom route. After the upgrade we're getting the following excetion after submitting the Umbraco Form:

System.InvalidOperationException: No service for type '<customControllerName>' has been registered. at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType) at Umbraco.Cms.Web.Common.Routing.UmbracoVirtualPageRoute.SetupVirtualPageRoute(HttpContext httpContext) at Umbraco.Cms.Web.Website.Routing.UmbracoRouteValueTransformer.TransformAsync(HttpContext httpContext, RouteValueDictionary values) at Microsoft.AspNetCore.Mvc.Routing.DynamicControllerEndpointMatcherPolicy.ApplyAsync(HttpContext httpContext, CandidateSet candidates) at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.SelectEndpointWithPoliciesAsync(HttpContext httpContext, IEndpointSelectorPolicy[] policies, CandidateSet candidateSet) at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.<Invoke>g__AwaitMatch|8_1(EndpointRoutingMiddleware middleware, HttpContext httpContext, Task matchTask) at SixLabors.ImageSharp.Web.Middleware.ImageSharpMiddleware.Invoke(HttpContext httpContext, Boolean retry) at StackExchange.Profiling.MiniProfilerMiddleware.Invoke(HttpContext context) in C:\projects\dotnet\src\MiniProfiler.AspNetCore\MiniProfilerMiddleware.cs:line 121 at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext() --- End of stack trace from previous location --- at Umbraco.Cms.Web.Common.Middleware.PreviewAuthenticationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext() --- End of stack trace from previous location --- at Umbraco.Cms.Web.Common.Middleware.UmbracoRequestLoggingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext() --- End of stack trace from previous location --- at Umbraco.Forms.Web.HttpModules.ProtectFormUploadRequestsMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext() --- End of stack trace from previous location --- at Vendr.Umbraco.Web.Mvc.VendrRequestBufferingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext() --- End of stack trace from previous location --- at UmbracoVO.Startup.<>c__DisplayClass6_0.<<AddHttpHeaders>b__0>d.MoveNext() in C:\agent\_work\17\s\UmbracoVO\Startup.cs:line 127 --- End of stack trace from previous location --- at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task)

Is it possible to support this scenario?

[bergmania]

Hi @jeroenmink2
Im pretty sure this is fixed in 11.3 and 10.5 by this PR #13910

[jeroenmink2]

You're right, updated to 10.5 and it works again. Thanks!