Authentication Handler as an Endpoint

[Kahbazi]

Now that almost everything (Controller, RazorPage, SignalR Hub, HealthCheck, Grpc) in ASP.NET Core is becoming an Endpoint, I'm curious is there a plan to make authentication handlers into endpoints?

[khellang]

It can't, as authentication needs to happen before the endpoint is executed. Endpoints are... endpoints, not middleware pipelines 😊

[Kahbazi]

I think I should have explain my point more. I'm not talking about the authentication process itself. I'm talking about the endpoints which I'm redirecting to from Identity Providers like /signin-microsoft or /signin-google . I believe these are endpoints which can be separated from authentication middleware.

[blowdart]

No there's not. Those aren't really endpoints, but ephemeral locations authentication intercepts, which, as Kristian points out needs to happen before normal endpoint code would execute.

What do you think would be gained by making them endpoints?

[Tratcher]

This is something I've briefly discussed with @rynowak. No, you couldn't convert all of an auth handler to endpoints, but moving some of the callbacks like /signin-microsoft to endpoints might be possible. That said, it would involve breaking up the auth handlers into several pieces and it's not clear what would be gained by doing so.

This part might scale better if you had a lot of auth handlers, routing is much more optimized for this scenario. The current code allocates every auth handler on every request (the schemes are singletons but the handlers are scoped).
https://github.com/aspnet/AspNetCore/blob/62351067ff4c1401556725b401478e648b66acdc/src/Security/Authentication/Core/src/AuthenticationMiddleware.cs#L42-L49

[brockallen]

+1 -- we discussed this somewhat already, but auth handlers are middleware for most requests, but endpoints for some specific requests (signouts and challenge callbacks). The signouts and challenge callbacks would benefit from being endpoints, I think -- mainly for the scale issue once you need 100+ OIDC/Ws-Fed/SAML handlers in your pipeline.

[Tratcher]

The transition might even be able to be done in a non-breaking way.

• Add a UseEndpoints option on the auth scheme options.
• The GetHandlerAsync allocation above would be skipped if UseEndpoints was enabled.
• something would give each scheme a chance to register endpoints if UseEndpoints were set.
• Since endpoints are largely stateless, they would probably allocate the handler when selected and call into their existing HandleRequestAsync logic.
• We'd have to ensure the dynamic add and remove scenarios still worked.

[khellang]

This also sounds semi-related to #6993?

[Tratcher]

@khellang distantly. Once you have endpoints then you can use them for url generation. However, most of the endpoint addresses in auth handlers are self-referential, generating them hasn't been much of an issue. The cookie auth urls called out in #6993 are more of an exception.

[Tratcher]

On a related perf note, this code even allocates handlers that don't implement IAuthenticationRequestHandler such as JwtBearer. We could probably add a scheme option to suppress that too.
https://github.com/aspnet/AspNetCore/blob/62351067ff4c1401556725b401478e648b66acdc/src/Security/Authentication/Core/src/AuthenticationMiddleware.cs#L42-L45

[Kahbazi]

you couldn't convert all of an auth handler to endpoints, but moving some of the callbacks like /signin-microsoft to endpoints might be possible.

Yes this was my intention. I should have specify better.
I would like to work on this if possible. Would you accept PR for this?

[Tratcher]

Would you accept PR for this?

Could you start by outlining a detailed design proposal? This has several moving parts that we need to make sure are accounted for before starting on a PR.

[analogrelay]

Triage: Seems reasonable, but definitely needs a bunch of design.

[Kahbazi]

Here's what I thought.

We can add a UseEndpoints option to AuthenticationOptions and use it to enable endpoints for all authentication handlers.

public class AuthenticationOptions
{
    public bool UseEndpoints { get; set; }
}

Adding and removing endpoints could happen in IAuthenticationSchemeProvider in AddSchema(AuthenticationScheme scheme) and RemoveScheme(string name). And IAuthenticationSchemeProvider should expose a EndpointDataSource via method or a property

public interface IAuthenticationSchemeProvider
{
    EndpointDataSource GetEndpointDataSource();
    //OR
    EndpointDataSource EndpointDataSource { get; }
}

I have two thought on how to get endpoints for one scheme

1. We could add a IEnumerable<Endpoint> property to AuthenticationScheme and fill it in AuthenticationBuilder extenstion methods. Here we can easily add the Endpoints to the data source, but I don't like the idea of having endpoints in extension methods.

public class AuthenticationScheme
{
    public IEnumerable<Endpoint> Endpoints { get; set; }
}

2. Add a ConfigureEndpoints() method to IAuthenticationRequestHandler so each handler can register their endpoints and deprecate HandleRequestAsync. In this way Endpoints are bind to IAuthenticationRequestHandler which is good but we need to do some Reflection or use ActivatorUtility in order to create a IAuthenticationRequestHandler from AuthenticationScheme .HandlerType.

public interface IAuthenticationRequestHandler
{
    void ConfigureEndpoints(IEndpointRouteBuilder endpointRouteBuilder);

    [Obsolete]
    Task<bool> HandleRequestAsync();
}

Since we have to cover adding and removing scheme in runtime a dynamic EndpointDataSource is needed. Well I found one in unit tests and with some change it fits our needs.

public class DynamicEndpointDataSource : EndpointDataSource
{
    private readonly ConcurrentDictionary<string, IEnumerable<Endpoint>> _endpoints;
    private CancellationTokenSource _cts;
    private CancellationChangeToken _changeToken;

    public DynamicEndpointDataSource()
    {
        _endpoints = new ConcurrentDictionary<string, IEnumerable<Endpoint>>();

        CreateChangeToken();
    }

    public override IChangeToken GetChangeToken() => _changeToken;

    public override IReadOnlyList<Endpoint> Endpoints => _endpoints.SelectMany(pair => pair.Value).ToArray();

    public void AddEndpoint(string schema, IEnumerable<Endpoint> endpoint)
    {
        _endpoints.TryAdd(schema, endpoint);

        TriggerChange();
    }

    public void RemoveEndpoint(string schema)
    {
        _endpoints.TryRemove(schema, out _);

        TriggerChange();
    }

    private void TriggerChange()
    {
        // Capture the old tokens so that we can raise the callbacks on them. This is important so that
        // consumers do not register callbacks on an inflight event causing a stackoverflow.
        var oldTokenSource = _cts;
        var oldToken = _changeToken;

        CreateChangeToken();

        // Raise consumer callbacks. Any new callback registration would happen on the new token
        // created in earlier step.
        oldTokenSource.Cancel();
    }

    private void CreateChangeToken()
    {
        _cts = new CancellationTokenSource();
        _changeToken = new CancellationChangeToken(_cts.Token);
    }
}

And add an extension method for adding the EndpointDataSource to IEndpointRouteBuilder

public static class EndpointRouteBuilderExtensions
{
    public static void MapAuthenticationEndpoints(this IEndpointRouteBuilder endpoints)
    {
        var authenticationSchemeProvider = endpoints.ServiceProvider.GetService<IAuthenticationSchemeProvider>();
        var endpointDataSource = authenticationSchemeProvider.GetEndpointDataSource();

        endpoints.DataSources.Add(endpointDataSource);
    }
}

AuthenticationMiddleware can check AuthenticationOptions.UseEndpoints for handling the request itself.

public class AuthenticationMiddleware
{
    public async Task Invoke(HttpContext context)
    {
        context.Features.Set<IAuthenticationFeature>(new AuthenticationFeature
        {
            ...

            if (!options.UseEndpoints)
            {
                // Give any IAuthenticationRequestHandler schemes a chance to handle the request
                var handlers = context.RequestServices.GetRequiredService<IAuthenticationHandlerProvider>();
                foreach (var scheme in await Schemes.GetRequestHandlerSchemesAsync())
                {
                    var handler = await handlers.GetHandlerAsync(context, scheme.Name) as IAuthenticationRequestHandler;
                    if (handler != null && await handler.HandleRequestAsync())
                    {
                        return;
                    }
                }
            }

        ...
    }
}

Let me know what do you think? ☺️

[Kahbazi]

One breaking behavior in switching to endpoint is that HandleRequestResult.SkipHandler() makes no sence for an Endpoint.

https://github.com/aspnet/AspNetCore/blob/d8381656429addead2e5eb22ba1356abfb419d86/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs#L65-L68

Also I'm not quite familiar with Negotiate Handler, does it also need to break into an endpoint? I feel like its business should move to HandleAuthenticateAsync, but I don't know if it's possible or not. 🤔

[Tratcher]

No, Negotaite, CertAuth, and JwtBearer do not have any dedicated endpoints. NegotiateAuth's HandleRequestAsync need to stay due to mutli-stage handshakes that could happen on any url path.

[Tratcher]

One breaking behavior in switching to endpoint is that HandleRequestResult.SkipHandler() makes no sence for an Endpoint.

True, you'd have to opt-out of endpoints if you wanted to use dynamic fallback logic. Most scenarios don't require this by default.

[Kahbazi]

NegotiateAuth's HandleRequestAsync need to stay

I was thinking of deprecating and eventually removing HandleRequestAsync. Is it possible to move all Netotiate process to HandleAuthenticateAsync?
Beside aspnet docs, where can I learn more on how negotiate works?

Also I would like to know your thoughts on my comment on how to move to Endpoint. Should I add more details? Is there something that I missed?

[Tratcher]

For compat reasons HandleRequestAsync is going to have to stay. Besides NegotiateAuth, there are also WsFed scenarios that require it. Not all WsFed providers use dedicated endpoints, many of them redirect you back to the site root. Thus you need to be able to have multiple components on a path and sometimes skip one in favor of the other. That scenario's disabled by default, but still needs to be possible.

I'm also less concerned about NegotiateAuth because that's not a component that makes sense to have multiple of (unlike OAuth or OIDC). If we can get the OAuth and OIDC providers moved to endpoints by default then that should be a good start here.

[davidfowl]

cc @rynowak

[kevinchalet]

One breaking behavior in switching to endpoint is that HandleRequestResult.SkipHandler() makes no sence for an Endpoint.

I hope SkipHandler() will still be supported in 5.0 as I use it... quite massively 😅

[Tratcher]

@PinpointTownes where do you use it? The idea is that callback paths like /sigin-oidc would use endpoints by default and SkipHandler would no longer work there. You should be able to opt out of using endpoints though, and get the SkipHandler behavior back.

[Kahbazi]

You should be able to opt out of using endpoints

@Tratcher are you thinking about opting out for all schemes or per scheme?

[kevinchalet]

@PinpointTownes where do you use it?

In OpenIddict, pretty much everywhere, as it's what we use to enable pass-through. Once this mode is enabled, OpenIddict validates requests for you but eventually calls context.SkipHandler() so that you can handle the rest of the request outside OpenIddict: in a MVC controller, in a custom middleware, in a Carter module (e.g to render a consent page for the authorization endpoint or to return some JSON profile data from the userinfo endpoint).

[Tratcher]

@Tratcher are you thinking about opting out for all schemes or per scheme?

Per scheme. E.g. someone might need to opt out for WsFed but not for OIDC.

[Tratcher]

In OpenIddict, pretty much everywhere, as it's what we use to enable pass-through. Once this mode is enabled, OpenIddict validates requests for you but eventually calls context.SkipHandler() so that you can handle the rest of the request outside OpenIddict: in a MVC controller, in a custom middleware, in a Carter module (e.g to render a consent page for the authorization endpoint or to return some JSON profile data from the userinfo endpoint).

Is it used on callback paths? Or only on normally pass through components like JWT validation?

[Kahbazi]

Does AddAuthentication().AddGoogleEndpoints() would be enough to add the Endpoint to the IEndpointRouteBuilder, or do you think a MapAuthenticationEndpoints(this IEndpointRouteBuilder endpoints) is needed?

[HaoK]

I think we should make it smart enough to automatically register any authentication endpoints if any were registered without any additional calls. The extension methods should do that magic underneath the covers.

[Tratcher]

@HaoK that seems overkill, it forks the ecosystem. Moving people over to endpoints by default should be good for the vast majority of cases. It's only some corner cases that need to opt-out.

[HaoK]

I don't know, we've already had lots of behavior changes with endpoint routing to auth already. So I can't imagine we won't run into more. Forking the handlers would better guarantee there's at least a workaround in case/when we accidentally break something. Opting into the new endpoint enabled handlers would only require them changing their implementation to derive from the new Endpoint aware base class. Seems like this is something auth handlers should be aware of and opt into. We can still make endpoints the default for our auth handlers since we control the extension methods.

[Tratcher]

Adding a base class is maximally disruptive since we already have a class hierarchy (AuthenticationHandler, RemoteAuthenticationHandler, OAuthHandler), you'd have to fork the entire hierarchy. Maintaining two copies of every auth handler and base class is not sustainable.

Yes there's risk with this change, but we should be able to manage that with opt-out options and avoid mass duplication.

[HaoK]

Yeah if there wasn't an easy way to refactor things to reuse most of the logic in both hierarchies, that would be no good. My main point is the handler implementation code has been mostly unchanged since katana, we know how hard it is to extend/customize, as we've never spent the time to make that easy. Seems like this is an opportunity to see if we can finally make things nicer since endpoint support is a big change

[HaoK]

I guess lets just see how the PR goes, given that there's already stuff like HandleEndpointPost/GetAsync showing up, seems like we are going to have resolve some of these questions anyways

[blowdart]

I'd rather try and think it through design wise rather than go code first. This is a major enough change that we need a consistent approach.

[HaoK]

Yeah I agree, but the prototype PR might be good to flush out some of the design issues we need to discuss.

[rynowak]

This is something I've briefly discussed with @rynowak. No, you couldn't convert all of an auth handler to endpoints, but moving some of the callbacks like /signin-microsoft to endpoints might be possible. That said, it would involve breaking up the auth handlers into several pieces and it's not clear what would be gained by doing so

I'm catching up to this discussion a little bit late, but I'm wondering what kinds of concrete answers have cropped out to this question (emphasis mine).

---

In general endpoints are useful because....

• The app developer needs to configure cross-cutting concerns (auth/cors)
• Middleware need to collaborate
• You want to make a deferred decision

Are any of these things true for this case?

[Tratcher]

@rynowak the one scenario I've identified above is a perf optimization to avoid allocating and looping through every auth handler on every request. This would be covered more by routing than endpoints. #17615 (comment)

[khellang]

Another benefit is simply having the callback endpoints be part of the endpoint collection for other middleware to discover, like OpenAPI can read its metadata and include it in the document.

[Kahbazi]

It also could be useful for throttling or logging callback endpoints.

[Tratcher]

like OpenAPI can read its metadata and include it in the document.

Not that anything should be calling these endpoints directly, an initial challenge is required in almost all cases.

[Tratcher]

I went over this again with @rynowak and came to the conclusion that what we want for these callback paths is routing, not endpoints. We haven't identified any other concrete scenarios that need endpoints.

The theory is that the auth middleware or service would build their own internal route table mapping callback paths to auth schemes & handlers (not Endpoints). This would re-use existing routing code internally rather than participate in the larger route table. The middleware would then run that handler's HandleRequestAsync method. Schemes would opt-in/out via options, and be given some way to register and unregister their callbacks. (@PinpointTownes) Skip would still mostly work by allowing you to keep executing after the new routing component and still run the rest of the auth middleware logic (AuthenticateAsync for the default scheme, etc.) and the request pipeline. The loop through schemes to run HandleRequestAsync would still happen if no route was found, but schemes that had registered callbacks would opt out of that part.

[kevinchalet]

@Tratcher perfect, thanks! 👏

[Kahbazi]

We haven't identified any other concrete scenarios that need endpoints.

I don't know if these would count as good scenarios:

• Analysis for endpoints including callback and remote signout endpoints.

• Eventually ConcurrencyMiddleware will be able to have different queue for different Endpoint. User could add ConcurrencyMiddleware for callbacks or remote signout endpoints.

• User could add a middleware which has a IP white list or black list for these endpoints.

[Tratcher]

@Kahbazi I don't see why you'd do any of those things specifically for these auth endpoints and not for the reset of your app.

[Kahbazi]

I'm thinking all my app has these things but maybe I wan't different settings for these endpoints. For example I have ConcurrencyMiddleware and it will choose a queue based on some attribute in Endpoint Metadata. Now there's no way for me to have ConcurrencyMiddleware for auth callbacks.

Also I could write a analysis middleware based on Endpoint which generate report for all endpoints and the callbacks would be included in the report automatically.

[Kahbazi]

we want for these callback paths is routing, not endpoints

@Tratcher Is this decision final? I think by having just routing for authentication callbacks, there would be some limitations. I'm just expanding my previous example here.

Let's say I have a ConcurrencyMiddleware which has different queues for different kind of Endpoints. I can mark my Endpoints with some attribute to assign them to a specific queue. This would be my pipeline.

app.UseRouting();
app.ConcurrencyMiddleware();
app.UseAuthentication();
app.UseAuthorization();
app.UseEndpoints();

Now I can't assign a specific queue for callbacks. The only option for them could be a default fallback queue which I might only wanna use it for when there's no Endpoint found at routing.
I even may want to assign different authentication callbacks to different queue which I can't with this design.

I promise this is my last attempt to apply Endpoints for authentication callbacks 😅
I would like to know your thoughts.

[Tratcher]

Nothing's final, but the case for endpoints has not been compelling. The use of endpoints also has tradeoffs with fallback patterns currently supported by several of the auth handlers.

How would you add metadata/attributes to the callback endpoints when they're created and registered by the auth service?

[Kahbazi]

The use of endpoints also has tradeoffs with fallback patterns

As you said, there could be an opt out option.

How would you add metadata/attributes to the callback endpoints

By adding a Metadata on AuthenticationOptions like MicrosoftAccountOptions.
Or even changing CallBackPath from PathString to a class with PathString and Metadata, since some handlers have multiple callback path 🤔

[Tratcher]

How would you add metadata/attributes to the callback endpoints

By adding a Metadata on AuthenticationOptions like MicrosoftAccountOptions.
Or even changing CallBackPath from PathString to a class with PathString and Metadata, since some handlers have multiple callback path 🤔

Needs some more thought. There's a strong preference for not having API breaks required for this change, only additions.

[Kahbazi]

We could add the new class (let's call it CallBackEndpoint) and keep the CallBackPath (maybe with an ObsoleteAttribute) and if CallBackEndpoint has been set, we ignore CallBackPath and if CallBackEndpoint is null, we could set it from CallBackPath with default metadata.
Default metadata could specify the scheme, handler type, etc