Add support for same-site cookie attribute

[johnkdev]

No description provided.

[markt-asf]

The configuration should be on the CookieProcessor, not the Context. We also need to look at https://scotthelme.co.uk/tough-cookies/ but that isn't a requirement to get this PR applied.

[markt-asf]

Sorry, any new configuration option needs to be added to the docs as well.

[ChristopherSchultz]

Thank you for the proposed patch.

Understanding that changing the definition of a class in the javax.servlet namespace (namely, Cookie) has some challenges, it seems to me that this is the wrong approach. I think the right approach is to allow individual cookies to have the "samesite" setting set individually.

Changing Cookie would allow fewer changes to the core API and the SameSiteCookies class (which should be an enum IMO) would not need to exist.

What are our options when it comes to messing-around with the servlet API classes?

Would it be better to apply a variant of this patch wait for another servlet spec release to "fix" the Cookie API? Or would it be better to provide another (different) container-specific workaround for things in the meantime? As much as we all hate system properties, this might be a good time to use one, since (a) it's intended to be temporary (pending a spec revision) and (b) it will require fewer changes to the internal Tomcat API which will just have to be un-done when the spec revision is published.

[markt-asf]

We have no latitude to change any part of a spec API. Being able to set a default for this is better than not being able to set anything at all. As and when the spec catches up, the per cookie setting can override any application wide default.

[johnkdev]

I've moved the configuration to CookieProcessor and updated the docs.

[markt-asf]

Thanks for the updates. I have a few comments on the updated PR:

• You can reduce the code duplication by using CookieProcessorBase
• SameSiteCookies looks to be a good candidate for an Enum unless I am missing something
• Feel free to add a changelog entry for this

[ChristopherSchultz]

+1 for making SameSiteCookies into an enum. It will also protect against a malicious application causing Tomcat to emit a broken/malicious HTTP response header.

[johnkdev]

I've updated the code and added an entry to the changelog.

[markt-asf]

Thanks for the PR. It has been applied to master. I'll close this PR once I have completed back-porting it.

[markt-asf]

Back-ported to 8.5.x

[senthalan]

@markt-asf Is this ported to 7.0.x as well?

[markt-asf]

No. This was not back-ported to 7.0.x (which has rather different Cookie support).

[simonsteiner1984]

For tomcat 7, best I could do was, which works in chrome and not firefox, is there another way?
filterConfig.getServletContext().getSessionCookieConfig().setComment(";SameSite=None;Secure");

[Chintagious]

@simonsteiner1984 - I needed a solution to set SameSite=None for my JSESSIONID cookie only; so I made filter based off of the answer here:
https://stackoverflow.com/questions/17991090/tomcat-7-sessionid-cookie-disable-http-only-and-secure

You can likely adapt it as needed for your issue.

If you look at the answer, I've kept everything the same except for the updateCookie(), which looks like:

protected void updateCookie(Collection<String> cookiesAfterCreateSession) {
    if(cookiesAfterCreateSession != null && !response.isCommitted()) {
        // search if a cookie JSESSIONID and SameSite not set
        Optional<String> cookieJSessionId = cookiesAfterCreateSession.stream()
                .filter(cookie -> cookie.startsWith("JSESSIONID") && !cookie.contains("SameSite"))
                .findAny();
        if(cookieJSessionId.isPresent()) {
            // remove all Set-Cookie and add the SameSite version of the JSESSIONID Cookie
            response.setHeader("Set-Cookie", cookieJSessionId.get() + ";SameSite=None");

            // re-add all other Cookies
            cookiesAfterCreateSession.stream()
                    .filter(cookie -> !cookie.startsWith("JSESSIONID"))
                    .forEach(cookie -> response.addHeader("Set-Cookie", cookie));
        }
    }
}