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

NPE in Quarkus Rest Client when receiving 401 with OIDC Token Propagation #35730

Closed
BendixP opened this issue Sep 5, 2023 · 9 comments
Closed
Labels
Milestone

Comments

@BendixP
Copy link

BendixP commented Sep 5, 2023

Describe the bug

We have a setup where a User is logged into our Quarkus Application "A" with OIDC through Azure AD. We then make a request to our Application "B" using the quarkus rest client and token propagation (on-behalf-of flow). If the user is not authorized to use that endpoint on Application "B" we get the following stack trace in "A":

2023-08-30 13:24:05,520 WARN  [org.company.application-a.ser.ExceptionMappers] (vert.x-eventloop-thread-1) Received: 'Server response is: 401' when invoking: Rest Client method: 'org.company.application-a.client.AppAApiClient#getSelf'
2023-08-30 13:24:05,521 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (vert.x-eventloop-thread-1) HTTP Request to /api/users/self failed, error id: 9f71d367-094e-4055-aae9-9c2682ce78c3-2: java.lang.NullPointerException: Cannot invoke "jakarta.ws.rs.core.MultivaluedMap.keySet()" because the return value of "jakarta.ws.rs.core.Response.getHeaders()" is null
        at org.jboss.resteasy.reactive.server.handlers.ResponseHandler.fromResponse(ResponseHandler.java:198)
        at org.jboss.resteasy.reactive.server.handlers.ResponseHandler.handle(ResponseHandler.java:58)
        at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:145)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:145)
        at org.jboss.resteasy.reactive.server.vertx.VertxResteasyReactiveRequestContext$1$1.handle(VertxResteasyReactiveRequestContext.java:78)
        at org.jboss.resteasy.reactive.server.vertx.VertxResteasyReactiveRequestContext$1$1.handle(VertxResteasyReactiveRequestContext.java:75)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:264)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:246)
        at io.vertx.core.impl.EventLoopContext.lambda$runOnContext$0(EventLoopContext.java:43)
        at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
        at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
        at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:833)

Expected behavior

A more descriptive error should be thrown. So that the error can be handled or propagated back to the user.

Actual behavior

A NPE is thrown as seen in the above stack trace.

How to Reproduce?

We use following dependencies. The setup is described above. It is not entirely clear what combination of causes leads to the NPEs.

            quarkus-rest-client-reactive
            quarkus-rest-client-reactive-jackson
            quarkus-oidc-token-propagation-reactive

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.3.1

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.4 (dfbb324ad4a7c8fb0bf182e6d91b0ae20e3d2dd9)

Additional information

We use ubi8/openjdk-17:1.16 docker image

@BendixP BendixP added the kind/bug Something isn't working label Sep 5, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 5, 2023

/cc @cescoffier (rest-client), @geoand (rest-client), @pedroigor (oidc), @sberyozkin (oidc)

@geoand
Copy link
Contributor

geoand commented Sep 5, 2023

Hm... this looks weird and could be more complex than simply guarding against a null value.

Would you be able to provide a sample that behaves as you describe?

@sberyozkin
Copy link
Member

Hey, I'm checking the tests right now as well

@geoand
Copy link
Contributor

geoand commented Sep 5, 2023

I will however open a PR guarding against the NPE, as there is nothing in the Javadoc saying that the value has to be non-null

geoand added a commit to geoand/quarkus that referenced this issue Sep 5, 2023
@sberyozkin
Copy link
Member

We have a few tests in the main repository but also in the quickstart, which first this case precisely, but the negative test for the token propagation was missing, so I added it:

quarkusio/quarkus-quickstarts#1323

(There is an exception mapper there propagating ClientWebApplicationException status)

@BendixP FYI, looks like something else in your setup is affecting it, but the NPE guard Georgios is referring to should get the exception returned

geoand added a commit to geoand/quarkus that referenced this issue Sep 5, 2023
sberyozkin added a commit that referenced this issue Sep 5, 2023
Guard against null headers when converting a provided Response
@sberyozkin
Copy link
Member

@BendixP Can you please check what happens now after #35731 has been merged ?

@BendixP
Copy link
Author

BendixP commented Sep 7, 2023

@sberyozkin I'm going to check it, as soon as I have the time. The setup is not trivial unfortunately.

@BendixP
Copy link
Author

BendixP commented Sep 11, 2023

@sberyozkin @geoand I tested the same situation with version 3.4.0.CR1. I now receive the correct stack trace of a 401 Error. It looks something like this:

2023-09-11 09:19:11,333 WARN  [org.company.applycation-a.ser.ExceptionMappers] (vert.x-eventloop-thread-1) Received: 'Server response is: 401' when invoking: Rest Client method: 'org.company.application-a.application-b.client.BApiClient#getSelf': org.jboss.resteasy.reactive.ClientWebApplicationException: Received: 'Server response is: 401' when invoking: Rest Client method: 'org.company.application-a.application-b.client.BApiClient#getSelf'
        at org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.unwrapException(RestClientRequestContext.java:185)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.handleException(AbstractResteasyReactiveContext.java:326)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:175)
        at org.jboss.resteasy.reactive.client.impl.RestClientRequestContext$1.lambda$execute$0(RestClientRequestContext.java:302)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:277)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:259)
        at io.vertx.core.impl.EventLoopContext.lambda$runOnContext$0(EventLoopContext.java:43)
        at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
        at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
        at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: org.jboss.resteasy.reactive.client.api.WebClientApplicationException: Server response is: 401
        at org.jboss.resteasy.reactive.client.handlers.ClientSetResponseEntityRestHandler.handle(ClientSetResponseEntityRestHandler.java:32)
        at org.jboss.resteasy.reactive.client.handlers.ClientSetResponseEntityRestHandler.handle(ClientSetResponseEntityRestHandler.java:23)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.invokeHandler(AbstractResteasyReactiveContext.java:231)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
        ... 12 more

So from my point of view everything is now working as expected and I can properly handle the Error. Thanks for the fast response 👍

@geoand
Copy link
Contributor

geoand commented Sep 11, 2023

Thanks for checking @BendixP

@geoand geoand closed this as completed Sep 11, 2023
@geoand geoand added this to the 3.4.0.CR1 milestone Sep 11, 2023
aloubyansky pushed a commit to aloubyansky/quarkus that referenced this issue Oct 25, 2023
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants