#1252 Fix HttpContext in DelegatingHandler

[jlukawska]

Fixes #1252

The problem

After changes from fe3e8bd23a666daa31ed2e8b00dce2c4630c23a7 there is a new instance of HttpContext created for each request (or even more instances for aggregated routes). If HttpContextAccessor is used to obtain a HttpContext instance in a delegating handler class, it will return the original HttpContext without e.g. authentication data that is added to the new instance.

Proposed Changes

• To obtain a HttpContext, a delegating handler class should implement IDelegatingHandlerWithHttpContext interface
• DelegatingHandlerHandlerFactory will add the valid HttpContext as a property to the delegating handler class

[raman-m]

Hi J! Thanks for a great PR!

I've merged latest top-commits from develop, resolved merge conflicts, fixed a few compile errors. But all acceptance ClaimsInDelegatingHandlerTests are failing because of this error 😞

Message: 
System.Net.Http.HttpRequestException : Response status code does not indicate success: 400 (Bad Request).

  Stack Trace: 
HttpResponseMessage.EnsureSuccessStatusCode()
Steps.GivenIHaveAToken(String url, String username) line 918
Steps.GivenIHaveAToken(String url) line 897
<>c__DisplayClass1_0`1.<GetStepAction>b__0(Object o)
StepExecutor.Execute(Step step, Object testObject)
<>c__DisplayClass3_0.<ExecuteStep>b__0()
AsyncTestRunner.Run(Func`1 performStep)
ScenarioExecutor.ExecuteStep(Step step)
ExceptionProcessor.Process(Story story)
Engine.Run()
BDDfyExtensions.BDDfy(Object testObject, String scenarioTitle, String caller)
ClaimsInDelegatingHandlerTests.should_expose_claims_in_route_specific_delegating_handler() line 144
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

I believe a small fix in test setup section should fix the problem of 400 Bad Request.

Could you have a look to it and fix the issue please?

[raman-m]

@jlukawska The feature branch has been rebased onto ThreeMammals:develop successfully! We are going to code review stage...

[raman-m]

@davipsr @bjartekh @AgentShark Could you review the code please? Your opinion is prioritized for sure because of past discussions of the #1252 issue.

---

@amweiss Because you're the #1252 reporter, I believe this PR will be interesting to look into.

[raman-m]

@RaynaldM Hey Ray! Thanks for code review!

When you've approved the PR, does it mean that all issues from your code review are minor? I suggest not giving an approval if issues are major, and they should be fixed. OK? 😉

[raman-m]

Build 2170 has been failed after last changes in Polly provider.

@RaynaldM @jlukawska Could you look into build logs and understand the problem please?

It seems we have to fix these tests:

• Ocelot.AcceptanceTests.ClaimsInDelegatingHandlerTests.Should_update_claims_in_global_delegating_handler
• Ocelot.AcceptanceTests.ClaimsInDelegatingHandlerTests.Should_expose_claims_in_global_delegating_handler
• Ocelot.AcceptanceTests.ClaimsInDelegatingHandlerTests.Should_expose_claims_in_route_specific_delegating_handler

All these tests belong to the ClaimsInDelegatingHandlerTests class. I guess this is setup test problem, or test server cannot start.

[jlukawska]

This PR was created 4 years ago and almost all modified files no longer exist. As I saw in https://github.com/ThreeMammals/Ocelot/issues/1252 there is another idea to solve this issue. I'm closing this PR for now.

[raman-m]

I understand your decision to close this PR. However, you are welcome to open a new PR if you wish to contribute. It is crucial to use the new code base from the development branch. The internal Ocelot system kernel underwent a redesign last year, and we need to devise a new solution or re-investigate the reported issue.