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

Calling HttpServletRequest.login to perform servlet authentication in Undertow causes NPE #26249

Merged
merged 1 commit into from Jun 23, 2022

Conversation

TFyre
Copy link
Contributor

@TFyre TFyre commented Jun 20, 2022

When trying to authenticate from a vaadin-flow application (undertow servlet) it causes a NPE in undertow.

Request for Comment

QuarkusIdentityManager is an example implementation to try authentication via IdentityProviderManager using a username/password combination.

Its possible to inject an alternative bean from your application by annotating it with @Alternative and @Priority.

Is there a better approach?

How to reproduce the NPE

Example project:
https://github.com/TFyre/quarkus-vaadin-flow
Run with: mvn quarkus:dev
URL: http://127.0.0.1:8080

  • MainView allows anonymous access
  • AdminView only allows user with admin role
  • Login will attempt to login via HttpServletRequest.login

The project is forked from https://github.com/vaadin/base-starter-flow-quarkus. Ive added quarkus-security-jpa to be able to use Vaadin Access (@RolesAllowed / @AnonymousAllowed) annotations on views.

Credentials to login: admin / admin

Stacktrace

SEVERE [com.exa.sta.bas.MainView] (executor-thread-5) null: java.lang.NullPointerException: Cannot invoke "io.undertow.security.idm.IdentityManager.verify(String, io.undertow.security.idm.Credential)" because "this.identityManager" is null
	at io.undertow.security.impl.SecurityContextImpl.login(SecurityContextImpl.java:198)
	at io.undertow.servlet.spec.HttpServletRequestImpl.login(HttpServletRequestImpl.java:467)
	at javax.servlet.http.HttpServletRequestWrapper.login(HttpServletRequestWrapper.java:294)
	at com.example.starter.base.security.SecurityUtils.login(SecurityUtils.java:18)
	at com.example.starter.base.LoginView.onLogin(LoginView.java:50)
	at com.vaadin.flow.component.ComponentEventBus.fireEventForListener(ComponentEventBus.java:206)
	at com.vaadin.flow.component.ComponentEventBus.handleDomEvent(ComponentEventBus.java:448)
	at com.vaadin.flow.component.ComponentEventBus.lambda$addDomTrigger$dd1b7957$1(ComponentEventBus.java:265)
	at com.vaadin.flow.internal.nodefeature.ElementListenerMap.lambda$fireEvent$2(ElementListenerMap.java:447)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at com.vaadin.flow.internal.nodefeature.ElementListenerMap.fireEvent(ElementListenerMap.java:447)
	at com.vaadin.flow.server.communication.rpc.EventRpcHandler.handleNode(EventRpcHandler.java:62)
	at com.vaadin.flow.server.communication.rpc.AbstractRpcInvocationHandler.handle(AbstractRpcInvocationHandler.java:75)
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocationData(ServerRpcHandler.java:438)
	at com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$1(ServerRpcHandler.java:419)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:419)
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:320)
	at com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:115)
	at com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40)
	at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1564)
	at com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:299)
	at com.vaadin.quarkus.QuarkusVaadinServlet.service(QuarkusVaadinServlet.java:84)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:590)
	at io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74)
	at io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:63)
	at io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:68)
	at io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36)
	at io.undertow.servlet.handlers.RedirectDirHandler.handleRequest(RedirectDirHandler.java:67)
	at io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:133)
	at io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46)
	at io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:65)
	at io.undertow.security.handlers.AuthenticationMechanismsHandler.handleRequest(AuthenticationMechanismsHandler.java:60)
	at io.undertow.servlet.handlers.security.CachedAuthenticatedSessionHandler.handleRequest(CachedAuthenticatedSessionHandler.java:77)
	at io.undertow.security.handlers.NotificationReceiverHandler.handleRequest(NotificationReceiverHandler.java:50)
	at io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:247)
	at io.undertow.servlet.handlers.ServletInitialHandler.access$100(ServletInitialHandler.java:56)
	at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:111)
	at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:108)
	at io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48)
	at io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43)
	at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$9$1.call(UndertowDeploymentRecorder.java:590)
	at io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:227)
	at io.undertow.servlet.handlers.ServletInitialHandler.handleRequest(ServletInitialHandler.java:152)
	at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$1.handleRequest(UndertowDeploymentRecorder.java:119)
	at io.undertow.server.Connectors.executeRootHandler(Connectors.java:284)
	at io.undertow.server.DefaultExchangeHandler.handle(DefaultExchangeHandler.java:18)
	at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$5$1.run(UndertowDeploymentRecorder.java:412)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:554)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)

@sberyozkin
Copy link
Member

Hi @TFyre Thanks for this PR, it looks like it is the best what can be done, however I'd also like @stuartwdouglas to have a look, Stuart can you please review ?
@TFyre Can you please look at the existing undertow integration tests, it would be good to test this new code. I think you can configure Basic authentication directly in application.properties to simplify the test

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please add the test; also waiting for @stuartwdouglas to approve

@TFyre
Copy link
Contributor Author

TFyre commented Jun 21, 2022

Will add the tests later today 😃

@TFyre
Copy link
Contributor Author

TFyre commented Jun 21, 2022

Test added

@quarkus-bot

This comment has been minimized.

… `identityManager` is null when calling `HttpServletRequest.login`
@sberyozkin
Copy link
Member

Lets merge it now and improve if necessary going forward

@sberyozkin sberyozkin merged commit eaa6654 into quarkusio:main Jun 23, 2022
@quarkus-bot quarkus-bot bot added this to the 2.11 - main milestone Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants