Expose method for testing for a local URL consumable without depending on MVC

[martincostello]

Background and Motivation

I have an endpoint in an Minimal API application using Razor Slices so that native AoT could be supported whilst having a web UI that has an endpoint for handling rendering a sign in page similar to this:

builder.MapGet("/sign-in", (HttpContext context) =>
{
    if (context.User.Identity?.IsAuthenticated == true)
    {
        string url = "/";

        if (context.Request.Query.TryGetValue("ReturnUrl", out var returnUrl))
        {
            url = returnUrl.ToString();
        }

        return Results.LocalRedirect(url);
    }

    return Results.Extensions.RazorSlice<Slices.SignIn>();
});

This protects against open redirects via ?ReturnUrl={value} using Results.LocalRedirect(), but in the case that a non-local URL is provided, this causes an HTTP 500 due to these lines of code:

aspnetcore/src/Http/Http.Results/src/RedirectHttpResult.cs

Lines 99 to 102 in 90a622d

	if (AcceptLocalUrlOnly && !isLocalUrl)
	{
	throw new InvalidOperationException("The supplied URL is not local. A URL with an absolute path is considered local if it does not have a host/authority part. URLs using virtual paths ('~/') are also local.");
	}

IUrlHelper.IsLocal() isn't usable here because it's part of MVC which I don't have otherwise added to the application.

To get the desired behaviour of "use ReturnUrl if local, otherwise just ignore it and go to the homepage" I'm left with a few options including:

1. Bring in MVC to use IUrlHelper.IsLocalUrl();
2. Copy the implementation of SharedUrlHelper.IsLocalUrl();
3. Use reflection to invoke SharedUrlHelper.IsLocalUrl().

This feels like a "missing piece" of functionality for general utility of lightweight Minimal APIs where MVC isn't a desired dependency of the application.

Proposed API

namespace Microsoft.AspNetCore.Http;

+public static class UrlHelper
+{
+    public static bool IsLocalUrl(string? url) => SharedUrlHelper.IsLocalUrl(url);
+}

Usage Examples

builder.MapGet("/sign-in", (HttpContext context) =>
{
    if (context.User.Identity?.IsAuthenticated == true)
    {
        string url = "/";

        if (context.Request.Query.TryGetValue("ReturnUrl", out var returnUrl))
        {
            var maybeLocal = returnUrl.ToString();
            if (UrlHelper.IsLocalUrl(maybeLocal))
            {
                url = maybeLocal;
            }
        }

        return Results.LocalRedirect(url);
    }

    return Results.Extensions.RazorSlice<Slices.SignIn>();
});

Alternative Designs

namespace Microsoft.AspNetCore.Http;

public sealed partial class RedirectHttpResult
{
+   public static bool IsLocalUrl(string? url) => SharedUrlHelper.IsLocalUrl(url);
}

Or, a new abstraction similar to IUrlHelper that is uncoupled from MVC, but then there's lots more open questions about what other functionality should or shouldn't be in it.

Risks

Duplication of class name with Microsoft.AspNetCore.Mvc.Routing.UrlHelper.

[dotnet-policy-service]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[captainsafia]

Filing this for API review. I personally like the style of exposing a static method on the RedirectResult class but I'll leave that to discuss in API review.

Parking in the Backlog for now until we review and figure out when it gets conclusion.

[amcasey]

[API Review]

1. Do we want to expose this?
2. Where do we want to put it?
3. RedirectHttpResult?
4. A new static helper type?

• This would have been useful here
• Concern: "local url" isn't standard terminology and might be misunderstood
• MVC exposes it via DI, possibly so that it can be overridden
• The argument against reimplementation is that the user's version could end up being inconsistent with ours, causing problems
• We don't have a lot of precedent for a static utility class
  • Putting it on RedirectHttpResult might be a good compromise

namespace Microsoft.AspNetCore.Http;

public sealed partial class RedirectHttpResult
{
+   public static bool IsLocalUrl(string? url) => SharedUrlHelper.IsLocalUrl(url);
}

API approved!