Skip to content
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

SameSite=None cookie attribute is not supported #89

Open
atzoum opened this issue Nov 2, 2021 · 9 comments
Open

SameSite=None cookie attribute is not supported #89

atzoum opened this issue Nov 2, 2021 · 9 comments

Comments

@atzoum
Copy link

atzoum commented Nov 2, 2021

Since undertow version 2.1.0.Final (feature UNDERTOW-1600), Undertow supports SameSite=None attributes in cookies, through the SameSiteCookieHandler.
Unfortunately, it seems that quarkus-http has been left behind with undertow's latest developments.

Are there any plans to catch up with undertow's latest versions, or otherwise is there any other way to enable the SameSite=None cookie attribute with Quarkus when using Undertow?

@stuartwdouglas
Copy link
Member

As quarkus-http is based on vert.x you can use the vert.x routing context to set same site cookies, or use Quarkus's same site config mechanism: http://quarkus.io/guides/http-reference#same-site-cookie

Basically it allows you to set the attribute based on a cookie name.

@atzoum
Copy link
Author

atzoum commented Nov 3, 2021

As quarkus-http is based on vert.x you can use the vert.x routing context to set same site cookies, or use Quarkus's same site config mechanism: http://quarkus.io/guides/http-reference#same-site-cookie

Basically it allows you to set the attribute based on a cookie name.

Thank you for the tip, however the relevant vertx extension doesn't handle cookies set by undertow properly, since ResponseWrapper#addCookie is never called from Undertow.

Unfortunately, due to the above, Quarkus' same-site-cookie configuration has no effect to undertow's JSESSIONID session cookie.

After having a closer look into the two HttpServerResponse implementations, maybe a headersEndHandler would be more appropriate to intercept and handle the SameSite attribute of all cookies, which I presume would cover cookies set by undertow as well.

Here is a sample POC implementation, I believe it would make sense to adapt Quarkus' same-site-cookie handling implementation accordingly

public class SameSiteCookieFilter implements Filter {
    private boolean secureCookie;
    private boolean sameSiteNone;
    
    /**
     * {@inheritDoc}
     */
    @Override
    public void init(FilterConfig filterConfig) throws ServletException {
        secureCookie = ConfigProvider.getConfig().getOptionalValue("http.cookie.secure", Boolean.class).orElse(true);
        sameSiteNone = ConfigProvider.getConfig().getOptionalValue("http.cookie.samesite.none", Boolean.class).orElse(true);
    }
    /**
     * {@inheritDoc}
     */
    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
        if (sameSiteNone && secureCookie) {
            RoutingContext routingContext = CDI.current().select(CurrentVertxRequest.class).get().getCurrent();
            HttpServerResponse httpServerResponse = routingContext.response();
            HttpServerRequest httpServerRequest = routingContext.request();
            if (SameSiteNoneIncompatibleClientChecker.shouldSendSameSiteNone(httpServerRequest.getHeader(HttpHeaders.USER_AGENT))) {
                CDI.current().select(CurrentVertxRequest.class).get().getCurrent().addHeadersEndHandler(v -> {
                    Map<String, ServerCookie> cookies = extractCookies(httpServerResponse.headers().getAll(HttpHeaders.SET_COOKIE));
                    if (cookies.size() > 0) {
                        httpServerResponse.headers().remove(HttpHeaders.SET_COOKIE);
                        cookies.values().stream().forEach(cookie -> {
                            cookie.setSameSite(CookieSameSite.NONE);
                            cookie.setSecure(true);
                            httpServerResponse.addCookie(cookie);
                        });
                    }
                });
            }
        }
        chain.doFilter(request, response);
    }
    
    private static Map<String, ServerCookie> extractCookies(List<String> cookieHeaders) {
        if (cookieHeaders != null) {
          List<io.netty.handler.codec.http.cookie.Cookie> nettyCookies = cookieHeaders.stream().map(ClientCookieDecoder.STRICT::decode).collect(Collectors.toList());
          Map<String, ServerCookie> cookies = new HashMap<>(nettyCookies.size());
          for (io.netty.handler.codec.http.cookie.Cookie cookie : nettyCookies) {
            ServerCookie ourCookie = new CookieImpl(cookie);
            ourCookie.setChanged(true);
            cookies.put(ourCookie.getName(), ourCookie);
          }
          return cookies;
        } else {
          return new HashMap<>(4);
        }
      }
}

Is there any other way to intercept the response and alter the JSESSIONID cookie?

@rewweRrr
Copy link

Hi! We have the same problem with sameSite cookie. I've prepared simple example:

application.yml

quarkus:
    http:
        same-site-cookie:
            testCookie:
                value: strict

TestController

package com.netcracker.cloud.bss.portal.web.api;

import com.netcracker.cloud.bss.portal.constant.UrlConstants;

import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.NewCookie;
import javax.ws.rs.core.Response;

@Path("")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public class TestController {

    @GET
    @Path("/test")
    public Response getTest() {
        NewCookie cookie = new NewCookie("testCookie", "value");
        return Response.ok("Test").cookie(cookie).build();
    }
}

As a result sameSite Strict doesn't appear

@aelfric
Copy link

aelfric commented Feb 8, 2022

Is there any known workaround for this? In my case, the application I am using needs to be loaded in a iframe from another site and can no longer track sessions in the iframe because of the browser's cookie policy.

@stuartwdouglas
Copy link
Member

I have opened #97 which backports the upstream Undertow samesite support. Once it is in and released we can integrate it with the existing vert.x config.

@aelfric
Copy link

aelfric commented Aug 27, 2022

It looks like #97 was merged. What are the steps to be able to take advantage of it?

@alexlitovsky
Copy link

Is there any known workaround for this? In my case, the application I am using needs to be loaded in a iframe from another site and can no longer track sessions in the iframe because of the browser's cookie policy.

@aelfric A workaround is to handle this in META-INF/undertow-handlers.conf
samesite-cookie(mode=None)

@aelfric
Copy link

aelfric commented Jan 21, 2023

Sorry just saw this update, but that undertow-handlers workaround doesn't seem to do it for me. I just bumped the quarkus version to 2.15.3 but still not able to get the cookies working as expected.

@md-consulting
Copy link

Dear community,

Any updates here? Quarkus 3.6 will be out soon and still there seems to be this bug.

The documentation from https://quarkus.io/guides/http-reference Point 14 says that : One can easily add a SameSite cookie property to any of the cookies set by a Quarkus endpoint by listing a cookie name and a SameSite attribute, for example:
quarkus.http.same-site-cookie.session.value=Strict

This does not seem to work though, with or without the Vert.x extension.

With the web.xml file, you can set secure and path, but not SameSite. I also tried the to set in the undertow conf file : path(/)->samesite(‘None’), without any success either.

If someone knows any other possibility, please feel free to share it.

Any help massively appreciated!

Thank you and best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants