Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to propagate HTTP headers and cookies #73

Closed
jwalcorn opened this issue Feb 5, 2018 · 27 comments
Closed

Need to propagate HTTP headers and cookies #73

jwalcorn opened this issue Feb 5, 2018 · 27 comments
Milestone

Comments

@jwalcorn
Copy link

jwalcorn commented Feb 5, 2018

If microservice A wants to call microservice B via the rest client "remote interface" for B, there should be an option to flow along the HTTP headers and cookies, from the inbound request to A, to the outbound request to B. Automatically forwarding along security credentials, like an Authentication header, or an LTPA SSO cookie, is one key scenario, though there are likely others too. Without this, a secured microservice B will reject the calls from A, if the Auth header, or the SSO cookie, don't flow along on the requests from microservice to microservice.

I was thinking a new CDI annotation, perhaps named @PropagateHeaders, could be defined at the point of injection. It probably needs to take the HttpServletRequest as a param (so it has something to get the inbound headers/cookies from) or maybe use of an @PostContruct annotation.

I don't have all the answers - I just know we need a way to tell the rest client that it needs to not lose the security (or other) headers/cookies that flowed in to microservice A, when it calls microservice B. This is possibly related to whatever resolution occurs for issue #16.

@jwalcorn
Copy link
Author

jwalcorn commented Feb 5, 2018

FYI, today my code manually copies these headers/cookies. I have the following method:

//forward headers (including cookies) from inbound request
private static void copyFromRequest(HttpURLConnection conn, HttpServletRequest request) {
	Enumeration<String> headers = request.getHeaderNames();
	if (headers != null) {
		while (headers.hasMoreElements()) {
			String headerName = headers.nextElement(); //"Authorization" and "Cookie" are especially important headers
			String headerValue = request.getHeader(headerName);
			conn.setRequestProperty(headerName, headerValue); //odd it's called request property here, rather than header...
		}
	}
}

I'd like to switch to using the strongly-typed "remote interface" for my JAX-RS service, but I need it to do something like the above, so that I don't lose security context on microservice to microservice calls.

@jwalcorn
Copy link
Author

jwalcorn commented Feb 5, 2018

Note I had to add an @context param to my JAX-RS method to get access to the HttpServletRequest, to use when my Portfolio microservice makes a call to the downstream StockQuote microservice:

@GET
@Path("/{owner}")
@Produces("application/json")
public JsonObject getPortfolio(@PathParam("owner") String owner, @Context HttpServletRequest request) throws IOException, SQLException {

@jwalcorn
Copy link
Author

jwalcorn commented Feb 5, 2018

Attaching my Portfolio.java, that shows the ugly manual way I'm making microservice to microservice calls today. I want to replace that with this much cleaner strongly-typed REST client approach instead, once it can make calls without losing the security context. Note I had to zip it, since Git won't let me attach a file with a .java extension.
Portfolio.zip

@derekm
Copy link
Contributor

derekm commented Feb 5, 2018

Can you set these forwardings up using @HeaderParam and @CookieParam in parent/wrapper resources that return your resource as a subresource?

I'm able to do this kind of thing with jersey-proxy-client.

@derekm
Copy link
Contributor

derekm commented Feb 5, 2018

So let's say I have a RootResource class that models everything available under /. I might do:

Client client = ClientBuilder.newClient();

WebTarget root = client.target("{host}").resolveTemplate("host", this.host, false);

rawEndpoint = WebResourceFactory.newResource(RootResource.class, root);

User user = rawEndpoint.users().sessions().post(new Credentials(username, password));

authEndpoint = WebResourceFactory.newResource(AuthResource.class, root);

rootEndpoint = authEndpoint.withAuthToken(user.sessionToken);
// all requests off of rootEndpoint will have X-Auth-Token header

where AuthResource looks like:

public interface AuthResource {

	@Path("/")
	RootResource withAuthToken(@HeaderParam("X-Auth-Token") final String sessionToken);

}

and where RootResource is anything you want it to be.

I'm a big believer that a single JAX-RS interface should only model the single path parts under it, returning subresources until you get to terminal HTTP verb actions (making proxy clients model the URL structure, where / in URLs becomes (). in fluent API calls [you see this technique above where I send a username/password JSON object via POST /users/sessions to retrieve an authenticated User document]).

@jwalcorn
Copy link
Author

Forwarding the Open Tracing headers is another use case here.

@Emily-Jiang - this is the Git issue I mentioned on our call earlier.

@derekm
Copy link
Contributor

derekm commented Feb 28, 2018

@jwalcorn -- Can you confirm whether Rest Client 1.0 supports the approach I advocate above? I think it should! @johnament wanted to make sure subresources were well-supported by 1.0. So long as parent resource context is added to the subresource when it is returned, you should be able to do what you want in a very declarative manner.

Rest Client 1.0 spec does show @HeaderParam examples, but they're very contrived and encouraging of bad practice, IMHO.

@derekm
Copy link
Contributor

derekm commented Feb 28, 2018

The down side of my approach is it can't blindly forward all headers. The headers have to be explicitly defined in a parent resource, which is perfect for all the cases I've encountered. I, frankly, don't want to forward every junk header that might be included (I'm looking at you, X-Requested-With!!!).

In my cases, all the headers I want to setup or forward are well-known, so my approach is more than adequate.

@derekm
Copy link
Contributor

derekm commented Feb 28, 2018

In fact, I would say blindly forwarding all headers could be dangerous and possibly lead to "header injection" scenarios.

@jwalcorn
Copy link
Author

@derekm - if I understand your suggestion, it would make the header values "first class" parameters on the call to the REST API/operation, but I'd still have to do something like I originally described to pluck such values off of the incoming request, so that I could forward them as such parameters to the outgoing call. Getting to where that could happen automatically is what I was hoping for. But thanks for the suggestions - I'll give that a try!

@derekm
Copy link
Contributor

derekm commented Feb 28, 2018

Yes, you would have to pick the specific headers you want to forward off of the incoming @Context HttpServletRequest request or @Context HttpHeaders headers.

I would make these class-level members instead of method parameters as you show above, that way you don't pollute the public API you can publish to others in an interfaces jar.

(I put my most meaningful JAX-RS annotations on interfaces only so that I can publish interfaces jars for others to use with their preferred proxy clients. The way I organize my endpoints, all @Context annotations are relegated to the implementations of the interfaces.)

@derekm
Copy link
Contributor

derekm commented Feb 28, 2018

To your point, @jwalcorn, @HeaderParam could be used to declaratively pick specific headers out of the context instead of programmatically picking them out. You still would have to programmatically forward the picked-out header to another resource, but it should be a simple pass-thru at that point:

public interface ClientResource {
  @Path("/")
  RootResource withHeaders(@HeaderParam("X-Random-Header-Forwarded") randomHeader);
}

@Path("random")
@RequestScoped
public class RandomEndpoint {

  @HeaderParam("X-Random-Header")
  String randomHeader;

  // setup proxy client as appropriate with ClientResource

  @GET
  public ResponsePojoHere get(/* @HeaderParam could go here, too */) {
    return proxyClient.withHeaders(randomHeader).whateverElse().post(somePostBodyPojoHere);
  }

}

This would forward incoming X-Random-Header as X-Random-Header-Forwarded on the outgoing request. The two headers could, of course, have the same name, but I named them differently for illustrative purposes.

@andymc12
Copy link
Contributor

andymc12 commented Apr 9, 2018

Hi @jwalcorn - apologies, but I'm struggling with the requirement of this issue. Is the requirement to be able to forward all headers/cookies? Or forward only select headers/cookies?

I agree with @derekm that blindly forwarding all headers/cookies is dangerous. Would it be sufficient to have an annotation that forwards a set of headers/cookies that is either specified in the annotation or via MP Config?

@jwalcorn
Copy link
Author

jwalcorn commented Apr 9, 2018

Hi Andy. I think if there were an easy way to get the other MicroProfile-related headers to flow, that would be sufficient. Mostly that means the "Authorization" header (where the JWT flows) and the OpenTracing headers. I don't really feel I should have to code up my "remote interface" differently because I want these MicroProfile headers for JWT and/or OpenTracing to flow. Maybe some annotation/CDI thing on the remote interface could help? Happy to have a discussion at your convenience.

@andymc12
Copy link
Contributor

So I think this could be accomplished using @BeanParam where the specific bean has fields annotated with @CookieParam and @HeaderParam, and then the bean would be passed into the client interface method. Even so, I can imagine that it is still not as simple or clean as the @PropagateHeader annotation you proposed.

So maybe what we need is something like:

  @RegisterRestClient
  @Propagate(cookies={"MyCookie1"
                      "MyCookie2"},
             headers={"Authorization",
                      "MyHeader1"})
  public interface MyServiceClient { //...

Then the MP Rest Client implementation would be responsible for obtaining the cookies and headers from the current incoming request (using JAX-RS) and putting them on the outbound (MP Rest Client) request. In order for this to work the MP Rest Client implementation would need to register a hook with the JAX-RS runtime that it is running in (i.e. a ContainerRequestFilter) - currently, JAX-RS doesn't have a spec-defined means to register providers globally - see chkal's comment on JakartaEE JAX-RS issue 596 for more details. Most JAX-RS implementations have their own proprietary mechanism to support this though.

The other thing we talked about on the call today was the notion of automatically setting some headers to be forwarded via MP Config. Then anything declared in the @Propagate annotation would add additional headers/cookies to be propagated.

@andymc12 andymc12 added this to the Future milestone Apr 18, 2018
@johnament
Copy link
Contributor

Adding a Propagate would mean that you need client stubs that are aware of how they're being used. E.g. if my microservice is messaging based, then the receipt of a message has no HTTP request in context and as a result wouldn't have headers to send over. Should it fail if Propagate is setup? If so, then we need a second interface.

@jwalcorn
Copy link
Author

jwalcorn commented May 8, 2018

Any news on this? The only thing I really need is the Authorization header, containing the JWT, to get propagated, so that the thing I'm calling doesn't reject the call for lack of the JWT with the right claims. I think most people using this will need the auth to flow. Everything else can wait till later, but this otherwise really cool feature is severely hampered by not having built-in JWT integration (which is odd, given JWT is one of the MicroProfile specs). I'm fine with making it a param on the Java interface (and maybe such propagation code would only get called if its is defined as needed by the method on the interface). Let me know if I can help. Thanks!

@andymc12
Copy link
Contributor

andymc12 commented May 9, 2018

Hi @jwalcorn - nothing from a spec perspective, but I coded up a sample that should make propagation possible:

https://github.com/andymc12/HttpHeaderPropagation/blob/master/src/io/openliberty/propagate/HeaderPropagationFilter.java

This will register a ContainerRequestFilter for incoming JAX-RS request and will store the requested headers in a ThreadLocal object. Then when the outbound MP Rest Client call is invoked, it will run through this same class as a ClientRequestFilter and will add those headers to the outbound request. Can you give this approach a try? If this works, we may want to document it on a MP Rest Client blog post/doc page/etc. rather than modify the spec. Thanks

@justineechen
Copy link

Hi @jwalcorn, I was wondering if you successfully propagated the auth header because I am having a similar issue with that.

@Emily-Jiang
Copy link
Member

For moving forward, I agree that it would be nice to automatically propagate authorization headers and open tracing headers. I think this will massively improve usability. We should mandate this in the spec.

@pilhuhn
Copy link

pilhuhn commented Aug 21, 2018

Another use case is the propagation of tracing headers. See also
microprofile/microprofile-opentracing#82 and
#35 (comment)
@pavolloffay

@pavolloffay
Copy link
Member

Tracing integration would not probably use this feature. Tracing uses incoming headers to identify parent span but then it propagates different headers to outbound request (headers of span created in the process). Maybe it could change the values. But it can also add new headers - baggage propagation. Tracing is built on top of context propagation so tracing could be used to implement @Propagate

However, this propagation is useful when tracing is done only via sidecar - e.g. spans not created from the process.

@Emily-Jiang
Copy link
Member

Just heads up... we are trying to find a solution for eclipse/microprofile-config#382 yesterday and discussed a couple of potential solutions. We will continue the discussion in the coming week...

@andymc12 andymc12 removed this from the Future milestone Sep 21, 2018
@andymc12 andymc12 added this to the 1.2 milestone Sep 21, 2018
@jwcarman
Copy link
Contributor

Would this propagation work properly on another thread? If I do some async operation and call a remote service, would it still propagate?

@andymc12
Copy link
Contributor

@jwcarman my workaround proposal in this thread would not work with async methods as-is. It should be possible to make it work using AsyncInvocationInterceptors to transfer the thread locals to the async thread though.

The final solution for this issue should work with async methods though - certainly this can be controlled in the MP Rest Client implementation. I can foresee issues with async methods on the server-side, since JAX-RS has no concept of AsyncInvocationInterceptors.

@Emily-Jiang
Copy link
Member

@andymc12 do you still need help from MP Config?

@andymc12
Copy link
Contributor

Resolved by PRs #143 and #149.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants