HttpServletRequest.logout() not functioning

[octopusthu]

Summary

Fail to logout in a Spring MVC Controller via HttpServletRequest.logout().

@Controller
@RequestMapping(value = "/xxx")
public class XxxControler {
    @GetMapping(value = "/yyy")
    public String someMethod(HttpServletRequest request) throws Exception {
        request.logout();
        return "some JSP";
    }

}

Actual Behavior

SecurityContext is not cleared. The user is still treated as authenticated.

Expected Behavior

SecurityContext should be cleared and the user should be treated as unauthenticated.

Version

Spring Boot 1.5.4 (with Spring Security 4.2.3)

[jdtgh]

I ran into this same issue with Spring Boot 2.2.5 (Spring Security 5.2.1). Here is what I think is happening

1. AbstractConfiguredSecurityBuilder maintains configurers in a LinkedHashMap which means they are traversed in the order they are added.
2. I am extending WebSecurityConfigurerAdapter which means ServletApiConfigurer is registered before LogoutConfigurer in the LinkedHashMap.
3. LogoutConfigurer manually adds the SecurityContextLogoutHandler and LogoutSuccessEventPublishingLogoutHandler to its collection of LogoutHandlers during it's configure method.
4. ServletApiConfigurer references the LogoutConfigurer in it's configure method to get the collection of LogoutHandlers, however because of the ordering from steps 1 & 2, ServletApiConfigurer is configured before the LogoutConfigurer and so the SecurityContextLogoutHandler and LogoutSuccessEventPublishingLogoutHandler is not provided to the HttpServlet3RequestFactory. This then means calls to HttpServletRequest.logout do not properly logout the spring-security context.

I was able to workaround the issue by passing disableDefaults=true to the parent WebSecurityConfigurerAdapter, copying the code and then changing the order servletApi and logout is called. Obviously a dirty hack but it confirmed my suspicion.

Based on the spring-security documentation on HttpServletRequest.logout this definitely seems like a bug.

Maybe the solution is as simple as moving the code which adds the extra LogoutHandler implementations into the init method so that it always runs first irrespective of the order the configurers were added.

[dgsaigit]

@jdtgh Spring Security‘s version is 5.2.2 in spring boot 2.2.5, and it has no this bug.
I suggest you use the latest version to try again.

[jdtgh]

I upgraded to Spring Boot 2.2.6 and can still replicate the issue. I have narrowed it down to being caused by me disabling CSRF, which eliminates all the LogoutHandler classes registered in the default HttpSecurity configuration.

I have created a simple project which demonstrates the problem.

https://github.com/jdtgh/spring-security-4760

If you fire the application up as is you get the following spring-security config

        http.csrf().disable()
            .authorizeRequests()
            .anyRequest()
            .permitAll();

If you hit http://localhost:8080/custom-logout in your browser you get the following logged which demonstrates no LogoutHandlers are registered and the security context is not cleared.

org.springframework.security.authentication.AnonymousAuthenticationToken@61d1acc: Principal: anonymousUser; Credentials: [PROTECTED]; Authenticated: true; Details: org.springframework.security.web.authentication.WebAuthenticationDetails@957e: RemoteIpAddress: 127.0.0.1; SessionId: null; Granted Authorities: ROLE_ANONYMOUS
2020-03-27 13:15:19.026 DEBUG 21204 --- [nio-8080-exec-1] o.s.s.w.s.HttpServlet3RequestFactory     : logoutHandlers is null, so allowing original HttpServletRequest to handle logout
org.springframework.security.authentication.AnonymousAuthenticationToken@61d1acc: Principal: anonymousUser; Credentials: [PROTECTED]; Authenticated: true; Details: org.springframework.security.web.authentication.WebAuthenticationDetails@957e: RemoteIpAddress: 127.0.0.1; SessionId: null; Granted Authorities: ROLE_ANONYMOUS

If you modify the SecurityConfig by adding an arbitrary LogoutHandler that does nothing, it works as expected and clears the security context.

        http.csrf().disable()
            .logout().addLogoutHandler( (req, resp, auth) -> {} ).and()
            .authorizeRequests()
            .anyRequest()
            .permitAll();

org.springframework.security.authentication.AnonymousAuthenticationToken@b5cf9941: Principal: anonymousUser; Credentials: [PROTECTED]; Authenticated: true; Details: org.springframework.security.web.authentication.WebAuthenticationDetails@b364: RemoteIpAddress: 0:0:0:0:0:0:0:1; SessionId: null; Granted Authorities: ROLE_ANONYMOUS
null

[dgsaigit]

@jdtgh @spring-issuemaster
The cause of this bug is when instantiating HttpServlet3RequestFactory：

	public void setLogoutHandlers(List<LogoutHandler> logoutHandlers) {
		this.logoutHandler = CollectionUtils.isEmpty(logoutHandlers) ? null : new CompositeLogoutHandler(logoutHandlers);
	}

By default, .servletApi () is in front of logout (), so the logoutHandlers is empty。

Solving this problem is simple, just put .servletApi () after .logout (). like that:
org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter#getHttp

		if (!disableDefaults) {
			// @formatter:off
			http
				.csrf().and()
				.addFilter(new WebAsyncManagerIntegrationFilter())
				.exceptionHandling().and()
				.headers().and()
				.sessionManagement().and()
				.securityContext().and()
				.requestCache().and()
				.anonymous().and()
				.apply(new DefaultLoginPageConfigurer<>()).and()
				.logout().and()
                                .servletApi();
			// @formatter:on

you can test like that :

public class SecurityConfig extends WebSecurityConfigurerAdapter
{
    @Override
    protected void configure( final HttpSecurity http ) throws Exception
    {
        // This configuration does not work as no LogoutHandler's are configured
        http.csrf().disable()
                .servletApi().disable()
                .servletApi().and()
            .authorizeRequests()
            .anyRequest()
            .permitAll();

        // This configuration works as the WebSecurityConfigurerAdapter defaults
        // configure csrf which includes a CsrfLogoutHandler
//        http.authorizeRequests()
//            .anyRequest()
//            .permitAll();

        // It is not unique to CsrfFilter as this works by adding an arbitrary LogoutHandler
//        http.csrf().disable()
//            .logout().addLogoutHandler( (req, resp, auth) -> {} ).and()
//            .authorizeRequests()
//            .anyRequest()
//            .permitAll();
    }
}

[rwinch]

Can someone provide a complete and minimal sample (i.e. GitHub project) demonstrating the issue?

[jdtgh]

I provided a sample project link in my last comment.

#4760 (comment)

[dgsaigit]

@rwinch
this is my demo，and I wrote a test case, you can try it:

https://github.com/sai-studio/spring-security-bug-4760