dotnet / dotnet/aspnetcore

Claim-Based Authorization For MVC Actions

Open
#49,431 24 comments 2 reactions 0 assignees View on GitHub
api-needs-work area-auth enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.