Claim-Based Authorization For MVC Actions
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
Note: the original suggestion is moot - there's a new proposal [further down](https://github.com/dotnet/aspnetcore/issues/49431#issuecomment-1636977890).
## 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 in`MinimalApi` 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:
```json
{
"sub": "user_id",
"permission": ["P1", "P2"]
}
```
There is an easy way to have this in **MinimalApis**:
```c#
app.MapGet("/", () => "Granted").RequireAuthorization(policy => policy.RequireClaim("permission", "P1"));
```
But what about `Controller Actions`? The developer must hack all of the following steps!
```c#
builder.Services.AddAuthorization(options =>
{
options.AddPolicy("RequiresPermission", policy => policy.AddRequirements(new MustHavePermissionRequirement()));
});
```
```c#
[RequiresPermission("P1")]
public async Task Index()
{
return "Granted";
}
```
```c#
[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](https://source.dot.net/#Microsoft.AspNetCore.Authorization/PassThroughAuthorizationHandler.cs)) `MustHavePermissionRequirement` should look like sth like this:
```c#
public Task HandleAsync(AuthorizationHandlerContext context)
{
if (context.Resource is HttpContext httpContext)
{
var endpoint = httpContext.GetEndpoint();
var authDatum = endpoint?.Metadata.GetOrderedMetadata() ?? Array.Empty();
var permissionsString = authDatum.Select(x => x.Permissions).FirstOrDefault();
var requiredPermissions = permissionsString?.Split(",").Select(x => x.Trim()) ?? Array.Empty();
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](https://source.dot.net/#Microsoft.AspNetCore.Authorization.Policy/AuthorizationMiddleware.cs,109), there is a [CombineAsync](https://source.dot.net/#Microsoft.AspNetCore.Authorization/AuthorizationPolicy.cs,152) 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`:
```c#
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:
```diff
namespace Microsoft.AspNetCore.Authorization;
public interface IAuthorizeData
{
///
/// Gets or sets the policy name that determines access to the resource.
///
string? Policy { get; set; }
///
/// Gets or sets a comma delimited list of roles that are allowed to access the resource.
///
string? Roles { get; set; }
+ ///
+ /// Gets or sets the claim type name that is allowed to access the resource
+ ///
+ string? AllowedClaim { get; set; }
+
+ ///
+ /// Gets or sets a comma delimited list of claims that are allowed to access the resource.
+ ///
+ string? AllowedClaimValues { get; set; }
+
///
/// Gets or sets a comma delimited list of schemes from which user information is constructed.
///
string? AuthenticationSchemes { get; set; }
}
```
So that we can explicitly combine allowed claims with the other policies in `CombineAsync` method like this:
```c#
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
```c#
[Authorize(AllowedClaimType="permission", AllowedClaimValues="p1, p2")]
public async Task 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](https://github.com/dotnet/aspnetcore/compare/main...amiru3f:aspnetcore:main)
Contributor guide
Research direction
Start with the newer proposal in the issue comments, then read AuthorizationMiddleware.cs, AuthorizationPolicy.cs, IAuthorizeData, and AuthorizeAttribute. Review the noted Razor authorization integrations and existing authorization tests before defining the affected public APIs and test coverage; done means an accepted design and complete implementation across the affected integrations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- authorization, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100