HttpContext lost between HttpClientWrapper and DelegatingHandler

[amweiss]

Expected Behavior / New Feature

A DelegatingHandler with IHttpContextAccessor injected can access the current context correctly.

Actual Behavior / Motivation for New Feature

As of version 15.0.7, a custom DelegatingHandler we use now only has access to an empty context and thus httpContextAccessor.HttpContext.User.FindFirst(ClaimTypes.NameIdentifier)?.Value is now null instead of the name like it was in 15.0.6

Steps to Reproduce the Problem

1. Create a DelegatingHandler that takes in IHttpContextAccessor as an injected dependency.
2. Register the handler on the route.
3. Add JWT auth to the route.
4. In the SendAsync method, call httpContextAccessor.HttpContext.User.FindFirst(ClaimTypes.NameIdentifier)?.Value
5. See that the value is null

Specifications

• Version: 15.0.7
• Platform: dotnet 3.1.300
• Subsystem: MacOS

[davipsr]

Same issue here, version 16.0.1

[jlukawska]

Hello,
The reason is that after changes done in fe3e8bd a new HttpContext instance is created for every request (or even more instances for aggregated routes). AuthenticationMiddleware (and others) receives a new HttpContext instance, but HttpContextAccessor still has a reference to the original instance. That's why HttpContext obtained by HttpContextAccessor doesn't have any claims.

I've proposed a solution (see pull request #1268 ) where the proper HttpContext will be added as a property to a delegating handler class if the class implements the specified interface.

[bjartekh]

I tried the code submitted in #1268 (by implementing IDelegatingHandlerWithHttpContext in my DelegatingHandler) but I experienced that HttpContext.User.Claims were not updated after the first request (so any subsequent requests would use the same user claims).

Steps to reproduce:

• Obtain an accessToken for ClientID A (from IdentityServer4)

• Use accessToken A towards Route 1 in Ocelot

• Observe that HttpContext.User.Claims contained claims present in accessToken A

• Obtain an accessToken for ClientID B (from IdentityServer4)

• Use accessToken B towards Route 1 in Ocelot

• Observe that HttpContext.User.Claims contained claims present in accessToken A

Is this a scenario you've tested?

My workaround-hack for getting user claims to be available in the DelegatingHandler, was to fetch the claims from the httpContext in PreAuthorizationMiddleware and then set them in a custom request scoped RequestData object.

And when I needed that data in the DelegatingHandler, I would just obtain it like this:
var rqData = (RequestData)_httpContextAccessor.HttpContext?.RequestServices.GetService(typeof(RequestData));

[jlukawska]

@bjartekh thanks, you're right. There's a problem with caching. I'll look into it if I find time.

Unfortunately Ocelot seems to be abandoned after breaking changes with HttpContext :/

[jlukawska]

@bjartekh if you are interested, I've just fixed the problem with cache.

[AgentShark]

The Ocelot project seems dead, but if anyone experiences this issue, here is a workaround.

Set the IHttpContextAccessor.Context yourself.

1. Register the IHttpContextAccessor dependency.

// Startup.cs
services.AddHttpContextAccessor();

2. In your Ocelot configuration add this code to any Ocelot pipeline middleware that runs after AuthenticationMiddleware. Example used is PreQueryStringBuilderMiddleware, but you can also use PreAuthorizationMiddleware or AuthorizationMiddleware.

// Startup.cs
app.UseOcelot(oc =>
{
    oc.PreQueryStringBuilderMiddleware = async (context, next) => 
    {
        var contextAccessor = context.RequestServices.GetRequiredService<IHttpContextAccessor>();
        contextAccessor.HttpContext = context;

        await next.Invoke();
    };
}).Wait();

3. Then inject IHttpContextAccessor into any DelegatingHandler as normal and the context will have a user identity based on the downstream route called.

Why it works
Ocelot creates a new HttpContext and calls its middleware pipeline for each downstream route needed to fulfill the request. In the case of aggregators, Ocelot creates multiple contexts, one for each downstream route. Each HttpContext goes through Ocelot's AuthenticationMiddleware and we save the context after authentication happens to use later in the downstream call pipeline in a delegating handler. The concrete HttpContextAccessor saves the context in an AsyncLocal<T> variable, thus the context is not shared between calls because each downstream call happens in a new async scope.

Learn more about AsyncLocal.

[raman-m]

Hi @AgentShark !
Thanks for the nice coding recipe!

Does it solve the problem with user claims (aka User property)?

What about to make it as an additional method for the IOcelotBuilder interface of DependencyInjection part of Ocelot?
...to use it like this:

services
    .AddOcelot()
    .AddHttpContextAccessor();

or
Make it as an additional extension method of the IApplicationBuilder interface?
...to use it like this:

app.UseOcelot()
    .UseHttpContextAccessor();

[AgentShark]

@raman-m Yes it solves the problem with user claims. AddHttpContextAccessor() can be registered anywhere in services it is an extension of IServiceCollection. I don't think there is any such thing as UseHttpContextAccessor(). See Docs

[raman-m]

@amweiss Hi Adam!

I would like personally to thank you for reporting this issue being found in v15.x! Yeah, it was a large upgrade to .NET 5 with a lot of breaking changes made by Tom.

Are you still interested to collaborate on this issue?

Specifications

• Version: 15.0.7
• Platform: dotnet 3.1.300
• Subsystem: MacOS

Are you able to upgrade your solution to .NET 7 with usage of Ocelot v19.0.2 (latest ver.)?
Could you report that the bug is still in v19.0.2 please?

@davipsr and @bjartekh
Could I pleasantly ask you to check the latest v19.0.2 please?

@AgentShark
What version of Ocelot do you use for your solution proposed in comment on June 26, 2023?

[amweiss]

I no longer use this project so I won't be going back to verify.

[raman-m]

@amweiss
This is last notification for you...

I no longer use this project so I won't be going back to verify.

Sorry for disturbing you! You can unsubscribe from notifications for this issue.
And I won't address you in discussions anymore.

[AgentShark]

@raman-m I use v18.0.0 where the issue is still present.

[raman-m]

@AgentShark wrote on June 26, 2023:

Yes it solves the problem with user claims.

How can we verify your solution? Do you have some time for contribution?
Could you upload your app sample to GitHub please, as a separate repo?
Also, you can fork the Ocelot repo, create feature branch, and upload your solution as sample in samples folder.
And, please, make a reference to Ocelot project to be sure we test the latest version (.NET 7, v19.0.2).

---

I don't think there is any such thing as UseHttpContextAccessor(). See Docs

Okay... It seems you misunderstood me.
I meant that we can wrap this your code:

app.UseOcelot(oc =>
{
    oc.PreQueryStringBuilderMiddleware = async (context, next) => 
    {
        var contextAccessor = context.RequestServices.GetRequiredService<IHttpContextAccessor>();
        contextAccessor.HttpContext = context;
        await next.Invoke();
    };
}).Wait();

...as UseAAA() extension for the IApplicationBuilder interface in the OcelotMiddlewareExtensions class.
And/Or
We can develop AddAAA() method for the OcelotBuilder class, which will wrap services.AddHttpContextAccessor() and Ocelot Middleware Configuration as in the Delegating Handlers feature.
I believe we have to extend the OcelotPipelineConfiguration class.

Finally we need to update Ocelot docs:

• Middleware Injection and Overrides
• Delegating Handlers

---

See Docs (Access HttpContext from custom components)

Microsoft Learn: Access HttpContext in ASP.NET Core
Exactly! I would say we need to follow the best practices by Microsoft.
I believe your solution is easier one and solid, but it requires some project update with your proposed functionality.

[raman-m]

TODO

• Create a new issue labeled as a feature request for the design of middleware to be integrated into the Ocelot pipeline as the final step before sending the request

[raman-m]

+ Accepted 

...due to open PR #1268

[raman-m]

@ggnaegi, FYI, the linked pull request #1268 was closed a few days ago due to a significant number of merge conflicts.
I require your assistance to determine the appropriate action for this issue. Since we've implemented the new MessageInvokerPool routing design in the system kernel, it appears this issue may no longer be pertinent. Ultimately, we must replicate the bug testing on the most recent version.

[ggnaegi]

@raman-m Do we have this issue in a multiplexing context?