CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.06k stars 174 forks source link

endpoints.MapCarter() prevents multiple calls to UseEndpoints #226

Closed jussimattila closed 4 years ago

jussimattila commented 4 years ago

I just updated to Carter 5.0, but had to roll back to 4.2. I noticed three separate issues, this I think is the most interesting of them:

I have a request pipeline something like this:

// Unauthenticated health checks endpoint
app.UseRouting();
app.UseEndpoints(x => { x.MapHealthChecks("/health"); });

// Authenticate JWT token and handle API requests
app.UseAuthentication();
app.UseEndpoints(x => { x.MapCarter(); });

// Authenticate Cookie and handle other requests
app.Use(AuthenticateAuth0User);
app.UseStaticFiles();
app.UseEndpoints(x => { endpoints.MapControllerRoute("default", "{controller=Home}/{action=Index}/{id?}"); });

Setting up multiple endpoints at different places in the request pipeline used to work, but with Carter 5.0, routes requiring authentication fail with a 401. The reason seems to be that adding Carter causes all "endpoint routed" requests to terminate already at the first location where app.UseEndpoints(...) was called. Since in my situation the first call is before app.UseAuthentication(), none of the endpoint routed API or MVC requests are authenticated. Previously, API and MVC requests would have JWT token or Cookie authenticated users. Also, I think static files are still served correctly, so any endpoints that are not not mapped by endpoint routing still are authenticated and work correctly.

The other two issues were:

jchannon commented 4 years ago

Pinging @davidfowl I kind of consider Carter to be a terminating middleware, I think MVC is?

Are you saying you expect your /health request to continue for example

jussimattila commented 4 years ago

No, I'm saying that my API and MVC calls terminate already at app.UseEndpoints(x => { x.MapHealthChecks("/health"); });, when they should terminate at app.UseEndpoints(x => { x.MapCarter(); }); or app.UseEndpoints(x => { endpoints.MapControllerRoute(...); });, respectively.

The request pipeline never authenticates the user with endpoint routed endpoints (sorry), it always terminates at the first use of app.UseEndpoints(...). If I remove Carter 5.0 from the mix, request pipeline proceeds as inteded.

Also note that Carter would still route requests correctly, but fails with 401, since it terminates the pipeline before the authentication.

jchannon commented 4 years ago

What would you expect to happen here?

x.UseEndpoints(builder =>
{
  builder.MapHealthChecks("/health");
});

x.Use(async (context, next) =>
{
  await context.Response.WriteAsync("USE");
  await next();
});
jussimattila commented 4 years ago

Calls to "/health" terminate at MapHealthChecks. Other calls flow forward. This would work with just fine, even with Carter 5.0 added.

jchannon commented 4 years ago

OK, so what do you expect here:

x.UseEndpoints(builder =>
{
    builder.MapGet("/terminate", async context => await context.Response.WriteAsync("terminate"));
});

x.Use(async (context, next) =>
{
  await context.Response.WriteAsync("USE");
  await next();
});
jussimattila commented 4 years ago

Sorry that I'm having difficulty explaining this. With Carter 5.0, the functionality is equal to having this code:

app.UseRouting();
app.UseEndpoints(x => 
{ 
  x.MapHealthChecks("/health"); 
  x.MapCarter();
  x.MapControllerRoute("default", "{controller=Home}/{action=Index}/{id?}");
});

app.UseAuthentication();
app.UseStaticFiles();

But what I really have is the code in the original issue post.

Requests to API endpoints (implemented using Carter) terminate at the first instance of app.UseEndpoints(), before app.UseAuthentication(). The same happens with requests to MVC controllers. Removing Carter fixes the problem, requests are authenticated and terminated later in the pipeline.

jchannon commented 4 years ago

So if you remove Carter your MVC routes work as expected?

jussimattila commented 4 years ago

Exactly. Going back to 4.2 and app.UseCarter() works as well. I can investigate this further tomorrow at work. It could be that I am misusing the new routing system, doing something I'm not supposed to do...

jchannon commented 4 years ago

I’d be interested in seeing it without the authentication middleware and maybe some other arbitrary middleware just because security is a PITA! :)

On Tue, 10 Dec 2019 at 19:02, Jussi Mattila notifications@github.com wrote:

Exactly. Going back to 4.2 and app.UseCarter() works as well. I can investigate this further tomorrow at work. It could be that I am misusing the new routing system, doing something I'm not supposed to do...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/226?email_source=notifications&email_token=AAAZVJWXBRHZIOBFPMILLMLQX7RTNA5CNFSM4JZAPAP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGQL4AA#issuecomment-564182528, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJTRBGQE5LAWU4ZPVTTQX7RTNANCNFSM4JZAPAPQ .

jussimattila commented 4 years ago

I did a minimal repro and was able get this (surprising) functionality even without Carter being involved. So, it is not you, it's me. 😦

But now I'm even more surprised that my setup works in our application, because based on what I see, .NET Core 3 terminates the request pipeline where app.UseEndpoints() is configured for the first time, even if the routes have been mapped later in the pipeline.

I will close this issue as there is nothing in Carter that can be done.

I'm still curious as to how I should deal with my scenario. I have a "monolithic" server app that has routing for:

I've previously implemented this with a request pipeline that does more and more only when needed. For example, JWT auth is only done if needed and cookie auth is only done if public or Carter routes didn't already terminate the request. Cookie auth replaces the authentication so that with JWT token one is not authorized to access the UI.

I'll have to read up more on this endpoint routing, it seems.

jussimattila commented 4 years ago

Described the actual problem here: https://github.com/aspnet/AspNetCore/issues/17750

jchannon commented 4 years ago

Subscribed!

On Tue, 10 Dec 2019 at 20:25, Jussi Mattila notifications@github.com wrote:

Described the actual problem here: aspnet/AspNetCore#17750 https://github.com/aspnet/AspNetCore/issues/17750

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/226?email_source=notifications&email_token=AAAZVJWU62ZXDBLVFBTTCSDQX73J7A5CNFSM4JZAPAP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGQZZBI#issuecomment-564239493, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJRCS5FWLY7FUIJHT33QX73J7ANCNFSM4JZAPAPQ .