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

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

Open
haukec opened this issue May 19, 2021 · 5 comments
Open

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

haukec opened this issue May 19, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@haukec
Copy link

haukec commented May 19, 2021

Hi there,

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

  • Annotate a service method with @Allow(roles="admin")
  • Make a testcase to call that method without any token

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
Copy link
Owner

majusko commented May 19, 2021

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 majusko added the question Further information is requested label May 19, 2021
@majusko majusko self-assigned this May 19, 2021
@majusko
Copy link
Owner

majusko commented May 19, 2021

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
Copy link
Author

haukec commented May 20, 2021

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
Copy link
Owner

majusko commented May 20, 2021

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: haukec/demo#1

Thank you for finding this use case :)

Mario

@majusko majusko added bug Something isn't working and removed question Further information is requested labels May 20, 2021
@haukec
Copy link
Author

haukec commented May 20, 2021

Hi Mario,

great, it works 👍! 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants