majusko / grpc-jwt-spring-boot-starter

Spring boot starter for gRPC framework with JWT authorization
Other
39 stars 7 forks source link

@Allow not failing, if no Token is sent by client #71

Open haukec opened 3 years ago

haukec commented 3 years ago

Hi there,

I tried to follow your instructions and make a very simple test with it:

I would have expected the call to fail (and therefore, also the test). But instead, it returns, successfully.

Here is an excerpt of the service:

@GRpcService
public class DataServiceImpl extends DataServiceImplBase {
    @Override
    @Allow(roles= "admin")
    public void getFirstBrick (Empty request, StreamObserver<GetBrickResponse> responseObserver) {
        ...
    }
    ...
}

... and here is the test:

@Test
public void testGetFirstBrick() {
    Brick brick = dataClient.getFirstBrick();
    assertThat(brick.getHeaderLevel()).isNotNull();
    assertThat(brick.getPredecessor()).isNull();
}

Could you guide me here?

Kind regards Hauke

majusko commented 3 years ago

Hi @haukec , sure thing but unfortunately I just tried to debug the use case you are mentioning and cannot reproduce this. Your use case is even covered in automated tests on line 140 - testMissingAuth. The test is not totally reflecting your use case because there is also an owner field in the @Allow annotation so I added another test calling a different API that is exactly the same as your case and it's returning StatusRuntimeException with PERMISSION_DENIED code. Could you maybe make a public repo of your example and I would have a look?

Quick note:

I think the issue will be in your data client. dataClient.getFirstBrick(); would normally show compile error as Empty param is required. Try to have a look into tests and see how the client is initialized properly in gRPC:

final ExampleServiceGrpc.ExampleServiceBlockingStub stub = ExampleServiceGrpc.newBlockingStub(channel);

stub.saveExample(Empty.getDefaultInstance());

I'm gonna have a look into the example project as well and see if I can reproduce it there.

majusko commented 3 years ago

Hello @haukec , I just tried my gRPC example project and all works fine - please try to have a look at this project and make sure you have everything set up correctly as:

https://github.com/majusko/grpc-example

haukec commented 3 years ago

Hi @majusko,

thank you for your quick reply. I have set up a minimal project here. I did not even add a test case, but instead used grpcurl on the console:

$ grpcurl --plaintext -d '{"name": "test"}' localhost:9090 com.example.demo.HelloService/SayHello
{
  "message": "Hello test"
}

Could you take a quick glance on it? Probably, I just made some dump mistake...

Kind regards, Hauke

majusko commented 3 years ago

Ok, so I checked your example and found 2 issues. One is configuration error and then there is a bug in the library for this special case as well and the case is not covered in tests 😩. Will explain:

First, there is one small issue from your side: you should not use other grpc libraries like this one net.devh grpc-server-spring-boot-starter this library works with the lognet spring boot starter io.github.lognet - because of this there was an issue that you had the wrong annotation in the service class and the library did not found that service because it was looking for the org.lognet.springboot.grpc.GRpcService (this was maybe just caused by your debugging)

But the real cause is that you renamed the package name option java_package = "com.example.demo.lib"; and this case is not handled in the library because it seems that everyone, including me, is using the same package name as the project one (this is not confirmed, it's also possible that the grpc library changed in the last versions, this will be confirmed in the further investigation). This is obviously a bug in the library and I will deliver a bug fix for this very soon and also ad a test coverage for this.

However, you should be able to safely use the library, you just need to keep the package name the same as the project until 1.0.10 will be released. I'm on vacation currently but I believe it will be next week :)

I made a PR to your library that will work for you: https://github.com/haukec/demo/pull/1

Thank you for finding this use case :)

Mario

haukec commented 3 years ago

Hi Mario,

great, it works :+1:! Thanks a lot for your help!

$ grpcurl --plaintext -proto ./src/main/proto/example.proto -d '{"name": "test"}' localhost:6565 com.example.demo.HelloService/SayHello
ERROR:
  Code: PermissionDenied
  Message: Missing JWT data.

Perfect!

I was using net.devh grpc-server-spring-boot-starter for two reasons:

  1. The lognet-implementation has/had a Threading-issue, which could lead to loosing the SecurityContext in a multi-threaded environment with Interceptors. I am not sure, if the bug still exists, though.
  2. The net.devh-implementation supports the reflection api.

Thanks again, have a great day!

Hauke