Add JWT client/server support for gRPC

[Jotschi]

This PR adds JWT support to the GrpcServer / GrpcClient and it improves the client error handling.

The PR contains the initial work which includes the implementation and an integration test + preliminary doc changes.

I would like to get some feedback on this before I continue since I'm not sure whether JWT support in gRPC is something you want to have. I need it for my own project and would like to contribute the implementation here instead of keeping it in my own repo.

I hope this is the right repo since I also found https://github.com/vert-x3/vertx-grpc which also has a 4.4.0 tag. Is vertx-grpc deprecated?

NOTE: I use a patched vertx-dependencies to set the versions for the managed dependencies for the maven submodules of this repo.

JWT Server support

For the server a JWTAuthInterceptor has been added which can be added infront of the actual service. It extracts the token from the headers and adds the user to the gRPC context.

GrpcServer server = GrpcServer.server(vertx);
GrpcServiceBridge serverStub = JWTAuthInterceptor.create(authProvider, service);
serverStub.bind(server);

The Vert.x user can be accessed via the gRPC context:

public void sayHello(HelloRequest request, StreamObserver<HelloReply> responseObserver) {
  User identity = JWTAuthInterceptor.userFromContext();
   …
}

TODOS:

• Turn JWTAuthInterceptor into interface, add static create method
• Get rid of AtomicReference usage.
• Access user from context via static method instead of field access.
• Reduce interceptor code by returning a ServerServiceDefinition from JWTAuthInterceptor.create
• More tests

JWT Client support

I added a JWTGrpcClient which was inspired the way the OAuth2WebClient is used. It is a wrapper for the GrpcClient.

 JWTGrpcClient jwtClient = JWTGrpcClient.create(GrpcClient.client(vertx))
      .withCredentials(new TokenCredentials(TOKEN));

Error handling

A GrpcException will now be thrown which provides access to the gRPC status and http response.
This addresses #13

TODOS

• Add test
• Check whether GrpcException can be thrown in other locations.

Other TODOs

• Check formatting, Add more documentation

Questions

• I'm not sure whether the JWTGrpcClient should be in vertx-grpc-jwt-auth. I could add it to vertx-grpc-client but I would need to add the vertx-auth-common dependency there. Should it be kept in vertx-grpc-jwt-auth?
• The same is true for the JWTAuthInterceptor. I could add it to vertx-grpc-server but I would again need to taint the module with the vertx-auth-jwt dependency. Should it be kept in vertx-grpc-jwt-auth?

[Jotschi]

I have now completed the refactor and changed the implementation to use GrpcServerResponseImpl information to extract the JWT instead of the ServerInterceptor usage.
JavaDoc has been added, branch has been rebased, additional integration testcases have been added.

I can add more tests and additional docs but would like feedback first. I'll start using the API in the meanwhile and see whether I run in any major roadblocks.

[vietj]

Sorry I haven't see this on my radar yet

[Jotschi]

Sorry I haven't see this on my radar yet
@vietj No problem. I noticed you were busy working on Vert.x 5 which is IMHO more important than this PR.

[doctorpangloss]

Compare

https://github.com/Jotschi/vertx-grpc/blob/f7effc2905708c6a2ed3ad17bc4f94d5271e84cc/vertx-grpc-jwt-auth/src/main/java/io/vertx/grpc/server/auth/impl/JWTAuthGrpcHandlerImpl.java#L22

    parseAuthorization(req, requireAuthentication, parseAuthorization -> {

to

https://github.com/Jotschi/vertx-grpc/blob/f7effc2905708c6a2ed3ad17bc4f94d5271e84cc/vertx-grpc-jwt-auth/src/main/java/io/vertx/grpc/server/auth/impl/AbstractGrpcAuthorizationHandler.java#L77

protected final void parseAuthorization(GrpcServerRequest req, boolean optional, Handler<AsyncResult<String>> handler) {

    final String authorization = req.headers().get(HttpHeaders.AUTHORIZATION);

    if (authorization == null) {
      if (optional) {
        // this is allowed
        handler.handle(Future.succeededFuture());
      } else {
        handler.handle(Future.failedFuture(UNAUTHENTICATED));
      }
      return;
    }

[doctorpangloss]

The more I try to use this, the more work it needs 👀

I really expect the following to actually do authentication on everything that is bound

JWTGrpcServer.create(GrpcServer.server(vertx), JWTAuth.create(vertx, new JwtAuthOptions(...)));

instead it does nothing, it's my responsibility to call auth handler. It might as well be a static method, so that I can turn it into something useful.

I'm really worried with the direction GrpcServer is going in Vertx, because using interceptors and generated stubs is kind of the whole point of GRPC.

[Jotschi]

@doctorpangloss
I think I did experiment with an interceptor based implementation before but decided against it after discussing it with Julien. It is so long ago that I don't remember the details. Goal of this PR was to suggest a way to enhance the gRPC auth handling in Vert.x.
I think gRPC in Vert.x still need to find the right way to go forward. For now I created my own shim based on this PR that I'm using.

And yes. Of course this implementation needs work. I did not want to invest any more time into it without getting some feedback first. I initially created the PR to outline a potential way to improve the auth handling.

[Jotschi]

@vietj

I created a shim (artifact with patches for Vert.x 4.4.5) using the code of this PR and wrapped it in a dedicated artifact.
A PoC project which uses the shim is listed below.

• Shim: https://github.com/metaloom/vertx-grpc-jwt-shim
• PoC: https://github.com/metaloom/poc-grpc-vertx-server/tree/shim

The issue with requireAuthentication vs optional that was mentioned has been fixed in the shim.
The PoC also contains tests to verify authenticated / public request handling.

I think the API can still be improved. I would like to see official JWT support for gRPC in Vert.x. I don't mind if the outlined implementation is not suitable. Any pointers in the right direction are welcome.

For now I'll use my shim to provide the functionality.

[vietj]

we do have a contributor hangout in community every friday, perhaps we can discuss this during this meeting @Jotschi ?

[doctorpangloss]

I ended up exposing RoutingContext to the stubs, which solved everything!

[Jotschi]

QA from todays meeting in discord:

Ensure use of correct/dedicated packages to support JPMS

I moved the bulk of the code in a dedicated package: io.vertx.grpc.server.auth.jwt
Q: Should the package be different at the io.vertx.xyz (e.g. io.vertx.grpc-jwt) level?

Check compatibility official jwt example - https://github.com/grpc/grpc-java/tree/master/examples/example-jwt-auth

The client / server code uses Authorization metadata/header with regular Bearer prefix to handle JWT

Check whether it would be possible to move auth code to vertx-auth to support generic JWT support for http client.

Client auth code can be moved but thats only a very small amout of code. (Basically just setting the TokenCredentials in the header). Thats a one-liner.

Check whether the server JWT auth handler be reused for gRPC? Can it be used as a handler a regular http2 server?

The current JWTAuthHandlerImpl jwt handler is located in vertx-web it is designed to be used with vertx-web. A generic implementation of the handler could be created. Two additional implemenations could be created to support JWT in vertx-web and vertx-grpc. The current implementation is using HTTP status codes. The generic implementation would need to be code agnostic. A dedicated mapping to http code and gRPC codes would need to be implemented for each component (gRPC, vertx-web).

Additional questions for next meeting:

• Should the gRPC auth code moved to a vertx-auth module?
• Created dedicated module for vertx-grpc-client-jwt?

Extra branches:

• https://github.com/vert-x3/vertx-dependencies/compare/master...Jotschi:vertx-dependencies:dev-grpc-jwt-support?expand=1
• https://github.com/eclipse-vertx/vertx-auth/compare/master...Jotschi:vertx-auth:dev-grpc-jwt-support?expand=1

[Jotschi]

This PR is now obsolete and will be replaced by #75