Skip to content

Commit 9cbfe94

Browse files
peppescgpeppescg
committed
fix(auth): use BASE_URL for redirects and handle __Secure- cookie prefix
1 parent f4d315c commit 9cbfe94

4 files changed

Lines changed: 191 additions & 13 deletions

File tree

‎src/app/api/auth/token-refresh/route.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { NextRequest } from "next/server";
2+
import { beforeEach, describe, expect, it, vi } from "vitest";
3+
import { GET } from "./route";
4+
5+
const mockGetAccessToken = vi.hoisted(() => vi.fn());
6+
const mockHeaders = vi.hoisted(() => vi.fn());
7+
8+
vi.mock("next/headers", () => ({
9+
headers: mockHeaders,
10+
}));
11+
12+
vi.mock("@/lib/auth/auth", () => ({
13+
auth: {
14+
api: { getAccessToken: mockGetAccessToken },
15+
},
16+
}));
17+
18+
// BASE_URL defaults to "http://localhost:3000" in test (no BETTER_AUTH_URL set)
19+
20+
const INTERNAL_URL = "http://0.0.0.0:3000/api/auth/token-refresh";
21+
22+
function makeRequest(path = INTERNAL_URL) {
23+
return new NextRequest(path);
24+
}
25+
26+
function mockTokenSuccess(cookies: string[] = []) {
27+
mockGetAccessToken.mockResolvedValue({
28+
ok: true,
29+
status: 200,
30+
headers: { getSetCookie: () => cookies },
31+
});
32+
}
33+
34+
function mockTokenFailure(status = 401) {
35+
mockGetAccessToken.mockResolvedValue({
36+
ok: false,
37+
status,
38+
headers: { getSetCookie: () => [] },
39+
});
40+
}
41+
42+
describe("GET /api/auth/token-refresh", () => {
43+
beforeEach(() => {
44+
vi.clearAllMocks();
45+
mockHeaders.mockResolvedValue(new Headers());
46+
});
47+
48+
describe("redirect URL uses BASE_URL, not request.url", () => {
49+
it("redirects to BASE_URL/catalog on success (not 0.0.0.0)", async () => {
50+
mockTokenSuccess();
51+
const response = await GET(
52+
makeRequest(`${INTERNAL_URL}?redirect=%2Fcatalog`),
53+
);
54+
expect(response.headers.get("location")).toBe(
55+
"http://localhost:3000/catalog",
56+
);
57+
});
58+
59+
it("redirects to BASE_URL/signin on token refresh failure", async () => {
60+
vi.spyOn(console, "warn").mockImplementation(() => {});
61+
mockTokenFailure();
62+
const response = await GET(makeRequest(INTERNAL_URL));
63+
expect(response.headers.get("location")).toBe(
64+
"http://localhost:3000/signin",
65+
);
66+
});
67+
68+
it("redirects to BASE_URL/signin on unexpected error", async () => {
69+
vi.spyOn(console, "error").mockImplementation(() => {});
70+
mockGetAccessToken.mockRejectedValue(new Error("Network error"));
71+
const response = await GET(makeRequest(INTERNAL_URL));
72+
expect(response.headers.get("location")).toBe(
73+
"http://localhost:3000/signin",
74+
);
75+
});
76+
});
77+
78+
describe("open redirect protection", () => {
79+
it("falls back to /catalog when no redirect param", async () => {
80+
mockTokenSuccess();
81+
const response = await GET(makeRequest(INTERNAL_URL));
82+
expect(response.headers.get("location")).toBe(
83+
"http://localhost:3000/catalog",
84+
);
85+
});
86+
87+
it("falls back to /catalog for redirect starting with //", async () => {
88+
mockTokenSuccess();
89+
const response = await GET(
90+
makeRequest(`${INTERNAL_URL}?redirect=//evil.com`),
91+
);
92+
expect(response.headers.get("location")).toBe(
93+
"http://localhost:3000/catalog",
94+
);
95+
});
96+
97+
it("falls back to /catalog for redirect to external origin", async () => {
98+
mockTokenSuccess();
99+
const response = await GET(
100+
makeRequest(
101+
`${INTERNAL_URL}?redirect=${encodeURIComponent("https://evil.com/phishing")}`,
102+
),
103+
);
104+
expect(response.headers.get("location")).toBe(
105+
"http://localhost:3000/catalog",
106+
);
107+
});
108+
109+
it("accepts valid internal path with query string", async () => {
110+
mockTokenSuccess();
111+
const response = await GET(
112+
makeRequest(
113+
`${INTERNAL_URL}?redirect=${encodeURIComponent("/catalog?page=2")}`,
114+
),
115+
);
116+
expect(response.headers.get("location")).toBe(
117+
"http://localhost:3000/catalog?page=2",
118+
);
119+
});
120+
});
121+
122+
describe("Set-Cookie forwarding", () => {
123+
it("copies Set-Cookie headers from Better Auth response onto the redirect", async () => {
124+
const cookie =
125+
"__Secure-better-auth.account_data=newvalue; Path=/; HttpOnly; SameSite=Lax";
126+
mockTokenSuccess([cookie]);
127+
const response = await GET(
128+
makeRequest(`${INTERNAL_URL}?redirect=%2Fcatalog`),
129+
);
130+
expect(response.headers.get("set-cookie")).toBe(cookie);
131+
});
132+
133+
it("forwards multiple Set-Cookie headers", async () => {
134+
const cookies = [
135+
"__Secure-better-auth.account_data=val1; Path=/; HttpOnly",
136+
"__Secure-better-auth.session_token=val2; Path=/; HttpOnly",
137+
];
138+
mockTokenSuccess(cookies);
139+
const response = await GET(
140+
makeRequest(`${INTERNAL_URL}?redirect=%2Fcatalog`),
141+
);
142+
const setCookieHeader = response.headers.getSetCookie();
143+
expect(setCookieHeader).toHaveLength(2);
144+
expect(setCookieHeader[0]).toBe(cookies[0]);
145+
expect(setCookieHeader[1]).toBe(cookies[1]);
146+
});
147+
});
148+
});

‎src/app/api/auth/token-refresh/route.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { headers } from "next/headers";
22
import { type NextRequest, NextResponse } from "next/server";
33
import { auth } from "@/lib/auth/auth";
4-
import { OIDC_PROVIDER_ID } from "@/lib/auth/constants";
4+
import { BASE_URL, OIDC_PROVIDER_ID } from "@/lib/auth/constants";
5+
6+
const BASE_ORIGIN = new URL(BASE_URL).origin;
57

68
/**
79
* Route Handler for preemptive OIDC token refresh.
@@ -26,8 +28,8 @@ export async function GET(request: NextRequest) {
2628
let safeRedirect = "/catalog";
2729
if (redirectParam && !redirectParam.startsWith("//")) {
2830
try {
29-
const resolved = new URL(redirectParam, request.url);
30-
if (resolved.origin === request.nextUrl.origin) {
31+
const resolved = new URL(redirectParam, BASE_URL);
32+
if (resolved.origin === BASE_ORIGIN) {
3133
safeRedirect = resolved.pathname + resolved.search + resolved.hash;
3234
}
3335
} catch {
@@ -49,11 +51,11 @@ export async function GET(request: NextRequest) {
4951
"[TokenRefresh] getAccessToken failed:",
5052
tokenResponse.status,
5153
);
52-
return NextResponse.redirect(new URL("/signin", request.url));
54+
return NextResponse.redirect(new URL("/signin", BASE_URL));
5355
}
5456

5557
const redirectResponse = NextResponse.redirect(
56-
new URL(safeRedirect, request.url),
58+
new URL(safeRedirect, BASE_URL),
5759
);
5860

5961
// Copy Set-Cookie headers from Better Auth's internal response directly
@@ -79,6 +81,6 @@ export async function GET(request: NextRequest) {
7981
return redirectResponse;
8082
} catch (err) {
8183
console.error("[TokenRefresh] Unexpected error:", err);
82-
return NextResponse.redirect(new URL("/signin", request.url));
84+
return NextResponse.redirect(new URL("/signin", BASE_URL));
8385
}
8486
}

‎src/lib/auth/__tests__/utils.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,32 @@ describe("isTokenNearExpiry", () => {
102102
"better-auth-account",
103103
);
104104
});
105+
106+
it("reads __Secure- prefixed cookie (production HTTPS)", async () => {
107+
mockGetAll.mockReturnValue([
108+
{ name: "__Secure-better-auth.account_data", value: "some-jwe" },
109+
]);
110+
mockSymmetricDecodeJWT.mockResolvedValue({
111+
accessTokenExpiresAt: new Date(Date.now() + 60_000).toISOString(),
112+
});
113+
expect(await isTokenNearExpiry()).toBe(false);
114+
});
115+
116+
it("concatenates __Secure- prefixed chunked cookies in sorted order", async () => {
117+
mockGetAll.mockReturnValue([
118+
{ name: "__Secure-better-auth.account_data.1", value: "chunk-b" },
119+
{ name: "__Secure-better-auth.account_data.0", value: "chunk-a" },
120+
]);
121+
mockSymmetricDecodeJWT.mockResolvedValue({
122+
accessTokenExpiresAt: new Date(Date.now() + 60_000).toISOString(),
123+
});
124+
await isTokenNearExpiry();
125+
expect(mockSymmetricDecodeJWT).toHaveBeenCalledWith(
126+
"chunk-achunk-b",
127+
expect.any(String),
128+
"better-auth-account",
129+
);
130+
});
105131
});
106132

107133
describe("utils", () => {

‎src/lib/auth/utils.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,19 @@ export async function isTokenNearExpiry(marginMs = 10_000): Promise<boolean> {
3434
const allCookies = cookieStore.getAll();
3535

3636
// Better Auth may chunk large cookies as account_data.0, account_data.1, etc.
37-
const accountCookies = allCookies.filter(
38-
(c) =>
39-
c.name === "better-auth.account_data" ||
40-
c.name.startsWith("better-auth.account_data."),
41-
);
37+
// In production (HTTPS), cookies are prefixed with "__Secure-".
38+
const ACCOUNT_DATA_BASE = "better-auth.account_data";
39+
const normalize = (name: string) => name.replace(/^__Secure-/, "");
40+
const accountCookies = allCookies.filter((c) => {
41+
const n = normalize(c.name);
42+
return n === ACCOUNT_DATA_BASE || n.startsWith(`${ACCOUNT_DATA_BASE}.`);
43+
});
4244

4345
const baseCookie = accountCookies.find(
44-
(c) => c.name === "better-auth.account_data",
46+
(c) => normalize(c.name) === ACCOUNT_DATA_BASE,
4547
);
4648
const chunkedCookies = accountCookies.filter((c) =>
47-
c.name.startsWith("better-auth.account_data."),
49+
normalize(c.name).startsWith(`${ACCOUNT_DATA_BASE}.`),
4850
);
4951

5052
let jwe: string;

0 commit comments

Comments
 (0)