mvysny / karibu-testing

Vaadin Server-Side Browserless Containerless Unit Testing
Apache License 2.0
105 stars 14 forks source link

Vaadin 24 IllegalStateException when using AuthenticationContext.logout #148

Open simasch opened 1 year ago

simasch commented 1 year ago

I migrated an app to Vaadin 24 and some of my tests fail. This one is happening when using Vaadins AuthenticatoinContext and calling logout

java.lang.IllegalStateException: invalidated: MockHttpSession(sessionId='1', creationTime=1681563208936, maxInactiveInterval=30, attributes={com.vaadin.flow.server.VaadinSession.Vaadin Servlet=com.github.mvysny.kaributesting.v10.spring.MockSpringVaadinSession@388b9799, Vaadin Servlet.lock=java.util.concurrent.locks.ReentrantLock@4225b4e2[Locked by thread main]}, isValid=false)

    at com.github.mvysny.kaributesting.mockhttp.MockHttpSession.checkValid(MockHttpSession.kt:115)
    at com.github.mvysny.kaributesting.mockhttp.MockHttpSession.getAttribute(MockHttpSession.kt:61)
    at com.vaadin.flow.server.WrappedHttpSession.getAttribute(WrappedHttpSession.java:54)
    at com.vaadin.flow.spring.security.VaadinAwareSecurityContextHolderStrategy.getFromVaadinSession(VaadinAwareSecurityContextHolderStrategy.java:71)
    at com.vaadin.flow.spring.security.VaadinAwareSecurityContextHolderStrategy.getContext(VaadinAwareSecurityContextHolderStrategy.java:56)
    at org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler.logout(SecurityContextLogoutHandler.java:74)
    at org.springframework.security.web.authentication.logout.CompositeLogoutHandler.logout(CompositeLogoutHandler.java:54)
    at com.vaadin.flow.spring.security.AuthenticationContext.logout(AuthenticationContext.java:128)
mvysny commented 1 year ago

Thank you, that's an interesting finding indeed!

Technically, MockHttpSession seems to be doing the right thing. The HttpSession.getAttribute() javadoc says that the IllegalStateException is thrown "if this method is called on an invalidated session". MockHttpSession.invalidate() has been called, since the isValid flag is set to false, as shown by the MockHttpSession.toString() in the exception above: MockHttpSession(sessionId='1', ..., isValid=false).

This opens up a bunch of questions:

  1. Could it be that Tomcat's HttpSession implementation behaves slightly differently? The answer to this probably depends on the second question:
  2. Who and when calls the MockHttpSession.invalidate() call - is it Vaadin, Spring or someone else?
  3. Is Tomcat's HttpSession already invalidated during the call to VaadinAwareSecurityContextHolderStrategy.getFromVaadinSession()? If yes, that would imply that Tomcat's HttpSession behaves slightly differently, i.e. it allows access to an attribute even though the session is invalidated. If no, that implies that Karibu's mocking is somewhat imperfect.

This can be worked around quite easily, by disabling checks in MockHttpSession. But before embarking on that route, I'd really love to understand what exactly is going on - I'd love to have answers on the questions above. @simasch could you please investigate a bit and add more information in this regard?

simasch commented 1 year ago

The SecurityContextLogouthandler ist calling invalidate

image

mvysny commented 1 year ago

Is the SecurityContextLogouthandler also calling com.vaadin.flow.spring.security.AuthenticationContext.logout() as seen in the first stacktrace? I'm curious whether Spring invalidates the session first, then calls AuthenticationContext.logout() in the same request which manipulates attributes in the invalidated session. If that is what happens, then that goes against the Servlet spec. However, apparently Servlet containers are okay with that, so perhaps there's something we don't understand: e.g. when a session is invalidated, a new one is immediately created, with no attributes. However, that could lead to a NPE in the caller of VaadinAwareSecurityContextHolderStrategy.getFromVaadinSession().

It would be great to fully figure out the ordering of events and what's exactly going on, before we start disabling protective measurements on the Fake HttpSession implementation.