merge: Prevent deletion or suspension of system accounts (resolves #625) (!666)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/666

Closes #625

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

packages/backend/src/core/DeleteAccountService.ts

@@ -13,6 +13,7 @@ import { GlobalEventService } from '@/core/GlobalEventService.js';
 import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { ApRendererService } from '@/core/activitypub/ApRendererService.js';
 import { ModerationLogService } from '@/core/ModerationLogService.js';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 @Injectable()
 export class DeleteAccountService {
@@ -38,6 +39,7 @@ export class DeleteAccountService {
 	}, moderator?: MiUser): Promise<void> {
 		const _user = await this.usersRepository.findOneByOrFail({ id: user.id });
 		if (_user.isRoot) throw new Error('cannot delete a root account');
+		if (isSystemAccount(_user)) throw new Error('cannot delete a system account');
 
 		if (moderator != null) {
 			this.moderationLogService.log(moderator, 'deleteAccount', {

packages/backend/src/core/UserSuspendService.ts

@@ -15,6 +15,7 @@ import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { bindThis } from '@/decorators.js';
 import { RelationshipJobData } from '@/queue/types.js';
 import { ModerationLogService } from '@/core/ModerationLogService.js';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 @Injectable()
 export class UserSuspendService {
@@ -38,6 +39,8 @@ export class UserSuspendService {
 
 	@bindThis
 	public async suspend(user: MiUser, moderator: MiUser): Promise<void> {
+		if (isSystemAccount(user)) throw new Error('cannot suspend a system account');
+
 		await this.usersRepository.update(user.id, {
 			isSuspended: true,
 		});

packages/backend/src/core/entities/UserEntityService.ts

@@ -53,6 +53,7 @@ import type { OnModuleInit } from '@nestjs/common';
 import type { NoteEntityService } from './NoteEntityService.js';
 import type { DriveFileEntityService } from './DriveFileEntityService.js';
 import type { PageEntityService } from './PageEntityService.js';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 const Ajv = _Ajv.default;
 const ajv = new Ajv();
@@ -614,6 +615,7 @@ export class UserEntityService implements OnModuleInit {
 				backgroundId: user.backgroundId,
 				isModerator: isModerator,
 				isAdmin: isAdmin,
+				isSystem: isSystemAccount(user),
 				injectFeaturedNote: profile!.injectFeaturedNote,
 				receiveAnnouncementEmail: profile!.receiveAnnouncementEmail,
 				alwaysMarkNsfw: profile!.alwaysMarkNsfw,

packages/backend/src/misc/is-system-account.ts

@@ -0,0 +1,16 @@
+/*
+ * SPDX-FileCopyrightText: hazelnoot and other Sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+interface UserLike {
+	readonly username: string;
+	readonly host: string | null;
+}
+
+/**
+ * Checks if the given user represents a system account, such as instance.actor.
+ */
+export function isSystemAccount(user: UserLike): boolean {
+	return user.host == null && user.username.includes('.');
+}

packages/backend/src/models/json-schema/user.ts

@@ -121,6 +121,11 @@ export const packedUserLiteSchema = {
 			nullable: false, optional: true,
 			default: false,
 		},
+		isSystem: {
+			type: 'boolean',
+			nullable: false, optional: true,
+			default: false,
+		},
 		isSilenced: {
 			type: 'boolean',
 			nullable: false, optional: false,

packages/backend/src/server/api/SigninApiService.ts

@@ -26,6 +26,7 @@ import { RateLimiterService } from './RateLimiterService.js';
 import { SigninService } from './SigninService.js';
 import type { AuthenticationResponseJSON } from '@simplewebauthn/types';
 import type { FastifyReply, FastifyRequest } from 'fastify';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 @Injectable()
 export class SigninApiService {
@@ -125,6 +126,12 @@ export class SigninApiService {
 			});
 		}
 
+		if (isSystemAccount(user)) {
+			return error(403, {
+				id: 's8dhsj9s-a93j-493j-ja9k-kas9sj20aml2',
+			});
+		}
+
 		const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id });
 
 		if (!user.approved && instance.approvalRequiredForSignup) {

packages/backend/src/server/api/endpoints/admin/reset-password.ts

@@ -11,6 +11,7 @@ import type { UsersRepository, UserProfilesRepository } from '@/models/_.js';
 import { DI } from '@/di-symbols.js';
 import { secureRndstr } from '@/misc/secure-rndstr.js';
 import { ModerationLogService } from '@/core/ModerationLogService.js';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 export const meta = {
 	tags: ['admin'],
@@ -63,6 +64,10 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 				throw new Error('cannot reset password of root');
 			}
 
+			if (isSystemAccount(user)) {
+				throw new Error('cannot reset password of system account');
+			}
+
 			const passwd = secureRndstr(8);
 
 			// Generate hash of password

packages/backend/src/server/api/endpoints/admin/show-user.ts

@@ -11,6 +11,7 @@ import { RoleService } from '@/core/RoleService.js';
 import { RoleEntityService } from '@/core/entities/RoleEntityService.js';
 import { IdService } from '@/core/IdService.js';
 import { notificationRecieveConfig } from '@/models/json-schema/user.js';
+import { isSystemAccount } from '@/misc/is-system-account.js';
 
 export const meta = {
 	tags: ['admin'],
@@ -31,6 +32,10 @@ export const meta = {
 				type: 'boolean',
 				optional: false, nullable: false,
 			},
+			approved: {
+				type: 'boolean',
+				optional: false, nullable: false,
+			},
 			autoAcceptFollowed: {
 				type: 'boolean',
 				optional: false, nullable: false,
@@ -111,6 +116,10 @@ export const meta = {
 				type: 'boolean',
 				optional: false, nullable: false,
 			},
+			isSystem: {
+				type: 'boolean',
+				optional: false, nullable: false,
+			},
 			isSilenced: {
 				type: 'boolean',
 				optional: false, nullable: false,
@@ -240,6 +249,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 				mutedInstances: profile.mutedInstances,
 				notificationRecieveConfig: profile.notificationRecieveConfig,
 				isModerator: isModerator,
+				isSystem: isSystemAccount(user),
 				isSilenced: isSilenced,
 				isSuspended: user.isSuspended,
 				isHibernated: user.isHibernated,

packages/backend/test/unit/misc/is-system-account.ts

@@ -0,0 +1,47 @@
+/*
+ * SPDX-FileCopyrightText: hazelnoot and other Sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { isSystemAccount } from '@/misc/is-system-account.js';
+
+describe(isSystemAccount, () => {
+	it('should return true for instance.actor', () => {
+		expect(isSystemAccount({ username: 'instance.actor', host: null })).toBeTruthy();
+	});
+
+	it('should return true for relay.actor', () => {
+		expect(isSystemAccount({ username: 'relay.actor', host: null })).toBeTruthy();
+	});
+
+	it('should return true for any username with a dot', () => {
+		expect(isSystemAccount({ username: 'some.user', host: null })).toBeTruthy();
+		expect(isSystemAccount({ username: 'some.', host: null })).toBeTruthy();
+		expect(isSystemAccount({ username: '.user', host: null })).toBeTruthy();
+		expect(isSystemAccount({ username: '.', host: null })).toBeTruthy();
+	});
+
+	it('should return true for usernames with multiple dots', () => {
+		expect(isSystemAccount({ username: 'some.user.account', host: null })).toBeTruthy();
+		expect(isSystemAccount({ username: '..', host: null })).toBeTruthy();
+	});
+
+	it('should return false for usernames without a dot', () => {
+		expect(isSystemAccount({ username: 'instance_actor', host: null })).toBeFalsy();
+		expect(isSystemAccount({ username: 'instanceactor', host: null })).toBeFalsy();
+		expect(isSystemAccount({ username: 'relay_actor', host: null })).toBeFalsy();
+		expect(isSystemAccount({ username: 'relayactor', host: null })).toBeFalsy();
+		expect(isSystemAccount({ username: '', host: null })).toBeFalsy();
+	});
+
+	it('should return false for users from another instance', () => {
+		expect(isSystemAccount({ username: 'instance.actor', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: 'relay.actor', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: 'some.user', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: 'some.', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: '.user', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: '.', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: 'some.user.account', host: 'example.com' })).toBeFalsy();
+		expect(isSystemAccount({ username: '..', host: 'example.com' })).toBeFalsy();
+	});
+});