From 040edaa3f7ac359398dd1186cb0c9c110f8fa7cb Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 16:45:36 +0100 Subject: [PATCH 1/3] fix: forward every set-cookie header on logout forwardCookies called c.header once per cookie. Hono keeps only the last value of a repeated header, so the three clearing cookies from signOut collapsed into better-auth.dont_remember and the browser kept the stale session cookies. Pass append: true so each Set-Cookie survives as its own header. The new integration test asserts that all three arrive with Max-Age=0, Path=/, HttpOnly and SameSite=Lax. --- src/app/utils/cookies.js | 2 +- test/integration/cookie-attributes.test.js | 152 +++++++++++++++++++++ 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 test/integration/cookie-attributes.test.js diff --git a/src/app/utils/cookies.js b/src/app/utils/cookies.js index cc64a8e..2f19b1c 100644 --- a/src/app/utils/cookies.js +++ b/src/app/utils/cookies.js @@ -2,6 +2,6 @@ export const forwardCookies = (headers, c) => { const setCookieHeaders = headers.getSetCookie?.() || []; setCookieHeaders.forEach((cookie) => { - c.header("Set-Cookie", cookie); + c.header("Set-Cookie", cookie, { append: true }); }); }; diff --git a/test/integration/cookie-attributes.test.js b/test/integration/cookie-attributes.test.js new file mode 100644 index 0000000..7ea42ca --- /dev/null +++ b/test/integration/cookie-attributes.test.js @@ -0,0 +1,152 @@ +import { test } from "tap"; +import { getTestInstance } from "../helpers/test-instance.js"; +import { createApp } from "../../src/app/app.js"; +import { authorizeParams } from "../helpers/oauth-flow.js"; + +const SESSION_COOKIE = "better-auth.session_token"; + +const LOGOUT_COOKIES = [ + "better-auth.dont_remember", + "better-auth.session_data", + "better-auth.session_token", +]; + +function readSetCookie(headers) { + if (!headers) return []; + if (typeof headers.getSetCookie === "function") return headers.getSetCookie(); + if (typeof headers.get === "function") { + const value = headers.get("set-cookie"); + return value ? [value] : []; + } + if (Array.isArray(headers)) { + const entry = headers.find(([name]) => name.toLowerCase() === "set-cookie"); + return entry ? [entry[1]] : []; + } + return []; +} + +function cookieNamed(headers, name) { + return readSetCookie(headers).find((cookie) => cookie.startsWith(`${name}=`)); +} + +function attributes(cookie) { + return cookie + .split(";") + .slice(1) + .map((part) => part.trim()); +} + +function attributeValue(cookie, name) { + const part = attributes(cookie).find( + (attribute) => attribute.split("=")[0].toLowerCase() === name.toLowerCase(), + ); + return part?.split("=").slice(1).join("="); +} + +function hasLifetime(cookie) { + return Boolean( + attributeValue(cookie, "Max-Age") || attributeValue(cookie, "Expires"), + ); +} + +async function magicLinkHeaders(auth, getMagicLinks, email) { + const callbackURL = "http://localhost:3000/profile"; + await auth.api.signInMagicLink({ + body: { email, callbackURL }, + headers: new Headers(), + }); + const magicLink = getMagicLinks().find((link) => link.email === email); + if (!magicLink) throw new Error(`No magic link found for ${email}`); + + try { + const result = await auth.api.magicLinkVerify({ + query: { token: magicLink.token, callbackURL }, + headers: new Headers(), + returnHeaders: true, + }); + return result.headers; + } catch (error) { + if (error.statusCode === 302) return error.headers; + throw error; + } +} + +test("session cookie keeps its security attributes on magic-link verification", async (t) => { + const instance = await getTestInstance(t); + const headers = await magicLinkHeaders( + instance.auth, + instance.getMagicLinks, + "cookie-attributes@example.com", + ); + const cookie = cookieNamed(headers, SESSION_COOKIE); + + t.ok(cookie, "session cookie is set"); + t.equal(attributeValue(cookie, "HttpOnly"), "", "HttpOnly is set"); + t.equal(attributeValue(cookie, "SameSite"), "Lax", "SameSite=Lax is set"); + t.equal(attributeValue(cookie, "Path"), "/", "Path=/ is set"); + t.ok(hasLifetime(cookie), "session cookie has a lifetime"); +}); + +test("authorize response sets no cookie", async (t) => { + const instance = await getTestInstance(t); + const app = createApp(instance.auth, instance.db); + const { cookie: sessionCookie } = await instance.getAuthHeaders( + "authorize-cookie@example.com", + ); + + // OAuth state travels in a signed query string (sig=), not in a cookie. + const params = authorizeParams({ state: "authorize-cookie" }); + const res = await app.request( + `/api/auth/oauth2/authorize?${params.toString()}`, + { headers: { cookie: sessionCookie } }, + ); + + t.equal(res.status, 302, "authorize redirects to the callback"); + t.same(readSetCookie(res.headers), [], "no cookie is set"); +}); + +test("logout clears every better-auth cookie", async (t) => { + const instance = await getTestInstance(t); + const app = createApp(instance.auth, instance.db); + const { cookie: sessionCookie } = await instance.getAuthHeaders( + "logout-cookies@example.com", + ); + + const res = await app.request("/logout", { + method: "POST", + headers: { cookie: sessionCookie }, + }); + + t.equal(res.status, 302, "redirects after logout"); + const names = readSetCookie(res.headers) + .map((cookie) => cookie.split("=")[0]) + .sort(); + t.same(names, LOGOUT_COOKIES, "forwards one clearing cookie per cookie"); +}); + +test("logout clearing cookies keep their attributes", async (t) => { + const instance = await getTestInstance(t); + const app = createApp(instance.auth, instance.db); + const { cookie: sessionCookie } = await instance.getAuthHeaders( + "logout-attributes@example.com", + ); + + const res = await app.request("/logout", { + method: "POST", + headers: { cookie: sessionCookie }, + }); + const cookies = readSetCookie(res.headers); + + t.equal(cookies.length, LOGOUT_COOKIES.length, "every cookie is forwarded"); + for (const cookie of cookies) { + const name = cookie.split("=")[0]; + t.equal(attributeValue(cookie, "Max-Age"), "0", `${name} expires now`); + t.equal(attributeValue(cookie, "Path"), "/", `${name} keeps Path=/`); + t.equal(attributeValue(cookie, "HttpOnly"), "", `${name} keeps HttpOnly`); + t.equal( + attributeValue(cookie, "SameSite"), + "Lax", + `${name} keeps SameSite=Lax`, + ); + } +}); From 86fbcf3c218b6ae38841760b39d4be1b45130cdd Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 16:56:40 +0100 Subject: [PATCH 2/3] test: harden cookie attribute helpers readSetCookie returned only the first Set-Cookie entry when headers arrived in array shape, and magicLinkHeaders returned an undefined headers object that surfaced later as a generic assertion failure. Both are test-only diagnostics. --- test/integration/cookie-attributes.test.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/integration/cookie-attributes.test.js b/test/integration/cookie-attributes.test.js index 7ea42ca..b22d664 100644 --- a/test/integration/cookie-attributes.test.js +++ b/test/integration/cookie-attributes.test.js @@ -19,8 +19,9 @@ function readSetCookie(headers) { return value ? [value] : []; } if (Array.isArray(headers)) { - const entry = headers.find(([name]) => name.toLowerCase() === "set-cookie"); - return entry ? [entry[1]] : []; + return headers + .filter(([name]) => name.toLowerCase() === "set-cookie") + .map(([, value]) => value); } return []; } @@ -66,8 +67,13 @@ async function magicLinkHeaders(auth, getMagicLinks, email) { }); return result.headers; } catch (error) { - if (error.statusCode === 302) return error.headers; - throw error; + if (error.statusCode !== 302) throw error; + if (!error.headers) { + throw new Error("magicLinkVerify redirected without headers", { + cause: error, + }); + } + return error.headers; } } From 590a6f5f32ddb96a2492d079c37bbb908f497fb4 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 17:21:33 +0100 Subject: [PATCH 3/3] test: cover logout edge cases Three paths had no coverage: the GET verb on /logout, a Set-Cookie already present on the response before forwardCookies appends to it, and a signOut that returns no headers at all. Each fails when its target is removed. --- test/integration/cookie-attributes.test.js | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/integration/cookie-attributes.test.js b/test/integration/cookie-attributes.test.js index b22d664..bf04c08 100644 --- a/test/integration/cookie-attributes.test.js +++ b/test/integration/cookie-attributes.test.js @@ -1,6 +1,8 @@ import { test } from "tap"; +import { Hono } from "hono"; import { getTestInstance } from "../helpers/test-instance.js"; import { createApp } from "../../src/app/app.js"; +import { forwardCookies } from "../../src/app/utils/cookies.js"; import { authorizeParams } from "../helpers/oauth-flow.js"; const SESSION_COOKIE = "better-auth.session_token"; @@ -156,3 +158,56 @@ test("logout clearing cookies keep their attributes", async (t) => { ); } }); + +test("GET /logout clears every better-auth cookie", async (t) => { + const instance = await getTestInstance(t); + const app = createApp(instance.auth, instance.db); + const { cookie: sessionCookie } = await instance.getAuthHeaders( + "logout-get@example.com", + ); + + const res = await app.request("/logout", { + headers: { cookie: sessionCookie }, + }); + + t.equal(res.status, 302, "redirects after logout"); + const names = readSetCookie(res.headers) + .map((cookie) => cookie.split("=")[0]) + .sort(); + t.same(names, LOGOUT_COOKIES, "forwards one clearing cookie per cookie"); +}); + +test("forwardCookies keeps a Set-Cookie already on the response", async (t) => { + const app = new Hono(); + app.get("/forward", (c) => { + c.header("Set-Cookie", "existing=1; Path=/"); + const headers = new Headers(); + headers.append("set-cookie", "first=1; Path=/; HttpOnly"); + headers.append("set-cookie", "second=2; Path=/; HttpOnly"); + forwardCookies(headers, c); + return c.body(null); + }); + + const cookies = readSetCookie((await app.request("/forward")).headers); + + t.equal(cookies.length, 3, "no header is replaced or duplicated"); + t.match(cookies[0], /^existing=1/, "the existing cookie stays first"); + t.match(cookies[1], /^first=1/, "the first forwarded cookie follows"); + t.match(cookies[2], /^second=2/, "the second forwarded cookie follows"); +}); + +test("logout redirects when signOut returns no headers", async (t) => { + const instance = await getTestInstance(t); + const auth = { + api: { + getSession: async () => null, + signOut: async () => ({}), + }, + }; + const app = createApp(auth, instance.db); + + const res = await app.request("/logout"); + + t.equal(res.status, 302, "redirects despite the missing headers"); + t.same(readSetCookie(res.headers), [], "sets no cookie"); +});