fix(backend): check visibility of following/followers of remote users / feat: moderators can see following/followers of all users (#14375)

* 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.
This commit is contained in:
Daiki Mizukami 2024-08-09 12:10:51 +09:00 committed by GitHub
parent f244d42500
commit 0d508db8a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 149 additions and 39 deletions

View File

@ -1,7 +1,8 @@
## Unreleased ## Unreleased
### General ### General
- - Fix: リモートユーザのフォロー・フォロワーの一覧が非公開設定の場合も表示できてしまう問題を修正
- Enhance: モデレーターはすべてのユーザーのフォロー・フォロワーの一覧を見られるように
### Client ### Client
- -

View File

@ -48,7 +48,7 @@ import type { ApResolverService, Resolver } from '../ApResolverService.js';
import type { ApLoggerService } from '../ApLoggerService.js'; import type { ApLoggerService } from '../ApLoggerService.js';
// eslint-disable-next-line @typescript-eslint/consistent-type-imports // eslint-disable-next-line @typescript-eslint/consistent-type-imports
import type { ApImageService } from './ApImageService.js'; 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 nameLength = 128;
const summaryLength = 2048; const summaryLength = 2048;
@ -296,6 +296,21 @@ export class ApPersonService implements OnModuleInit {
const isBot = getApType(object) === 'Service' || getApType(object) === 'Application'; 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 bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/);
const url = getOneApHrefNullable(person.url); const url = getOneApHrefNullable(person.url);
@ -357,6 +372,8 @@ export class ApPersonService implements OnModuleInit {
description: _description, description: _description,
url, url,
fields, fields,
followingVisibility,
followersVisibility,
birthday: bday?.[0] ?? null, birthday: bday?.[0] ?? null,
location: person['vcard:Address'] ?? null, location: person['vcard:Address'] ?? null,
userHost: host, userHost: host,
@ -464,6 +481,23 @@ export class ApPersonService implements OnModuleInit {
const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); 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 bday = person['vcard:bday']?.match(/^\d{4}-\d{2}-\d{2}/);
const url = getOneApHrefNullable(person.url); const url = getOneApHrefNullable(person.url);
@ -532,6 +566,8 @@ export class ApPersonService implements OnModuleInit {
url, url,
fields, fields,
description: _description, description: _description,
followingVisibility,
followersVisibility,
birthday: bday?.[0] ?? null, birthday: bday?.[0] ?? null,
location: person['vcard:Address'] ?? null, location: person['vcard:Address'] ?? null,
}); });
@ -703,4 +739,16 @@ export class ApPersonService implements OnModuleInit {
return 'ok'; return 'ok';
} }
@bindThis
private async isPublicCollection(collection: string | ICollection | IOrderedCollection | undefined, resolver: Resolver): Promise<boolean> {
if (collection) {
const resolved = await resolver.resolveCollection(collection);
if (resolved.first || (resolved as ICollection).items || (resolved as IOrderedCollection).orderedItems) {
return true;
}
}
return false;
}
} }

View File

@ -97,13 +97,15 @@ export interface IActivity extends IObject {
export interface ICollection extends IObject { export interface ICollection extends IObject {
type: 'Collection'; type: 'Collection';
totalItems: number; totalItems: number;
items: ApObject; first?: IObject | string;
items?: ApObject;
} }
export interface IOrderedCollection extends IObject { export interface IOrderedCollection extends IObject {
type: 'OrderedCollection'; type: 'OrderedCollection';
totalItems: number; totalItems: number;
orderedItems: ApObject; first?: IObject | string;
orderedItems?: ApObject;
} }
export const validPost = ['Note', 'Question', 'Article', 'Audio', 'Document', 'Image', 'Page', 'Video', 'Event']; export const validPost = ['Note', 'Question', 'Article', 'Audio', 'Document', 'Image', 'Page', 'Video', 'Event'];

View File

@ -454,12 +454,12 @@ export class UserEntityService implements OnModuleInit {
} }
const followingCount = profile == null ? null : 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 : (profile.followingVisibility === 'followers') && (relation && relation.isFollowing) ? user.followingCount :
null; null;
const followersCount = profile == null ? 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 : (profile.followersVisibility === 'followers') && (relation && relation.isFollowing) ? user.followersCount :
null; null;

View File

@ -11,6 +11,7 @@ import { QueryService } from '@/core/QueryService.js';
import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js';
import { UtilityService } from '@/core/UtilityService.js'; import { UtilityService } from '@/core/UtilityService.js';
import { DI } from '@/di-symbols.js'; import { DI } from '@/di-symbols.js';
import { RoleService } from '@/core/RoleService.js';
import { ApiError } from '../../error.js'; import { ApiError } from '../../error.js';
export const meta = { export const meta = {
@ -81,6 +82,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
private utilityService: UtilityService, private utilityService: UtilityService,
private followingEntityService: FollowingEntityService, private followingEntityService: FollowingEntityService,
private queryService: QueryService, private queryService: QueryService,
private roleService: RoleService,
) { ) {
super(meta, paramDef, async (ps, me) => { super(meta, paramDef, async (ps, me) => {
const user = await this.usersRepository.findOneBy(ps.userId != null const user = await this.usersRepository.findOneBy(ps.userId != null
@ -93,23 +95,25 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id });
if (profile.followersVisibility === 'private') { if (profile.followersVisibility !== 'public' && !await this.roleService.isModerator(me)) {
if (me == null || (me.id !== user.id)) { if (profile.followersVisibility === 'private') {
throw new ApiError(meta.errors.forbidden); if (me == null || (me.id !== user.id)) {
}
} 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); 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);
}
}
} }
} }

View File

@ -12,6 +12,7 @@ import { QueryService } from '@/core/QueryService.js';
import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js'; import { FollowingEntityService } from '@/core/entities/FollowingEntityService.js';
import { UtilityService } from '@/core/UtilityService.js'; import { UtilityService } from '@/core/UtilityService.js';
import { DI } from '@/di-symbols.js'; import { DI } from '@/di-symbols.js';
import { RoleService } from '@/core/RoleService.js';
import { ApiError } from '../../error.js'; import { ApiError } from '../../error.js';
export const meta = { export const meta = {
@ -90,6 +91,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
private utilityService: UtilityService, private utilityService: UtilityService,
private followingEntityService: FollowingEntityService, private followingEntityService: FollowingEntityService,
private queryService: QueryService, private queryService: QueryService,
private roleService: RoleService,
) { ) {
super(meta, paramDef, async (ps, me) => { super(meta, paramDef, async (ps, me) => {
const user = await this.usersRepository.findOneBy(ps.userId != null const user = await this.usersRepository.findOneBy(ps.userId != null
@ -102,23 +104,25 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id }); const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id });
if (profile.followingVisibility === 'private') { if (profile.followingVisibility !== 'public' && !await this.roleService.isModerator(me)) {
if (me == null || (me.id !== user.id)) { if (profile.followingVisibility === 'private') {
throw new ApiError(meta.errors.forbidden); if (me == null || (me.id !== user.id)) {
}
} 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); 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);
}
}
} }
} }

View File

@ -20,7 +20,8 @@ import { CoreModule } from '@/core/CoreModule.js';
import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
import { LoggerService } from '@/core/LoggerService.js'; import { LoggerService } from '@/core/LoggerService.js';
import type { IActor, IApDocument, ICollection, IObject, IPost } from '@/core/activitypub/type.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 { secureRndstr } from '@/misc/secure-rndstr.js';
import { DownloadService } from '@/core/DownloadService.js'; import { DownloadService } from '@/core/DownloadService.js';
import { MetaService } from '@/core/MetaService.js'; import { MetaService } from '@/core/MetaService.js';
@ -86,6 +87,7 @@ async function createRandomRemoteUser(
} }
describe('ActivityPub', () => { describe('ActivityPub', () => {
let userProfilesRepository: UserProfilesRepository;
let imageService: ApImageService; let imageService: ApImageService;
let noteService: ApNoteService; let noteService: ApNoteService;
let personService: ApPersonService; let personService: ApPersonService;
@ -127,6 +129,8 @@ describe('ActivityPub', () => {
await app.init(); await app.init();
app.enableShutdownHooks(); app.enableShutdownHooks();
userProfilesRepository = app.get(DI.userProfilesRepository);
noteService = app.get<ApNoteService>(ApNoteService); noteService = app.get<ApNoteService>(ApNoteService);
personService = app.get<ApPersonService>(ApPersonService); personService = app.get<ApPersonService>(ApPersonService);
rendererService = app.get<ApRendererService>(ApRendererService); rendererService = app.get<ApRendererService>(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', () => { describe('Renderer', () => {
test('Render an announce with visibility: followers', () => { test('Render an announce with visibility: followers', () => {
rendererService.renderAnnounce('https://example.com/notes/00example', { rendererService.renderAnnounce('https://example.com/notes/00example', {

View File

@ -7,7 +7,7 @@ import * as Misskey from 'misskey-js';
import { $i } from '@/account.js'; import { $i } from '@/account.js';
export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): boolean { 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 === 'private') return false;
if (user.followingVisibility === 'followers' && !user.isFollowing) return false; if (user.followingVisibility === 'followers' && !user.isFollowing) return false;
@ -15,7 +15,7 @@ export function isFollowingVisibleForMe(user: Misskey.entities.UserDetailed): bo
return true; return true;
} }
export function isFollowersVisibleForMe(user: Misskey.entities.UserDetailed): boolean { 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 === 'private') return false;
if (user.followersVisibility === 'followers' && !user.isFollowing) return false; if (user.followersVisibility === 'followers' && !user.isFollowing) return false;