From 027ac5b5fbccbaee74fbe5349765422b4d01f95e Mon Sep 17 00:00:00 2001 From: takeseem Date: Wed, 1 Oct 2025 03:13:51 +0800 Subject: [PATCH] fix #2765: CookieSessionStore requires twice auth on first login Calling session.value() twice in the same request caused the session data to be corrupted when no session cookie existed. This made the first login fail to persist the user, requiring a second authentication. The fix is to separate initial cookie creation from subsequent cookie updates so session.value() is only invoked once. Also adjust SessionHandlerImpl to persist the authenticated user correctly. --- .../tests/CookieSessionHandlerTest.java | 60 +++++++++++++++++++ .../src/test/java/module-info.java | 2 + .../web/handler/impl/SessionHandlerImpl.java | 50 ++++++++-------- 3 files changed, 88 insertions(+), 24 deletions(-) diff --git a/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/io/vertx/ext/web/sstore/cookie/tests/CookieSessionHandlerTest.java b/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/io/vertx/ext/web/sstore/cookie/tests/CookieSessionHandlerTest.java index 6951b9a56f..3a5f7169b6 100644 --- a/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/io/vertx/ext/web/sstore/cookie/tests/CookieSessionHandlerTest.java +++ b/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/io/vertx/ext/web/sstore/cookie/tests/CookieSessionHandlerTest.java @@ -16,10 +16,17 @@ package io.vertx.ext.web.sstore.cookie.tests; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.Future; import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpMethod; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.auth.User; import io.vertx.ext.web.Session; +import io.vertx.ext.web.handler.BodyHandler; +import io.vertx.ext.web.handler.HttpException; import io.vertx.ext.web.handler.SessionHandler; +import io.vertx.ext.web.handler.SimpleAuthenticationHandler; import io.vertx.ext.web.sstore.cookie.CookieSessionStore; import io.vertx.ext.web.tests.handler.SessionHandlerTestBase; import org.junit.Ignore; @@ -38,6 +45,59 @@ public void setUp() throws Exception { store = CookieSessionStore.create(vertx, "KeyboardCat!", Buffer.buffer("salt")); } + @Test + public void testSessionAndUser() throws Exception { + router.route().handler(BodyHandler.create()); + router.route().handler(SessionHandler.create(store)); + router.route("/authenticate").handler(SimpleAuthenticationHandler.create().authenticate(rc -> { + JsonObject body = rc.body().asJsonObject(); + if (body.getString("username").equals("myuser")) { + return Future.succeededFuture(User.create(body)); + } else { + return Future.failedFuture(new HttpException(401)); + } + })).handler(rc -> { + if (rc.normalizedPath().equals("/authenticate")) { + rc.response().end("Authenticated"); + } else { + rc.next(); + } + }); + router.route("/private/*").handler(rc -> { + if (rc.user() == null) { + rc.response().setStatusCode(401).end(); + } else { + rc.next(); + } + }); + router.get("/private/whoami").handler(rc -> rc.end(rc.user().principal().encode())); + + testRequest(HttpMethod.GET, "/private/whoami", HttpResponseStatus.UNAUTHORIZED); + + JsonObject myuser = new JsonObject().put("username", "myuser").put("password", System.currentTimeMillis()); + AtomicReference cookie = new AtomicReference<>(); + testRequest(HttpMethod.POST, "/authenticate", + req -> req.putHeader("content-type", "application/json").send(myuser.toString()), + resp -> { + String setCookie = resp.headers().get("set-cookie"); + cookie.set(setCookie.substring(0, setCookie.indexOf(";"))); + resp.body().compose(body -> { + assertEquals("Authenticated", body.toString()); + return Future.succeededFuture(); + }).onFailure(this::fail); + }, 200, "OK", null); + assertNotNull(cookie.get()); + + testRequest(HttpMethod.GET, "/private/whoami", req -> { + req.putHeader("cookie", cookie.get()); + }, resp -> { + resp.body().compose(body -> { + assertEquals(myuser, body.toJsonObject()); + return Future.succeededFuture(); + }).onFailure(this::fail); + }, 200, "OK", null); + } + @Test public void testGetSession() throws Exception { Session session = store.createSession(30_000); diff --git a/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/module-info.java b/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/module-info.java index 8149983add..6cca181038 100644 --- a/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/module-info.java +++ b/vertx-web-session-stores/vertx-web-sstore-cookie/src/test/java/module-info.java @@ -20,4 +20,6 @@ requires io.vertx.web.tests; requires junit; requires io.vertx.testing.unit; + requires io.vertx.auth.common; + requires io.netty.codec.http; } diff --git a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java index 65c031fad4..e80968ffc0 100644 --- a/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java +++ b/vertx-web/src/main/java/io/vertx/ext/web/handler/impl/SessionHandlerImpl.java @@ -199,15 +199,19 @@ private Future flush(RoutingContext context, boolean skipCrc, boolean igno session.setAccessed(); } else { // the session cookie needs to be updated to the new id - final Cookie cookie = sessionCookie(context, session); + final Cookie cookie = requestSessionCookie(context); // restore defaults session.setAccessed(); - String cookieValue = session.value(); - if (Objects.nonNull(signature)) { - cookieValue = signature.sign(cookieValue); + if (cookie != null) { + String cookieValue = session.value(); + if (Objects.nonNull(signature)) { + cookieValue = signature.sign(cookieValue); + } + cookie.setValue(cookieValue); + setCookieProperties(cookie, false); + } else { + sessionCookie(context, session.value()); } - cookie.setValue(cookieValue); - setCookieProperties(cookie, false); } // we must invalidate the old id @@ -225,7 +229,9 @@ private Future flush(RoutingContext context, boolean skipCrc, boolean igno } else if (!lazySession || sessionUsed) { if (!cookieless) { // if lazy mode activated, no need to store the session nor to create the session cookie if not used. - sessionCookie(context, session); + if (requestSessionCookie(context) == null) { + sessionCookie(context, session.value()); + } } session.setAccessed(); return sessionStore.put(session) @@ -400,11 +406,7 @@ private String getSessionId(RoutingContext context) { return path.substring(s, e); } } else { - // only pick the first cookie, when multiple sessions are used: - // https://www.rfc-editor.org/rfc/rfc6265#section-5.4 - // The user agent SHOULD sort the cookie-list in the following order: - // Cookies with longer paths are listed before cookies with shorter paths. - Cookie cookie = context.request().getCookie(sessionCookieName); + Cookie cookie = requestSessionCookie(context); if (cookie != null) { // Look up sessionId if (Objects.nonNull(signature)) { @@ -475,22 +477,22 @@ private void createNewSession(RoutingContext context) { addStoreSessionHandler(context); } - private Cookie sessionCookie(final RoutingContext context, final Session session) { - // only pick the first cookie, when multiple sessions are used: - // https://www.rfc-editor.org/rfc/rfc6265#section-5.4 - // The user agent SHOULD sort the cookie-list in the following order: - // Cookies with longer paths are listed before cookies with shorter paths. - Cookie cookie = context.request().getCookie(sessionCookieName); - if (cookie != null) { - return cookie; - } - String cookieValue = session.value(); + /** + * only pick the first cookie, when multiple sessions are used: + * https://www.rfc-editor.org/rfc/rfc6265#section-5.4 + * The user agent SHOULD sort the cookie-list in the following order: + * Cookies with longer paths are listed before cookies with shorter paths. + */ + private Cookie requestSessionCookie(final RoutingContext context) { + return context.request().getCookie(sessionCookieName); + } + + private void sessionCookie(final RoutingContext context, String cookieValue) { if (Objects.nonNull(signature)) { cookieValue = signature.sign(cookieValue); } - cookie = Cookie.cookie(sessionCookieName, cookieValue); + Cookie cookie = Cookie.cookie(sessionCookieName, cookieValue); setCookieProperties(cookie, false); context.response().addCookie(cookie); - return cookie; } }