Explorar o código

RHL-012 feat(api): add DELETE endpoint for user management with validation and error handling

codeUWE hai 1 mes
pai
achega
2af9f2a7da

+ 56 - 0
app/api/admin/users/[userId]/route.js

@@ -349,3 +349,59 @@ export const PATCH = withErrorHandling(
 	},
 	{ logPrefix: "[api/admin/users/[userId]]" },
 );
+
+export const DELETE = withErrorHandling(
+	async function DELETE(request, ctx) {
+		const session = await getSession();
+
+		if (!session) {
+			throw unauthorized("AUTH_UNAUTHENTICATED", "Unauthorized");
+		}
+
+		requireUserManagement(session);
+
+		const { userId } = await ctx.params;
+
+		if (!userId) {
+			throw badRequest(
+				"VALIDATION_MISSING_PARAM",
+				"Missing required route parameter(s)",
+				{ params: ["userId"] },
+			);
+		}
+
+		if (!OBJECT_ID_RE.test(String(userId))) {
+			throw badRequest("VALIDATION_INVALID_FIELD", "Invalid userId", {
+				field: "userId",
+				value: userId,
+			});
+		}
+
+		// Safety: prevent deleting your own currently active account.
+		if (String(session.userId) === String(userId)) {
+			throw badRequest(
+				"VALIDATION_INVALID_FIELD",
+				"Cannot delete current user",
+				{
+					field: "userId",
+					reason: "SELF_DELETE_FORBIDDEN",
+				},
+			);
+		}
+
+		await getDb();
+
+		const deleted = await User.findByIdAndDelete(String(userId))
+			.select(
+				"_id username email role branchId mustChangePassword createdAt updatedAt",
+			)
+			.exec();
+
+		if (!deleted) {
+			throw notFound("USER_NOT_FOUND", "Not found", { userId: String(userId) });
+		}
+
+		return json({ ok: true, user: toSafeUser(deleted) }, 200);
+	},
+	{ logPrefix: "[api/admin/users/[userId]]" },
+);

+ 174 - 3
app/api/admin/users/[userId]/route.test.js

@@ -22,6 +22,7 @@ vi.mock("@/models/user", () => {
 		default: {
 			findById: vi.fn(),
 			findOne: vi.fn(),
+			findByIdAndDelete: vi.fn(),
 		},
 		USER_ROLES,
 	};
@@ -31,7 +32,7 @@ import { getSession } from "@/lib/auth/session";
 import { getDb } from "@/lib/db";
 import User from "@/models/user";
 
-import { PATCH, dynamic } from "./route.js";
+import { PATCH, DELETE, dynamic } from "./route.js";
 
 function createRequestStub(body) {
 	return {
@@ -257,7 +258,6 @@ describe("PATCH /api/admin/users/[userId]", () => {
 			exec: vi.fn().mockResolvedValue(user),
 		});
 
-		// No uniqueness checks needed here (we only change role + mustChangePassword)
 		const res = await PATCH(
 			createRequestStub({
 				role: "admin",
@@ -270,7 +270,6 @@ describe("PATCH /api/admin/users/[userId]", () => {
 
 		expect(res.status).toBe(200);
 
-		// Role changed => branchId must be cleared
 		expect(user.role).toBe("admin");
 		expect(user.branchId).toBe(null);
 		expect(user.mustChangePassword).toBe(false);
@@ -335,3 +334,175 @@ describe("PATCH /api/admin/users/[userId]", () => {
 		});
 	});
 });
+
+describe("DELETE /api/admin/users/[userId]", () => {
+	beforeEach(() => {
+		vi.clearAllMocks();
+		getDb.mockResolvedValue({});
+	});
+
+	it("returns 401 when unauthenticated", async () => {
+		getSession.mockResolvedValue(null);
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "507f1f77bcf86cd799439011" }),
+			},
+		);
+
+		expect(res.status).toBe(401);
+		expect(await res.json()).toEqual({
+			error: { message: "Unauthorized", code: "AUTH_UNAUTHENTICATED" },
+		});
+	});
+
+	it("returns 403 when authenticated but not allowed (admin)", async () => {
+		getSession.mockResolvedValue({
+			userId: "u1",
+			role: "admin",
+			branchId: null,
+			email: "admin@example.com",
+		});
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "507f1f77bcf86cd799439011" }),
+			},
+		);
+
+		expect(res.status).toBe(403);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Forbidden",
+				code: "AUTH_FORBIDDEN_USER_MANAGEMENT",
+			},
+		});
+
+		expect(User.findByIdAndDelete).not.toHaveBeenCalled();
+	});
+
+	it("returns 400 for invalid userId", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "dev",
+			branchId: null,
+			email: "dev@example.com",
+		});
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "nope" }),
+			},
+		);
+
+		expect(res.status).toBe(400);
+		expect(await res.json()).toMatchObject({
+			error: { code: "VALIDATION_INVALID_FIELD" },
+		});
+	});
+
+	it("returns 400 when trying to delete the current user (self delete)", async () => {
+		getSession.mockResolvedValue({
+			userId: "507f1f77bcf86cd799439011",
+			role: "superadmin",
+			branchId: null,
+			email: "superadmin@example.com",
+		});
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "507f1f77bcf86cd799439011" }),
+			},
+		);
+
+		expect(res.status).toBe(400);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Cannot delete current user",
+				code: "VALIDATION_INVALID_FIELD",
+				details: { field: "userId", reason: "SELF_DELETE_FORBIDDEN" },
+			},
+		});
+
+		expect(User.findByIdAndDelete).not.toHaveBeenCalled();
+	});
+
+	it("returns 404 when user does not exist", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "dev",
+			branchId: null,
+			email: "dev@example.com",
+		});
+
+		User.findByIdAndDelete.mockReturnValue({
+			select: vi.fn().mockReturnThis(),
+			exec: vi.fn().mockResolvedValue(null),
+		});
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "507f1f77bcf86cd799439099" }),
+			},
+		);
+
+		expect(res.status).toBe(404);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Not found",
+				code: "USER_NOT_FOUND",
+				details: { userId: "507f1f77bcf86cd799439099" },
+			},
+		});
+	});
+
+	it("returns 200 and deleted user payload on success", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "superadmin",
+			branchId: null,
+			email: "superadmin@example.com",
+		});
+
+		User.findByIdAndDelete.mockReturnValue({
+			select: vi.fn().mockReturnThis(),
+			exec: vi.fn().mockResolvedValue({
+				_id: "507f1f77bcf86cd799439099",
+				username: "todelete",
+				email: "todelete@example.com",
+				role: "branch",
+				branchId: "NL01",
+				mustChangePassword: true,
+				createdAt: new Date("2026-02-01T10:00:00.000Z"),
+				updatedAt: new Date("2026-02-02T10:00:00.000Z"),
+			}),
+		});
+
+		const res = await DELETE(
+			new Request("http://localhost/api/admin/users/x"),
+			{
+				params: Promise.resolve({ userId: "507f1f77bcf86cd799439099" }),
+			},
+		);
+
+		expect(res.status).toBe(200);
+
+		const body = await res.json();
+		expect(body).toMatchObject({
+			ok: true,
+			user: {
+				id: "507f1f77bcf86cd799439099",
+				username: "todelete",
+				email: "todelete@example.com",
+				role: "branch",
+				branchId: "NL01",
+				mustChangePassword: true,
+			},
+		});
+	});
+});

+ 27 - 21
app/api/admin/users/route.js

@@ -167,8 +167,6 @@ function toSafeUser(doc) {
 }
 
 function pickDuplicateField(err) {
-	// Mongo duplicate key errors are typically: err.code === 11000
-	// and may include keyPattern / keyValue.
 	if (!err || typeof err !== "object") return null;
 
 	const keyValue =
@@ -317,7 +315,7 @@ export const POST = withErrorHandling(
 		if (!isNonEmptyString(roleRaw)) missing.push("role");
 		if (!isNonEmptyString(initialPasswordRaw)) missing.push("initialPassword");
 
-		// branchId required iff role is branch (only if role is present)
+		// branchId required iff role is branch
 		if (role === USER_ROLES.BRANCH) {
 			if (!isNonEmptyString(branchIdRaw)) missing.push("branchId");
 		}
@@ -328,7 +326,6 @@ export const POST = withErrorHandling(
 			});
 		}
 
-		// Validate role
 		if (!ALLOWED_ROLES.has(role)) {
 			throw badRequest("VALIDATION_INVALID_FIELD", "Invalid role", {
 				field: "role",
@@ -378,25 +375,34 @@ export const POST = withErrorHandling(
 
 		await getDb();
 
-		// Uniqueness checks (explicit, predictable errors)
-		const existingUsername = await User.findOne({ username })
-			.select("_id")
-			.exec();
-
-		if (existingUsername) {
-			throw badRequest("VALIDATION_INVALID_FIELD", "Username already exists", {
-				field: "username",
-				value: username,
-			});
+		// Uniqueness checks (return one field OR both fields)
+		const [existingUsername, existingEmail] = await Promise.all([
+			User.findOne({ username }).select("_id").exec(),
+			User.findOne({ email }).select("_id").exec(),
+		]);
+
+		const duplicateFields = [];
+		if (existingUsername) duplicateFields.push("username");
+		if (existingEmail) duplicateFields.push("email");
+
+		if (duplicateFields.length === 1) {
+			const field = duplicateFields[0];
+
+			throw badRequest(
+				"VALIDATION_INVALID_FIELD",
+				field === "username"
+					? "Username already exists"
+					: "Email already exists",
+				{ field },
+			);
 		}
 
-		const existingEmail = await User.findOne({ email }).select("_id").exec();
-
-		if (existingEmail) {
-			throw badRequest("VALIDATION_INVALID_FIELD", "Email already exists", {
-				field: "email",
-				value: email,
-			});
+		if (duplicateFields.length > 1) {
+			throw badRequest(
+				"VALIDATION_INVALID_FIELD",
+				"Username and email already exist",
+				{ fields: duplicateFields },
+			);
 		}
 
 		const passwordHash = await bcrypt.hash(initialPassword, BCRYPT_SALT_ROUNDS);

+ 168 - 1
app/api/admin/users/route.test.js

@@ -159,6 +159,23 @@ describe("GET /api/admin/users", () => {
 		const body = await res.json();
 
 		expect(body.items).toHaveLength(2);
+		expect(body.items[0]).toMatchObject({
+			id: "507f1f77bcf86cd799439013",
+			username: "u3",
+			email: "u3@example.com",
+			role: "admin",
+			branchId: null,
+			mustChangePassword: false,
+		});
+		expect(body.items[1]).toMatchObject({
+			id: "507f1f77bcf86cd799439012",
+			username: "u2",
+			email: "u2@example.com",
+			role: "branch",
+			branchId: "NL01",
+			mustChangePassword: true,
+		});
+
 		expect(body.nextCursor).toBe(buildCursor("507f1f77bcf86cd799439012"));
 	});
 });
@@ -336,9 +353,109 @@ describe("POST /api/admin/users", () => {
 			requireNumber: true,
 		});
 		expect(Array.isArray(body.error.details.reasons)).toBe(true);
+
+		expect(User.create).not.toHaveBeenCalled();
 	});
 
-	it("returns 200 and creates user with hashed password + mustChangePassword=true", async () => {
+	it("returns 400 when username already exists", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "superadmin",
+			branchId: null,
+			email: "superadmin@example.com",
+		});
+
+		User.findOne.mockImplementation((query) => {
+			if (query?.username) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue({ _id: "x" }),
+				};
+			}
+			if (query?.email) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue(null),
+				};
+			}
+			return {
+				select: vi.fn().mockReturnThis(),
+				exec: vi.fn().mockResolvedValue(null),
+			};
+		});
+
+		const res = await POST(
+			createRequestStub({
+				username: "NewUser",
+				email: "new@example.com",
+				role: "admin",
+				initialPassword: "StrongPassword123",
+			}),
+		);
+
+		expect(res.status).toBe(400);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Username already exists",
+				code: "VALIDATION_INVALID_FIELD",
+				details: { field: "username" },
+			},
+		});
+
+		expect(User.create).not.toHaveBeenCalled();
+		expect(bcryptHash).not.toHaveBeenCalled();
+	});
+
+	it("returns 400 when email already exists", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "superadmin",
+			branchId: null,
+			email: "superadmin@example.com",
+		});
+
+		User.findOne.mockImplementation((query) => {
+			if (query?.username) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue(null),
+				};
+			}
+			if (query?.email) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue({ _id: "y" }),
+				};
+			}
+			return {
+				select: vi.fn().mockReturnThis(),
+				exec: vi.fn().mockResolvedValue(null),
+			};
+		});
+
+		const res = await POST(
+			createRequestStub({
+				username: "newuser",
+				email: "NEW@EXAMPLE.COM",
+				role: "admin",
+				initialPassword: "StrongPassword123",
+			}),
+		);
+
+		expect(res.status).toBe(400);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Email already exists",
+				code: "VALIDATION_INVALID_FIELD",
+				details: { field: "email" },
+			},
+		});
+
+		expect(User.create).not.toHaveBeenCalled();
+		expect(bcryptHash).not.toHaveBeenCalled();
+	});
+
+	it("returns 400 when username AND email already exist", async () => {
 		getSession.mockResolvedValue({
 			userId: "u2",
 			role: "superadmin",
@@ -347,6 +464,56 @@ describe("POST /api/admin/users", () => {
 		});
 
 		User.findOne.mockImplementation((query) => {
+			if (query?.username) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue({ _id: "x" }),
+				};
+			}
+			if (query?.email) {
+				return {
+					select: vi.fn().mockReturnThis(),
+					exec: vi.fn().mockResolvedValue({ _id: "y" }),
+				};
+			}
+			return {
+				select: vi.fn().mockReturnThis(),
+				exec: vi.fn().mockResolvedValue(null),
+			};
+		});
+
+		const res = await POST(
+			createRequestStub({
+				username: "newuser",
+				email: "new@example.com",
+				role: "admin",
+				initialPassword: "StrongPassword123",
+			}),
+		);
+
+		expect(res.status).toBe(400);
+		expect(await res.json()).toEqual({
+			error: {
+				message: "Username and email already exist",
+				code: "VALIDATION_INVALID_FIELD",
+				details: { fields: ["username", "email"] },
+			},
+		});
+
+		expect(User.create).not.toHaveBeenCalled();
+		expect(bcryptHash).not.toHaveBeenCalled();
+	});
+
+	it("returns 200 and creates user with hashed password + mustChangePassword=true", async () => {
+		getSession.mockResolvedValue({
+			userId: "u2",
+			role: "superadmin",
+			branchId: null,
+			email: "superadmin@example.com",
+		});
+
+		// No duplicates
+		User.findOne.mockImplementation(() => {
 			return {
 				select: vi.fn().mockReturnThis(),
 				exec: vi.fn().mockResolvedValue(null),