Expose security context of a feign client to an hystrix command.

[daniellavoie]

PR for issue #1053

Currently leverages on Command Hook registration of Hystrix. With this current implementation, users won't be able to register a custom Hook. Needs to be addressed. Waiting for project leads feedback.

[daniellavoie]

I'm actually trying to figure out why the RestTemplateRetryTest test only crashes during maven build. I'm not encountering any error when running the full JUnit suite. Any guess ?

[spencergibb]

I get the same error on master.

[daniellavoie]

Yeap, same here. I'm also surprised to see my new test on error. Going to dig on it but I don't have the same behavior than travis when running maven install.

[dsyer]

You shouldn't need both this and spring-security-core

[daniellavoie]

You are right, I didn't realized spring-cloud-commonswas already referencing the spring security libs. Thanks for pointing.

[dsyer]

If hooks are singular (as stated in spring-attic/spring-cloud-security#87), we would need an extension mechanism, so users can add their own hooks. Would they work as a composite (and the user can add one to the composite by configuring a bean)?

I'm not sure I understand the use case for this one yet though. What is there in the SecurityContext that is useful in a Feign interceptor? The tests show a header being sent with the current user name, but it seems somewhat artificial.

[daniellavoie]

I'll give you that the use case is not quite obvious. Let me explain it to you.

I am interested in having access to the SecurityContext inside a Feign Interceptor so I can automatically inject custom headers or parameters required a backend service. In my case, the backend targeted by Feign requires a username inside the header for audit purpose for every endpoints. Rather than having to define the username as a parameter to every Feign endpoints, I prefer having a centralized interceptor for this. This backend is secured with generic credential which is unrelated to the user logged inside my Security Context.

The test I contributed tries to reproduce this use case inside a single Web Service.

• TestController represent a backend service which requires the username as part of the header.
• ProxyTestController represent my service secured endpoint who uses Feign to call the backend service. It uses the Feign TestInterceptor to automatically inject the username header from the SecurityContext.
• TestInterceptor injects the SecurityContext username. At the moment, it also inject a Basic Auth Header because the backend url is secured by the same filter than ProxyTestController. I'll change that. For my demonstration, the backend service does not need to be secured.

I'll go through your reviews comments and reply to them.

Hope I could be helpful with those explanations.

[daniellavoie]

In the mean time, I am going to think about the cleanest extension mechanism.

Difficulty resides in the fact that users might expect to directly registers their hook with Hystrix API without knowing about our pre registered one. A runtime exception with explanations could be triggered if we detect that our Spring Cloud hook was removed. Our hook could then make use of an optional bean providing extension capabilities. What do you think about it ?

[dsyer]

I think if people want to use the raw Netflix API then they will forgo the opportunity to benefit from our customizations (i.e. don't worry about it).

[daniellavoie]

Alright then ! I'll just at least provide provide an extension bean. By the way, how do you feel about this feature ? Have I made myself more clear on the use case ? If you are interested into integrating that feature, I'll prepare the doc into spring-cloud-netflix-docs.

[dsyer]

I'm not sure yet. Probably something like this makes sense. I'm a little wary of adding 2 new filters to the stack, but maybe we can find a different way, or make them optional somehow, once we have the feature clearly defined.

[daniellavoie]

I understand your concerns with the new filters. I'll think about other alternatives. I'm open to suggestion if you have any idea :) I'm guessing aop could be another way around but I'm not a big fan of overusing aspects. In the meantime. I could deactivate the filters by default and make them optional with a property.

Since this feature answer an edge case, deactivating it by default would make sense.

[daniellavoie]

By the way, I am currently investigating the issue with the RestTemplateRestryTest. The problem comes from the retry handler of the ribbon configuration who gets overwritten when running the full maven build. the tests sets up a next server retry count to 10 but gets overwritten by the default handler. I'm close to finding out why it happens. I'll try to push a PR once I identified the source. This will help to fix the travis build.

Regards

[dsyer]

Travis is a bit mysterious sometimes. Don't worry about it.

[daniellavoie]

Victory ! I've been encountering some weird behavior on properties defined with @WebIntegration. So I moved them to application.yml. Can't tell why but Travis gave me different behavior than locally.

I implemented the extendable hook mechanism. Users only need to declare a bean implementing CustomCommandHook.

[daniellavoie]

I'll rework that PR with the proper documentation within asciidoc sources.

[shuraa]

I think you should add javadoc here.

[daniellavoie]

I agree, I'll add the javadoc on my next PR which will also inclode the asciidoc update.

[daniellavoie]

I actually realized that this feature does not limit itseft to a feign hystrix command. It allows you to access the SecurityContext from any Hystrix Command. I'll rework the PR description and package naming. Many people are complaning about not being able to access SecurityContext without using SEMAPHORE isolation.

[daniellavoie]

I'm closing this PR since I had to renamed the branch. Replaced by PR #1093 .