"Freeze" a room when the last admin of that room leaves #59
Conversation
|
I know it makes things more complicated, but I think this PR should be split in two: one on mainline Synapse to add the capability to send an event to the third party event rules, and one here built on top if it to implement this specific behaviour. Otherwise we'll end up diverging from mainline's API or creating merge sadness. |
|
@babolivier When you say built on top, are you recommending a merge from mainline, or just cherry-picking the commit? |
Ideally merging from mainline. If that's not possible then maybe a merge from the feature branch, or cherry-picking. |
|
@babolivier OK, I'll break this up then. |
anoadragon453
force-pushed
the
anoa/freeze_rooms
branch
2 times, most recently
from
d7cb62f
to
763fa63
Compare
When a membership event is checked by the room access rules, we check if it's both a leave membership, as well as whether this belongs to the last admin (PL>=100) user in a room. If it does, we freeze the room. When freezing the room, we take the existing power level state event of the room, modify it so that everything requires PL100 (meaning the admin could come back and fix the room if desired), sends the state event, and then actually leaves. We are careful to only do this if the user leaving is a local user. Otherwise, we wouldn't be able to puppet them and send a state event into the room. To enable this, we also now pass server_name to room access rules.
Turns out we were overridding the content of the old power level event in the room, which means that when it came time to persist the new power level event, the code compared the content of the two and determined that the new state event was a duplicated, and thus it wasn't persisted - so clients didn't see it down sync, and the new power levels were not enforced after a server restart. Copying the old power levels dict instead of using it directly solves the issue.
anoadragon453
force-pushed
the
anoa/freeze_rooms
branch
from
643082a
to
ac5d666
Compare
| # Ensure state_default and events_default keys exist and are 100 | ||
| # Else a lower PL user could potentially send state events that | ||
| # aren't explicitly mentioned elsewhere in the power level dict | ||
| new_content["state_default"] = 100 | ||
| new_content["events_default"] = 100 |
There was a problem hiding this comment.
we might still have issues with missing ban, kick, etc keys, each of which default to 50?
There was a problem hiding this comment.
where is defined this default to 50 for missing ban, kick and others?
There was a problem hiding this comment.
It's written in the spec: https://matrix.org/docs/spec/client_server/r0.6.1#m-room-power-levels
There was a problem hiding this comment.
We're going to go with setting these to 100 if they're missing for now as we set them to 100 above if they're present anyways.
If the last admin of a room departs, and thus the room no longer has any admins within it, we "freeze" the room. Freezing a room means that the power level required to do anything in the room (sending messages, inviting others etc) will require power level 100.
At the moment, an admin can come back and unfreeze the room manually. The plan is to eventually make unfreezing of the room automatic on admin rejoin, though that will be in a separate PR.
This could work in mainline, however if the admin who leaves is on a homeserver without this functionality, then the room isn't frozen. I imagine this would probably be pretty confusing to people. Part of this feature was allowing Synapse modules to send events, which has been implemented in mainline at matrix-org/synapse#8479, and cherry-picked to the
dinsicfork in 62c7b10. The actual freezing logic has been implemented here in the RoomAccessRules module.Reviewable commit-by-commit, commit messages have the breakdown.