fix(admin): reject synthetic user ids on mutating routes (#2711)

fluxer_api/src/api/admin/tests/AdminSyntheticUserMutations.test.ts

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {AdminACLs} from '@fluxer/constants/src/AdminACLs';
+import {DELETED_USER_ID} from '@fluxer/constants/src/UserConstants';
+import {afterAll, beforeAll, beforeEach, describe, expect, test} from 'vitest';
+import {createTestAccount, setUserACLs} from '../../auth/tests/AuthTestUtils';
+import {createUserID} from '../../BrandedTypes';
+import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness';
+import {HTTP_STATUS} from '../../test/TestConstants';
+import {createBuilder} from '../../test/TestRequestBuilder';
+import {UserRepository} from '../../user/repositories/UserRepository';
+
+const SYNTHETIC_USER_IDS = ['0', String(DELETED_USER_ID)];
+
+const MUTATIONS: Array<{verb: 'put' | 'patch' | 'delete'; path: string; acl: string; body?: unknown}> = [
+	{verb: 'patch', path: 'username', acl: AdminACLs.USER_UPDATE_USERNAME, body: {username: 'takenover'}},
+	{verb: 'patch', path: 'email', acl: AdminACLs.USER_UPDATE_EMAIL, body: {email: '[email protected]'}},
+	{verb: 'patch', path: 'flags', acl: AdminACLs.USER_UPDATE_FLAGS, body: {add_flags: ['STAFF'], remove_flags: []}},
+	{verb: 'put', path: 'acls', acl: AdminACLs.ACL_SET_USER, body: {acls: [AdminACLs.AUTHENTICATE]}},
+	{verb: 'put', path: 'ban', acl: AdminACLs.USER_TEMP_BAN, body: {duration_hours: 1, reason: 'test'}},
+	{verb: 'put', path: 'deletion', acl: AdminACLs.USER_DELETE, body: {delay_days: 1}},
+	{verb: 'delete', path: 'profile-fields', acl: AdminACLs.USER_UPDATE_PROFILE, body: {fields: ['bio']}},
+	{verb: 'put', path: 'bot-status', acl: AdminACLs.USER_UPDATE_BOT_STATUS, body: {bot: true}},
+	{verb: 'put', path: 'system-status', acl: AdminACLs.USER_UPDATE_BOT_STATUS, body: {system: true}},
+];
+
+const CASES = SYNTHETIC_USER_IDS.flatMap((userId) =>
+	MUTATIONS.map((mutation) => ({
+		userId,
+		...mutation,
+		name: `${mutation.verb.toUpperCase()} ${mutation.path} on ${userId}`,
+	})),
+);
+
+describe('admin mutations against synthetic accounts', () => {
+	let harness: ApiTestHarness;
+	beforeAll(async () => {
+		harness = await createApiTestHarness();
+	});
+	beforeEach(async () => {
+		await harness.reset();
+	});
+	afterAll(async () => {
+		await harness?.shutdown();
+	});
+	test.each(CASES)('$name is rejected and writes no row', async (testCase) => {
+		const admin = await createTestAccount(harness);
+		await setUserACLs(harness, admin, [AdminACLs.AUTHENTICATE, testCase.acl]);
+		const request = createBuilder(harness, `${admin.token}`)[testCase.verb](
+			`/admin/users/${testCase.userId}/${testCase.path}`,
+		);
+		if (testCase.body !== undefined) {
+			request.body(testCase.body);
+		}
+		await request.expect(HTTP_STATUS.NOT_FOUND, 'UNKNOWN_USER').execute();
+		expect(await new UserRepository().listUsers([createUserID(BigInt(testCase.userId))])).toEqual([]);
+	});
+	test.each(SYNTHETIC_USER_IDS)('reading user %s stays a 200 with no results', async (userId) => {
+		const admin = await createTestAccount(harness);
+		await setUserACLs(harness, admin, [AdminACLs.AUTHENTICATE, AdminACLs.USER_LOOKUP]);
+		const result = await createBuilder<{users: Array<{id: string}>}>(harness, `${admin.token}`)
+			.get(`/admin/users/${userId}`)
+			.expect(HTTP_STATUS.OK)
+			.execute();
+		expect(result.users).toEqual([]);
+	});
+	test.each(['%200', '0%0A', '%091', '1%20'])('padded user id %s is rejected the same way', async (encodedUserId) => {
+		const admin = await createTestAccount(harness);
+		await setUserACLs(harness, admin, [AdminACLs.AUTHENTICATE, AdminACLs.USER_UPDATE_USERNAME]);
+		await createBuilder(harness, `${admin.token}`)
+			.patch(`/admin/users/${encodedUserId}/username`)
+			.body({username: 'takenover'})
+			.expect(HTTP_STATUS.NOT_FOUND, 'UNKNOWN_USER')
+			.execute();
+		expect(await new UserRepository().listUsers([createUserID(0n), createUserID(DELETED_USER_ID)])).toEqual([]);
+	});
+	test.each(SYNTHETIC_USER_IDS)('force-adding user %s to a guild is rejected', async (userId) => {
+		const admin = await createTestAccount(harness);
+		await setUserACLs(harness, admin, [AdminACLs.AUTHENTICATE, AdminACLs.GUILD_FORCE_ADD_MEMBER]);
+		await createBuilder(harness, `${admin.token}`)
+			.put(`/admin/guilds/1/members/${userId}`)
+			.expect(HTTP_STATUS.NOT_FOUND, 'UNKNOWN_USER')
+			.execute();
+	});
+});

fluxer_api/src/api/admin/tests/AdminSyntheticUserServiceGuards.test.ts

@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {DELETED_USER_ID} from '@fluxer/constants/src/UserConstants';
+import {UnknownUserError} from '@fluxer/errors/src/domains/user/UnknownUserError';
+import {describe, expect, test} from 'vitest';
+import {createGuildID, createUserID, type UserID} from '../../BrandedTypes';
+import type {IChannelRepository} from '../../channel/IChannelRepository';
+import {UserMessageDeletionService} from '../../channel/services/message/UserMessageDeletionService';
+import type {IGuildRepositoryAggregate} from '../../guild/repositories/IGuildRepositoryAggregate';
+import {GuildMemberOperationsService} from '../../guild/services/member/GuildMemberOperationsService';
+import type {IPurgeQueue} from '../../infrastructure/BunnyPurgeQueue';
+import type {IGatewayService} from '../../infrastructure/IGatewayService';
+import type {IStorageService} from '../../infrastructure/IStorageService';
+
+const SYNTHETIC_USER_IDS: Array<[string, UserID]> = [
+	['0', createUserID(0n)],
+	['1', createUserID(DELETED_USER_ID)],
+];
+
+const GUILD_ID = createGuildID(7000n);
+
+function unusableDependency(): object {
+	return new Proxy(
+		{},
+		{
+			get() {
+				throw new Error('dependency touched before the synthetic user guard ran');
+			},
+		},
+	);
+}
+
+function createMessageDeletionService(): UserMessageDeletionService {
+	const unusable = unusableDependency();
+	return new UserMessageDeletionService({
+		channelRepository: unusable as IChannelRepository,
+		gatewayService: unusable as IGatewayService,
+		storageService: unusable as IStorageService,
+		purgeQueue: unusable as IPurgeQueue,
+	});
+}
+
+function createGuildMemberOperationsService(): GuildMemberOperationsService {
+	const guildRepository = {
+		async findUnique() {
+			return {id: GUILD_ID, features: new Set(), memberCount: 0};
+		},
+		async getMember() {
+			return null;
+		},
+	} as unknown as IGuildRepositoryAggregate;
+	const unusable = unusableDependency();
+	return new GuildMemberOperationsService(
+		guildRepository,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+		unusable as never,
+	);
+}
+
+describe('service level guards for synthetic accounts', () => {
+	test.each(SYNTHETIC_USER_IDS)('deleteUserMessagesBulk refuses user %s', async (_label, userId) => {
+		await expect(createMessageDeletionService().deleteUserMessagesBulk(userId)).rejects.toBeInstanceOf(
+			UnknownUserError,
+		);
+	});
+	test.each(SYNTHETIC_USER_IDS)('addUserToGuild refuses user %s', async (_label, userId) => {
+		await expect(
+			createGuildMemberOperationsService().addUserToGuild(
+				{
+					userId,
+					guildId: GUILD_ID,
+					skipBanCheck: true,
+					skipGuildLimitCheck: true,
+					skipRiskGate: true,
+					requestCache: new Map(),
+				} as never,
+				unusableDependency() as never,
+			),
+		).rejects.toBeInstanceOf(UnknownUserError);
+	});
+});

fluxer_api/src/api/channel/services/message/UserMessageDeletionService.ts

@@ -4,6 +4,7 @@ import {snowflakeToDate} from '@fluxer/snowflake/src/Snowflake';
 import type {ChannelID, GuildID, MessageID, UserID} from '../../../BrandedTypes';
 import {createChannelID} from '../../../BrandedTypes';
 import type {IChannelRepository} from '../../../channel/IChannelRepository';
+import {assertMutableUserId} from '../../../constants/Core';
 import type {IPurgeQueue} from '../../../infrastructure/BunnyPurgeQueue';
 import type {IGatewayService} from '../../../infrastructure/IGatewayService';
 import type {IStorageService} from '../../../infrastructure/IStorageService';
@@ -74,6 +75,7 @@ export class UserMessageDeletionService {
 	}
 
 	async deleteUserMessagesBulk(userId: UserID, options: BulkDeleteUserMessagesOptions = {}): Promise<number> {
+		assertMutableUserId(userId);
 		const {beforeTimestamp = Number.POSITIVE_INFINITY, channelIdAllowlist, onProgress} = options;
 		Logger.debug({userId, beforeTimestamp}, 'Starting bulk user message deletion');
 		const messagesByChannel = await this.collectUserMessages(userId, beforeTimestamp, channelIdAllowlist);

fluxer_api/src/api/constants/Core.ts

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
 import {DELETED_USER_ID} from '@fluxer/constants/src/UserConstants';
+import {UnknownUserError} from '@fluxer/errors/src/domains/user/UnknownUserError';
 import {createUserID, type UserID} from '../BrandedTypes';
 
 export const SYSTEM_USER_ID = createUserID(0n);
@@ -8,3 +9,9 @@ export const SYSTEM_USER_ID = createUserID(0n);
 export function isSyntheticUserId(userId: UserID): boolean {
 	return userId === SYSTEM_USER_ID || userId === DELETED_USER_ID;
 }
+
+export function assertMutableUserId(userId: UserID): void {
+	if (isSyntheticUserId(userId)) {
+		throw new UnknownUserError();
+	}
+}

fluxer_api/src/api/guild/services/member/GuildMemberOperationsService.ts

@@ -36,6 +36,7 @@ import {requireEmailVerified} from '../../../auth/EmailVerificationUtils';
 import type {GuildID, InviteCode, RoleID, UserID} from '../../../BrandedTypes';
 import {createChannelID, createRoleID} from '../../../BrandedTypes';
 import type {ChannelService} from '../../../channel/services/ChannelService';
+import {assertMutableUserId} from '../../../constants/Core';
 import type {GuildMemberRow} from '../../../database/types/GuildTypes';
 import {contentModerationService} from '../../../infrastructure/ContentModerationService';
 import type {EntityAssetService, PreparedAssetUpload} from '../../../infrastructure/EntityAssetService';
@@ -493,6 +494,7 @@ export class GuildMemberOperationsService {
 			if (!guild) throw new UnknownGuildError();
 			const existingMember = await this.guildRepository.getMember(guildId, userId);
 			if (existingMember) return existingMember;
+			assertMutableUserId(userId);
 			const user = await this.userRepository.findUnique(userId);
 			if (!user) throw new UnknownGuildError();
 			if (!skipBanCheck) {

fluxer_api/src/api/middleware/AdminMiddleware.ts

@@ -6,15 +6,18 @@ import {AccessDeniedError} from '@fluxer/errors/src/domains/core/AccessDeniedErr
 import {MissingACLError} from '@fluxer/errors/src/domains/core/MissingACLError';
 import {MissingPermissionsError} from '@fluxer/errors/src/domains/core/MissingPermissionsError';
 import {UnauthorizedError} from '@fluxer/errors/src/domains/core/UnauthorizedError';
+import {SnowflakeType} from '@fluxer/schema/src/primitives/SchemaPrimitives';
 import type {Context} from 'hono';
 import {createMiddleware} from 'hono/factory';
-import {createApplicationID} from '../BrandedTypes';
+import {createApplicationID, createUserID} from '../BrandedTypes';
+import {assertMutableUserId} from '../constants/Core';
 import {Logger} from '../Logger';
 import type {User} from '../models/User';
 import type {HonoEnv} from '../types/HonoEnv';
 
 const ADMIN_OAUTH2_APPLICATION_ID_BRANDED = createApplicationID(ADMIN_OAUTH2_APPLICATION_ID);
 type AdminAuthTokenType = 'bearer' | 'session' | 'admin_api_key';
+const ADMIN_TARGET_USER_PARAMS = ['user_id', 'target_user_id'] as const;
 
 function ensureBearerIsBuiltInAdminApplication(ctx: Context<HonoEnv>): void {
 	if (ctx.get('oauthBearerApplicationId') !== ADMIN_OAUTH2_APPLICATION_ID_BRANDED) {
@@ -57,6 +60,23 @@ function getRequestAdminACLs(ctx: Context<HonoEnv>, adminUser: User, tokenType:
 	return tokenType === 'admin_api_key' ? (ctx.get('adminApiKeyAcls') ?? new Set()) : adminUser.acls;
 }
 
+function ensureAdminTargetIsMutable(ctx: Context<HonoEnv>): void {
+	if (ctx.req.method === 'GET') {
+		return;
+	}
+	for (const paramName of ADMIN_TARGET_USER_PARAMS) {
+		const rawUserId = ctx.req.param(paramName);
+		if (rawUserId === undefined) {
+			continue;
+		}
+		const parsedUserId = SnowflakeType.safeParse(rawUserId);
+		if (!parsedUserId.success) {
+			continue;
+		}
+		assertMutableUserId(createUserID(parsedUserId.data));
+	}
+}
+
 function requireAdminAccess(requiredACLs: ReadonlyArray<string>) {
 	return createMiddleware<HonoEnv>(async (ctx, next) => {
 		const adminUser = ctx.get('user');
@@ -80,6 +100,7 @@ function requireAdminAccess(requiredACLs: ReadonlyArray<string>) {
 		if (!hasAnyAdminACL(requestAcls, requiredACLs)) {
 			throw new MissingACLError(requiredACLs[0] ?? AdminACLs.AUTHENTICATE);
 		}
+		ensureAdminTargetIsMutable(ctx);
 		ctx.set('adminUserId', adminUser.id);
 		ctx.set('adminUserAcls', requestAcls);
 		await next();

fluxer_api/src/api/user/repositories/account/crud/UserDataRepository.ts

@@ -4,6 +4,7 @@ import {DELETED_USER_ID, UserFlags} from '@fluxer/constants/src/UserConstants';
 import {UnknownUserError} from '@fluxer/errors/src/domains/user/UnknownUserError';
 import {BACKGROUND_READ_TIMEOUT_MS} from '@pkgs/cassandra/src/Client';
 import {createUserID, type UserID} from '../../../../BrandedTypes';
+import {isSyntheticUserId} from '../../../../constants/Core';
 import {fetchMany, fetchOne, fetchPage, upsertOne} from '../../../../database/CassandraQueryExecution';
 import {Db, type DbOp, nextVersion} from '../../../../database/CassandraTypes';
 import {
@@ -45,6 +46,12 @@ type UserPatch = Partial<{
 	[K in Exclude<keyof UserRow, 'user_id'> & string]: DbOp<UserRow[K]>;
 }>;
 
+function assertWritableUserId(userId: UserID): void {
+	if (isSyntheticUserId(userId)) {
+		throw new Error(`Refusing to write a users row for synthetic user ${userId}`);
+	}
+}
+
 export class UserDataRepository {
 	async findUnique(userId: UserID): Promise<User | null> {
 		if (userId === FLUXER_BOT_USER_ID) {
@@ -125,6 +132,7 @@ export class UserDataRepository {
 		updatedData: UserRow;
 	}> {
 		const userId = data.user_id;
+		assertWritableUserId(userId);
 		const result = await executeVersionedUpdate<UserRow, 'user_id'>(
 			async () => {
 				return fetchOne<UserRow>(FETCH_USER_BY_ID_CQL, {user_id: userId});
@@ -152,6 +160,7 @@ export class UserDataRepository {
 		previousData: UserRow | null;
 		updatedData: UserRow;
 	}> {
+		assertWritableUserId(userId);
 		const result = await executeVersionedUpdate<UserRow, 'user_id'>(
 			async () => {
 				return fetchOne<UserRow>(FETCH_USER_BY_ID_CQL, {user_id: userId});
@@ -183,6 +192,7 @@ export class UserDataRepository {
 		};
 	}> {
 		const {userId, lastActiveAt, lastActiveIp} = params;
+		assertWritableUserId(userId);
 		const previousData = (await this.getActivityTracking(userId)) ?? {last_active_at: null, last_active_ip: null};
 		await upsertOne(
 			Users.patchByPk(
@@ -219,6 +229,7 @@ export class UserDataRepository {
 	): Promise<{
 		finalVersion: number | null;
 	}> {
+		assertWritableUserId(userId);
 		const result = await executeVersionedUpdate<UserRow, 'user_id'>(
 			async () => {
 				return fetchOne<UserRow>(FETCH_USER_BY_ID_CQL, {user_id: userId});

fluxer_api/src/api/user/tests/UserSyntheticWriteGuard.test.ts

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {DELETED_USER_ID} from '@fluxer/constants/src/UserConstants';
+import {afterAll, beforeAll, beforeEach, describe, expect, test} from 'vitest';
+import {createUserID, type UserID} from '../../BrandedTypes';
+import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness';
+import {UserRepository} from '../repositories/UserRepository';
+
+const SYNTHETIC_USER_IDS: Array<[string, UserID]> = [
+	['0', createUserID(0n)],
+	['1', createUserID(DELETED_USER_ID)],
+];
+
+describe('users table write guard for synthetic accounts', () => {
+	let harness: ApiTestHarness;
+	beforeAll(async () => {
+		harness = await createApiTestHarness();
+	});
+	afterAll(async () => {
+		await harness.shutdown();
+	});
+	beforeEach(async () => {
+		await harness.resetData();
+	});
+	test.each(SYNTHETIC_USER_IDS)('patchUpsert refuses to create a row for user %s', async (_label, userId) => {
+		const repository = new UserRepository();
+		await expect(repository.patchUpsert(userId, {bio: 'written by a test'})).rejects.toThrow();
+		expect(await repository.listUsers([userId])).toEqual([]);
+	});
+	test.each(SYNTHETIC_USER_IDS)('updateLastActiveAt refuses to create a row for user %s', async (_label, userId) => {
+		const repository = new UserRepository();
+		await expect(repository.updateLastActiveAt({userId, lastActiveAt: new Date(0)})).rejects.toThrow();
+		expect(await repository.listUsers([userId])).toEqual([]);
+	});
+	test.each(
+		SYNTHETIC_USER_IDS,
+	)('findUnique still synthesises user %s after a refused write', async (_label, userId) => {
+		const repository = new UserRepository();
+		await expect(repository.patchUpsert(userId, {bio: 'written by a test'})).rejects.toThrow();
+		const user = await repository.findUnique(userId);
+		expect(user).not.toBeNull();
+		expect(user?.id.toString()).toBe(userId.toString());
+	});
+});