Allow direct configuration of authorization policies via endpoint metadata

[DamianEdwards]

Relates to #34545

We should allow the definition and/or application of authorization policies to specific endpoints via endpoint metadata at the time they're declared. This will make configuration of resource authorization for Minimal API style applications much simpler and more inline with the principals of Minimal APIs while still enabling re-use of policies via language features rather than relying on their definition at the time authorization is added to DI.

The AuthorizationMiddleware would be updated to retrieve metadata for the current request and ensure any instances that implement IAuthorizationRequirement are passed to the IAuthorizationService for evaluation (e.g. as IAuthorizationHandler or IAuthorizeData, etc.).

New extension methods would be added to enable setting AuthorizationPolicy on endpoint definitions, as well as methods for setting IAuthorizationRequirement, IAuthorizationHandler or IAuthorizeData as metadata:

// Set authorization metadata via an instance of AuthorizationPolicy
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, AuthorizationPolicy policy) where TBuilder : IEndpointConventionBuilder;

// Set authorization metadata via a callback accepting Action<AuthorizationPolicyBuilder>
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, Action<AuthorizationPolicyBuilder> configurePolicy) where TBuilder : IEndpointConventionBuilder;

// Set authoriziation metadata via an instance of IAuthorizationRequirement
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, IAuthorizationRequirement authoriziationRequirement) where TBuilder : IEndpointConventionBuilder;

// Set authoriziation metadata via an instance of IAuthorizationHandler
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, IAuthorizationHandler authoriziationHandler) where TBuilder : IEndpointConventionBuilder;

// Set authoriziation metadata via an instance of IAuthorizeData
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, IAuthorizeData authorizeData) where TBuilder : IEndpointConventionBuilder;

Example usage:

...
app.UseAuthentication();
app.UseAuthorization();

// Create and use a policy on multiple endpoints
var policy = new AuthorizationPolicyBuilder()
    .RequireAuthenticatedUser()
    .RequireClaim("some:claim", "this-value")
    .Build();

app.MapGet("/protected", () => "you are allowed!")
    .RequireAuthorization(policy);

app.MapGet("/also-protected", () => "you are allowed!")
    .RequireAuthorization(policy);

// Create and pass a policy inline to the endpoint definition
app.MapGet("/fowlers-only-policy", () => "you are allowed!")
    .RequireAuthorization(new AuthorizationPolicyBuilder().RequireUserName("Fowler").Build());

// Define a policy directly on the endpoint via a callback accepting Action<AuthorizationPolicyBuilder>
app.MapGet("/fowlers-only-builder", () => "you are allowed!")
    .RequireAuthorization(p => p.RequireUserName("Fowler"));

// Use a custom attribute that implements IAuthorizationRequirement and IAuthorizationHandler to allow declarative metadata based authorization
app.MapGet("/fowlers-only-attribute", [RequiresUsername("Fowler")] () =>"you are allowed!");

// Use the attribute imperatively
app.MapGet("/fowlers-only-inline", () => "you are allowed!")
    .RequireAuthorization(new RequiresUsernameAttribute("Fowler"));
...

public class RequiresUsernameAttribute : Attribute, IAuthorizationHandler, IAuthorizationRequirement
{
    public RequiresUsernameAttribute(string username)
    {
        Username = username;
    }

    public string Username { get; set; }

    public Task HandleAsync(AuthorizationHandlerContext context)
    {
        if (context.User.Identity?.Name == Username)
        {
            context.Succeed(this);
        }

        return Task.CompletedTask;
    }
}

[ghost]

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.

[BrennanConroy]

public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, IAuthorizeData authorizeData) where TBuilder : IEndpointConventionBuilder;

We have this one already

aspnetcore/src/Security/Authorization/Policy/src/AuthorizationEndpointConventionBuilderExtensions.cs

Line 60 in d4e70bd

public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, params IAuthorizeData[] authorizeData)

[davidfowl]

@BrennanConroy to make the attribute work with SignalR, we need to pass the hub method metadata to the auth system so that it can invoke these attributes.

@DamianEdwards I'm not yet sure if we need to make changes to the authZ APIs to allow passing the endpoint metadata as requirements or if the caller needs to do that manually.

[HaoK]

Should we add an example of what a more complicated Permissions system would look like to see how much better this is now that the handler has access to the Attribute data? i.e. Policy = "Read" / "Write" / "Delete" can become new RequirePermission(Permissions.Read | Write | Delete) right?

[davidfowl]

Yes! @HaoK, exactly. So something like this:

public class RequirePermissionAttribute : Attribute, IAuthorizationHandler, IAuthorizationRequirement
{
    public RequirePermission(Permissions permissions)
    {
        Username = permissions;
    }

    public Permissions Permissions { get; set; }

    public Task HandleAsync(AuthorizationHandlerContext context)
    { 
        var userPerms = GetPermissionsForUser(context.User);
        if (userPerms.HasAll(Permissions))
        {
            context.Succeed(this);
        }

        return Task.CompletedTask;
    }
}

[HaoK]

So the one caveat is that the current limitation with the instance based Handler/Requirements, is you lose the ability to inject as easily, which is something worth mentioning up front, so maybe its worth updating the permission example to demonstrate how they'd get the DbContext from the request and pass it through to the GetPermissionsForUser call to make it more 'real'. Worse comes to worse they can probably always just service locate off the request in the handler context

[davidfowl]

Here's what I was thinking because this issue doesn't quite do it justice:

To summarize what feature we're adding here so it's clear:

This extends how and where you get to define authorization requirements. Today the only way to use this list of requirement is via a policy name. The policy name maps to a list of requirements and a list of authentication schemes. So there's this indirection that stops you from defining data on your resource.

Before

var builder = WebApplication.CreateBuilder();
builder.Services.AddAuthorization(options => options.AddPolicy("Policy", pb => pb.RequireClaim("myclaim")));

app.MapGet("/authed", [Authorize("Policy")] () => { });
app.MapGet("/authed2", () => { }).RequireAuthorization("Policy");

public class MyController : ControllerBase
{
    [HttpGet("/authed3")] 
    [Authorize("Policy")]
    public string Get() => "Hello";
}

public class MyHub : Hub
{
    [Authorize("Policy")]
    public Task Send(string s) => Clients.All.SendAsync("Send", "Hello World");
}

After

using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.SignalR;

var builder = WebApplication.CreateBuilder();
builder.Services.AddAuthorization();
builder.Services.AddSingleton<IAuthorizationHandler, RequiredClaimsAuthorizationHandler>();
var app = builder.Build();

app.UseAuthentication();
app.UseAuthorization();

app.MapGet("/authed", [RequireClaims("myclaim")] () => { });
app.MapGet("/authed2", () => { }).WithMetadata(new RequireClaims("myclaim"));

public class MyController : ControllerBase
{
    [HttpGet("/authed3")]
    [RequireClaims("myclaim")]
    public string Get() => "Hello";
}

public class MyHub : Hub
{
    [RequireClaims("myclaim")]
    public Task Send(string s) => Clients.All.SendAsync("Send", "Hello World");
}

class RequireClaims : Attribute, IAuthorizationRequirement
{
    public RequireClaims(string claimType, params string[] allowedValues)
    {
        ClaimType = claimType;
        AllowedValues = allowedValues;
    }

    public string ClaimType { get; set; }
    public string[] AllowedValues { get; set; }
}

class RequiredClaimsAuthorizationHandler : AuthorizationHandler<RequireClaims>
{
    protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, RequireClaims requirement)
    {
        if (context.User != null)
        {
            var found = false;
            if (requirement.AllowedValues == null || !requirement.AllowedValues.Any())
            {
                found = context.User.Claims.Any(c => string.Equals(c.Type, requirement.ClaimType, StringComparison.OrdinalIgnoreCase));
            }
            else
            {
                found = context.User.Claims.Any(c => string.Equals(c.Type, requirement.ClaimType, StringComparison.OrdinalIgnoreCase)
                                                    && requirement.AllowedValues.Contains(c.Value, StringComparer.Ordinal));
            }
            if (found)
            {
                context.Succeed(requirement);
            }
        }
        return Task.CompletedTask;
    }
}

This lets you define the requirement directly on the resource and the job of the framework is to pass this to the authZ system. This lets handlers do their thing and work as they do today. The attribute here is using the trick where the requirement can also be a handler but it doesn't require that.

[DamianEdwards]

Note you still need to call builder.Services.AddAuthorization() and app.UseAuthorization() right now (in your "After" sample) but we're going to look at ways to potentially improve that separately.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 7 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.

[davidfowl]

@BrennanConroy I added you for the SignalR work (though once we decide what do to anyone could do it).

[pranavkm]

API review:

We're happy to approve these two APIs:

// Set authorization metadata via an instance of AuthorizationPolicy
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, AuthorizationPolicy policy) where TBuilder : IEndpointConventionBuilder;

// Set authorization metadata via a callback accepting Action<AuthorizationPolicyBuilder>
public static TBuilder RequireAuthorization<TBuilder>(this TBuilder builder, Action<AuthorizationPolicyBuilder> configurePolicy) where TBuilder : IEndpointConventionBuilder;

The next two overloads look like they need further discussion outside of the scope of API review and will be reviewed later.

[HaoK]

Just checking to see what the next steps for this issue are, I have some cycles now to help, is getting #39892 in the first step?

[DamianEdwards]

@HaoK I'm actually not entirely sure what's next here, but it seems like that PR is probably it?

[HaoK]

Sure the sticking point I believe in your PR is what do we do by default in combining that with the default policy/etc right? How do we make it easy to compose / override the empty [Authorize] things

[davidfowl]

Right the problem was, what happens when you only have a requirement and but you have a requirement.

[DamianEdwards]

@HaoK I successfully tested this out in my experiments repo: https://github.com/DamianEdwards/AspNetCoreDevJwts/blob/preview4/SampleWebApi/Program.cs#L33

I also added two more extension methods that build on top of this: RequireRole(params string[] roles) and RequireScope(params string[] scopes) that I'd be interested in discussing the utility of adding to the framework.

[HaoK]

RequireRole seems pretty useful, but RequireScope seems very specific to jwt, is it that much worse if you had to do: .RequireClaim("scope", "protected:read"); instead? Alternatively we could just have that helper live in whatever jwt helper package we have similar to what you are doing, just not sure any Scope helpers make much sense in the base *.Authorization framework code

[HaoK]

what happens when you only have a requirement and but you have a requirement.

I'm going to guess that you mean when you only have a requirement but you also have a default policy?

[davidfowl]

I'm going to guess that you mean when you only have a requirement but you also have a default policy?

You forget to setup the auth middleware (both N and Z) and you add a requirement to your endpoint. Today we have this check

aspnetcore/src/Http/Routing/src/EndpointMiddleware.cs

Line 38 in 9490438

if (endpoint.Metadata.GetMetadata<IAuthorizeData>() != null &&

, we'd need to add one for auth requirements too? Feels a little messy.

[davidfowl]

Actually hold on, did we miss this for auth policies too? What happens if you attach a policy and don't have anything setup in the pipeline?

[HaoK]

Hrm, yeah I think its just metadata that's ignored

[HaoK]

Can we just be smart and auto inject the Authorization middleware if we detect any endpoints with authorization metadata? Where metadata would be any IAuthorizeData/requirements/Policy. Or are we still concerned with ordering?

[davidfowl]

I think we just need to throw for this metadata. @DamianEdwards is looking at auto injecting middlware in the web application builder cases if you configure auth.

[HaoK]

I'm using this to track the authz caching work discussed to speed up the hot path so endpoints don't combine authz policies every time

[HaoK]

Cache work done in #43124