fix(auth): key rate limits on the resolved client IP and restrict the Expo auth proxy

Resolve the client IP from the TCP peer plus trusted-proxy X-Forwarded-For hops (TRUSTED_PROXIES) and hand it to Better Auth in an internal header, so spoofed forwarding headers can no longer rotate sign-in rate-limit buckets. The Expo authorization proxy now only redirects to the OIDC provider's authorization endpoint. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-16 05:25:42 -04:00 · 2026-10-05 00:09:53 -04:00
1 parent b8b5b31fa1
commit e5209f80fe
@@ -0,0 +1,101 @@
+import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
+
+import { auth } from "@sofa/auth/server";
+
+import { clearAuthorizationEndpointCache } from "../src/expo-proxy-guard";
+import app from "../src/routes/auth";
+
+vi.mock("@sofa/auth/server", () => ({
+  auth: {
+    api: {},
+    handler: vi.fn<(req: Request) => Promise<Response>>(async () => new Response("ok")),
+  },
+}));
+
+const handler = vi.mocked(auth.handler);
+
+const fetchMock = vi.fn<(input: string | URL | Request, init?: RequestInit) => Promise<Response>>(
+  async () =>
+    Response.json({ authorization_endpoint: "https://idp.example.com/application/o/authorize/" }),
+);
+
+const ALLOWED = "https://idp.example.com/application/o/authorize/?client_id=sofa&state=abc";
+
+const proxy = (target: string) =>
+  app.request(`/expo-authorization-proxy?authorizationURL=${encodeURIComponent(target)}`);
+
+beforeEach(() => {
+  handler.mockClear();
+  fetchMock.mockClear();
+  clearAuthorizationEndpointCache();
+  process.env.OIDC_CLIENT_ID = "sofa";
+  process.env.OIDC_CLIENT_SECRET = "test-secret";
+  process.env.OIDC_ISSUER_URL = "https://idp.example.com/application/o/sofa";
+  vi.stubGlobal("fetch", fetchMock);
+});
+
+afterEach(() => {
+  delete process.env.OIDC_CLIENT_ID;
+  delete process.env.OIDC_CLIENT_SECRET;
+  delete process.env.OIDC_ISSUER_URL;
+  vi.unstubAllGlobals();
+});
+
+describe("expo-authorization-proxy guard", () => {
+  test("rejects a foreign host", async () => {
+    const res = await proxy("https://evil.example.net/application/o/authorize/?state=abc");
+    expect(res.status).toBe(400);
+    expect(handler).not.toHaveBeenCalled();
+  });
+
+  test("allows the exact authorization endpoint plus query", async () => {
+    const res = await proxy(ALLOWED);
+    expect(res.status).toBe(200);
+    expect(await res.text()).toBe("ok");
+    expect(handler).toHaveBeenCalledTimes(1);
+  });
+
+  test("rejects the same host on a different path", async () => {
+    const res = await proxy(
+      "https://idp.example.com/application/o/sofa/end-session/?next=https://evil.example.net",
+    );
+    expect(res.status).toBe(400);
+  });
+
+  test("fetches the discovery document once", async () => {
+    await proxy(ALLOWED);
+    await proxy(ALLOWED);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock.mock.calls[0][0]).toBe(
+      "https://idp.example.com/application/o/sofa/.well-known/openid-configuration",
+    );
+  });
+
+  test("does not cache a discovery failure", async () => {
+    fetchMock.mockResolvedValueOnce(new Response("nope", { status: 500 }));
+    expect((await proxy(ALLOWED)).status).toBe(400);
+    expect((await proxy(ALLOWED)).status).toBe(200);
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+  });
+
+  test("rejects everything when OIDC is not configured", async () => {
+    delete process.env.OIDC_CLIENT_SECRET;
+    const res = await proxy(ALLOWED);
+    expect(res.status).toBe(400);
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+});
+
+describe("auth handler forwarding", () => {
+  test("never trusts a client-sent IP header on its own", async () => {
+    await app.request("/get-session", {
+      headers: { "x-forwarded-for": "6.6.6.6", "x-sofa-client-ip": "6.6.6.6" },
+    });
+    expect(handler.mock.calls[0][0].headers.get("x-sofa-client-ip")).toBeNull();
+  });
+
+  test("forwards the legacy callback path to the core endpoint", async () => {
+    await app.request("/oauth2/callback/oidc?code=x&state=y");
+    expect(new URL(handler.mock.calls[0][0].url).pathname).toBe("/callback/oidc");
+  });
+});
@@ -0,0 +1,109 @@
+import { getIP } from "@better-auth/core/utils/ip";
+import { describe, expect, test } from "vitest";
+
+import { CLIENT_IP_HEADER } from "@sofa/auth/config";
+
+import {
+  buildTrustedProxyList,
+  DEFAULT_TRUSTED_PROXIES,
+  normalizeAddress,
+  parseTrustedProxies,
+  resolveClientIp,
+} from "../src/client-ip";
+
+const defaults = buildTrustedProxyList(DEFAULT_TRUSTED_PROXIES).list;
+const none = buildTrustedProxyList([]).list;
+
+describe("resolveClientIp", () => {
+  test("direct internet client spoofing X-Forwarded-For is keyed on the peer", () => {
+    expect(resolveClientIp("6.6.6.6", "203.0.113.9", defaults)).toBe("203.0.113.9");
+  });
+
+  test("direct internet client with no header", () => {
+    expect(resolveClientIp(null, "203.0.113.9", defaults)).toBe("203.0.113.9");
+  });
+
+  test("LAN client direct, no header (all-trusted fallback)", () => {
+    expect(resolveClientIp(null, "192.168.1.5", defaults)).toBe("192.168.1.5");
+  });
+
+  test("dual-stack mapped peer is normalized", () => {
+    expect(resolveClientIp(null, "::ffff:192.168.1.5", defaults)).toBe("192.168.1.5");
+  });
+
+  test("local proxy, internet client", () => {
+    expect(resolveClientIp("198.51.100.7", "172.18.0.5", defaults)).toBe("198.51.100.7");
+  });
+
+  test("local proxy, LAN client", () => {
+    expect(resolveClientIp("192.168.1.5", "127.0.0.1", defaults)).toBe("192.168.1.5");
+  });
+
+  test("spoof through a local proxy", () => {
+    expect(resolveClientIp("6.6.6.6, 198.51.100.7", "172.18.0.5", defaults)).toBe("198.51.100.7");
+  });
+
+  test("malformed hop left of a LAN client", () => {
+    expect(resolveClientIp("garbage, 192.168.1.5", "127.0.0.1", defaults)).toBe("192.168.1.5");
+  });
+
+  test("malformed hop right after the peer", () => {
+    expect(resolveClientIp("garbage", "127.0.0.1", defaults)).toBe("127.0.0.1");
+  });
+
+  test("trust nothing", () => {
+    expect(resolveClientIp("6.6.6.6", "127.0.0.1", none)).toBe("127.0.0.1");
+  });
+
+  test("IPv6 client through a local proxy", () => {
+    expect(resolveClientIp("2001:db8::1", "::1", defaults)).toBe("2001:db8::1");
+  });
+
+  test("unknown peer", () => {
+    expect(resolveClientIp("6.6.6.6", undefined, defaults)).toBeNull();
+  });
+});
+
+describe("TRUSTED_PROXIES parsing", () => {
+  test("parseTrustedProxies", () => {
+    expect(parseTrustedProxies(undefined)).toBe(DEFAULT_TRUSTED_PROXIES);
+    expect(parseTrustedProxies("  ")).toBe(DEFAULT_TRUSTED_PROXIES);
+    expect(parseTrustedProxies("None")).toEqual([]);
+    expect(parseTrustedProxies(" 1.2.3.4 , 10.0.0.0/24 ")).toEqual(["1.2.3.4", "10.0.0.0/24"]);
+  });
+
+  test("buildTrustedProxyList reports invalid entries", () => {
+    expect(
+      buildTrustedProxyList([
+        "1.2.3.4",
+        "10.0.0.0/24",
+        "nope",
+        "10.0.0.0/33",
+        "::1/129",
+        "1.2.3.4/8/1",
+        "10.0.0.0/x",
+      ]).invalid,
+    ).toEqual(["nope", "10.0.0.0/33", "::1/129", "1.2.3.4/8/1", "10.0.0.0/x"]);
+    expect(buildTrustedProxyList(DEFAULT_TRUSTED_PROXIES).invalid).toEqual([]);
+  });
+
+  test("normalizeAddress leaves plain IPv6 unchanged", () => {
+    expect(normalizeAddress("2001:db8::1")).toBe("2001:db8::1");
+  });
+});
+
+describe("Better Auth reads the resolved header", () => {
+  const options = { advanced: { ipAddress: { ipAddressHeaders: [CLIENT_IP_HEADER] } } };
+
+  test("uses the internal header and ignores a client-sent X-Forwarded-For", () => {
+    expect(getIP(new Headers({ [CLIENT_IP_HEADER]: "198.51.100.7" }), options)).toBe(
+      "198.51.100.7",
+    );
+    expect(
+      getIP(
+        new Headers({ [CLIENT_IP_HEADER]: "192.168.1.5", "x-forwarded-for": "6.6.6.6" }),
+        options,
+      ),
+    ).toBe("192.168.1.5");
+  });
+});