mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-10-28 06:05:24 +00:00
fix: deleting your own account removes all of its data
This commit is contained in:
1 parent
ab3edbdf7b
commit
dda70dcc5d
4 files changed
+258
-80
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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" });
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -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 });
|
||||
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
const req = { userId, body } as unknown as Request;
|
||||
const res = {
|
||||
statusCode: 200,
|
||||
jsonBody: null as Record<string, unknown> | null,
|
||||
status(code: number) {
|
||||
this.statusCode = code;
|
||||
return this;
|
||||
},
|
||||
json(payload: Record<string, unknown>) {
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user