.sessionRegistry(sessionRegistry()) doesn't seem necessary; documentation should be updated accordingly

[candrews]

Describe the bug I've set up Spring Session JDBC and Spring Security in my Spring Boot 2.2.7 application. I've set the maximum number of sessions for a user to 1 with http.sessionManagement().maximumSessions(1) And that works. Which surprises me. I did not do .sessionRegistry(sessionRegistry()) as documented at https://docs.spring.io/spring-session/docs/current/reference/html5/#spring-security-concurrent-sessions So it seems that .sessionRegistry(sessionRegistry()) is not necessary leading me to believe that the documentation should be updated to remove this step.

To Reproduce

• Make a an application using Spring Boot 2.2.7, Spring Security, and Spring Session
• Configure Spring Security using http.sessionManagement().maximumSessions(1)

Login twice as the same user with different HTTP session (you can use a different browser, private browsing mode, different computers, etc).

When the when the second session logs in, the first session is logged out.

Expected behavior Based on https://docs.spring.io/spring-session/docs/current/reference/html5/#spring-security-concurrent-sessions since .sessionRegistry(sessionRegistry()) is not set, I would have expected the first session to remain authenticated when the second session logs in. Sample https://github.com/candrews/spring-session-session-registry-sample

A test is included. I expect the test not to pass unless .sessionRegistry(sessionRegistry()) is done. Test: https://github.com/candrews/spring-session-session-registry-sample/blob/master/src/test/java/com/example/demo/SingleConcurrentSessionConfigurerTest.java Configuration (see comments): https://github.com/candrews/spring-session-session-registry-sample/blob/master/src/main/java/com/example/demo/WebSecurityConfiguration.java

Run ./mvnw test to run the test.

[rwinch]

@candrews Thanks for the report. As far as I can tell the sample you provided will use the default in memory implementation of SessionRegistry (SessionRegistryImpl). This works for a single JVM as long as the JVM isn't restarted. However, if you have more than one JVM or if you restart the JVM it will no longer work. In order to actually use Spring Session and work with multiple JVMs and restarting the JVM, you must provide the SessionRegistry as instructed in the documentation. Please let me know if I am missing something?

[candrews]

That makes sense... Do you have a suggestion as to how I could write a test to prove that?

[rwinch]

Looks like it is currently using an in memory database. So the first step is to setup the sample to use a data store that is shared (i.e. an external mysql instance). Then get two copies of the app started up on different ports. Then try the same experiment but the first user hits the first app instance and the second user hits the second app instance. There isn't a need for a load balancer for this exercise, but it it is important to remember that cookies are not isolated by port (only scheme, host, and path). This means even though you are running on different ports, you will need to use incognito or another browser instance.

[candrews]

I was hoping to this testing in the form of an integration test like I did in my example with https://github.com/candrews/spring-session-session-registry-sample/blob/master/src/test/java/com/example/demo/SingleConcurrentSessionConfigurerTest.java I'll see if there's a way to start 2 Spring Boot instances and use mock mvc to test each... If you want to provide a tip or link to an existing example, I certainly wouldn't complain :-)

Also - I feel like when Spring Session is used, Spring Boot should autoconfigure this so manually setting up wouldn't be necessary. Do you agree? If so, any tips on how you'd like that implemented so I can submit a PR doing so?

[rwinch]

I was hoping to this testing in the form of an integration test like I did in my example with https://github.com/candrews/spring-session-session-registry-sample/blob/master/src/test/java/com/example/demo/SingleConcurrentSessionConfigurerTest.java I'll see if there's a way to start 2 Spring Boot instances and use mock mvc to test each... If you want to provide a tip or link to an existing example, I certainly wouldn't complain :-)

You could expose SpringSessionBackedSessionRegistry as a @Bean and @Autowire the SessionRegistry and validate that it contains the principal after authenticating.

Also - I feel like when Spring Session is used, Spring Boot should autoconfigure this so manually setting up wouldn't be necessary. Do you agree?

I don't feel like this is true. Not everyone wants concurrency control, so that shouldn't be done by default.

We could create a ticket so that if you configure maximumSessions then any SessionRegistry beans would be picked up automatically. That would simplify it a little in that you would only need to expose SpringSessionBackedSessionRegistry as a Bean. If you would like to work on this, please create a separate ticket with Spring Security and I can point you in the right direction to getting a PR together.

[candrews]

I took a stab at doing the two things you pointed out. Notably I haven't tested them or polished them - before investing that kind of effort, I'm hoping you indicate if my approach is on the right track.

We could create a ticket so that if you configure maximumSessions then any SessionRegistry beans would be picked up automatically.

How's this look? https://github.com/spring-projects/spring-security/commit/1bf05ca1944e8e51100ff37c3dfed02220baa6d8

That would simplify it a little in that you would only need to expose SpringSessionBackedSessionRegistry as a Bean.

I think autoconfiguration can do this as appropriate... how's this? https://github.com/spring-projects/spring-boot/commit/a9154ce5e27a0886db1052e5af6b83b0496fdca3

Thank you again!

[rwinch]

I commented on the commit you referenced for maximumSessions.

For the autoconfiguration, you will want to discuss with the Boot team. My impression is that it is unlikely Boot would want to create a bean that users do not necessarily want to use. The bean would only be created if concurrency control within Spring Security is activated and there isn't a good mechanism in place to do this.