Cap the profile email-address conflict answer

Changing the profile address asks whether another account already holds
it, and the answer is 409 versus 200 — an existence check over every
address on the instance, open to any signed-in caller. Neither route
that reaches it had a ceiling of any kind and the API wrapper supplies
no default, so the only bound on a sweep was the network.

Both routes now share one per-account bucket, ten an hour, charged
before the question is asked so a refused caller learns nothing. Only a
request whose address actually differs from the one on file is counted,
so re-saving a settings form that carries the unchanged address stays
free. The refusal is the standard 429 with Retry-After and the
X-RateLimit triple, carries `profile.update.emailRateLimited`, and is
published on both operations.
This commit is contained in:
Marc-André Bombeck committed 2026-09-11 14:05:25 +02:00
1 parent 6a5fb338b1
commit fe420f76d8
17 files changed
+596 -181

No files matched your search

+183 -164
View File
File diff suppressed because it is too large. Load diff
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "Es wurde nichts gespeichert. Korrigiere das markierte Feld und versuche es erneut.",
"invalidBody": "Die Profilaktualisierung konnte nicht gelesen werden.",
"emailInUse": "Diese E-Mail-Adresse wird bereits verwendet."
"emailInUse": "Diese E-Mail-Adresse wird bereits verwendet.",
"emailRateLimited": "Zu viele Änderungen der E-Mail-Adresse. Versuche es später erneut."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "Nothing was saved. Check the highlighted field and try again.",
"invalidBody": "The profile update could not be read.",
"emailInUse": "This email address is already in use."
"emailInUse": "This email address is already in use.",
"emailRateLimited": "Too many email-address changes. Try again later."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "No se guardó nada. Revisa el campo marcado e inténtalo de nuevo.",
"invalidBody": "No se pudo leer la actualización del perfil.",
"emailInUse": "Esta dirección de correo ya está en uso."
"emailInUse": "Esta dirección de correo ya está en uso.",
"emailRateLimited": "Demasiados cambios de dirección de correo. Inténtalo más tarde."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "Rien n'a été enregistré. Corrigez le champ signalé et réessayez.",
"invalidBody": "La mise à jour du profil n'a pas pu être lue.",
"emailInUse": "Cette adresse e-mail est déjà utilisée."
"emailInUse": "Cette adresse e-mail est déjà utilisée.",
"emailRateLimited": "Trop de changements d'adresse e-mail. Réessayez plus tard."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "Non è stato salvato nulla. Controlla il campo evidenziato e riprova.",
"invalidBody": "Impossibile leggere l'aggiornamento del profilo.",
"emailInUse": "Questo indirizzo email è già in uso."
"emailInUse": "Questo indirizzo email è già in uso.",
"emailRateLimited": "Troppi cambi di indirizzo email. Riprova più tardi."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "저장된 내용이 없어요. 표시된 항목을 확인하고 다시 시도해 주세요.",
"invalidBody": "프로필 변경 내용을 읽지 못했어요.",
"emailInUse": "이미 사용 중인 이메일 주소예요."
"emailInUse": "이미 사용 중인 이메일 주소예요.",
"emailRateLimited": "이메일 주소를 너무 자주 변경했어요. 잠시 후 다시 시도해 주세요."
}
}
},
+2 -1
View File
@@ -125,7 +125,8 @@
"update": {
"nothingSaved": "Nic nie zostało zapisane. Sprawdź oznaczone pole i spróbuj ponownie.",
"invalidBody": "Nie udało się odczytać aktualizacji profilu.",
"emailInUse": "Ten adres e-mail jest już używany."
"emailInUse": "Ten adres e-mail jest już używany.",
"emailRateLimited": "Zbyt wiele zmian adresu e-mail. Spróbuj ponownie później."
}
}
},
@@ -0,0 +1,132 @@
/**
* The profile email-address ceiling is the one the contract describes.
*
* Changing the address asks whether another account already holds it, and the
* answer is a 409 versus a 200 — an existence check over every address on the
* instance, available to any signed-in caller, including the native client's
* own wildcard token. Neither route that reaches it had a ceiling of any kind,
* and `apiHandler` supplies no default, so the sweep was bounded only by the
* network.
*
* A published number is worth reading only if it cannot drift from the code,
* so this pulls the bucket key and the window out of `prisma.$queryRaw` — the
* technique `rate-limit-auth-surface.test.ts` and the record-write contract
* both use — and holds the emitted OpenAPI description to what came back.
* Reading both from the same constant would prove nothing: the sentence is
* hand-written, deliberately, and this is what keeps the hand-written half
* honest.
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
PROFILE_EMAIL_BUCKET_PREFIX,
PROFILE_EMAIL_LIMIT,
PROFILE_EMAIL_WINDOW_MS,
checkProfileEmailRateLimit,
} from "@/lib/rate-limit";
import { buildOpenApiDocument } from "@/lib/openapi/registry";
import { profileEmailRateLimitResponse } from "@/lib/openapi/routes/shared";
import { ERROR_CODE_CATALOGUE } from "@/lib/openapi/error-codes";
vi.mock("@/lib/db", () => ({ prisma: { $queryRaw: vi.fn() } }));
import { prisma } from "@/lib/db";
/** The published sentence, read from the object the route modules spread. */
const published = profileEmailRateLimitResponse["429"].description;
/** Both doors onto the one handler that answers the conflict. */
const OPERATIONS: Array<[path: string, method: "put" | "patch"]> = [
["/api/auth/profile", "put"],
["/api/user/profile", "patch"],
];
beforeEach(() => {
vi.mocked(prisma.$queryRaw).mockReset();
vi.mocked(prisma.$queryRaw).mockResolvedValue([
{ count: 1, reset_at: new Date(Date.now() + 60_000) },
] as never);
});
describe("the profile email-address bucket", () => {
it("keys on the acting account under the name the contract publishes", async () => {
await checkProfileEmailRateLimit("account-7");
const call = vi.mocked(prisma.$queryRaw).mock.calls.at(-1)!;
// `$queryRaw`INSERT … VALUES (${key}, …)`` lands the key at index 1 and
// the window interval at index 2. Read from the query rather than from
// the constant, so a renamed prefix that never reached the SQL fails.
expect(call[1]).toBe("profile-email:account-7");
expect(call[2]).toBe(`${PROFILE_EMAIL_WINDOW_MS} milliseconds`);
expect(`${PROFILE_EMAIL_BUCKET_PREFIX}:account-7`).toBe(call[1]);
expect(published).toContain("`profile-email:<accountId>`");
});
it("gives a different account a different bucket, not a shared one", async () => {
await checkProfileEmailRateLimit("account-7");
const first = vi.mocked(prisma.$queryRaw).mock.calls.at(-1)![1];
await checkProfileEmailRateLimit("account-8");
const second = vi.mocked(prisma.$queryRaw).mock.calls.at(-1)![1];
expect(first).not.toBe(second);
});
it("refuses at the cap the contract publishes", async () => {
vi.mocked(prisma.$queryRaw).mockResolvedValueOnce([
{ count: PROFILE_EMAIL_LIMIT, reset_at: new Date(Date.now() + 60_000) },
] as never);
await expect(
checkProfileEmailRateLimit("account-7"),
).resolves.toMatchObject({
allowed: true,
limit: PROFILE_EMAIL_LIMIT,
remaining: 0,
});
vi.mocked(prisma.$queryRaw).mockResolvedValueOnce([
{
count: PROFILE_EMAIL_LIMIT + 1,
reset_at: new Date(Date.now() + 60_000),
},
] as never);
await expect(
checkProfileEmailRateLimit("account-7"),
).resolves.toMatchObject({ allowed: false });
const windowSeconds = PROFILE_EMAIL_WINDOW_MS / 1000;
expect(published).toContain(
`${PROFILE_EMAIL_LIMIT} requests per ${windowSeconds} seconds`,
);
});
it("says the headers describe that bucket, and declares them", () => {
expect(Object.keys(profileEmailRateLimitResponse["429"].headers)).toEqual([
"Retry-After",
"X-RateLimit-Limit",
"X-RateLimit-Remaining",
"X-RateLimit-Reset",
]);
expect(published).toContain("Retry-After");
});
it("publishes a code a client can branch on, and registers it", () => {
expect(published).toContain("profile.update.emailRateLimited");
expect(ERROR_CODE_CATALOGUE.profile).toContain(
"profile.update.emailRateLimited",
);
});
it("is published on both operations that answer it", () => {
const doc = buildOpenApiDocument() as unknown as {
paths: Record<
string,
Record<string, { responses?: Record<string, { description?: string }> }>
>;
};
const unpublished = OPERATIONS.filter(
([path, method]) =>
doc.paths[path]?.[method]?.responses?.["429"]?.description !==
published,
).map(([path]) => path);
expect(unpublished).toEqual([]);
});
});
@@ -0,0 +1,148 @@
/**
* The email-address conflict answer is metered, and metered before it answers.
*
* `applyProfileUpdate` is the one place both profile routes reach, and its
* 409 tells any signed-in caller whether a given address belongs to an
* account on this instance. Until this bucket existed neither route had a
* ceiling of any kind — `apiHandler` supplies none — so one account could
* walk an address book at whatever rate the network allowed.
*
* Three things are pinned here. The charge happens BEFORE the uniqueness
* probe, so a refused request never learns the answer it was refused for.
* A save that carries the address already on file is not charged at all, so
* a settings form that re-posts the whole profile does not spend an honest
* afternoon of edits on the ceiling. And the refusal carries the code the
* contract publishes.
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
vi.mock("@/lib/db", () => ({
prisma: {
user: { findUnique: vi.fn(), update: vi.fn() },
},
}));
vi.mock("@/lib/auth/audit", () => ({
auditLog: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("@/lib/rate-limit", () => ({
checkProfileEmailRateLimit: vi.fn(),
}));
import { applyProfileUpdate } from "../profile-update";
import { prisma } from "@/lib/db";
import { checkProfileEmailRateLimit } from "@/lib/rate-limit";
const USER_ID = "user-1";
const ON_FILE = "mine@example.test";
/** The `where` clauses the helper can send, told apart by their shape. */
function selfLookups() {
return vi
.mocked(prisma.user.findUnique)
.mock.calls.filter((c) => "id" in (c[0]!.where as object));
}
function emailProbes() {
return vi
.mocked(prisma.user.findUnique)
.mock.calls.filter((c) => "email" in (c[0]!.where as object));
}
beforeEach(() => {
vi.resetAllMocks();
vi.mocked(prisma.user.update).mockResolvedValue({
id: USER_ID,
username: "u",
email: ON_FILE,
} as never);
vi.mocked(checkProfileEmailRateLimit).mockResolvedValue({
allowed: true,
limit: 10,
remaining: 9,
resetAt: Date.now() + 3_600_000,
});
vi.mocked(prisma.user.findUnique).mockImplementation((async (args: {
where: Record<string, unknown>;
}) => {
if ("id" in args.where) return { email: ON_FILE };
return null;
}) as never);
});
describe("applyProfileUpdate — the email conflict is metered", () => {
it("charges the acting account before asking about the address", async () => {
await applyProfileUpdate(USER_ID, { email: "someone@example.test" });
expect(checkProfileEmailRateLimit).toHaveBeenCalledWith(USER_ID);
expect(emailProbes()).toHaveLength(1);
// Ordering, not just presence: the probe must not have run first.
const chargedAt = vi.mocked(checkProfileEmailRateLimit).mock
.invocationCallOrder[0];
const probedAt = vi.mocked(prisma.user.findUnique).mock.invocationCallOrder[
selfLookups().length
];
expect(chargedAt).toBeLessThan(probedAt);
});
it("refuses with 429 and the published code, without asking about the address", async () => {
vi.mocked(checkProfileEmailRateLimit).mockResolvedValue({
allowed: false,
limit: 10,
remaining: 0,
resetAt: Date.now() + 3_600_000,
});
const result = await applyProfileUpdate(USER_ID, {
email: "someone@example.test",
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.status).toBe(429);
expect(result.errorCode).toBe("profile.update.emailRateLimited");
}
// The whole point: a refused caller is told nothing about the address.
expect(emailProbes()).toHaveLength(0);
expect(prisma.user.update).not.toHaveBeenCalled();
});
it("does not charge a save that carries the address already on file", async () => {
const result = await applyProfileUpdate(USER_ID, {
// Same address, differently typed — normalisation is what decides.
email: " Mine@Example.Test ",
heightCm: 180,
});
expect(result.ok).toBe(true);
expect(checkProfileEmailRateLimit).not.toHaveBeenCalled();
expect(emailProbes()).toHaveLength(0);
expect(prisma.user.update).toHaveBeenCalledTimes(1);
});
it("does not charge an update that never touches the address", async () => {
await applyProfileUpdate(USER_ID, { heightCm: 181 });
expect(checkProfileEmailRateLimit).not.toHaveBeenCalled();
});
it("still answers 409 when the bucket has room and the address is taken", async () => {
vi.mocked(prisma.user.findUnique).mockImplementation((async (args: {
where: Record<string, unknown>;
}) => {
if ("id" in args.where) return { email: ON_FILE };
return { id: "someone-else" };
}) as never);
const result = await applyProfileUpdate(USER_ID, {
email: "taken@example.test",
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.status).toBe(409);
expect(result.errorCode).toBe("profile.update.emailInUse");
}
expect(checkProfileEmailRateLimit).toHaveBeenCalledTimes(1);
});
});
@@ -6,6 +6,9 @@ vi.mock("@/lib/db", () => ({
findUnique: vi.fn(),
update: vi.fn(),
},
// The email-address change is metered before it asks whether another
// account holds the address; the limiter talks to Postgres directly.
$queryRaw: vi.fn(),
},
}));
@@ -34,6 +37,10 @@ const STUB_USER = {
beforeEach(() => {
vi.resetAllMocks();
vi.mocked(prisma.user.update).mockResolvedValue(STUB_USER as never);
// A bucket with room left, unless a test says otherwise.
vi.mocked(prisma.$queryRaw).mockResolvedValue([
{ count: 1, reset_at: new Date(Date.now() + 60_000) },
] as never);
});
describe("applyProfileUpdate timezone validation", () => {
+39 -10
View File
@@ -13,6 +13,7 @@ import {
invalidateUserProfile,
} from "@/lib/cache/invalidate";
import { sanitiseZodIssues, type SanitisedZodIssue } from "@/lib/api-response";
import { checkProfileEmailRateLimit } from "@/lib/rate-limit";
import { z } from "zod/v4";
const extendedProfileSchema = profileSchema.extend({
@@ -238,17 +239,45 @@ export async function applyProfileUpdate(
const normalizedEmail = data.email ? data.email.trim().toLowerCase() : null;
if (data.email !== undefined && normalizedEmail) {
const existing = await prisma.user.findUnique({
where: { email: normalizedEmail },
select: { id: true },
// A settings form re-posts the whole profile, so the address arrives
// with every save whether or not it changed. Those saves ask nothing
// about anyone else's account — the address on file already belongs to
// this caller — so they skip both the uniqueness probe and the budget.
// Charging them would spend an honest afternoon of edits on the
// ceiling and lock the account out of its own settings page.
const current = await prisma.user.findUnique({
where: { id: userId },
select: { email: true },
});
if (existing && existing.id !== userId) {
return {
ok: false,
status: 409,
message: "Email already in use",
errorCode: "profile.update.emailInUse",
};
if (current?.email !== normalizedEmail) {
// Charged BEFORE the probe, not after: a refused request must not
// learn the answer it was refused for. The 409 below tells any
// signed-in caller whether an address is registered here, and until
// this bucket existed neither route that reaches it had a ceiling of
// any kind — `apiHandler` supplies no default.
const rl = await checkProfileEmailRateLimit(userId);
if (!rl.allowed) {
return {
ok: false,
status: 429,
message: "Too many email-address changes. Try again later.",
errorCode: "profile.update.emailRateLimited",
};
}
const existing = await prisma.user.findUnique({
where: { email: normalizedEmail },
select: { id: true },
});
if (existing && existing.id !== userId) {
return {
ok: false,
status: 409,
message: "Email already in use",
errorCode: "profile.update.emailInUse",
};
}
}
}
+1
View File
@@ -250,6 +250,7 @@ export const ERROR_CODE_CATALOGUE: Readonly<Record<string, readonly string[]>> =
practitioner: ["practitioner.invalid"],
profile: [
"profile.update.emailInUse",
"profile.update.emailRateLimited",
"profile.update.invalidBody",
"profile.update.nothingSaved",
],
+2
View File
@@ -38,6 +38,7 @@ import {
} from "@/lib/validations/step-up";
import {
dataEnvelope,
profileEmailRateLimitResponse,
stdResponses,
errorEnvelope,
loginPasswordSchema,
@@ -1766,6 +1767,7 @@ export const authPaths: NonNullable<ZodOpenApiObject["paths"]> = {
content: { "application/json": { schema: errorEnvelope } },
},
...stdResponses,
...profileEmailRateLimitResponse,
"422": {
description:
"Either the body was not a usable object (`meta.errorCode` = `profile.update.invalidBody`) or every supplied field was rejected (`profile.update.nothingSaved`). Both carry the per-field issue list. A body where SOME fields validate never reaches this response — it succeeds with `rejectedFields`.",
+7
View File
@@ -46,6 +46,7 @@ import {
errorEnvelope,
invalidBaseTokenResponse,
malformedJsonResponse,
profileEmailRateLimitResponse,
recordRefusal,
stdResponses,
updatedAtTokenField,
@@ -1476,7 +1477,13 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
},
},
},
"409": {
description:
"The requested email belongs to another account (`meta.errorCode` = `profile.update.emailInUse`). Nothing was written. Shared with PUT /api/auth/profile, which runs the same handler.",
content: { "application/json": { schema: errorEnvelope } },
},
...stdResponses,
...profileEmailRateLimitResponse,
},
},
},
+26
View File
@@ -318,6 +318,32 @@ export const recordWriteRateLimitResponse = {
},
};
/**
* The 429 the profile email-address bucket answers with.
*
* Changing the address asks whether another account already holds it, and the
* answer is a 409 versus a 200 — an existence check over every address on the
* instance, available to any signed-in caller. Neither route that reaches it
* had a ceiling of any kind, and `apiHandler` supplies no default, so the
* sweep was bounded only by the network. This is the response that names the
* bucket now capping it.
*
* Spread AFTER `...stdResponses` so it replaces the generic 429 on those two
* operations. The headers block is the one the standard 429 already declares.
*
* `profile-email-rate-limit-contract.test.ts` holds the numbers in this
* sentence to the constants in `src/lib/rate-limit.ts`, so the paragraph
* cannot drift away from the bucket it describes.
*/
export const profileEmailRateLimitResponse = {
"429": {
description:
"Too many email-address changes. The address change shares one per-account bucket across both profile routes — `profile-email:<accountId>`, 10 requests per 3600 seconds — keyed on the ACTING account, so an account cannot collect a fresh allowance by probing a different address. Nothing was written. `meta.errorCode` is `profile.update.emailRateLimited`. Only a request whose `email` actually differs from the address on file is counted: re-saving a form that carries the unchanged address is free, and so is any update that does not touch `email` at all. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing.",
content: { "application/json": { schema: errorEnvelope } },
headers: stdResponses["429"].headers,
},
};
// ── Idempotent writes ────────────────────────────────────────────────
/**
+37
View File
@@ -160,6 +160,43 @@ export async function checkRecordWriteRateLimit(
);
}
/**
* Per-account ceiling for the profile email-address change.
*
* Changing the address means asking whether another account already holds
* it, and the answer to that question is 409 versus 200 — an existence check
* over every address on the instance, available to any signed-in caller. The
* two routes that reach it (`PUT /api/auth/profile`, `PATCH
* /api/user/profile`) had no ceiling at all and `apiHandler` supplies none,
* so a single account could walk an address book at whatever rate it liked.
*
* Keyed on the acting account rather than on the address, the frozen
* precedent from the record writes: an attacker cannot collect a fresh
* allowance by switching the address they probe, and one account's sweep
* cannot lock a stranger out of their own settings page.
*
* Ten an hour is far above honest use — an address is changed once in a
* while, not ten times a day — and it is only charged when the submitted
* address actually differs from the one on file, so re-saving a settings
* form that carries the unchanged address never spends the budget. What it
* ends is the unbounded sweep: two hundred and forty questions a day per
* account, each with an audit row beside it, instead of as many as the
* network will carry.
*/
export const PROFILE_EMAIL_BUCKET_PREFIX = "profile-email";
export const PROFILE_EMAIL_LIMIT = 10;
export const PROFILE_EMAIL_WINDOW_MS = 60 * 60 * 1000;
export async function checkProfileEmailRateLimit(
accountId: string,
): Promise<RateLimitResult> {
return checkRateLimit(
`${PROFILE_EMAIL_BUCKET_PREFIX}:${accountId}`,
PROFILE_EMAIL_LIMIT,
PROFILE_EMAIL_WINDOW_MS,
);
}
/**
* v1.22.0 — per-credential ceiling for the remote MCP endpoint (`/mcp`).
* The bucket is keyed by the `<userId>:<tokenId>` binding the Bearer