From 040edaa3f7ac359398dd1186cb0c9c110f8fa7cb Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 16:45:36 +0100 Subject: [PATCH 1/4] 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/4] 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 c3eb151daab3edeb2444df656e8c17598454a783 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 20:42:01 +0100 Subject: [PATCH 3/4] refactor: forward set-cookie from the signOut response logout copied Set-Cookie values from a Headers object onto the Hono context through a hand-rolled helper. Hono's c.header replaces rather than appends, which is how two of the three clearing cookies were dropped before #103. Ask for the signOut response itself with asResponse: true and append its set-cookie values to the redirect Response. No code in this repo then decides how cookies are forwarded, so a future handler cannot lose headers the same way. forwardCookies and its module are gone. --- src/app/handlers/logout.js | 14 +++++++------- src/app/utils/cookies.js | 7 ------- 2 files changed, 7 insertions(+), 14 deletions(-) delete mode 100644 src/app/utils/cookies.js diff --git a/src/app/handlers/logout.js b/src/app/handlers/logout.js index 13279d4..99b632b 100644 --- a/src/app/handlers/logout.js +++ b/src/app/handlers/logout.js @@ -1,5 +1,4 @@ import { validateRedirectUrl } from "../utils/redirect.js"; -import { forwardCookies } from "../utils/cookies.js"; import { getAuthFromContext } from "../utils/auth.js"; /** @@ -13,14 +12,15 @@ export const logout = async (c) => { c.req.query("redirect_url"), "/?message=Bye!", ); - const status = await auth.api.signOut({ + const signOut = await auth.api.signOut({ headers: c.req.raw.headers, - returnHeaders: true, + asResponse: true, }); - // forward headers (cookies) - if (status.headers) { - forwardCookies(status.headers, c); + + const response = c.redirect(callbackURL); + for (const cookie of signOut.headers.getSetCookie()) { + response.headers.append("Set-Cookie", cookie); } - return c.redirect(callbackURL); + return response; }; diff --git a/src/app/utils/cookies.js b/src/app/utils/cookies.js deleted file mode 100644 index 2f19b1c..0000000 --- a/src/app/utils/cookies.js +++ /dev/null @@ -1,7 +0,0 @@ -// Helper function to forward Set-Cookie headers from better-auth response -export const forwardCookies = (headers, c) => { - const setCookieHeaders = headers.getSetCookie?.() || []; - setCookieHeaders.forEach((cookie) => { - c.header("Set-Cookie", cookie, { append: true }); - }); -}; From 383a0240458df42ae042dc42053cb3123be2de67 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Nov 2026 20:48:26 +0100 Subject: [PATCH 4/4] fix: surface a failed signOut on logout asResponse makes better-auth return APIErrors as error responses instead of throwing them (dispatch.mjs:193,249,258). logout never read the status, so a failed signOut redirected to /?message=Bye! while the session still stood. Return the signOut response and log the status when it is not ok. --- src/app/handlers/logout.js | 5 +++++ test/integration/cookie-attributes.test.js | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/app/handlers/logout.js b/src/app/handlers/logout.js index 99b632b..043629d 100644 --- a/src/app/handlers/logout.js +++ b/src/app/handlers/logout.js @@ -16,6 +16,11 @@ export const logout = async (c) => { headers: c.req.raw.headers, asResponse: true, }); + // asResponse makes better-auth return APIErrors instead of throwing them. + if (!signOut.ok) { + console.error(`Logout failed with status ${signOut.status}`); + return signOut; + } const response = c.redirect(callbackURL); for (const cookie of signOut.headers.getSetCookie()) { diff --git a/test/integration/cookie-attributes.test.js b/test/integration/cookie-attributes.test.js index b22d664..514a64e 100644 --- a/test/integration/cookie-attributes.test.js +++ b/test/integration/cookie-attributes.test.js @@ -156,3 +156,21 @@ test("logout clearing cookies keep their attributes", async (t) => { ); } }); + +test("logout surfaces a failed signOut instead of redirecting", async (t) => { + const auth = { + api: { + getSession: async () => null, + signOut: async () => + new Response(JSON.stringify({ message: "signOut failed" }), { + status: 500, + }), + }, + }; + const app = createApp(auth, null); + + const res = await app.request("/logout"); + + t.equal(res.status, 500, "propagates the signOut status"); + t.equal(res.headers.get("location"), null, "does not redirect"); +});