Route hijacking with restricted public access - protected page request is not handled by custom login page controller

[mattmawford]

Which exact Umbraco version are you using? For example: 8.13.1 - don't just write v8

9.0.0-rc001

Bug summary

In v8, when a custom controller is in place for route hijacking on a member login page, a request to a protected page is handled by the custom member login page controller, and returns the login page template.

In v9, a similar request, whilst still returning the login page template, is handled by the default render controller. In other words, the custom login page controller only works of you navigate direct to the login page - it does not work if the login template is presented due to access being restricted to the requested page.

Specifics

My login page document type is MemberLoginPage
In v8, my route hijacking controller is like:

public class MemberLoginPageController : RenderMvcController
{
    public override ActionResult Index(ContentModel model)
    {
        // do custom stuff here
        return CurrentTemplate(model);
    }
}

In v9, it's

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

    public override IActionResult Index()
    {
        // do custom stuff here
        return CurrentTemplate(CurrentPage);
    }
}

Steps to reproduce

Create 3 document types for HomePage, TextPage and MemberLoginPage. Create content, with Restrict Public Access on the Restricted Access TextPage node like this:

In Visual Studio, create a custom controller called MemberLoginPageController (as above in Specifics), and place a breakpoint on the return line in the Index() method. Debug the solution, and browse to /login/ and /restricted-access/ in turn.

Expected result / actual result

I'd expect the breakpoint to be hit when browsing to both /login/ and /restricted-access/, but the breakpoint is only hit when browsing to /login/.

[nul800sebastiaan]

Thanks for the report @mattmawford - I'll see if we can find someone to say something clever about this! The holiday season has kicked in though, so it might take a little longer, apologies in advance! 😅

[warrenbuckley]

Hi @mattmawford
Thanks for the report I am able to reproduce this bug as you describe.

I have done some initial investigation and will need some help from a fellow colleague who is on summer holiday as Seb mentions.

[warrenbuckley]

Investigation Notes

Here are some notes of my digging and debugging I have done for anyone else who comes along to this issue on return from holiday.

• PublicAccessMiddleware calls the method SetPublishedContentAsOtherPageAsync
• This will then assign the login node as the PublishedContent on the UmbracoRouteValues object
• With the updated PublishedContent UmbracoRouteValuesFactory called that calls CheckHijackedRoute
• This discovers all classes that are IRenderControllers with the ControllerActionSearcher
• It manages to find the HijackedController class MemberLoginController and the Index action
• The UmbracoRouteValues contains the correct controller and action to use
• This is set in HttpContext.Features

Comparing this with a direct request to the login page it will not call PublicAccessMiddleware but it will still call into UmbracoRoutesFactory and will carry on the logic to find a HijackedController and set it on the UmbracoRouteValues in the HttpContext.Features.

So I am now very perplexed, hopefully with these notes and a fresh pair of eyes on this can spot what the problem is.

[AndyButland]

Have perhaps made a bit of progress. The issue seems to be that the UmbracoRouteValueTransformer.TransformAsync() method is important for mapping the routing information stored in UmbracoRouteValues. This runs when requesting Umbraco pages.

In this case though, whilst it runs for the originally requested page, it doesn't run again after the PublicAccessMiddleware has done it's thing and so presumably the originally set values on the RouteValueDictionary don't get updated.

That's as far as I've got though... we don't explicitly call this method, rather the framework does, so I haven't managed to figure out any further.

[warrenbuckley]

Thanks for the notes @AndyButland I have done some hacking & poking around with not much luck, with our UmbracoRouteValueTransformer inheriting DynamicRouteValueTransformer from ASPNetCore we are unsure where in the request pipeline this gets called and I assume we are too late in the flow when we re-assign the PublishedContent Node/Template etc in the PublicAccessMiddleware that the controller routing from ASPNet has already been decided.

With you mentioning the values in RouteValueDictionary being incorrect my hack or thought was that perhaps I could modify the values & re-assign it back on the HTTPContext.Request like so, but this had no effect.

// Values in 'RouteValueDictionary' are wrong - update them
var routeValues = httpContext.Request.RouteValues;

// Modify values & re-assign
routeValues[Constants.Web.Routing.ControllerToken] = updatedRouteValues.ControllerName;
routeValues[Constants.Web.Routing.ActionToken] = updatedRouteValues.ActionName;
httpContext.Request.RouteValues = routeValues;

There is also this method but I am unsure how it is supposed to be used and wonder if this would help ensuring we route to the custom controller/hijacked route logic. httpContext.SetEndpoint()

Seems some further source code diving of the ASPNet repro may needed to be done to figure this out and needs some smarter people such as @bergmania to investigate

[AndyButland]

Afraid have made little more progress with further investigation, but just to document what I've tried.

Similar to Warren above - before I'd realised he'd tried! - I found looking to reset the route values didn't have any effect. Slightly different syntax, so worth noting I guess, but I suspect it's doing the same thing under the hood:

httpContext.GetRouteData().Values[Constants.Web.Routing.ControllerToken] = updatedRouteValues.ControllerName;
httpContext.GetRouteData().Values[Constants.Web.Routing.ActionToken] = updatedRouteValues.ActionName;

There's not much documentation on DynamicRouteValueTransformer, but from what there is I can see there's a parameter note that implementations should not modify the provided values parameter, rather return a new one. So I've done that, i.e. with...

var result = new RouteValueDictionary()
    {
        { ControllerToken, umbracoRouteValues.ControllerName },
    };

if (string.IsNullOrWhiteSpace(umbracoRouteValues.ActionName) == false)
{
    result.Add(ActionToken, umbracoRouteValues.ActionName);
}

return result;

... which is perhaps worth doing to make sure we are in alignment with this notice, but it doesn't help with the current issue.

I had one last thought - which is perhaps cheating - but I wondered why we don't simply issue a redirect from the PublicAccessMiddleware, rather than continuing with the current request and looking to modify it? Then of course the request pipeline and routing would start over, and we'd get the behaviour we expect. The URL would change, but seems that's not such a bad thing - in fact may well be expected in the sense of a redirect to a login page when you try to access something you don't have permissions for, even if this isn't how Umbraco has traditionally behaved.

[mattmawford]

@AndyButland - personally I feel it would be a shame to loose the existing behaviour. I find it's useful not to have been redirected to login, this way we have access to the original request and URL - essential when we want to know where the user is trying to go.

[AndyButland]

Yes, agreed, even if there were a change like this we'd want to pass on that information - so redirect e.g. to /login/?returnUrl=/my-restricted-page. But it's just a comment from me at the moment, no plans.

[konius]

Will this get addressed for v9 launch or use workaround for now?

[Shazwazza]

Fixed in #11155, ready for review

[konius]

I've tried this in v9-rc004, however if I have "Restrict Public Access" set on a page trying to access any pages underneath the LoginPageController gets hit all the time for any pages underneath the restricted page even when logged in and I can't navigate to any of the pages below.

I'd except these controllers to be hit:

• AccountPageController
• OrderHistoryController
• EditProfileController
• etc

[nul800sebastiaan]

@konius Unfortunately we found that this fix introduced a new bug in RC4, which is what you're seeing now. It will be fixed for the final release due out tomorrow. The PR to fix this new issue is here: #11193