Backport login-CSRF fix (GHSA-xf67-jxfx-jf88) to 1.x (Harper 4)

src/lib/browserBinding.ts

@@ -0,0 +1,68 @@
+/**
+ * Browser binding via stable per-browser `__Host-oauth_browser` cookie.
+ *
+ * Stores a SHA-256 hash of the cookie in the CSRF state; verified constant-time
+ * at callback before any code exchange.  Only the hash leaves the server.
+ * `__Host-` requires `Secure` + `Path=/` + no `Domain` (HTTPS only).
+ */
+
+import { createHash, randomBytes, timingSafeEqual } from 'node:crypto';
+import type { Request } from '../types.ts';
+
+export const BROWSER_SECRET_COOKIE_NAME = '__Host-oauth_browser';
+
+/** Rolling 7-day lifetime; refreshed on every flow initiation. */
+const BROWSER_SECRET_MAX_AGE_S = 7 * 24 * 60 * 60;
+
+export function generateBrowserSecret(): string {
+	return randomBytes(32).toString('base64url');
+}
+
+/** SHA-256 (base64url) of the browser secret.  Only the hash is stored server-side. */
+export function hashBrowserSecret(secret: string): string {
+	return createHash('sha256').update(secret).digest('base64url');
+}
+
+/** Build Set-Cookie value; `__Host-` requires `Secure` + `Path=/` + no `Domain`. Refreshes Max-Age. */
+export function buildBrowserSecretCookie(secret: string): string {
+	return `${BROWSER_SECRET_COOKIE_NAME}=${secret}; Max-Age=${BROWSER_SECRET_MAX_AGE_S}; Path=/; Secure; HttpOnly; SameSite=Lax`;
+}
+
+/** Extract Cookie header, joining Harper 4 Header array crumbs; falls back to plain-object for test doubles. */
+function readCookieHeader(request: Request | undefined): string | undefined {
+	const headers = request?.headers as any;
+	if (!headers) return undefined;
+	// Harper 4 runtime: Headers object with .get()
+	if (typeof headers.get === 'function') {
+		const raw = headers.get('cookie');
+		if (raw == null) return undefined;
+		return Array.isArray(raw) ? raw.join('; ') : String(raw);
+	}
+	// Test doubles: plain object
+	const raw = headers.cookie;
+	if (raw == null) return undefined;
+	return Array.isArray(raw) ? raw.join('; ') : String(raw);
+}
+
+/** Parse the `__Host-oauth_browser` cookie value from the request. */
+export function readBrowserSecret(request: Request | undefined): string | undefined {
+	const header = readCookieHeader(request);
+	if (typeof header !== 'string' || !header) return undefined;
+	for (const part of header.split(';')) {
+		const eq = part.indexOf('=');
+		if (eq === -1) continue;
+		if (part.slice(0, eq).trim() === BROWSER_SECRET_COOKIE_NAME) {
+			const value = part.slice(eq + 1).trim();
+			return /^[A-Za-z0-9_-]{1,64}$/.test(value) ? value : undefined;
+		}
+	}
+	return undefined;
+}
+
+/** Constant-time check that `secret` hashes to `expectedHash`. */
+export function browserSecretMatches(secret: string | undefined, expectedHash: string | undefined): boolean {
+	if (typeof secret !== 'string' || typeof expectedHash !== 'string' || !secret || !expectedHash) return false;
+	const actual = Buffer.from(hashBrowserSecret(secret));
+	const expected = Buffer.from(expectedHash);
+	return actual.length === expected.length && timingSafeEqual(actual, expected);
+}

src/lib/handlers.ts

@@ -18,6 +18,13 @@ import type {
 	OnLoginResultNeedsConfirmation,
 } from '../types.ts';
 import type { HookManager } from './hookManager.ts';
+import {
+	browserSecretMatches,
+	buildBrowserSecretCookie,
+	generateBrowserSecret,
+	hashBrowserSecret,
+	readBrowserSecret,
+} from './browserBinding.ts';
 
 /**
  * Sanitize a redirect parameter to prevent open redirect attacks
@@ -128,12 +135,17 @@ export async function handleLogin(
 	const referer = request.headers?.referer ? sanitizeRedirect(request.headers.referer) : undefined;
 	const originalUrl = redirectParam || referer || config.postLoginRedirect || '/';
 
+	// Browser binding: read or mint the stable __Host- cookie; store its hash in the CSRF state.
+	const existingSecret = readBrowserSecret(request);
+	const browserSecret = existingSecret ?? generateBrowserSecret();
+
 	// Generate CSRF token with metadata
 	// Bind token to provider to prevent cross-provider CSRF attacks
 	const csrfToken = await provider.generateCSRFToken({
 		originalUrl,
 		sessionId: request.session?.id,
 		providerName, // Bind state token to this provider
+		browserNonceHash: hashBrowserSecret(browserSecret),
 	});
 
 	// Build authorization URL with CSRF token as state parameter
@@ -144,7 +156,8 @@ export async function handleLogin(
 	return {
 		status: 302,
 		headers: {
-			Location: authUrl,
+			'Location': authUrl,
+			'Set-Cookie': buildBrowserSecretCookie(browserSecret),
 		},
 	};
 }
@@ -167,20 +180,19 @@ export async function handleCallback(
 	const error = target.get?.('error');
 	const errorDescription = target.get?.('error_description');
 
-	// Handle OAuth errors from provider
-	if (error) {
-		logger?.error?.(`OAuth error: ${error} - ${errorDescription}`);
-		const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', { error: 'oauth_failed', reason: error });
-		return {
-			status: 302,
-			headers: {
-				Location: errorUrl,
-			},
-		};
-	}
-
-	// Validate parameters
-	if (!code || !state) {
+	// No state token: handle stateless errors and missing params without token verification.
+	if (!state) {
+		if (error) {
+			// JSON.stringify: CRLF-safe logging of browser-controlled params (CWE-117).
+			logger?.error?.(`OAuth error: ${JSON.stringify(error)} - ${JSON.stringify(errorDescription)}`);
+			const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', { error: 'oauth_failed', reason: error });
+			return {
+				status: 302,
+				headers: {
+					Location: errorUrl,
+				},
+			};
+		}
 		logger?.warn?.('Missing required OAuth callback parameters');
 		const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', { error: 'invalid_request' });
 		return {
@@ -191,7 +203,7 @@ export async function handleCallback(
 		};
 	}
 
-	// Verify CSRF token
+	// Consume the single-use state before handling any upstream error or binding check.
 	const tokenData = await provider.verifyCSRFToken(state);
 	if (!tokenData) {
 		logger?.warn?.('Invalid or expired CSRF token');
@@ -247,6 +259,52 @@ export async function handleCallback(
 		};
 	}
 
+	// Browser binding: verify the __Host- cookie hash before the code exchange; absent hash passes (pre-upgrade tokens).
+	if (tokenData.browserNonceHash) {
+		if (!browserSecretMatches(readBrowserSecret(request), tokenData.browserNonceHash)) {
+			logger?.warn?.(`OAuth callback: login browser binding mismatch (provider '${providerName}')`);
+			const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
+				error: 'auth_failed',
+				reason: 'csrf',
+			});
+			return {
+				status: 302,
+				headers: {
+					Location: errorUrl,
+				},
+			};
+		}
+	}
+
+	// Handle provider errors after state consumption and binding verification.
+	if (error) {
+		// JSON.stringify: CRLF-safe logging of browser-controlled params (CWE-117).
+		logger?.error?.(`OAuth error: ${JSON.stringify(error)} - ${JSON.stringify(errorDescription)}`);
+		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
+			error: 'oauth_failed',
+			reason: error,
+		});
+		return {
+			status: 302,
+			headers: {
+				Location: errorUrl,
+			},
+		};
+	}
+
+	if (!code) {
+		logger?.warn?.('Missing required OAuth callback parameters');
+		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
+			error: 'invalid_request',
+		});
+		return {
+			status: 302,
+			headers: {
+				Location: errorUrl,
+			},
+		};
+	}
+
 	try {
 		// Exchange code for tokens
 		const tokenResponse = await provider.exchangeCodeForToken(code, config.redirectUri || '');
@@ -281,7 +339,9 @@ export async function handleCallback(
 		if (isGatedLoginOutcome(hookData)) {
 			const denied = hookData.status === 'denied';
 			const reason = denied ? hookData.error : undefined;
-			logger?.info?.(`OAuth login ${denied ? 'denied' : 'deferred'} by onLogin hook for user: ${user.username}`);
+			logger?.info?.(
+				`OAuth login ${denied ? 'denied' : 'deferred'} by onLogin hook for user: ${JSON.stringify(user.username)}`
+			);
 			if (hookData.redirect) {
 				return { status: 302, headers: { Location: resolveHookRedirect(hookData.redirect) } };
 			}
@@ -354,7 +414,7 @@ export async function handleCallback(
 			}
 
 			logger?.info?.(
-				`OAuth login successful for user: ${user.username}${tokenResponse.expires_in ? `, token expires in ${tokenResponse.expires_in}s` : ', token does not expire'}`
+				`OAuth login successful for user: ${JSON.stringify(user.username)}${tokenResponse.expires_in ? `, token expires in ${tokenResponse.expires_in}s` : ', token does not expire'}`
 			);
 		} else {
 			logger?.warn?.('No session available for OAuth user');

test/lib/browserBinding.test.js

@@ -0,0 +1,135 @@
+// Tests for the browser-binding cookie helpers.
+import { describe, it } from 'node:test';
+import assert from 'node:assert/strict';
+import {
+	BROWSER_SECRET_COOKIE_NAME,
+	browserSecretMatches,
+	buildBrowserSecretCookie,
+	generateBrowserSecret,
+	hashBrowserSecret,
+	readBrowserSecret,
+} from '../../dist/lib/browserBinding.js';
+
+describe('browserBinding', () => {
+	it('generates unique url-safe browser secrets', () => {
+		assert.notEqual(generateBrowserSecret(), generateBrowserSecret());
+		// base64url — no +, /, or = padding
+		assert.match(generateBrowserSecret(), /^[A-Za-z0-9_-]+$/);
+	});
+
+	it('hashBrowserSecret produces consistent SHA-256 base64url output', () => {
+		const secret = 'test-secret';
+		assert.equal(hashBrowserSecret(secret), hashBrowserSecret(secret));
+		assert.notEqual(hashBrowserSecret(secret), hashBrowserSecret('other'));
+		assert.match(hashBrowserSecret(secret), /^[A-Za-z0-9_-]+$/);
+	});
+
+	it('buildBrowserSecretCookie includes all required cookie attributes', () => {
+		const cookie = buildBrowserSecretCookie('mysecret');
+		assert.ok(cookie.startsWith(`${BROWSER_SECRET_COOKIE_NAME}=mysecret`));
+		assert.ok(cookie.includes('Path=/'));
+		assert.ok(cookie.includes('Secure'));
+		assert.ok(cookie.includes('HttpOnly'));
+		assert.ok(cookie.includes('SameSite=Lax'));
+		assert.ok(cookie.includes('Max-Age='));
+	});
+
+	describe('readBrowserSecret', () => {
+		it('reads the secret from a plain-object headers cookie', () => {
+			const secret = generateBrowserSecret();
+			const request = { headers: { cookie: `${BROWSER_SECRET_COOKIE_NAME}=${secret}` } };
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('reads the secret when other cookies precede it', () => {
+			const secret = 'abc123';
+			const request = {
+				headers: { cookie: `other=value; ${BROWSER_SECRET_COOKIE_NAME}=${secret}; trailing=x` },
+			};
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('reads the secret via .get() (Harper 4 runtime Headers shape)', () => {
+			const secret = 'runtime-secret';
+			const request = {
+				headers: {
+					get: (name) => (name === 'cookie' ? `${BROWSER_SECRET_COOKIE_NAME}=${secret}` : null),
+				},
+			};
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('reads the secret when .get() returns an array of cookie crumbs (HTTP/2 crumbling)', () => {
+			// Harper 4 Headers can split Cookie across array crumbs — binding cookie may be in any.
+			const secret = 'crumbled-secret';
+			const request = {
+				headers: {
+					get: (name) =>
+						name === 'cookie' ? ['session=abc', `${BROWSER_SECRET_COOKIE_NAME}=${secret}`, 'other=1'] : null,
+				},
+			};
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('reads the secret from a plain-object headers.cookie array', () => {
+			const secret = 'plain-array-secret';
+			const request = {
+				headers: { cookie: [`other=x`, `${BROWSER_SECRET_COOKIE_NAME}=${secret}`] },
+			};
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('returns undefined when the cookie is absent', () => {
+			assert.equal(readBrowserSecret({ headers: { cookie: 'other=value' } }), undefined);
+			assert.equal(readBrowserSecret({ headers: {} }), undefined);
+			assert.equal(readBrowserSecret(undefined), undefined);
+		});
+
+		it('returns the secret for a valid 43-char base64url value', () => {
+			const secret = generateBrowserSecret();
+			assert.equal(secret.length, 43);
+			assert.match(secret, /^[A-Za-z0-9_-]+$/);
+			const request = { headers: { cookie: `${BROWSER_SECRET_COOKIE_NAME}=${secret}` } };
+			assert.equal(readBrowserSecret(request), secret);
+		});
+
+		it('returns undefined for a malformed cookie value (contains disallowed chars)', () => {
+			const malformed = 'abc!@#$%^&*()malformed value with spaces';
+			const request = { headers: { cookie: `${BROWSER_SECRET_COOKIE_NAME}=${malformed}` } };
+			assert.equal(readBrowserSecret(request), undefined);
+		});
+
+		it('returns undefined for an over-length cookie value (65+ chars)', () => {
+			const overLength = 'a'.repeat(65);
+			const request = { headers: { cookie: `${BROWSER_SECRET_COOKIE_NAME}=${overLength}` } };
+			assert.equal(readBrowserSecret(request), undefined);
+		});
+	});
+
+	describe('browserSecretMatches', () => {
+		it('returns true for a matching secret and hash', () => {
+			const secret = generateBrowserSecret();
+			assert.ok(browserSecretMatches(secret, hashBrowserSecret(secret)));
+		});
+
+		it('returns false for a mismatched secret', () => {
+			const secret = generateBrowserSecret();
+			assert.ok(!browserSecretMatches('wrong-secret', hashBrowserSecret(secret)));
+		});
+
+		it('returns false when secret or hash is undefined/empty', () => {
+			assert.ok(!browserSecretMatches(undefined, hashBrowserSecret('x')));
+			assert.ok(!browserSecretMatches('x', undefined));
+			assert.ok(!browserSecretMatches('', 'anyhash'));
+		});
+
+		it('returns false without throwing when either argument is a non-string (number, object)', () => {
+			assert.doesNotThrow(() => {
+				assert.ok(!browserSecretMatches(/** @type {any} */ (42), hashBrowserSecret('x')));
+				assert.ok(!browserSecretMatches('x', /** @type {any} */ (42)));
+				assert.ok(!browserSecretMatches(/** @type {any} */ ({ valueOf: () => 'x' }), hashBrowserSecret('x')));
+				assert.ok(!browserSecretMatches(/** @type {any} */ (null), hashBrowserSecret('x')));
+			});
+		});
+	});
+});