mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(auth-check-user): match identifier verbatim and gate behind per-IP rate-limit
Two correctness gaps on the v1.4.41 onboarding-discovery endpoint, caught in the v1.4.41 QA round: 1. The route called `identifier.toLowerCase()` before the OR-match on `username` / `email`, but `registerSchema` applies no transform on write — both columns are stored exactly as typed. A user registered as `MixedCase@Example.com` would never resolve through this route, and iOS onboarding would route them to the sign-up branch despite an existing account. 2. The route header claimed an edge-level limit on `/api/auth/*` covered brute-force enumeration; no such middleware exists. Sibling `/api/auth/passkey/login-options` carries a per-route throttle. We mirror that shape — `checkRateLimit("auth:check-user:<ip>", 30, 15 * 60 * 1000)` — and rewrite the header comment so the rationale matches reality. Tests pin both invariants: a `MixedCase@Example.com` probe hits `findFirst` with the case preserved, and a 429 short-circuits the `findFirst` call entirely.
This commit is contained in:
1 parent
f2a1e8cec8
commit
2f52f66879
2 files changed
+73
-7
No files matched your search
@@ -24,6 +24,16 @@ vi.mock("@/lib/db-compat", () => ({
|
||||
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/rate-limit", () => ({
|
||||
checkRateLimit: vi.fn().mockResolvedValue({
|
||||
allowed: true,
|
||||
limit: 30,
|
||||
remaining: 29,
|
||||
resetAt: Date.now() + 15 * 60 * 1000,
|
||||
}),
|
||||
rateLimitHeaders: vi.fn(() => ({})),
|
||||
}));
|
||||
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
@@ -35,6 +45,7 @@ vi.mock("next/headers", () => ({
|
||||
|
||||
import { POST } from "@/app/api/auth/check-user/route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
|
||||
interface CheckUserBody {
|
||||
data: {
|
||||
@@ -54,6 +65,12 @@ function makeRequest(body: unknown) {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({
|
||||
allowed: true,
|
||||
limit: 30,
|
||||
remaining: 29,
|
||||
resetAt: Date.now() + 15 * 60 * 1000,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -114,4 +131,34 @@ describe("POST /api/auth/check-user", () => {
|
||||
const res = await POST(makeRequest({}));
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
|
||||
it("queries the identifier exactly as iOS sends it (no case-fold)", async () => {
|
||||
// Register stores email + username verbatim (no `.toLowerCase()`
|
||||
// transform in `registerSchema`); folding here would route a
|
||||
// mixed-case existing account to the sign-up branch.
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue(null);
|
||||
await POST(makeRequest({ identifier: "MixedCase@Example.com" }));
|
||||
expect(prisma.user.findFirst).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: {
|
||||
OR: [
|
||||
{ username: "MixedCase@Example.com" },
|
||||
{ email: "MixedCase@Example.com" },
|
||||
],
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns 429 when the per-IP rate-limit is exhausted", async () => {
|
||||
vi.mocked(checkRateLimit).mockResolvedValueOnce({
|
||||
allowed: false,
|
||||
limit: 30,
|
||||
remaining: 0,
|
||||
resetAt: Date.now() + 15 * 60 * 1000,
|
||||
});
|
||||
const res = await POST(makeRequest({ identifier: "anyone@example.com" }));
|
||||
expect(res.status).toBe(429);
|
||||
expect(prisma.user.findFirst).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -25,17 +25,23 @@
|
||||
* whether the identifier matched (callers learn account-existence
|
||||
* either way — that is the explicit contract iOS needs). The
|
||||
* handler does NOT echo the identifier back.
|
||||
* - No rate-limit middleware added here; the higher-level edge limit
|
||||
* on `/api/auth/*` covers brute-force enumeration concerns, and the
|
||||
* route is functionally equivalent in information leak to the
|
||||
* existing `/api/auth/passkey/login-options` request that already
|
||||
* accepts an identifier.
|
||||
* - Per-IP rate-limited (30 requests / 15 min) so the by-design
|
||||
* enumeration contract cannot be brute-forced. Mirrors the
|
||||
* `/api/auth/passkey/login-options` throttle shape.
|
||||
* - The identifier is queried EXACTLY as iOS sends it (no
|
||||
* `.toLowerCase()` normalisation). Email + username columns are
|
||||
* stored verbatim by `/api/auth/register`, so case-folding here
|
||||
* would route legitimate existing users to the sign-up branch.
|
||||
* A future server-side normalisation pass on register write is
|
||||
* the long-term fix.
|
||||
*/
|
||||
import { z } from "zod/v4";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiSuccess, apiError, safeJson } from "@/lib/api-response";
|
||||
import { apiSuccess, apiError, safeJson, getClientIp } from "@/lib/api-response";
|
||||
import { apiHandler } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
import { NextResponse } from "next/server";
|
||||
import type { NextRequest } from "next/server";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
@@ -51,6 +57,19 @@ export type CheckUserBranch =
|
||||
| "exists";
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const ip = getClientIp(request) ?? "unknown";
|
||||
const rl = await checkRateLimit(
|
||||
`auth:check-user:${ip}`,
|
||||
30,
|
||||
15 * 60 * 1000,
|
||||
);
|
||||
if (!rl.allowed) {
|
||||
return NextResponse.json(
|
||||
{ data: null, error: "Too many requests. Please try again later." },
|
||||
{ status: 429, headers: rateLimitHeaders(rl) },
|
||||
);
|
||||
}
|
||||
|
||||
const { data: body, error: jsonError } = await safeJson(request);
|
||||
if (jsonError) return jsonError;
|
||||
|
||||
@@ -59,7 +78,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
return apiError("identifier required", 422);
|
||||
}
|
||||
|
||||
const identifier = parsed.data.identifier.toLowerCase();
|
||||
const identifier = parsed.data.identifier;
|
||||
|
||||
// Match on either username or email — both are unique in the schema.
|
||||
const user = await prisma.user.findFirst({
|
||||
|
||||
Reference in new issue
Block a user