Claim-Based Authorization For MVC Actions

[amiru3f]

Note: the original suggestion is moot - there's a new proposal further down.

Background and Motivation

Hey guys.

As you know, there are several ways of Authorization in Aspnetcore. Policy, Role, and Claim based authorization.
Consider a situation that I want to protect a specific action (either inMinimalApi or Controller based style) depending on some specific user Permissions with the following considerations:

• Permissions are simply located in the user claims with ClaimType=permission
• Claims are available in either JWT, SAML token or even intercepted through IClaimsTransformation before entering the authorization process

As an example, the user claims look like sth like this:

{
 "sub": "user_id",
 "permission": ["P1", "P2"]
}

There is an easy way to have this in MinimalApis:

app.MapGet("/", () => "Granted").RequireAuthorization(policy => policy.RequireClaim("permission", "P1"));

But what about Controller Actions? The developer must hack all of the following steps!

builder.Services.AddAuthorization(options =>
{
    options.AddPolicy("RequiresPermission", policy => policy.AddRequirements(new MustHavePermissionRequirement()));
});

[RequiresPermission("P1")]
public async Task<string> Index()
{
    return "Granted";
}

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)]
internal sealed class RequiresPermissionAttribute : Attribute, IAuthorizeData
{
    public string Permissions { get; }
    public RequiresPermissionAttribute(string permissions)
    {
        Permissions = permissions;
        Policy = "RequiresPermission";
    }

    //
    // Summary:
    //     Gets or sets the policy name that determines access to the resource.
    public string? Policy { get; set; }

    //
    // Summary:
    //     Gets or sets a comma delimited list of roles that are allowed to access the resource.
    public string? Roles { get; set; }

    //
    // Summary:
    //     Gets or sets a comma delimited list of schemes from which user information is
    //     constructed.
    public string? AuthenticationSchemes { get; set; }
}

The self-handling (Please see this) MustHavePermissionRequirement should look like sth like this:

public Task HandleAsync(AuthorizationHandlerContext context)
{
    if (context.Resource is HttpContext httpContext)
    {
        var endpoint = httpContext.GetEndpoint();
        var authDatum = endpoint?.Metadata.GetOrderedMetadata<RequiresPermissionAttribute>() ?? Array.Empty<RequiresPermissionAttribute>();
        var permissionsString = authDatum.Select(x => x.Permissions).FirstOrDefault();
        var requiredPermissions = permissionsString?.Split(",").Select(x => x.Trim()) ?? Array.Empty<string>();
        var userPermissions = context.User.Claims.Where(x => x.Type == "permission").ToList();

        //To check if the user has the specific permission or not

        context.Succeed(this);
    }

    return Task.CompletedTask;
}

But why?
The reason is if you take a look at AuthorizationMiddlware, there is a CombineAsync method which only cares about RoleBased authorization and also the IAuthorizeData interface does not support AllowedClaimType and AllowedClaimValues out-of-the-box.
There is a piece of CombineAsync method code that tries to extract the allowedRoles from [Authorize] attribute which is inherited from IAuthorizeData:

var rolesSplit = authorizeDatum.Roles?.Split(',');
if (rolesSplit?.Length > 0)
{
    var trimmedRolesSplit = rolesSplit.Where(r => !string.IsNullOrWhiteSpace(r)).Select(r => r.Trim());
    policyBuilder.RequireRole(trimmedRolesSplit);
    useDefaultPolicy = false;
}

But what I guess that can be done is:

Proposed API

The IAuthorizeData and consequently AuthorizeAttribute can support AllowedClaimType and AllowedClaimValues in order to be used in CombineAsync method like this:

namespace Microsoft.AspNetCore.Authorization;

public interface IAuthorizeData
{
    /// <summary>
    /// Gets or sets the policy name that determines access to the resource.
    /// </summary>
    string? Policy { get; set; }
    /// <summary>
    /// Gets or sets a comma delimited list of roles that are allowed to access the resource.
    /// </summary>
    string? Roles { get; set; }

+  /// <summary>
+  /// Gets or sets the claim type name that is allowed to access the resource
+  /// </summary>
+  string? AllowedClaim { get; set; }
+ 
+  /// <summary>
+  /// Gets or sets a comma delimited list of claims that are allowed to access the resource.
+  /// </summary>
+  string? AllowedClaimValues { get; set; }
+ 
    /// <summary>
    /// Gets or sets a comma delimited list of schemes from which user information is constructed.
    /// </summary>
    string? AuthenticationSchemes { get; set; }
}

So that we can explicitly combine allowed claims with the other policies in CombineAsync method like this:

var allowedClaim = authorizeDatum.AllowedClaim;
var claimSplit = authorizeDatum.AllowedClaimValues?.Split(',');

if (string.IsNullOrWhiteSpace(allowedClaim) && claimSplit?.Length > 0)
{
    var trimmedClaimSplit = claimSplit.Where(r => !string.IsNullOrWhiteSpace(r)).Select(r => r.Trim());
    policyBuilder.RequireClaim(allowedClaim!, trimmedClaimSplit);
    useDefaultPolicy = false;
}

And all of the code hacks can be removed!

Usage Examples

[Authorize(AllowedClaimType="permission", AllowedClaimValues="p1, p2")]
public async Task<string> Index()
{
    return "Granted";
}

Risks

• Razor pages and components' authorization integration (AuthorizeRouteView, AuthorizeView, AuthorizeDataAdapter)
• This is a public API change that is widely used, so any kinda breaking change can be also a risk
• For the sake of multiple Claim support, can have multiple [Authorize] on the same action

P.S. I've already implemented this in my own GitHub repo. So fill free to check if it's feasible enough to submit a pull request: link-to-the-changes

[davidfowl]

Take a look at the new feature we added in .NET 8 https://devblogs.microsoft.com/dotnet/asp-net-core-updates-in-dotnet-8-preview-4/#improved-support-for-custom-authorization-policies-with-iauthorizationrequirementdata. This should solve the problem for MVC apps without the need to hack around with strings. This feature allows your attribute to express requirements directly.

[amiru3f]

Awesome. Totally agree that it solves the problem

[davidfowl]

To follow up, here's what the above proposal would look like with the new functionality:

[AttributeUsage(AttributeTargets.Method, Inherited = false, AllowMultiple = true)]
sealed class RequireClaimsAttribute(string Name, string AllowedValues) : Attribute, IAuthorizationRequirementData
{
    public IEnumerable<IAuthorizationRequirement> GetRequirements()
    {
        yield return new ClaimsAuthorizationRequirement(Name, AllowedValues.Split(','));
    }
}

[Authorize]
[RequiredClaims("permission", "p1, p2")]
public async Task<string> Index()
{
    return "Granted";
}

[davidfowl]

Maybe we should ship this attribute in the box? It would be the equivalent of https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.authorization.authorizationpolicybuilder.requireclaim?view=aspnetcore-7.0.

Thoughts @Tratcher @halter73 ?

[amiru3f]

@davidfowl Thanks for the follow up. Actually, I've implemented that exactly like the one you mentioned, in our codes.
Thrilled to submit a pull request if it's considered by you

[davidfowl]

@amiru3f Can you reopen this issue and propose this new API instead? That would be a reasonable thing for us to add (considering all of the overloads as well).

[amiru3f]

@davidfowl sure!

Background and Motivation

Considering the problem defined above ☝️, and a user with following sample claims

{
 "sub": "user_id",
 "permission": ["P1", "P2"]
}

There is an easy way to solve that in MinimalApis:

app.MapGet("/", () => "Granted").RequireAuthorization(policy => policy.RequireClaim("permission", "P1"));

But for the sake of MVC apps, using .NET 8 Preview 4 feature, can be solved like this:

[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
[DebuggerDisplay("{ToString(),nq}")]
public class RequiresClaimAttribute(string Name, string AllowedValues) : Attribute, IAuthorizationRequirementData
{
    public IEnumerable<IAuthorizationRequirement> GetRequirements()
    {
        yield return new ClaimsAuthorizationRequirement(Name, AllowedValues?.Split(',')?.Select(value => value.Trim()));
    }
}

[Authorize] //even this can be removed by inheritance
[RequiresClaim("permission", "P1", "P2")]
//principal has to have "P1" or "P2" permission in his claims
public string Index() => "Granted";

Proposed API

The RequireClaim attribute can be supported byAspnetcore OOTB

[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
[DebuggerDisplay("{ToString(),nq}")]
public class RequiresClaimAttribute : Attribute, IAuthorizationRequirementData
{
    public RequiresClaimAttribute(string claimType, params string[] allowedValues)
    {
        _claimType = claimType;
        _allowedValues = allowedValues;
    }

    private readonly string _claimType;

    private readonly IEnumerable<string>? _allowedValues;

    public IEnumerable<IAuthorizationRequirement> GetRequirements()
    {
        yield return new ClaimsAuthorizationRequirement(_claimType, _allowedValues);
    }

    /// <inheritdoc/>>
    public override string ToString()
    {
        return DebuggerHelpers.GetDebugText(nameof(_claimType), _claimType, nameof(_allowedValues), _allowedValues);
    }
}

P.S. By inheriting RequireClaimAttribute from AuthorizeAttribute there would be less code in action but less readable and consistent (glad to know your opinion as well)

P.S. With respect to #49431 (comment) we can also simplify the usage of

app.MapGet("/", () => "Granted").RequireAuthorization(policy => policy.RequireClaim("permission", "P1"));

by adding RequireClaim extension directly to EndpointConventionBuilder which can be used like:

app.MapGet("/", () => "Granted").RequireClaim("permission", "P1");

Usage Examples

[Authorize]
[RequiresClaim("permission", "P1", "P2")] //Also can be used in controller/class level
public string Index() => "Granted";

Risks

• Role and Claims based authorization may look different in implementation:
  => [Authorize(Role=bla)] and [Authorize] + [RequiresClaim].
• Maybe sth like[AuthorizeClaimAttribute] could be a case!
• Not sure about making the attribute sealed

[brockallen]

[RequiredClaims("permission", "p1, p2")]

FWIW, hard coding these sorts of things in a string in metadata is exactly what the ASP.NET Core policy based authorization system was was trying to get away from when ASP.NET Core v1.0 was developed. Might as well go back to using the Authorize attribute with roles.