MSC4167: Copy bans on room upgrade #4167
Conversation
Signed-off-by: Matthias Ahouansou <[email protected]>
tulir
added
proposal
| In rooms with many banned users, it may take a while to perform the upgrade, as all the bans will need | ||
| to be performed. However, not doing this would allow previously banned users to access the room, which | ||
| is why this is deemed as worth it. |
There was a problem hiding this comment.
Additional potential issue: remote servers iirc are informed of the membership change, causing the banned user to potentially receive "You were banned from : " prompts.
There was a problem hiding this comment.
remote servers iirc are informed of the membership change
Why do you think that? Even if it were true, this scenario doesn't really make sense in situations where the remote server isn't even aware of a room.
There was a problem hiding this comment.
if you have @alice:matrix.org present within a room, and @bob:matrix.org becomes banned, then @bob:matrix.org will be force joined to the room and notified that they are banned (The event is sent to them down /sync iirc). This is because @bob:matrix.org's server (matrix.org) is sent the ban because @alice:matrix.org is participating in the room. Which is an issue because now bob is aware or has been reminded of the new room's existence.
There was a problem hiding this comment.
I don't really know if there's anything in the spec that causes that behaviour or of it's all just implicit and Synapse is doing something without thinking too much, which is still a problem.
There was a problem hiding this comment.
Just to check, this only happens with users on the same server as the user upgrading the room, right? Because last I checked if you /send events to remote servers from rooms they don't know about, they should get rejected.
There was a problem hiding this comment.
Notifying a user that they were banned in a room they never participated in (or are not currently participating in, i.e. their membership is leave) seems like strange behavior anyways, and I don't think it is something that should happen.
There was a problem hiding this comment.
Just to check, this only happens with users on the same server as the user upgrading the room, right?
Haven't tested.
seems like strange behavior anyways, and I don't think it is something that should happen.
It does happen though until there is some work to clarify or make sure that this doesn't happen (if it is even possible to do so since it might be quite hard for a server to work out the extent that a user was participating in the room or not).
There was a problem hiding this comment.
it might be quite hard for a server to work out the extent that a user was participating in the room or not
It shouldn't be, since the server already needs to know their membership to send the event over /sync anyways (since the rooms field has different sub-fields for events depending on their current membership).
| after the upgrade. However, the spec currently states that "Membership events should not be transferred | ||
| to the new room due to technical limitations of servers not being able to impersonate people from other | ||
| homeservers". While this is most notably true for `join` membership events, this is not true for `ban` | ||
| membership, which users likely want copied over to the new room. |
There was a problem hiding this comment.
There is the argument to be made that users want the ability to control this behavior.
Why? Because if the room in question uses a Bot like Draupnir or Mjolnir they have no need to clone the bans potentially as they do all their bans via the bot. If all the bans are done via the bot why clutter the room state the room might argue.
There was a problem hiding this comment.
Ok then, adding a query parameter like copy_membership could be a good way to toggle this then. I don't specify bans with this parameter as in the future there may be other membership states which could be copied over, such as mutes. Does that sound good?
Also I'm not sure whether to mention in the MSC whether mutes should also be copied over once in the spec, and what to do about other future membership states.
There was a problem hiding this comment.
To not dig too deep into the intracies of #3909 and #4106 they have a problem in that being muted under them is counted as being a room member. Now if you copied the leave mute states that could work without issue. And you could in theory allow preemptively setting it via a trivial from my perspective change to #3909 that would be inherited by #4106
Current day mutes as in mutes before 3909 are coppied as they are baked into the powerlevels event.
There was a problem hiding this comment.
Implementation requirements:
- Server
There was a problem hiding this comment.
Implementations:
- Synapse: Transfer bans on room upgrade synapse#4642
- Dendrite: Added /upgrade endpoint dendrite#2307
| When a room upgrade is performed, servers SHOULD copy membership states with the `ban` `membership` to | ||
| the new room, to ensure that users banned in the old room are still banned in the new room. | ||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
There are potential safety concerns with this behaviour, fwiw. Namely:
- Banned users are informed that they're banned from the new room too, which leaks the room ID. (mentioned here: https://github.com/matrix-org/matrix-spec-proposals/pull/4167/files#r1667731522 )
- Starting the room off with a bunch of abuse-containing state events isn't great
There was a problem hiding this comment.
Its good that Draupnir and Mjolnir allows you to avoid these issues if you are willing to accept the tradeoff that comes with reactive sanctions instead of preemptive sanctions.
| @@ -0,0 +1,40 @@ | |||
| # MSC4167: Copy bans on room upgrade | |||
There was a problem hiding this comment.
It's unclear to me if we want to be more prescriptive in the room upgrade process: matrix-org/matrix-spec#1906 exists for discussion.
Rendered
Implementations: