Page tree
Skip to end of metadata
Go to start of metadata




Safari

Webkit based browsers on Mac (safari) and iOS (safair, chome, firefox etc)  are currently affected by a bug that treats SameSite=None or SameSite=nonesense cookies as SameSite=Strict (https://bugs.webkit.org/show_bug.cgi?id=198181). We believe the fix for this will only take effect from MacOS 10.15 and iOS 13. Consequently, any attempt to maintain the current functional behaviour of cookies by setting SameSite=None on unpatched versions of Webkit will break SSO. 

Implementation

Following on from IdP SameSite Testing, here we describe a new Servlet Filter (SameSiteSessionCookieFilter) for appending the same-site cookie flag to specified cookies. The SameSiteSessionCookieFilter wraps the HttpResponse with a SameSiteResponseProxy proxy. The proxy overrides the getWriter, sendError, getOutputStream, and sendRedirect Response methods such that any attempt from a Servlet to commit a response back to the client invokes the 'append same site attribute' logic over the current set of Set-Cookie headers.

Affected cookies are specified by injecting a set of cookie names into an internal map using XML-based spring bean configuration. Each cookie name is related to a key, the key corresponds to the same-site attribute value to set e.g. one-of {none,lax,strict}. Any Set-Cookie header that already contains a same-site cookie flag is not affected, even if configured differently in the filters internal same-site map. Any cookie in a Set-Cookie header that is not contained in the filters internal same-site map is ignored - left as is.

The basic algorithm is:

  1. On writing a response to the client.
  2. Extract all Set-Cookie headers.
  3. For each header.
    1. Parse the Set-Cookie header to retrieve the cookie name
    2. If the cookie name exists in the sameSiteCookies map.
      1. check the cookie header does not already have the same-site flag set.
        1. Append the same-site flag to the cookie with the specified attribute value.
      2. Else copy the header back into the response unmodified.
    3. Else copy the header back into the response unmodified.

The implementation can be found on my personal repository [git@git.shibboleth.net:philsmart/java-support] feature branch [feature/same-site-filter].


Design Considerations

  1. The addCookie method of the HTTPServletResponse may seem like a an obvious integration point, but due to limitations with the current servlet specification and Cookie API, SameSite can not be added through the Cookie object in a way the container would appropriately serialise into the response.
    1. In addition, the servlet container is responsible for adding the HttpSession ID (JSESSIONID) without control given to the filter chain and hence any proxy implementation - It does so directly on the response object. However, importantly, the JSESSIONID Set-Cookie header can later be accessed when a response is committed by a the Servlet e.g. webflow, using the methods the proxy overrides.
  2. Similar functionality could be achieved by developing a DynamicResponseHeaderFiltercallback function, encapsulating the same logic as the filter described here. You would, however, need to make sure the DynamicResponseHeaderFilter was ordered before the CookieBufferingFilterin the web.xml - which it is not currently.
  3. The internal sameSiteCookies Map is a flat map that maps cookie names to SameSite attribute values (Map<String,SameSiteValue>). The API for the Filter sets sameSiteCookies as a Map of SameSite attribute value to a List of cookie names (Map<SameSiteValue,List<String>>. This disconnect occurs for the following reasons.
    1. If the API presented the internal map of String to SameSiteValue, the map, particularly through XML configuration, may be created with multiple entries for a given cookie (for example SameSite Strict and None for the JSESISONID), the last entry of which overwriting any previous entries. The deployer may not be fully aware of this, as no error would be thrown, and may misread the configuration and hence expect different behaviour. Consequently, the API allows the configuration of multiple same-site values for a given cookie, but throws an error if duplicate cookie names are found when constructing the internal map. 
      1. It is then easier to work internally with the flattened cookie map.
    2. If ever required, it would be easier to specify the list of cookies per SameSiteValue in the idp.properties e.g.idp.cookie.sameSite.none=JSESSIONID,shib_idp_session. 


Configuration

The Filter is wrapped inside a spring DelegatingFilterProxy in order to simplify argument injection by using spring bean configuration. The filter must be specified in the web.xml e.g.:

Filter Definition
<filter>
        <filter-name>SameSiteSessionCookieFilter</filter-name>
        <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class>
        <init-param>
            <param-name>targetBeanName</param-name>
            <param-value>shibboleth.SameSiteCookieHeaderFilter</param-value>
        </init-param>      
</filter>

The filter-mapping must be registered before the CookieBufferingFilter otherwise the Set-Cookie headers are not dumped into the real response when the SameSiteSessionCookieFilter filter is run. e.g.

Filter Mapping
<filter-mapping>
   <filter-name>SameSiteSessionCookieFilter</filter-name>
   <url-pattern>/*</url-pattern>
</filter-mapping>
<filter-mapping>
   <filter-name>CookieBufferingFilter</filter-name>
   ...
     

The spring SameSiteCookieHeaderFilter bean is then defined in the global-system.xml configuration:

Filter Spring Bean
<bean id="shibboleth.SameSiteCookieHeaderFilter"
     	class="net.shibboleth.utilities.java.support.net.SameSiteCookieHeaderFilter">
    	<property name="sameSiteCookies">
    		<map>
    			<entry key="None" value="JSESSIONID,shib_idp_session,
    			%{idp.storage.clientSessionStorageName:shib_idp_session_ss},
    			%{idp.storage.clientPersistentStorageName:shib_idp_persistent_ss}"/>
    		</map>
    	</property> 	
    </bean>

The bean configuration above configures the JSESSIONID, shib_idp_session, shib_idp_session_ss and shib_idp_persistent_ss cookies with the SameSite=None cookie flag.


Testing


The test class SameSiteCookieHeaderFilterTest tests the functionality of the SameSiteCookieHeaderFilter filter. However, due to a bug I found with the MockHttpServletResponse [1], the TestNG tests will fail when using spring-test versions 5.1 up to but not including version 5.1.10.


Appendix A - Filter Code

Note, the git repository listed in the implementation section will potentially contain a newer version of this static code listing.


SameSiteCookieHeaderFilter.java
/*
 * Licensed to the University Corporation for Advanced Internet Development,
 * Inc. (UCAID) under one or more contributor license agreements.  See the
 * NOTICE file distributed with this work for additional information regarding
 * copyright ownership. The UCAID licenses this file to You under the Apache
 * License, Version 2.0 (the "License"); you may not use this file except in
 * compliance with the License.  You may obtain a copy of the License at
 *
 *    http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package net.shibboleth.utilities.java.support.net;

import java.io.IOException;
import java.io.PrintWriter;
import java.net.HttpCookie;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.net.HttpHeaders;

import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;


/**
 * Implementation of an HTTP servlet {@link Filter} which conditionally adds the SameSite attribute to cookies.
 * 
 * <p>Affected cookies are configured and placed into a Map of cookie name to same-site attribute value.</p>
 * 
 * <p>Cookies with an existing same-site cookie flag, or those not present in the {@code sameSiteCookies} Map, are 
 * left unaltered - copied back into the response without modification. 
 * 
 * <p>A single cookie can only have at most one same-site value set. Attempts in the configuration to 
 * give more than one same-site value to a cookie are caught during argument injection and throw an
 * {@link IllegalArgumentException}.</p>
 * 
 */
public class SameSiteCookieHeaderFilter implements Filter {
    
    /** Class logger. */
    @Nonnull private final Logger log = LoggerFactory.getLogger(SameSiteCookieHeaderFilter.class);
    
    /** The name of the same-site cookie attribute.*/
    private static final String SAMESITE_ATTRIBITE_NAME="SameSite";
      
    /** The allowed same-site cookie attribute values.*/
    public enum SameSiteValue{       
        
        /**
         * Send the cookie for 'same-site' requests only.
         */
        Strict("Strict"),
        /**
         * Send the cookie for 'same-site' requests along with 'cross-site' top 
         * level navigations using safe HTTP methods (GET, HEAD, OPTIONS, and TRACE).
         */
        Lax("Lax"),        
        /**
         * Send the cookie for 'same-site' and 'cross-site' requests.
         */
        None("None");
        
        /** The same-site attribute value.*/
        @Nonnull @NotEmpty private String value;
        
        /**
         * Constructor.
         *
         * @param attrValue the same-site attribute value.
         */
        private SameSiteValue(@Nonnull @NotEmpty final String attrValue) {
            value = Constraint.isNotEmpty(attrValue, "the same-site attribute value can not be empty");
         }

        /**
         * Get the same-site attribute value.
         * 
         * @return Returns the value.
         */
        public String getValue() {
            return value;
        }
        
    }
    
    /** Map of cookie name to same-site attribute value.*/
    @Nonnull @NonnullElements private Map<String,SameSiteValue> sameSiteCookies;
   
    
    /** Constructor.*/
    public SameSiteCookieHeaderFilter() {
        sameSiteCookies = Collections.emptyMap();
    }
    
    /**
     * Set the names of cookies to add the same-site attribute to. 
     * 
     * <p>The argument map is flattened to remove the nested collection. The argument map allows duplicate 
     * cookie names to appear in order to detect configuration errors which would otherwise not be found during 
     * argument injection e.g. trying to set a session identifier cookie as both SameSite=Strict and SameSite=None. 
     * Instead, duplicates are detected here, throwing a terminating {@link IllegalArgumentException} if found.</p>
     * 
     * @param map the map of same-site attribute values to cookie names.
     */
    public void setSameSiteCookies(@Nullable @NonnullElements final Map<SameSiteValue,List<String>> map) {
        if (map != null) {
            sameSiteCookies = new HashMap<>(4);
            for (final Map.Entry<SameSiteValue,List<String>> entry : map.entrySet()) {
                
                for (final String cookieName : entry.getValue()) {
                   if (sameSiteCookies.get(cookieName)!=null) {
                       log.error("Duplicate cookie name [{}] found in SameSite cookie map, "
                               + "please check configuration.",cookieName);
                       throw new IllegalArgumentException("Duplicate cookie name found in SameSite cookie map");
                   }  
                   final String trimmedName = StringSupport.trimOrNull(cookieName);
                    if (trimmedName!=null) {
                        sameSiteCookies.put(cookieName, entry.getKey());
                    }
                }                
            }
        } else {
            sameSiteCookies = Collections.emptyMap();
        }
        
    }
    
    /** {@inheritDoc} */
    public void init(@Nonnull final FilterConfig filterConfig) throws ServletException {
    }
    
    /** {@inheritDoc} */
    public void destroy() {
    }
    
    /** {@inheritDoc} */
    public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain)
            throws IOException, ServletException {
        
        if (!(request instanceof HttpServletRequest)) {
            throw new ServletException("Request is not an instance of HttpServletRequest");
        }

        if (!(response instanceof HttpServletResponse)) {
            throw new ServletException("Response is not an instance of HttpServletResponse");
        }
        
        chain.doFilter(request, new SameSiteResponseProxy((HttpServletResponse)response));
        
    }
    
    /**
     * An implementation of the {@link HttpServletResponse} which adds the same-site flag to {@literal Set-Cookie}
     * headers for the set of configured cookies.
     */
    private class SameSiteResponseProxy extends HttpServletResponseWrapper{

        /** The response. */
        @Nonnull private final HttpServletResponse response;
        
        /**
         * Constructor.
         *
         * @param resp the response to delegate to
         */
        public SameSiteResponseProxy(@Nonnull final HttpServletResponse resp) {
            super(resp);
            response = resp;
        }
        
        /** {@inheritDoc} */
        @Override
        public void sendError(final int sc) throws IOException {
            appendSameSite();
            super.sendError(sc);
        }
        
        /** {@inheritDoc} */
        @Override
        public PrintWriter getWriter() throws IOException {
            appendSameSite();
            return super.getWriter();
        }
        
        /** {@inheritDoc} */
        @Override
        public void sendError(final int sc, final String msg) throws IOException {
            appendSameSite();
            super.sendError(sc, msg);
        }
        
        /** {@inheritDoc} */
        @Override
        public void sendRedirect(final String location) throws IOException {
            appendSameSite();
            super.sendRedirect(location);
        }
        
        /** {@inheritDoc} */
        @Override
        public ServletOutputStream getOutputStream() throws IOException {
            appendSameSite();
            return super.getOutputStream();
        }
        
        /** 
         * Add the SameSite attribute to those cookies configured in the {@code sameSiteCookies} map iff 
         * they do not already contain the same-site flag. All other cookies are copied over to the response
         * without modification.
         * */
        private void appendSameSite() {
            
            final Collection<String> cookieheaders = response.getHeaders(HttpHeaders.SET_COOKIE);
            
            boolean firstHeader = true;
            for (final String cookieHeader : cookieheaders) {
                
                if (StringSupport.trimOrNull(cookieHeader)==null) {
                    continue;
                }
                
                List<HttpCookie> parsedCookies = null;
                try {
                    //this parser only parses name and value, we only need the name.
                    parsedCookies = HttpCookie.parse(cookieHeader);
                } catch(final IllegalArgumentException e) {
                    //should not get here
                   log.trace("Cookie header [{}] violates the cookie specification and will be ignored",cookieHeader);
                }
                
                if (parsedCookies==null || parsedCookies.size()!=1) {
                    //should be one cookie
                    continue;
                }
                
                final SameSiteValue sameSiteValue = sameSiteCookies.get(parsedCookies.get(0).getName());
                if (sameSiteValue!=null) {
                    appendSameSiteAttribute(cookieHeader, sameSiteValue.getValue(), firstHeader);
                } else {
                    //copy it over unaltered
                    if (firstHeader) {                      
                        response.setHeader(HttpHeaders.SET_COOKIE,cookieHeader);                        
                    } else {
                        response.addHeader(HttpHeaders.SET_COOKIE, cookieHeader);
                    }
                }
                firstHeader=false;
                
            }
        }
        
        /**
         * Append the SameSite cookie attribute with the specified samesite-value to the {@code cookieHeader} 
         * iff it does not already have one set.           
         * 
         * @param cookieHeader the cookie header value.
         * @param sameSiteValue the SameSite attribute value e.g. None, Lax, or Strict.
         * @param first is this the first Set-Cookie header.
         */
        private void appendSameSiteAttribute(@Nonnull final String cookieHeader, @Nonnull final String sameSiteValue,
                @Nonnull final boolean first) {
            
            String sameSiteSetCookieValue =  cookieHeader;
            
            //only add if does not already exist, else leave
            if (!cookieHeader.contains(SAMESITE_ATTRIBITE_NAME)) {
                sameSiteSetCookieValue = String.format("%s; %s", cookieHeader, 
                        SAMESITE_ATTRIBITE_NAME+"="+sameSiteValue);                
            } 
            
            if (first) {                
                response.setHeader(HttpHeaders.SET_COOKIE,sameSiteSetCookieValue);
                return;
            }            
            response.addHeader(HttpHeaders.SET_COOKIE, sameSiteSetCookieValue);        
           
            
        } 
        
    }
    
    

}

SameSiteCookieHeaderFilterTest.java
/*
 * Licensed to the University Corporation for Advanced Internet Development,
 * Inc. (UCAID) under one or more contributor license agreements.  See the
 * NOTICE file distributed with this work for additional information regarding
 * copyright ownership. The UCAID licenses this file to You under the Apache
 * License, Version 2.0 (the "License"); you may not use this file except in
 * compliance with the License.  You may obtain a copy of the License at
 *
 *    http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package net.shibboleth.utilities.java.support.net;

import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Writer;
import java.net.HttpCookie;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.servlet.Filter;
import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.mock.http.server.reactive.MockServerHttpResponse;
import org.springframework.mock.web.MockCookie;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockFilterConfig;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.test.util.ReflectionTestUtils;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import com.google.common.net.HttpHeaders;

import net.shibboleth.utilities.java.support.net.SameSiteCookieHeaderFilter.SameSiteValue;

/**
 * Tests for {@link SameSiteCookieHeaderFilter}.
 */
public class SameSiteCookieHeaderFilterTest {

    private MockHttpServletRequest request;

    private MockHttpServletResponse response;

    @BeforeMethod public void setUp() {
        MockHttpServletRequest mockRequest = new MockHttpServletRequest();
        mockRequest.setMethod("POST");
        mockRequest.setRequestURI("/foo");
        request = mockRequest;

        MockHttpServletResponse mockResponse = new MockHttpServletResponse();
        mockResponse.addHeader(HttpHeaders.SET_COOKIE,
                "JSESSIONID=jyohu8ttc3dp1g3yqe8g8ff7y;Path=/idp;Secure;HttpOnly");
        mockResponse.addHeader(HttpHeaders.SET_COOKIE,
                "shib_idp_session_ss=AAdzZWNyZXQyzL1Rzi9ROe3%2BGk%2B6%2B;Path=/idp;HttpOnly");
        mockResponse.addHeader(HttpHeaders.SET_COOKIE,
                "shib_idp_session=8ee460bc0b3695c477b2b5f3e192ddf7297baa7ee01bd2bcf24695f8c21cb3a2;Path=/idp;HttpOnly");
        //add a cookie with existing SameSite value - should ignore and copy over.
        mockResponse.addHeader(HttpHeaders.SET_COOKIE,
                "existing_same_site=already-same-site;Path=/idp;HttpOnly;SameSite=None");
        //ignore this, copy it over as is.
        mockResponse.addHeader(HttpHeaders.SET_COOKIE,
                "ignore_copy_over=copy-over;Path=/idp;HttpOnly");

        response = mockResponse;
    }

    @AfterMethod public void tearDown() {
        HttpServletRequestResponseContext.clearCurrent();
    }
    
    /** Test a null init value, which should not trigger an exception.*/
    @Test public void testNullInitValues() {
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        filter.setSameSiteCookies(null);
    }
    
    /** Test an empty cookie name is not added to the internal map.*/
    @Test public void testEmptyCookieNameInitValue() {
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {""});
        cookies.put(SameSiteValue.None, noneCookies);
        filter.setSameSiteCookies(cookies);
        
        testSameSiteMapSize("sameSiteCookies", 0, filter);
    }
   
    /** Test the correct number of cookies are added to the internal filter cookie map.*/
    @Test public void testInitValues() {
        
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
        List<String> laxCookies = Arrays.asList(new String[] {"another-cookie-lax"});
        List<String> strictCookies = Arrays.asList(new String[] {"another-cookie-strict"});
        cookies.put(SameSiteValue.None, noneCookies);
        cookies.put(SameSiteValue.Lax, laxCookies);
        cookies.put(SameSiteValue.Strict, strictCookies);
        filter.setSameSiteCookies(cookies);
        
        testSameSiteMapSize("sameSiteCookies", 6, filter);
    }
    
    /** Test failure on duplicated cookie names*/
    @Test(expectedExceptions=IllegalArgumentException.class) public void testDuplicateInitValues() {
        
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
        List<String> laxCookies = Arrays.asList(new String[] {"JSESSIONID"});
        cookies.put(SameSiteValue.None, noneCookies);
        cookies.put(SameSiteValue.Lax, laxCookies);
        filter.setSameSiteCookies(cookies);

    }
    
    /** Test empty SameSite cookie map, which should not trigger an exception, and just copy over the
     * existing cookies. */
    @Test public void testEmptySameSiteCookieMap() throws IOException, ServletException {
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        filter.setSameSiteCookies(null);
        
        Servlet redirectServlet = new TestRedirectServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(redirectServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        final Collection<String> headers = response.getHeaders(HttpHeaders.SET_COOKIE); 
        
        
        Assert.assertEquals(headers.size(), 5);
    }
    

    /** Test the samesite filter works correctly with None values when a redirect response is issued. */
    @Test public void testRedirectResponseSameSiteNone() throws IOException, ServletException {

       
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
        cookies.put(SameSiteValue.None, noneCookies);
        filter.setSameSiteCookies(cookies);

        Servlet redirectServlet = new TestRedirectServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(redirectServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        testExpectedHeadersInResponse("None",(MockHttpServletResponse)mockRedirectChain.getResponse(), 
                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
                Arrays.asList(new String[] {"ignore_copy_over"}),5);

    }
    
    /** Test the samesite filter works correctly with Lax values when a redirect response is issued. */
    @Test public void testRedirectResponseSameSiteLax() throws IOException, ServletException {

       
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"});
        cookies.put(SameSiteValue.Lax, noneCookies);
        filter.setSameSiteCookies(cookies);
        
        Servlet redirectServlet = new TestRedirectServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(redirectServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        //as "existing_same_site" is None, ignore it here.
        testExpectedHeadersInResponse("Lax",(MockHttpServletResponse)mockRedirectChain.getResponse(), 
                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"}),
                Arrays.asList(new String[] {"ignore_copy_over"}),5);

    }
    
    /** Test the samesite filter works correctly with Strict values when a redirect response is issued. */
    @Test public void testRedirectResponseSameSiteStrict() throws IOException, ServletException {

        
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"});
        cookies.put(SameSiteValue.Strict, noneCookies);
        filter.setSameSiteCookies(cookies);

        Servlet redirectServlet = new TestRedirectServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(redirectServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        //as "existing_same_site" is None, ignore it here.
        testExpectedHeadersInResponse("Strict",(MockHttpServletResponse)mockRedirectChain.getResponse(), 
                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"}),
                Arrays.asList(new String[] {"ignore_copy_over"}),5);

    }
    
    

    /** Test the samesite filter works correctly when an output stream is written to and flushed. */
    @Test public void testGetOutputStreamResponse() throws IOException, ServletException {

        
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
        cookies.put(SameSiteValue.None, noneCookies);
        filter.setSameSiteCookies(cookies);

        Servlet outputStreamServlet = new TestOutputStreamServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(outputStreamServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        testExpectedHeadersInResponse("None",(MockHttpServletResponse)mockRedirectChain.getResponse(), 
                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
                Arrays.asList(new String[] {"ignore_copy_over"}),5);

    }
    
    /** Test the samesite filter works correctly when the response print writer is written to and closed.*/
    @Test public void testPrintWriterResponse() throws IOException, ServletException {

        
        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
        Map<SameSiteValue,List<String>> cookies = new HashMap<>();
        List<String> noneCookies = Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
        cookies.put(SameSiteValue.None, noneCookies);
        filter.setSameSiteCookies(cookies);

        Servlet printWriterServlet = new TestPrintWriterServlet();
        MockFilterChain mockRedirectChain = new MockFilterChain(printWriterServlet, filter);

        mockRedirectChain.doFilter(request, response);

        Assert.assertTrue(mockRedirectChain.getResponse() instanceof MockHttpServletResponse);
        
        testExpectedHeadersInResponse("None",(MockHttpServletResponse)mockRedirectChain.getResponse(), 
                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
                Arrays.asList(new String[] {"ignore_copy_over"}),5);

    }
    
    /**
     * Get the field from the filter (even if private), check the field is of type {@link Set}, and compare
     * the size of the set to the expected size.
     * 
     * @param fieldName the name of the field on the object of type {@link Map}.
     * @param expectedSize the expected size of the map.
     * @param filter the filter with the field to get.
     */
    private void testSameSiteMapSize(String fieldName, int expectedSize, Filter filter) {
        
        Object sameSiteSet = ReflectionTestUtils.getField(filter, fieldName);
        Assert.assertNotNull(sameSiteSet);
        Assert.assertTrue(sameSiteSet instanceof Map);
        Assert.assertEquals(((Map)sameSiteSet).size(),expectedSize);
    }
    
    /**
     * Test the Set-Cookie headers in the response contain the {@literal SameSite=<sameSiteValue>} attribute if they are named
     * in the {@code cookiesWithSamesite} list, and do not if named in the {@code cookiesWithoutSameSite} list.
     * <p>
     * Also checks the number of Set-Cookie headers matches {@code numberOfHeaders}. This makes sure the filter
     * is not adding or removing headers during operation - it should only ever append the SameSite attribute
     * to existing cookies.
     * </p>
     * 
     * @param sameSiteValue the value of samesite to check for.
     * @param response the http servlet response.
     * @param cookiesWithSamesite the list of cookies that should have the {@literal SameSite=None} attribute set.
     * @param cookiesWithoutSameSite the list of cookies that should not have the {@literal SameSite} attribute set.
     * @param numberOfHeaders the number of Set-Cookie headers expected in the response.
     */
    private void testExpectedHeadersInResponse(final String sameSiteValue, final MockHttpServletResponse response, 
            final List<String> cookiesWithSamesite, final List<String> cookiesWithoutSameSite, final int numberOfHeaders) {
        
        final Collection<String> headers = response.getHeaders(HttpHeaders.SET_COOKIE); 
        
        Assert.assertEquals(headers.size(), numberOfHeaders);       
        
        for (String header : headers) {
            
            List<HttpCookie> cookies = HttpCookie.parse(header);
            Assert.assertNotNull(cookies);
            Assert.assertTrue(cookies.size()==1);
            Cookie cookie = response.getCookie(cookies.get(0).getName());
            Assert.assertNotNull(cookie);
            Assert.assertTrue(cookie instanceof MockCookie);
            MockCookie mockCookie = (MockCookie)cookie;
                      
            if (cookiesWithSamesite.contains(mockCookie.getName())) {
                Assert.assertNotNull(mockCookie.getSameSite());
                Assert.assertEquals(mockCookie.getSameSite(),sameSiteValue);                           
                      
            }
            else if (cookiesWithoutSameSite.contains(mockCookie.getName())) {
                Assert.assertNull(mockCookie.getSameSite());
                           
            }
        }
        
        


    }

    /**
     * Servlet that initiates a redirect on the response.
     */
    public class TestRedirectServlet implements Servlet {

        /** {@inheritDoc} */
        public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
            Assert.assertNotNull(req, "HttpServletRequest was null");
            Assert.assertNotNull(res, "HttpServletResponse was null");
            ((HttpServletResponse) res).sendRedirect("/redirect");
        }

        /** {@inheritDoc} */
        public void init(ServletConfig config) throws ServletException {
        }

        /** {@inheritDoc} */
        public ServletConfig getServletConfig() {
            return null;
        }

        /** {@inheritDoc} */
        public String getServletInfo() {
            return null;
        }

        /** {@inheritDoc} */
        public void destroy() {
        }

    }

    /**
     * Servlet that opens an output stream on the response.
     */
    public class TestOutputStreamServlet implements Servlet {

        /** {@inheritDoc} */
        public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
            Assert.assertNotNull(req, "HttpServletRequest was null");
            Assert.assertNotNull(res, "HttpServletResponse was null");

            // write nothing to the output stream.
            final Writer out = new OutputStreamWriter(((HttpServletResponse) res).getOutputStream(), "UTF-8");
            out.flush();

        }

        /** {@inheritDoc} */
        public void init(ServletConfig config) throws ServletException {
        }

        /** {@inheritDoc} */
        public ServletConfig getServletConfig() {
            return null;
        }

        /** {@inheritDoc} */
        public String getServletInfo() {
            return null;
        }

        /** {@inheritDoc} */
        public void destroy() {
        }

    }
    
    /**
     * Servlet that opens a print writer on the response.
     */
    public class TestPrintWriterServlet implements Servlet {

        /** {@inheritDoc} */
        public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
            Assert.assertNotNull(req, "HttpServletRequest was null");
            Assert.assertNotNull(res, "HttpServletResponse was null");

            // write nothing to the print writer.
            PrintWriter writer = ((HttpServletResponse) res).getWriter();
            writer.flush();

        }

        /** {@inheritDoc} */
        public void init(ServletConfig config) throws ServletException {
        }

        /** {@inheritDoc} */
        public ServletConfig getServletConfig() {
            return null;
        }

        /** {@inheritDoc} */
        public String getServletInfo() {
            return null;
        }

        /** {@inheritDoc} */
        public void destroy() {
        }

    }

}


References

[1] https://github.com/spring-projects/spring-framework/issues/23512




  • No labels