Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Aug 9, 2024
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## Unreleased

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

### Client
-
Expand Down
50 changes: 49 additions & 1 deletion packages/backend/src/core/activitypub/models/ApPersonService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -532,6 +566,8 @@ export class ApPersonService implements OnModuleInit {
url,
fields,
description: _description,
followingVisibility,
followersVisibility,
birthday: bday?.[0] ?? null,
location: person['vcard:Address'] ?? null,
});
Expand Down Expand Up @@ -703,4 +739,16 @@ export class ApPersonService implements OnModuleInit {

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;
}
}
Comment on lines +744 to 754
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

itemsが0より上だった場合に公開扱いして良いのか?

Mastodonのすごくわかりづらいコードも、判定に使用しているのはfirstだけでitemsは考慮してないように見えるのだわね。
https://github.com/mastodon/mastodon/pull/11673/files#diff-0bc8e960429e6c65ff6b5f31c40c67ed4cab1904dcf2dcb9f13d1503338c84ef

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Activity Streams勧告のCollections節によると、MastodonのようにfirstプロパティでCollectionPageオブジェクトを指す形式の他にitemsCollectionに直接埋め込む形も規定されているので、単にMastodonのコードがMastodon以外のサーバを想定していないだけであろうと認識しています。

ちなみに0のような整数値を取るのはitemsでなくtotalItemsプロパティですね。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

itemsでなくtotalItemsプロパティですね。

ごめんそれと間違ってたのだわ

6 changes: 4 additions & 2 deletions packages/backend/src/core/activitypub/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/core/entities/UserEntityService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
34 changes: 19 additions & 15 deletions packages/backend/src/server/api/endpoints/users/followers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -81,6 +82,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // 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
Expand All @@ -93,22 +95,24 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // 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);
}
}
}
}
Expand Down
34 changes: 19 additions & 15 deletions packages/backend/src/server/api/endpoints/users/following.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -90,6 +91,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // 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
Expand All @@ -102,22 +104,24 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // 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);
}
}
}
}
Expand Down
53 changes: 52 additions & 1 deletion packages/backend/test/unit/activitypub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -86,6 +87,7 @@ async function createRandomRemoteUser(
}

describe('ActivityPub', () => {
let userProfilesRepository: UserProfilesRepository;
let imageService: ApImageService;
let noteService: ApNoteService;
let personService: ApPersonService;
Expand Down Expand Up @@ -127,6 +129,8 @@ describe('ActivityPub', () => {
await app.init();
app.enableShutdownHooks();

userProfilesRepository = app.get(DI.userProfilesRepository);

noteService = app.get<ApNoteService>(ApNoteService);
personService = app.get<ApPersonService>(ApPersonService);
rendererService = app.get<ApRendererService>(ApRendererService);
Expand Down Expand Up @@ -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', {
Expand Down
4 changes: 2 additions & 2 deletions packages/frontend/src/scripts/isFfVisibleForMe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ 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;

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;
Expand Down
Loading