From 1eda1760d12ca6671c3f14ac6662b6433a5f8986 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 7 Jan 2023 19:28:18 +0100 Subject: [PATCH] server: refactor to always use deleteAccount service This should reduce code duplication around how deletion of an actor is handled. --- packages/backend/src/models/entities/user.ts | 9 +++++- .../src/queue/processors/db/delete-account.ts | 6 ++-- .../remote/activitypub/kernel/delete/actor.ts | 15 +++------ .../api/endpoints/admin/accounts/delete.ts | 31 ++++--------------- .../backend/src/services/delete-account.ts | 22 +++++++------ 5 files changed, 34 insertions(+), 49 deletions(-) diff --git a/packages/backend/src/models/entities/user.ts b/packages/backend/src/models/entities/user.ts index 449debcfc..0aee4c90b 100644 --- a/packages/backend/src/models/entities/user.ts +++ b/packages/backend/src/models/entities/user.ts @@ -155,7 +155,14 @@ export class User { }) public isExplorable: boolean; - // アカウントが削除されたかどうかのフラグだが、完全に削除される際は物理削除なので実質削除されるまでの「削除が進行しているかどうか」のフラグ + // for local users: + // Indicates a deletion in progress. + // A hard delete of the record will follow after the deletion finishes. + // + // for remote users: + // Indicates the user was deleted by an admin. + // The users' data is not deleted from the database to keep them from reappearing. + // A hard delete of the record may follow if we receive a matching Delete activity. @Column('boolean', { default: false, comment: 'Whether the User is deleted.', diff --git a/packages/backend/src/queue/processors/db/delete-account.ts b/packages/backend/src/queue/processors/db/delete-account.ts index 96f60cfe5..84e28f25d 100644 --- a/packages/backend/src/queue/processors/db/delete-account.ts +++ b/packages/backend/src/queue/processors/db/delete-account.ts @@ -83,10 +83,8 @@ export async function deleteAccount(job: Bull.Job): Promise } } - // soft指定されている場合は物理削除しない - if (job.data.soft) { - // nop - } else { + // No physical deletion if soft is specified. + if (!job.data.soft) { await Users.delete(job.data.user.id); } diff --git a/packages/backend/src/remote/activitypub/kernel/delete/actor.ts b/packages/backend/src/remote/activitypub/kernel/delete/actor.ts index 9467eb535..ea75a9739 100644 --- a/packages/backend/src/remote/activitypub/kernel/delete/actor.ts +++ b/packages/backend/src/remote/activitypub/kernel/delete/actor.ts @@ -1,7 +1,7 @@ -import { createDeleteAccountJob } from '@/queue/index.js'; import { CacheableRemoteUser } from '@/models/entities/user.js'; import { Users } from '@/models/index.js'; import { apLogger } from '@/remote/activitypub/logger.js'; +import { deleteAccount } from '@/services/delete-account.js'; export async function deleteActor(actor: CacheableRemoteUser, uri: string): Promise { apLogger.info(`Deleting the Actor: ${uri}`); @@ -17,14 +17,9 @@ export async function deleteActor(actor: CacheableRemoteUser, uri: string): Prom return 'ok: gone'; } if (user.isDeleted) { - apLogger.info('skip: already deleted'); + // the actual deletion already happened by an admin, just delete the record + await Users.delete(actor.id); + } else { + await deleteAccount(actor); } - - const job = await createDeleteAccountJob(actor); - - await Users.update(actor.id, { - isDeleted: true, - }); - - return `ok: queued ${job.name} ${job.id}`; } diff --git a/packages/backend/src/server/api/endpoints/admin/accounts/delete.ts b/packages/backend/src/server/api/endpoints/admin/accounts/delete.ts index b8a8b0618..19e3cd604 100644 --- a/packages/backend/src/server/api/endpoints/admin/accounts/delete.ts +++ b/packages/backend/src/server/api/endpoints/admin/accounts/delete.ts @@ -1,8 +1,6 @@ import { Users } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; -import { doPostSuspend } from '@/services/suspend-user.js'; -import { publishUserEvent } from '@/services/stream.js'; -import { createDeleteAccountJob } from '@/queue/index.js'; +import { deleteAccount } from '@/services/delete-account.js'; import define from '../../../define.js'; export const meta = { @@ -24,7 +22,10 @@ export const paramDef = { // eslint-disable-next-line import/no-default-export export default define(meta, paramDef, async (ps) => { - const user = await Users.findOneBy({ id: ps.userId }); + const user = await Users.findOneBy({ + id: ps.userId, + isDeleted: false, + }); if (user == null) { throw new ApiError('NO_SUCH_USER'); @@ -34,25 +35,5 @@ export default define(meta, paramDef, async (ps) => { throw new ApiError('IS_MODERATOR'); } - if (Users.isLocalUser(user)) { - // 物理削除する前にDelete activityを送信する - await doPostSuspend(user).catch(() => {}); - - createDeleteAccountJob(user, { - soft: false, - }); - } else { - createDeleteAccountJob(user, { - soft: true, // リモートユーザーの削除は、完全にDBから物理削除してしまうと再度連合してきてアカウントが復活する可能性があるため、soft指定する - }); - } - - await Users.update(user.id, { - isDeleted: true, - }); - - if (Users.isLocalUser(user)) { - // Terminate streaming - publishUserEvent(user.id, 'terminate', {}); - } + await deleteAccount(user); }); diff --git a/packages/backend/src/services/delete-account.ts b/packages/backend/src/services/delete-account.ts index ed3e381c4..9c8d4c277 100644 --- a/packages/backend/src/services/delete-account.ts +++ b/packages/backend/src/services/delete-account.ts @@ -7,17 +7,21 @@ export async function deleteAccount(user: { id: string; host: string | null; }): Promise { - // Send Delete activity before physical deletion - await doPostSuspend(user).catch(() => {}); - - createDeleteAccountJob(user, { - soft: false, - }); - await Users.update(user.id, { isDeleted: true, }); - // Terminate streaming - publishUserEvent(user.id, 'terminate', {}); + if (Users.isLocalUser(user)) { + // Terminate streaming + publishUserEvent(user.id, 'terminate', {}); + } + + // Send Delete activity before physical deletion + await doPostSuspend(user).catch(() => {}); + + createDeleteAccountJob(user, { + // Deleting remote users is specified as SOFT, because if they are physically deleted + // from the DB completely, they may be reassociated and their accounts may be reinstated. + soft: Users.isLocalUser(user), + }); }