From c92e2d55a5ee6093d62758bcf8bd344111b2dbf4 Mon Sep 17 00:00:00 2001
From: eternal-flame-AD <yume@yumechi.jp>
Date: Tue, 19 Nov 2024 21:07:02 -0600
Subject: [PATCH] fix(backend): Atomically mark remote account deletions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
---
 .forgejo/workflows/test-federation.yml        | 59 +++++++++++++++++++
 .../1732071810971-IndexUserDeleted.js         | 16 +++++
 .../src/core/activitypub/ApInboxService.ts    |  9 +--
 .../activitypub/models/ApPersonService.ts     |  4 +-
 4 files changed, 78 insertions(+), 10 deletions(-)
 create mode 100644 .forgejo/workflows/test-federation.yml
 create mode 100644 packages/backend/migration/1732071810971-IndexUserDeleted.js

diff --git a/.forgejo/workflows/test-federation.yml b/.forgejo/workflows/test-federation.yml
new file mode 100644
index 0000000000..2876334133
--- /dev/null
+++ b/.forgejo/workflows/test-federation.yml
@@ -0,0 +1,59 @@
+name: Test (federation)
+
+on:
+  push:
+    branches:
+      - master
+      - develop
+    paths:
+      - packages/backend/**
+      - packages/misskey-js/**
+      - .forgejo/workflows/test-federation.yml
+  pull_request:
+    paths:
+      - packages/backend/**
+      - packages/misskey-js/**
+      - .forgejo/workflows/test-federation.yml
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        node-version: [22.11.0]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
+      - name: Install pnpm
+        uses: pnpm/action-setup@v4
+      - name: Install FFmpeg
+        uses: https://github.com/FedericoCarboni/setup-ffmpeg@v3
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4.0.3
+        with:
+          node-version: ${{ matrix.node-version }}
+          cache: 'pnpm'
+      - name: Build Misskey
+        run: |
+          corepack enable && corepack prepare
+          pnpm i --frozen-lockfile
+          pnpm build
+      - name: Setup
+        run: |
+          cd packages/backend/test-federation
+          bash ./setup.sh
+          chmod 644 ./certificates/*.test.key
+      - name: Start servers
+        # https://github.com/docker/compose/issues/1294#issuecomment-374847206
+        run: |
+          cd packages/backend/test-federation
+          docker compose up -d --scale tester=0
+      - name: Test
+        run: |
+          cd packages/backend/test-federation
+          docker compose run --no-deps tester
+      - name: Stop servers
+        run: |
+          cd packages/backend/test-federation
+          docker compose down
diff --git a/packages/backend/migration/1732071810971-IndexUserDeleted.js b/packages/backend/migration/1732071810971-IndexUserDeleted.js
new file mode 100644
index 0000000000..b4c3d714ad
--- /dev/null
+++ b/packages/backend/migration/1732071810971-IndexUserDeleted.js
@@ -0,0 +1,16 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project and yumechi
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+export class IndexUserDeleted1732071810971 {
+    name = 'IndexUserDeleted1732071810971'
+
+    async up(queryRunner) {
+        await queryRunner.query(`CREATE INDEX IF NOT EXISTS "IDX_199b79e682bdc5ba946f491686" ON "user" ("isDeleted")`);
+    }
+
+    async down(queryRunner) {
+        await queryRunner.query(`DROP INDEX IF EXISTS "IDX_199b79e682bdc5ba946f491686"`);
+    }
+}
diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts
index f3aa46292e..fccf86cb91 100644
--- a/packages/backend/src/core/activitypub/ApInboxService.ts
+++ b/packages/backend/src/core/activitypub/ApInboxService.ts
@@ -509,19 +509,12 @@ export class ApInboxService {
 			return `skip: delete actor ${actor.uri} !== ${uri}`;
 		}
 
-		const user = await this.usersRepository.findOneBy({ id: actor.id });
-		if (user == null) {
-			return 'skip: actor not found';
-		} else if (user.isDeleted) {
+		if (!(await this.usersRepository.update({ id: actor.id, isDeleted: false }, { isDeleted: true })).affected) {
 			return 'skip: already deleted';
 		}
 
 		const job = await this.queueService.createDeleteAccountJob(actor);
 
-		await this.usersRepository.update(actor.id, {
-			isDeleted: true,
-		});
-
 		this.globalEventService.publishInternalEvent('remoteUserUpdated', { id: actor.id });
 
 		return `ok: queued ${job.name} ${job.id}`;
diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts
index 8c4e40c561..7de850c511 100644
--- a/packages/backend/src/core/activitypub/models/ApPersonService.ts
+++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts
@@ -557,7 +557,7 @@ export class ApPersonService implements OnModuleInit {
 		if (moving) updates.movedAt = new Date();
 
 		// Update user
-		await this.usersRepository.update(exist.id, updates);
+		await this.usersRepository.update({ id: exist.id, isDeleted: false }, updates);
 
 		if (person.publicKey) {
 			await this.userPublickeysRepository.update({ userId: exist.id }, {
@@ -662,7 +662,7 @@ export class ApPersonService implements OnModuleInit {
 
 	@bindThis
 	public async updateFeatured(userId: MiUser['id'], resolver?: Resolver): Promise<void> {
-		const user = await this.usersRepository.findOneByOrFail({ id: userId });
+		const user = await this.usersRepository.findOneByOrFail({ id: userId, isDeleted: false });
 		if (!this.userEntityService.isRemoteUser(user)) return;
 		if (!user.featured) return;