[java-support] branch master updated: IDP-1476 - Need a filter to append SameSite to cookies

Scott Cantor cantor.2 at osu.edu
Wed Dec 18 19:57:08 EST 2019


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch master
in repository java-support.

View the commit online:
http://git.shibboleth.net/view/?p=java-support.git;a=commit;h=ad8a59d6b9e306d97bd7cf6fb448ce8a076c8ec3

The following commit(s) were added to refs/heads/master by this push:
       new  ad8a59d   IDP-1476 - Need a filter to append SameSite to cookies
ad8a59d is described below

commit ad8a59d6b9e306d97bd7cf6fb448ce8a076c8ec3
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Dec 18 19:57:03 2019 -0500

    IDP-1476 - Need a filter to append SameSite to cookies
    
    https://issues.shibboleth.net/jira/browse/IDP-1476
    
    Add explicit Null to enum to enable configuration override.
---
 .../support/net/SameSiteCookieHeaderFilter.java    | 29 +++++++--
 .../net/SameSiteCookieHeaderFilterTest.java        | 74 ++++++++++++++--------
 2 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/src/main/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilter.java b/src/main/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilter.java
index 97e0a2c..2bf2738 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilter.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilter.java
@@ -88,7 +88,11 @@ public class SameSiteCookieHeaderFilter implements Filter {
         /**
          * Send the cookie for 'same-site' and 'cross-site' requests.
          */
-        None("None");
+        None("None"),
+        /**
+         * Specify nothing.
+         */
+        Null("Null");
         
         /** The same-site attribute value.*/
         @Nonnull @NotEmpty private String value;
@@ -239,12 +243,13 @@ public class SameSiteCookieHeaderFilter implements Filter {
             appendSameSite();
             return super.getOutputStream();
         }
-        
+
+// Checkstyle: CyclomaticComplexity OFF
         /** 
          * 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);
@@ -272,13 +277,22 @@ public class SameSiteCookieHeaderFilter implements Filter {
                 
                 final SameSiteValue sameSiteValue = sameSiteCookies.get(parsedCookies.get(0).getName());
                 if (sameSiteValue != null) {
-                    appendSameSiteAttribute(cookieHeader, sameSiteValue.getValue(), firstHeader);
-                } else if (defaultValue != null) {
+                    if (sameSiteValue != 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);
+                        }
+                    }
+                } else if (defaultValue != null && defaultValue != SameSiteValue.Null) {
                     appendSameSiteAttribute(cookieHeader, defaultValue.getValue(), firstHeader);
                 } else {
                     // Copy it over unaltered.
-                    if (firstHeader) {                      
-                        response.setHeader(HttpHeaders.SET_COOKIE, cookieHeader);                        
+                    if (firstHeader) {
+                        response.setHeader(HttpHeaders.SET_COOKIE, cookieHeader);
                     } else {
                         response.addHeader(HttpHeaders.SET_COOKIE, cookieHeader);
                     }
@@ -287,6 +301,7 @@ public class SameSiteCookieHeaderFilter implements Filter {
                 
             }
         }
+// Checkstyle: CyclomaticComplexity ON
         
         /**
          * Append the SameSite cookie attribute with the specified samesite-value to the {@code cookieHeader} 
diff --git a/src/test/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilterTest.java b/src/test/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilterTest.java
index 2675e66..6e8b7fc 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilterTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/net/SameSiteCookieHeaderFilterTest.java
@@ -22,7 +22,6 @@ 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.Collections;
 import java.util.HashMap;
@@ -90,16 +89,17 @@ public class SameSiteCookieHeaderFilterTest {
     }
     
     /** Test a null init value, which should not trigger an exception.*/
-    @Test(enabled=false) public void testNullInitValues() {
+    @Test public void testNullInitValues() {
         SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
         filter.setSameSiteCookies(null);
+        filter.setDefaultValue(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[] {""});
+        List<String> noneCookies = List.of(new String[] {""});
         cookies.put(SameSiteValue.None, noneCookies);
         filter.setSameSiteCookies(cookies);
         
@@ -110,9 +110,9 @@ public class SameSiteCookieHeaderFilterTest {
     @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"});
+        List<String> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
+        List<String> laxCookies = List.of(new String[] {"another-cookie-lax"});
+        List<String> strictCookies = List.of(new String[] {"another-cookie-strict"});
         cookies.put(SameSiteValue.None, noneCookies);
         cookies.put(SameSiteValue.Lax, laxCookies);
         cookies.put(SameSiteValue.Strict, strictCookies);
@@ -125,8 +125,8 @@ public class SameSiteCookieHeaderFilterTest {
     @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"});
+        List<String> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
+        List<String> laxCookies = List.of(new String[] {"JSESSIONID"});
         cookies.put(SameSiteValue.None, noneCookies);
         cookies.put(SameSiteValue.Lax, laxCookies);
         filter.setSameSiteCookies(cookies);
@@ -150,7 +150,27 @@ public class SameSiteCookieHeaderFilterTest {
         
         Assert.assertEquals(headers.size(), 5);
     }
-    
+
+    /** Test empty SameSite cookie map and Null default, which should not trigger an exception, and just copy over the
+     * existing cookies. */
+    @Test public void testEmptySameSiteCookieMapAndNullDefault() throws IOException, ServletException {
+        
+        SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
+        filter.setSameSiteCookies(null);
+        filter.setDefaultValue(SameSiteValue.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 empty SameSite cookie map, which should not trigger an exception, and should apply
      * a default. */
     @Test public void testEmptySameSiteCookieMapWithDefault() throws IOException, ServletException {
@@ -171,7 +191,7 @@ public class SameSiteCookieHeaderFilterTest {
         Assert.assertEquals(headers.size(), 5);
         testExpectedHeadersInResponse(SameSiteValue.Strict.getValue(),
                 (MockHttpServletResponse)mockRedirectChain.getResponse(), 
-                Arrays.asList(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss", "ignore_copy_over"}),
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss", "ignore_copy_over"}),
                 Collections.emptyList(), 5);
     }
 
@@ -180,7 +200,7 @@ public class SameSiteCookieHeaderFilterTest {
        
         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> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
         cookies.put(SameSiteValue.None, noneCookies);
         filter.setSameSiteCookies(cookies);
 
@@ -192,8 +212,8 @@ public class SameSiteCookieHeaderFilterTest {
         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);
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
+                List.of(new String[] {"ignore_copy_over"}),5);
     }
 
     /** Test the samesite filter works correctly with None values when a redirect response is issued. */
@@ -201,7 +221,7 @@ public class SameSiteCookieHeaderFilterTest {
        
         SameSiteCookieHeaderFilter filter = new SameSiteCookieHeaderFilter();
         Map<SameSiteValue,List<String>> cookies = new HashMap<>();
-        List<String> noneCookies = Arrays.asList(new String[] {"shib_idp_session","shib_idp_session_ss","existing_same_site"});
+        List<String> noneCookies = List.of(new String[] {"shib_idp_session","shib_idp_session_ss","existing_same_site"});
         cookies.put(SameSiteValue.None, noneCookies);
         filter.setSameSiteCookies(cookies);
         filter.setDefaultValue(SameSiteValue.None);
@@ -214,7 +234,7 @@ public class SameSiteCookieHeaderFilterTest {
         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","ignore_copy_over"}),
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site","ignore_copy_over"}),
                 Collections.emptyList(), 5);
     }
     
@@ -223,7 +243,7 @@ public class SameSiteCookieHeaderFilterTest {
        
         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"});
+        List<String> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"});
         cookies.put(SameSiteValue.Lax, noneCookies);
         filter.setSameSiteCookies(cookies);
         
@@ -236,8 +256,8 @@ public class SameSiteCookieHeaderFilterTest {
         
         //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);
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"}),
+                List.of(new String[] {"ignore_copy_over"}),5);
     }
     
     /** Test the samesite filter works correctly with Strict values when a redirect response is issued. */
@@ -245,7 +265,7 @@ public class SameSiteCookieHeaderFilterTest {
         
         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"});
+        List<String> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"});
         cookies.put(SameSiteValue.Strict, noneCookies);
         filter.setSameSiteCookies(cookies);
 
@@ -258,8 +278,8 @@ public class SameSiteCookieHeaderFilterTest {
         
         //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);
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss"}),
+                List.of(new String[] {"ignore_copy_over"}),5);
     }
 
     /** Test the samesite filter works correctly when an output stream is written to and flushed. */
@@ -267,7 +287,7 @@ public class SameSiteCookieHeaderFilterTest {
         
         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> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
         cookies.put(SameSiteValue.None, noneCookies);
         filter.setSameSiteCookies(cookies);
 
@@ -279,8 +299,8 @@ public class SameSiteCookieHeaderFilterTest {
         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);
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
+                List.of(new String[] {"ignore_copy_over"}),5);
     }
     
     /** Test the samesite filter works correctly when the response print writer is written to and closed.*/
@@ -288,7 +308,7 @@ public class SameSiteCookieHeaderFilterTest {
         
         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> noneCookies = List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"});
         cookies.put(SameSiteValue.None, noneCookies);
         filter.setSameSiteCookies(cookies);
 
@@ -300,8 +320,8 @@ public class SameSiteCookieHeaderFilterTest {
         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);
+                List.of(new String[] {"JSESSIONID","shib_idp_session","shib_idp_session_ss","existing_same_site"}),
+                List.of(new String[] {"ignore_copy_over"}),5);
     }
     
     /**

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list