Fix #2672 - Connect signed PuSH subscription requests to instance domain#4205
Fix #2672 - Connect signed PuSH subscription requests to instance domain#4205
Conversation
|
Bonus round: at the cost of backward compatibility, we could require that the domain field be filled, or not send private toots to it - that way we'd get rid of the non-Mastodon instances getting private toots issue. But that's up for debate since it'd exclude outdated Mastodon instances too |
ClearlyClaire
left a comment
ClearlyClaire
left a comment
There was a problem hiding this comment.
Apart from one corner case that is not new to this PR, looks good to me.
| @domains.include?(Addressable::URI.parse(callback_url).host) | ||
| def allowed_to_receive?(callback_url, domain) | ||
| (!domain.nil? && @domains.include?(domain)) || @domains.include?(Addressable::URI.parse(callback_url).host) | ||
| end |
There was a problem hiding this comment.
This is not new to this PR, but I think you actually want .normalized_host instead of .host.
There was a problem hiding this comment.
If I remember correctly, callback_url is normalized before save.
There was a problem hiding this comment.
Ah, seems to be the case, indeed.
|
Ah, I didn't adjust the tests. Also, the method signature having so many params does look bad, but I dunno how to improve it. |
|
@Gargron I'm not completely against enforcing the subscription requests to be signed for private toot delivery, but I think this should be done in a later release, exactly for the reasons you mentioned. |
…ance domain (mastodon#4205) * Fix mastodon#2672 - Connect signed PuSH subscription requests to instance domain Resolves mastodon#2739 * Fix return of locate_subscription * Fix tests
2 participants
Resolves #2739