mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
fix(auth): require a fresh proof before a second factor is added
A browser session on its own could start TOTP enrollment or register a security key. Combined with a pending secret counting as a factor, that let a stolen session give itself a second factor, register a passkey with it, and keep access after the owner changed the password. TOTP setup and security-key registration now pass on the cookie path only when the session signed in or completed a second factor in the last five minutes, or when the body carries a proof in the step-up shape (password, a confirmed authenticator code, or a passkey or security-key assertion). The settings cards ask for that proof when the server answers auth.reproof.required and retry the same call with it. The Bearer path is unchanged: its elevation already is the proof. Every route that checks a password or a factor as a re-proof now uses one verifier and one per-account budget of five attempts per fifteen minutes, shared with the step-up mint, and records each refused proof as auth.mfa.failed. Passkey enrollment and password change had neither the limit (passkey enrollment) nor the audit row (both). A structural test freezes the set of credential-verifying routes and requires a throttle and an audit call in each.
This commit is contained in:
1 parent
e1718e3c30
commit
7b5bc130d8
27 files changed
+1763
-584
No files matched your search
@@ -10,7 +10,14 @@
|
||||
*
|
||||
* Gated by `requireMfaManagementAuth`: a cookie session, or a Bearer token
|
||||
* presenting a single-use step-up elevation minted against a re-proved factor.
|
||||
* A token on its own can still never enrol MFA. The recovery-code batch is
|
||||
* A token on its own can still never enrol MFA.
|
||||
*
|
||||
* A cookie session on its own cannot either. Enrolling hands the caller a
|
||||
* factor that later satisfies every step-up gate, so a stolen session must not
|
||||
* be able to mint one: the cookie arm also passes `checkCookieEnrollmentProof`
|
||||
* — a sign-in or second factor inside the last five minutes, or a password /
|
||||
* factor proof in the body (the step-up mint's shapes). The Bearer arm already
|
||||
* carries that proof in its elevation. The recovery-code batch is
|
||||
* issued at `/confirm` (after the factor is proven), not here, so an abandoned
|
||||
* setup never persists codes.
|
||||
*/
|
||||
@@ -26,6 +33,7 @@ import { prisma } from "@/lib/db";
|
||||
import { encrypt } from "@/lib/crypto";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
import { generateTotpSecret, buildOtpauthUri } from "@/lib/auth/mfa/totp";
|
||||
import { checkCookieEnrollmentProof } from "@/lib/auth/existing-factor-proof";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
@@ -57,6 +65,17 @@ export const POST = apiHandler(async (req: Request) => {
|
||||
throw new HttpError(409, "A second factor is already active");
|
||||
}
|
||||
|
||||
if (auth.transport === "cookie") {
|
||||
const refusal = await checkCookieEnrollmentProof({
|
||||
user,
|
||||
sessionId: auth.session.id,
|
||||
request: req,
|
||||
ipAddress: getClientIp(req),
|
||||
stage: "totp_enroll",
|
||||
});
|
||||
if (refusal) return refusal;
|
||||
}
|
||||
|
||||
// Rate limit and the already-active check have passed; the write is next.
|
||||
await auth.commitElevation();
|
||||
|
||||
|
||||
@@ -31,11 +31,16 @@ vi.mock("@/lib/auth/audit", () => ({
|
||||
|
||||
vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/auth/existing-factor-proof", () => ({
|
||||
checkCookieEnrollmentProof: vi.fn(),
|
||||
}));
|
||||
|
||||
import { POST as REGISTER_OPTIONS } from "../options/route";
|
||||
import { POST as REGISTER_VERIFY } from "../verify/route";
|
||||
import { requireMfaManagementAuth } from "@/lib/api-handler";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { checkCookieEnrollmentProof } from "@/lib/auth/existing-factor-proof";
|
||||
import {
|
||||
createMfaRegistrationOptions,
|
||||
verifyMfaRegistration,
|
||||
@@ -63,8 +68,16 @@ beforeEach(() => {
|
||||
commitElevation: vi.fn().mockResolvedValue(undefined),
|
||||
} as never);
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never);
|
||||
vi.mocked(checkCookieEnrollmentProof).mockResolvedValue(null);
|
||||
});
|
||||
|
||||
function optionsRequest(): NextRequest {
|
||||
return new NextRequest(
|
||||
"http://localhost/api/auth/me/mfa/webauthn/register/options",
|
||||
{ method: "POST" },
|
||||
);
|
||||
}
|
||||
|
||||
describe("POST /api/auth/me/mfa/webauthn/register/options", () => {
|
||||
it("returns ceremony options + challenge id", async () => {
|
||||
vi.mocked(createMfaRegistrationOptions).mockResolvedValue({
|
||||
@@ -72,7 +85,7 @@ describe("POST /api/auth/me/mfa/webauthn/register/options", () => {
|
||||
challengeId: "ch-1",
|
||||
} as never);
|
||||
|
||||
const res = await REGISTER_OPTIONS();
|
||||
const res = await REGISTER_OPTIONS(optionsRequest());
|
||||
const body = (await res.json()) as {
|
||||
data: { options: unknown; challengeId: string };
|
||||
};
|
||||
@@ -86,6 +99,40 @@ describe("POST /api/auth/me/mfa/webauthn/register/options", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("security-key enrollment needs a fresh proof on the cookie arm", () => {
|
||||
it("stops before any challenge exists when the session cannot show one", async () => {
|
||||
vi.mocked(checkCookieEnrollmentProof).mockResolvedValue(
|
||||
new Response(null, { status: 401 }),
|
||||
);
|
||||
const res = await REGISTER_OPTIONS(optionsRequest());
|
||||
expect(res.status).toBe(401);
|
||||
expect(createMfaRegistrationOptions).not.toHaveBeenCalled();
|
||||
expect(checkCookieEnrollmentProof).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sessionId: "sess-1",
|
||||
stage: "security_key_enroll",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves the Bearer arm to its elevation", async () => {
|
||||
vi.mocked(requireMfaManagementAuth).mockResolvedValue({
|
||||
transport: "bearer",
|
||||
user: USER,
|
||||
apiTokenId: "tok-1",
|
||||
accessTokenHash: "h",
|
||||
commitElevation: vi.fn().mockResolvedValue(undefined),
|
||||
} as never);
|
||||
vi.mocked(createMfaRegistrationOptions).mockResolvedValue({
|
||||
options: {},
|
||||
challengeId: "ch-2",
|
||||
} as never);
|
||||
const res = await REGISTER_OPTIONS(optionsRequest());
|
||||
expect(res.status).toBe(200);
|
||||
expect(checkCookieEnrollmentProof).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/auth/me/mfa/webauthn/register/verify", () => {
|
||||
const credential = {
|
||||
id: "cred-1",
|
||||
|
||||
@@ -5,16 +5,21 @@
|
||||
* session or a Bearer token presenting a single-use step-up elevation; a token
|
||||
* on its own can never enrol MFA. Returns the SimpleWebAuthn creation options +
|
||||
* the server-issued challenge id to present back at /register/verify.
|
||||
*
|
||||
* The cookie arm additionally passes `checkCookieEnrollmentProof`: a session
|
||||
* alone cannot add a security key, for the same reason it cannot enroll TOTP —
|
||||
* the new key would satisfy every step-up gate from then on.
|
||||
*/
|
||||
import { apiHandler, requireMfaManagementAuth } from "@/lib/api-handler";
|
||||
import { apiError, apiSuccess } from "@/lib/api-response";
|
||||
import { apiError, apiSuccess, getClientIp } from "@/lib/api-response";
|
||||
import { checkCookieEnrollmentProof } from "@/lib/auth/existing-factor-proof";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
import { createMfaRegistrationOptions } from "@/lib/auth/mfa/webauthn";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
export const POST = apiHandler(async () => {
|
||||
export const POST = apiHandler(async (req: Request) => {
|
||||
const auth = await requireMfaManagementAuth();
|
||||
const { user } = auth;
|
||||
|
||||
@@ -31,6 +36,17 @@ export const POST = apiHandler(async () => {
|
||||
return res;
|
||||
}
|
||||
|
||||
if (auth.transport === "cookie") {
|
||||
const refusal = await checkCookieEnrollmentProof({
|
||||
user,
|
||||
sessionId: auth.session.id,
|
||||
request: req,
|
||||
ipAddress: getClientIp(req),
|
||||
stage: "security_key_enroll",
|
||||
});
|
||||
if (refusal) return refusal;
|
||||
}
|
||||
|
||||
await auth.commitElevation();
|
||||
|
||||
const { options, challengeId } = await createMfaRegistrationOptions(
|
||||
|
||||
@@ -2,16 +2,20 @@ import { NextRequest } from "next/server";
|
||||
import {
|
||||
createAuthenticationOptions,
|
||||
createRegistrationOptions,
|
||||
verifyAuthentication,
|
||||
} from "@/lib/auth/passkey";
|
||||
import { createMfaAuthenticationOptions } from "@/lib/auth/mfa/webauthn";
|
||||
import {
|
||||
createMfaAuthenticationOptions,
|
||||
verifyMfaAuthentication,
|
||||
} from "@/lib/auth/mfa/webauthn";
|
||||
import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
recordReproofFailure,
|
||||
throttleReproof,
|
||||
verifyExistingFactorProof,
|
||||
} from "@/lib/auth/existing-factor-proof";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { apiError, apiSuccess, safeJson } from "@/lib/api-response";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
getClientIp,
|
||||
safeJson,
|
||||
} from "@/lib/api-response";
|
||||
import { apiHandler, requireCookieAuth } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { stepUpMintSchema } from "@/lib/validations/step-up";
|
||||
@@ -51,50 +55,25 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const parsed = stepUpMintSchema.safeParse(body);
|
||||
if (!parsed.success) return proofRequired();
|
||||
|
||||
let proved = false;
|
||||
// Every proof counts against the account's shared re-proof bucket and every
|
||||
// refusal is audited. Without both, a stolen session could guess the account
|
||||
// password or a TOTP code here as fast as it could send requests.
|
||||
const ip = getClientIp(request);
|
||||
const limited = await throttleReproof(user.id, ip, "passkey_enroll");
|
||||
if (limited) return limited;
|
||||
|
||||
if (parsed.data.method === "password") {
|
||||
proved = Boolean(
|
||||
user.passwordHash &&
|
||||
(await verifyPassword(user.passwordHash, parsed.data.password)),
|
||||
const outcome = await verifyExistingFactorProof(user, parsed.data);
|
||||
if (!outcome.ok) {
|
||||
await recordReproofFailure(
|
||||
user.id,
|
||||
ip,
|
||||
"passkey_enroll",
|
||||
parsed.data.method,
|
||||
outcome.reason,
|
||||
);
|
||||
} else if (parsed.data.method === "totp") {
|
||||
const result = await verifyMfaFactor(user, "totp", parsed.data.code);
|
||||
proved = result.ok;
|
||||
} else {
|
||||
const expectedType =
|
||||
parsed.data.method === "passkey"
|
||||
? "authentication"
|
||||
: "mfa_authentication";
|
||||
const challenge = await prisma.authChallenge.findUnique({
|
||||
where: { id: parsed.data.challengeId },
|
||||
select: { userId: true, type: true },
|
||||
});
|
||||
|
||||
if (challenge?.userId === user.id && challenge.type === expectedType) {
|
||||
try {
|
||||
if (parsed.data.method === "passkey") {
|
||||
const result = await verifyAuthentication(
|
||||
parsed.data.challengeId,
|
||||
parsed.data.credential,
|
||||
);
|
||||
proved =
|
||||
result.verification.verified && result.passkey.userId === user.id;
|
||||
} else {
|
||||
proved = await verifyMfaAuthentication(
|
||||
parsed.data.challengeId,
|
||||
user.id,
|
||||
parsed.data.credential,
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
proved = false;
|
||||
}
|
||||
}
|
||||
return proofRequired();
|
||||
}
|
||||
|
||||
if (!proved) return proofRequired();
|
||||
|
||||
// A strong existing factor refreshes the session's MFA stamp. Password proof
|
||||
// deliberately clears it: registration is authorized by this single-use,
|
||||
// session-bound challenge but cannot upgrade password-only authentication.
|
||||
|
||||
@@ -14,7 +14,10 @@ import {
|
||||
returnAllZodIssues,
|
||||
safeJson,
|
||||
} from "@/lib/api-response";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import {
|
||||
recordReproofFailure,
|
||||
throttleReproof,
|
||||
} from "@/lib/auth/existing-factor-proof";
|
||||
import {
|
||||
apiHandler,
|
||||
requireFreshMfaIfEnrolled,
|
||||
@@ -35,14 +38,12 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
// contract. Mirrors the step-up on MFA-disable + recovery-code regen.
|
||||
const { user } = await requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
|
||||
const rl = await checkRateLimit(
|
||||
`auth:password:${user.id}`,
|
||||
5,
|
||||
15 * 60 * 1000,
|
||||
);
|
||||
if (!rl.allowed) {
|
||||
return apiError("Too many attempts. Please wait 15 minutes.", 429);
|
||||
}
|
||||
// The current-password check below is a guessing oracle, so it draws on the
|
||||
// account's shared re-proof budget: a guess spent here is one the step-up
|
||||
// mint and the enrollment proofs cannot spend again.
|
||||
const ip = getClientIp(request);
|
||||
const limited = await throttleReproof(user.id, ip, "password_change");
|
||||
if (limited) return limited;
|
||||
|
||||
const { data: body, error: jsonError } = await safeJson(request, {
|
||||
maxBytes: 64 * 1024,
|
||||
@@ -62,6 +63,13 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
|
||||
const currentValid = await verifyPassword(user.passwordHash, currentPassword);
|
||||
if (!currentValid) {
|
||||
await recordReproofFailure(
|
||||
user.id,
|
||||
ip,
|
||||
"password_change",
|
||||
"password",
|
||||
"bad_password",
|
||||
);
|
||||
return apiError("Current password is incorrect", 401);
|
||||
}
|
||||
|
||||
|
||||
@@ -40,16 +40,17 @@ import {
|
||||
} from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import {
|
||||
checkAuthSurfaceRateLimit,
|
||||
checkRateLimit,
|
||||
rateLimitHeaders,
|
||||
} from "@/lib/rate-limit";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
import { verifyAuthentication } from "@/lib/auth/passkey";
|
||||
import { verifyMfaAuthentication } from "@/lib/auth/mfa/webauthn";
|
||||
import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor";
|
||||
import {
|
||||
REPROOF_LIMIT,
|
||||
REPROOF_WINDOW_MS,
|
||||
reproofBucket,
|
||||
verifyExistingFactorProof,
|
||||
} from "@/lib/auth/existing-factor-proof";
|
||||
import {
|
||||
mintStepUpElevation,
|
||||
isFreshFactorMethod,
|
||||
@@ -65,8 +66,8 @@ export const dynamic = "force-dynamic";
|
||||
* against a user-supplied string — the same 5-per-15-minutes the password-change
|
||||
* route uses, for the same reason.
|
||||
*/
|
||||
const MINT_LIMIT = 5;
|
||||
const MINT_WINDOW_MS = 15 * 60 * 1000;
|
||||
const MINT_LIMIT = REPROOF_LIMIT;
|
||||
const MINT_WINDOW_MS = REPROOF_WINDOW_MS;
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const auth = await requireBearerAuth();
|
||||
@@ -91,8 +92,10 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
});
|
||||
}
|
||||
|
||||
// The account's shared re-proof bucket: a guess spent here is a guess the
|
||||
// web enrollment surfaces cannot spend again, and the other way round.
|
||||
const userRl = await checkRateLimit(
|
||||
`auth:step-up:${user.id}`,
|
||||
reproofBucket(user.id),
|
||||
MINT_LIMIT,
|
||||
MINT_WINDOW_MS,
|
||||
);
|
||||
@@ -122,78 +125,14 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
}
|
||||
|
||||
const method: StepUpMethod = parsed.data.method;
|
||||
let proved = false;
|
||||
// The shared verifier: the same challenge ownership and ceremony checks, and
|
||||
// the same confirmed-secret rule for TOTP, as every other re-proof surface.
|
||||
// The replay guard and the accepted-step burn behave exactly as they do at
|
||||
// login and at MFA-disable, so a code spent here cannot be replayed there.
|
||||
const outcome = await verifyExistingFactorProof(user, parsed.data);
|
||||
const proved = outcome.ok;
|
||||
// Audit-only detail. Never reaches the response — see the file header.
|
||||
let failure = "invalid";
|
||||
|
||||
/**
|
||||
* Resolve a WebAuthn challenge and confirm it belongs to THIS account before
|
||||
* any verification runs. Both ceremony helpers resolve a challenge by id
|
||||
* alone, so without this a challenge minted for another account could be
|
||||
* carried into the ceremony.
|
||||
*/
|
||||
const challengeBelongsToCaller = async (id: string): Promise<boolean> => {
|
||||
const row = await prisma.authChallenge.findUnique({
|
||||
where: { id },
|
||||
select: { userId: true },
|
||||
});
|
||||
return Boolean(row && row.userId === user.id);
|
||||
};
|
||||
|
||||
if (parsed.data.method === "password") {
|
||||
if (!user.passwordHash) {
|
||||
// An SSO-provisioned account has no password to re-prove. Same refusal as
|
||||
// a wrong one: the account's credential shape is not a token holder's to
|
||||
// enumerate. `/api/auth/step-up/options` is the honest discovery path —
|
||||
// it 409s when there is no credential of the requested kind either.
|
||||
failure = "no_password";
|
||||
} else {
|
||||
proved = await verifyPassword(user.passwordHash, parsed.data.password);
|
||||
if (!proved) failure = "bad_password";
|
||||
}
|
||||
} else if (parsed.data.method === "totp") {
|
||||
// The shared factor verifier, so the replay guard and the accepted-step
|
||||
// burn behave exactly as they do at login and at MFA-disable. A code spent
|
||||
// here cannot be replayed there.
|
||||
const result = await verifyMfaFactor(user, "totp", parsed.data.code);
|
||||
proved = result.ok;
|
||||
if (!proved) failure = result.replay ? "totp_replay" : "bad_totp";
|
||||
} else if (parsed.data.method === "webauthn") {
|
||||
if (!(await challengeBelongsToCaller(parsed.data.challengeId))) {
|
||||
failure = "foreign_challenge";
|
||||
} else {
|
||||
try {
|
||||
// Scoped to the caller's own second-factor credentials by the verifier.
|
||||
proved = await verifyMfaAuthentication(
|
||||
parsed.data.challengeId,
|
||||
user.id,
|
||||
parsed.data.credential,
|
||||
);
|
||||
if (!proved) failure = "bad_assertion";
|
||||
} catch {
|
||||
// A malformed / expired challenge is a failed attempt, not a 500.
|
||||
failure = "bad_assertion";
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!(await challengeBelongsToCaller(parsed.data.challengeId))) {
|
||||
failure = "foreign_challenge";
|
||||
} else {
|
||||
try {
|
||||
const result = await verifyAuthentication(
|
||||
parsed.data.challengeId,
|
||||
parsed.data.credential,
|
||||
);
|
||||
// Both halves matter: a verified assertion against SOMEONE ELSE'S
|
||||
// passkey proves possession of a factor, just not of this account's.
|
||||
proved =
|
||||
result.verification.verified && result.passkey.userId === user.id;
|
||||
if (!proved) failure = "bad_assertion";
|
||||
} catch {
|
||||
failure = "bad_assertion";
|
||||
}
|
||||
}
|
||||
}
|
||||
const failure = outcome.ok ? "" : outcome.reason;
|
||||
|
||||
if (!proved) {
|
||||
await auditLog("auth.stepup.mint.failed", {
|
||||
|
||||
Reference in new issue
Block a user