From b14f3e8cdc5f15e87660931d1effff10e7ec63fa Mon Sep 17 00:00:00 2001 From: Johann150 Date: Wed, 22 Mar 2023 18:29:19 +0100 Subject: [PATCH] server: properly handle logical deletion closes https://akkoma.dev/FoundKeyGang/FoundKey/issues/329 --- .../backend/src/server/api/common/getters.ts | 31 ++++++++++++++----- .../api/endpoints/admin/users/delete.ts | 10 ++---- .../backend/src/server/api/private/signin.ts | 1 + packages/backend/src/server/web/index.ts | 5 +++ .../backend/src/services/delete-account.ts | 14 ++++++--- packages/backend/src/services/user-cache.ts | 24 ++++++++------ 6 files changed, 56 insertions(+), 29 deletions(-) diff --git a/packages/backend/src/server/api/common/getters.ts b/packages/backend/src/server/api/common/getters.ts index cc15a932b..a34d8ac1c 100644 --- a/packages/backend/src/server/api/common/getters.ts +++ b/packages/backend/src/server/api/common/getters.ts @@ -1,3 +1,4 @@ +import { IsNull, Not } from 'typeorm'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { User } from '@/models/entities/user.js'; import { Note } from '@/models/entities/note.js'; @@ -28,8 +29,12 @@ export async function getNote(noteId: Note['id'], me: { id: User['id'] } | null) /** * Get user for API processing */ -export async function getUser(userId: User['id']) { - const user = await Users.findOneBy({ id: userId }); +export async function getUser(userId: User['id'], includeSuspended = false) { + const user = await Users.findOneBy( + id: userId, + isDeleted: false, + isSuspended: !includeSuspended, + }); if (user == null) { throw new ApiError('NO_SUCH_USER'); @@ -41,10 +46,15 @@ export async function getUser(userId: User['id']) { /** * Get remote user for API processing */ -export async function getRemoteUser(userId: User['id']) { - const user = await getUser(userId); +export async function getRemoteUser(userId: User['id'], includeSuspended = false) { + const user = await Users.findOneBy( + id: userId, + host: Not(IsNull()), + isDeleted: false, + isSuspended: !includedSuspended, + }); - if (!Users.isRemoteUser(user)) { + if (user == null) { throw new ApiError('NO_SUCH_USER'); } @@ -54,10 +64,15 @@ export async function getRemoteUser(userId: User['id']) { /** * Get local user for API processing */ -export async function getLocalUser(userId: User['id']) { - const user = await getUser(userId); +export async function getLocalUser(userId: User['id'], includeSuspended = false) { + const user = await Users.findOneBy( + id: userId, + host: IsNull(), + isDeleted: false, + isSuspended: !includeSuspended, + }); - if (!Users.isLocalUser(user)) { + if (user == null) { throw new ApiError('NO_SUCH_USER'); } diff --git a/packages/backend/src/server/api/endpoints/admin/users/delete.ts b/packages/backend/src/server/api/endpoints/admin/users/delete.ts index 6445accb9..9112f50f5 100644 --- a/packages/backend/src/server/api/endpoints/admin/users/delete.ts +++ b/packages/backend/src/server/api/endpoints/admin/users/delete.ts @@ -2,6 +2,7 @@ import { Users } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; import { deleteAccount } from '@/services/delete-account.js'; import define from '@/server/api/define.js'; +import { getUser } from '@/server/api/common/getters.js'; export const meta = { tags: ['admin'], @@ -22,14 +23,9 @@ 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, - isDeleted: false, - }); + const user = await getUser(ps.userId, true); - if (user == null) { - throw new ApiError('NO_SUCH_USER'); - } else if (user.isAdmin) { + if (user.isAdmin) { throw new ApiError('IS_ADMIN'); } else if (user.isModerator) { throw new ApiError('IS_MODERATOR'); diff --git a/packages/backend/src/server/api/private/signin.ts b/packages/backend/src/server/api/private/signin.ts index 2f4a3ca47..b09262bb1 100644 --- a/packages/backend/src/server/api/private/signin.ts +++ b/packages/backend/src/server/api/private/signin.ts @@ -47,6 +47,7 @@ export default async (ctx: Koa.Context) => { const user = await Users.findOneBy({ usernameLower: username.toLowerCase(), host: IsNull(), + isDeleted: false, }) as ILocalUser; if (user == null) { diff --git a/packages/backend/src/server/web/index.ts b/packages/backend/src/server/web/index.ts index fec6663fa..7fe462d43 100644 --- a/packages/backend/src/server/web/index.ts +++ b/packages/backend/src/server/web/index.ts @@ -223,6 +223,7 @@ const getFeed = async (acct: string) => { usernameLower: username.toLowerCase(), host: host ?? IsNull(), isSuspended: false, + isDeleted: false, }); return user && await packFeed(user); @@ -272,6 +273,7 @@ router.get(['/@:user', '/@:user/:sub'], async (ctx, next) => { usernameLower: username.toLowerCase(), host: host ?? IsNull(), isSuspended: false, + isDeleted: false, }); if (user != null) { @@ -304,6 +306,7 @@ router.get('/users/:user', async ctx => { id: ctx.params.user, host: IsNull(), isSuspended: false, + isDeleted: false, }); if (user == null) { @@ -419,6 +422,8 @@ router.get('/@:user/pages/:page', async (ctx, next) => { const user = await Users.findOneBy({ usernameLower: username.toLowerCase(), host: host ?? IsNull(), + isSuspended: false, + isDeleted: false, }); if (user == null) return; diff --git a/packages/backend/src/services/delete-account.ts b/packages/backend/src/services/delete-account.ts index 9c8d4c277..2fa6e004b 100644 --- a/packages/backend/src/services/delete-account.ts +++ b/packages/backend/src/services/delete-account.ts @@ -1,4 +1,4 @@ -import { Users } from '@/models/index.js'; +import { AccessTokens, Users } from '@/models/index.js'; import { createDeleteAccountJob } from '@/queue/index.js'; import { publishUserEvent } from './stream.js'; import { doPostSuspend } from './suspend-user.js'; @@ -7,9 +7,15 @@ export async function deleteAccount(user: { id: string; host: string | null; }): Promise { - await Users.update(user.id, { - isDeleted: true, - }); + await Promise.all([ + Users.update(user.id, { + isDeleted: true, + }), + // revoke all of the users access tokens to block API access + AccessTokens.delete({ + userId: user.id, + }), + ]); if (Users.isLocalUser(user)) { // Terminate streaming diff --git a/packages/backend/src/services/user-cache.ts b/packages/backend/src/services/user-cache.ts index 31a95ffe6..b4bbb6f1a 100644 --- a/packages/backend/src/services/user-cache.ts +++ b/packages/backend/src/services/user-cache.ts @@ -6,15 +6,15 @@ import { subscriber } from '@/db/redis.js'; export const userByIdCache = new Cache( Infinity, - async (id) => await Users.findOneBy({ id }) ?? undefined, + async (id) => await Users.findOneBy({ id, isDeleted: false }) ?? undefined, ); export const localUserByNativeTokenCache = new Cache( Infinity, - async (token) => await Users.findOneBy({ token, host: IsNull() }) as ILocalUser | null ?? undefined, + async (token) => await Users.findOneBy({ token, host: IsNull(), isDeleted: false }) as ILocalUser | null ?? undefined, ); export const uriPersonCache = new Cache( Infinity, - async (uri) => await Users.findOneBy({ uri }) ?? undefined, + async (uri) => await Users.findOneBy({ uri, isDeleted: false }) ?? undefined, ); subscriber.on('message', async (_, data) => { @@ -28,14 +28,18 @@ subscriber.on('message', async (_, data) => { case 'userChangeModeratorState': case 'remoteUserUpdated': { const user = await Users.findOneByOrFail({ id: body.id }); - userByIdCache.set(user.id, user); - for (const [k, v] of uriPersonCache.cache.entries()) { - if (v.value.id === user.id) { - uriPersonCache.set(k, user); + if (user.isDeleted) { + userByIdCache.delete(user.id); + uriPersonCache.delete(user.uri); + if (Users.isLocalUser(user)) { + localUserByNativeTokenCache.delete(user.token); + } + } else { + userByIdCache.set(user.id, user); + uriPersonCache.set(user.uri, user); + if (Users.isLocalUser(user)) { + localUserByNativeTokenCache.set(user.token, user); } - } - if (Users.isLocalUser(user)) { - localUserByNativeTokenCache.set(user.token, user); } break; }