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

How to pass a header to a CallCredentials? #7415

Closed
asarkar opened this issue Sep 11, 2020 · 7 comments
Closed

How to pass a header to a CallCredentials? #7415

asarkar opened this issue Sep 11, 2020 · 7 comments
Labels

Comments

@asarkar
Copy link

asarkar commented Sep 11, 2020

I've a CallCredentials that uses a header value to do a look up in the DB, and then apply the result as an extra header.
Since CallCredentials doesn't have access to metadata, I wrote a ClientInterceptor; in ForwardingClientCall.start, I get the header value and copy it to the Context.

val userId = headers.get(USER_ID_HEADER_KEY)
val ctx = Context.current().withValue(USER_ID_CONTEXT_KEY, userId)
ctx.run { super.start(responseListener, headers) }

In the CallCredentials, I get the value set before.

USER_ID_CONTEXT_KEY.get(Context.current())

However, this is coming out as null.

My questions:

  1. Why does the above not work? I can confirm that both the ClientInterceptor and the CallCredentials are running on the same thread. However, I've noticed that the Context object id changes.
  2. Even though MetadataApplier doesn't have public access to the headers, MetadataApplierImpl does as field origHeaders. The latter isn't a public class, so there's no way to get the headers without reflection trickery.
  3. There's a Contexts.interceptCall method meant for intercepting a ServerCall with a Context parameter; why is there no equivalent method for intercepting a ClientCall?
@sanjaypujare
Copy link
Contributor

However, this is coming out as null.

My questions:

  1. Why does the above not work? I can confirm that both the ClientInterceptor and the CallCredentials are running on the same thread. However, I've noticed that the Context object id changes.

That's probably by design: the interceptors are run with a Context (that's shared with the application) but the CallCredentials are executed with a different Context as you can see in ClientCalls.blockingUnaryCall.

  1. Even though MetadataApplier doesn't have public access to the headers, MetadataApplierImpl does as field origHeaders. The latter isn't a public class, so there's no way to get the headers without reflection trickery.

That's correct. What is your question?

  1. There's a Contexts.interceptCall method meant for intercepting a ServerCall with a Context parameter; why is there no equivalent method for intercepting a ClientCall?

The comment for io.grpc.Contexts.interceptCall explains it well? On the server side the interceptor may need to augment the Context in which the application does work when receiving events from the client. On the client side the requirement doesn't exist (in fact it is quite different and that may explain your observed behavior on the client side).

@asarkar
Copy link
Author

asarkar commented Sep 11, 2020

@sanjaypujare Thanks for your response, but it doesn't answer the basic question: if CallCredentials needs access to requests headers, how does it do that? If that's by design, it seems like a fundamental design flaw; the whole point of CallCredentials is to add auth metadata to the call, and it may need access to the original headers in order to do so. A common example would be to add a HMAC signature.

One way to get around this problem could be replacing the CallCredentials with a regular client interceptor, but that seems to defeat the purpose of CallCredentials.

@sanjaypujare
Copy link
Contributor

One way to get around this problem could be replacing the CallCredentials with a regular client interceptor, but that seems to defeat the purpose of CallCredentials.

Yes I was thinking the same. If you need access to the original headers, then a regular client interceptor is the way to go.

if CallCredentials needs access to requests headers, how does it do that? If that's by design, it seems like a fundamental design flaw;

The CallCredentials code could be 3rd party code that should not have access to the headers and payload coming from the client. So that might be the reason for this design decision. CC @ejona86 for comments.

@ejona86
Copy link
Member

ejona86 commented Sep 11, 2020

I've a CallCredentials that uses a header value to do a look up in the DB, and then apply the result as an extra header.

Why? That is really strange. Why wouldn't you just pass what identity you wanted to the CallCredentials when creating them?

If that's by design, it seems like a fundamental design flaw; the whole point of CallCredentials is to add auth metadata to the call, and it may need access to the original headers in order to do so. A common example would be to add a HMAC signature.

Not at all. It was a conscious decision not to support HMAC (much at all and) in CallCredentials. HMAC via headers is incompatible with streaming. If you want HMAC, you have to put the HMAC in the message itself.

HMAC is mostly dead for general-purpose authentication; it was mainly for the plaintext HTTP days. CallCredentials is designed for bearer tokens over a secure connection. HMAC is still useful, but for application-specific uses.

@asarkar
Copy link
Author

asarkar commented Sep 15, 2020

@ejona86 I maintain service-A and client-B in the following scenario:

client-A -> service-A -> client-B -> service-B

service-B needs some auth stuff which are derived from headers sent by client-A. Since neither service-A nor client-B have direct access to the request headers from client-A, I've a ServerInterceptor for service-A that extracts the headers and puts in the gRPC context, and the service-A then call client-B with those headers in the request, whereas client-B finally creates a new stub every time using MetadataUtils.attachHeaders(stub, extraHeaders) before calling service-B.

If there's a better way to do this, I'd love to know.

@ejona86
Copy link
Member

ejona86 commented Sep 15, 2020

I've a ServerInterceptor for service-A that extracts the headers and puts in the gRPC context

Good. That's what I'd expect.

whereas client-B finally creates a new stub every time using MetadataUtils.attachHeaders(stub, extraHeaders) before calling service-B.

That's fine if you are doing a one-off. However, this pattern you're doing is fairly common, and would more commonly be accomplished with CallCredentials:

ClientIdentity identity = getIdentityFromContext();
stub.withCallCredentials(new ClientIdentityCredentials(identity))
    .theRpcMethod(...);

Or something closer to maybe what you want:

// The credentials themselves will call getIdentityFromContext()
stub = stub.withCallCredentials(new AmbientClientIdentityCredentials());
// reuse stub across RPCs

I do think Context is provided to CallCredentials. So if we go and look at your original code:

val userId = headers.get(USER_ID_HEADER_KEY)
val ctx = Context.current().withValue(USER_ID_CONTEXT_KEY, userId)
ctx.run { super.start(responseListener, headers) }

You are changing the context for start(). The context used is the one from newCall(), not start, which explains why you aren't seeing the value appear. You'd need to delay the call to next.newCall() until start() is called to give you time to dig through the headers. (Aside: I do think it was an API mistake to include headers in start() instead of newCall(), but it was less obvious at the time and we can't easily change it now.)

We discourage ClientInterceptors from modifying Context, as that mutation will be visible in StreamObserver callbacks which may then perform additional RPCs and then you get into a confusing situation. If the application itself modifies the context before calling the stub, then it is obvious the context will have those modifications for the callbacks and newCall() or start() have the same context exposed to them.

@asarkar
Copy link
Author

asarkar commented Sep 17, 2020

You'd need to delay the call to next.newCall() until start() is called

I can confirm that doing so makes the context available to the CallCredentials. To do so, I subclassed ForwardingClientCall, not the usual SimpleForwardingClientCall; the difference is that the latter initializes the delegate in the constructor, while in the former, we can wait until method delegate() is invoked.

We discourage ClientInterceptors from modifying Context

However, I ended up not using the CallCredentials due to the above suggestion, and just stayed with a ClientInterceptor.

In the end, I still don't see any good reason in a design that goes out of its way to hide the headers from the CallCredentials; nothing would change for worse if CallCredentials had read-only access to the original headers. However, having understood the problem, and thanks to @ejona86's suggestion of delaying the newCall, there exists a workaround, albeit non-intuitive and discouraged as bad practice.

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

No branches or pull requests

3 participants