Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes 10730 - Route hijacking with public access #11155

Merged
merged 1 commit into from
Sep 21, 2021

Conversation

Shazwazza
Copy link
Contributor

Fixes #10730 - Route hijacking with public access

The reason why it doesn't work is because DynamicRouteValueTransformer is responsible for assigning the controller/action tokens. ASP.NET then uses it's response to map to an endpoint (controller/action). Our public access middleware executes after the transformer and re-assigns our own UmbracoRouteValues so that the correct page is rendered, but this doesn't affect the results returned from our transformer - which is why the original endpoint is selected.

The endpoint selection is stored in the HttpContext as a feature: IEndpointFeature. It may be possible to replace this before endpoint is executed within the middleware but this is against best practices (undocumented) and will probably result in some caching not being done behind the scenes in aspnet.

The only real way to deal with this is going to be to move the public access code directly up into the original routing pipeline and not use middleware so that we can return the correct route values from within our route transformer so aspnet selects the correct endpoint from that data.

  • Changes PublicAccessMiddleware to IPublicAccessRequestHandler and puts the logic for this directly inside of the transformer
  • The IPublicAccessRequestHandler no longer sets the httpfeature for UmbracoRouteValues, it just returns new ones or the original ones
  • The transformer logic rearranged a bit:
    • We were checking for posted values even if there was no content matched, so the no content check was moved above
    • We check public access before checking posted values - since this would have also occurred with the old middleware solution

Testing

@bergmania
Copy link
Member

I can confirm this fixes #10730 and surface controllers do still work as expected.. Code changes also makes good sense, with this description :)

Amazing work - as always 💪

@bergmania bergmania merged commit 4edf29e into v9/dev Sep 21, 2021
@bergmania bergmania deleted the v9/bugfix/10730-route-hijacking-public-access branch September 21, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants