New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring Security Concurrent Session Integration #65

Closed
rwinch opened this Issue Nov 18, 2014 · 20 comments

Comments

Projects
None yet
7 participants
@rwinch
Member

rwinch commented Nov 18, 2014

Spring Security allows for ensuring only one user with a given username is authenticated at a time through the concurrency control. This is nice for something that is a subscription model (i.e. pay per user) to ensure that people are not sharing usernames.

If two sessions exist with the same username, Spring Security will invalidate one of the sessions. Currently it does this by marking a session as invalid and the next time the user visits the page the session is actually invalidated. This is necessary due to Java EE limitations.

Spring Security should provide an implementation of concurrency control that will use Spring Session's APIs to immediately invalidate a session.

@rwinch rwinch changed the title from Spring Security Concurrency Integration to Spring Security Concurrent Session Integration Nov 24, 2014

@yacota

This comment has been minimized.

Show comment
Hide comment
@yacota

yacota Jan 28, 2015

Is there any plan to include this issue in a release?
From one of your comments at https://spring.io/blog/2014/11/18/spring-session-1-0-0-rc1-released#comment-1699673845 ... we'll have to wait until 1.1.0 at least.

yacota commented Jan 28, 2015

Is there any plan to include this issue in a release?
From one of your comments at https://spring.io/blog/2014/11/18/spring-session-1-0-0-rc1-released#comment-1699673845 ... we'll have to wait until 1.1.0 at least.

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Jan 28, 2015

Member

@yacota Thanks for your interest. At this time there are no concrete plans of when this integration would be made. Of course a good PR with proper testing would almost guarantee inclusion in the next feature release :)

One other question is...does integration between Spring Session & Security belong in Spring Security or in Spring Session? This will be something that we can easily change later, but it is something I may want feedback from @wilkinsona on.

Member

rwinch commented Jan 28, 2015

@yacota Thanks for your interest. At this time there are no concrete plans of when this integration would be made. Of course a good PR with proper testing would almost guarantee inclusion in the next feature release :)

One other question is...does integration between Spring Session & Security belong in Spring Security or in Spring Session? This will be something that we can easily change later, but it is something I may want feedback from @wilkinsona on.

@yacota

This comment has been minimized.

Show comment
Hide comment
@yacota

yacota Jan 29, 2015

Thanks @rwinch for your rapid answer.
I see your point about where does this integration belong to.
Another point I am afraid of is that implementation details could vary "a lot" depending on which session repository is used (Redis, Hazelcast, ...), maybe a core API interface that deals with main concepts like session invalidation, .... with other jars that really implement it (-redis.jar, -hazelcast.jar) could work.
I think that this is a similar approach took in Spring Social design

Thanks

yacota commented Jan 29, 2015

Thanks @rwinch for your rapid answer.
I see your point about where does this integration belong to.
Another point I am afraid of is that implementation details could vary "a lot" depending on which session repository is used (Redis, Hazelcast, ...), maybe a core API interface that deals with main concepts like session invalidation, .... with other jars that really implement it (-redis.jar, -hazelcast.jar) could work.
I think that this is a similar approach took in Spring Social design

Thanks

@jsbingist

This comment has been minimized.

Show comment
Hide comment
@jsbingist

jsbingist Jun 25, 2015

Hello, I am new here. Can anyone explain what this issue about? I was confused when it links to a section in Spring Security official doc that I am reading (and suspecting I found a bug).

jsbingist commented Jun 25, 2015

Hello, I am new here. Can anyone explain what this issue about? I was confused when it links to a section in Spring Security official doc that I am reading (and suspecting I found a bug).

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Jun 25, 2015

Member

@dnang See if the updated description answers your question.

Member

rwinch commented Jun 25, 2015

@dnang See if the updated description answers your question.

@jsbingist

This comment has been minimized.

Show comment
Hide comment
@jsbingist

jsbingist Jun 26, 2015

Excellent @rwinch, it's clear now.

jsbingist commented Jun 26, 2015

Excellent @rwinch, it's clear now.

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Aug 12, 2015

Member

@dnang As an FYI this is now supported in the latest snapshot (indirectly) through resolving #4 See the updated documentation http://docs.spring.io/spring-session/docs/current-SNAPSHOT/reference/html5/#httpsession-httpsessionlistener

We still plan on adding an improved implementation as described in this issue, but I wanted to communicate that you can somewhat work around the limitation now.

Member

rwinch commented Aug 12, 2015

@dnang As an FYI this is now supported in the latest snapshot (indirectly) through resolving #4 See the updated documentation http://docs.spring.io/spring-session/docs/current-SNAPSHOT/reference/html5/#httpsession-httpsessionlistener

We still plan on adding an improved implementation as described in this issue, but I wanted to communicate that you can somewhat work around the limitation now.

@dllongo

This comment has been minimized.

Show comment
Hide comment
@dllongo

dllongo Dec 10, 2015

Hi @rwinch , I'm trying to make the spring session/security works with the maxSessionsPreventsLogin(true), but in my login page, even after call logout url the response in login is:
Your login attempt was not successful, try again.
Reason: Maximum sessions of 1 for this principal exceeded

can you help me?

dllongo commented Dec 10, 2015

Hi @rwinch , I'm trying to make the spring session/security works with the maxSessionsPreventsLogin(true), but in my login page, even after call logout url the response in login is:
Your login attempt was not successful, try again.
Reason: Maximum sessions of 1 for this principal exceeded

can you help me?

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Dec 10, 2015

Member

Are you using 1.1.0.M1? Did you follow the instructions in the reference?
http://docs.spring.io/spring-session/docs/1.1.0.M1/reference/html5/#httpsession-httpsessionlistener
On Dec 10, 2015 7:05 AM, "Diogo Longo" notifications@github.com wrote:

Hi @rwinch https://github.com/rwinch , I'm trying to make the spring
session/security works with the maxSessionsPreventsLogin(true), but in my
login page, even after call logout url the response in login is:

Your login attempt was not successful, try again. Reason: Maximum
sessions of 1 for this principal exceeded

can you help me?


Reply to this email directly or view it on GitHub
#65 (comment)
.

Member

rwinch commented Dec 10, 2015

Are you using 1.1.0.M1? Did you follow the instructions in the reference?
http://docs.spring.io/spring-session/docs/1.1.0.M1/reference/html5/#httpsession-httpsessionlistener
On Dec 10, 2015 7:05 AM, "Diogo Longo" notifications@github.com wrote:

Hi @rwinch https://github.com/rwinch , I'm trying to make the spring
session/security works with the maxSessionsPreventsLogin(true), but in my
login page, even after call logout url the response in login is:

Your login attempt was not successful, try again. Reason: Maximum
sessions of 1 for this principal exceeded

can you help me?


Reply to this email directly or view it on GitHub
#65 (comment)
.

@dllongo

This comment has been minimized.

Show comment
Hide comment
@dllongo

dllongo Dec 10, 2015

I'm was using Snapshot, changing to 1.1.0.M1 the same error occurred, I published the project on github
https://github.com/dllongo/LoginSpring

I would appreciate if you could take a look, Tks!

dllongo commented Dec 10, 2015

I'm was using Snapshot, changing to 1.1.0.M1 the same error occurred, I published the project on github
https://github.com/dllongo/LoginSpring

I would appreciate if you could take a look, Tks!

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Dec 10, 2015

Member

@dllongo I sent a PR dllongo/LoginSpring#1 with details of the issues documented in the git commit messages

Member

rwinch commented Dec 10, 2015

@dllongo I sent a PR dllongo/LoginSpring#1 with details of the issues documented in the git commit messages

@dllongo

This comment has been minimized.

Show comment
Hide comment
@dllongo

dllongo Dec 11, 2015

@rwinch your PR works, tks!

dllongo commented Dec 11, 2015

@rwinch your PR works, tks!

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Dec 11, 2015

Member

@dllongo I'm glad that helps!

Member

rwinch commented Dec 11, 2015

@dllongo I'm glad that helps!

@jkubrynski

This comment has been minimized.

Show comment
Hide comment
@jkubrynski

jkubrynski Mar 9, 2016

Contributor

I see few problems here:

  1. There is no feature in SessionRepository to retrieve all sessions/principals
  2. SessionRegistry operates a lot on principal which is available on Session only via attribute.
  3. Information like lastRequest is available only for ExpiringSession

The hardest to do is currently 1 because it needs changes in all repositories.

Contributor

jkubrynski commented Mar 9, 2016

I see few problems here:

  1. There is no feature in SessionRepository to retrieve all sessions/principals
  2. SessionRegistry operates a lot on principal which is available on Session only via attribute.
  3. Information like lastRequest is available only for ExpiringSession

The hardest to do is currently 1 because it needs changes in all repositories.

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Mar 10, 2016

Member

@jkubrynski Thank you for your analysis. I think we have a few options here:

  • Extract a simplified interface out. Many of the methods on SessionRegistry are not actually used by the framework. For this reason, I think we could likely extract a simplified interface from SessionRegistry. Then all the locations that use SessionRegistry would leverage that.
  • We could create a new improved API (i.e. one that doesn't rely on SessionInformation.expireNow() with only the methods necessary. We could then provide an adapter to adapt the old API to the new API (and vice versa). We would also deprecate the old API. This option is probably ideal, but a bit more ambitious that the first option.
  • We could provide different implementations of all the logic for session control. This seems like the less attractive option.
Member

rwinch commented Mar 10, 2016

@jkubrynski Thank you for your analysis. I think we have a few options here:

  • Extract a simplified interface out. Many of the methods on SessionRegistry are not actually used by the framework. For this reason, I think we could likely extract a simplified interface from SessionRegistry. Then all the locations that use SessionRegistry would leverage that.
  • We could create a new improved API (i.e. one that doesn't rely on SessionInformation.expireNow() with only the methods necessary. We could then provide an adapter to adapt the old API to the new API (and vice versa). We would also deprecate the old API. This option is probably ideal, but a bit more ambitious that the first option.
  • We could provide different implementations of all the logic for session control. This seems like the less attractive option.
@jkubrynski

This comment has been minimized.

Show comment
Hide comment
@jkubrynski

jkubrynski Mar 10, 2016

Contributor

It looks like first solution is optimal as there is not too much side work to do, and we still get full functionality. As I understand there should be a dedicated ticket for this in SpringSecurity project?

Contributor

jkubrynski commented Mar 10, 2016

It looks like first solution is optimal as there is not too much side work to do, and we still get full functionality. As I understand there should be a dedicated ticket for this in SpringSecurity project?

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Mar 10, 2016

Member

@jkubrynski Thanks! I have yet to create any Spring Security issues for this (I hadn't fully planned everything out yet). You will likely want to create one for any refactoring that needs done.

Member

rwinch commented Mar 10, 2016

@jkubrynski Thanks! I have yet to create any Spring Security issues for this (I hadn't fully planned everything out yet). You will likely want to create one for any refactoring that needs done.

@dreis2211

This comment has been minimized.

Show comment
Hide comment
@dreis2211

dreis2211 commented Mar 11, 2016

+1

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 9, 2016

Spring Security Concurrent Session Integration #65
 add SpringSessionBackedSessionRegistry

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 9, 2016

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 11, 2016

Spring Security Concurrent Session Integration #65
support marking SessionInformations as expired before deleting the Session

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 11, 2016

Spring Security Concurrent Session Integration #65
support marking SessionInformations as expired before deleting the Session

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 12, 2016

Spring Security Concurrent Session Integration #65
 add SpringSessionBackedSessionRegistry

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 12, 2016

jkuipers added a commit to jkuipers/spring-session that referenced this issue Apr 12, 2016

Spring Security Concurrent Session Integration #65
support marking SessionInformations as expired before deleting the Session

rwinch added a commit that referenced this issue Sep 2, 2016

Spring security session registry (#473)
*  Spring Security Concurrent Session Integration #65

 add SpringSessionBackedSessionRegistry

*  Spring Security Concurrent Session Integration #65

 add documentation

*  Spring Security Concurrent Session Integration #65

support marking SessionInformations as expired before deleting the Session

@rwinch rwinch added this to the 1.3.0 M1 milestone Nov 22, 2016

@rwinch rwinch self-assigned this Nov 22, 2016

@rwinch rwinch added the http label Nov 22, 2016

@rwinch

This comment has been minimized.

Show comment
Hide comment
@rwinch

rwinch Nov 23, 2016

Member

Fixed via #473

Member

rwinch commented Nov 23, 2016

Fixed via #473

@rwinch rwinch closed this Nov 23, 2016

@savinov

This comment has been minimized.

Show comment
Hide comment
@savinov

savinov Jun 29, 2017

Usefull feature, good job!

savinov commented Jun 29, 2017

Usefull feature, good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment