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

Conversation

tesaguri
Copy link
Contributor

@tesaguri tesaguri commented Aug 7, 2024

What

リモートユーザのプロフィールを取得する際にActivityPubアクターのfollowingfollowersコレクションの取得を試み、その内容が取得できない場合は当該ユーザのフォロー・フォロワーの一覧を非公開としてデータベースに登録するようにします。

また、モデレータが全てのローカル・リモートユーザのフォロー・フォロワーの一覧を取得できるようにします。

Why

Resolves #13362.

Additional info (optional)

最低限の単体テストは追加していますが実際のサーバにおける連合の挙動は未確認です。

先行事例:mastodon/mastodon#11673https://github.com/mastodon/mastodon/blob/9d8dfeb5fbbc274482489a3ac9f22dd736da156c/app/services/activitypub/process_account_service.rb#L262-L281

本パッチでは「フォロワー限定」の公開範囲はとりあえず一律で非公開として扱うものとしています。

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/backend:test labels Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 7, 2024

このPRによるapi.jsonの差分

差分はこちら

Get diff files from Workflow Page

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 42.10526% with 55 lines in your changes missing coverage. Please review.

Project coverage is 39.97%. Comparing base (61eec93) to head (ff79faa).
Report is 260 commits behind head on develop.

Files Patch % Lines
...end/src/core/activitypub/models/ApPersonService.ts 61.22% 19 Missing ⚠️
...ackend/src/server/api/endpoints/users/followers.ts 5.26% 18 Missing ⚠️
...ackend/src/server/api/endpoints/users/following.ts 5.26% 18 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #14375       +/-   ##
============================================
- Coverage    66.21%   39.97%   -26.25%     
============================================
  Files          990     1544      +554     
  Lines       117137   190414    +73277     
  Branches      4489     2717     -1772     
============================================
- Hits         77568    76124     -1444     
- Misses       39538   113729    +74191     
- Partials        31      561      +530     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Sayamame-beans
Copy link
Member

13362をresolveするか微妙に思います
(要件に※ただし https://github.com/misskey-dev/misskey/issues/13127 のようにモデレーションに影響が出ることが考えられる為権限がある場合は見えるようにがあります。このPRではリモートユーザーのfollowing/follwersにprivateの概念を導入するものと思いますので、13362をcloseするには全ユーザーに対してこの要件を満たす実装を行ったPRを別途用意する必要がありそうです?)

@tesaguri
Copy link
Contributor Author

tesaguri commented Aug 7, 2024

#13128 に倣ってモデレータが非公開のfollowing/followersを見られるようにする機能もとりあえず本PRにまとめて追加しておきました。

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Aug 7, 2024
@tesaguri tesaguri changed the title fix(backend): check visibility of following/followers of remote users fix(backend): check visibility of following/followers of remote users / feat: moderators can see following/followers of all users Aug 7, 2024
@tesaguri
Copy link
Contributor Author

tesaguri commented Aug 8, 2024

6d91616d9eeb8dの間の変更はCHANGELOG.mdのみであってバックエンドのE2Eテストが新たに落ちるような要素がないので、flakyな失敗とみなして良さそうですかね?)

@Sayamame-beans
Copy link
Member

(再試行で通りましたね)

`createPerson`と`if`の条件を統一するとともに、異常系の
処理をearly returnに追い出すための変更。
@zyoshoka
Copy link
Contributor

zyoshoka commented Aug 8, 2024

モデレーターが users/followingusers/followers を取得できるようにするのであれば、users/showfollowingCountfollowersCount を読めるようにするのが自然かと思います。

Related to #12214

@syuilo syuilo merged commit 0d508db into misskey-dev:develop Aug 9, 2024
26 checks passed
@syuilo
Copy link
Member

syuilo commented Aug 9, 2024

🙏🏿🙏🏿

@tesaguri tesaguri deleted the remote-ff-visibility branch August 9, 2024 10:11
LemonDouble pushed a commit to LemonDouble/misskey that referenced this pull request Aug 19, 2024
… / feat: moderators can see following/followers of all users (misskey-dev#14375)

* fix(backend): check visibility of following/followers of remote users

Resolves misskey-dev#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 misskey-dev#14375 (comment).
Comment on lines +744 to 754
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;
}
}
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プロパティですね。

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend:test packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
5 participants