Cookie http-only setting has no effect when using Spring Session #15163
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| public DefaultCookieSerializer cookieSerializer() { | ||
| Cookie cookie = this.serverProperties.getServlet().getSession().getCookie(); | ||
| DefaultCookieSerializer cookieSerializer = new DefaultCookieSerializer(); | ||
| if (cookie.getName() != null) { |
There was a problem hiding this comment.
We could probably use the PropertyMapper here:
Something like:
PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull();
map.from(cookie::getName).to(cookie::setName);
...There was a problem hiding this comment.
That's very nice - will get to that shortly.
philwebb
added
type: bug
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean({ CookieSerializer.class, | ||
| HeaderHttpSessionIdResolver.class }) |
There was a problem hiding this comment.
After taking a second look I think this would be better off being conditional on missing CookieSerializer and conditional on CookieHttpSessionIdResolver, because that's the component using the cookie serializer we're configuring here. Otherwise this would switch off when HeaderHttpSessionIdResolver bean is present, but not when users provide their custom HttpSessionIdResolver implementation.
Thoughts?
There was a problem hiding this comment.
After giving this some more thought, I've added a commit with custom condition that kicks in if either of the following is true:
- no
HttpSessionIdResolverandCookieSerializerbeans are registered - this is the default state with Spring Session CookieHttpSessionIdResolverbean is registered butCookieSerializeris not - this is when users configure a customCookieHttpSessionIdResolverbut they don't register theCookieSerializer
This covers the case (a quite valid one) when users provide their custom HttpSessionIdResolver implementation. That shouldn't result in CookieSerializer being configured. There's a test in second commit that covers this scenario.
IMO the options are either to have this composite condition approach, or abandon the case when users provide their own CookieHttpSessionIdResolver but not CookieSerializer - one might argue that since CookieHttpSessionIdResolver itself is final and CookieSerializer is the only thing can be configure on it that it's an edge case.
Do you have any thoughts on this @wilkinsona?
There was a problem hiding this comment.
Thanks, @vpavic. The scenario that you've described is the one that I needed to refresh my memory about. I like the custom condition approach that you've implemented. This looks ready to merge to me. Thanks for the PR.
wilkinsona
changed the title
* gh-15163: Polish "Auto-configure Spring Session's cookie serializer" Auto-configure Spring Session's cookie serializer
|
Thanks again, @vpavic. I've merged the changes in 2.0.x and forwards into master. |
This PR enhances the existing Spring Session auto-configuration with
DefaultCookieSerializerconfiguration. ThisDefaultCookieSerializeris configurable usingServerPropertiesi.e.server.servlet.session.cookie.*configuration properties.By doing this, users are able to fully configure
DefaultCookieSerializeroffserver.servlet.session.cookie.*which removes the limitation of Spring Session's native configuration that is unable to reliably sethttpOnlyandsecureattributes.This resolves #15139 - note that even though that issue is assigned to
2.0.xI've targeted this atmasterbranch as it feels like a big change to have so late in the 2.0 lifecycle. The changes here take basically the same approach as in #10440.