Skip to content

Bridge admin power levels to Matrix when someone joins the Gitter room or changes the security descriptor

Eric Eastwood requested to merge 2860-bridge-permissions-admins-to-matrix into develop

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:

  1. When someone joins the room, check if they are an admin and also add power levels
  2. When someone changes the security descriptor of a room, make sure to:
    • If room sd.type === null, we can short-circuit and only look at sd.extraAdmins
    • If we can compare the previous and current sd, check whether only the sd.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.
  3. 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

How do we know we are not leaking these new security descriptor update/patch events?

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:

  1. dataChange2 events are emitted from live-collection-group-security-descriptors.js/live-collection-room-security-descriptors.js
  2. Those data2 events are listened to by the server/event-listeners/bayeux-events-bridge.js and published to the cluster (websockets)
  3. 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 have validator functions to make sure only people only the right people can listen
  4. 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 a patch (meaning we can shortcut). Make sure we test that it adds and removes people
    • When only sd had an update but it's for a type: 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
Edited by Eric Eastwood

Merge request reports