Bridge admin power levels to Matrix when someone joins the Gitter room or changes the security descriptor
Bridge admin power levels to Matrix when someone joins the Gitter room or changes the security descriptor
Fix #2860 (closed)
Part of #2857 (closed) -> #2609 (closed)
Bridge synchronization plan
Since some room permissions are based on GitHub/GitLab projects, we don't have a list of all of the admins for the room. We only know at the time of some user accessing the Gitter room, whether they are an admin. In order to get around this, we only synchronize admin power levels for people who are joined to the Gitter room.
I think we can go pretty far with this, if we follow these rules:
- When someone joins the room, check if they are an admin and also add power levels
- When someone changes the security descriptor of a room, make sure to:
- If room
sd.type === null
, we can short-circuit and only look atsd.extraAdmins
- If we can compare the previous and current
sd
, check whether only thesd.extraAdmins
changed and we can again short-circuit. - Otherwise, if something other than the
sd.extraAdmins
changed, loop through room members and check for new admins - This has potential race conditions that we need to keep in mind; like if there are thousands of room members to loop through, other security descriptor updates could happen in sequence that race each other or an admin could join while this loop is running and reset their permissions.
- Since there isn't an atomic way to edit power levels, I'm not sure there is a solid way to avoid this. The best we can do is minimize time between when we fetch and update.
- If room
- When someone changes the security descriptor of a group, loop through any rooms that inherit permissions from the group and make sure to:
- Same as above
Dev notes
update
/patch
events?
How do we know we are not leaking these new security descriptor Once concern I had when adding the events for the room/group security descriptor update
/patch
was whether we are now leaking this information out over the bayeux bridge and the websocket connection. Here is how that data is flowing and why it's fine:
-
dataChange2
events are emitted fromlive-collection-group-security-descriptors.js
/live-collection-room-security-descriptors.js
- Those
data2
events are listened to by theserver/event-listeners/bayeux-events-bridge.js
and published to the cluster (websockets) - But if we look at
server/web/bayeux/authorisor.js
, we can see that we only allow a specific subset of those channels to be subscribed to. And they havevalidator
functions to make sure only people only the right people can listen - We can see that there is no route that would match our new routes (
/groups/${groupId}/security
,/rooms/${roomId}/security
) routes so we not leaking anything extra.
Todo
-
Add tests for room security descriptor change -
When only sd.extraAdmins
had apatch
(meaning we can shortcut). Make sure we test that it adds and removes people -
When only sd
had anupdate
but it's for atype: null
/admins: 'MANUAL'
sort of room (meaning we can shortcut). Make sure we test that it adds and removes people -
When the sd.type
changed (meaning we have to loop over all room members to figure it out). Make sure we test that it adds and removes people
-
-
Add tests for group security descriptor change -
Ensure it updates any room in the group that inherits the group permissions
-