Atomic AP account deletion
Some checks failed
Lint / pnpm_install (push) Successful in 1m57s
Test (backend) / e2e (22.11.0) (push) Failing after 2m2s
Publish Docker image / Build (push) Successful in 5m5s
Test (federation) / test (22.11.0) (push) Failing after 1m36s
Test (production install and build) / production (22.11.0) (push) Successful in 1m14s
Lint / lint (backend) (push) Successful in 2m21s
Test (backend) / unit (22.11.0) (push) Failing after 8m22s
Lint / lint (frontend) (push) Successful in 2m19s
Lint / lint (frontend-shared) (push) Failing after 1m51s
Lint / lint (frontend-embed) (push) Successful in 2m34s
Lint / lint (misskey-bubble-game) (push) Successful in 2m25s
Lint / lint (misskey-js) (push) Successful in 2m16s
Lint / lint (misskey-reversi) (push) Successful in 2m19s
Lint / lint (sw) (push) Successful in 2m19s
Lint / typecheck (misskey-js) (push) Successful in 1m44s
Lint / typecheck (backend) (push) Failing after 2m0s
Lint / typecheck (sw) (push) Successful in 1m36s

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
This commit is contained in:
ゆめ 2024-11-19 19:22:54 -06:00
parent 416d71002a
commit 1d7e141d01
No known key found for this signature in database
14 changed files with 189 additions and 45 deletions

View file

@ -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
sudo 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

View file

@ -24,7 +24,7 @@ import { UtilityService } from '@/core/UtilityService.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
import { QueueService } from '@/core/QueueService.js';
import type { UsersRepository, NotesRepository, FollowingsRepository, AbuseUserReportsRepository, FollowRequestsRepository, MiMeta } from '@/models/_.js';
import type { NotesRepository, FollowingsRepository, AbuseUserReportsRepository, FollowRequestsRepository, MiMeta, ActiveUsersRepository } from '@/models/_.js';
import { bindThis } from '@/decorators.js';
import type { MiRemoteUser } from '@/models/User.js';
import { GlobalEventService } from '@/core/GlobalEventService.js';
@ -58,8 +58,8 @@ export class ApInboxService {
@Inject(DI.meta)
private meta: MiMeta,
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
@Inject(DI.notesRepository)
private notesRepository: NotesRepository,
@ -380,7 +380,7 @@ export class ApInboxService {
return 'skip: ブロックしようとしているユーザーはローカルユーザーではありません';
}
await this.userBlockingService.block(await this.usersRepository.findOneByOrFail({ id: actor.id }), await this.usersRepository.findOneByOrFail({ id: blockee.id }));
await this.userBlockingService.block(await this.activeUsersRepository.findOneByOrFail({ id: actor.id }), await this.activeUsersRepository.findOneByOrFail({ id: blockee.id }));
return 'ok';
}
@ -509,7 +509,7 @@ export class ApInboxService {
return `skip: delete actor ${actor.uri} !== ${uri}`;
}
const user = await this.usersRepository.findOneBy({ id: actor.id });
const user = await this.activeUsersRepository.findOneBy({ id: actor.id });
if (user == null) {
return 'skip: actor not found';
} else if (user.isDeleted) {
@ -518,10 +518,6 @@ export class ApInboxService {
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}`;
@ -561,7 +557,7 @@ export class ApInboxService {
.filter(uri => uri.startsWith(this.config.url + '/users/'))
.map(uri => uri.split('/').at(-1))
.filter(x => x != null);
const users = await this.usersRepository.findBy({
const users = await this.activeUsersRepository.findBy({
id: In(userIds),
});
if (users.length < 1) return 'skip';
@ -715,7 +711,7 @@ export class ApInboxService {
return 'skip: ブロック解除しようとしているユーザーはローカルユーザーではありません';
}
await this.userBlockingService.unblock(await this.usersRepository.findOneByOrFail({ id: actor.id }), blockee);
await this.userBlockingService.unblock(await this.activeUsersRepository.findOneByOrFail({ id: actor.id }), blockee);
return 'ok';
}

View file

@ -8,7 +8,7 @@ import promiseLimit from 'promise-limit';
import { DataSource } from 'typeorm';
import { ModuleRef } from '@nestjs/core';
import { DI } from '@/di-symbols.js';
import type { FollowingsRepository, InstancesRepository, MiMeta, UserProfilesRepository, UserPublickeysRepository, UsersRepository } from '@/models/_.js';
import type { ActiveUsersRepository, FollowingsRepository, InstancesRepository, MiMeta, UserProfilesRepository, UserPublickeysRepository, UsersRepository } from '@/models/_.js';
import type { Config } from '@/config.js';
import type { MiLocalUser, MiRemoteUser } from '@/models/User.js';
import { MiUser } from '@/models/User.js';
@ -91,6 +91,9 @@ export class ApPersonService implements OnModuleInit {
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
@Inject(DI.userProfilesRepository)
private userProfilesRepository: UserProfilesRepository,
@ -211,13 +214,13 @@ export class ApPersonService implements OnModuleInit {
// URIがこのサーバーを指しているならデータベースからフェッチ
if (uri.startsWith(`${this.config.url}/`)) {
const id = uri.split('/').pop();
const u = await this.usersRepository.findOneBy({ id }) as MiLocalUser | null;
const u = await this.activeUsersRepository.findOneBy({ id }) as MiLocalUser | null;
if (u) this.cacheService.uriPersonCache.set(uri, u);
return u;
}
//#region このサーバーに既に登録されていたらそれを返す
const exist = await this.usersRepository.findOneBy({ uri }) as MiLocalUser | MiRemoteUser | null;
const exist = await this.activeUsersRepository.findOneBy({ uri }) as MiLocalUser | MiRemoteUser | null;
if (exist) {
this.cacheService.uriPersonCache.set(uri, exist);
@ -401,7 +404,7 @@ export class ApPersonService implements OnModuleInit {
// duplicate key error
if (isDuplicateKeyValueError(e)) {
// /users/@a => /users/:id のように入力がaliasなときにエラーになることがあるのを対応
const u = await this.usersRepository.findOneBy({ uri: person.id });
const u = await this.activeUsersRepository.findOneBy({ uri: person.id });
if (u == null) throw new Error('already registered');
user = u as MiRemoteUser;
@ -560,7 +563,7 @@ export class ApPersonService implements OnModuleInit {
await this.usersRepository.update(exist.id, updates);
if (person.publicKey) {
await this.userPublickeysRepository.update({ userId: exist.id }, {
await this.usersRepository.update({ userId: exist.id }, {
keyId: person.publicKey.id,
keyPem: person.publicKey.publicKeyPem,
});
@ -662,7 +665,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.activeUsersRepository.findOneByOrFail({ id: userId });
if (!this.userEntityService.isRemoteUser(user)) return;
if (!user.featured) return;
@ -720,7 +723,7 @@ export class ApPersonService implements OnModuleInit {
if (dst && this.userEntityService.isLocalUser(dst)) {
// targetがローカルユーザーだった場合データベースから引っ張ってくる
dst = await this.usersRepository.findOneByOrFail({ uri: src.movedToUri }) as MiLocalUser;
dst = await this.activeUsersRepository.findOneByOrFail({ uri: src.movedToUri }) as MiLocalUser;
} else if (dst) {
if (movePreventUris.includes(src.movedToUri)) return 'skip: circular move';

View file

@ -24,6 +24,7 @@ import {
passwordSchema,
} from '@/models/User.js';
import type {
ActiveUsersRepository,
BlockingsRepository,
FollowingsRepository,
FollowRequestsRepository,
@ -37,7 +38,6 @@ import type {
UserNotePiningsRepository,
UserProfilesRepository,
UserSecurityKeysRepository,
UsersRepository,
} from '@/models/_.js';
import { bindThis } from '@/decorators.js';
import { RoleService } from '@/core/RoleService.js';
@ -101,8 +101,8 @@ export class UserEntityService implements OnModuleInit {
@Inject(DI.redis)
private redisClient: Redis.Redis,
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
@Inject(DI.userSecurityKeysRepository)
private userSecurityKeysRepository: UserSecurityKeysRepository,
@ -410,7 +410,7 @@ export class UserEntityService implements OnModuleInit {
includeSecrets: false,
}, options);
const user = typeof src === 'object' ? src : await this.usersRepository.findOneByOrFail({ id: src });
const user = typeof src === 'object' ? src : await this.activeUsersRepository.findOneByOrFail({ id: src });
const isDetailed = opts.schema !== 'UserLite';
const meId = me ? me.id : null;
@ -662,7 +662,7 @@ export class UserEntityService implements OnModuleInit {
const _users = users.filter((user): user is MiUser => typeof user !== 'string');
if (_users.length !== users.length) {
_users.push(
...await this.usersRepository.findBy({
...await this.activeUsersRepository.findBy({
id: In(users.filter((user): user is string => typeof user === 'string')),
}),
);

View file

@ -16,6 +16,7 @@ export const DI = {
//#region Repositories
usersRepository: Symbol('usersRepository'),
activeUsersRepository: Symbol('activeUsersRepository'),
notesRepository: Symbol('notesRepository'),
announcementsRepository: Symbol('announcementsRepository'),
announcementReadsRepository: Symbol('announcementReadsRepository'),

View file

@ -10,6 +10,7 @@ import {
MiAbuseReportNotificationRecipient,
MiAbuseUserReport,
MiAccessToken,
MiActiveUser,
MiAd,
MiAnnouncement,
MiAnnouncementRead,
@ -87,6 +88,12 @@ const $usersRepository: Provider = {
inject: [DI.db],
};
const $activeUsersRepository: Provider = {
provide: DI.activeUsersRepository,
useFactory: (db: DataSource) => db.getRepository(MiActiveUser).extend(miRepository as MiRepository<MiUser>),
inject: [DI.db],
};
const $notesRepository: Provider = {
provide: DI.notesRepository,
useFactory: (db: DataSource) => db.getRepository(MiNote).extend(miRepository as MiRepository<MiNote>),
@ -499,6 +506,7 @@ const $reversiGamesRepository: Provider = {
imports: [],
providers: [
$usersRepository,
$activeUsersRepository,
$notesRepository,
$announcementsRepository,
$announcementReadsRepository,
@ -570,6 +578,7 @@ const $reversiGamesRepository: Provider = {
],
exports: [
$usersRepository,
$activeUsersRepository,
$notesRepository,
$announcementsRepository,
$announcementReadsRepository,

View file

@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { Entity, Column, Index, OneToOne, JoinColumn, PrimaryColumn } from 'typeorm';
import { Entity, Column, Index, OneToOne, JoinColumn, PrimaryColumn, ViewEntity } from 'typeorm';
import { id } from './util/id.js';
import { MiDriveFile } from './DriveFile.js';
@ -286,6 +286,15 @@ export class MiUser {
}
}
@ViewEntity({
expression: (db) => db.createQueryBuilder().from(MiUser, 'user').where({
isDeleted: false,
}),
materialized: false,
})
export class MiActiveUser extends MiUser {
}
export type MiLocalUser = MiUser & {
host: null;
uri: null;

View file

@ -57,7 +57,7 @@ import { MiRelay } from '@/models/Relay.js';
import { MiSignin } from '@/models/Signin.js';
import { MiSwSubscription } from '@/models/SwSubscription.js';
import { MiUsedUsername } from '@/models/UsedUsername.js';
import { MiUser } from '@/models/User.js';
import { MiActiveUser, MiUser } from '@/models/User.js';
import { MiUserIp } from '@/models/UserIp.js';
import { MiUserKeypair } from '@/models/UserKeypair.js';
import { MiUserList } from '@/models/UserList.js';
@ -172,6 +172,7 @@ export {
MiSignin,
MiSwSubscription,
MiUsedUsername,
MiActiveUser,
MiUser,
MiUserIp,
MiUserKeypair,
@ -244,6 +245,7 @@ export type SigninsRepository = Repository<MiSignin> & MiRepository<MiSignin>;
export type SwSubscriptionsRepository = Repository<MiSwSubscription> & MiRepository<MiSwSubscription>;
export type UsedUsernamesRepository = Repository<MiUsedUsername> & MiRepository<MiUsedUsername>;
export type UsersRepository = Repository<MiUser> & MiRepository<MiUser>;
export type ActiveUsersRepository = Pick<Repository<MiActiveUser> & MiRepository<MiActiveUser>, 'findOneBy' | 'findOneByOrFail' | 'findBy' | 'countBy'>;
export type UserIpsRepository = Repository<MiUserIp> & MiRepository<MiUserIp>;
export type UserKeypairsRepository = Repository<MiUserKeypair> & MiRepository<MiUserKeypair>;
export type UserListsRepository = Repository<MiUserList> & MiRepository<MiUserList>;

View file

@ -44,9 +44,7 @@ export class DeleteAccountProcessorService {
}
@bindThis
public async process(job: Bull.Job<DbUserDeleteJobData>): Promise<string | void> {
this.logger.info(`Deleting account of ${job.data.user.id} ...`);
private async processImpl(job: Bull.Job<DbUserDeleteJobData>): Promise<string | void> {
const user = await this.usersRepository.findOneBy({ id: job.data.user.id });
if (user == null) {
return;
@ -130,4 +128,23 @@ export class DeleteAccountProcessorService {
return 'Account deleted';
}
@bindThis
public async process(job: Bull.Job<DbUserDeleteJobData>): Promise<string | void> {
this.logger.info(`Deleting account of ${job.data.user.id} ...`);
// Atomically "soft lock" the entry without actually locking the row
if (! (await this.usersRepository.update({ id: job.data.user.id, isDeleted: false }, { isDeleted: true })).affected) {
this.logger.debug(`Account of ${job.data.user.id} is already being deleted`);
return 'Account deletion already in progress';
}
try {
return await this.processImpl(job);
} catch (e) {
await this.usersRepository.update({ id: job.data.user.id }, { isDeleted: false });
this.logger.error(`Failed to delete account of ${job.data.user.id}: {e.name}: ${e instanceof Error ? e.message : e}`);
return;
}
}
}

View file

@ -6,7 +6,7 @@
import { Inject, Injectable } from '@nestjs/common';
import { MoreThan } from 'typeorm';
import { DI } from '@/di-symbols.js';
import type { UsersRepository, DriveFilesRepository, MiDriveFile } from '@/models/_.js';
import type { DriveFilesRepository, MiDriveFile, ActiveUsersRepository } from '@/models/_.js';
import type Logger from '@/logger.js';
import { DriveService } from '@/core/DriveService.js';
import { bindThis } from '@/decorators.js';
@ -20,7 +20,7 @@ export class DeleteDriveFilesProcessorService {
constructor(
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
private activeUsersRepository: ActiveUsersRepository,
@Inject(DI.driveFilesRepository)
private driveFilesRepository: DriveFilesRepository,
@ -35,7 +35,7 @@ export class DeleteDriveFilesProcessorService {
public async process(job: Bull.Job<DbJobDataWithUser>): Promise<void> {
this.logger.info(`Deleting drive files of ${job.data.user.id} ...`);
const user = await this.usersRepository.findOneBy({ id: job.data.user.id });
const user = await this.activeUsersRepository.findOneBy({ id: job.data.user.id });
if (user == null) {
return;
}

View file

@ -4,7 +4,7 @@
*/
import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository } from '@/models/_.js';
import type { ActiveUsersRepository } from '@/models/_.js';
import { Endpoint } from '@/server/api/endpoint-base.js';
import { QueryService } from '@/core/QueryService.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -47,14 +47,14 @@ export const paramDef = {
@Injectable()
export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export
constructor(
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
private userEntityService: UserEntityService,
private queryService: QueryService,
) {
super(meta, paramDef, async (ps, me) => {
const query = this.usersRepository.createQueryBuilder('user')
const query = this.activeUsersRepository.createQueryBuilder('user')
.where('user.isExplorable = TRUE')
.andWhere('user.isSuspended = FALSE');

View file

@ -5,7 +5,7 @@
import { Brackets } from 'typeorm';
import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
import type { UserProfilesRepository, ActiveUsersRepository } from '@/models/_.js';
import type { MiUser } from '@/models/User.js';
import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -45,8 +45,8 @@ export const paramDef = {
@Injectable()
export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export
constructor(
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
@Inject(DI.userProfilesRepository)
private userProfilesRepository: UserProfilesRepository,
@ -61,7 +61,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
let users: MiUser[] = [];
const nameQuery = this.usersRepository.createQueryBuilder('user')
const nameQuery = this.activeUsersRepository.createQueryBuilder('user')
.where(new Brackets(qb => {
qb.where('user.name &@~ :query', { query: ps.query });
@ -101,7 +101,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
profQuery.andWhere('prof.userHost IS NOT NULL');
}
const query = this.usersRepository.createQueryBuilder('user')
const query = this.activeUsersRepository.createQueryBuilder('user')
.where(`user.id IN (${ profQuery.getQuery() })`)
.andWhere(new Brackets(qb => {
qb

View file

@ -5,7 +5,7 @@
import { In, IsNull } from 'typeorm';
import { Inject, Injectable } from '@nestjs/common';
import type { UsersRepository } from '@/models/_.js';
import type { ActiveUsersRepository } from '@/models/_.js';
import type { MiUser } from '@/models/User.js';
import { Endpoint } from '@/server/api/endpoint-base.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
@ -82,8 +82,8 @@ export const paramDef = {
@Injectable()
export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-disable-line import/no-default-export
constructor(
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.activeUsersRepository)
private activeUsersRepository: ActiveUsersRepository,
private userEntityService: UserEntityService,
private remoteUserResolveService: RemoteUserResolveService,
@ -102,7 +102,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
return [];
}
const users = await this.usersRepository.findBy(isModerator ? {
const users = await this.activeUsersRepository.findBy(isModerator ? {
id: In(ps.userIds),
} : {
id: In(ps.userIds),
@ -132,7 +132,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
? { id: ps.userId }
: { usernameLower: ps.username!.toLowerCase(), host: IsNull() };
user = await this.usersRepository.findOneBy(q);
user = await this.activeUsersRepository.findOneBy(q);
}
if (user == null || (!isModerator && user.isSuspended)) {

View file

@ -450,6 +450,54 @@ describe('User', () => {
strictEqual(followers.length, 0); // Alice's Follow is not processed
});
});
describe('Deletion atomicity', () => {
const REPS = 5;
const N_USERS = 100;
let users: LoginUser[];
let observer_a: LoginUser, observer_b: LoginUser;
beforeAll(async () => {
observer_a = await createAccount('a.test');
observer_b = await createAccount('b.test');
for (let i = 0; i < N_USERS; i++) {
users.push(await createAccount('a.test'));
users.push(await createAccount('b.test'));
}
});
for (let i = 0; i < REPS; i++) {
test('Follow all users', async () => {
await Promise.all(users.flatMap(async (user, i) => {
await observer_a.client.request('following/create', { userId: user.id });
await observer_b.client.request('following/create', { userId: user.id });
}));
});
test('Delete all users while updating them', async () => {
await Promise.all(users.flatMap(async (user, i) => {
await user.client.request('i/update', { name: `I'm deleting my account ${i}` });
await user.client.request('i/delete-account', { password: user.password });
}));
});
test('Check consistency', async () => {
await Promise.all(users.flatMap(async (user, i) => {
await rejects(
async () => await user.client.request('users/show', { userId: user.id }),
(err: any) => {
strictEqual(err.code, 'NO_SUCH_USER');
return true;
},
);
}));
const following_a = await observer_a.client.request('users/following', { userId: observer_a.id });
strictEqual(following_a.length, 0);
const following_b = await observer_b.client.request('users/following', { userId: observer_b.id });
strictEqual(following_b.length, 0);
});
}
});
});
describe('Suspension', () => {