diff --git a/AGENTS.md b/AGENTS.md index 1b98d4e..ffc1260 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,7 +191,7 @@ src/ │ ├── routes/ # Route handlers (auth, home, profile, admin, whoami, health) │ ├── components/ # HTML rendering functions │ ├── handlers/ # Business logic handlers (e.g., logout) -│ ├── utils/ # Utilities (cookies, redirects, magic link email) +│ ├── utils/ # Utilities (redirects, magic link email) │ ├── db/ # Planner OAuth client seeding │ └── demo/ # Demo application showing auth integration ├── auth/ # GitHub provider and ID token claim helpers diff --git a/src/app/handlers/logout.js b/src/app/handlers/logout.js index 13279d4..42b510d 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,20 @@ 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); + // asResponse turns APIErrors into error responses; other errors still throw. + if (!signOut.ok) { + console.error(`Logout failed with status ${signOut.status}`); + return signOut; } - return c.redirect(callbackURL); + const response = c.redirect(callbackURL); + for (const cookie of signOut.headers.getSetCookie()) { + response.headers.append("Set-Cookie", cookie); + } + + return response; }; diff --git a/src/app/utils/cookies.js b/src/app/utils/cookies.js deleted file mode 100644 index cc64a8e..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); - }); -}; diff --git a/test/helpers/test-instance.js b/test/helpers/test-instance.js index 90aeb97..fe5b9f2 100644 --- a/test/helpers/test-instance.js +++ b/test/helpers/test-instance.js @@ -178,49 +178,9 @@ function createTestClient(pool) { */ function createGetAuthHeaders(auth, magicLinksStore) { return async function getAuthHeaders(email) { - // Send magic link - await auth.api.signInMagicLink({ - body: { - email, - callbackURL: "http://localhost:3000/profile", - }, - headers: new Headers(), - }); - - // Get the magic link from the test array - const magicLink = magicLinksStore.find((link) => link.email === email); - - if (!magicLink) { - throw new Error(`No magic link found for ${email}`); - } - - // Verify the magic link to get a session - let result; - try { - result = await auth.api.magicLinkVerify({ - query: { - token: magicLink.token, - callbackURL: "http://localhost:3000/profile", - }, - headers: new Headers(), - returnHeaders: true, - }); - } catch (error) { - // magicLinkVerify may throw an APIError with the redirect response - const cookieFromError = extractCookieFromError(error); - if (cookieFromError) { - return cookieFromError; - } - throw new Error(`Magic link verification failed: ${error.message}`, { - cause: error, - }); - } - - if (!result.headers) { - throw new Error("No headers returned from magic link verification"); - } + const headers = await magicLinkVerifyHeaders(auth, magicLinksStore, email); + const setCookie = setCookieHeader(headers); - const setCookie = result.headers.get("set-cookie"); if (!setCookie) { throw new Error("No session cookie in magic link verification response"); } @@ -230,31 +190,64 @@ function createGetAuthHeaders(auth, magicLinksStore) { } /** - * Extract session cookie from a 302 redirect error response - * @param {Error} error - The error thrown by magicLinkVerify - * @returns {{cookie: string}|null} Session cookie object or null if not found + * Verify a magic link and return the headers carrying the session cookie. + * + * Better Auth returns those headers as a Headers object on success and may + * throw an APIError with them as an array of pairs on the redirect path. + * Both shapes reach this function until the API stabilises. + * @param {Object} auth - Better Auth instance + * @param {Array} magicLinksStore - Array storing captured magic links + * @param {string} email - Address to sign in + * @returns {Headers|Array} Response headers */ -function extractCookieFromError(error) { - if (error.statusCode !== 302) { - return null; - } +export async function magicLinkVerifyHeaders(auth, magicLinksStore, email) { + const callbackURL = "http://localhost:3000/profile"; + await auth.api.signInMagicLink({ + body: { email, callbackURL }, + headers: new Headers(), + }); + + const magicLink = magicLinksStore.find((link) => link.email === email); - // ponytail: Better Auth returns headers in two shapes (Headers object vs - // array) depending on the error path. Handle both until the API stabilises. - let setCookie; - if (error.headers && typeof error.headers.get === "function") { - setCookie = error.headers.get("set-cookie"); - } else if (error.headers && Array.isArray(error.headers)) { - const setCookieHeader = error.headers.find( - (h) => h[0].toLowerCase() === "set-cookie", - ); - setCookie = setCookieHeader ? setCookieHeader[1] : null; + if (!magicLink) { + throw new Error(`No magic link found for ${email}`); } - if (setCookie) { - return parseSessionCookie(setCookie); + try { + const result = await auth.api.magicLinkVerify({ + query: { token: magicLink.token, callbackURL }, + headers: new Headers(), + returnHeaders: true, + }); + + if (!result.headers) { + throw new Error("No headers returned from magic link verification"); + } + + return result.headers; + } catch (error) { + if (error.statusCode !== 302) throw error; + if (!error.headers) { + throw new Error("magicLinkVerify redirected without headers", { + cause: error, + }); + } + return error.headers; } +} +/** + * Read the set-cookie header from either shape Better Auth returns + * @param {Headers|Array} headers - Response headers + * @returns {string|null} The set-cookie header value + */ +function setCookieHeader(headers) { + if (!headers) return null; + if (typeof headers.get === "function") return headers.get("set-cookie"); + if (Array.isArray(headers)) { + const found = headers.find(([name]) => name.toLowerCase() === "set-cookie"); + return found ? found[1] : null; + } return null; } diff --git a/test/integration/cookie-attributes.test.js b/test/integration/cookie-attributes.test.js new file mode 100644 index 0000000..ce3b728 --- /dev/null +++ b/test/integration/cookie-attributes.test.js @@ -0,0 +1,170 @@ +import { test } from "tap"; +import { + getTestInstance, + magicLinkVerifyHeaders, +} 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)) { + return headers + .filter(([name]) => name.toLowerCase() === "set-cookie") + .map(([, value]) => value); + } + 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"), + ); +} + +test("session cookie keeps its security attributes on magic-link verification", async (t) => { + const instance = await getTestInstance(t); + const headers = await magicLinkVerifyHeaders( + 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("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("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`, + ); + } +}); + +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"); +});