Skip to content

Simplify authentication setups with a defaultSchemeSelector overload #42834

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

Closed
1 task done
NinoFloris opened this issue Jul 20, 2022 · 9 comments
Closed
1 task done
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Milestone

Comments

@NinoFloris
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

No response

Describe the solution you'd like

Since @davidfowl talked about auth on twitter I've been thinking about what would help smooth the learning curve from easy to more complex auth setups. In our app we have quite a complex one and I remember the time when I was struggling to implement that when the 2.0 auth stack was released.

One of the issues is that the cognitive load is high for a beginner. And just like you have done with the change of picking up the single scheme as the default scheme in #42828 it pays to gradually introduce concepts when there are many.

When it comes to default scheme selection there are a few options now:

  • For simple apps we can now do AddAuthentication().AddCookie() no need to learn about schemes yet.
  • For truly 'standard' apps with multiple schemes AddAuthentication(defaultScheme: string).AddCookie().AddBearer() is next.

Then we have more 'complex' apps, where multiple schemes exist and 'default scheme' should be selected based on the request. For example you need to switch from cookie to bearer on api paths, or when there is an Auth header on the request etcetera.

At this point it gets messy:

  • You have to wade through the IMO confusing number of forwarding properties and their priorities — hopefully you've managed to ignore them on all the scheme options to this point (these should have only existed on policy scheme options, but that ship has sailed).
  • You realize ForwardDefaultSelector is the one you want. The rest is noise IMO and 99% of the time serve as red herrings (A quick github search shows ForwardDefaultSelector has most of the legitimate/useful implementations).
  • You probably want to learn about policy schemes as they keep your sensitive auth code readable.
  • At the same time you need to grok the stack well enough to understand your policy scheme now needs to replace the default scheme.
  • Alternatively - forwarding without a policy scheme - you need to add the forwarding to the current default scheme's options. Returning null or forwarding to your own scheme in the cases where you want to keep the handler as is, not a lot less confusing.

What would help reduce this complexity jump (for an arguably simple use case) is another AddAuthentication overload:

AuthenticationBuilder AddAuthentication(this IServiceCollection, Func<HttpContext,string> defaultSchemeSelector);

This would add a policy scheme as the default scheme (it should have a well known scheme constant like cookie) and setup the redirection func on its options.

Having this overload would bypass all the noise mentioned previously, exposing the most useful forwarding primitive in a logical place and introduce you to a minimal form of the underlying concepts which helps if you ever need to dive deeper.

Additional context

No response

@javiercn javiercn added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jul 20, 2022
@Tratcher
Copy link
Member

Tratcher commented Jul 20, 2022

Once you need to decide per-request which auth kind to use that's no longer a 'default'. Trying to make 'default' dynamic requires duplicating too much logic from routing or elsewhere. Instead you can mark the resources specifically E.g. You can mark API controllers to use a specific scheme. @DamianEdwards is also working on a feature that would do this:

For example you need to switch from cookie to bearer on api paths

It is possible to do what you've asked, and yes we have a feature for that, but I don't think the actual implementation of said callback leads to a clean solution. We hope to find cleaner solutions like the features Damian is working on.

@DamianEdwards
Copy link
Member

What feature am I working on that is applicable here? Do you mean the idea of metadata-only endpoints? That doesn't add anything for controllers as they already get projected to endpoints and thus can easily have metadata.

@Tratcher
Copy link
Member

@DamianEdwards
Copy link
Member

OK so just the idea of path-based authorization all up? To use the example about APIs using a different AuthN scheme, you're imagining something like this?

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
    .AddCookie()
    .AddJwtBearer("Api");

builder.Services.AddAuthorizationBuilder();

builder.Services.AddControllers();

var app = builder.Builder();

// Require anything under "/api" to use the "Api" scheme
app.RequireAuthorization("/api", policy =>
    policy.AddAuthenticationSchemes("Api")
          .RequireAuthenticatedUser());

app.MapControllers();

var apis = app.MapGroup("/api");
apis.MapGet("/hello", (ClaimsPrincipal user) => $"Hello {user.Identity?.Name}");

app.Run();

[ApiController]
// This is under "/api" so will get the AuthZ metadata from the app.RequireAuthorization("/api") call
[Route("/api/my")]
// Not actually needed but safety guarantee to ensure this controller is always marked as requiring AuthZ
[Authorize]
public class MyApiController : Controller
{
    [HttpGet]
    public IActionResult Get() => Text($"Hello from {nameof(MyApiController)}");
}

// This will use default AuthN/AuthZ
[Route("/pages")]
public class MyPagesController : Controller
{
    [HttpGet("hello")]
    public IActionResult Hello() => View();
}

@NinoFloris
Copy link
Author

Trying to make 'default' dynamic requires duplicating too much logic from routing or elsewhere

There is no problem running UseAuthentication after UseRouting, you have full visibility into the selected endpoint and its metadata if you would be so inclined. Additionally it reads as if you are arguing against policy schemes as a whole as they don't do much more than exposing apis you believe cannot lead to clean solutions.

It is possible to do what you've asked, and yes we have a feature for that, but I don't think the actual implementation of said callback leads to a clean solution. We hope to find cleaner solutions like the features Damian is working on.

The problem with tying scheme selection to endpoints is that it's not always that simple, not every auth decision is endpoint based, and not every response originates from an endpoint either. In the cases that remain it could work out very well OR needlessly require metadata to drive yet another stack (authz) just to do what you could do with the callback.

To give a compelling example from our own stack: we change the default scheme based on whether we're coming from an internal socket or not. The external/internal socket set of auth schemes are effectively mutually exclusive. I have to make that request based decision somewhere and the auth middleware running a selector callback is absolutely the right place.

Another example that's tasteful IMO is large divisions within an app: things like api vs html routes, or more generally apps running a few different 'services' under one middleware pipeline. Anywhere where you would be tempted to use app.Map(...) is a candidate for request based selection, it's there you want auth to naturally split along the same lines.

Yet another example would be switching based on custom headers or config during development (which IMO wouldn't be enough reason for a new api on its own, but it's another common case).

Only improving the apis for the metadata based approach leads to MVC-style design thinking. Where every part of the stack needs to concern itself with everything else, you might as well roll auth and authz back together, and into one big ball with endpoint routing.

That's a joke, but I hope we can avoid the dogmatism of certain things being 'cleaner' than others. Absent context, no approach is unconditionally best. I probably don't need to tell you that when it's coming from an MS employee opinions can quickly be taken as universal truth, which is an easy way to drive people away.

I believe we can improve the stack on both sides, so let's discuss the proposal based on its merits (which may not be sufficient, and that's fine too).

@davidfowl
Copy link
Member

The push towards something endpoint based makes sense the moment you start to rewrite routing (so anything path based matching). It’s much cleaner than using Map or writing a bad version of routing.

I think it’s fine to have a request based predicate if it doesn’t lead to something that looks like routing.

@ghost
Copy link

ghost commented Sep 21, 2022

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Sep 21, 2022
@DamianEdwards
Copy link
Member

Issue about adding support for metadata-only endpoints that could enable path-based authorization #43642

@ghost ghost locked as resolved and limited conversation to collaborators Oct 21, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Projects
None yet
Development

No branches or pull requests

6 participants