diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ffa99fee..2a1e3de41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed - The log line for a rejected proxy login now shows the source address, so you can add it to TRUSTED_PROXY_AUTH_TRUSTED_PROXIES +- Deleting your own account now removes all of its data, like an admin delete does, instead of leaving some behind ## 26.10.5 diff --git a/src/backend/database/routes/user-delete-account-routes.ts b/src/backend/database/routes/user-delete-account-routes.ts new file mode 100644 index 000000000..27fbf48ab --- /dev/null +++ b/src/backend/database/routes/user-delete-account-routes.ts @@ -0,0 +1,95 @@ +import type { AuthenticatedRequest } from "../../../types/index.js"; +import type { RequestHandler, Router } from "express"; +import bcrypt from "bcryptjs"; +import { authLogger } from "../../utils/logger.js"; +import { createCurrentUserRepository } from "../repositories/factory.js"; +import { deleteUserAndRelatedData } from "./delete-user-data.js"; + +function isNonEmptyString(val: unknown): val is string { + return typeof val === "string" && val.trim().length > 0; +} + +export function registerUserDeleteAccountRoutes( + router: Router, + authenticateJWT: RequestHandler, +): void { + /** + * @openapi + * /users/delete-account: + * delete: + * summary: Delete user account + * description: Deletes the authenticated user's account and all of their data. + * tags: + * - Users + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * properties: + * password: + * type: string + * responses: + * 200: + * description: Account deleted successfully. + * 400: + * description: Password is required. + * 401: + * description: Incorrect password. + * 403: + * description: Cannot delete external authentication accounts or the last admin user. + * 404: + * description: User not found. + * 500: + * description: Failed to delete account. + */ + router.delete("/delete-account", authenticateJWT, async (req, res) => { + const userId = (req as AuthenticatedRequest).userId; + const { password } = req.body ?? {}; + + if (!isNonEmptyString(password)) { + return res + .status(400) + .json({ error: "Password is required to delete account" }); + } + + try { + const userRepository = createCurrentUserRepository(); + const userRecord = await userRepository.findById(userId); + if (!userRecord) { + return res.status(404).json({ error: "User not found" }); + } + + if (userRecord.isExternal) { + return res.status(403).json({ + error: + "Cannot delete external authentication accounts through this endpoint", + }); + } + + const isMatch = await bcrypt.compare(password, userRecord.passwordHash); + if (!isMatch) { + authLogger.warn( + `Incorrect password provided for account deletion: ${userRecord.username}`, + ); + return res.status(401).json({ error: "Incorrect password" }); + } + + if (userRecord.isAdmin && (await userRepository.countAdmins()) <= 1) { + return res + .status(403) + .json({ error: "Cannot delete the last admin user" }); + } + + // Same cleanup as an admin delete, so plugins hear user.deleted too. + await deleteUserAndRelatedData(userId); + + authLogger.success(`User account deleted: ${userRecord.username}`); + res.json({ message: "Account deleted successfully" }); + } catch (err) { + authLogger.error("Failed to delete user account", err); + res.status(500).json({ error: "Failed to delete account" }); + } + }); +} diff --git a/src/backend/database/routes/users.ts b/src/backend/database/routes/users.ts index 598a6b40e..fbb49ccae 100644 --- a/src/backend/database/routes/users.ts +++ b/src/backend/database/routes/users.ts @@ -31,6 +31,7 @@ import { shownIdentity } from "../../sync/shown-identity.js"; import { registerUserExternalAccountRoutes } from "./user-external-account-routes.js"; import { registerUserPasswordResetRoutes } from "./user-password-reset-routes.js"; import { registerUserAdminRoutes } from "./user-admin-routes.js"; +import { registerUserDeleteAccountRoutes } from "./user-delete-account-routes.js"; import { registerUserDataAccessRoutes } from "./user-data-access-routes.js"; import { listExternalLoginMethods, registerAuthRoutes } from "./auth-routes.js"; import { registerAuthCompatRoutes } from "./auth-compat-routes.js"; @@ -1229,86 +1230,7 @@ router.patch("/password-reset-allowed", authenticateJWT, async (req, res) => { } }); -/** - * @openapi - * /users/delete-account: - * delete: - * summary: Delete user account - * description: Deletes the authenticated user's account. - * tags: - * - Users - * requestBody: - * required: true - * content: - * application/json: - * schema: - * type: object - * properties: - * password: - * type: string - * responses: - * 200: - * description: Account deleted successfully. - * 400: - * description: Password is required. - * 401: - * description: Incorrect password. - * 403: - * description: Cannot delete external authentication accounts or the last admin user. - * 404: - * description: User not found. - * 500: - * description: Failed to delete account. - */ -router.delete("/delete-account", authenticateJWT, async (req, res) => { - const userId = (req as AuthenticatedRequest).userId; - const { password } = req.body; - - if (!isNonEmptyString(password)) { - return res - .status(400) - .json({ error: "Password is required to delete account" }); - } - - try { - const userRecord = await findCurrentUser(userId); - if (!userRecord) { - return res.status(404).json({ error: "User not found" }); - } - - if (userRecord.isExternal) { - return res.status(403).json({ - error: - "Cannot delete external authentication accounts through this endpoint", - }); - } - - const isMatch = await bcrypt.compare(password, userRecord.passwordHash); - if (!isMatch) { - authLogger.warn( - `Incorrect password provided for account deletion: ${userRecord.username}`, - ); - return res.status(401).json({ error: "Incorrect password" }); - } - - if (userRecord.isAdmin) { - const adminCount = await createCurrentUserRepository().countAdmins(); - if (adminCount <= 1) { - return res - .status(403) - .json({ error: "Cannot delete the last admin user" }); - } - } - - await createCurrentUserRepository().delete(userId); - - authLogger.success(`User account deleted: ${userRecord.username}`); - res.json({ message: "Account deleted successfully" }); - } catch (err) { - authLogger.error("Failed to delete user account", err); - res.status(500).json({ error: "Failed to delete account" }); - } -}); +registerUserDeleteAccountRoutes(router, authenticateJWT); registerUserPasswordResetRoutes(router, { authManager }); diff --git a/src/backend/tests/database/routes/user-delete-account-routes.test.ts b/src/backend/tests/database/routes/user-delete-account-routes.test.ts new file mode 100644 index 000000000..fbd90c1c1 --- /dev/null +++ b/src/backend/tests/database/routes/user-delete-account-routes.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { Request, RequestHandler, Response } from "express"; + +const state = vi.hoisted(() => ({ + users: new Map< + string, + { + id: string; + username: string; + isAdmin: boolean; + isExternal: boolean; + passwordHash: string; + } + >(), + adminCount: 1, + deleted: [] as string[], +})); + +vi.mock("../../../database/db/index.js", () => ({ db: {} })); + +vi.mock("../../../utils/logger.js", () => ({ + authLogger: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + success: vi.fn(), + }, +})); + +vi.mock("bcryptjs", () => ({ + default: { + compare: async (password: string, hash: string) => + hash === `hash:${password}`, + }, +})); + +vi.mock("../../../database/repositories/factory.js", () => ({ + createCurrentUserRepository: () => ({ + findById: async (id: string) => state.users.get(id) ?? null, + countAdmins: async () => state.adminCount, + }), +})); + +vi.mock("../../../database/routes/delete-user-data.js", () => ({ + deleteUserAndRelatedData: async (userId: string) => { + state.deleted.push(userId); + }, +})); + +const { registerUserDeleteAccountRoutes } = + await import("../../../database/routes/user-delete-account-routes.js"); + +let handler: RequestHandler | null = null; +registerUserDeleteAccountRoutes( + { + delete: (_path: string, ...handlers: RequestHandler[]) => { + handler = handlers[handlers.length - 1]; + }, + } as unknown as import("express").Router, + (_req, _res, next) => next(), +); + +async function deleteAccount(userId: string, body: Record) { + const req = { userId, body } as unknown as Request; + const res = { + statusCode: 200, + jsonBody: null as Record | null, + status(code: number) { + this.statusCode = code; + return this; + }, + json(payload: Record) { + this.jsonBody = payload; + return this; + }, + }; + await handler!(req, res as unknown as Response, () => {}); + return res; +} + +beforeEach(() => { + state.adminCount = 1; + state.deleted = []; + state.users = new Map([ + [ + "u1", + { + id: "u1", + username: "user", + isAdmin: false, + isExternal: false, + passwordHash: "hash:secret", + }, + ], + [ + "admin1", + { + id: "admin1", + username: "admin", + isAdmin: true, + isExternal: false, + passwordHash: "hash:secret", + }, + ], + [ + "sso1", + { + id: "sso1", + username: "sso", + isAdmin: false, + isExternal: true, + passwordHash: "hash:secret", + }, + ], + ]); +}); + +describe("DELETE /delete-account", () => { + it("runs the full user cleanup", async () => { + const res = await deleteAccount("u1", { password: "secret" }); + expect(res.statusCode).toBe(200); + expect(state.deleted).toEqual(["u1"]); + }); + + it("needs a password", async () => { + const res = await deleteAccount("u1", {}); + expect(res.statusCode).toBe(400); + expect(state.deleted).toEqual([]); + }); + + it("refuses a wrong password", async () => { + const res = await deleteAccount("u1", { password: "nope" }); + expect(res.statusCode).toBe(401); + expect(state.deleted).toEqual([]); + }); + + it("refuses external accounts", async () => { + const res = await deleteAccount("sso1", { password: "secret" }); + expect(res.statusCode).toBe(403); + expect(state.deleted).toEqual([]); + }); + + it("refuses the last admin", async () => { + const res = await deleteAccount("admin1", { password: "secret" }); + expect(res.statusCode).toBe(403); + expect(state.deleted).toEqual([]); + }); + + it("lets an admin go when another admin is left", async () => { + state.adminCount = 2; + const res = await deleteAccount("admin1", { password: "secret" }); + expect(res.statusCode).toBe(200); + expect(state.deleted).toEqual(["admin1"]); + }); + + it("404s for an unknown user", async () => { + const res = await deleteAccount("ghost", { password: "secret" }); + expect(res.statusCode).toBe(404); + }); +});