Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions server/src/middleware/apiAuthMiddleware.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AuthType, ErrCode } from "@autumn/shared";
import { verifyKey } from "@/internal/dev/api-keys/apiKeyUtils.js";
import { dashboardOrigins } from "@/utils/constants.js";
import { isDashboardOrigin } from "@/utils/constants.js";
import RecaseError from "@/utils/errorUtils.js";
import { withOrgAuth } from "./authMiddleware.js";
import { verifyBearerPublishableKey } from "./publicAuthMiddleware.js";
Expand All @@ -15,7 +15,7 @@ const verifySecretKey = async (req: any, res: any, next: any) => {

if (!authHeader || !authHeader.startsWith("Bearer ")) {
const origin = req.get("origin");
if (dashboardOrigins.includes(origin)) {
if (isDashboardOrigin({ origin })) {
return withOrgAuth(req, res, next);
} else {
throw new RecaseError({
Expand Down Expand Up @@ -100,7 +100,7 @@ export const apiAuthMiddleware = async (req: any, res: any, next: any) => {
} catch (error: any) {
if (error instanceof RecaseError) {
if (error.code === ErrCode.InvalidSecretKey) {
const apiKey = req.headers["authorization"]?.split(" ")[1];
const apiKey = req.headers.authorization?.split(" ")[1];
error.message = `Invalid secret key: ${maskApiKey(apiKey)}`;
}

Expand Down
3 changes: 3 additions & 0 deletions server/src/utils/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import sendOTPEmail from "@/internal/emails/sendOTPEmail.js";
import { afterOrgCreated } from "./authUtils/afterOrgCreated.js";
import { beforeSessionCreated } from "./authUtils/beforeSessionCreated.js";
import { ADMIN_USER_IDs } from "./constants.js";
import { getSelfHostedOrigins } from "./corsOrigins.js";

export const auth = betterAuth({
baseURL: process.env.BETTER_AUTH_URL,
Expand Down Expand Up @@ -68,6 +69,8 @@ export const auth = betterAuth({
"https://*.useautumn.com",
];

origins.push(...getSelfHostedOrigins());

// Better Auth validates origins independently from app-level CORS.
// Allow local multi-port setups for any non-production runtime.
if (process.env.NODE_ENV !== "production") {
Expand Down
20 changes: 18 additions & 2 deletions server/src/utils/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import "dotenv/config";
import { CusProductStatus } from "@autumn/shared";
import { getClientOrigin } from "./corsOrigins.js";

const BREAK_API_VERSION = 0.2;

Expand All @@ -20,14 +21,29 @@ export const ADMIN_USER_IDs = [
"K7NDwSwohMCV9BXJ3Yb5MxgeXhWcwj0L", // charlie sandbox
];

export const dashboardOrigins = [
const DEFAULT_DASHBOARD_ORIGINS = [
"http://localhost:3000",
"https://app.useautumn.com",
"https://staging.useautumn.com",
"https://dev.useautumn.com",
process.env.CLIENT_URL!,
];

export const getDashboardOrigins = (): string[] => {
const clientOrigin = getClientOrigin();
const origins = [...DEFAULT_DASHBOARD_ORIGINS];
if (clientOrigin) {
origins.push(clientOrigin);
}

return [...new Set(origins)];
};

export const isDashboardOrigin = ({ origin }: { origin?: string }) => {
if (!origin) return false;

return getDashboardOrigins().includes(origin);
};

export const WEBHOOK_EVENTS = [
"checkout.session.completed",
"customer.subscription.created",
Expand Down
33 changes: 33 additions & 0 deletions server/src/utils/corsOrigins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,42 @@ export const ALLOWED_ORIGINS = [
"https://localhost:8080",
];

const toOrigin = ({ url }: { url?: string }): string | undefined => {
if (!url) return undefined;
const trimmedUrl = url.trim();
if (!trimmedUrl) return undefined;

try {
const parsedUrl = new URL(trimmedUrl);
if (!["http:", "https:"].includes(parsedUrl.protocol)) return undefined;
return parsedUrl.origin;
} catch {
return undefined;
}
};

export const getClientOrigin = (): string | undefined => {
return toOrigin({ url: process.env.CLIENT_URL });
};

export const getCheckoutBaseOrigin = (): string | undefined => {
return toOrigin({ url: process.env.CHECKOUT_BASE_URL });
};

export const getSelfHostedOrigins = (): string[] => {
const origins = [getClientOrigin(), getCheckoutBaseOrigin()];

return [
...new Set(origins.filter((origin): origin is string => Boolean(origin))),
];
};

/** Allow any localhost origin in dev for multi-worktree support */
export const isAllowedOrigin = (origin: string): string | undefined => {
if (ALLOWED_ORIGINS.includes(origin)) return origin;
for (const selfHostedOrigin of getSelfHostedOrigins()) {
if (origin === selfHostedOrigin) return origin;
}
if (
process.env.NODE_ENV !== "production" &&
/^https?:\/\/localhost:\d+$/.test(origin)
Expand Down
120 changes: 119 additions & 1 deletion server/tests/unit/corsOrigins.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,33 @@
import { afterEach, describe, expect, test } from "bun:test";
import { ALLOWED_ORIGINS, isAllowedOrigin } from "@/utils/corsOrigins.js";

const restoreEnvVar = ({
key,
value,
}: {
key: "NODE_ENV" | "CLIENT_URL" | "CHECKOUT_BASE_URL";
value: string | undefined;
}) => {
if (value === undefined) {
delete process.env[key];
return;
}

process.env[key] = value;
};

describe("isAllowedOrigin", () => {
const originalNodeEnv = process.env.NODE_ENV;
const originalClientUrl = process.env.CLIENT_URL;
const originalCheckoutBaseUrl = process.env.CHECKOUT_BASE_URL;

afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
restoreEnvVar({ key: "NODE_ENV", value: originalNodeEnv });
restoreEnvVar({ key: "CLIENT_URL", value: originalClientUrl });
restoreEnvVar({
key: "CHECKOUT_BASE_URL",
value: originalCheckoutBaseUrl,
});
});
Comment on lines 24 to 31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afterEach sets env vars to the string "undefined" instead of unsetting them

In Node.js/Bun, assigning undefined to a process.env key converts it to the string "undefined", rather than deleting the key. If CLIENT_URL or CHECKOUT_BASE_URL were unset in the environment before the test suite ran (the common case in CI), originalClientUrl and originalCheckoutBaseUrl will both be undefined. After each self-hosted test, afterEach will leave process.env.CLIENT_URL === "undefined" (a truthy string), causing any subsequent test that doesn't explicitly delete the key to see a non-empty CLIENT_URL.

The existing test ordering means this doesn't currently break anything, but it is incorrect cleanup and will silently corrupt state if tests are reordered or new tests are added. The cleanup should use delete when the original value was absent:

Suggested change
afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
process.env.CLIENT_URL = originalClientUrl;
process.env.CHECKOUT_BASE_URL = originalCheckoutBaseUrl;
});
afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
if (originalClientUrl === undefined) {
delete process.env.CLIENT_URL;
} else {
process.env.CLIENT_URL = originalClientUrl;
}
if (originalCheckoutBaseUrl === undefined) {
delete process.env.CHECKOUT_BASE_URL;
} else {
process.env.CHECKOUT_BASE_URL = originalCheckoutBaseUrl;
}
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/tests/unit/corsOrigins.test.ts
Line: 9-13

Comment:
**`afterEach` sets env vars to the string `"undefined"` instead of unsetting them**

In Node.js/Bun, assigning `undefined` to a `process.env` key converts it to the **string** `"undefined"`, rather than deleting the key. If `CLIENT_URL` or `CHECKOUT_BASE_URL` were unset in the environment before the test suite ran (the common case in CI), `originalClientUrl` and `originalCheckoutBaseUrl` will both be `undefined`. After each self-hosted test, `afterEach` will leave `process.env.CLIENT_URL === "undefined"` (a truthy string), causing any subsequent test that doesn't explicitly `delete` the key to see a non-empty `CLIENT_URL`.

The existing test ordering means this doesn't currently break anything, but it is incorrect cleanup and will silently corrupt state if tests are reordered or new tests are added. The cleanup should use `delete` when the original value was absent:

```suggestion
	afterEach(() => {
		process.env.NODE_ENV = originalNodeEnv;
		if (originalClientUrl === undefined) {
			delete process.env.CLIENT_URL;
		} else {
			process.env.CLIENT_URL = originalClientUrl;
		}
		if (originalCheckoutBaseUrl === undefined) {
			delete process.env.CHECKOUT_BASE_URL;
		} else {
			process.env.CHECKOUT_BASE_URL = originalCheckoutBaseUrl;
		}
	});
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing pattern — the original test already does process.env.NODE_ENV = originalNodeEnv the same way. Happy to fix all three in this PR if maintainers prefer, but wanted to keep the diff focused.


describe("production", () => {
Expand Down Expand Up @@ -63,4 +85,100 @@ describe("isAllowedOrigin", () => {
expect(isAllowedOrigin("http://localhost:3000?x=1")).toBeUndefined();
});
});

describe("self-hosted env URLs", () => {
test("allows CLIENT_URL in production", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL =
"https://autumn-dashboard-production.up.railway.app";
expect(
isAllowedOrigin("https://autumn-dashboard-production.up.railway.app"),
).toBe("https://autumn-dashboard-production.up.railway.app");
});

test("allows CLIENT_URL origin when URL has path and trailing slash", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL =
"https://autumn-dashboard-production.up.railway.app/app/";
expect(
isAllowedOrigin("https://autumn-dashboard-production.up.railway.app"),
).toBe("https://autumn-dashboard-production.up.railway.app");
});

test("allows CHECKOUT_BASE_URL in production", () => {
process.env.NODE_ENV = "production";
process.env.CHECKOUT_BASE_URL =
"https://autumn-checkout-production.up.railway.app";
expect(
isAllowedOrigin("https://autumn-checkout-production.up.railway.app"),
).toBe("https://autumn-checkout-production.up.railway.app");
});

test("trims whitespace around self-hosted env URLs", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL = " https://dashboard.mycompany.com/app/ ";
expect(isAllowedOrigin("https://dashboard.mycompany.com")).toBe(
"https://dashboard.mycompany.com",
);
});

test("allows CHECKOUT_BASE_URL origin when URL has path", () => {
process.env.NODE_ENV = "production";
process.env.CHECKOUT_BASE_URL =
"https://autumn-checkout-production.up.railway.app/c";
expect(
isAllowedOrigin("https://autumn-checkout-production.up.railway.app"),
).toBe("https://autumn-checkout-production.up.railway.app");
});

test("allows both CLIENT_URL and CHECKOUT_BASE_URL simultaneously", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL = "https://dashboard.mycompany.com";
process.env.CHECKOUT_BASE_URL = "https://checkout.mycompany.com";
expect(isAllowedOrigin("https://dashboard.mycompany.com")).toBe(
"https://dashboard.mycompany.com",
);
expect(isAllowedOrigin("https://checkout.mycompany.com")).toBe(
"https://checkout.mycompany.com",
);
});

test("still rejects unrelated origins when env URLs are set", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL = "https://dashboard.mycompany.com";
process.env.CHECKOUT_BASE_URL = "https://checkout.mycompany.com";
expect(isAllowedOrigin("https://evil.com")).toBeUndefined();
expect(
isAllowedOrigin("https://not-autumn.up.railway.app"),
).toBeUndefined();
});

test("ignores invalid self-hosted URL env values", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL = "autumn-dashboard-production.up.railway.app";
process.env.CHECKOUT_BASE_URL = "not-a-url";

expect(
isAllowedOrigin("https://autumn-dashboard-production.up.railway.app"),
).toBeUndefined();
});

test("ignores non-http protocols in self-hosted env URLs", () => {
process.env.NODE_ENV = "production";
process.env.CLIENT_URL = "ftp://dashboard.mycompany.com";

expect(
isAllowedOrigin("https://dashboard.mycompany.com"),
).toBeUndefined();
});

test("rejects custom domains when env URLs are unset", () => {
process.env.NODE_ENV = "production";
delete process.env.CLIENT_URL;
delete process.env.CHECKOUT_BASE_URL;
expect(
isAllowedOrigin("https://autumn-dashboard-production.up.railway.app"),
).toBeUndefined();
});
});
});