Set SameSite Strict on Form, OIDC and WebAuthn cookies

quarkusio · Jan 17, 2023 · e36686a · e36686a
1 parent f251cd6
commit e36686a
Show file tree

Hide file tree

Showing 12 changed files with 89 additions and 14 deletions.
diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java
@@ -568,6 +568,15 @@ public static enum Source {
     @ConfigGroup
     public static class Authentication {
 
+        /**
+         * SameSite attribute values for the session, state and post logout cookies.
+         */
+        public enum CookieSameSite {
+            STRICT,
+            LAX,
+            NONE
+        }
+
         /**
          * Authorization code flow response mode
          */
@@ -716,6 +725,12 @@ public enum ResponseMode {
         @ConfigItem
         public Optional<String> cookieDomain = Optional.empty();
 
+        /**
+         * SameSite attribute for the session, state and post logout cookies.
+         */
+        @ConfigItem(defaultValue = "strict")
+        public CookieSameSite cookieSameSite = CookieSameSite.STRICT;
+
         /**
          * If this property is set to 'true' then an OIDC UserInfo endpoint will be called.
          */
@@ -934,6 +949,14 @@ public Optional<ResponseMode> getResponseMode() {
         public void setResponseMode(ResponseMode responseMode) {
             this.responseMode = Optional.of(responseMode);
         }
+
+        public CookieSameSite getCookieSameSite() {
+            return cookieSameSite;
+        }
+
+        public void setCookieSameSite(CookieSameSite cookieSameSite) {
+            this.cookieSameSite = cookieSameSite;
+        }
     }
 
     /**

diff --git a/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
@@ -48,6 +48,7 @@
 import io.smallrye.mutiny.Uni;
 import io.vertx.core.MultiMap;
 import io.vertx.core.http.Cookie;
+import io.vertx.core.http.CookieSameSite;
 import io.vertx.core.http.HttpHeaders;
 import io.vertx.core.http.impl.CookieImpl;
 import io.vertx.core.http.impl.ServerCookie;
@@ -838,6 +839,7 @@ static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcCo
         if (auth.cookieDomain.isPresent()) {
             cookie.setDomain(auth.getCookieDomain().get());
         }
+        cookie.setSameSite(CookieSameSite.valueOf(auth.cookieSameSite.name()));
         context.response().addCookie(cookie);
         return cookie;
     }

diff --git a/...ecurity-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnRecorder.java b/...ecurity-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnRecorder.java
@@ -69,7 +69,7 @@ public WebAuthnAuthenticationMechanism get() {
                 WebAuthnRunTimeConfig config = WebAuthnRecorder.this.config.getValue();
                 PersistentLoginManager loginManager = new PersistentLoginManager(key, config.cookieName,
                         config.sessionTimeout.toMillis(),
-                        config.newCookieInterval.toMillis(), false);
+                        config.newCookieInterval.toMillis(), false, config.cookieSameSite.name());
                 String loginPage = config.loginPage.startsWith("/") ? config.loginPage : "/" + config.loginPage;
                 return new WebAuthnAuthenticationMechanism(loginManager, loginPage);
             }

diff --git a/...ty-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnRunTimeConfig.java b/...ty-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnRunTimeConfig.java
@@ -9,6 +9,7 @@
 import io.quarkus.runtime.annotations.ConfigItem;
 import io.quarkus.runtime.annotations.ConfigPhase;
 import io.quarkus.runtime.annotations.ConfigRoot;
+import io.quarkus.vertx.http.runtime.FormAuthConfig.CookieSameSite;
 import io.vertx.ext.auth.webauthn.Attestation;
 import io.vertx.ext.auth.webauthn.AuthenticatorAttachment;
 import io.vertx.ext.auth.webauthn.AuthenticatorTransport;
@@ -21,6 +22,15 @@
 @ConfigRoot(name = "webauthn", phase = ConfigPhase.RUN_TIME)
 public class WebAuthnRunTimeConfig {
 
+    /**
+     * SameSite attribute values for the session cookie.
+     */
+    public enum CookieSameSite {
+        STRICT,
+        LAX,
+        NONE
+    }
+
     /**
      * The origin of the application. The origin is basically protocol, host and port.
      *
@@ -222,4 +232,10 @@ public static class RelyingPartyConfig {
      */
     @ConfigItem(defaultValue = "quarkus-credential")
     public String cookieName;
+
+    /**
+     * SameSite attribute for the session cookie.
+     */
+    @ConfigItem(defaultValue = "strict")
+    public CookieSameSite cookieSameSite = CookieSameSite.STRICT;
 }
diff --git a/...eployment/src/test/java/io/quarkus/vertx/http/security/CombinedFormBasicAuthTestCase.java b/...eployment/src/test/java/io/quarkus/vertx/http/security/CombinedFormBasicAuthTestCase.java
@@ -1,5 +1,6 @@
 package io.quarkus.vertx.http.security;
 
+import static io.restassured.matcher.RestAssuredMatchers.detailedCookie;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
@@ -62,7 +63,7 @@ public void testFormBasedAuthSuccess() {
                 .assertThat()
                 .statusCode(302)
                 .header("location", containsString("/login"))
-                .cookie("quarkus-redirect-location", containsString("/admin"));
+                .cookie("quarkus-redirect-location", detailedCookie().sameSite("Strict").value(containsString("/admin")));
 
         RestAssured
                 .given()
@@ -76,7 +77,7 @@ public void testFormBasedAuthSuccess() {
                 .assertThat()
                 .statusCode(302)
                 .header("location", containsString("/admin"))
-                .cookie("quarkus-credential", notNullValue());
+                .cookie("quarkus-credential", detailedCookie().sameSite("Strict"));
 
         RestAssured
                 .given()

diff --git a/...http/deployment/src/test/java/io/quarkus/vertx/http/security/FormAuthCookiesTestCase.java b/...http/deployment/src/test/java/io/quarkus/vertx/http/security/FormAuthCookiesTestCase.java
@@ -1,5 +1,6 @@
 package io.quarkus.vertx.http.security;
 
+import static io.restassured.matcher.RestAssuredMatchers.detailedCookie;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
@@ -43,7 +44,6 @@
 import io.quarkus.test.common.http.TestHTTPResource;
 import io.restassured.RestAssured;
 import io.restassured.filter.cookie.CookieFilter;
-import io.restassured.matcher.RestAssuredMatchers;
 
 public class FormAuthCookiesTestCase {
 
@@ -58,6 +58,7 @@ public class FormAuthCookiesTestCase {
             "quarkus.http.auth.form.timeout=PT2S\n" +
             "quarkus.http.auth.form.new-cookie-interval=PT1S\n" +
             "quarkus.http.auth.form.cookie-name=laitnederc-sukrauq\n" +
+            "quarkus.http.auth.form.cookie-same-site=lax\n" +
             "quarkus.http.auth.form.http-only-cookie=true\n" +
             "quarkus.http.auth.session.encryption-key=CHANGEIT-CHANGEIT-CHANGEIT-CHANGEIT-CHANGEIT\n";
 
@@ -92,7 +93,7 @@ public void testFormBasedAuthSuccess() {
                 .assertThat()
                 .statusCode(302)
                 .header("location", containsString("/login"))
-                .cookie("quarkus-redirect-location", containsString("/admin%E2%9D%A4"));
+                .cookie("quarkus-redirect-location", detailedCookie().value(containsString("/admin%E2%9D%A4")).sameSite("Lax"));
 
         RestAssured
                 .given()
@@ -106,7 +107,8 @@ public void testFormBasedAuthSuccess() {
                 .assertThat()
                 .statusCode(302)
                 .header("location", containsString("/admin%E2%9D%A4"))
-                .cookie("laitnederc-sukrauq", RestAssuredMatchers.detailedCookie().value(notNullValue()).httpOnly(true));
+                .cookie("laitnederc-sukrauq", detailedCookie().value(notNullValue())
+                        .httpOnly(true).sameSite("Lax"));
 
         RestAssured
                 .given()

diff --git a/...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/FormAuthConfig.java b/...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/FormAuthConfig.java
@@ -11,7 +11,16 @@
 @ConfigGroup
 public class FormAuthConfig {
     /**
-     * If form authentication is enabled
+     * SameSite attribute values for the session and location cookies.
+     */
+    public enum CookieSameSite {
+        STRICT,
+        LAX,
+        NONE
+    }
+
+    /**
+     * If form authentication is enabled.
      */
     @ConfigItem
     public boolean enabled;
@@ -103,4 +112,10 @@ public class FormAuthConfig {
      */
     @ConfigItem(defaultValue = "false")
     public boolean httpOnlyCookie;
+
+    /**
+     * SameSite attribute for the session and location cookies.
+     */
+    @ConfigItem(defaultValue = "strict")
+    public CookieSameSite cookieSameSite = CookieSameSite.STRICT;
 }
diff --git a/...ime/src/main/java/io/quarkus/vertx/http/runtime/security/FormAuthenticationMechanism.java b/...ime/src/main/java/io/quarkus/vertx/http/runtime/security/FormAuthenticationMechanism.java
@@ -22,6 +22,7 @@
 import io.vertx.core.Handler;
 import io.vertx.core.MultiMap;
 import io.vertx.core.http.Cookie;
+import io.vertx.core.http.CookieSameSite;
 import io.vertx.core.http.HttpMethod;
 import io.vertx.ext.web.RoutingContext;
 
@@ -38,12 +39,12 @@ public class FormAuthenticationMechanism implements HttpAuthenticationMechanism
     private final String locationCookie;
     private final String landingPage;
     private final boolean redirectAfterLogin;
-
+    private final CookieSameSite cookieSameSite;
     private final PersistentLoginManager loginManager;
 
     public FormAuthenticationMechanism(String loginPage, String postLocation,
             String usernameParameter, String passwordParameter, String errorPage, String landingPage,
-            boolean redirectAfterLogin, String locationCookie, PersistentLoginManager loginManager) {
+            boolean redirectAfterLogin, String locationCookie, String cookieSameSite, PersistentLoginManager loginManager) {
         this.loginPage = loginPage;
         this.postLocation = postLocation;
         this.usernameParameter = usernameParameter;
@@ -52,6 +53,7 @@ public FormAuthenticationMechanism(String loginPage, String postLocation,
         this.errorPage = errorPage;
         this.landingPage = landingPage;
         this.redirectAfterLogin = redirectAfterLogin;
+        this.cookieSameSite = CookieSameSite.valueOf(cookieSameSite);
         this.loginManager = loginManager;
     }
 
@@ -122,6 +124,7 @@ protected void handleRedirectBack(final RoutingContext exchange) {
         if (redirect != null) {
             verifyRedirectBackLocation(exchange.request().absoluteURI(), redirect.getValue());
             redirect.setSecure(exchange.request().isSSL());
+            redirect.setSameSite(cookieSameSite);
             location = redirect.getValue();
             exchange.response().addCookie(redirect.setMaxAge(0));
         } else {
@@ -146,7 +149,7 @@ protected void verifyRedirectBackLocation(String requestURIString, String redire
 
     protected void storeInitialLocation(final RoutingContext exchange) {
         exchange.response().addCookie(Cookie.cookie(locationCookie, exchange.request().absoluteURI())
-                .setPath("/").setSecure(exchange.request().isSSL()));
+                .setPath("/").setSameSite(cookieSameSite).setSecure(exchange.request().isSSL()));
     }
 
     protected void servePage(final RoutingContext exchange, final String location) {

diff --git a/...tp/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java b/...tp/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java
@@ -266,7 +266,7 @@ public FormAuthenticationMechanism get() {
                 }
                 FormAuthConfig form = buildTimeConfig.auth.form;
                 PersistentLoginManager loginManager = new PersistentLoginManager(key, form.cookieName, form.timeout.toMillis(),
-                        form.newCookieInterval.toMillis(), form.httpOnlyCookie);
+                        form.newCookieInterval.toMillis(), form.httpOnlyCookie, form.cookieSameSite.name());
                 String loginPage = form.loginPage.startsWith("/") ? form.loginPage : "/" + form.loginPage;
                 String errorPage = form.errorPage.startsWith("/") ? form.errorPage : "/" + form.errorPage;
                 String landingPage = form.landingPage.startsWith("/") ? form.landingPage : "/" + form.landingPage;
@@ -276,7 +276,7 @@ public FormAuthenticationMechanism get() {
                 String locationCookie = form.locationCookie;
                 boolean redirectAfterLogin = form.redirectAfterLogin;
                 return new FormAuthenticationMechanism(loginPage, postLocation, usernameParameter, passwordParameter,
-                        errorPage, landingPage, redirectAfterLogin, locationCookie, loginManager);
+                        errorPage, landingPage, redirectAfterLogin, locationCookie, form.cookieSameSite.name(), loginManager);
             }
         };
     }

diff --git a/.../runtime/src/main/java/io/quarkus/vertx/http/runtime/security/PersistentLoginManager.java b/.../runtime/src/main/java/io/quarkus/vertx/http/runtime/security/PersistentLoginManager.java
@@ -17,6 +17,7 @@
 
 import io.quarkus.security.identity.SecurityIdentity;
 import io.vertx.core.http.Cookie;
+import io.vertx.core.http.CookieSameSite;
 import io.vertx.ext.web.RoutingContext;
 
 /**
@@ -36,13 +37,15 @@ public class PersistentLoginManager {
     private final SecureRandom secureRandom = new SecureRandom();
     private final long newCookieIntervalMillis;
     private final boolean httpOnlyCookie;
+    private final CookieSameSite cookieSameSite;
 
     public PersistentLoginManager(String encryptionKey, String cookieName, long timeoutMillis, long newCookieIntervalMillis,
-            boolean httpOnlyCookie) {
+            boolean httpOnlyCookie, String cookieSameSite) {
         this.cookieName = cookieName;
         this.newCookieIntervalMillis = newCookieIntervalMillis;
         this.timeoutMillis = timeoutMillis;
         this.httpOnlyCookie = httpOnlyCookie;
+        this.cookieSameSite = CookieSameSite.valueOf(cookieSameSite);
         try {
             if (encryptionKey == null) {
                 this.secretKey = KeyGenerator.getInstance("AES").generateKey();
@@ -135,7 +138,8 @@ public void save(String value, RoutingContext context, String cookieName, Restor
             message.put(encrypted);
             String cookieValue = Base64.getEncoder().encodeToString(message.array());
             context.addCookie(
-                    Cookie.cookie(cookieName, cookieValue).setPath("/").setSecure(secureCookie).setHttpOnly(httpOnlyCookie));
+                    Cookie.cookie(cookieName, cookieValue).setPath("/").setSameSite(cookieSameSite).setSecure(secureCookie)
+                            .setHttpOnly(httpOnlyCookie));
         } catch (Exception e) {
             throw new RuntimeException(e);
         }

diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties
@@ -106,6 +106,7 @@ quarkus.oidc.tenant-https.authentication.cookie-suffix=test
 quarkus.oidc.tenant-https.authentication.error-path=/tenant-https/error
 quarkus.oidc.tenant-https.authentication.pkce-required=true
 quarkus.oidc.tenant-https.authentication.pkce-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU
+quarkus.oidc.tenant-https.authentication.cookie-same-site=lax
 
 quarkus.oidc.tenant-javascript.auth-server-url=${keycloak.url}/realms/quarkus
 quarkus.oidc.tenant-javascript.client-id=quarkus-app

diff --git a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
@@ -51,6 +51,10 @@ public void testCodeFlowNoConsent() throws IOException {
                     .loadWebResponse(new WebRequest(URI.create("http://localhost:8081/index.html").toURL()));
             verifyLocationHeader(webClient, webResponse.getResponseHeaderValue("location"), null, "web-app", false);
 
+            String stateCookieString = webResponse.getResponseHeaderValue("Set-Cookie");
+            assertTrue(stateCookieString.startsWith("q_auth_Default_test="));
+            assertTrue(stateCookieString.contains("SameSite=Strict"));
+
             webClient.getCookieManager().clearCookies();
 
             webClient.getOptions().setRedirectEnabled(true);
@@ -168,6 +172,10 @@ public void testCodeFlowForceHttpsRedirectUriAndPkce() throws Exception {
             verifyLocationHeader(webClient, keycloakUrl, "tenant-https_test", "xforwarded%2Ftenant-https",
                     true);
 
+            String stateCookieString = webResponse.getResponseHeaderValue("Set-Cookie");
+            assertTrue(stateCookieString.startsWith("q_auth_tenant-https_test="));
+            assertTrue(stateCookieString.contains("SameSite=Lax"));
+
             HtmlPage page = webClient.getPage(keycloakUrl);
 
             assertEquals("Sign in to quarkus", page.getTitleText());