Skip to content

CookieSameSiteSupplier influences session cookie #39766

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
cbrachem opened this issue Feb 27, 2024 · 4 comments
Closed

CookieSameSiteSupplier influences session cookie #39766

cbrachem opened this issue Feb 27, 2024 · 4 comments
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@cbrachem
Copy link

cbrachem commented Feb 27, 2024

When upgrading an application from Spring Boot 2.7 to 3.2, we noticed a change in behavior for the session cookie. In 3.2, it uses the SameSite attribute provided by a CookieSameSiteSupplier bean. This is not the case in 2.7, where only setting the server.servlet.session.cookie.same-site config property influences the session cookie. I think the change was introduced with the upgrade to Jetty 10. Now, creating the session cookie goes through the SuppliedSameSiteCookieHandlerWrapper.
Mitigating the effects of this change is trivial (just adapt the CookieSameSiteSupplier bean to exclude the session cookie by name), but I would like to have the documentation match the actual behavior. At the time of writing this, it says:

If you want to change the SameSite attribute on other cookies added to your HttpServletResponse, you can use a CookieSameSiteSupplier

which imho implies that it does not affect the session cookie.

Changing the documentation and maybe adding a note for a breaking change in behavior is probably the easiest fix, and even allows for more flexible configuration as seen here, for example. I haven't looked into the alternative yet, i.e. making the CookieSameSiteSupplier not affect the session cookie, but I'm willing to bring a PR either way if this is to be fixed.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 27, 2024
@wilkinsona
Copy link
Member

Thanks, @cbrachem. From your description it sounds like the problem will be Jetty-specific and that neither Tomcat nor Undertow will behave the same way. To help us to verify if that's the case, could you please provide a minimal sample that reproduces the behaviour you have described?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Mar 12, 2024
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Mar 19, 2024
@cbrachem
Copy link
Author

@wilkinsona You are right, the problem is Jetty-specific. I've created a sample application here: https://github.com/cbrachem/samesitecookiedemo

Please note that there is a CookieSameSiteSupplier bean and also server.servlet.session.cookie.same-site is set in the application properties.
The given test fails if jetty is used, but not for undertow or tomcat. (You'd need to comment out the appropriate blocks inside build.gradle.)

The reason is that JettyServletWebServerFactory.addHandlerWrapper adds a SuppliedSameSiteCookieHandlerWrapper if any CookieSameSiteSuppliers are set. If you set a breakpoint inside SuppliedSameSiteCookieHeaders.onAddSetCookieField you can observe that the originally lax session cookie is turned strict.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Mar 20, 2024
@philwebb philwebb added type: regression A regression from a previous release and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Apr 22, 2024
@philwebb philwebb added this to the 3.1.x milestone Apr 22, 2024
@mhalbritter
Copy link
Contributor

This works in 3.1.x, it only starts breaking in 3.2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

5 participants