misskey-dev / misskey

🌎 A completely free and open interplanetary microblogging platform 🚀
https://misskey-hub.net/
GNU Affero General Public License v3.0
9.93k stars 1.34k forks source link

一人のリモートユーザーが複数の公開鍵を持つコーナーケースを処理 #13950

Open dahlia opened 3 months ago

dahlia commented 3 months ago

What

一人のリモートユーザーが複数の公開鍵を持つコーナーケースを処理します.

This patch handles the corner case where a remote user has multiple public keys.

Why

理論上、アクターのpublicKeyは複数の公開鍵を含む配列にすることができます。Mastodonもこのケースを処理しています。

In theory, an actor's publicKey could be an array containing multiple public keys. Mastodon also already handles this case.

Additional info (optional)

現在はMiUserPublicKeyの主キーがuserIdなので、一人のユーザが複数の公開鍵を持っている場合、最初の鍵だけを保存するように実装しました。将来的にはMiUserPublicKeyの主キーを(userId, keyId)に変える方が良いかもしれません。

For now, the primary key of MiUserPublicKey is userId, so my implementation only stores the first key if a user has multiple public keys. In the future, it might be better to change the primary key of MiUserPublicKey to (userId, keyId).

Checklist

github-actions[bot] commented 3 months ago

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

差分はこちら ```diff ```

Get diff files from Workflow Page

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 40.69%. Comparing base (43cccaa) to head (a143a5c). Report is 186 commits behind head on develop.

Files Patch % Lines
...end/src/core/activitypub/models/ApPersonService.ts 50.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #13950 +/- ## ============================================ - Coverage 77.84% 40.69% -37.15% ============================================ Files 185 1532 +1347 Lines 25595 189360 +163765 Branches 487 5755 +5268 ============================================ + Hits 19924 77068 +57144 - Misses 5664 111743 +106079 - Partials 7 549 +542 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tai-cha commented 3 months ago

may related to: https://github.com/misskey-dev/misskey/pull/13464 (複数の鍵を実際に持とうとしているPRがあります)

tamaina commented 3 months ago

理論上(In theory)、アクターのpublicKeyは複数の公開鍵を含む配列にすることができます。Mastodonもこのケースを処理しています。

配列でもただのオブジェクトでもいいのはActivityStreamsの仕様?

tamaina commented 3 months ago

配列でもただのオブジェクトでもいいのはActivityStreamsの仕様?

JSON LDの仕様だった

dahlia commented 3 months ago

@tamaina https://github.com/misskey-dev/misskey/commit/64004fdea244f039c34e9f59a103be9b9e164f77のコミットでパッチが既に適用されているようですが、このPRはもう閉じてもいいのでしょうか?

tamaina commented 3 months ago

@dahlia 64004fd は#13464 のコミットので… 私には判断できないですね

dahlia commented 3 months ago

それでは、https://github.com/misskey-dev/misskey/pull/13464のPRがマージされたら、このPRも閉じます。

tamaina commented 2 months ago

https://github.com/misskey-dev/misskey/pull/13464 をマージしました 🙏

tesaguri commented 2 months ago

13464 がrevertされましたが( 337b42bcb179bdfb993888ed94342a0158e8f3cb )、仮に次のリリースまでに同PRのレビューが完了する見込みがないのでしたら、比較的レビューの負荷が軽いと思われる本PRを先行してマージするというのはいかがでしょうか?

最終的に複数の公開鍵の受け入れに本格対応するにしても、先行して本PRのような保守的な変更で前方互換性を担保しておいた方が何かと役立ちうるのではないかと考えます。(実際に本PRの変更に依存した仕様を採用するかはさておいても、設計上の選択肢は多いに越したことはないので。)

tai-cha commented 2 months ago

コメントを受けて一応reopen(もし再度ed25519のほう #14278 が入るならまたクローズすることになりそう)