fix(#411): remediate backend review findings — COOKIE_DOMAIN, TRUSTED_ORIGINS validation, verifySession
- Wire COOKIE_DOMAIN env var into BetterAuth cookie config - Add URL validation for TRUSTED_ORIGINS (rejects non-HTTP, invalid URLs) - Include original parse error in validateRedirectUri error message - Distinguish infrastructure errors from auth errors in verifySession (Prisma/connection errors now propagate as 500 instead of masking as 401) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -35,6 +35,7 @@ describe("auth.config", () => {
|
||||
delete process.env.NEXT_PUBLIC_APP_URL;
|
||||
delete process.env.NEXT_PUBLIC_API_URL;
|
||||
delete process.env.TRUSTED_ORIGINS;
|
||||
delete process.env.COOKIE_DOMAIN;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -185,6 +186,7 @@ describe("auth.config", () => {
|
||||
|
||||
expect(() => validateOidcConfig()).toThrow("OIDC_REDIRECT_URI must be a valid URL");
|
||||
expect(() => validateOidcConfig()).toThrow("not-a-url");
|
||||
expect(() => validateOidcConfig()).toThrow("Parse error:");
|
||||
});
|
||||
|
||||
it("should throw when OIDC_REDIRECT_URI path does not start with /auth/callback", () => {
|
||||
@@ -404,6 +406,40 @@ describe("auth.config", () => {
|
||||
expect(origins).toContain("http://localhost:3001");
|
||||
expect(origins).toHaveLength(5);
|
||||
});
|
||||
|
||||
it("should reject invalid URLs in TRUSTED_ORIGINS with a warning", () => {
|
||||
process.env.TRUSTED_ORIGINS = "not-a-url,https://valid.example.com";
|
||||
process.env.NODE_ENV = "production";
|
||||
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const origins = getTrustedOrigins();
|
||||
|
||||
expect(origins).toContain("https://valid.example.com");
|
||||
expect(origins).not.toContain("not-a-url");
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Ignoring invalid URL in TRUSTED_ORIGINS: "not-a-url"')
|
||||
);
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should reject non-HTTP origins in TRUSTED_ORIGINS with a warning", () => {
|
||||
process.env.TRUSTED_ORIGINS = "ftp://files.example.com,https://valid.example.com";
|
||||
process.env.NODE_ENV = "production";
|
||||
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const origins = getTrustedOrigins();
|
||||
|
||||
expect(origins).toContain("https://valid.example.com");
|
||||
expect(origins).not.toContain("ftp://files.example.com");
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Ignoring non-HTTP origin in TRUSTED_ORIGINS")
|
||||
);
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("createAuth - session and cookie configuration", () => {
|
||||
@@ -504,5 +540,43 @@ describe("auth.config", () => {
|
||||
};
|
||||
expect(config.advanced.defaultCookieAttributes.secure).toBe(false);
|
||||
});
|
||||
|
||||
it("should set cookie domain when COOKIE_DOMAIN env var is present", () => {
|
||||
process.env.COOKIE_DOMAIN = ".mosaicstack.dev";
|
||||
const mockPrisma = {} as PrismaClient;
|
||||
createAuth(mockPrisma);
|
||||
|
||||
expect(mockBetterAuth).toHaveBeenCalledOnce();
|
||||
const config = mockBetterAuth.mock.calls[0][0] as {
|
||||
advanced: {
|
||||
defaultCookieAttributes: {
|
||||
httpOnly: boolean;
|
||||
secure: boolean;
|
||||
sameSite: string;
|
||||
domain?: string;
|
||||
};
|
||||
};
|
||||
};
|
||||
expect(config.advanced.defaultCookieAttributes.domain).toBe(".mosaicstack.dev");
|
||||
});
|
||||
|
||||
it("should not set cookie domain when COOKIE_DOMAIN env var is absent", () => {
|
||||
delete process.env.COOKIE_DOMAIN;
|
||||
const mockPrisma = {} as PrismaClient;
|
||||
createAuth(mockPrisma);
|
||||
|
||||
expect(mockBetterAuth).toHaveBeenCalledOnce();
|
||||
const config = mockBetterAuth.mock.calls[0][0] as {
|
||||
advanced: {
|
||||
defaultCookieAttributes: {
|
||||
httpOnly: boolean;
|
||||
secure: boolean;
|
||||
sameSite: string;
|
||||
domain?: string;
|
||||
};
|
||||
};
|
||||
};
|
||||
expect(config.advanced.defaultCookieAttributes.domain).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user