From 0d508db8a7a36218d38231af4e718aff0e94d9bc Mon Sep 17 00:00:00 2001 From: Daiki Mizukami Date: Fri, 9 Aug 2024 12:10:51 +0900 Subject: [PATCH] fix(backend): check visibility of following/followers of remote users / feat: moderators can see following/followers of all users (#14375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): check visibility of following/followers of remote users Resolves https://github.com/misskey-dev/misskey/issues/13362. * test(backend): add tests for visibility of following/followers of remote users * docs(changelog): update CHANGELOG.md * feat: moderators can see following/followers of all users * docs(changelog): update CHANGELOG.md * refactor(backend): minor refactoring `createPerson`と`if`の条件を統一するとともに、異常系の 処理をearly returnに追い出すための変更。 * feat(backend): moderators can see following/followers count of all users As per https://github.com/misskey-dev/misskey/pull/14375#issuecomment-2275044908. --- CHANGELOG.md | 3 +- .../activitypub/models/ApPersonService.ts | 50 ++++++++++++++++- packages/backend/src/core/activitypub/type.ts | 6 ++- .../src/core/entities/UserEntityService.ts | 4 +- .../server/api/endpoints/users/followers.ts | 34 ++++++------ .../server/api/endpoints/users/following.ts | 34 ++++++------ packages/backend/test/unit/activitypub.ts | 53 ++++++++++++++++++- .../frontend/src/scripts/isFfVisibleForMe.ts | 4 +- 8 files changed, 149 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86e33a8272..c18cccc44e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ ## Unreleased ### General -- +- Fix: リモートユーザのフォロー・フォロワーの一覧が非公開設定の場合も表示できてしまう問題を修正 +- Enhance: モデレーターはすべてのユーザーのフォロー・フォロワーの一覧を見られるように ### Client - diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 457205e023..f3ddf3952c 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -48,7 +48,7 @@ import type { ApResolverService, Resolver } from '../ApResolverService.js'; import type { ApLoggerService } from '../ApLoggerService.js'; // eslint-disable-next-line @typescript-eslint/consistent-type-imports import type { ApImageService } from './ApImageService.js'; -import type { IActor, IObject } from '../type.js'; +import type { IActor, ICollection, IObject, IOrderedCollection } from '../type.js'; const nameLength = 128; const summaryLength = 2048; @@ -296,6 +296,21 @@ export class ApPersonService implements OnModuleInit { const isBot = getApType(object) === 'Service' || getApType(object) === 'Application'; + const [followingVisibility, followersVisibility] = await Promise.all( + [ + this.isPublicCollection(person.following, resolver), + this.isPublicCollection(person.followers, resolver), + ].map((p): Promise<'public' | 'private'> => p + .then(isPublic => isPublic ? 'public' : 'private') + .catch(err => { + if (!(err instanceof StatusError) || err.isRetryable) { + this.logger.error('error occurred while fetching following/followers collection', { stack: err }); + } + return 'private'; + }) + ) + ); + const bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/); const url = getOneApHrefNullable(person.url); @@ -357,6 +372,8 @@ export class ApPersonService implements OnModuleInit { description: _description, url, fields, + followingVisibility, + followersVisibility, birthday: bday?.[0] ?? null, location: person['vcard:Address'] ?? null, userHost: host, @@ -464,6 +481,23 @@ export class ApPersonService implements OnModuleInit { const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); + const [followingVisibility, followersVisibility] = await Promise.all( + [ + this.isPublicCollection(person.following, resolver), + this.isPublicCollection(person.followers, resolver), + ].map((p): Promise<'public' | 'private' | undefined> => p + .then(isPublic => isPublic ? 'public' : 'private') + .catch(err => { + if (!(err instanceof StatusError) || err.isRetryable) { + this.logger.error('error occurred while fetching following/followers collection', { stack: err }); + // Do not update the visibiility on transient errors. + return undefined; + } + return 'private'; + }) + ) + ); + const bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/); const url = getOneApHrefNullable(person.url); @@ -532,6 +566,8 @@ export class ApPersonService implements OnModuleInit { url, fields, description: _description, + followingVisibility, + followersVisibility, birthday: bday?.[0] ?? null, location: person['vcard:Address'] ?? null, }); @@ -703,4 +739,16 @@ export class ApPersonService implements OnModuleInit { return 'ok'; } + + @bindThis + private async isPublicCollection(collection: string | ICollection | IOrderedCollection | undefined, resolver: Resolver): Promise { + if (collection) { + const resolved = await resolver.resolveCollection(collection); + if (resolved.first || (resolved as ICollection).items || (resolved as IOrderedCollection).orderedItems) { + return true; + } + } + + return false; + } } diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts index 5b6c6c8ca6..131c518c0a 100644 --- a/packages/backend/src/core/activitypub/type.ts +++ b/packages/backend/src/core/activitypub/type.ts @@ -97,13 +97,15 @@ export interface IActivity extends IObject { export interface ICollection extends IObject { type: 'Collection'; totalItems: number; - items: ApObject; + first?: IObject | string; + items?: ApObject; } export interface IOrderedCollection extends IObject { type: 'OrderedCollection'; totalItems: number; - orderedItems: ApObject; + first?: IObject | string; + orderedItems?: ApObject; } export const validPost = ['Note', 'Question', 'Article', 'Audio', 'Document', 'Image', 'Page', 'Video', 'Event']; diff --git a/packages/backend/src/core/entities/UserEntityService.ts b/packages/backend/src/core/entities/UserEntityService.ts index 7fd093c191..9bf568bc90 100644 --- a/packages/backend/src/core/entities/UserEntityService.ts +++ b/packages/backend/src/core/entities/UserEntityService.ts @@ -454,12 +454,12 @@ export class UserEntityService implements OnModuleInit { } const followingCount = profile == null ? null : - (profile.followingVisibility === 'public') || isMe ? user.followingCount : + (profile.followingVisibility === 'public') || isMe || iAmModerator ? user.followingCount : (profile.followingVisibility === 'followers') && (relation && relation.isFollowing) ? user.followingCount : null; const followersCount = profile == null ? null : - (profile.followersVisibility === 'public') || isMe ? user.followersCount : + (profile.followersVisibility === 'public') || isMe || iAmModerator ? user.followersCount : (profile.followersVisibility === 'followers') && (relation && relation.isFollowing) ? user.followersCount : null; diff --git a/packages/backend/src/server/api/endpoints/users/followers.ts b/packages/backend/src/server/api/endpoints/users/followers.ts index 7ce7734f53..a8b4319a61 100644 --- a/packages/backend/src/server/api/endpoints/users/followers.ts +++ b/packages/backend/src/server/api/endpoints/users/followers.ts @@ -11,6 +11,7 @@ import { QueryService } from '@/core/QueryService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { DI } from '@/di-symbols.js'; +import { RoleService } from '@/core/RoleService.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -81,6 +82,7 @@ export default class extends Endpoint { // eslint- private utilityService: UtilityService, private followingEntityService: FollowingEntityService, private queryService: QueryService, + private roleService: RoleService, ) { super(meta, paramDef, async (ps, me) => { const user = await this.usersRepository.findOneBy(ps.userId != null @@ -93,23 +95,25 @@ export default class extends Endpoint { // eslint- const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); - if (profile.followersVisibility === 'private') { - if (me == null || (me.id !== user.id)) { - throw new ApiError(meta.errors.forbidden); - } - } else if (profile.followersVisibility === 'followers') { - if (me == null) { - throw new ApiError(meta.errors.forbidden); - } else if (me.id !== user.id) { - const isFollowing = await this.followingsRepository.exists({ - where: { - followeeId: user.id, - followerId: me.id, - }, - }); - if (!isFollowing) { + if (profile.followersVisibility !== 'public' && !await this.roleService.isModerator(me)) { + if (profile.followersVisibility === 'private') { + if (me == null || (me.id !== user.id)) { throw new ApiError(meta.errors.forbidden); } + } else if (profile.followersVisibility === 'followers') { + if (me == null) { + throw new ApiError(meta.errors.forbidden); + } else if (me.id !== user.id) { + const isFollowing = await this.followingsRepository.exists({ + where: { + followeeId: user.id, + followerId: me.id, + }, + }); + if (!isFollowing) { + throw new ApiError(meta.errors.forbidden); + } + } } } diff --git a/packages/backend/src/server/api/endpoints/users/following.ts b/packages/backend/src/server/api/endpoints/users/following.ts index 6b3389f0b2..feda5bb353 100644 --- a/packages/backend/src/server/api/endpoints/users/following.ts +++ b/packages/backend/src/server/api/endpoints/users/following.ts @@ -12,6 +12,7 @@ import { QueryService } from '@/core/QueryService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { DI } from '@/di-symbols.js'; +import { RoleService } from '@/core/RoleService.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -90,6 +91,7 @@ export default class extends Endpoint { // eslint- private utilityService: UtilityService, private followingEntityService: FollowingEntityService, private queryService: QueryService, + private roleService: RoleService, ) { super(meta, paramDef, async (ps, me) => { const user = await this.usersRepository.findOneBy(ps.userId != null @@ -102,23 +104,25 @@ export default class extends Endpoint { // eslint- const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); - if (profile.followingVisibility === 'private') { - if (me == null || (me.id !== user.id)) { - throw new ApiError(meta.errors.forbidden); - } - } else if (profile.followingVisibility === 'followers') { - if (me == null) { - throw new ApiError(meta.errors.forbidden); - } else if (me.id !== user.id) { - const isFollowing = await this.followingsRepository.exists({ - where: { - followeeId: user.id, - followerId: me.id, - }, - }); - if (!isFollowing) { + if (profile.followingVisibility !== 'public' && !await this.roleService.isModerator(me)) { + if (profile.followingVisibility === 'private') { + if (me == null || (me.id !== user.id)) { throw new ApiError(meta.errors.forbidden); } + } else if (profile.followingVisibility === 'followers') { + if (me == null) { + throw new ApiError(meta.errors.forbidden); + } else if (me.id !== user.id) { + const isFollowing = await this.followingsRepository.exists({ + where: { + followeeId: user.id, + followerId: me.id, + }, + }); + if (!isFollowing) { + throw new ApiError(meta.errors.forbidden); + } + } } } diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index 6962608106..763ce2b336 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -20,7 +20,8 @@ import { CoreModule } from '@/core/CoreModule.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { LoggerService } from '@/core/LoggerService.js'; import type { IActor, IApDocument, ICollection, IObject, IPost } from '@/core/activitypub/type.js'; -import { MiMeta, MiNote } from '@/models/_.js'; +import { MiMeta, MiNote, UserProfilesRepository } from '@/models/_.js'; +import { DI } from '@/di-symbols.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; import { DownloadService } from '@/core/DownloadService.js'; import { MetaService } from '@/core/MetaService.js'; @@ -86,6 +87,7 @@ async function createRandomRemoteUser( } describe('ActivityPub', () => { + let userProfilesRepository: UserProfilesRepository; let imageService: ApImageService; let noteService: ApNoteService; let personService: ApPersonService; @@ -127,6 +129,8 @@ describe('ActivityPub', () => { await app.init(); app.enableShutdownHooks(); + userProfilesRepository = app.get(DI.userProfilesRepository); + noteService = app.get(ApNoteService); personService = app.get(ApPersonService); rendererService = app.get(ApRendererService); @@ -205,6 +209,53 @@ describe('ActivityPub', () => { }); }); + describe('Collection visibility', () => { + test('Public following/followers', async () => { + const actor = createRandomActor(); + actor.following = { + id: `${actor.id}/following`, + type: 'OrderedCollection', + totalItems: 0, + first: `${actor.id}/following?page=1`, + }; + actor.followers = `${actor.id}/followers`; + + resolver.register(actor.id, actor); + resolver.register(actor.followers, { + id: actor.followers, + type: 'OrderedCollection', + totalItems: 0, + first: `${actor.followers}?page=1`, + }); + + const user = await personService.createPerson(actor.id, resolver); + const userProfile = await userProfilesRepository.findOneByOrFail({ userId: user.id }); + + assert.deepStrictEqual(userProfile.followingVisibility, 'public'); + assert.deepStrictEqual(userProfile.followersVisibility, 'public'); + }); + + test('Private following/followers', async () => { + const actor = createRandomActor(); + actor.following = { + id: `${actor.id}/following`, + type: 'OrderedCollection', + totalItems: 0, + // first: … + }; + actor.followers = `${actor.id}/followers`; + + resolver.register(actor.id, actor); + //resolver.register(actor.followers, { … }); + + const user = await personService.createPerson(actor.id, resolver); + const userProfile = await userProfilesRepository.findOneByOrFail({ userId: user.id }); + + assert.deepStrictEqual(userProfile.followingVisibility, 'private'); + assert.deepStrictEqual(userProfile.followersVisibility, 'private'); + }); + }); + describe('Renderer', () => { test('Render an announce with visibility: followers', () => { rendererService.renderAnnounce('https://example.com/notes/00example', { diff --git a/packages/frontend/src/scripts/isFfVisibleForMe.ts b/packages/frontend/src/scripts/isFfVisibleForMe.ts index 406404c462..e28e5725bc 100644 --- a/packages/frontend/src/scripts/isFfVisibleForMe.ts +++ b/packages/frontend/src/scripts/isFfVisibleForMe.ts @@ -7,7 +7,7 @@ import * as Misskey from 'misskey-js'; import { $i } from '@/account.js'; export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): boolean { - if ($i && $i.id === user.id) return true; + if ($i && ($i.id === user.id || $i.isAdmin || $i.isModerator)) return true; if (user.followingVisibility === 'private') return false; if (user.followingVisibility === 'followers' && !user.isFollowing) return false; @@ -15,7 +15,7 @@ export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): bo return true; } export function isFollowersVisibleForMe(user: Misskey.entities.UserDetailed): boolean { - if ($i && $i.id === user.id) return true; + if ($i && ($i.id === user.id || $i.isAdmin || $i.isModerator)) return true; if (user.followersVisibility === 'private') return false; if (user.followersVisibility === 'followers' && !user.isFollowing) return false;