dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.59k stars 10.06k forks source link

Add support for metadata-only endpoints to support path-based authorization, output caching, etc. #43642

Open DamianEdwards opened 2 years ago

DamianEdwards commented 2 years ago

Related #43352

There are scenarios where it is desirable to affect the behavior of endpoint-aware middleware that runs in a request pipeline that will be handled by a non-endpoint-aware middleware (rather than an actual endpoint) such that the endpoint-aware middleware performs its operations as if the request-handling middleware were actually an endpoint.

For example, the static files middleware handles requests but does not do so via endpoints. Rather, it is a terminal middleware for requests with paths that map to static files in the configured file provider. The authorization middleware is an endpoint-aware middleware that uses metadata from the current request endpoint to perform authorization actions and will no-op for requests with no active endpoint. This means that one can't use the authorization middleware to enforce authorization for static files. The output cache middleware is similar.

The repo AspNetCorePathAuthorization demonstrates a concept of "metadata-only endpoints" that allows endpoints to be registered that have only metadata, and no actual endpoint handler. These endpoints only exist for the purpose of effectively adding metadata to arbitrary route paths, either adding metadata to existing real endpoints, or providing metadata for requests with no real endpoint but with a matching path. This metadata can then be used by endpoint-aware middleware to perform operations relevant for the current request.

An example of what this API could look like:

var builder = WebApplication.CreateBuilder(args);

var app = builder.Build();

// Add metadata to every request in the app
app.MapMetadata("/{**subpath}", new { Whatever = "This is on every request now!" });

// Add metadata to all requests under /public in the app
app.MapMetadata("/public/{**subpath}", new MessageMetadata("Hello from /public!"));

// Potentially terminal middleware
app.Use(async (ctx, next) =>
{
    // This obviously isn't a good example but something like the static files middleware effectively does this
    if (ctx.Request.Path.StartsWithSegments("/public"))
    {
        var message = ctx.GetEndpoint()?.Metadata.GetMetadata<MessageMetadata>()?.Message;

        await ctx.Response.WriteAsync($"{message ?? "No message metadata found :("}");
        return;
    }

    await next();
});

app.Run();

record MessageMetadata(string Message);

Higher level APIs like Authorization and Output Caching could be updated to leverage this with new top-level APIs for configuring authorization and output caching based on path, e.g.:

// Authorize all requests under /users
app.RequireAuthorization("/users");

// Disable authorization for all requests under /public
app.AllowAnonymous("/public");

// Cache all requests under /expensivestuff
app.CacheOutput("/expensivestuff");
rynowak commented 2 years ago

I worry about examples like these, and user not understand the blast radius of making a change:

// Disable authorization for all requests under /public
app.AllowAnonymous("/public");

If my mental model is correct this does define an endpoint, and that endpoint would be a candidate for path matching along with all the other endpoints, including the ability to cause ambiguities (HTTP 500).

Basically these features will only work the way you when they don't overlap with other endpoints in your app. The sample here with AllowAnonymous feels like it's applying a path-matching rule, but it's actually changing your route table.

So my feedback is that this feels like it's going to be misunderstood. It puts a lot of burden on the user to understand what happens behind the scenes.

Take this example (modified from yours):

// Disable authorization for all requests under /public
app.AllowAnonymous("/public");

// Cache all requests under /public
app.CacheOutput("/public");

What happens? I think most users would expect paths under /public to be public and cached. I think it will cause an ambiguity in the route table and throw.

DamianEdwards commented 2 years ago

@rynowak the ambiguities are resolved via a custom IEndpointSelectorPolicy to avoid the issues you refer to. ~In other words, metadata-only endpoints will never be selected as matches during routing~ (see below). Ideally this would happen during the building of the route graph, rather than per-request, via a custom INodeBuilderPolicy or some such.

Edit: metadata-only endpoints can be selected as the match during routing, but only in the case there are no other candidates available. When they do match, they have a null handler, so while there's an active endpoint for the request to carry the metadata, there's no request delegate to execute.

captainsafia commented 2 years ago

It seems like this feature lends itself nicely to the design of "route-based endpoint conventions" given that "metadata-only endpoints" doesn't apply exclusively to routes that do not map to an endpoint handler.

Looking through this particular example:

// Authorize all requests under /users
app.RequireAuthorization("/users");

I wonder how this feature would intersect with route groups. When and why would a user use the syntax above when they could also do:

var users = app.MapGroup("/users");
users.RequireAuthorization();

Is the assumption that route group-based conventions would intersect with "metadata-only endpoints" in some way via the implementation?

a concept of "metadata-only endpoints" that allows endpoints to be registered that have only metadata, and no actual endpoint handler.

How common are these types of endpoints? Static file support seems like a big one. Are there other scenarios in app where a path is not going to be tied to a handler?

DamianEdwards commented 2 years ago

One would use app.RequireAuthorization("/users"); when it is known that there will be requests served under that path that aren't implemented as endpoints or they wish not to use route groups for some reason, e.g. there are files served by the static files middleware under that path.

Ultimately, the metadata on metadata-only endpoints (MOE) applies to any request that matches the route pattern for the MOE, which for the cases outlined would almost certainly include greedily capturing any sub-path. This is true whether there are actual endpoints in play or not, so they interact in that they compose, i.e. the metadata from the MOE is copied to the actual endpoint (in the prototype implementation the endpoint is actually replaced by the MatcherPolicy per request but if this were a first-class feature I'd expect this to be done during the building of the route graph).

How common are these types of endpoints? Static file support seems like a big one. Are there other scenarios in app where a path is not going to be tied to a handler?

Hard to say. Prior to ASP.NET Core 3.0 it was everything that was request-handling. Anything that didn't move to be endpoint-based would count, and I think things like Swashbuckle still use terminal middleware too.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.