Skip to content

Commit 27ce724

Browse files
fix: harden auth origin and verification flows
1 parent bbd22bb commit 27ce724

11 files changed

Lines changed: 269 additions & 30 deletions

File tree

apps/web/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"next": "16.2.6",
2727
"otpauth": "^9.5.0",
2828
"qrcode": "^1.5.4",
29-
"react": "^19.2.4",
29+
"react": "19.2.4",
3030
"react-dom": "19.2.4",
3131
"recharts": "^3.8.1",
3232
"undici": "^7.25.0"
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const createEmailVerificationRequest = vi.fn();
4+
const getCurrentUserProfile = vi.fn();
5+
const cookiesGet = vi.fn();
6+
7+
vi.mock("next/headers", () => ({
8+
cookies: vi.fn(async () => ({
9+
get: cookiesGet,
10+
})),
11+
}));
12+
13+
vi.mock("@/lib/auth", () => ({
14+
createEmailVerificationRequest,
15+
getCurrentUserProfile,
16+
SESSION_COOKIE_NAME: "diffaudit_session",
17+
}));
18+
19+
describe("email verification route", () => {
20+
beforeEach(() => {
21+
vi.resetModules();
22+
createEmailVerificationRequest.mockReset();
23+
getCurrentUserProfile.mockReset();
24+
cookiesGet.mockReset();
25+
});
26+
27+
it("keeps unauthenticated requests rejected", async () => {
28+
cookiesGet.mockReturnValue(undefined);
29+
const route = await import("./route");
30+
31+
const response = await route.POST();
32+
const payload = await response.json();
33+
34+
expect(response.status).toBe(401);
35+
expect(payload).toEqual({ message: "Unauthorized." });
36+
expect(createEmailVerificationRequest).not.toHaveBeenCalled();
37+
});
38+
39+
it("does not create or return browser-visible verification tokens", async () => {
40+
cookiesGet.mockReturnValue({ value: "session-token" });
41+
getCurrentUserProfile.mockReturnValue({
42+
id: "user-1",
43+
username: "reviewer",
44+
pendingEmail: "reviewer@example.test",
45+
});
46+
const route = await import("./route");
47+
48+
const response = await route.POST();
49+
const payload = await response.json();
50+
51+
expect(response.status).toBe(501);
52+
expect(payload).toEqual({
53+
message: "Email verification is not available.",
54+
code: "email_verification_unavailable",
55+
});
56+
expect(getCurrentUserProfile).toHaveBeenCalledWith("session-token");
57+
expect(createEmailVerificationRequest).not.toHaveBeenCalled();
58+
expect(JSON.stringify(payload)).not.toContain("verificationUrl");
59+
});
60+
});

apps/web/src/app/api/auth/email-verification/route.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { cookies } from "next/headers";
22
import { NextResponse } from "next/server";
33

44
import {
5-
createEmailVerificationRequest,
65
getCurrentUserProfile,
76
SESSION_COOKIE_NAME,
87
} from "@/lib/auth";
@@ -16,16 +15,8 @@ export async function POST() {
1615
return NextResponse.json({ message: "Unauthorized." }, { status: 401 });
1716
}
1817

19-
const platformUrl = process.env.DIFFAUDIT_PLATFORM_URL ?? "http://localhost:3000";
20-
const request = createEmailVerificationRequest(profile.id, platformUrl);
21-
22-
if (!request) {
23-
return NextResponse.json({ message: "No pending email to verify." }, { status: 400 });
24-
}
25-
2618
return NextResponse.json({
27-
ok: true,
28-
email: request.email,
29-
verificationUrl: request.verificationUrl,
30-
});
19+
message: "Email verification is not available.",
20+
code: "email_verification_unavailable",
21+
}, { status: 501 });
3122
}

apps/web/src/app/api/auth/github/callback/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ export async function GET(request: Request) {
7070
const clientSecret = process.env.GITHUB_CLIENT_SECRET;
7171
const platformUrl = resolvePlatformUrl(request);
7272

73+
if (!platformUrl) {
74+
return NextResponse.json({ message: "Platform public URL is not configured." }, { status: 500 });
75+
}
76+
7377
const cookieStore = await cookies();
7478
const storedState = readStoredState(cookieStore.get(STATE_COOKIE)?.value);
7579
cookieStore.delete(STATE_COOKIE);

apps/web/src/app/api/auth/github/route.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ export async function GET(request: Request) {
2121
return NextResponse.json({ message: "GitHub OAuth is not configured." }, { status: 500 });
2222
}
2323

24+
if (!platformUrl) {
25+
return NextResponse.json({ message: "Platform public URL is not configured." }, { status: 500 });
26+
}
27+
2428
const state = crypto.randomBytes(16).toString("hex");
2529
const cookieStore = await cookies();
2630
const currentUser = getCurrentUserProfile(cookieStore.get(SESSION_COOKIE_NAME)?.value);
2731
const mode = intent === "connect" && currentUser ? "connect" : "login";
2832

2933
if (intent === "connect" && !currentUser) {
30-
return NextResponse.redirect(new URL(`/login?redirectTo=${encodeURIComponent(redirectTo)}`, request.url));
34+
return NextResponse.redirect(new URL(`/login?redirectTo=${encodeURIComponent(redirectTo)}`, platformUrl));
3135
}
3236

3337
const payload = Buffer.from(JSON.stringify({

apps/web/src/app/api/auth/google/callback/route.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ export async function GET(request: Request) {
8686
const clientSecret = process.env.GOOGLE_CLIENT_SECRET;
8787
const platformUrl = resolvePlatformUrl(request);
8888

89+
if (!platformUrl) {
90+
return NextResponse.json({ message: "Platform public URL is not configured." }, { status: 500 });
91+
}
92+
8993
const cookieStore = await cookies();
9094
const storedState = readStoredState(cookieStore.get(STATE_COOKIE)?.value);
9195
cookieStore.delete(STATE_COOKIE);

apps/web/src/app/api/auth/google/route.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ export async function GET(request: Request) {
2121
return NextResponse.json({ message: "Google OAuth is not configured." }, { status: 500 });
2222
}
2323

24+
if (!platformUrl) {
25+
return NextResponse.json({ message: "Platform public URL is not configured." }, { status: 500 });
26+
}
27+
2428
const state = crypto.randomBytes(16).toString("hex");
2529
const cookieStore = await cookies();
2630
const currentUser = getCurrentUserProfile(cookieStore.get(SESSION_COOKIE_NAME)?.value);
2731
const mode = intent === "connect" && currentUser ? "connect" : "login";
2832

2933
if (intent === "connect" && !currentUser) {
30-
return NextResponse.redirect(new URL(`/login?redirectTo=${encodeURIComponent(redirectTo)}`, request.url));
34+
return NextResponse.redirect(new URL(`/login?redirectTo=${encodeURIComponent(redirectTo)}`, platformUrl));
3135
}
3236

3337
const storedState = Buffer.from(JSON.stringify({
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const cookiesMock = vi.fn();
4+
5+
vi.mock("next/headers", () => ({
6+
cookies: cookiesMock,
7+
}));
8+
9+
const originalNodeEnv = process.env.NODE_ENV;
10+
11+
function hostileRequest(path: string) {
12+
return new Request(`https://internal.invalid${path}`, {
13+
headers: {
14+
host: "attacker.example.test",
15+
"x-forwarded-host": "attacker.example.test",
16+
"x-forwarded-proto": "https",
17+
},
18+
});
19+
}
20+
21+
describe("OAuth public origin", () => {
22+
beforeEach(() => {
23+
vi.resetModules();
24+
cookiesMock.mockReset();
25+
cookiesMock.mockResolvedValue({
26+
get: vi.fn(),
27+
set: vi.fn(),
28+
delete: vi.fn(),
29+
});
30+
process.env.NODE_ENV = "production";
31+
process.env.GITHUB_CLIENT_ID = "github-client";
32+
process.env.GITHUB_CLIENT_SECRET = "github-secret";
33+
process.env.GOOGLE_CLIENT_ID = "google-client";
34+
process.env.GOOGLE_CLIENT_SECRET = "google-secret";
35+
delete process.env.DIFFAUDIT_PLATFORM_URL;
36+
});
37+
38+
afterEach(() => {
39+
process.env.NODE_ENV = originalNodeEnv;
40+
delete process.env.GITHUB_CLIENT_ID;
41+
delete process.env.GITHUB_CLIENT_SECRET;
42+
delete process.env.GOOGLE_CLIENT_ID;
43+
delete process.env.GOOGLE_CLIENT_SECRET;
44+
delete process.env.DIFFAUDIT_PLATFORM_URL;
45+
});
46+
47+
it("fails GitHub OAuth closed instead of deriving redirect_uri from Host headers", async () => {
48+
const route = await import("./github/route");
49+
50+
const response = await route.GET(hostileRequest("/api/auth/github"));
51+
const payload = await response.json();
52+
53+
expect(response.status).toBe(500);
54+
expect(response.headers.get("location")).toBeNull();
55+
expect(payload).toEqual({ message: "Platform public URL is not configured." });
56+
expect(cookiesMock).not.toHaveBeenCalled();
57+
});
58+
59+
it("fails Google OAuth closed instead of deriving redirect_uri from Host headers", async () => {
60+
const route = await import("./google/route");
61+
62+
const response = await route.GET(hostileRequest("/api/auth/google"));
63+
const payload = await response.json();
64+
65+
expect(response.status).toBe(500);
66+
expect(response.headers.get("location")).toBeNull();
67+
expect(payload).toEqual({ message: "Platform public URL is not configured." });
68+
expect(cookiesMock).not.toHaveBeenCalled();
69+
});
70+
71+
it("uses the configured public URL for GitHub OAuth redirect_uri", async () => {
72+
process.env.DIFFAUDIT_PLATFORM_URL = "https://diffaudit.example.test";
73+
const route = await import("./github/route");
74+
75+
const response = await route.GET(hostileRequest("/api/auth/github"));
76+
const location = response.headers.get("location");
77+
const redirect = new URL(location ?? "");
78+
79+
expect(response.status).toBe(307);
80+
expect(redirect.origin).toBe("https://github.com");
81+
expect(redirect.searchParams.get("redirect_uri")).toBe(
82+
"https://diffaudit.example.test/api/auth/github/callback",
83+
);
84+
});
85+
86+
it("uses the configured public URL for unauthenticated connect redirects", async () => {
87+
process.env.DIFFAUDIT_PLATFORM_URL = "https://diffaudit.example.test";
88+
const route = await import("./github/route");
89+
90+
const response = await route.GET(
91+
hostileRequest("/api/auth/github?intent=connect&redirectTo=/workspace/account"),
92+
);
93+
const location = response.headers.get("location");
94+
const redirect = new URL(location ?? "");
95+
96+
expect(response.status).toBe(307);
97+
expect(redirect.origin).toBe("https://diffaudit.example.test");
98+
expect(redirect.pathname).toBe("/login");
99+
expect(redirect.searchParams.get("redirectTo")).toBe("/workspace/account");
100+
});
101+
102+
it("does not redirect callback errors to Host-derived origins", async () => {
103+
const route = await import("./github/callback/route");
104+
105+
const response = await route.GET(hostileRequest("/api/auth/github/callback?error=denied"));
106+
const payload = await response.json();
107+
108+
expect(response.status).toBe(500);
109+
expect(response.headers.get("location")).toBeNull();
110+
expect(payload).toEqual({ message: "Platform public URL is not configured." });
111+
expect(cookiesMock).not.toHaveBeenCalled();
112+
});
113+
});

apps/web/src/lib/auth.test.ts

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@ import {
1313
googleOAuthConfigured,
1414
protectedApiPath,
1515
protectedPagePath,
16+
resolveConfiguredPlatformUrl,
1617
resolvePlatformUrl,
1718
sanitizeRedirectPath,
1819
verifyCredentials,
1920
} from "./auth";
2021
import { resetDbForTests } from "./db";
2122

2223
let tempDir = "";
24+
const originalNodeEnv = process.env.NODE_ENV;
2325

2426
beforeEach(() => {
2527
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "diffaudit-auth-"));
@@ -36,6 +38,7 @@ afterEach(() => {
3638
delete process.env.DIFFAUDIT_SHARED_USERNAME;
3739
delete process.env.DIFFAUDIT_SHARED_PASSWORD;
3840
delete process.env.DIFFAUDIT_PLATFORM_URL;
41+
process.env.NODE_ENV = originalNodeEnv;
3942
fs.rmSync(tempDir, { recursive: true, force: true });
4043
});
4144

@@ -66,6 +69,19 @@ describe("auth route helpers", () => {
6669
expect(sanitizeRedirectPath("//evil.example/path")).toBe(DEFAULT_REDIRECT_PATH);
6770
});
6871

72+
it("rejects redirect targets that can be normalized into external origins", () => {
73+
expect(sanitizeRedirectPath("/\\\\evil.example/path")).toBe(DEFAULT_REDIRECT_PATH);
74+
expect(sanitizeRedirectPath("/%5C%5Cevil.example/path")).toBe(DEFAULT_REDIRECT_PATH);
75+
expect(sanitizeRedirectPath("/%5c%5cevil.example/path")).toBe(DEFAULT_REDIRECT_PATH);
76+
});
77+
78+
it("rejects redirect targets with whitespace or control characters", () => {
79+
expect(sanitizeRedirectPath(" /workspace")).toBe(DEFAULT_REDIRECT_PATH);
80+
expect(sanitizeRedirectPath("/workspace ")).toBe(DEFAULT_REDIRECT_PATH);
81+
expect(sanitizeRedirectPath("/\t/evil.example")).toBe(DEFAULT_REDIRECT_PATH);
82+
expect(sanitizeRedirectPath("/%09/evil.example")).toBe(DEFAULT_REDIRECT_PATH);
83+
});
84+
6985
it("protects only the workspace routes and not the marketing pages", () => {
7086
expect(protectedPagePath("/")).toBe(false);
7187
expect(protectedPagePath("/trial")).toBe(false);
@@ -95,17 +111,49 @@ describe("auth route helpers", () => {
95111
).toBe(true);
96112
});
97113

98-
it("resolves oauth public URL from the request when configured URL is bind-only", () => {
114+
it("rejects bind-only configured platform URLs in production", () => {
115+
process.env.NODE_ENV = "production";
99116
process.env.DIFFAUDIT_PLATFORM_URL = "http://0.0.0.0:3000";
100117

101118
const request = new Request("http://127.0.0.1:3000/api/auth/google", {
102119
headers: {
103-
host: "diffaudit.example.test",
120+
host: "attacker.example.test",
121+
"x-forwarded-host": "attacker.example.test",
104122
"x-forwarded-proto": "https",
105123
},
106124
});
107125

108-
expect(resolvePlatformUrl(request)).toBe("https://diffaudit.example.test");
126+
expect(resolvePlatformUrl(request)).toBeNull();
127+
expect(resolveConfiguredPlatformUrl()).toBeNull();
128+
});
129+
130+
it("rejects unconfigured production platform URLs instead of trusting forwarded hosts", () => {
131+
process.env.NODE_ENV = "production";
132+
133+
const request = new Request("https://internal.invalid/api/auth/google", {
134+
headers: {
135+
host: "attacker.example.test",
136+
"x-forwarded-host": "attacker.example.test",
137+
"x-forwarded-proto": "https",
138+
},
139+
});
140+
141+
expect(resolvePlatformUrl(request)).toBeNull();
142+
});
143+
144+
it("keeps localhost origin fallback for local development only", () => {
145+
process.env.NODE_ENV = "development";
146+
147+
const request = new Request("http://127.0.0.1:3000/api/auth/google", {
148+
headers: {
149+
host: "attacker.example.test",
150+
"x-forwarded-host": "attacker.example.test",
151+
"x-forwarded-proto": "https",
152+
},
153+
});
154+
155+
expect(resolvePlatformUrl(request)).toBe("http://127.0.0.1:3000");
156+
expect(resolveConfiguredPlatformUrl()).toBe("http://localhost:3000");
109157
});
110158

111159
it("prefers a valid configured public platform URL for oauth redirects", () => {

0 commit comments

Comments
 (0)