-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Description
Is your feature request related to a problem? Please describe.
I've been playing around with the new minimal APIs in a toy TodoApp sample application where for demonstration purposes the Todo items belong to a user and are stored in a database as belonging to a specific user Id.
Using endpoints to implement a simple API it looks like something like this prior to using the new minimal APIs:
builder.MapGet("/api/items", async (context) =>
{
var userId = context.User.FindFirst(ClaimTypes.NameIdentifier).Value;
var service = context.RequestServices.GetRequiredService<ITodoService>();
var model = await service.GetListAsync(userId, context.RequestAborted);
await context.Response.WriteAsJsonAsync(model);
}).RequireAuthorization();
Converting this over to use the new minimal APIs with a daily build of ASP.NET Core 6 preview 6 gives me something like this:
builder.MapGet("/api/items", async (ITodoService service, HttpContext context, CancellationToken cancellationToken) =>
{
var userId = context.User.FindFirst(ClaimTypes.NameIdentifier)!.Value;
return await service.GetListAsync(userId, cancellationToken);
}).RequireAuthorization();
While an improvement over the endpoint version, the HttpContext
parameter is retained due to the need to access the user to access the claims, which in some ways seems a shame for this example because all other knowledge of the HttpContext
has been abstracted away by the minimal APIs.
I could remove it and push the user down into the service and get it via IHttpContextAccessor
or I could register ClaimsPrincipal
as a scoped service to have it bound as a parameter, but they both come with the trade off that adding HttpContext accessor to DI entails.
builder.Services.AddHttpContextAccessor();
builder.Services.AddScoped<ClaimsPrincipal>((p) =>
{
var context = p.GetRequiredService<IHttpContextAccessor>();
return context.HttpContext.User;
});
Given that CancellationToken
has special casing in RequestDelegateFactory
and is supported via the custom CancellationTokenModelBinder
class for MVC, that made me wonder whether it would be worth doing a similar thing for ClaimsPrincipal
in minimal actions (and MVC?) that would automatically bind ClaimsPrincipal
to HttpContext.User
?
Either solution changes the API endpoint to something like this:
builder.MapGet("/api/items", async (ITodoService service, ClaimsPrincipal user, CancellationToken cancellationToken) =>
{
var userId = user.FindFirst(ClaimTypes.NameIdentifier).Value;
return await service.GetListAsync(userId, cancellationToken);
}).RequireAuthorization();
Describe the solution you'd like
Update RequestDelegateFactory
to support automatic binding of ClaimsPrincipal
to HttpContext.User
, which seems like would need just two small changes.
First adding an expression to get the user from the HttpContext
:
private static readonly MemberExpression UserExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.User));
Secondly updating the CreateArgument(ParameterInfo, FactoryContext)
method to handle a parameter of type ClaimsPrincipal
:
else if (parameter.ParameterType == typeof(ClaimsPrincipal))
{
return UserExpr;
}
If such support was added and parity with MVC was also desired, then also adding a corresponding model binder like this which is pretty much the same implementation as the one for CancellationToken
. In the case of MVC I guess the potential need is less than with minimal actions due to the ControllerBase.User
property being readily accessible. I only thought of this in the first place because at first I thought the model binding infrastructure came into play to bind the minimal API parameters until I looked at the RequestDelegateFactory
implementation.
public class ClaimsPrincipalModelBinder : IModelBinder
{
public Task BindModelAsync(ModelBindingContext bindingContext)
{
if (bindingContext == null)
{
throw new ArgumentNullException(nameof(bindingContext));
}
var model = (object)bindingContext.HttpContext.User;
bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true });
bindingContext.Result = ModelBindingResult.Success(model);
return Task.CompletedTask;
}
}
public class ClaimsPrincipalModelBinderProvider : IModelBinderProvider
{
private readonly ClaimsPrincipalModelBinder _modelBinder = new();
public IModelBinder? GetBinder(ModelBinderProviderContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}
if (context.Metadata.ModelType == typeof(ClaimsPrincipal))
{
return _modelBinder;
}
return null;
}
}
If support for this was deemed to be worth adding, I'd be happy to submit a PR to do so.