diff --git a/server/src/__tests__/better-auth.test.ts b/server/src/__tests__/better-auth.test.ts index 79bfa3e7..dd672d67 100644 --- a/server/src/__tests__/better-auth.test.ts +++ b/server/src/__tests__/better-auth.test.ts @@ -4,6 +4,7 @@ import { getCookies } from "better-auth/cookies"; import { buildBetterAuthAdvancedOptions, deriveAuthCookiePrefix, + deriveAuthTrustedOrigins, } from "../auth/better-auth.js"; const ORIGINAL_INSTANCE_ID = process.env.PAPERCLIP_INSTANCE_ID; @@ -40,4 +41,38 @@ describe("Better Auth cookie scoping", () => { useSecureCookies: false, }); }); + + it("adds hostname port variants for authenticated mode on non-default ports", () => { + const trustedOrigins = deriveAuthTrustedOrigins({ + deploymentMode: "authenticated", + authBaseUrlMode: "auto", + authPublicBaseUrl: undefined, + allowedHostnames: ["Board.Example.Test"], + port: 3101, + } as Parameters[0]); + + expect(trustedOrigins).toEqual(expect.arrayContaining([ + "https://board.example.test", + "http://board.example.test", + "https://board.example.test:3101", + "http://board.example.test:3101", + ])); + }); + + it("prefers an explicit resolved listen port over the configured port", () => { + const trustedOrigins = deriveAuthTrustedOrigins({ + deploymentMode: "authenticated", + authBaseUrlMode: "auto", + authPublicBaseUrl: undefined, + allowedHostnames: ["board.example.test"], + port: 3100, + } as Parameters[0], { listenPort: 3101 }); + + expect(trustedOrigins).toEqual(expect.arrayContaining([ + "https://board.example.test:3101", + "http://board.example.test:3101", + ])); + expect(trustedOrigins).not.toContain("https://board.example.test:3100"); + expect(trustedOrigins).not.toContain("http://board.example.test:3100"); + }); }); diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index a08e0f02..35fb0767 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -2,17 +2,50 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const { createAppMock, + createBetterAuthInstanceMock, createDbMock, detectPortMock, - loadConfigMock, + deriveAuthTrustedOriginsMock, feedbackExportServiceMock, feedbackServiceFactoryMock, fakeServer, + loadConfigMock, } = vi.hoisted(() => { const createAppMock = vi.fn(async () => ((_: unknown, __: unknown) => {}) as never); + const createBetterAuthInstanceMock = vi.fn(() => ({})); const createDbMock = vi.fn(() => ({}) as never); const detectPortMock = vi.fn(async (port: number) => port); - const loadConfigMock = vi.fn(() => ({ + const deriveAuthTrustedOriginsMock = vi.fn(() => []); + const feedbackExportServiceMock = { + flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), + }; + const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); + const fakeServer = { + once: vi.fn().mockReturnThis(), + off: vi.fn().mockReturnThis(), + listen: vi.fn((_port: number, _host: string, callback?: () => void) => { + callback?.(); + return fakeServer; + }), + close: vi.fn(), + }; + const loadConfigMock = vi.fn(); + + return { + createAppMock, + createBetterAuthInstanceMock, + createDbMock, + detectPortMock, + deriveAuthTrustedOriginsMock, + feedbackExportServiceMock, + feedbackServiceFactoryMock, + fakeServer, + loadConfigMock, + }; +}); + +function buildTestConfig(overrides: Record = {}) { + return { deploymentMode: "authenticated", deploymentExposure: "private", bind: "loopback", @@ -48,31 +81,9 @@ const { heartbeatSchedulerEnabled: false, heartbeatSchedulerIntervalMs: 30000, companyDeletionEnabled: false, - })); - const feedbackExportServiceMock = { - flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })), + ...overrides, }; - const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock); - const fakeServer = { - once: vi.fn().mockReturnThis(), - off: vi.fn().mockReturnThis(), - listen: vi.fn((_port: number, _host: string, callback?: () => void) => { - callback?.(); - return fakeServer; - }), - close: vi.fn(), - }; - - return { - createAppMock, - createDbMock, - detectPortMock, - loadConfigMock, - feedbackExportServiceMock, - feedbackServiceFactoryMock, - fakeServer, - }; -}); +} vi.mock("node:http", () => ({ createServer: vi.fn(() => fakeServer), @@ -169,8 +180,8 @@ vi.mock("../board-claim.js", () => ({ vi.mock("../auth/better-auth.js", () => ({ createBetterAuthHandler: vi.fn(() => undefined), - createBetterAuthInstance: vi.fn(() => ({})), - deriveAuthTrustedOrigins: vi.fn(() => []), + createBetterAuthInstance: createBetterAuthInstanceMock, + deriveAuthTrustedOrigins: deriveAuthTrustedOriginsMock, resolveBetterAuthSession: vi.fn(async () => null), resolveBetterAuthSessionFromHeaders: vi.fn(async () => null), })); @@ -180,6 +191,9 @@ import { startServer } from "../index.ts"; describe("startServer feedback export wiring", () => { beforeEach(() => { vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); + createBetterAuthInstanceMock.mockReturnValue({}); + deriveAuthTrustedOriginsMock.mockReturnValue([]); process.env.BETTER_AUTH_SECRET = "test-secret"; }); @@ -197,9 +211,56 @@ describe("startServer feedback export wiring", () => { }); }); +describe("startServer authenticated auth origin setup", () => { + beforeEach(() => { + vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); + createBetterAuthInstanceMock.mockReturnValue({}); + deriveAuthTrustedOriginsMock.mockReturnValue([]); + process.env.BETTER_AUTH_SECRET = "test-secret"; + }); + + it("derives trusted origins from the detected listen port before auth initializes", async () => { + loadConfigMock.mockReturnValue(buildTestConfig({ + port: 3210, + allowedHostnames: ["board.example.test"], + authBaseUrlMode: "explicit", + authPublicBaseUrl: "http://127.0.0.1:3210", + })); + detectPortMock.mockResolvedValueOnce(3211); + deriveAuthTrustedOriginsMock.mockImplementation( + (_config: { port: number; authPublicBaseUrl?: string }, opts?: { listenPort?: number }) => [ + `http://board.example.test:${opts?.listenPort ?? 0}`, + ], + ); + + await startServer(); + + expect(deriveAuthTrustedOriginsMock).toHaveBeenCalledWith( + expect.objectContaining({ + port: 3210, + authPublicBaseUrl: "http://127.0.0.1:3211/", + }), + { listenPort: 3211 }, + ); + expect(createBetterAuthInstanceMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + port: 3210, + authPublicBaseUrl: "http://127.0.0.1:3211/", + }), + ["http://board.example.test:3211"], + ); + expect(createAppMock.mock.calls[0]?.[1]).toMatchObject({ + serverPort: 3211, + }); + }); +}); + describe("startServer PAPERCLIP_API_URL handling", () => { beforeEach(() => { vi.clearAllMocks(); + loadConfigMock.mockReturnValue(buildTestConfig()); process.env.BETTER_AUTH_SECRET = "test-secret"; delete process.env.PAPERCLIP_API_URL; }); @@ -221,12 +282,11 @@ describe("startServer PAPERCLIP_API_URL handling", () => { }); it("rewrites explicit-port auth public URLs when detect-port selects a new port", async () => { - loadConfigMock.mockReturnValueOnce({ - ...loadConfigMock(), + loadConfigMock.mockReturnValueOnce(buildTestConfig({ port: 3100, authBaseUrlMode: "explicit", authPublicBaseUrl: "http://my-host.ts.net:3100", - }); + })); detectPortMock.mockResolvedValueOnce(3110); const started = await startServer(); @@ -237,12 +297,11 @@ describe("startServer PAPERCLIP_API_URL handling", () => { }); it("keeps no-port auth public URLs stable when detect-port selects a new port", async () => { - loadConfigMock.mockReturnValueOnce({ - ...loadConfigMock(), + loadConfigMock.mockReturnValueOnce(buildTestConfig({ port: 3100, authBaseUrlMode: "explicit", authPublicBaseUrl: "https://paperclip.example", - }); + })); detectPortMock.mockResolvedValueOnce(3110); const started = await startServer(); diff --git a/server/src/auth/better-auth.ts b/server/src/auth/better-auth.ts index 402dad52..3c84b9f5 100644 --- a/server/src/auth/better-auth.ts +++ b/server/src/auth/better-auth.ts @@ -61,7 +61,7 @@ function headersFromExpressRequest(req: Request): Headers { return headersFromNodeHeaders(req.headers); } -export function deriveAuthTrustedOrigins(config: Config): string[] { +export function deriveAuthTrustedOrigins(config: Config, opts?: { listenPort?: number }): string[] { const baseUrl = config.authBaseUrlMode === "explicit" ? config.authPublicBaseUrl : undefined; const trustedOrigins = new Set(); @@ -73,18 +73,24 @@ export function deriveAuthTrustedOrigins(config: Config): string[] { } } if (config.deploymentMode === "authenticated") { + const port = opts?.listenPort ?? config.port; + const needsPortVariants = port !== 80 && port !== 443; for (const hostname of config.allowedHostnames) { const trimmed = hostname.trim().toLowerCase(); if (!trimmed) continue; trustedOrigins.add(`https://${trimmed}`); trustedOrigins.add(`http://${trimmed}`); + if (needsPortVariants) { + trustedOrigins.add(`https://${trimmed}:${port}`); + trustedOrigins.add(`http://${trimmed}:${port}`); + } } } return Array.from(trustedOrigins); } -export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins?: string[]): BetterAuthInstance { +export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins: string[]): BetterAuthInstance { const baseUrl = config.authBaseUrlMode === "explicit" ? config.authPublicBaseUrl : undefined; const secret = process.env.BETTER_AUTH_SECRET ?? process.env.PAPERCLIP_AGENT_JWT_SECRET; if (!secret) { @@ -93,15 +99,13 @@ export function createBetterAuthInstance(db: Db, config: Config, trustedOrigins? "For local development, set BETTER_AUTH_SECRET=paperclip-dev-secret in your .env file.", ); } - const effectiveTrustedOrigins = trustedOrigins ?? deriveAuthTrustedOrigins(config); - const publicUrl = process.env.PAPERCLIP_PUBLIC_URL ?? baseUrl; const isHttpOnly = publicUrl ? publicUrl.startsWith("http://") : false; const authConfig = { baseURL: baseUrl, secret, - trustedOrigins: effectiveTrustedOrigins, + trustedOrigins, database: drizzleAdapter(db, { provider: "pg", schema: { diff --git a/server/src/index.ts b/server/src/index.ts index 4dbf346f..35e2ca5f 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -468,6 +468,12 @@ export async function startServer(): Promise { } } } + + const requestedListenPort = config.port; + const listenPort = await detectPort(requestedListenPort); + if (config.authBaseUrlMode === "explicit" && config.authPublicBaseUrl) { + config.authPublicBaseUrl = rewriteLocalUrlPort(config.authPublicBaseUrl, listenPort); + } let authReady = config.deploymentMode === "local_trusted"; let betterAuthHandler: RequestHandler | undefined; @@ -488,7 +494,7 @@ export async function startServer(): Promise { resolveBetterAuthSession, resolveBetterAuthSessionFromHeaders, } = await import("./auth/better-auth.js"); - const derivedTrustedOrigins = deriveAuthTrustedOrigins(config); + const derivedTrustedOrigins = deriveAuthTrustedOrigins(config, { listenPort }); const envTrustedOrigins = (process.env.BETTER_AUTH_TRUSTED_ORIGINS ?? "") .split(",") .map((value) => value.trim()) @@ -513,17 +519,10 @@ export async function startServer(): Promise { await initializeBoardClaimChallenge(db as any, { deploymentMode: config.deploymentMode }); authReady = true; } - - const listenPort = await detectPort(config.port); - if (listenPort !== config.port) { - config.port = listenPort; - } + if (resolvedEmbeddedPostgresPort !== null && resolvedEmbeddedPostgresPort !== config.embeddedPostgresPort) { config.embeddedPostgresPort = resolvedEmbeddedPostgresPort; } - if (config.authBaseUrlMode === "explicit" && config.authPublicBaseUrl) { - config.authPublicBaseUrl = rewriteLocalUrlPort(config.authPublicBaseUrl, listenPort); - } maybePersistWorktreeRuntimePorts({ serverPort: listenPort, databasePort: resolvedEmbeddedPostgresPort, @@ -627,8 +626,8 @@ export async function startServer(): Promise { server.keepAliveTimeout = 185000; server.headersTimeout = 186000; - if (listenPort !== config.port) { - logger.warn(`Requested port is busy; using next free port (requestedPort=${config.port}, selectedPort=${listenPort})`); + if (listenPort !== requestedListenPort) { + logger.warn(`Requested port is busy; using next free port (requestedPort=${requestedListenPort}, selectedPort=${listenPort})`); } const runtimeListenHost = config.host; @@ -821,7 +820,7 @@ export async function startServer(): Promise { deploymentMode: config.deploymentMode, deploymentExposure: config.deploymentExposure, authReady, - requestedPort: config.port, + requestedPort: requestedListenPort, listenPort, uiMode, db: startupDbInfo,