Skip to content

Commit

Permalink
Merge pull request #30413 from sberyozkin/2.13
Browse files Browse the repository at this point in the history
Set Path and SameSite Strict attributes on Form, OIDC and WebAuthn cookies
  • Loading branch information
gsmet authored Jan 18, 2023
2 parents f251cd6 + 47bb512 commit 1edac3d
Show file tree
Hide file tree
Showing 14 changed files with 305 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ 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(),
config.cookiePath.orElse(null));
String loginPage = config.loginPage.startsWith("/") ? config.loginPage : "/" + config.loginPage;
return new WebAuthnAuthenticationMechanism(loginManager, loginPage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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.
*
Expand Down Expand Up @@ -222,4 +231,16 @@ 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;

/**
* The cookie path for the session cookies.
*/
@ConfigItem(defaultValue = "/")
public Optional<String> cookiePath = Optional.of("/");
}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -62,7 +63,8 @@ public void testFormBasedAuthSuccess() {
.assertThat()
.statusCode(302)
.header("location", containsString("/login"))
.cookie("quarkus-redirect-location", containsString("/admin"));
.cookie("quarkus-redirect-location",
detailedCookie().sameSite("Strict").path(equalTo("/")).value(containsString("/admin")));

RestAssured
.given()
Expand All @@ -76,7 +78,7 @@ public void testFormBasedAuthSuccess() {
.assertThat()
.statusCode(302)
.header("location", containsString("/admin"))
.cookie("quarkus-credential", notNullValue());
.cookie("quarkus-credential", detailedCookie().sameSite("Strict").path(equalTo("/")));

RestAssured
.given()
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {

Expand All @@ -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";

Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
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;
import static org.hamcrest.Matchers.nullValue;

import java.util.function.Supplier;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;
import io.restassured.filter.cookie.CookieFilter;

public class FormBasicAuthHttpRootNoCookiePathTestCase {

private static final String APP_PROPS = "" +
"quarkus.http.root-path=/root\n" +
"quarkus.http.auth.form.enabled=true\n" +
"quarkus.http.auth.form.login-page=login\n" +
"quarkus.http.auth.form.cookie-path=\n" +
"quarkus.http.auth.form.error-page=error\n" +
"quarkus.http.auth.form.landing-page=landing\n" +
"quarkus.http.auth.policy.r1.roles-allowed=admin\n" +
"quarkus.http.auth.permission.roles1.paths=/root/admin\n" +
"quarkus.http.auth.permission.roles1.policy=r1\n";

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest().setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(TestIdentityProvider.class, TestTrustedIdentityProvider.class, TestIdentityController.class,
PathHandler.class)
.addAsResource(new StringAsset(APP_PROPS), "application.properties");
}
});

@BeforeAll
public static void setup() {
TestIdentityController.resetRoles()
.add("admin", "admin", "admin");
}

@Test
public void testFormBasedAuthSuccess() {
CookieFilter cookies = new CookieFilter();
RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.get("/admin")
.then()
.assertThat()
.statusCode(302)
.header("location", containsString("/login"))
.cookie("quarkus-redirect-location",
detailedCookie().value(containsString("/root/admin")).path(nullValue()));

RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.formParam("j_username", "admin")
.formParam("j_password", "admin")
.post("/j_security_check")
.then()
.assertThat()
.statusCode(302)
.header("location", containsString("/root/admin"))
.cookie("quarkus-credential",
detailedCookie().value(notNullValue()).path(nullValue()));

RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.get("/admin")
.then()
.assertThat()
.statusCode(200)
.body(equalTo("admin:/root/admin"));

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
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;

import java.util.function.Supplier;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;
import io.restassured.filter.cookie.CookieFilter;

public class FormBasicAuthHttpRootTestCase {

private static final String APP_PROPS = "" +
"quarkus.http.root-path=/root\n" +
"quarkus.http.auth.form.enabled=true\n" +
"quarkus.http.auth.form.login-page=login\n" +
"quarkus.http.auth.form.cookie-path=/root\n" +
"quarkus.http.auth.form.error-page=error\n" +
"quarkus.http.auth.form.landing-page=landing\n" +
"quarkus.http.auth.policy.r1.roles-allowed=admin\n" +
"quarkus.http.auth.permission.roles1.paths=/root/admin\n" +
"quarkus.http.auth.permission.roles1.policy=r1\n";

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest().setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(TestIdentityProvider.class, TestTrustedIdentityProvider.class, TestIdentityController.class,
PathHandler.class)
.addAsResource(new StringAsset(APP_PROPS), "application.properties");
}
});

@BeforeAll
public static void setup() {
TestIdentityController.resetRoles()
.add("admin", "admin", "admin");
}

@Test
public void testFormBasedAuthSuccess() {
CookieFilter cookies = new CookieFilter();
RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.get("/admin")
.then()
.assertThat()
.statusCode(302)
.header("location", containsString("/login"))
.cookie("quarkus-redirect-location",
detailedCookie().value(containsString("/root/admin")).path(equalTo("/root")));

RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.formParam("j_username", "admin")
.formParam("j_password", "admin")
.post("/j_security_check")
.then()
.assertThat()
.statusCode(302)
.header("location", containsString("/root/admin"))
.cookie("quarkus-credential",
detailedCookie().value(notNullValue()).path(equalTo("/root")));

RestAssured
.given()
.filter(cookies)
.redirects().follow(false)
.when()
.get("/admin")
.then()
.assertThat()
.statusCode(200)
.body(equalTo("admin:/root/admin"));

}
}
Loading

0 comments on commit 1edac3d

Please sign in to comment.