spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.75k stars 5.88k forks source link

DSL builder constructs inconsistent state with custom RememberMeServices #3762

Open chrylis opened 8 years ago

chrylis commented 8 years ago

Based on the recommendation in #3147, I attempted to implement session extension (not really remember-me) by subclassing NullRememberMeServices, overriding loginSuccess to set a long timeout, and adding the following to my DSL configuration block:

.and()
    .rememberMe().rememberMeServices(new LongSessionRememberMeServices())

Logging out promptly broke. On debugging, it appears that a new null logout handler is being registered with the LogoutFilter, and then when the filter iterates over the handlers it throws an NPE. This doesn't happen with services that extend AbstractRememberMeServices because there is magic logic that treats RememberMeServices & LogoutHandler specially inside RememberMeConfigurer#getRememberMeServices(H,String). Then in RememberMeConfigurer#init(H):234, the logoutHandler is registered without a defensive null check.

The upshot is that registering a RememberMeServices that does not also implement LogoutFilter will cause a distant NPE whenever logout is attempted. Line 233 should read:

if (logoutConfigurer != null && loginHandler != null) {

Manually skipping the then block in the debugger produced the expected correct behavior.

(I'd submit a PR, but I don't know where the appropriate test coverage for this case should go.)

rwinch commented 8 years ago

@chrylis Thanks for creating this ticket. The tests can go in RememberMeConfigurerTests