Fix checking whether a room can be published on creation. (#11392)

If `room_list_publication_rules` was configured with a rule with a
non-wildcard alias and a room was created with an alias then an
internal server error would have been thrown.

This fixes the error and properly applies the publication rules
during room creation.

2 files changed, 32 insertions, 23 deletions

diff --git a/synapse/config/room_directory.py b/synapse/config/room_directory.py
index 56981cac79..57316c59b6 100644
--- a/synapse/config/room_directory.py
+++ b/synapse/config/room_directory.py
@@ -1,4 +1,5 @@
 # Copyright 2018 New Vector Ltd
+# Copyright 2021 Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -12,6 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from typing import List
+
+from synapse.types import JsonDict
 from synapse.util import glob_to_regex
 
 from ._base import Config, ConfigError
@@ -20,7 +24,7 @@ from ._base import Config, ConfigError
 class RoomDirectoryConfig(Config):
     section = "roomdirectory"
 
-    def read_config(self, config, **kwargs):
+    def read_config(self, config, **kwargs) -> None:
         self.enable_room_list_search = config.get("enable_room_list_search", True)
 
         alias_creation_rules = config.get("alias_creation_rules")
@@ -47,7 +51,7 @@ class RoomDirectoryConfig(Config):
                 _RoomDirectoryRule("room_list_publication_rules", {"action": "allow"})
             ]
 
-    def generate_config_section(self, config_dir_path, server_name, **kwargs):
+    def generate_config_section(self, config_dir_path, server_name, **kwargs) -> str:
         return """
         # Uncomment to disable searching the public room list. When disabled
         # blocks searching local and remote room lists for local and remote
@@ -113,16 +117,16 @@ class RoomDirectoryConfig(Config):
         #    action: allow
         """
 
-    def is_alias_creation_allowed(self, user_id, room_id, alias):
+    def is_alias_creation_allowed(self, user_id: str, room_id: str, alias: str) -> bool:
         """Checks if the given user is allowed to create the given alias
 
         Args:
-            user_id (str)
-            room_id (str)
-            alias (str)
+            user_id: The user to check.
+            room_id: The room ID for the alias.
+            alias: The alias being created.
 
         Returns:
-            boolean: True if user is allowed to create the alias
+            True if user is allowed to create the alias
         """
         for rule in self._alias_creation_rules:
             if rule.matches(user_id, room_id, [alias]):
@@ -130,16 +134,18 @@ class RoomDirectoryConfig(Config):
 
         return False
 
-    def is_publishing_room_allowed(self, user_id, room_id, aliases):
+    def is_publishing_room_allowed(
+        self, user_id: str, room_id: str, aliases: List[str]
+    ) -> bool:
         """Checks if the given user is allowed to publish the room
 
         Args:
-            user_id (str)
-            room_id (str)
-            aliases (list[str]): any local aliases associated with the room
+            user_id: The user ID publishing the room.
+            room_id: The room being published.
+            aliases: any local aliases associated with the room
 
         Returns:
-            boolean: True if user can publish room
+            True if user can publish room
         """
         for rule in self._room_list_publication_rules:
             if rule.matches(user_id, room_id, aliases):
@@ -153,11 +159,11 @@ class _RoomDirectoryRule:
     creating an alias or publishing a room.
     """
 
-    def __init__(self, option_name, rule):
+    def __init__(self, option_name: str, rule: JsonDict):
         """
         Args:
-            option_name (str): Name of the config option this rule belongs to
-            rule (dict): The rule as specified in the config
+            option_name: Name of the config option this rule belongs to
+            rule: The rule as specified in the config
         """
 
         action = rule["action"]
@@ -181,18 +187,18 @@ class _RoomDirectoryRule:
         except Exception as e:
             raise ConfigError("Failed to parse glob into regex") from e
 
-    def matches(self, user_id, room_id, aliases):
+    def matches(self, user_id: str, room_id: str, aliases: List[str]) -> bool:
         """Tests if this rule matches the given user_id, room_id and aliases.
 
         Args:
-            user_id (str)
-            room_id (str)
-            aliases (list[str]): The associated aliases to the room. Will be a
-                single element for testing alias creation, and can be empty for
-                testing room publishing.
+            user_id: The user ID to check.
+            room_id: The room ID to check.
+            aliases: The associated aliases to the room. Will be a single element
+                for testing alias creation, and can be empty for testing room
+                publishing.
 
         Returns:
-            boolean
+            True if the rule matches.
         """
 
         # Note: The regexes are anchored at both ends
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index f9a099c4f3..88053f9869 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -775,8 +775,11 @@ class RoomCreationHandler:
             raise SynapseError(403, "Room visibility value not allowed.")
 
         if is_public:
+            room_aliases = []
+            if room_alias:
+                room_aliases.append(room_alias.to_string())
             if not self.config.roomdirectory.is_publishing_room_allowed(
-                user_id, room_id, room_alias
+                user_id, room_id, room_aliases
             ):
                 # Let's just return a generic message, as there may be all sorts of
                 # reasons why we said no. TODO: Allow configurable error messages