From 7113b5f60495578cbb7cd4d9ac3370669c33a3c7 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 17 Aug 2022 22:40:00 +0200 Subject: [PATCH] fix: Restore user deletion through API, increase rate limit --- server/routes/api/users.test.ts | 16 ++++++++++++- server/routes/api/users.ts | 40 ++++++++++++++++++++++----------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/server/routes/api/users.test.ts b/server/routes/api/users.test.ts index e6bb4d249..1ce4d9c2c 100644 --- a/server/routes/api/users.test.ts +++ b/server/routes/api/users.test.ts @@ -328,7 +328,7 @@ describe("#users.delete", () => { expect(res.status).toEqual(400); }); - it("should require correct code", async () => { + it("should require correct code when no id passed", async () => { const user = await buildAdmin(); await buildUser({ teamId: user.teamId, @@ -357,6 +357,20 @@ describe("#users.delete", () => { expect(res.status).toEqual(200); }); + it("should allow deleting user account as admin", async () => { + const admin = await buildAdmin(); + const user = await buildUser({ + teamId: admin.teamId, + }); + const res = await server.post("/api/users.delete", { + body: { + id: user.id, + token: admin.getJwtToken(), + }, + }); + expect(res.status).toEqual(200); + }); + it("should require authentication", async () => { const res = await server.post("/api/users.delete"); const body = await res.json(); diff --git a/server/routes/api/users.ts b/server/routes/api/users.ts index c33d4510d..4dcae8d25 100644 --- a/server/routes/api/users.ts +++ b/server/routes/api/users.ts @@ -24,6 +24,7 @@ import { assertSort, assertPresent, assertArray, + assertUuid, } from "@server/validation"; import pagination from "./middlewares/pagination"; @@ -402,23 +403,36 @@ router.post( router.post( "users.delete", auth(), - rateLimiter(RateLimiterStrategy.FivePerHour), + rateLimiter(RateLimiterStrategy.TenPerHour), async (ctx) => { - const { code = "" } = ctx.body; - const { user } = ctx.state; + const { id, code = "" } = ctx.body; + let user: User; + + if (id) { + assertUuid(id, "id must be a UUID"); + user = await User.findByPk(id, { + rejectOnEmpty: true, + }); + } else { + user = ctx.state.user; + } authorize(user, "delete", user); - const deleteConfirmationCode = user.deleteConfirmationCode; + // If we're attempting to delete our own account then a confirmation code + // is required. This acts as CSRF protection. + if (!id || id === ctx.state.user.id) { + const deleteConfirmationCode = user.deleteConfirmationCode; - if ( - emailEnabled && - (code.length !== deleteConfirmationCode.length || - !crypto.timingSafeEqual( - Buffer.from(code), - Buffer.from(deleteConfirmationCode) - )) - ) { - throw ValidationError("The confirmation code was incorrect"); + if ( + emailEnabled && + (code.length !== deleteConfirmationCode.length || + !crypto.timingSafeEqual( + Buffer.from(code), + Buffer.from(deleteConfirmationCode) + )) + ) { + throw ValidationError("The confirmation code was incorrect"); + } } await userDestroyer({