From 89405009ec42cdbc811682abadebd20cfaba2ab2 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 12 Aug 2024 13:10:35 +0200 Subject: [PATCH] set a Max-Age on the auth cookie --- commafeed-server/TODO.md | 4 - ...okieMaxAgeFormAuthenticationMechanism.java | 115 ++++++++++++++++++ .../com/commafeed/integration/SecurityIT.java | 27 ++++ 3 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 commafeed-server/src/main/java/com/commafeed/security/mechanism/CookieMaxAgeFormAuthenticationMechanism.java diff --git a/commafeed-server/TODO.md b/commafeed-server/TODO.md index 0a6d77bd..49ee11f4 100644 --- a/commafeed-server/TODO.md +++ b/commafeed-server/TODO.md @@ -3,10 +3,6 @@ TODO MVP: -- cookie duration too short - - https://github.com/quarkusio/quarkus/issues/42463 - - Rewrite cookie with https://quarkus.io/guides/rest#request-or-response-filters in the mean time - - update github actions - release after tag - new job that downloads all artifacts because we need them all to create the release diff --git a/commafeed-server/src/main/java/com/commafeed/security/mechanism/CookieMaxAgeFormAuthenticationMechanism.java b/commafeed-server/src/main/java/com/commafeed/security/mechanism/CookieMaxAgeFormAuthenticationMechanism.java new file mode 100644 index 00000000..97445772 --- /dev/null +++ b/commafeed-server/src/main/java/com/commafeed/security/mechanism/CookieMaxAgeFormAuthenticationMechanism.java @@ -0,0 +1,115 @@ +package com.commafeed.security.mechanism; + +import java.security.SecureRandom; +import java.util.Base64; +import java.util.Set; + +import io.quarkus.security.identity.IdentityProviderManager; +import io.quarkus.security.identity.SecurityIdentity; +import io.quarkus.security.identity.request.AuthenticationRequest; +import io.quarkus.vertx.http.runtime.FormAuthConfig; +import io.quarkus.vertx.http.runtime.FormAuthRuntimeConfig; +import io.quarkus.vertx.http.runtime.HttpBuildTimeConfig; +import io.quarkus.vertx.http.runtime.HttpConfiguration; +import io.quarkus.vertx.http.runtime.security.ChallengeData; +import io.quarkus.vertx.http.runtime.security.FormAuthenticationMechanism; +import io.quarkus.vertx.http.runtime.security.HttpAuthenticationMechanism; +import io.quarkus.vertx.http.runtime.security.HttpCredentialTransport; +import io.quarkus.vertx.http.runtime.security.PersistentLoginManager; +import io.smallrye.mutiny.Uni; +import io.vertx.core.http.Cookie; +import io.vertx.core.http.impl.ServerCookie; +import io.vertx.ext.web.RoutingContext; +import jakarta.annotation.Priority; +import jakarta.inject.Singleton; +import lombok.extern.slf4j.Slf4j; + +/** + * HttpAuthenticationMechanism that wraps FormAuthenticationMechanism and sets a Max-Age on the cookie because it has no value by default. + * + * This is a workaround for https://github.com/quarkusio/quarkus/issues/42463 + */ +@Priority(1) +@Singleton +@Slf4j +public class CookieMaxAgeFormAuthenticationMechanism implements HttpAuthenticationMechanism { + + // the temp encryption key, persistent across dev mode restarts + static volatile String encryptionKey; + + private final FormAuthenticationMechanism delegate; + + public CookieMaxAgeFormAuthenticationMechanism(HttpConfiguration httpConfiguration, HttpBuildTimeConfig buildTimeConfig) { + String key; + if (httpConfiguration.encryptionKey.isEmpty()) { + if (encryptionKey != null) { + // persist across dev mode restarts + key = encryptionKey; + } else { + byte[] data = new byte[32]; + new SecureRandom().nextBytes(data); + key = encryptionKey = Base64.getEncoder().encodeToString(data); + log.warn("Encryption key was not specified for persistent FORM auth, using temporary key {}", key); + } + } else { + key = httpConfiguration.encryptionKey.get(); + } + + FormAuthConfig form = buildTimeConfig.auth.form; + FormAuthRuntimeConfig runtimeForm = httpConfiguration.auth.form; + String loginPage = startWithSlash(runtimeForm.loginPage.orElse(null)); + String errorPage = startWithSlash(runtimeForm.errorPage.orElse(null)); + String landingPage = startWithSlash(runtimeForm.landingPage.orElse(null)); + String postLocation = startWithSlash(form.postLocation); + String usernameParameter = runtimeForm.usernameParameter; + String passwordParameter = runtimeForm.passwordParameter; + String locationCookie = runtimeForm.locationCookie; + String cookiePath = runtimeForm.cookiePath.orElse(null); + boolean redirectAfterLogin = landingPage != null; + String cookieSameSite = runtimeForm.cookieSameSite.name(); + + PersistentLoginManager loginManager = new PersistentLoginManager(key, runtimeForm.cookieName, runtimeForm.timeout.toMillis(), + runtimeForm.newCookieInterval.toMillis(), runtimeForm.httpOnlyCookie, cookieSameSite, cookiePath) { + @Override + public void save(String value, RoutingContext context, String cookieName, RestoreResult restoreResult, boolean secureCookie) { + super.save(value, context, cookieName, restoreResult, secureCookie); + + // add max age to the cookie + Cookie cookie = context.request().getCookie(cookieName); + if (cookie instanceof ServerCookie sc && sc.isChanged()) { + cookie.setMaxAge(runtimeForm.timeout.toSeconds()); + } + } + }; + + this.delegate = new FormAuthenticationMechanism(loginPage, postLocation, usernameParameter, passwordParameter, errorPage, + landingPage, redirectAfterLogin, locationCookie, cookieSameSite, cookiePath, loginManager); + } + + @Override + public Uni authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) { + return delegate.authenticate(context, identityProviderManager); + } + + @Override + public Uni getChallenge(RoutingContext context) { + return delegate.getChallenge(context); + } + + @Override + public Set> getCredentialTypes() { + return delegate.getCredentialTypes(); + } + + @Override + public Uni getCredentialTransport(RoutingContext context) { + return delegate.getCredentialTransport(context); + } + + private static String startWithSlash(String page) { + if (page == null) { + return null; + } + return page.startsWith("/") ? page : "/" + page; + } +} diff --git a/commafeed-server/src/test/java/com/commafeed/integration/SecurityIT.java b/commafeed-server/src/test/java/com/commafeed/integration/SecurityIT.java index 3864cd7b..5ba7c6c6 100644 --- a/commafeed-server/src/test/java/com/commafeed/integration/SecurityIT.java +++ b/commafeed-server/src/test/java/com/commafeed/integration/SecurityIT.java @@ -1,6 +1,9 @@ package com.commafeed.integration; +import java.net.HttpCookie; import java.util.Base64; +import java.util.List; +import java.util.stream.Collectors; import org.apache.hc.core5.http.HttpStatus; import org.junit.jupiter.api.Assertions; @@ -27,6 +30,30 @@ class SecurityIT extends BaseIT { } } + @Test + void formLogin() { + List cookies = login(); + + try (Response response = getClient().target(getApiBaseUrl() + "user/profile") + .request() + .header(HttpHeaders.COOKIE, cookies.stream().map(HttpCookie::toString).collect(Collectors.joining(";"))) + .get()) { + Assertions.assertEquals(HttpStatus.SC_OK, response.getStatus()); + cookies.forEach(c -> Assertions.assertTrue(c.getMaxAge() > 0)); + } + } + + @Test + void basicAuthLogin() { + String auth = "Basic " + Base64.getEncoder().encodeToString("admin:admin".getBytes()); + try (Response response = getClient().target(getApiBaseUrl() + "user/profile") + .request() + .header(HttpHeaders.AUTHORIZATION, auth) + .get()) { + Assertions.assertEquals(HttpStatus.SC_OK, response.getStatus()); + } + } + @Test void wrongPassword() { String auth = "Basic " + Base64.getEncoder().encodeToString("admin:wrong-password".getBytes());