Skip to content

Allow dynamic Redis namespace configuration #1301

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

Closed
danielfariati opened this issue Dec 26, 2018 · 4 comments
Closed

Allow dynamic Redis namespace configuration #1301

danielfariati opened this issue Dec 26, 2018 · 4 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@danielfariati
Copy link

danielfariati commented Dec 26, 2018

Allow to dynamically define whats the current namespace for Redis.

The scenario is that I currently have a multi-tenant service (subdomain based) that will consult the Session Data within Redis, so I would like to define the tenant to be part of the namespace for the current request.

There is an option to customize the namespace via @EnableRedisHttpSession(redisNamespace = "foo") , but it is not dynamic, so it wouldn't change based on the current tenant.

Calling the setRedisKeyNamespace for the RedisOperationsSessionRepository would also not work well because, as far as I know, it is a singleton.

Also, overriding the RedisOperationsSessionRepository is currently a pain because there are some private methods that are using the this.namespace directly.

@danielfariati
Copy link
Author

My main concern is that two different tenants end up with the same Session ID, and so the wrong session, for the wrong tenant is passed up.
I pass up the Session ID to other services with HttpSessionIdResolver.xAuthToken().
I don't know how the Session ID is generated, so I want to define the tenant as a part of the namespace just to be sure, but maybe this isn't even a real problem.

@vpavic vpavic self-assigned this Dec 26, 2018
@vpavic
Copy link
Contributor

vpavic commented Jan 20, 2019

@danielfariati Default session id generation strategy is UUID based so it shouldn't be an issue that they all share the same namespace. Having that in mind, dynamic resolution of namespace would be an overhead that doesn't actually address any apparent problem. Can you confirm that your concerns with multi tenant configuration are purely around conflicting session ids?

Also note that we have #11 still open to consider providing a custom session id generation strategy.

@vpavic vpavic added the status: waiting-for-feedback We need additional information before we can continue label Jan 20, 2019
@danielfariati
Copy link
Author

Yes, that was my only concern.
Thanks for the help!

@vpavic vpavic added for: stack-overflow A question that's better suited to stackoverflow.com and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 21, 2019
@vpavic
Copy link
Contributor

vpavic commented Jan 21, 2019

Thanks for following up @danielfariati!

@vpavic vpavic added status: declined A suggestion or change that we don't feel we should currently apply and removed for: stack-overflow A question that's better suited to stackoverflow.com labels Jan 21, 2019
vpavic added a commit to vpavic/spring-session that referenced this issue Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants