vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
603 stars 165 forks source link

Exception in push connection when logging out #11026

Open Hayo-Baan opened 3 years ago

Hayo-Baan commented 3 years ago

Description of the bug / feature

Since switching to the default Push transport (#10931) we always get an “Exception in push connection” when logging out.

Note: the exception is not thrown when simply closing the browser window…

Minimal reproducible example

Our application is a pretty basic view/form application. Login/authentication is handled by a remote service using REST calls (KioskAuthenticationProvider). Logout is via a “logout” anchor in the top bar of the layout.

Our security configuration is as follows:

@RequiredArgsConstructor
@EnableWebSecurity
@Configuration
public class SecurityConfiguration extends WebSecurityConfigurerAdapter {

    private static final String LOGIN_PROCESSING_URL = "/login";
    private static final String LOGIN_FAILURE_URL = "/login?error";
    private static final String LOGIN_URL = "/login";
    private static final String LOGOUT_SUCCESS_URL = "/login";

    private final LoginService loginService;

    /**
     * Tests if the request is an internal framework request. The test consists of
     * checking if the request parameter is present and if its value is consistent
     * with any of the request types know.
     *
     * @param request {@link HttpServletRequest}
     * @return true if the request is an internal framework request. False otherwise.
     */
    public static boolean isFrameworkInternalRequest(HttpServletRequest request) {
        final String parameterValue = request.getParameter(ApplicationConstants.REQUEST_TYPE_PARAMETER);
        return parameterValue != null &&
               Stream.of(HandlerHelper.RequestType.values()).anyMatch(r -> r.getIdentifier().equals(parameterValue));
    }

    @Bean
    public AuthenticationProvider authenticationProvider() {
        return new KioskAuthenticationProvider(loginService);
    }

    @Override
    protected void configure(AuthenticationManagerBuilder auth) {
        auth.authenticationProvider(authenticationProvider());
    }

    /**
     * Require login to access internal pages and configure login form.
     */
    @Override
    protected void configure(HttpSecurity http) throws Exception {
        // Not using Spring CSRF here to be able to use plain HTML for the login page
        http.csrf()
            .disable()

            // Register our CustomRequestCache, that saves unauthorized access attempts, so
            // the user is redirected after login.
            .requestCache()
            .requestCache(new SimpleRequestCache())

            // Restrict access to our application.
            .and()
            .authorizeRequests()

            // Allow health checks
            .antMatchers("/actuator/health")
            .permitAll()

            // Allow all Vaadin internal requests.
            .requestMatchers(SecurityConfiguration::isFrameworkInternalRequest)
            .permitAll()

            // Allow all other requests by logged in users.
            .anyRequest()
            .authenticated()

            // Configure the login page.
            .and()
            .formLogin()
            .loginPage(LOGIN_URL)
            .permitAll()
            .loginProcessingUrl(LOGIN_PROCESSING_URL)
            .failureUrl(LOGIN_FAILURE_URL)

            // Configure logout
            .and()
            .logout()
            .logoutSuccessUrl(LOGOUT_SUCCESS_URL);
    }

    /**
     * Allows access to static resources, bypassing Spring security.
     */
    @Override
    public void configure(WebSecurity web) {
        web.ignoring().antMatchers(
                // Client-side JS
                "/VAADIN/**",

                // the standard favicon URI
                "/favicon.ico",

                // the robots exclusion standard
                "/robots.txt",

                // web application manifest
                "/manifest.webmanifest", "/sw.js", "/offline.html",

                // icons and images
                "/icons/**", "/META-INF/resources/images/**", "/styles/**");
    }

}

The UI is configured as follows:

/**
 * Component to set up the Vaadin UI.
 *
 * Sets up the locale and adds a {@link BeforeEnterEvent} handler to redirect to the login page if a user isn't already
 * logged-in.
 */
@Component
public class ManagementServiceInitListener implements VaadinServiceInitListener {

    public ManagementServiceInitListener(@NotNull CommonProperties commonProperties) {
        Locale.setDefault(commonProperties.getLocaleFromProperties());
    }

    @Override
    public void serviceInit(@NotNull ServiceInitEvent event) {
        event.getSource().addUIInitListener(uiEvent -> {
            UI ui = uiEvent.getUI();
            ui.addBeforeEnterListener(this::beforeEnter);
        });
    }

    /**
     * Reroutes the user to the login page unless already logged-in.
     *
     * @param event before navigation event with event details.
     */
    protected void beforeEnter(@NotNull BeforeEnterEvent event) {
        if (!LoginView.class.equals(event.getNavigationTarget()) && !LoginService.isUserLoggedIn()) {
            event.rerouteTo(LoginView.class);
        }
    }

}

The login view is straightforward:

@Route("login")
@PageTitle("Login | Management Console")
public class LoginView extends VerticalLayout implements BeforeEnterObserver {

    private final LoginForm loginForm;

    public LoginView() {
        addClassName("login-view");
        setSizeFull();
        setJustifyContentMode(JustifyContentMode.CENTER);
        setAlignItems(Alignment.CENTER);

        H1 title = new H1("Management Console");

        loginForm = new LoginForm(createDutchLogin());
        configureLoginForm();

        add(title, loginForm);
    }

    @Override
    public void beforeEnter(@NotNull BeforeEnterEvent beforeEnterEvent) {
        if (beforeEnterEvent.getLocation().getQueryParameters().getParameters().containsKey("error")) {
            loginForm.setError(true);
        }
    }

    private @NotNull LoginI18n createDutchLogin() {
        LoginI18n loginI18n = LoginI18n.createDefault();

        // Form
        LoginI18n.Form form = loginI18n.getForm();
        form.setTitle("Inloggen");
        form.setUsername("Gebruikersnaam");
        form.setPassword("Wachtwoord");
        form.setForgotPassword("Wachtwoord vergeten?");
        form.setSubmit("Log in");

        // Error Message
        LoginI18n.ErrorMessage errorMessage = loginI18n.getErrorMessage();
        errorMessage.setTitle("Probleem bij het inloggen");
        errorMessage.setMessage("Controleer of u de juiste gegevens heeft ingevoerd en probeer het dan nogmaals.");

        return loginI18n;
    }

    private void configureLoginForm() {
        loginForm.setForgotPasswordButtonVisible(false);
        loginForm.setAction("login");
    }

}

We don't have a logout view; you simply get redirected to login. However, to try to follow the advise in https://vaadin.com/forum/thread/17520891/flow-logout, I did add a logout view with VaadinSession.getCurrent().getSession().invalidate(); in the BeforeEnter (note: adding the navigation line caused an invalid redirect in the browser!). Interestingly this was actually quite hard to get to trigger, I had to change the route to something other than logout (including the anchor) for it to trigger. But even when that still did not get rid of the push exception…

Am I triggering the logout wrongly?

Expected behavior

No more exceptions in the push connection when logging out.

Actual behavior

Example exception

2021-05-18 11:27:53.227 ERROR 58262 --- [sphere-Shared-3] c.v.f.s.c.PushAtmosphereHandler          : Exception in push connection

java.io.IOException: Connection remotely closed for 82791b2a-7216-4486-8ad2-b33e423d1b78
    at org.atmosphere.websocket.WebSocket.write(WebSocket.java:230) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.websocket.WebSocket.write(WebSocket.java:220) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.websocket.WebSocket.write(WebSocket.java:46) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.cpr.AtmosphereResponseImpl$Stream.write(AtmosphereResponseImpl.java:957) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.handler.AbstractReflectorAtmosphereHandler.onStateChange(AbstractReflectorAtmosphereHandler.java:155) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at com.vaadin.flow.server.communication.PushAtmosphereHandler.onStateChange(PushAtmosphereHandler.java:54) ~[flow-server-2.6.0.jar:2.6.0]
    at org.atmosphere.cpr.DefaultBroadcaster.invokeOnStateChange(DefaultBroadcaster.java:1037) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.cpr.DefaultBroadcaster.prepareInvokeOnStateChange(DefaultBroadcaster.java:1057) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.cpr.DefaultBroadcaster.executeAsyncWrite(DefaultBroadcaster.java:871) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at org.atmosphere.cpr.DefaultBroadcaster$2.run(DefaultBroadcaster.java:474) ~[atmosphere-runtime-2.4.30.slf4jvaadin1.jar:2.4.30.slf4jvaadin1]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[na:na]
    at java.base/java.lang.Thread.run(Thread.java:834) ~[na:na]

Versions:

- Vaadin / Flow version: 14.6.0
- Java version: 11
- OS version: AWS & Mac OS
miguelatvaadin commented 3 years ago

Hi @HayoBaanAtHand ,

thanks a lot for creating this issue.

I see you have more or less followed the guidelines from the user guide, and that you are using the spring security. The only piece I miss there is the one from where you are asynchronously updating the UI with ui.access.

Please note that VaadinSession.getCurrent().getSession().invalidate(); is not necessary when using spring security. I'm afraid I was not aware that you were using spring security when I told you about https://vaadin.com/forum/thread/17520891/flow-logout.

After working on your case trying to mimic your scenario, I have not been able to reproduce the issue (never got the "Connection remotely closed", whatever I did). That's essential for us to be able to sort it out.

As an addendum, I have been able to find some few posts in forums from users reporting the same exception but, unfortunately, there was no progress. I understand the problem disappeared for them, whatever they did. I did not find any other issue where this exception was reported, in our repo.

So, we would need to reproduce the error to work further on it. Please feel free to provide more info so we can reproduce the issue. I will leave this issue open for some time to give you the chance to provide a reproduceable use case, before closing this issue

Best,

Hayo-Baan commented 3 years ago

I indeed followed the guidelines quite closely 😄 (this is our first Vaadin project). The problem exhibits itself always, even if after logging in (which visits a simple welcome page with just a logo and some text) you directly log out. I don't think push has come into play there at all.

Good to hear I don't need the session invalidation with Spring security. That rules-out that part. I'll see if I can reproduce it with a small stand-alone demo program. Note: Since others have had this problem too and it went away for them, it could be a combination of library versions.

miguelatvaadin commented 3 years ago

I don't think push has come into play there at all

mmm... the atmosphere is used for push

I'll see if I can reproduce it with a small stand-alone demo program

That would be great. We are really interested on sorting it out, but we first need to be able to reproduce it. Thanks, @HayoBaanAtHand !

Hayo-Baan commented 3 years ago

I've succeeded in creating a stand-alone mini application that exhibits the issue. I have attached it. The application can be started from the command-line using e.g. mvn -U spring-boot:run -Dspring-boot.run.arguments='--spring.config.location=file:config/bootstrap.properties' The app can then be reached at http://localhost:8080/management provide any name/password (I removed the actual checks) to get to the welcome page. When you then logout using the logout link at the top right, you logout and the log shows the push exception.

pushError.zip

miguelatvaadin commented 3 years ago

Thanks a lot for the project, @HayoBaanAtHand . It has made the difference!

I have finally been able to see the error. Hurray!

This is my setup:

I will now label the issue so it can be addressed

Hayo-Baan commented 3 years ago

Interesting: Safari also doesn't throw the error.

MariusWit commented 2 years ago

Just had the same issue with Firefox only with Push enabled. It seems doing additional navigational changes on logout result in the exception being thrown. This seems to happen for rerouting and for example UI.getCurrent().getPage().setLocation(LOGOUT_SUCCESS_URL)

removing this line and relying on the URL set by http.logout().logoutSuccessUrl(LOGOUT_SUCCESS_URL) works.

In your case you're redirecting with event.rerouteTo(LoginView.class) The condition you set !LoginView.class.equals(event.getNavigationTarget() doesn't prevent the reroute on Firefox because it makes an additional request to the URL the user was on when the logout is invoked. This leads to the condition being true at least once and thus leads to a redirection right after logout.

The additonal request to the URL does not happen with other browsers I tried, not sure if it's a Vaadin related or Firefox related issue.

Hayo-Baan commented 2 years ago

Thanks for your feedback! Good to hear you got rid of the push exception error when using Firefox.

I've investigated your remark some more to see if I could get it fixed in my case too. You suggested that the event.rerouteTo(LoginView.class) in my ManagementServiceInitListener (see below) could be the cause of the problem. However, after some debugging, I actually found this line is never called at all; the condition never holds true. I guess that the redirect already takes place under water. I've now removed the ManagementServiceInitListener component altogether. The push exceptions, however, still occur (for me). I've added an updated version of a demo application that exhibits this bug.

/**
 * Component to set up the Vaadin UI.
 * <p>
 * Sets up the locale and adds a {@link BeforeEnterEvent} handler to redirect to the login page if a user isn't already
 * logged-in.
 */
@Component
public class ManagementServiceInitListener implements VaadinServiceInitListener {

    @Override
    public void serviceInit(ServiceInitEvent event) {
        event.getSource().addUIInitListener(uiEvent -> {
            UI ui = uiEvent.getUI();
            ui.addBeforeEnterListener(this::beforeEnter);
        });
    }

    /**
     * Reroutes the user to the login page unless already logged-in.
     *
     * @param event before navigation event with event details.
     */
    protected void beforeEnter(BeforeEnterEvent event) {
         if (!LoginView.class.equals(event.getNavigationTarget()) &&
            !SecurityContextHolder.getContext().getAuthentication().isAuthenticated()) {
            event.rerouteTo(LoginView.class);
        }
    }

}

Bug.zip

iaklass commented 1 year ago

Is there any update on this issue? I am getting the exact same error as @Hayo-Baan when using Firefox as @miguelatvaadin mentioned. Chrome and Edge does not cause this error.

@MariusWit I get this error although I do not navigate explicitly on logout. The logout action in my application looks like this:

@Service
public class SecurityService {

    @Autowired
    private final AuthenticationContext authenticationContext;
...

    public void logout() {
        this.authenticationContext.logout();
    }
}
eliamanara commented 9 months ago

Having the same issue as @iaklass here. Any update?

berowil commented 7 months ago

I can confirm the problem as well. Waiting patiently for a solution.

fred314 commented 1 month ago

I have the same problem here (Spring Boot 3.2.3 and Vaadin 24.3.6 for the moment).

1/ I get the same error with Firefox (and not with Chrome) when I log out with UI.getCurrent().getPage().setLocation("/logout"); and with the following security config:

@Slf4j
@Configuration
@EnableWebSecurity
public class SecurityConfig extends VaadinWebSecurity {

    @Autowired
    private ClientRegistrationRepository clientRegistrationRepository;

    @Autowired
    private HandlerMappingIntrospector handler;

    @Bean
    @Primary
    public GrantedAuthoritiesMapper userAuthoritiesMapperForKeycloak() {
        return authorities -> {
            Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
            var authority = authorities.iterator().next();

            if (authority instanceof OidcUserAuthority oidcUserAuthority) {
                var userInfo = oidcUserAuthority.getUserInfo();

                if (userInfo.hasClaim("realm_access")) {
                    var realmAccess = userInfo.getClaimAsMap("realm_access");
                    if (realmAccess.get("roles") instanceof Collection<?> roles) {
                        mappedAuthorities.addAll(roles.stream()
                                .map(role -> new SimpleGrantedAuthority(role.toString().toUpperCase()))
                                .toList());
                    }
                }
            }
            return mappedAuthorities;
        };
    }

    @Override
    public void configure(HttpSecurity http) throws Exception {
        MvcRequestMatcher.Builder builder = new MvcRequestMatcher.Builder(handler);
        http
                .authorizeHttpRequests(c -> c
                        .requestMatchers(builder.pattern("/actuator/**")).permitAll()
                );

        super.configure(http);

        http
                .oauth2Login(c -> c
                        .successHandler(new VaadinSavedRequestAwareAuthenticationSuccessHandler())
                )
                .sessionManagement(c -> c
                        .sessionAuthenticationStrategy(new RegisterSessionAuthenticationStrategy(new SessionRegistryImpl()))
                )
                .logout(c -> c
                        .logoutSuccessHandler(oidcLogoutSuccessHandler())
                        .logoutRequestMatcher(builder.pattern(HttpMethod.GET, "/logout"))
                );
    }

    @Bean
    public LogoutSuccessHandler oidcLogoutSuccessHandler() {
        OidcClientInitiatedLogoutSuccessHandler successHandler = new OidcClientInitiatedLogoutSuccessHandler(clientRegistrationRepository);
        successHandler.setPostLogoutRedirectUri("{baseUrl}/home");
        return successHandler;
    }
}

2/ If I replace in my security config .logoutRequestMatcher(builder.pattern(HttpMethod.GET, "/logout")) by .logoutUrl("/logout").permitAll(), then I don't see the error when I log out with Firefox but I have to confirm the logout on the Spring security page (same behavior with Chrome) image

3/ And I get the error with Firefox (and not with Chrome) when I log out with authenticationContext.logout(); instead of UI.getCurrent().getPage().setLocation("/logout"); with both of the security config described above.

I hope this could help.

EDIT 1: to complete my message, those tests were executed with @Push(value = PushMode.MANUAL). And for my second point, I did not get the error if I have only one tab opened in Firefox. But for all additional tabs, I still get the error. EDIT 2: if I change the transport to Long Polling in the Push annotation (and not the default Websocket XHR), the code authenticationContext.logout(); doesn't work (nothing happened). On the other hand, the code UI.getCurrent().getPage().setLocation("/logout"); works and the error is not there but I see occasionally the following warning:

2024-08-06T13:36:29.101+02:00  WARN [ui,,] 211824 --- [Atmosphere-Shared-10] org.atmosphere.cpr.DefaultBroadcaster    : This message com.vaadin.flow.server.communication.AtmospherePushConnection$PushMessage@213d34ec will be lost for AtmosphereResource 661028ba-d330-4be1-a338-0cf7a1f1f927, adding it to the BroadcasterCache
2024-08-06T13:36:29.101+02:00  WARN [ui,,] 211824 --- [Atmosphere-Shared-10] org.atmosphere.cpr.DefaultBroadcaster    : Failed to execute a write operation for Broadcaster /*

java.lang.IllegalStateException: The request object has been recycled and is no longer associated with this facade
    at org.apache.catalina.connector.RequestFacade.checkFacade(RequestFacade.java:855) ~[tomcat-embed-core-10.1.19.jar:10.1.19]
    at org.apache.catalina.connector.RequestFacade.removeAttribute(RequestFacade.java:419) ~[tomcat-embed-core-10.1.19.jar:10.1.19]
    at org.apache.catalina.core.ApplicationHttpRequest.removeAttribute(ApplicationHttpRequest.java:270) ~[tomcat-embed-core-10.1.19.jar:10.1.19]
    at jakarta.servlet.ServletRequestWrapper.removeAttribute(ServletRequestWrapper.java:246) ~[tomcat-embed-core-10.1.19.jar:6.0]
    at jakarta.servlet.ServletRequestWrapper.removeAttribute(ServletRequestWrapper.java:246) ~[tomcat-embed-core-10.1.19.jar:6.0]
    at jakarta.servlet.ServletRequestWrapper.removeAttribute(ServletRequestWrapper.java:246) ~[tomcat-embed-core-10.1.19.jar:6.0]
    at jakarta.servlet.ServletRequestWrapper.removeAttribute(ServletRequestWrapper.java:246) ~[tomcat-embed-core-10.1.19.jar:6.0]
    at jakarta.servlet.ServletRequestWrapper.removeAttribute(ServletRequestWrapper.java:246) ~[tomcat-embed-core-10.1.19.jar:6.0]
    at org.atmosphere.cpr.AtmosphereRequestImpl.removeAttribute(AtmosphereRequestImpl.java:679) ~[atmosphere-runtime-3.0.3.slf4jvaadin2.jar:3.0.3.slf4jvaadin2]
    at org.atmosphere.cpr.DefaultBroadcaster.executeAsyncWrite(DefaultBroadcaster.java:911) ~[atmosphere-runtime-3.0.3.slf4jvaadin2.jar:3.0.3.slf4jvaadin2]
    at org.atmosphere.cpr.DefaultBroadcaster$2.run(DefaultBroadcaster.java:477) ~[atmosphere-runtime-3.0.3.slf4jvaadin2.jar:3.0.3.slf4jvaadin2]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:317) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[na:na]
    at java.base/java.lang.Thread.run(Thread.java:1583) ~[na:na]