matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.82k stars 2.13k forks source link

Implement MSC4028: push all encrypted events. #16361

Closed clokep closed 1 year ago

clokep commented 1 year ago

See MSC4028.

Fixes #15847

clokep commented 1 year ago

My gut reaction is that pushing all encrypted events to clients sounds excessive and might mean the push gateway gets rate limited. But I have no actual data on that. Maybe this is what a mobile app with E2EE messaging has to do? (Are there any open source services like this (Signal?) we could compare to?)

This sounds like feedback for the MSC, not the implementation.

Does this only do push-gateway push, or does it also mean that we'll send out emails to people with email notifications turned on every time they get an encrypted message?

Hmm, I think this means we would end up sending emails too, which is not ideal. Again this might be good feedback for the MSC?

Also: is it worth a test case for the new push rule? (Not sure if we test them individually?)

We don't usually test each base rule individually, but I can add one if you'd like. (Especially for really simple ones I'm unconvinced it is very useful TBH.)

DMRobertson commented 1 year ago

We don't usually test each base rule individually, but I can add one if you'd like. (Especially for really simple ones I'm unconvinced it is very useful TBH.)

That's fair---just wanted to sanity check what the done thing here was.

clokep commented 1 year ago

I can move the feedback over to the MSC if you'd like.

DMRobertson commented 1 year ago

I can move the feedback over to the MSC if you'd like.

Ahh, I left a minimum-effort block comment on the MSC just now.

clokep commented 1 year ago

Happy that this implements the MSC.

I'm going to merge this for now, it is disabled by default so this shouldn't cause issues and we can take the concerns to the MSC?