v1.2.0:3 — close login timing oracle, enforce exerciseId ownership on workout writes
Two P3 multi-user hardening fixes from the 2026-06-13 full-eval. Login timing oracle: both login paths (the UI server action and POST /api/auth) returned immediately on an unknown email but ran bcrypt.compare when the email matched a user, so response latency revealed which emails have accounts. New verifyPasswordOrDummy() in lib/auth runs bcrypt against a fixed dummy hash when there is no user, so every attempt spends exactly one bcrypt; the two error branches in each route collapse into one. exerciseId ownership: exercises are per-user, but the workout create / PATCH (set-replace) / add-sets and CSV import-save routes wrote SetLogs from a client-supplied exerciseId with no ownership check — letting a user attach another user's exercise to their own workout, which leaks that exercise's name/notes on fetch and wires up a cross-user onDelete: Cascade link. All four now reject unowned ids with 400 via the shared lib/exerciseOwnership helper; the pre-existing inline checks in both programs routes are refactored onto the same helper. App-code only — no schema, no API contract change, no data migration.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { NextRequest, NextResponse } from 'next/server';
|
||||
import { z } from 'zod';
|
||||
import { verifyPassword, createSession } from '@/lib/auth';
|
||||
import { verifyPasswordOrDummy, createSession } from '@/lib/auth';
|
||||
import { prisma } from '@/lib/prisma';
|
||||
import { readJsonBody } from '@/lib/http';
|
||||
import { rateLimit, clientIpFromHeaders } from '@/lib/rateLimit';
|
||||
@@ -33,17 +33,14 @@ export async function POST(request: NextRequest) {
|
||||
where: { email },
|
||||
});
|
||||
|
||||
if (!user) {
|
||||
return NextResponse.json(
|
||||
{ error: 'Invalid email or password' },
|
||||
{ status: 401 }
|
||||
);
|
||||
}
|
||||
// Always run a bcrypt compare (against a dummy hash when the email is
|
||||
// unknown) so response time doesn't reveal whether an account exists.
|
||||
const isValid = await verifyPasswordOrDummy(
|
||||
password,
|
||||
user?.passwordHash ?? null,
|
||||
);
|
||||
|
||||
// Verify the password
|
||||
const isValid = await verifyPassword(password, user.passwordHash);
|
||||
|
||||
if (!isValid) {
|
||||
if (!user || !isValid) {
|
||||
return NextResponse.json(
|
||||
{ error: 'Invalid email or password' },
|
||||
{ status: 401 }
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Prisma } from "@prisma/client";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
import { getProgramById } from "@/lib/db/programs";
|
||||
|
||||
/**
|
||||
@@ -86,25 +87,20 @@ export async function PATCH(
|
||||
const body = await readJsonBody(request);
|
||||
const validated = patchSchema.parse(body);
|
||||
|
||||
// If replacing the tree, verify exercise ownership.
|
||||
// If replacing the tree, verify exercise ownership
|
||||
// (see lib/exerciseOwnership).
|
||||
if (validated.weeks) {
|
||||
const allExerciseIds = new Set<string>();
|
||||
for (const w of validated.weeks)
|
||||
for (const d of w.days)
|
||||
for (const ex of d.exercises) allExerciseIds.add(ex.exerciseId);
|
||||
if (allExerciseIds.size > 0) {
|
||||
const owned = await prisma.exercise.findMany({
|
||||
where: { userId: user.id, id: { in: Array.from(allExerciseIds) } },
|
||||
select: { id: true },
|
||||
});
|
||||
const ownedIds = new Set(owned.map((e) => e.id));
|
||||
const bad = Array.from(allExerciseIds).filter((id) => !ownedIds.has(id));
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
|
||||
const bad = await findUnownedExerciseIds(user.id, allExerciseIds);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Prisma } from "@prisma/client";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
import { getPrograms } from "@/lib/db/programs";
|
||||
|
||||
/**
|
||||
@@ -65,25 +66,19 @@ export async function POST(request: NextRequest) {
|
||||
const body = await readJsonBody(request);
|
||||
const validated = createProgramSchema.parse(body);
|
||||
|
||||
// Verify any referenced exerciseIds belong to this user.
|
||||
// Verify any referenced exerciseIds belong to this user
|
||||
// (see lib/exerciseOwnership).
|
||||
const allExerciseIds = new Set<string>();
|
||||
for (const w of validated.weeks)
|
||||
for (const d of w.days)
|
||||
for (const ex of d.exercises) allExerciseIds.add(ex.exerciseId);
|
||||
|
||||
if (allExerciseIds.size > 0) {
|
||||
const owned = await prisma.exercise.findMany({
|
||||
where: { userId: user.id, id: { in: Array.from(allExerciseIds) } },
|
||||
select: { id: true },
|
||||
});
|
||||
const ownedIds = new Set(owned.map((e) => e.id));
|
||||
const bad = Array.from(allExerciseIds).filter((id) => !ownedIds.has(id));
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
const bad = await findUnownedExerciseIds(user.id, allExerciseIds);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 },
|
||||
);
|
||||
}
|
||||
|
||||
const program = await prisma.$transaction(async (tx) => {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
import { z } from "zod";
|
||||
|
||||
// GET: Get workout by ID
|
||||
@@ -101,6 +102,21 @@ export async function PATCH(
|
||||
const body = await readJsonBody(request);
|
||||
const validated = updateWorkoutSchema.parse(body);
|
||||
|
||||
// When replacing sets, every referenced exercise must belong to this
|
||||
// user (see lib/exerciseOwnership).
|
||||
if (validated.sets) {
|
||||
const bad = await findUnownedExerciseIds(
|
||||
user.id,
|
||||
validated.sets.map((s) => s.exerciseId),
|
||||
);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const workoutData: Record<string, unknown> = {};
|
||||
if (validated.name !== undefined) workoutData.name = validated.name;
|
||||
if (validated.notes !== undefined) workoutData.notes = validated.notes || null;
|
||||
|
||||
@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
import { z } from "zod";
|
||||
|
||||
const addSetsSchema = z.object({
|
||||
@@ -51,6 +52,15 @@ export async function POST(
|
||||
const body = await readJsonBody(request);
|
||||
const validated = addSetsSchema.parse(body);
|
||||
|
||||
// The exercise must belong to this user (see lib/exerciseOwnership).
|
||||
const bad = await findUnownedExerciseIds(user.id, [validated.exerciseId]);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
|
||||
// Delete existing sets for this exercise in this workout (replace mode)
|
||||
await prisma.setLog.deleteMany({
|
||||
where: {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { z } from "zod";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
|
||||
const setSchema = z.object({
|
||||
reps: z.number().int().positive().optional(),
|
||||
@@ -44,6 +45,22 @@ export async function POST(request: Request) {
|
||||
const body = await readJsonBody(request);
|
||||
const validated = saveImportSchema.parse(body);
|
||||
|
||||
// An explicitly-matched `existingExerciseId` must belong to this user;
|
||||
// name-matched and newly-created exercises are owned by construction
|
||||
// (see lib/exerciseOwnership).
|
||||
const claimedIds = validated.workouts.flatMap((w) =>
|
||||
w.exercises
|
||||
.map((e) => e.existingExerciseId)
|
||||
.filter((id): id is string => !!id)
|
||||
);
|
||||
const bad = await findUnownedExerciseIds(user.id, claimedIds);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
|
||||
// Load all user exercises for matching
|
||||
const existingExercises = await prisma.exercise.findMany({
|
||||
where: { userId: user.id },
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Prisma } from "@prisma/client";
|
||||
import { getCurrentUser } from "@/lib/auth";
|
||||
import { prisma } from "@/lib/prisma";
|
||||
import { readJsonBody } from "@/lib/http";
|
||||
import { findUnownedExerciseIds } from "@/lib/exerciseOwnership";
|
||||
|
||||
// Schema now supports creating empty workouts (just date) or with sets
|
||||
const createWorkoutSchema = z.object({
|
||||
@@ -140,6 +141,19 @@ export async function POST(request: NextRequest) {
|
||||
const body = await readJsonBody(request);
|
||||
const validated = createWorkoutSchema.parse(body);
|
||||
|
||||
// Every referenced exercise must belong to this user (see
|
||||
// lib/exerciseOwnership).
|
||||
const bad = await findUnownedExerciseIds(
|
||||
user.id,
|
||||
validated.sets.map((s) => s.exerciseId),
|
||||
);
|
||||
if (bad.length > 0) {
|
||||
return NextResponse.json(
|
||||
{ error: "Some exerciseIds don't exist in your library", details: bad },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
|
||||
const workoutDate = validated.date ? new Date(validated.date) : new Date();
|
||||
|
||||
const createData: Prisma.WorkoutCreateInput = {
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { redirect } from 'next/navigation';
|
||||
import { cookies, headers } from 'next/headers';
|
||||
import { verifyPassword, createSession } from '@/lib/auth';
|
||||
import { verifyPasswordOrDummy, createSession } from '@/lib/auth';
|
||||
import { prisma } from '@/lib/prisma';
|
||||
import { rateLimit, clientIpFromHeaders } from '@/lib/rateLimit';
|
||||
|
||||
@@ -24,14 +24,14 @@ export async function loginAction(email: string, password: string) {
|
||||
where: { email },
|
||||
});
|
||||
|
||||
if (!user) {
|
||||
return { error: 'Invalid email or password' };
|
||||
}
|
||||
// Always run a bcrypt compare (against a dummy hash when the email is
|
||||
// unknown) so response time doesn't reveal whether an account exists.
|
||||
const isValid = await verifyPasswordOrDummy(
|
||||
password,
|
||||
user?.passwordHash ?? null,
|
||||
);
|
||||
|
||||
// Verify the password
|
||||
const isValid = await verifyPassword(password, user.passwordHash);
|
||||
|
||||
if (!isValid) {
|
||||
if (!user || !isValid) {
|
||||
return { error: 'Invalid email or password' };
|
||||
}
|
||||
|
||||
|
||||
@@ -24,6 +24,33 @@ export async function verifyPassword(
|
||||
return bcrypt.compare(password, hash);
|
||||
}
|
||||
|
||||
/**
|
||||
* A valid bcrypt hash (cost 10, matching real password hashes) of a
|
||||
* throwaway string. Not a secret — it verifies no real password. Its only
|
||||
* job is to give the no-such-user login path something to bcrypt against.
|
||||
*/
|
||||
const DUMMY_PASSWORD_HASH =
|
||||
"$2b$10$4Q3ukhdLWRqxvYHp4JezhuSPskBFVXvewuUhhfUML64nh4xBuYyPC";
|
||||
|
||||
/**
|
||||
* Verify a password against a user's hash, or against a fixed dummy hash
|
||||
* when `hash` is null (no user matched the email). Either way exactly one
|
||||
* bcrypt.compare runs, so an unknown-email attempt costs the same wall
|
||||
* time as a real one — closing the timing oracle that would otherwise let
|
||||
* an attacker enumerate which emails have accounts. The null-hash path
|
||||
* always returns false.
|
||||
*/
|
||||
export async function verifyPasswordOrDummy(
|
||||
password: string,
|
||||
hash: string | null,
|
||||
): Promise<boolean> {
|
||||
if (hash === null) {
|
||||
await bcrypt.compare(password, DUMMY_PASSWORD_HASH);
|
||||
return false;
|
||||
}
|
||||
return bcrypt.compare(password, hash);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a session token for a user (30-day expiration).
|
||||
*
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { prisma } from "./prisma";
|
||||
|
||||
/**
|
||||
* Return the subset of `exerciseIds` that do NOT belong to `userId`.
|
||||
*
|
||||
* Exercises are per-user (`Exercise.userId`, `@@unique([userId, name])`) —
|
||||
* even curated-library entries are copied per account. Routes that write
|
||||
* SetLogs from a client-supplied exerciseId must check ownership first;
|
||||
* otherwise a user could attach another user's exercise to their own
|
||||
* workout, which leaks that exercise's name/notes back on fetch and wires
|
||||
* up a cross-user `onDelete: Cascade` dependency.
|
||||
*
|
||||
* Unknown ids and ids owned by someone else are deliberately
|
||||
* indistinguishable in the result, so a caller's 400 can't be used to
|
||||
* probe which exerciseIds exist. An empty input returns an empty array
|
||||
* (no query).
|
||||
*/
|
||||
export async function findUnownedExerciseIds(
|
||||
userId: string,
|
||||
exerciseIds: Iterable<string>,
|
||||
): Promise<string[]> {
|
||||
const ids = Array.from(new Set(exerciseIds));
|
||||
if (ids.length === 0) return [];
|
||||
|
||||
const owned = await prisma.exercise.findMany({
|
||||
where: { userId, id: { in: ids } },
|
||||
select: { id: true },
|
||||
});
|
||||
const ownedIds = new Set(owned.map((e) => e.id));
|
||||
|
||||
return ids.filter((id) => !ownedIds.has(id));
|
||||
}
|
||||
@@ -1,5 +1,9 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { hashPassword, verifyPassword } from '@/lib/auth';
|
||||
import {
|
||||
hashPassword,
|
||||
verifyPassword,
|
||||
verifyPasswordOrDummy,
|
||||
} from '@/lib/auth';
|
||||
|
||||
// Pure-function bits of lib/auth.ts (no Prisma, no cookies).
|
||||
|
||||
@@ -27,3 +31,28 @@ describe('hashPassword / verifyPassword', () => {
|
||||
expect(hash.startsWith('$2')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('verifyPasswordOrDummy', () => {
|
||||
it('verifies a correct password against a real hash', async () => {
|
||||
const hash = await hashPassword('hunter2');
|
||||
expect(await verifyPasswordOrDummy('hunter2', hash)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a wrong password against a real hash', async () => {
|
||||
const hash = await hashPassword('hunter2');
|
||||
expect(await verifyPasswordOrDummy('wrong', hash)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false without throwing when there is no user (null hash)', async () => {
|
||||
expect(await verifyPasswordOrDummy('anything', null)).toBe(false);
|
||||
});
|
||||
|
||||
it('still spends bcrypt time on the null-hash path (timing-oracle guard)', async () => {
|
||||
// A real cost-10 bcrypt.compare is tens of ms; a path that skipped
|
||||
// bcrypt would return in well under 1ms. 5ms is a safe lower bound,
|
||||
// so this fails if someone removes the dummy compare.
|
||||
const start = Date.now();
|
||||
await verifyPasswordOrDummy('anything', null);
|
||||
expect(Date.now() - start).toBeGreaterThan(5);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,6 +13,9 @@ import { NextRequest } from 'next/server';
|
||||
import { prisma } from '@/lib/prisma';
|
||||
import { GET as getExercises, POST as postExercise } from '@/app/api/exercises/route';
|
||||
import { POST as postWorkout, GET as getWorkouts } from '@/app/api/workouts/route';
|
||||
import { POST as postSets } from '@/app/api/workouts/[id]/sets/route';
|
||||
import { PATCH as patchWorkout } from '@/app/api/workouts/[id]/route';
|
||||
import { POST as postImportSave } from '@/app/api/workouts/import/save/route';
|
||||
|
||||
// `NextRequest` accepts a slightly stricter RequestInit (no `signal:
|
||||
// null`), so cast the standard RequestInit to the constructor's
|
||||
@@ -321,4 +324,110 @@ describe('POST /api/workouts', () => {
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/invalid/i);
|
||||
});
|
||||
|
||||
it("rejects a set referencing another user's exerciseId with 400", async () => {
|
||||
const alice = await makeUser({ email: 'a@x' });
|
||||
const bob = await makeUser({ email: 'b@x' });
|
||||
const bobExercise = await prisma.exercise.create({
|
||||
data: { userId: bob.id, name: 'Bob Squat', type: 'barbell', muscleGroups: '[]' },
|
||||
});
|
||||
getCurrentUserMock.mockResolvedValue(alice);
|
||||
const res = await postWorkout(
|
||||
jsonReq('http://x/api/workouts', {
|
||||
name: 'Steal attempt',
|
||||
sets: [
|
||||
{ exerciseId: bobExercise.id, setNumber: 1, reps: 5, weightUnit: 'lbs' },
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/library/i);
|
||||
// Nothing was written — the guard runs before any create.
|
||||
expect(await prisma.workout.count()).toBe(0);
|
||||
expect(await prisma.setLog.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/workouts/[id]/sets', () => {
|
||||
it("rejects adding sets for another user's exerciseId with 400", async () => {
|
||||
const alice = await makeUser({ email: 'a@x' });
|
||||
const bob = await makeUser({ email: 'b@x' });
|
||||
const aliceWorkout = await prisma.workout.create({
|
||||
data: { userId: alice.id, date: new Date(), name: 'Leg Day' },
|
||||
});
|
||||
const bobExercise = await prisma.exercise.create({
|
||||
data: { userId: bob.id, name: 'Bob Squat', type: 'barbell', muscleGroups: '[]' },
|
||||
});
|
||||
getCurrentUserMock.mockResolvedValue(alice);
|
||||
const res = await postSets(
|
||||
jsonReq('http://x/api/workouts/' + aliceWorkout.id + '/sets', {
|
||||
exerciseId: bobExercise.id,
|
||||
sets: [{ setNumber: 1, reps: 5, weightUnit: 'lbs' }],
|
||||
}),
|
||||
{ params: Promise.resolve({ id: aliceWorkout.id }) },
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/library/i);
|
||||
expect(await prisma.setLog.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PATCH /api/workouts/[id]', () => {
|
||||
it("rejects replacing sets with another user's exerciseId (400, nothing written)", async () => {
|
||||
const alice = await makeUser({ email: 'a@x' });
|
||||
const bob = await makeUser({ email: 'b@x' });
|
||||
const aliceWorkout = await prisma.workout.create({
|
||||
data: { userId: alice.id, date: new Date(), name: 'Day' },
|
||||
});
|
||||
const bobExercise = await prisma.exercise.create({
|
||||
data: { userId: bob.id, name: 'Bob Squat', type: 'barbell', muscleGroups: '[]' },
|
||||
});
|
||||
getCurrentUserMock.mockResolvedValue(alice);
|
||||
const res = await patchWorkout(
|
||||
jsonReq(
|
||||
'http://x/api/workouts/' + aliceWorkout.id,
|
||||
{
|
||||
sets: [
|
||||
{ exerciseId: bobExercise.id, setNumber: 1, reps: 5, weightUnit: 'lbs' },
|
||||
],
|
||||
},
|
||||
{ method: 'PATCH' },
|
||||
),
|
||||
{ params: Promise.resolve({ id: aliceWorkout.id }) },
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/library/i);
|
||||
// The guard runs before the set-replace transaction.
|
||||
expect(await prisma.setLog.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/workouts/import/save', () => {
|
||||
it("rejects an existingExerciseId owned by another user (400)", async () => {
|
||||
const alice = await makeUser({ email: 'a@x' });
|
||||
const bob = await makeUser({ email: 'b@x' });
|
||||
const bobExercise = await prisma.exercise.create({
|
||||
data: { userId: bob.id, name: 'Bob Squat', type: 'barbell', muscleGroups: '[]' },
|
||||
});
|
||||
getCurrentUserMock.mockResolvedValue(alice);
|
||||
const res = await postImportSave(
|
||||
jsonReq('http://x/api/workouts/import/save', {
|
||||
workouts: [
|
||||
{
|
||||
date: new Date().toISOString(),
|
||||
exercises: [
|
||||
{ name: 'Squat', existingExerciseId: bobExercise.id, sets: [{ reps: 5 }] },
|
||||
],
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(res.status).toBe(400);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/library/i);
|
||||
expect(await prisma.workout.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user