vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 535 forks source link

ChainAuthHandlerImpl#postAuthentication throws NPE #2611

Open halber opened 6 months ago

halber commented 6 months ago

Version

4.5.6

Context

This commit https://github.com/vert-x3/vertx-web/commit/f8565123f6130725af93dc94fae55d85d5033dce 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)
andrei-tulba commented 5 months ago

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 :slightly_smiling_face: ). @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

tsegismont commented 2 months ago

@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 https://github.com/vert-x3/vertx-web/commit/f8565123f6130725af93dc94fae55d85d5033dce.

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 commented 2 months ago

@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 commented 2 months ago

FYI @tsegismont :point_up:

tsegismont commented 2 months ago

Thanks @andrei-tulba

wolftree-games commented 2 weeks ago

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 commented 2 weeks ago

@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 commented 2 weeks ago

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);
   }