diff --git a/.changeset/fiery-guests-visit.md b/.changeset/fiery-guests-visit.md new file mode 100644 index 0000000000..37ed8cdd83 --- /dev/null +++ b/.changeset/fiery-guests-visit.md @@ -0,0 +1,10 @@ +--- +"better-auth": minor +"@better-auth/electron": minor +--- + +Harden the Electron OAuth flow and tighten custom-scheme trusted-origin matching. + +The Electron sign-in flow now mandates PKCE S256. Plain PKCE is rejected: the `code_challenge_method` parameter is gone and every authorization code is verified by hashing the verifier with SHA-256. The server no longer trusts an `electron-origin` header to set the request Origin. The Electron client now sends a real `Origin` (for example `myapp:/`), so upgrade the `@better-auth/electron` client and server together and make sure your app's scheme is in `trustedOrigins`. The unused `disableOriginOverride` option is removed. + +Custom-scheme entries in `trustedOrigins` now match by scheme and authority instead of string prefix. A host-less entry such as `myapp://` or `exp://` still trusts every host of that scheme, but a host-bearing entry such as `myapp://callback` matches that host exactly, so it is no longer satisfied by `myapp://callback.attacker.tld`. diff --git a/docs/content/docs/integrations/electron.mdx b/docs/content/docs/integrations/electron.mdx index c6e78efdda..8c250998cd 100644 --- a/docs/content/docs/integrations/electron.mdx +++ b/docs/content/docs/integrations/electron.mdx @@ -292,7 +292,6 @@ function SignIn({ client_id?: string | undefined; state?: string | undefined; code_challenge?: string | undefined; - code_challenge_method?: string | undefined; }>; }) { const query = use(searchParams); @@ -601,18 +600,6 @@ electron({ }); ``` -#### `disableOriginOverride?` - -Override the origin for Electron API routes. (defaults to `false`) - -Enable this if you're facing cors origin issues with Electron API routes. - -```ts title="web/auth.ts" -electron({ - disableOriginOverride: true, -}); -``` - ### Proxy client #### `protocol` diff --git a/docs/content/docs/integrations/expo.mdx b/docs/content/docs/integrations/expo.mdx index 7cf39f18a3..0e63cb135c 100644 --- a/docs/content/docs/integrations/expo.mdx +++ b/docs/content/docs/integrations/expo.mdx @@ -176,7 +176,7 @@ Expo is a popular framework for building cross-platform apps with React Native. // Development mode - Expo's exp:// scheme with local IP ranges ...(process.env.NODE_ENV === "development" ? [ - "exp://", // Trust all Expo URLs (prefix matching) + "exp://", // Trust any host of the exp:// scheme "exp://**", // Trust all Expo URLs (wildcard matching) "exp://192.168.*.*:*/**", // Trust 192.168.x.x IP range with any port and path ] : []) diff --git a/docs/content/docs/reference/options.mdx b/docs/content/docs/reference/options.mdx index d5ddfd0734..135fe43c03 100644 --- a/docs/content/docs/reference/options.mdx +++ b/docs/content/docs/reference/options.mdx @@ -156,7 +156,7 @@ export const auth = betterAuth({ **Notes** * For `http://` and `https://` URLs, patterns match the full origin. Paths and query strings are ignored. - * For custom schemes (like `exp://` or `myapp://`), patterns match against the full URL including paths when wildcards are present, or use prefix matching when no wildcards exist. + * For custom schemes (like `exp://` or `myapp://`), wildcard patterns match against the full URL including paths. A non-wildcard pattern matches by scheme and authority: a host-less entry such as `myapp://` or `exp://` trusts every host of that scheme, while a host-bearing entry such as `myapp://callback` must match that host exactly (so it is not satisfied by `myapp://callback.attacker.tld`). * The separator is `/` (forward slash). This means `http://*.example.com` matches both `http://api.example.com` and `http://api.app.example.com`. diff --git a/packages/better-auth/src/auth/trusted-origins.test.ts b/packages/better-auth/src/auth/trusted-origins.test.ts index 7acbbd4899..c09808d078 100644 --- a/packages/better-auth/src/auth/trusted-origins.test.ts +++ b/packages/better-auth/src/auth/trusted-origins.test.ts @@ -297,6 +297,133 @@ describe("trusted origins", () => { }); }); + describe("custom-scheme origin matching", () => { + it("should reject URLs where the host extends the trusted pattern", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://callback"], + }); + + await expect( + isTrustedOrigin("myapp://callback.attacker.tld"), + ).resolves.toBe(false); + + await expect( + isTrustedOrigin("myapp://callback.attacker.tld/path"), + ).resolves.toBe(false); + }); + + it("should allow exact custom-scheme origin match", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://callback"], + }); + + await expect(isTrustedOrigin("myapp://callback")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp://callback/")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp://callback/path")).resolves.toBe( + true, + ); + // A query or fragment directly on the host is not part of the + // authority, so the host still matches exactly. + await expect(isTrustedOrigin("myapp://callback?token=x")).resolves.toBe( + true, + ); + await expect(isTrustedOrigin("myapp://callback#frag")).resolves.toBe( + true, + ); + }); + + it("should match custom-scheme with empty host (myapp:/)", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp:/"], + }); + + await expect(isTrustedOrigin("myapp:/")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp://")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp:/auth/callback")).resolves.toBe(true); + }); + + it("should reject different custom schemes", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://callback"], + }); + + await expect(isTrustedOrigin("otherapp://callback")).resolves.toBe(false); + }); + + it("should still support wildcard matching for custom schemes", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["exp://192.168.*.*:*/*"], + }); + + await expect( + isTrustedOrigin("exp://192.168.1.100:8081/--/"), + ).resolves.toBe(true); + + await expect(isTrustedOrigin("exp://10.0.0.1:8081/--/")).resolves.toBe( + false, + ); + }); + + it("should trust any host for a host-less custom-scheme pattern", async () => { + // The Expo plugin registers a bare `exp://` in development to trust + // every Expo Go dev origin, and apps register `myapp://` to trust any + // deep link. A host-less pattern matches any host of the scheme. + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["exp://", "myapp://"], + }); + + await expect(isTrustedOrigin("exp://192.168.1.5:8081/--/")).resolves.toBe( + true, + ); + await expect(isTrustedOrigin("exp://localhost:8081/--/")).resolves.toBe( + true, + ); + await expect(isTrustedOrigin("myapp://callback")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp://other-host/path")).resolves.toBe( + true, + ); + await expect(isTrustedOrigin("evil://anything")).resolves.toBe(false); + }); + + it("should match custom-scheme host case-insensitively", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://callback"], + }); + + await expect(isTrustedOrigin("myapp://CALLBACK")).resolves.toBe(true); + await expect(isTrustedOrigin("MyApp://Callback")).resolves.toBe(true); + }); + + it("should pin the path when the pattern includes one", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://host/cb"], + }); + + await expect(isTrustedOrigin("myapp://host/cb")).resolves.toBe(true); + await expect(isTrustedOrigin("myapp://host/cb/extra")).resolves.toBe( + true, + ); + await expect(isTrustedOrigin("myapp://host/cbx")).resolves.toBe(false); + await expect(isTrustedOrigin("myapp://host/other")).resolves.toBe(false); + }); + + it("should not let a path-pinned pattern be bypassed with traversal", async () => { + const { isTrustedOrigin } = await createAuthTestInstance({ + trustedOrigins: ["myapp://host/cb"], + }); + + await expect(isTrustedOrigin("myapp://host/cb/../evil")).resolves.toBe( + false, + ); + await expect( + isTrustedOrigin("myapp://host/cb/%2e%2e/evil"), + ).resolves.toBe(false); + await expect( + isTrustedOrigin("myapp://host/cb%2f..%2fevil"), + ).resolves.toBe(false); + }); + }); + describe("dynamic trusted origins", () => { it("should allow dynamically computed trusted origins", async () => { const { isTrustedOrigin } = await createAuthTestInstance({ diff --git a/packages/better-auth/src/auth/trusted-origins.ts b/packages/better-auth/src/auth/trusted-origins.ts index 0ceb8a2886..05dd101c5a 100644 --- a/packages/better-auth/src/auth/trusted-origins.ts +++ b/packages/better-auth/src/auth/trusted-origins.ts @@ -1,6 +1,68 @@ import { getHost, getOrigin, getProtocol } from "../utils/url"; import { wildcardMatch } from "../utils/wildcard"; +/** + * Resolves `.` and `..` segments in a path after percent-decoding so a + * path-pinned pattern cannot be bypassed with traversal: e.g. + * `myapp://host/cb/../evil` must not satisfy pattern `myapp://host/cb`. + * Returns "" for an empty or root path. + */ +const normalizePath = (path: string): string => { + let decoded = path; + try { + decoded = decodeURIComponent(path); + } catch { + // Not valid percent-encoding; fall back to the raw path. + } + const segments: string[] = []; + for (const segment of decoded.split("/")) { + if (segment === "..") { + segments.pop(); + } else if (segment !== "." && segment !== "") { + segments.push(segment); + } + } + return segments.length > 0 ? `/${segments.join("/")}` : ""; +}; + +/** + * Splits a custom-scheme origin into its scheme, authority and path using + * plain string operations. + * + * `new URL()` is deliberately avoided here: its parsing of non-special schemes + * (e.g. `myapp://`, `exp://`) is not consistent across the runtimes Better Auth + * targets (Node, Bun, Deno, Cloudflare Workers), and the result of an origin + * check must not depend on which engine extracts the authority. + * + * Scheme and authority are lower-cased (matching how a URL canonicalizes its + * host); the path is percent-decoded and resolved so traversal cannot bypass + * a path-pinned pattern. + */ +const parseCustomSchemeOrigin = (value: string) => { + const schemeEnd = value.indexOf(":"); + if (schemeEnd <= 0) { + return null; + } + const scheme = value.slice(0, schemeEnd).toLowerCase(); + let rest = value.slice(schemeEnd + 1); + let authority = ""; + if (rest.startsWith("//")) { + rest = rest.slice(2); + // The authority ends at the first "/", "?" or "#" (RFC 3986); the + // remainder is the path, with any query/fragment stripped below. + const authorityEnd = rest.search(/[/?#]/); + if (authorityEnd === -1) { + authority = rest; + rest = ""; + } else { + authority = rest.slice(0, authorityEnd); + rest = rest.slice(authorityEnd); + } + } + const path = normalizePath(rest.replace(/[?#].*$/, "")); + return { scheme, authority: authority.toLowerCase(), path }; +}; + /** * Matches the given url against an origin or origin pattern * See "options.trustedOrigins" for details of supported patterns @@ -41,7 +103,29 @@ export const matchesOriginPattern = ( return wildcardMatch(pattern)(host); } const protocol = getProtocol(url); - return protocol === "http:" || protocol === "https:" || !protocol - ? pattern === getOrigin(url) - : url.startsWith(pattern); + if (protocol === "http:" || protocol === "https:" || !protocol) { + return pattern === getOrigin(url); + } + // Custom schemes (e.g. myapp://, exp://). A pattern matches by scheme and, + // when it pins a host, by exact authority, so "myapp://callback" is not + // satisfied by "myapp://callback.attacker.tld". A host-less pattern + // ("myapp://", "exp://", "myapp:/") trusts every host of the scheme, since + // for custom schemes the OS-registered scheme is the trust boundary. + const parsed = parseCustomSchemeOrigin(url); + const parsedPattern = parseCustomSchemeOrigin(pattern); + if (!parsed || !parsedPattern || parsed.scheme !== parsedPattern.scheme) { + return false; + } + if (parsedPattern.authority && parsed.authority !== parsedPattern.authority) { + return false; + } + // A pattern without a path trusts every path; otherwise the url path must + // equal the pattern path or be nested beneath it. + if (!parsedPattern.path) { + return true; + } + return ( + parsed.path === parsedPattern.path || + parsed.path.startsWith(`${parsedPattern.path}/`) + ); }; diff --git a/packages/electron/src/authenticate.ts b/packages/electron/src/authenticate.ts index fd7babb3b7..60c9573b01 100644 --- a/packages/electron/src/authenticate.ts +++ b/packages/electron/src/authenticate.ts @@ -2,11 +2,11 @@ import type { BetterAuthClientOptions } from "@better-auth/core"; import type { User } from "@better-auth/core/db"; import { BetterAuthError } from "@better-auth/core/error"; import { base64Url } from "@better-auth/utils/base64"; -import { createHash } from "@better-auth/utils/hash"; import type { BetterFetch, CreateFetchOption } from "@better-fetch/fetch"; import { APIError, getBaseURL, safeJSONParse } from "better-auth"; import { signInSocial } from "better-auth/api"; import { generateRandomString } from "better-auth/crypto"; +import { generateCodeChallenge } from "better-auth/oauth2"; import { shell } from "electron"; import * as z from "zod"; import type { ElectronClientOptions } from "./types/client"; @@ -45,10 +45,8 @@ export async function requestAuth( const state = generateRandomString(16, "A-Z", "a-z", "0-9"); - const codeVerifier = base64Url.encode(randomBytes(32)); - const codeChallenge = base64Url.encode( - await createHash("SHA-256").digest(codeVerifier), - ); + const codeVerifier = base64Url.encode(randomBytes(32), { padding: false }); + const codeChallenge = await generateCodeChallenge(codeVerifier); ((globalThis as any)[kElectron] ??= new Map()).set( state, @@ -83,7 +81,6 @@ export async function requestAuth( } url.searchParams.set("client_id", options.clientID || "electron"); url.searchParams.set("code_challenge", codeChallenge); - url.searchParams.set("code_challenge_method", "S256"); url.searchParams.set("state", state); await shell.openExternal(url.toString(), { diff --git a/packages/electron/src/client.ts b/packages/electron/src/client.ts index 679ea4c657..2b673593c4 100644 --- a/packages/electron/src/client.ts +++ b/packages/electron/src/client.ts @@ -108,10 +108,10 @@ export const electronClient = (options: O) => { options ||= {}; options.credentials = "omit"; options.headers = { + origin: `${scheme}:/`, ...options.headers, cookie, "user-agent": app.userAgentFallback, - "electron-origin": `${scheme}:/`, "x-skip-oauth-proxy": "true", }; diff --git a/packages/electron/src/index.ts b/packages/electron/src/index.ts index a6fc9dd088..72bc358f86 100644 --- a/packages/electron/src/index.ts +++ b/packages/electron/src/index.ts @@ -57,15 +57,9 @@ export const electron = (options?: ElectronOptions | undefined) => { client_id: string; state: string; code_challenge: string; - code_challenge_method?: string | undefined; }, ) => { - const { - client_id, - state, - code_challenge, - code_challenge_method = "plain", - } = payload; + const { client_id, state, code_challenge } = payload; const userId = ctx.context.session?.user.id || ctx.context.newSession?.user.id; if (!userId || client_id !== opts.clientID) { @@ -88,7 +82,6 @@ export const electron = (options?: ElectronOptions | undefined) => { value: JSON.stringify({ userId, codeChallenge: code_challenge, - codeChallengeMethod: code_challenge_method.toLowerCase(), state, }), expiresAt, @@ -110,22 +103,6 @@ export const electron = (options?: ElectronOptions | undefined) => { return { id: "electron", version: PACKAGE_VERSION, - async onRequest(request, _ctx) { - if (opts.disableOriginOverride || request.headers.get("origin")) { - return; - } - - const electronOrigin = request.headers.get("electron-origin"); - if (!electronOrigin) { - return; - } - - const req = request.clone(); - req.headers.set("origin", electronOrigin); - return { - request: req, - }; - }, hooks: { after: [ { @@ -159,7 +136,6 @@ export const electron = (options?: ElectronOptions | undefined) => { const querySchema = z.object({ client_id: z.string(), code_challenge: z.string().nonempty(), - code_challenge_method: z.string().optional().default("plain"), state: z.string().nonempty(), }); const cookie = ctx.context.createAuthCookie("transfer_token", { diff --git a/packages/electron/src/routes.ts b/packages/electron/src/routes.ts index 4bb46166fa..d622c80bcf 100644 --- a/packages/electron/src/routes.ts +++ b/packages/electron/src/routes.ts @@ -1,19 +1,22 @@ -import { Buffer } from "node:buffer"; -import { timingSafeEqual } from "node:crypto"; import type { GenericEndpointContext } from "@better-auth/core"; import { APIError, BASE_ERROR_CODES } from "@better-auth/core/error"; import { safeJSONParse } from "@better-auth/core/utils/json"; -import { base64Url } from "@better-auth/utils/base64"; -import { createHash } from "@better-auth/utils/hash"; import { betterFetch } from "@better-fetch/fetch"; import { createAuthEndpoint, sessionMiddleware } from "better-auth/api"; import { setSessionCookie } from "better-auth/cookies"; import type { User } from "better-auth/db"; import { parseUserOutput } from "better-auth/db"; +import { generateCodeChallenge } from "better-auth/oauth2"; import * as z from "zod"; import { ELECTRON_ERROR_CODES } from "./error-codes"; import type { ElectronOptions } from "./types"; +type ElectronVerificationValue = { + userId: string; + codeChallenge: string; + state: string; +}; + const electronTokenBodySchema = z.object({ token: z.string().nonempty(), state: z.string().nonempty(), @@ -64,7 +67,7 @@ export const electronToken = (_opts: ElectronOptions) => throw APIError.from("NOT_FOUND", ELECTRON_ERROR_CODES.INVALID_TOKEN); } - const tokenRecord = safeJSONParse>(token.value); + const tokenRecord = safeJSONParse(token.value); if (!tokenRecord) { throw APIError.from( "INTERNAL_SERVER_ERROR", @@ -82,30 +85,14 @@ export const electronToken = (_opts: ElectronOptions) => ELECTRON_ERROR_CODES.MISSING_CODE_CHALLENGE, ); } - if (tokenRecord.codeChallengeMethod === "s256") { - const codeChallenge = Buffer.from( - base64Url.decode(tokenRecord.codeChallenge), + // PKCE is always S256: the stored challenge is the SHA-256 digest of + // the verifier, so a legacy or plaintext challenge fails this check. + const codeChallenge = await generateCodeChallenge(ctx.body.code_verifier); + if (codeChallenge !== tokenRecord.codeChallenge) { + throw APIError.from( + "BAD_REQUEST", + ELECTRON_ERROR_CODES.INVALID_CODE_VERIFIER, ); - const codeVerifier = Buffer.from( - await createHash("SHA-256").digest(ctx.body.code_verifier), - ); - - if ( - codeChallenge.length !== codeVerifier.length || - !timingSafeEqual(codeChallenge, codeVerifier) - ) { - throw APIError.from( - "BAD_REQUEST", - ELECTRON_ERROR_CODES.INVALID_CODE_VERIFIER, - ); - } - } else { - if (tokenRecord.codeChallenge !== ctx.body.code_verifier) { - throw APIError.from( - "BAD_REQUEST", - ELECTRON_ERROR_CODES.INVALID_CODE_VERIFIER, - ); - } } await ctx.context.internalAdapter.deleteVerificationByIdentifier( `electron:${ctx.body.token}`, @@ -146,7 +133,6 @@ const electronInitOAuthProxyQuerySchema = z.object({ provider: z.string().nonempty(), state: z.string(), code_challenge: z.string(), - code_challenge_method: z.string().optional(), }); export const electronInitOAuthProxy = (opts: ElectronOptions) => @@ -200,10 +186,6 @@ export const electronInitOAuthProxy = (opts: ElectronOptions) => const searchParams = new URLSearchParams(); searchParams.set("client_id", opts.clientID || "electron"); searchParams.set("code_challenge", ctx.query.code_challenge); - searchParams.set( - "code_challenge_method", - ctx.query.code_challenge_method || "plain", - ); searchParams.set("state", ctx.query.state); const res = await betterFetch<{ url: string | undefined; @@ -245,7 +227,6 @@ const electronTransferUserQuerySchema = z.object({ client_id: z.string(), state: z.string(), code_challenge: z.string(), - code_challenge_method: z.string().optional(), }); const electronTransferUserBodySchema = z.object({ callbackURL: z.string().optional(), @@ -262,7 +243,6 @@ export const electronTransferUser = ( client_id: string; state: string; code_challenge: string; - code_challenge_method?: string | undefined; }, ) => Promise; }, diff --git a/packages/electron/src/types/options.ts b/packages/electron/src/types/options.ts index bc06008725..caa14ca72f 100644 --- a/packages/electron/src/types/options.ts +++ b/packages/electron/src/types/options.ts @@ -26,11 +26,4 @@ export interface ElectronOptions extends ElectronSharedOptions { * @default "better-auth" */ cookiePrefix?: string | undefined; - /** - * Override the origin for Electron API routes. - * Enable this if you're facing cors origin issues with Electron API routes. - * - * @default false - */ - disableOriginOverride?: boolean | undefined; } diff --git a/packages/electron/test/electron.test.ts b/packages/electron/test/electron.test.ts index abe615452c..73529308cb 100644 --- a/packages/electron/test/electron.test.ts +++ b/packages/electron/test/electron.test.ts @@ -2,11 +2,11 @@ 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 { createHash } from "@better-auth/utils/hash"; 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 { beforeEach, describe, expect, vi } from "vitest"; import { authenticate, kElectron } from "../src/authenticate"; import { electronClient } from "../src/client"; @@ -81,6 +81,10 @@ vi.mock("electron", () => mockElectron); describe("Electron", () => { const { auth, client, proxyClient, options, customFetchImpl } = testUtils(); + async function s256Challenge(verifier: string) { + return generateCodeChallenge(verifier); + } + it("should throw error when making requests outside the main process", async ({ setProcessType, }) => { @@ -112,6 +116,7 @@ describe("Electron", () => { (globalThis as any)[kElectron] = new Map([ ["abc", "test-challenge"], ]); + const challenge = await s256Challenge("test-challenge"); const { error } = await proxyClient.signUp.email( { @@ -122,8 +127,7 @@ describe("Electron", () => { { query: { client_id: "electron", - code_challenge: "test-challenge", - code_challenge_method: "plain", + code_challenge: challenge, state: "abc", }, onResponse: async (ctx) => { @@ -149,6 +153,7 @@ describe("Electron", () => { (globalThis as any)[kElectron] = new Map([ ["abc", "test-challenge"], ]); + const challenge = await s256Challenge("test-challenge"); const { data } = await proxyClient.signUp.email( { @@ -159,8 +164,7 @@ describe("Electron", () => { { query: { client_id: "electron", - code_challenge: "test-challenge", - code_challenge_method: "plain", + code_challenge: challenge, state: "abc", }, }, @@ -183,9 +187,7 @@ describe("Electron", () => { }); const codeVerifier = base64Url.encode(randomBytes(32)); - const codeChallenge = base64Url.encode( - await createHash("SHA-256").digest(codeVerifier), - ); + const codeChallenge = await s256Challenge(codeVerifier); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -195,7 +197,6 @@ describe("Electron", () => { value: JSON.stringify({ userId: user.id, codeChallenge, - codeChallengeMethod: "s256", state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -236,8 +237,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -247,8 +251,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -308,8 +311,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -319,8 +325,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 999), @@ -389,6 +394,9 @@ describe("Electron", () => { }) => { setProcessType("browser"); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + // Create verification referencing a non-existent user id const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -397,8 +405,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: "non-existent-user", - codeChallenge: "x", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300_000), @@ -409,7 +416,11 @@ describe("Electron", () => { client .$fetch("/electron/token", { method: "POST", - body: { token: identifier, code_verifier: "x", state: "abc" }, + body: { + token: identifier, + code_verifier: codeVerifier, + state: "abc", + }, throw: true, customFetchImpl: (url, init) => { const req = new Request(url.toString(), init); @@ -439,6 +450,9 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ model: "verification", @@ -446,8 +460,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "x", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300_000), @@ -461,7 +474,11 @@ describe("Electron", () => { await expect( client.$fetch("/electron/token", { method: "POST", - body: { token: identifier, code_verifier: "x", state: "abc" }, + body: { + token: identifier, + code_verifier: codeVerifier, + state: "abc", + }, throw: true, customFetchImpl: (url, init) => { const req = new Request(url.toString(), init); @@ -523,8 +540,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -534,8 +554,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -577,8 +596,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -588,8 +610,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -620,9 +641,7 @@ describe("Electron", () => { }); const codeVerifier = base64Url.encode(randomBytes(32)); - const codeChallenge = base64Url.encode( - await createHash("SHA-256").digest(codeVerifier), - ); + const codeChallenge = await s256Challenge(codeVerifier); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -632,7 +651,6 @@ describe("Electron", () => { value: JSON.stringify({ userId: user.id, codeChallenge, - codeChallengeMethod: "s256", state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -865,8 +883,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -876,8 +897,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, }), expiresAt: new Date(Date.now() + 300 * 1000), }, @@ -907,8 +927,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -918,8 +941,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "def", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -986,9 +1008,7 @@ describe("Electron", () => { }); const actualVerifier = base64Url.encode(randomBytes(32)); - const actualChallenge = base64Url.encode( - await createHash("SHA-256").digest(actualVerifier), - ); + const actualChallenge = await s256Challenge(actualVerifier); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -998,7 +1018,6 @@ describe("Electron", () => { value: JSON.stringify({ userId: user.id, codeChallenge: actualChallenge, - codeChallengeMethod: "s256", state: "abc", }), expiresAt: new Date(Date.now() + 300_000), @@ -1022,12 +1041,188 @@ describe("Electron", () => { }), ).rejects.toThrowError("BAD_REQUEST"); }); + + it("should reject a non-S256 (plaintext) challenge at the token endpoint", async ({ + setProcessType, + }) => { + setProcessType("browser"); + + const { user } = await auth.api.signInEmail({ + body: { email: "test@test.com", password: "password" }, + }); + + // A plaintext-PKCE row (challenge equals the raw verifier) is what the + // pre-hardening flow could persist. It must never be exchangeable: the + // token endpoint always verifies the verifier's SHA-256 digest. + const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); + await (await auth.$context).adapter.create({ + model: "verification", + data: { + identifier: `electron:${identifier}`, + value: JSON.stringify({ + userId: user.id, + codeChallenge: "attacker-challenge", + state: "abc", + }), + expiresAt: new Date(Date.now() + 300_000), + }, + }); + + const res = await client.$fetch("/electron/token", { + method: "POST", + body: { + token: identifier, + code_verifier: "attacker-challenge", + state: "abc", + }, + customFetchImpl: (url, init) => { + const req = new Request(url.toString(), init); + return auth.handler(req); + }, + }); + expect((res.error as any)?.code).toBe( + ELECTRON_ERROR_CODES.INVALID_CODE_VERIFIER.code, + ); + }); + + it("should issue an authorization code for an S256 PKCE challenge", async () => { + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + + (globalThis as any)[kElectron] = new Map([ + ["abc", codeVerifier], + ]); + + const { data } = await proxyClient.signUp.email( + { + email: "pkce-default-s256@test.com", + password: "password", + name: "Default S256", + }, + { + query: { + client_id: "electron", + code_challenge: codeChallenge, + state: "abc", + }, + }, + ); + + expect(data).not.toBeNull(); + expect(data).toHaveProperty("electron_authorization_code"); + }); + + it("should forward the challenge to sign-in/social without a method param", async () => { + const { auth: proxyAuth } = testUtils({ + plugins: [electron()], + }); + const { runMigrations } = await getMigrations(proxyAuth.options); + await runMigrations(); + + let capturedUrl: string | null = null; + const originalFetch = globalThis.fetch; + + //@ts-expect-error - intentionally mocking fetch + globalThis.fetch = async (input: any, init?: any) => { + const url = + typeof input === "string" + ? input + : input instanceof URL + ? input.toString() + : input.url; + if (url.includes("/sign-in/social")) { + capturedUrl = url; + } + return proxyAuth.handler(new Request(url, init)); + }; + try { + const codeChallenge = await s256Challenge("test-verifier"); + await proxyAuth.handler( + new Request( + `http://localhost:3000/api/auth/electron/init-oauth-proxy?provider=google&state=abc&code_challenge=${encodeURIComponent(codeChallenge)}`, + { method: "GET" }, + ), + ); + expect(capturedUrl).not.toBeNull(); + const params = new URLSearchParams(new URL(capturedUrl!).search); + expect(params.get("code_challenge")).toBe(codeChallenge); + expect(params.get("code_challenge_method")).toBeNull(); + } finally { + globalThis.fetch = originalFetch; + } + }); + }); + + describe("origin header hardening", () => { + it("should not substitute electron-origin into Origin", async () => { + const res = await auth.handler( + new Request("http://localhost:3000/api/auth/get-session", { + method: "GET", + headers: { + "electron-origin": "myapp:/", + }, + }), + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toBeNull(); + }); + + it("should accept requests with direct origin header from electron client", async () => { + await auth.api.signUpEmail({ + body: { + email: "origin-direct@test.com", + password: "password", + name: "Origin Direct", + }, + }); + + const res = await auth.handler( + new Request("http://localhost:3000/api/auth/sign-in/email", { + method: "POST", + headers: { + "content-type": "application/json", + origin: "myapp:/", + }, + body: JSON.stringify({ + email: "origin-direct@test.com", + password: "password", + }), + }), + ); + expect(res.status).toBe(200); + }); + + it("should not use electron-origin to spoof origin in hooks", async () => { + let observedOrigin: string | null = null; + const { auth: authInstance } = testUtils({ + plugins: [electron()], + hooks: { + before: createAuthMiddleware(async (ctx) => { + observedOrigin = ctx.request?.headers.get("origin") ?? null; + }), + }, + }); + const { runMigrations } = await getMigrations(authInstance.options); + await runMigrations(); + await authInstance.handler( + new Request("http://localhost:3000/api/auth/get-session", { + method: "GET", + headers: { + "electron-origin": "http://attacker.com", + }, + }), + ); + expect(observedOrigin).not.toBe("http://attacker.com"); + }); }); describe("cookies", () => { async function setupSessionWithTokenExchange() { + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const { user } = await auth.api.signInEmail({ body: { email: "test@test.com", password: "password" }, @@ -1039,8 +1234,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -1050,7 +1244,7 @@ describe("Electron", () => { method: "POST", body: { token: identifier, - code_verifier: "test-challenge", + code_verifier: codeVerifier, state: "abc", }, }); @@ -1238,12 +1432,11 @@ describe("Electron", () => { }); }); - it("should modify origin header to electron origin if origin is not set", async ({ + it("should set origin header directly from electron client", async ({ setProcessType, }) => { setProcessType("browser"); - let originalOrigin: string | null = null; let origin: string | null = null; const { auth, client } = testUtils({ hooks: { @@ -1251,16 +1444,7 @@ describe("Electron", () => { origin = ctx.request?.headers.get("origin") ?? null; }), }, - plugins: [ - { - id: "test", - async onRequest(request, ctx) { - const origin = request.headers.get("origin"); - originalOrigin = origin; - }, - }, - electron(), - ], + plugins: [electron()], }); const { runMigrations } = await getMigrations(auth.options); await runMigrations(); @@ -1271,17 +1455,16 @@ describe("Electron", () => { callbackURL: "http://localhost:3000/callback", }); expect(origin).toBe("myapp:/"); - expect(originalOrigin).toBeNull(); }); - it("should not modify origin header if origin is set", async ({ + it("should allow caller to override origin header", async ({ setProcessType, }) => { setProcessType("browser"); - const originalOrigin = "test.com"; let origin: string | null = null; const { auth, client } = testUtils({ + trustedOrigins: ["http://custom-origin.com"], hooks: { before: createAuthMiddleware(async (ctx) => { origin = ctx.request?.headers.get("origin") ?? null; @@ -1300,21 +1483,21 @@ describe("Electron", () => { }, { headers: { - origin: originalOrigin, + origin: "http://custom-origin.com", }, }, ); - expect(origin).toBe(originalOrigin); + expect(origin).toBe("http://custom-origin.com"); }); - it("should not modify origin header if disableOriginOverride is set", async ({ + it("should not use electron-origin header for origin substitution", async ({ setProcessType, }) => { setProcessType("browser"); let origin: string | null = null; - const { auth, client } = testUtils({ - plugins: [electron({ disableOriginOverride: true })], + const { auth } = testUtils({ + plugins: [electron()], hooks: { before: createAuthMiddleware(async (ctx) => { origin = ctx.request?.headers.get("origin") ?? null; @@ -1323,13 +1506,15 @@ describe("Electron", () => { }); const { runMigrations } = await getMigrations(auth.options); await runMigrations(); - await client.signUp.email({ - name: "Test User", - email: "test@test.com", - password: "password", - callbackURL: "http://localhost:3000/callback", - }); - expect(origin).toBe(null); + await auth.handler( + new Request("http://localhost:3000/api/auth/get-session", { + method: "GET", + headers: { + "electron-origin": "http://attacker.com", + }, + }), + ); + expect(origin).not.toBe("http://attacker.com"); }); it("should register ipc handlers", async ({ setProcessType }) => { @@ -1367,9 +1552,7 @@ describe("Electron", () => { }); const codeVerifier = base64Url.encode(randomBytes(32)); - const codeChallenge = base64Url.encode( - await createHash("SHA-256").digest(codeVerifier), - ); + const codeChallenge = await s256Challenge(codeVerifier); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -1379,7 +1562,6 @@ describe("Electron", () => { value: JSON.stringify({ userId: user.id, codeChallenge, - codeChallengeMethod: "s256", state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -1440,9 +1622,7 @@ describe("Electron", () => { }); const codeVerifier = base64Url.encode(randomBytes(32)); - const codeChallenge = base64Url.encode( - await createHash("SHA-256").digest(codeVerifier), - ); + const codeChallenge = await s256Challenge(codeVerifier); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); await (await auth.$context).adapter.create({ @@ -1452,7 +1632,6 @@ describe("Electron", () => { value: JSON.stringify({ userId: user.id, codeChallenge, - codeChallengeMethod: "s256", state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -1636,8 +1815,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -1647,8 +1829,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -1694,8 +1875,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -1705,8 +1889,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000), @@ -1878,8 +2061,11 @@ describe("Electron", () => { }, }); + const codeVerifier = base64Url.encode(randomBytes(32)); + const codeChallenge = await s256Challenge(codeVerifier); + (globalThis as any)[kElectron] = new Map([ - ["abc", "test-challenge"], + ["abc", codeVerifier], ]); const identifier = generateRandomString(16, "A-Z", "a-z", "0-9"); @@ -1889,8 +2075,7 @@ describe("Electron", () => { identifier: `electron:${identifier}`, value: JSON.stringify({ userId: user.id, - codeChallenge: "test-challenge", - codeChallengeMethod: "plain", + codeChallenge, state: "abc", }), expiresAt: new Date(Date.now() + 300 * 1000),