Merge pull request #251 from bmhm/SHIRO-789

[SHIRO-789] Add SameSite option to AbstractShiroWebConfiguration.buildCookie

support/spring/src/main/java/org/apache/shiro/spring/web/config/AbstractShiroWebConfiguration.java

@@ -66,6 +66,9 @@ public class AbstractShiroWebConfiguration extends AbstractShiroConfiguration {
     @Value("#{ @environment['shiro.sessionManager.cookie.secure'] ?: false }")
     protected boolean sessionIdCookieSecure;
 
+    @Value("#{ @environment['shiro.sessionManager.cookie.sameSite'] ?: T(org.apache.shiro.web.servlet.Cookie.SameSiteOptions).LAX  }")
+    protected Cookie.SameSiteOptions sessionIdCookieSameSite;
+
 
     // RememberMe Cookie info
 
@@ -84,6 +87,9 @@ public class AbstractShiroWebConfiguration extends AbstractShiroConfiguration {
     @Value("#{ @environment['shiro.rememberMeManager.cookie.secure'] ?: false }")
     protected boolean rememberMeCookieSecure;
 
+    @Value("#{ @environment['shiro.rememberMeManager.cookie.sameSite'] ?: T(org.apache.shiro.web.servlet.Cookie.SameSiteOptions).LAX }")
+    protected Cookie.SameSiteOptions rememberMeSameSite;
+
 
     protected SessionManager nativeSessionManager() {
         DefaultWebSessionManager webSessionManager = new DefaultWebSessionManager();
@@ -104,7 +110,8 @@ protected Cookie sessionCookieTemplate() {
                 sessionIdCookieMaxAge,
                 sessionIdCookiePath,
                 sessionIdCookieDomain,
-                sessionIdCookieSecure);
+                sessionIdCookieSecure,
+                sessionIdCookieSameSite);
     }
 
     protected Cookie rememberMeCookieTemplate() {
@@ -113,16 +120,22 @@ protected Cookie rememberMeCookieTemplate() {
                 rememberMeCookieMaxAge,
                 rememberMeCookiePath,
                 rememberMeCookieDomain,
-                rememberMeCookieSecure);
+                rememberMeCookieSecure,
+                rememberMeSameSite);
     }
 
     protected Cookie buildCookie(String name, int maxAge, String path, String domain, boolean secure) {
+        return buildCookie(name, maxAge, path, domain, secure, Cookie.SameSiteOptions.LAX);
+    }
+
+    protected Cookie buildCookie(String name, int maxAge, String path, String domain, boolean secure, Cookie.SameSiteOptions sameSiteOption) {
         Cookie cookie = new SimpleCookie(name);
         cookie.setHttpOnly(true);
         cookie.setMaxAge(maxAge);
         cookie.setPath(path);
         cookie.setDomain(domain);
         cookie.setSecure(secure);
+        cookie.setSameSite(sameSiteOption);
 
         return cookie;
     }
@@ -135,6 +148,7 @@ protected SessionManager sessionManager() {
         return new ServletContainerSessionManager();
     }
 
+    @Override
     protected RememberMeManager rememberMeManager() {
         CookieRememberMeManager cookieRememberMeManager = new CookieRememberMeManager();
         cookieRememberMeManager.setCookie(rememberMeCookieTemplate());
@@ -151,6 +165,7 @@ protected SessionStorageEvaluator sessionStorageEvaluator() {
         return new DefaultWebSessionStorageEvaluator();
     }
 
+    @Override
     protected SessionsSecurityManager createSecurityManager() {
 
         DefaultWebSecurityManager securityManager = new DefaultWebSecurityManager();

support/spring/src/test/groovy/org/apache/shiro/spring/web/config/ShiroWebConfigurationTest.groovy

@@ -18,24 +18,17 @@
  */
 package org.apache.shiro.spring.web.config
 
-import org.apache.shiro.event.EventBus
-import org.apache.shiro.event.support.DefaultEventBus
 import org.apache.shiro.mgt.SecurityManager
 import org.apache.shiro.mgt.SessionStorageEvaluator
 import org.apache.shiro.realm.text.TextConfigurationRealm
-import org.apache.shiro.spring.config.EventBusTestConfiguration
-import org.apache.shiro.spring.config.RealmTestConfiguration
-import org.apache.shiro.spring.config.ShiroAnnotationProcessorConfiguration
-import org.apache.shiro.spring.config.ShiroBeanConfiguration
-import org.apache.shiro.spring.security.interceptor.AuthorizationAttributeSourceAdvisor
 import org.apache.shiro.spring.testconfig.EventBusTestConfiguration
 import org.apache.shiro.spring.testconfig.RealmTestConfiguration
-import org.apache.shiro.web.mgt.CookieRememberMeManager
 import org.apache.shiro.web.mgt.DefaultWebSessionStorageEvaluator
 import org.apache.shiro.web.mgt.WebSecurityManager
 import org.apache.shiro.web.servlet.Cookie
-
 import org.junit.Test
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.function.Executable
 import org.junit.runner.RunWith
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.beans.factory.annotation.Qualifier
@@ -102,6 +95,13 @@ public class ShiroWebConfigurationTest {
         println(message);
     }
 
+    @Test
+    void testSameSiteOptionExpression() {
+        ExpressionParser parser = new SpelExpressionParser();
+        Executable expressionParser = () -> parser.parseExpression("T(org.apache.shiro.web.servlet.Cookie.SameSiteOptions).LAX")
+        Assertions.assertDoesNotThrow expressionParser;
+    }
+
     @Test
     public void rememberMeCookie() {
         assertEquals "rememberMe", rememberMeCookie.name
@@ -112,4 +112,10 @@ public class ShiroWebConfigurationTest {
         assertSame "JSESSIONID", sessionCookieTemplate.name
 
     }
+
+    @Test
+    void sameSiteOption() {
+        assertSame Cookie.SameSiteOptions.LAX, rememberMeCookie.sameSite
+        assertSame Cookie.SameSiteOptions.LAX, sessionCookieTemplate.sameSite
+    }
 }

support/spring/src/test/java/org/apache/shiro/spring/web/config/ShiroWebConfigurationTestSameSiteStrict.java

@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF 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 org.apache.shiro.spring.web.config;
+
+import org.apache.shiro.spring.testconfig.EventBusTestConfiguration;
+import org.apache.shiro.spring.testconfig.RealmTestConfiguration;
+import org.apache.shiro.web.servlet.Cookie;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@ExtendWith(SpringExtension.class)
+@ContextConfiguration(classes = {EventBusTestConfiguration.class, RealmTestConfiguration.class, ShiroWebConfiguration.class})
+@TestPropertySource
+public class ShiroWebConfigurationTestSameSiteStrict {
+
+    @Autowired
+    ShiroWebConfiguration shiroWebConfiguration;
+
+    @Test
+    void testStrictSameSite() {
+        // given
+        // org/apache/shiro/spring/web/config/ShiroWebConfigurationTestSameSiteStrict.properties
+
+        // then
+        assertEquals(Cookie.SameSiteOptions.STRICT, shiroWebConfiguration.sessionIdCookieSameSite);
+        assertEquals(Cookie.SameSiteOptions.STRICT, shiroWebConfiguration.rememberMeSameSite);
+
+        assertEquals(Cookie.SameSiteOptions.STRICT, shiroWebConfiguration.sessionCookieTemplate().getSameSite());
+        assertEquals(Cookie.SameSiteOptions.STRICT, shiroWebConfiguration.rememberMeCookieTemplate().getSameSite());
+    }
+}

support/spring/src/test/resources/org/apache/shiro/spring/web/config/ShiroWebConfigurationTestSameSiteStrict.properties

@@ -0,0 +1,21 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF 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.
+#
+
+shiro.sessionManager.cookie.sameSite = STRICT
+shiro.rememberMeManager.cookie.sameSite = STRICT

web/src/main/java/org/apache/shiro/web/servlet/Cookie.java

@@ -35,7 +35,7 @@ public interface Cookie {
      * The value of deleted cookie (with the maxAge 0).
      */
     public static final String DELETED_COOKIE_VALUE = "deleteMe";
-    
+
 
     /**
      * The number of seconds in one year (= 60 * 60 * 24 * 365).
@@ -47,9 +47,28 @@ public interface Cookie {
      */
     public static final String ROOT_PATH = "/";
 
+    /**
+     * The SameSite attribute of the Set-Cookie HTTP response header allows you to declare if your cookie should be restricted to a first-party or same-site context.
+     */
     public enum SameSiteOptions {
+        /**
+         * Cookies will be sent in all contexts, i.e sending cross-origin is allowed.
+         *
+         * <p>None used to be the default value, but recent browser versions made Lax the default value
+         * to have reasonably robust defense against some classes of cross-site request forgery (CSRF) attacks.</p>
+         *
+         * <p>None requires the Secure attribute in latest browser versions. See below for more information.</p>
+         */
         NONE,
+        /**
+         * Cookies are allowed to be sent with top-level navigations and will be sent along with GET requests
+         * initiated by third party website. This is the default value in modern browsers as of 2020.
+         */
         LAX,
+        /**
+         * Cookies will only be sent in a first-party context
+         * and not be sent along with requests initiated by third party websites.
+         */
         STRICT,
     }
 

web/src/main/java/org/apache/shiro/web/servlet/SimpleCookie.java

@@ -108,87 +108,111 @@ public SimpleCookie(Cookie cookie) {
         this.sameSite = cookie.getSameSite();
     }
 
+    @Override
     public String getName() {
         return name;
     }
 
+    @Override
     public void setName(String name) {
         if (!StringUtils.hasText(name)) {
             throw new IllegalArgumentException("Name cannot be null/empty.");
         }
         this.name = name;
     }
 
+    @Override
     public String getValue() {
         return value;
     }
 
+    @Override
     public void setValue(String value) {
         this.value = value;
     }
 
+    @Override
     public String getComment() {
         return comment;
     }
 
+    @Override
     public void setComment(String comment) {
         this.comment = comment;
     }
 
+    @Override
     public String getDomain() {
         return domain;
     }
 
+    @Override
     public void setDomain(String domain) {
         this.domain = domain;
     }
 
+    @Override
     public String getPath() {
         return path;
     }
 
+    @Override
     public void setPath(String path) {
         this.path = path;
     }
 
+    @Override
     public int getMaxAge() {
         return maxAge;
     }
 
+    @Override
     public void setMaxAge(int maxAge) {
         this.maxAge = Math.max(DEFAULT_MAX_AGE, maxAge);
     }
 
+    @Override
     public int getVersion() {
         return version;
     }
 
+    @Override
     public void setVersion(int version) {
         this.version = Math.max(DEFAULT_VERSION, version);
     }
 
+    @Override
     public boolean isSecure() {
         return secure;
     }
 
+    @Override
     public void setSecure(boolean secure) {
         this.secure = secure;
     }
 
+    @Override
     public boolean isHttpOnly() {
         return httpOnly;
     }
 
+    @Override
     public void setHttpOnly(boolean httpOnly) {
         this.httpOnly = httpOnly;
     }
 
+    @Override
     public SameSiteOptions getSameSite() {
         return sameSite;
     }
 
+    @Override
     public void setSameSite(SameSiteOptions sameSite) {
         this.sameSite = sameSite;
+        if (this.sameSite == SameSiteOptions.NONE) {
+            // do not allow invalid cookies. Only secure cookies are allowed if SameSite is set to NONE.
+            setSecure(true);
+        }
     }
 
     /**
@@ -213,6 +237,7 @@ private String calculatePath(HttpServletRequest request) {
         return path;
     }
 
+    @Override
     public void saveTo(HttpServletRequest request, HttpServletResponse response) {
 
         String name = getName();
@@ -388,6 +413,7 @@ private static String toCookieDate(Date date) {
         return fmt.format(date);
     }
 
+    @Override
     public void removeFrom(HttpServletRequest request, HttpServletResponse response) {
         String name = getName();
         String value = DELETED_COOKIE_VALUE;
@@ -405,6 +431,7 @@ public void removeFrom(HttpServletRequest request, HttpServletResponse response)
         log.trace("Removed '{}' cookie by setting maxAge=0", name);
     }
 
+    @Override
     public String readValue(HttpServletRequest request, HttpServletResponse ignored) {
         String name = getName();
         String value = null;