flow icon indicating copy to clipboard operation
flow copied to clipboard

Exception in push connection when logging out

Open Hayo-Baan opened this issue 3 years ago • 7 comments

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

Hayo-Baan avatar May 18 '21 10:05 Hayo-Baan

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,

miguelatvaadin avatar May 20 '21 06:05 miguelatvaadin

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.

Hayo-Baan avatar May 20 '21 06:05 Hayo-Baan

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 !

miguelatvaadin avatar May 20 '21 07:05 miguelatvaadin

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

Hayo-Baan avatar May 20 '21 11:05 Hayo-Baan

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:

  • jdk 11.0.9 from Oracle
  • maven 3.6.3
  • linux
  • with chrome it works
  • with firefox the exception is thrown

I will now label the issue so it can be addressed

miguelatvaadin avatar May 20 '21 14:05 miguelatvaadin

Interesting: Safari also doesn't throw the error.

Hayo-Baan avatar May 20 '21 14:05 Hayo-Baan

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.

MariusWit avatar Aug 13 '22 23:08 MariusWit

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

Hayo-Baan avatar Aug 19 '22 13:08 Hayo-Baan

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

iaklass avatar Aug 29 '23 12:08 iaklass

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

eliamanara avatar Dec 06 '23 09:12 eliamanara

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

berowil avatar Jan 18 '24 11:01 berowil