Override any missing default power level keys with DINUM's defaults when creating a room (#68)

The createRoom flow in DINUM's Synapse (through the AccessRules module which has hooks for all of this) already rejects a power levels content dict if it doesn't have high enough power levels to satisfy DINUM's [requirements](https://github.com/matrix-org/synapse-dinsic/blob/ac50ed353b5fdbdf9f853be0d94b6fccaf33973e/synapse/third_party_rules/access_rules.py#L233-L252).

This PR ensures that any keys that aren't provided are replaced with the defaults, instead of just assuming the whole dict was correct (and thus those keys were set to mainline Synapse's default instead).

Author: anoadragon453

changelog.d/68.misc

@@ -0,0 +1 @@
+Override any missing default power level keys with DINUM's defaults when creating a room.

synapse/third_party_rules/access_rules.py

@@ -189,34 +189,40 @@ async def on_create_room(
         ) and access_rule == AccessRules.DIRECT:
             raise SynapseError(400, "Invalid access rule")
 
+        default_power_levels = self._get_default_power_levels(
+            requester.user.to_string()
+        )
+
         # Check if the creator can override values for the power levels.
         allowed = self._is_power_level_content_allowed(
-            config.get("power_level_content_override", {}), access_rule
+            config.get("power_level_content_override", {}),
+            access_rule,
+            default_power_levels,
         )
         if not allowed:
             raise SynapseError(400, "Invalid power levels content override")
 
-        use_default_power_levels = True
-        if config.get("power_level_content_override"):
-            use_default_power_levels = False
+        custom_user_power_levels = config.get("power_level_content_override")
 
         # Second loop for events we need to know the current rule to process.
         for event in config.get("initial_state", []):
             if event["type"] == EventTypes.PowerLevels:
                 allowed = self._is_power_level_content_allowed(
-                    event["content"], access_rule
+                    event["content"], access_rule, default_power_levels
                 )
                 if not allowed:
                     raise SynapseError(400, "Invalid power levels content")
 
-                use_default_power_levels = False
-
-        # If power levels were not overridden by the user, override with DINUM's preferred
-        # defaults instead
-        if use_default_power_levels:
-            config["power_level_content_override"] = self._get_default_power_levels(
-                requester.user.to_string()
-            )
+                custom_user_power_levels = event["content"]
+        if custom_user_power_levels:
+            # If the user is using their own power levels, but failed to provide an expected
+            # key in the power levels content dictionary, fill it in from the defaults instead
+            for key, value in default_power_levels.items():
+                custom_user_power_levels.setdefault(key, value)
+        else:
+            # If power levels were not overridden by the user, completely override with the
+            # defaults instead
+            config["power_level_content_override"] = default_power_levels
 
         return True
 
@@ -710,7 +716,11 @@ def _on_membership_or_invite_direct(
         return True
 
     def _is_power_level_content_allowed(
-        self, content: Dict, access_rule: str, on_room_creation: bool = True
+        self,
+        content: Dict,
+        access_rule: str,
+        default_power_levels: Optional[Dict] = None,
+        on_room_creation: bool = True,
     ) -> bool:
         """Check if a given power levels event is permitted under the given access rule.
 
@@ -721,6 +731,8 @@ def _is_power_level_content_allowed(
         Args:
             content: The content of the m.room.power_levels event to check.
             access_rule: The access rule in place in this room.
+            default_power_levels: The default power levels when a room is created with
+                the specified access rule. Required if on_room_creation is True.
             on_room_creation: True if this call is happening during a room's
                 creation, False otherwise.
 
@@ -733,12 +745,19 @@ def _is_power_level_content_allowed(
         # have a special circumstance, but still want to encourage a certain pattern during
         # room creation.
         if on_room_creation:
-            # If invite requirements are <PL50
-            if content.get("invite", 50) < 50:
+            # We specifically don't fail if "invite" or "state_default" are None, as those
+            # values should be replaced with our "default" power level values anyways,
+            # which are compliant
+
+            invite = default_power_levels["invite"]
+            state_default = default_power_levels["state_default"]
+
+            # If invite requirements are less than our required defaults
+            if content.get("invite", invite) < invite:
                 return False
 
-            # If "other" state requirements are <PL100
-            if content.get("state_default", 100) < 100:
+            # If "other" state requirements are less than our required defaults
+            if content.get("state_default", state_default) < state_default:
                 return False
 
         # Check if we need to apply the restrictions with the current rule.

tests/rest/client/test_room_access_rules.py

@@ -185,6 +185,49 @@ def test_create_room_fails_on_incorrect_power_level_rules(self):
             expected_code=400,
         )
 
+    def test_create_room_with_missing_power_levels_use_default_values(self):
+        """
+        Tests that a room created with custom power levels, but without defining invite or state_default
+        succeeds, but the missing values are replaced with the defaults.
+        """
+
+        # Attempt to create a room without defining "invite" or "state_default"
+        modified_power_levels = RoomAccessRules._get_default_power_levels(self.user_id)
+        del modified_power_levels["invite"]
+        del modified_power_levels["state_default"]
+        room_id = self.create_room(
+            direct=True,
+            rule=AccessRules.DIRECT,
+            initial_state=[
+                {"type": "m.room.power_levels", "content": modified_power_levels}
+            ],
+        )
+
+        # This should succeed, but the defaults should be put in place instead
+        room_power_levels = self.helper.get_state(
+            room_id, "m.room.power_levels", self.tok
+        )
+        self.assertEqual(room_power_levels["invite"], 50)
+        self.assertEqual(room_power_levels["state_default"], 100)
+
+        # And now the same test, but using power_levels_content_override instead
+        # of initial_state (which takes a slightly different codepath)
+        modified_power_levels = RoomAccessRules._get_default_power_levels(self.user_id)
+        del modified_power_levels["invite"]
+        del modified_power_levels["state_default"]
+        room_id = self.create_room(
+            direct=True,
+            rule=AccessRules.DIRECT,
+            power_levels_content_override=modified_power_levels,
+        )
+
+        # This should succeed, but the defaults should be put in place instead
+        room_power_levels = self.helper.get_state(
+            room_id, "m.room.power_levels", self.tok
+        )
+        self.assertEqual(room_power_levels["invite"], 50)
+        self.assertEqual(room_power_levels["state_default"], 100)
+
     def test_existing_room_can_change_power_levels(self):
         """Tests that a room created with default power levels can have their power levels
         dropped after room creation
@@ -975,6 +1018,7 @@ def create_room(
         rule=None,
         preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT,
         initial_state=None,
+        power_levels_content_override=None,
         expected_code=200,
     ):
         content = {"is_direct": direct, "preset": preset}
@@ -990,6 +1034,9 @@ def create_room(
 
             content["initial_state"] += initial_state
 
+        if power_levels_content_override:
+            content["power_levels_content_override"] = power_levels_content_override
+
         request, channel = self.make_request(
             "POST", "/_matrix/client/r0/createRoom", content, access_token=self.tok,
         )