From fa569b4c3df54b515e1307ff30b4378c968dd3e1 Mon Sep 17 00:00:00 2001 From: Taesu Date: Thu, 17 Sep 2026 18:17:55 +0900 Subject: [PATCH] fix(electron): honor cookie security policies --- docs/content/docs/integrations/electron.mdx | 10 ++ .../better-auth/src/cookies/cookies.test.ts | 19 ++++ packages/electron/src/cookies.ts | 10 ++ packages/electron/src/index.ts | 38 +++++-- packages/electron/src/proxy.ts | 39 ++++++-- packages/electron/src/types/client.ts | 6 +- packages/electron/test/electron.test.ts | 99 ++++++++++++++++++- 7 files changed, 200 insertions(+), 21 deletions(-) diff --git a/docs/content/docs/integrations/electron.mdx b/docs/content/docs/integrations/electron.mdx index a6b2c7d9f6..cabe186c7f 100644 --- a/docs/content/docs/integrations/electron.mdx +++ b/docs/content/docs/integrations/electron.mdx @@ -664,6 +664,16 @@ electronProxyClient({ }); ``` +#### `cookieSecurity?` + +Security policy used to read the redirect cookie. Match the server's explicit `advanced.cookieSecurity` value. + +```ts title="web/auth-client.ts" +electronProxyClient({ + cookieSecurity: "host", +}); +``` + ### Client plugin #### `signInURL` diff --git a/packages/better-auth/src/cookies/cookies.test.ts b/packages/better-auth/src/cookies/cookies.test.ts index ef61cfd446..99d254dcd0 100644 --- a/packages/better-auth/src/cookies/cookies.test.ts +++ b/packages/better-auth/src/cookies/cookies.test.ts @@ -1772,6 +1772,25 @@ describe("Cookie Cache Field Filtering", () => { }); describe("Cookie Chunking", () => { + /** + * @see https://developer.mozilla.org/en-US/docs/Web/Privacy/Privacy_sandbox/Partitioned_cookies + */ + it("counts security attributes added during serialization", () => { + const options = { partitioned: true, secure: false }; + const name = "better-auth.session_data.99"; + const maxCookieSize = 4050; + const overhead = serializeCookie(name, "", options).length; + const cookie = serializeCookie( + name, + "x".repeat(maxCookieSize - overhead), + options, + ); + + expect(cookie).toHaveLength(maxCookieSize); + expect(cookie).toContain("; Secure; Partitioned"); + expect(options).toEqual({ partitioned: true, secure: false }); + }); + it("should chunk cookies when they exceed 4KB", async () => { // Create a large string that will exceed the cookie size limit const largeString = "x".repeat(2000); diff --git a/packages/electron/src/cookies.ts b/packages/electron/src/cookies.ts index 6ce075664f..2483d1848e 100644 --- a/packages/electron/src/cookies.ts +++ b/packages/electron/src/cookies.ts @@ -1,6 +1,9 @@ +import type { CookieSecurity } from "@better-auth/core"; import { cookieNameRegex, + HOST_COOKIE_PREFIX, parseSetCookieHeader, + SECURE_COOKIE_PREFIX, stripCookieSecurityPrefix, } from "better-auth/cookies"; @@ -9,6 +12,13 @@ interface StoredCookie { expires: string | null; } +export function getCookieSecurity(cookieName: string): CookieSecurity { + const name = cookieName.toLowerCase(); + if (name.startsWith(HOST_COOKIE_PREFIX.toLowerCase())) return "host"; + if (name.startsWith(SECURE_COOKIE_PREFIX.toLowerCase())) return "secure"; + return "none"; +} + export function getSetCookie(header: string, prevCookie?: string | undefined) { const parsed = parseSetCookieHeader(header); let toSetCookie: Record = {}; diff --git a/packages/electron/src/index.ts b/packages/electron/src/index.ts index 056d3d283d..5eeafd9a9c 100644 --- a/packages/electron/src/index.ts +++ b/packages/electron/src/index.ts @@ -10,7 +10,10 @@ import { base64Url } from "@better-auth/utils/base64"; import type { BetterAuthPlugin } from "better-auth"; import { safeJSONParse } from "better-auth"; import { generateRandomString } from "better-auth/crypto"; +import type { CookieOptions } from "better-call"; +import { getCookieKey } from "better-call"; import * as z from "zod"; +import { getCookieSecurity } from "./cookies"; import { ELECTRON_ERROR_CODES } from "./error-codes"; import { electronInitOAuthProxy, @@ -45,6 +48,21 @@ export const electron = (options?: ElectronOptions | undefined) => { cookieNamespace: options?.cookieNamespace ?? options?.cookiePrefix ?? "better-auth", }; + const createElectronCookie = ( + ctx: GenericEndpointContext, + name: string, + attributes?: Partial, + ) => { + const cookie = ctx.context.createAuthCookie(name, attributes); + const security = getCookieSecurity(cookie.name); + const logicalName = `${opts.cookieNamespace}.${name}`; + return { + ...cookie, + name: + getCookieKey(logicalName, security === "none" ? undefined : security) ?? + logicalName, + }; + }; const hookMatcher = (ctx: HookEndpointContext) => { return !!( @@ -81,8 +99,6 @@ export const electron = (options?: ElectronOptions | undefined) => { throw APIError.from("BAD_REQUEST", ELECTRON_ERROR_CODES.MISSING_PKCE); } - const redirectCookieName = `${opts.cookieNamespace}.${opts.clientID}`; - const identifier = generateRandomString(32, "a-z", "A-Z", "0-9"); const codeExpiresInMs = opts.codeExpiresIn * 1000; const expiresAt = new Date(Date.now() + codeExpiresInMs); @@ -100,11 +116,15 @@ export const electron = (options?: ElectronOptions | undefined) => { new TextEncoder().encode(JSON.stringify({ identifier, state })), ); - ctx.setCookie(redirectCookieName, redirectToken, { - ...ctx.context.authCookies.sessionToken.attributes, + const redirectCookie = createElectronCookie(ctx, opts.clientID, { maxAge: opts.redirectCookieExpiresIn, httpOnly: false, }); + ctx.setCookie( + redirectCookie.name, + redirectToken, + redirectCookie.attributes, + ); return identifier; }; @@ -117,17 +137,17 @@ export const electron = (options?: ElectronOptions | undefined) => { { matcher: (ctx) => !hookMatcher(ctx), handler: createAuthMiddleware(async (ctx) => { + const cookie = createElectronCookie(ctx, "transfer_token", { + maxAge: opts.codeExpiresIn, + }); const transferCookie = await ctx.getSignedCookie( - `${opts.cookieNamespace}.transfer_token`, + cookie.name, ctx.context.secret, ); if (!ctx.context.newSession?.session || !transferCookie) { return; } - const cookie = ctx.context.createAuthCookie("transfer_token", { - maxAge: opts.codeExpiresIn, - }); // Refresh the transfer cookie to extend its validity // Avoids expiration during multi-step auth flows on active usage // Can still expire when no endpoint is hit within the valid period @@ -147,7 +167,7 @@ export const electron = (options?: ElectronOptions | undefined) => { code_challenge: z.string().nonempty(), state: z.string().nonempty(), }); - const cookie = ctx.context.createAuthCookie("transfer_token", { + const cookie = createElectronCookie(ctx, "transfer_token", { maxAge: opts.codeExpiresIn, }); if ( diff --git a/packages/electron/src/proxy.ts b/packages/electron/src/proxy.ts index 58665b33ee..6330db0fa4 100644 --- a/packages/electron/src/proxy.ts +++ b/packages/electron/src/proxy.ts @@ -1,5 +1,7 @@ -import type { BetterAuthClientPlugin } from "@better-auth/core"; +import type { BetterAuthClientPlugin, CookieSecurity } from "@better-auth/core"; import { parseCookies } from "better-auth/cookies"; +import { getCookieKey } from "better-call"; +import { getCookieSecurity } from "./cookies"; import type { electron } from "./index"; import type { ElectronProxyClientOptions } from "./types/client"; import { parseProtocolScheme } from "./utils"; @@ -22,20 +24,33 @@ export const electronProxyClient = (options: ElectronProxyClientOptions) => { options.cookieNamespace ?? options.cookiePrefix ?? "better-auth", }; const redirectCookieName = `${opts.cookieNamespace}.${opts.clientID}`; + const resolveRedirectCookieName = (security: CookieSecurity) => + getCookieKey( + redirectCookieName, + security === "none" ? undefined : security, + ) ?? redirectCookieName; + const redirectCookieNames = + opts.cookieSecurity !== undefined + ? [resolveRedirectCookieName(opts.cookieSecurity)] + : [resolveRedirectCookieName("secure"), redirectCookieName]; const { scheme } = parseProtocolScheme(opts.protocol); return { id: "electron-proxy", version: PACKAGE_VERSION, getActions: () => { - const getAuthorizationCode = () => { + const getAuthorizationCookie = () => { if (typeof document === "undefined") return null; - const authorizationCode = parseCookies(document.cookie).get( - redirectCookieName, - ); - return authorizationCode ?? null; + const cookies = parseCookies(document.cookie); + for (const name of redirectCookieNames) { + const value = cookies.get(name); + if (value) return { name, value }; + } + return null; }; + const getAuthorizationCode = () => + getAuthorizationCookie()?.value ?? null; return { electron: { @@ -69,14 +84,18 @@ export const electronProxyClient = (options: ElectronProxyClientOptions) => { const interval = cfg?.interval || 100; const handleRedirect = () => { - const authorizationCode = getAuthorizationCode(); - if (!authorizationCode) { + const authorizationCookie = getAuthorizationCookie(); + if (!authorizationCookie) { return false; } - document.cookie = `${redirectCookieName}=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/`; + const secure = + getCookieSecurity(authorizationCookie.name) === "none" + ? "" + : "; Secure"; + document.cookie = `${authorizationCookie.name}=; Max-Age=0; Path=/${secure}`; window.location.replace( - `${scheme}:/${opts.callbackPath}#token=${authorizationCode}`, + `${scheme}:/${opts.callbackPath}#token=${authorizationCookie.value}`, ); return true; }; diff --git a/packages/electron/src/types/client.ts b/packages/electron/src/types/client.ts index a17b1b217f..f912889efa 100644 --- a/packages/electron/src/types/client.ts +++ b/packages/electron/src/types/client.ts @@ -1,4 +1,4 @@ -import type { Awaitable } from "@better-auth/core"; +import type { Awaitable, CookieSecurity } from "@better-auth/core"; import type { User } from "@better-auth/core/db"; import type { ElectronSharedOptions } from "./options"; @@ -155,6 +155,10 @@ export interface ElectronProxyClientOptions * This option will be removed in a future minor release. */ cookiePrefix?: string | undefined; + /** + * Match the server's explicit `advanced.cookieSecurity` setting. + */ + cookieSecurity?: CookieSecurity | undefined; } export type * from "../authenticate"; diff --git a/packages/electron/test/electron.test.ts b/packages/electron/test/electron.test.ts index a8ffd2affe..79e96ad5e9 100644 --- a/packages/electron/test/electron.test.ts +++ b/packages/electron/test/electron.test.ts @@ -2,15 +2,18 @@ import { randomBytes } from "node:crypto"; import { createAuthMiddleware } from "@better-auth/core/api"; import { BetterAuthError } from "@better-auth/core/error"; import { base64Url } from "@better-auth/utils/base64"; +import { betterAuth } from "better-auth"; import { createAuthClient } from "better-auth/client"; import { parseSetCookieHeader } from "better-auth/cookies"; import { generateRandomString } from "better-auth/crypto"; import { getMigrations } from "better-auth/db/migration"; import { generateCodeChallenge } from "better-auth/oauth2"; +import { oAuthProxy } from "better-auth/plugins"; +import Database from "better-sqlite3"; import { beforeEach, describe, expect, vi } from "vitest"; import { authenticate, kElectron } from "../src/authenticate"; import { electronClient } from "../src/client"; -import { getCookie } from "../src/cookies"; +import { getCookie, getCookieSecurity } from "../src/cookies"; import { ELECTRON_ERROR_CODES } from "../src/error-codes"; import { electron } from "../src/index"; import { electronProxyClient } from "../src/proxy"; @@ -105,6 +108,100 @@ describe("Electron", () => { ).toThrow("either cookieNamespace or cookiePrefix"); }); + describe("host cookie security", () => { + it.each([ + ["electron-auth.electron", "none"], + ["__Secure-electron-auth.electron", "secure"], + ["__secure-electron-auth.electron", "secure"], + ["__Host-electron-auth.electron", "host"], + ["__HOST-electron-auth.electron", "host"], + ] as const)("detects %s as %s", (name, security) => { + expect(getCookieSecurity(name)).toBe(security); + }); + + it("applies the host prefix to transfer and redirect cookies", async () => { + const hostAuth = betterAuth({ + baseURL: "https://localhost:3000", + database: new Database(":memory:"), + emailAndPassword: { enabled: true }, + advanced: { cookieSecurity: "host" }, + plugins: [electron({ cookieNamespace: "electron-auth" }), oAuthProxy()], + trustedOrigins: ["com.example.app:/"], + }); + const { runMigrations } = await getMigrations(hostAuth.options); + await runMigrations(); + const hostProxyClient = createAuthClient({ + baseURL: "https://localhost:3000", + fetchOptions: { + customFetchImpl: (url, init) => + hostAuth.handler(new Request(url.toString(), init)), + }, + plugins: [ + electronProxyClient({ + protocol: "com.example.app", + cookieNamespace: "electron-auth", + cookieSecurity: "host", + }), + ], + }); + const codeVerifier = "host-cookie-verifier"; + const codeChallenge = await s256Challenge(codeVerifier); + let cookieNames: string[] = []; + + const { error } = await hostProxyClient.signUp.email( + { + email: "host-cookie@test.com", + password: "password", + name: "Host Cookie", + }, + { + query: { + client_id: "electron", + code_challenge: codeChallenge, + state: "host-state", + }, + onResponse({ response }) { + cookieNames = [ + ...parseSetCookieHeader( + response.headers.get("set-cookie") ?? "", + ).keys(), + ]; + }, + }, + ); + + expect(error).toBeNull(); + expect(cookieNames).toContain("__Host-electron-auth.transfer_token"); + expect(cookieNames).toContain("__Host-electron-auth.electron"); + expect(cookieNames).not.toContain("electron-auth.transfer_token"); + expect(cookieNames).not.toContain("electron-auth.electron"); + }); + + it("reads only the configured redirect cookie security mode", () => { + const plugin = electronProxyClient({ + protocol: "com.example.app", + cookieNamespace: "electron-auth", + cookieSecurity: "host", + }); + const actions = plugin.getActions(); + vi.stubGlobal("document", { + cookie: + "electron-auth.electron=attacker; __Secure-electron-auth.electron=attacker; __Host-electron-auth.electron=valid", + }); + + try { + expect(actions.electron.getAuthorizationCode()).toBe("valid"); + vi.stubGlobal("document", { + cookie: + "electron-auth.electron=attacker; __Secure-electron-auth.electron=attacker", + }); + expect(actions.electron.getAuthorizationCode()).toBeNull(); + } finally { + vi.unstubAllGlobals(); + } + }); + }); + async function s256Challenge(verifier: string) { return generateCodeChallenge(verifier); }