The Wayback Machine - https://web.archive.org/web/20220422184617/https://github.com/dotnet/aspnetcore/issues/34545
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Configuring minimal APIs for authorization should be suitably simple & featured #34545

Open
DamianEdwards opened this issue Jul 20, 2021 · 15 comments
Assignees
Labels
area-web-frameworks Cost:XL feature-minimal-actions Needs: Design

Comments

@DamianEdwards
Copy link
Contributor

@DamianEdwards DamianEdwards commented Jul 20, 2021

Configuring authentication and authorization requirements for minimal APIs should be suitably inline with the overall experience.

Details required...

@DamianEdwards DamianEdwards added area-runtime feature-minimal-actions labels Jul 20, 2021
@DamianEdwards DamianEdwards added the Needs: Design label Jul 20, 2021
@rafikiassumani-msft rafikiassumani-msft added this to Ready in Minimal APIs 6.0 via automation Jul 20, 2021
@rafikiassumani-msft rafikiassumani-msft moved this from Ready to Need review in Minimal APIs 6.0 Jul 20, 2021
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Jul 20, 2021

Thanks for contacting us.

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

@bradygaster
Copy link
Member

@bradygaster bradygaster commented Jul 20, 2021

@DamianEdwards @davidfowl @LadyNaggaga do we have a minimum set of authz providers we wish to support? "that which we support now," meaning Microsoft Identity Platform (MIP/AAD), ASP.NET Core identity, and identity server? I'd prioritize them in that order, tbh.

@davidfowl
Copy link
Member

@davidfowl davidfowl commented Jul 20, 2021

I don't know the scope of this work, this doesn't even work by default (AFAIK) using MVC, so it's low on the priority list IMO.

@DamianEdwards
Copy link
Contributor Author

@DamianEdwards DamianEdwards commented Jul 20, 2021

Right, we need to figure out what's required here. It wasn't intended to be a "make the thing work automagically" but rather to force us to actually try it (I think @glennc started) and if anything falls out of that that's considered a real showstopper then it would get paged-in.

@LadyNaggaga
Copy link

@LadyNaggaga LadyNaggaga commented Jul 20, 2021

Here is @glennc sample. This is how Glenn got auth to work with minimal APIs. I don't know if this is the experience we want

@DamianEdwards
Copy link
Contributor Author

@DamianEdwards DamianEdwards commented Jul 20, 2021

Wow OK, so the actual adding auth & CORS to the app isn't bad at all, but configuring Swashbuckle to support the auth scheme is quite a lot of code.

@rafikiassumani-msft rafikiassumani-msft removed this from the Next sprint planning milestone Jul 22, 2021
@rafikiassumani-msft rafikiassumani-msft added this to the 6.0-rc2 milestone Jul 22, 2021
@rafikiassumani-msft rafikiassumani-msft removed this from Need review in Minimal APIs 6.0 Jul 22, 2021
@adityamandaleeka adityamandaleeka added area-web-frameworks and removed area-runtime labels Aug 27, 2021
@rafikiassumani-msft rafikiassumani-msft removed this from the 6.0-rc2 milestone Sep 14, 2021
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Sep 14, 2021
@rafikiassumani-msft
Copy link
Contributor

@rafikiassumani-msft rafikiassumani-msft commented Dec 28, 2021

Items to consider:

  • Simple configurations for Oauth2 (JWT), Identity Providers
  • Simple ways to validate claims at the endpoint level. This will include providing simple extension methods or attributes to validate roles, scopes, groups at the endpoint level as opposed to global policy configurations.
  • Should we consider providing a simpler way to issue JWT tokens (First-class API) baked into the framework
  • AuthN and Authorization for grouped endpoints

I have written some raw thoughts in the following gist: https://gist.github.com/rafikiassumani-msft/1c8bfa9b4b05444a5473efe2ecbe7191

@rafikiassumani-msft rafikiassumani-msft added the Cost:XL label Dec 28, 2021
@rafikiassumani-msft rafikiassumani-msft added the triage-focus label Jan 11, 2022
@rafikiassumani-msft rafikiassumani-msft self-assigned this Jan 13, 2022
@captainsafia
Copy link
Contributor

@captainsafia captainsafia commented Jan 24, 2022

I had originally categorized this under the OpenAPI work in #34514. Since this topic isn't strictly related to OpenAPI + auth, I'm removing it from that epic.

Note: we have another issue that more specifically tracks OpenAPI + auth.

@DamianEdwards
Copy link
Contributor Author

@DamianEdwards DamianEdwards commented Jan 25, 2022

@davidfowl and I chatted about this briefly last week and got the start of an idea. Not saying much I know but we'll look into it a bit more and report back. General gist of the idea is to elevate authnz configuration to a builder-level concern and auto-add the required services and middleware, along the lines of:

var builder = WebApplication.CreateBuilder();

// Top-level auth configuration, auto-adds required middleware to pipeline after routing middleware
builder.ConfigureAuthentication(AuthenticationKind.JwtBearer);
// Or perhaps a property?
builder.Authentication.Kind = AuthenticationKind.JwtBearer;
// Maybe overloads of Create/CreateBuilder?
// var builder = WebApplication.CreateBuilder(AuthenticationKind.JwtBearer);
// Maybe simple top-level policy too
builder.Authorization.AddPolicy("todo:read-write",
    p => p.RequireAuthenticatedUser().RequireClaim("scope", "todo:read-write")

builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSqlite<TodoDb>("Data Source=todos.db");

var app = builder.Builder();

app.MapGet("/todos", async (TodoDb db) => await db.Todos.ToListAsync());
app.MapGet("/todos/{id}", async (int id, TodoDb db) => await db.Todos.FindAsync(id) is Todo todo
    ? Results.Ok(todo)
    : Results.NotFound())
app.MapPost("/todos", async (Todo todo, TodoDb db) =>
    {
        db.Todos.Add(todo);
        await db.SaveChangesAsync();
        return Results.Created($"/todos/{todo.Id}", todo);
    })
    // This will throw at startup if authentication is not configured
    .RequireAuthorization("todo:read-write");
app.MapDelete("/todos/{id}", async (int id, TodoDb db) =>
    {
        if (await db.Todos.FindAsync(id) is Todo todo)
        {
            db.Todos.Remove(todo);
            await db.SaveChangesAsync();
            return Results.Ok(todo);
        }

        return Results.NotFound();
    })
    // Inline simple user claims checking, potentially need to figure out lifetime issues of these though
    .RequireAuthorization(user => user.HasClaim("scope", "todo:delete"));

app.Run();

class Todo
{
    public int Id { get; set; }
    public string? Title { get; set; }
    public bool IsComplete { get; set; }
}

class TodoDb : DbContext
{
    public TodoDb(DbContextOptions<TodoDb> options)
        : base(options) { }

    public DbSet<Todo> Todos => Set<Todo>();
}

@rafikiassumani-msft
Copy link
Contributor

@rafikiassumani-msft rafikiassumani-msft commented Jan 25, 2022

@DamianEdwards This is a good start for the discussion, a few things to consider:

  1. What if the API or endpoint requires multiple scopes? With the first attempt, will the expectation be to repeat the RequiredAuthorization as shown below?
  app.MapDelete("/todo/{id}", async (int id, Todo todo) => {...} )
       .RequireAuthorization(user => user.HasClaim("scope", "todo:delete")
       .RequireAuthorization(user => user.HasClaim("groups", "admin");

There are some nuances to clarify. RequireAuthorization(user => user.HasClaim("scope", "todo:delete") could be a little bit confusing as we are checking the claim, but yet passing the scope as the value. When doing API authZ, I have often seen two patterns:

  • You check the scopes as shown in the suggestion below:
   app.MapDelete("/todo/{id}", async (int id, Todo todo) => {...} )
       .RequireAuthorization(principal => principal.HasScopes("scope1", "scope2", ...)
  • You check the claims. Please note that scopes are usually mapped to claims (Not all the time but most of the time). See example below with a group or role claim checks :
  app.MapDelete("/todo/{id}", async (int id, Todo todo) => {...} )
       .RequireAuthorization(principal => principal.HasClaim("roles", "can:delete")
       .RequireAuthorization(principal => principal .HasClaim("groups", "admin");

Notice that I am using principal, because there might not be a user in context for the case of API-to-API call or background jobs that do a client credentials flow (Oauth2) for obtaining a JWT token.

  1. Create new extension methods RequiredScopes(...) or RequireCaims(...) that will be a combination of RequireAuthorization( ) plus scope checking or claim checking.

@DamianEdwards
Copy link
Contributor Author

@DamianEdwards DamianEdwards commented Jan 25, 2022

@rafikiassumani-msft yep treat my example as pseudo-code at best. Lots of details to consider.

@bloggrammer
Copy link

@bloggrammer bloggrammer commented Mar 2, 2022

How do you add custom attributes with the minimal API?

@davidfowl
Copy link
Member

@davidfowl davidfowl commented Mar 2, 2022

@bloggrammer What do you mean exactly?

@bloggrammer
Copy link

@bloggrammer bloggrammer commented Mar 2, 2022

@davidfowl please, look at this answer on SO (https://stackoverflow.com/a/68974923/12476466).

How can you implement such for a minimal API such that you can return a custom result like context.Result = new ChallengeResult(CookieAuthenticationDefaults.AuthenticationScheme);?

@kjpgit
Copy link

@kjpgit kjpgit commented Mar 27, 2022

I thought I'd add my 2 cents as a recent user of the minimal apis. I gave up on figuring out how the 'frame work' did auth and logging and config files -- for auth, everything was worded as "JWT", and too many builders and factories to comprehend, and I just want a generic header validator -- so I just did this: https://github.com/kjpgit/techdemo/blob/274811d854d614bb09966983153ebc2339db1504/dotnet_ultra_minimal/program.cs#L96-L108

And honestly, I like it. The MyUserContext is passed in explicitly to each function, or raises an exception if it can't be constructed, so you know it's correct unlike an [Attribute] which was silently ignored(!) when I tried it at the class level.

If your "new simple solution" isn't as simple as what I did... basically a few lines of code... I see no reason to switch :-)

On one person projects, just an AsyncLocal and a custom, trivial stage(s) in the pipeline to handle auth and final post-request structured logging seem sufficient for me...

@rafikiassumani-msft rafikiassumani-msft changed the title Configuring minimal APIs for authorization should be suitably simple & featured [Epic]: Configuring minimal APIs for authorization should be suitably simple & featured Apr 4, 2022
@rafikiassumani-msft rafikiassumani-msft removed the triage-focus label Apr 4, 2022
@rafikiassumani-msft rafikiassumani-msft changed the title [Epic]: Configuring minimal APIs for authorization should be suitably simple & featured Discussion: Configuring minimal APIs for authorization should be suitably simple & featured Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-frameworks Cost:XL feature-minimal-actions Needs: Design
Projects
None yet
Development

No branches or pull requests

9 participants