RequestUtil.isAnonymousRouteInternal should lock the session

[jorgheymans]

Description of the bug

RequestUtil.isAnonymousRoute is used as a RequestMatcher in VaadinWebSecurity. This leads to below exception (in development mode only) when it tries to check if the route is internal:

java.lang.IllegalStateException: Cannot access state in VaadinSession or UI without locking the session.
    at com.vaadin.flow.server.VaadinSession.checkHasLock(VaadinSession.java:572)
    at com.vaadin.flow.server.VaadinSession.checkHasLock(VaadinSession.java:586)
    at com.vaadin.flow.server.VaadinSession.getAttribute(VaadinSession.java:823)
    at com.vaadin.flow.server.SessionRouteRegistry.getSessionRegistry(SessionRouteRegistry.java:75)
    at com.vaadin.flow.router.Router.getRegistry(Router.java:282)
    at com.vaadin.flow.spring.security.RequestUtil.isAnonymousRouteInternal(RequestUtil.java:209)
    at com.vaadin.flow.spring.security.RequestUtil.isAnonymousRoute(RequestUtil.java:121)
    at org.springframework.security.web.util.matcher.RequestMatcher.matcher(RequestMatcher.java:48)
    at org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager.check(RequestMatcherDelegatingAuthorizationManager.java:79)
    at org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager.check(RequestMatcherDelegatingAuthorizationManager.java:48)

The funny thing is that this does not happen all the time. But when it does, i need to restart the application to make it go away.

Expected behavior

The exception should not happen.

Minimal reproducible example

I do not have an example that can always reproduce this, maybe it's a race condition somewhere.

Versions

Browser: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:126.0) Gecko/20100101 Firefox/126.0 Live reload: Java unavailable (HotswapAgent): Front end unavailable

Vaadin 24.3.12

[mcollovati]

It looks like that during the spring security filter execution, the VaadinSession thread local has a value, even if it should be null, since the filter is executed before VaadinServlet could set that value.

@jorgheymans could it be possible that in your code the VaadinSession thread local is set but not cleaned up (e.g. VaadinSession.setCurrent(...))?

[jorgheymans]

We don't have any reference to VaadinSession in our code. Our only customization is VaadinWebSecurity:

@EnableWebSecurity
@EnableMethodSecurity
@Configuration
@ConditionalOnClass(name = "com.vaadin.flow.spring.security.VaadinWebSecurity")
class CustomVaadinWebSecurity extends VaadinWebSecurity {

  @Autowired CustomAuthenticationFilter customAuthenticationFilter;

  @Override
  protected void configure(HttpSecurity http) throws Exception {
    // actuator must not require authentication
    http.authorizeHttpRequests(
        registry ->
            registry.requestMatchers(new AntPathRequestMatcher("/actuator/**")).permitAll());
    super.configure(http);
    http.addFilter(customAuthenticationFilter);
  }
}

[mcollovati]

So, there's something that during a request is setting the thread local but not cleaning it, causing the subsequent requests on the same thread to fail when the spring security filter is executed. Can you share some additional details about the application, for example if you are using add-ons, PUSH, some custom code that might interact with low-level API such as VaadinService, etc. ?

[jorgheymans]

• no PUSH
• addons: font-awesome-iron-iconset, panel-for-vaadin, viritin, togglebutton

• we do have a VaadinServiceInitListener :-(

  public class LocaleInitializer implements VaadinServiceInitListener {
  
  @Override
  public void serviceInit(ServiceInitEvent event) {
  event.getSource().addSessionInitListener(this::sessionInit);
  }
  
  private void sessionInit(SessionInitEvent event) {
  event.getSession().addRequestHandler(new LocaleRequestHandler());
  event.getSession().setErrorHandler(new CustomErrorHandler());
  }
  }

  and LocaleRequestHandler is this, i'm guessing that's our culprit ?

  public class LocaleRequestHandler implements RequestHandler {
  @Override
  public boolean handleRequest(
    VaadinSession session, VaadinRequest request, VaadinResponse response) throws IOException {
  Locale requestLocale = request.getLocale();
  session.lock();
  try {
    if (requestLocale != null && !requestLocale.equals(session.getLocale())) {
      log.debug("request locale is different from session locale, setting {}", requestLocale);
      session.setLocale(requestLocale);
    }
  } finally {
    session.unlock();
  }
  return false; // Return false to let other request handlers do their job
  }
  }

Not even sure we still need a locale request handler, as we've moved away from onLocaleChanged() in all our pages and a full page reload is done when the locale is changed in our app.

[mcollovati]

Thanks for sharing the details. I'll do further investigation.