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): Handle array values for Actor.publicKey #14181

Closed
wants to merge 2 commits into from

Conversation

tesaguri
Copy link
Contributor

What

リモートのActivityPubのActorpublicKeyの値として配列が設定されているケースに対応します。配列が設定されている場合はその先頭の値をアクターの公開鍵として採用します。

Why

ActivityPub and HTTP Signatures community group reportにpublicKeyが複数の値を持つ場合の処理についての記述があり、ActivityPub標準としてはpublicKeyは複数の値を取りうるという立場であることが読み取れます(関連:swicg/activitypub-http-signature#9)。

https://swicg.github.io/activitypub-http-signature/#how-to-obtain-a-signature-s-public-key:

  1. The public key object will be in the actor's publicKey property. If there are multiple values, find the one whose id matches the original keyId.

引用元の仕様では複数の値から目的の鍵のidに合致するものを選ぶように定められていますが、本パッチではとりあえずエラーを抑止するのを主目的とし、簡単のためMastodonの挙動に合わせて先頭の値を取る形で対応しています。

Additional info (optional)

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

`publicKey`は複数の値を取りうるとされているので、そのケースへの対応。

cf. https://swicg.github.io/activitypub-http-signature/#how-to-obtain-a-signature-s-public-key

引用元の仕様では複数の値から適するものを選ぶように定められているが、まず
は簡単のためMastodonと同様に先頭の値のみを取る形で対応した。
@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jul 11, 2024
@KisaragiEffective KisaragiEffective added the 🌌Federation The Federation/ActivityPub feature label Jul 11, 2024
@tesaguri tesaguri force-pushed the actor-publickey-array branch from a425244 to 4c48710 Compare July 11, 2024 13:23
@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Jul 11, 2024

Maybe related to #13464
(「複数の鍵に対応」というワードをこっちでも見たので)

Copy link
Contributor

github-actions bot commented Jul 11, 2024

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

差分はこちら

Get diff files from Workflow Page

Copy link

codecov bot commented Jul 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.15%. Comparing base (121af77) to head (450988e).

Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #14181       +/-   ##
============================================
+ Coverage    20.42%   40.15%   +19.72%     
============================================
  Files          698     1517      +819     
  Lines        97838   187536    +89698     
  Branches      1013     3470     +2457     
============================================
+ Hits         19984    75304    +55320     
- Misses       77335   111663    +34328     
- Partials       519      569       +50     

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

@kakkokari-gtyih
Copy link
Contributor

Maybe related to #13464 (「複数の鍵に対応」というワードをこっちでも見たので)

今コード見たけど #13464 でこの変更をカバーできている気がするのでどっちかマージでよさそう

@tamaina
Copy link
Contributor

tamaina commented Jul 12, 2024

DUPLICATE OF #13950

@tesaguri
Copy link
Contributor Author

tesaguri commented Jul 12, 2024

#13464 については大きな変更なのでまずは本パッチのような小さな変更で後方互換性だけ確保しておくのもありかと思っていましたが、#13950 についてはスコープがほぼ一致しているので、こちらはcloseしてしまった方が良さそうですね。

@tesaguri tesaguri closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌌Federation The Federation/ActivityPub feature packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants