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

ChainAuthHandlerImpl#postAuthentication throws NPE #2611

Open
halber opened this issue May 21, 2024 · 8 comments
Open

ChainAuthHandlerImpl#postAuthentication throws NPE #2611

halber opened this issue May 21, 2024 · 8 comments
Assignees
Labels
Milestone

Comments

@halber
Copy link

halber commented May 21, 2024

Version

4.5.6

Context

This commit f856512 causes an NPE when calling the postAuthentication method.

Do you have a reproducer?

git clone --branch reproducer --single-branch https://github.com/halber/vertx-web.git

Steps to reproduce

  1. mvn -Dtest=io.vertx.ext.web.handler.ChainAuthHandlerTest2 test

This test is a copy of the io.vertx.ext.web.handler.OtpHandlerTest#testVerifyAuthenticatorGoodCode. The only thing I changed was to add the OtpAuthHandler to the ChainAuthHandler. Line 89 to 91.

Stacktrace

java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "io.vertx.ext.web.RoutingContext.get(String)" is null
	at io.vertx.ext.web.handler.impl.ChainAuthHandlerImpl.postAuthentication(ChainAuthHandlerImpl.java:146)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl.handle(AuthenticationHandlerImpl.java:81)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl.handle(AuthenticationHandlerImpl.java:32)
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1347)
	at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:194)
	at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:136)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerInternal.postAuthentication(AuthenticationHandlerInternal.java:42)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl.lambda$handle$0(AuthenticationHandlerImpl.java:100)
	at io.vertx.core.impl.future.FutureImpl$1.onSuccess(FutureImpl.java:91)
	at io.vertx.core.impl.future.FutureBase.emitSuccess(FutureBase.java:67)
	at io.vertx.core.impl.future.FutureImpl.addListener(FutureImpl.java:231)
	at io.vertx.core.impl.future.FutureImpl.onSuccess(FutureImpl.java:87)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl.handle(AuthenticationHandlerImpl.java:87)
	at io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl.handle(AuthenticationHandlerImpl.java:32)
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1347)
	at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:194)
	at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:136)
	at io.vertx.ext.web.handler.impl.SessionHandlerImpl.handle(SessionHandlerImpl.java:347)
	at io.vertx.ext.web.handler.impl.SessionHandlerImpl.handle(SessionHandlerImpl.java:41)
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1347)
	at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:194)
	at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:136)
	at io.vertx.ext.web.handler.impl.BodyHandlerImpl$BHandler.doEnd(BodyHandlerImpl.java:365)
	at io.vertx.ext.web.handler.impl.BodyHandlerImpl$BHandler.end(BodyHandlerImpl.java:342)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:237)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:219)
	at io.vertx.core.http.impl.HttpEventHandler.handleEnd(HttpEventHandler.java:76)
	at io.vertx.core.http.impl.Http1xServerRequest.onEnd(Http1xServerRequest.java:541)
	at io.vertx.core.http.impl.Http1xServerRequest$1.handleMessage(Http1xServerRequest.java:104)
	at io.vertx.core.net.impl.InboundMessageQueue.test(InboundMessageQueue.java:73)
	at io.vertx.core.streams.impl.InboundReadQueue.drain(InboundReadQueue.java:250)
	at io.vertx.core.streams.impl.InboundReadQueue.drain(InboundReadQueue.java:224)
	at io.vertx.core.net.impl.InboundMessageQueue.drainInternal(InboundMessageQueue.java:164)
	at io.vertx.core.net.impl.InboundMessageQueue.drain(InboundMessageQueue.java:144)
	at io.vertx.core.http.impl.Http1xServerRequest.handleEnd(Http1xServerRequest.java:153)
	at io.vertx.core.http.impl.Http1xServerConnection.onEnd(Http1xServerConnection.java:197)
	at io.vertx.core.http.impl.Http1xServerConnection.onContent(Http1xServerConnection.java:188)
	at io.vertx.core.http.impl.Http1xServerConnection.handleOther(Http1xServerConnection.java:173)
	at io.vertx.core.http.impl.Http1xServerConnection.handleMessage(Http1xServerConnection.java:166)
	at io.vertx.core.net.impl.VertxConnection.read(VertxConnection.java:243)
	at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:153)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
	at io.netty.handler.codec.http.websocketx.extensions.WebSocketServerExtensionHandler.channelRead(WebSocketServerExtensionHandler.java:88)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.vertx.core.http.impl.Http1xOrH2CHandler.end(Http1xOrH2CHandler.java:61)
	at io.vertx.core.http.impl.Http1xOrH2CHandler.channelRead(Http1xOrH2CHandler.java:38)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	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:829)
@halber halber added the bug label May 21, 2024
@tsegismont tsegismont self-assigned this May 24, 2024
@tsegismont tsegismont added this to the 4.5.9 milestone May 24, 2024
@andrei-tulba
Copy link

andrei-tulba commented May 30, 2024

There also another problem with this fix (ArrayOutOfBoundException here). This will happen if the SimpleAuthenticationHandlerImpl@23305 gives an ok (recursion and graphs with a single index 🙂 ).
@tsegismont Not really sure if I need to create a separate issue with its own reproducer.

Build a following list of handlers to reproduce the issue (see image with nested any handlers)

Screenshot from 2024-05-30 19-40-25

halber added a commit to SAP/neonbee that referenced this issue Jun 6, 2024
A problem with the ChainAuthHandler was introduced in vert.x version
4.5.6. To mitigate the issue, we do not use the vert.x
ChainAuthHandler when only one AuthenticationHandler is used. The
problem still exists when more than one AuthenticationHandler is used.

vert-x3/vertx-web#2611
halber added a commit to SAP/neonbee that referenced this issue Jun 6, 2024
A problem with the ChainAuthHandler was introduced in vert.x version
4.5.6. To mitigate the issue, we do not use the vert.x
ChainAuthHandler when only one AuthenticationHandler is used. The
problem still exists when more than one AuthenticationHandler is used.

vert-x3/vertx-web#2611
@vietj vietj modified the milestones: 4.5.9, 4.5.10 Jul 17, 2024
@tsegismont tsegismont removed this from the 4.5.10 milestone Aug 29, 2024
@tsegismont tsegismont removed their assignment Aug 29, 2024
@tsegismont
Copy link
Contributor

@halber can you provide a reproducer that does not involve an MFA handler?

As far as I can tell, the reproducer also fails without commit f856512.

It fails differently, with java.lang.AssertionError: expected:<302> but was:<200>, because the ChainAuthHandler doesn't seem to play well with MFA (mfa is null in chain auth handler)

I tried to create a reproducer with a single, simple auth handler (basic auth) and it worked fine.

Can you provide a different reproducer?

@tsegismont
Copy link
Contributor

@andrei-tulba if I understand correctly, the issue is different (happens when ChainAuthHandler is nested inside another instance). Then yes, please open a separate issue, ideally with a simplified reproducer.

@andrei-tulba
Copy link

FYI @tsegismont ☝️

@tsegismont
Copy link
Contributor

Thanks @andrei-tulba

@wolftree-games
Copy link

wolftree-games commented Nov 7, 2024

Stumbled over the same error in version 4.5.10
For me setup is pretty simple

authHandler is a OAuth2Handler (Google)
jwtHandler the standard JwtAuthHandler

...

      ChainAuthHandler chainAuthHandler = ChainAuthHandler.any();
      chainAuthHandler.add(jwtAuthHandler);
      chainAuthHandler.add(authHandler);

      router.get("/loggedin")
          .handler(chainAuthHandler)

...

@tsegismont tsegismont self-assigned this Nov 7, 2024
@tsegismont tsegismont added this to the 4.5.11 milestone Nov 7, 2024
@tsegismont
Copy link
Contributor

@halber thanks, I'll give this a try and hopefully can reproduce. In the meantime, can you tell a bit more about how your jwtAuthHandler is initialized?

@wolftree-games
Copy link

wolftree-games commented Nov 7, 2024

I'am not the referenced person but maybe it helps because i run into the exact same error. my code example you see above, the handlers were setup this way:
The custom JwtHandler only overrides authenticate from JWTAuthHandlerImpl (rewrite jwt token from cookie to header if header is missing) then calling super, not really related.

   private void initGoogleAuth() {
      String google_clientid = config.gaClientId();
      String google_clientSecret = config.getSecret();
      authProvider = GoogleAuth.create(vertx, google_clientid, google_clientSecret);
      authHandler = OAuth2AuthHandler.create(vertx, authProvider,
              "http://...:8080/auth/google")
          .withScopes(List.of("openid", "email", "profile"))
          //.pkceVerifierLength(64)
          .setupCallback(router.route("/auth/google"));
   }

   private void initJwtAuth() throws IOException {
      JWTAuthOptions cfg = new JWTAuthOptions()
          .addPubSecKey(new PubSecKeyOptions()
              .setAlgorithm("RS256")
              .setBuffer(config.loadPublicKey()))
          .addPubSecKey(new PubSecKeyOptions()
              .setAlgorithm("RS256")
              .setBuffer(config.loadPrivateKey()));
      jwtAuthProvider = JWTAuth.create(vertx, cfg);
      jwtAuthHandler = WolftreeJwtAuthHandler.create(jwtAuthProvider);
   }

@vietj vietj modified the milestones: 4.5.11, 4.5.12 Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants