diff --git a/changelog.d/10930.bugfix b/changelog.d/10930.bugfix
new file mode 100644
index 0000000000..756bfe9107
--- /dev/null
+++ b/changelog.d/10930.bugfix
@@ -0,0 +1 @@
+Newly-created public rooms are now only assigned an alias if the room's creation has not been blocked by permission settings. Contributed by @AndrewFerr.
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 14ed7d9879..8567cb0e00 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -145,7 +145,7 @@ class DirectoryHandler:
if not self.config.roomdirectory.is_alias_creation_allowed(
user_id, room_id, room_alias_str
):
- # Lets just return a generic message, as there may be all sorts of
+ # Let's just return a generic message, as there may be all sorts of
# reasons why we said no. TODO: Allow configurable error messages
# per alias creation rule?
raise SynapseError(403, "Not allowed to create alias")
@@ -461,7 +461,7 @@ class DirectoryHandler:
if not self.config.roomdirectory.is_publishing_room_allowed(
user_id, room_id, room_aliases
):
- # Lets just return a generic message, as there may be all sorts of
+ # Let's just return a generic message, as there may be all sorts of
# reasons why we said no. TODO: Allow configurable error messages
# per alias creation rule?
raise SynapseError(403, "Not allowed to publish room")
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 6f39e9446f..cf01d58ea1 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -773,6 +773,15 @@ class RoomCreationHandler:
if not allowed_by_third_party_rules:
raise SynapseError(403, "Room visibility value not allowed.")
+ if is_public:
+ if not self.config.roomdirectory.is_publishing_room_allowed(
+ user_id, room_id, room_alias
+ ):
+ # Let's just return a generic message, as there may be all sorts of
+ # reasons why we said no. TODO: Allow configurable error messages
+ # per alias creation rule?
+ raise SynapseError(403, "Not allowed to publish room")
+
directory_handler = self.hs.get_directory_handler()
if room_alias:
await directory_handler.create_association(
@@ -783,15 +792,6 @@ class RoomCreationHandler:
check_membership=False,
)
- if is_public:
- if not self.config.roomdirectory.is_publishing_room_allowed(
- user_id, room_id, room_alias
- ):
- # Lets just return a generic message, as there may be all sorts of
- # reasons why we said no. TODO: Allow configurable error messages
- # per alias creation rule?
- raise SynapseError(403, "Not allowed to publish room")
-
preset_config = config.get(
"preset",
RoomCreationPreset.PRIVATE_CHAT
diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py
index 6a2e76ca4a..be008227df 100644
--- a/tests/handlers/test_directory.py
+++ b/tests/handlers/test_directory.py
@@ -15,8 +15,8 @@
from unittest.mock import Mock
-import synapse
import synapse.api.errors
+import synapse.rest.admin
from synapse.api.constants import EventTypes
from synapse.config.room_directory import RoomDirectoryConfig
from synapse.rest.client import directory, login, room
@@ -432,6 +432,106 @@ class TestCreateAliasACL(unittest.HomeserverTestCase):
self.assertEquals(200, channel.code, channel.result)
+class TestCreatePublishedRoomACL(unittest.HomeserverTestCase):
+ data = {"room_alias_name": "unofficial_test"}
+
+ servlets = [
+ synapse.rest.admin.register_servlets_for_client_rest_resource,
+ login.register_servlets,
+ directory.register_servlets,
+ room.register_servlets,
+ ]
+ hijack_auth = False
+
+ def prepare(self, reactor, clock, hs):
+ self.allowed_user_id = self.register_user("allowed", "pass")
+ self.allowed_access_token = self.login("allowed", "pass")
+
+ self.denied_user_id = self.register_user("denied", "pass")
+ self.denied_access_token = self.login("denied", "pass")
+
+ # This time we add custom room list publication rules
+ config = {}
+ config["alias_creation_rules"] = []
+ config["room_list_publication_rules"] = [
+ {"user_id": "*", "alias": "*", "action": "deny"},
+ {"user_id": self.allowed_user_id, "alias": "*", "action": "allow"},
+ ]
+
+ rd_config = RoomDirectoryConfig()
+ rd_config.read_config(config)
+
+ self.hs.config.roomdirectory.is_publishing_room_allowed = (
+ rd_config.is_publishing_room_allowed
+ )
+
+ return hs
+
+ def test_denied_without_publication_permission(self):
+ """
+ Try to create a room, register an alias for it, and publish it,
+ as a user without permission to publish rooms.
+ (This is used as both a standalone test & as a helper function.)
+ """
+ self.helper.create_room_as(
+ self.denied_user_id,
+ tok=self.denied_access_token,
+ extra_content=self.data,
+ is_public=True,
+ expect_code=403,
+ )
+
+ def test_allowed_when_creating_private_room(self):
+ """
+ Try to create a room, register an alias for it, and NOT publish it,
+ as a user without permission to publish rooms.
+ (This is used as both a standalone test & as a helper function.)
+ """
+ self.helper.create_room_as(
+ self.denied_user_id,
+ tok=self.denied_access_token,
+ extra_content=self.data,
+ is_public=False,
+ expect_code=200,
+ )
+
+ def test_allowed_with_publication_permission(self):
+ """
+ Try to create a room, register an alias for it, and publish it,
+ as a user WITH permission to publish rooms.
+ (This is used as both a standalone test & as a helper function.)
+ """
+ self.helper.create_room_as(
+ self.allowed_user_id,
+ tok=self.allowed_access_token,
+ extra_content=self.data,
+ is_public=False,
+ expect_code=200,
+ )
+
+ def test_can_create_as_private_room_after_rejection(self):
+ """
+ After failing to publish a room with an alias as a user without publish permission,
+ retry as the same user, but without publishing the room.
+
+ This should pass, but used to fail because the alias was registered by the first
+ request, even though the room creation was denied.
+ """
+ self.test_denied_without_publication_permission()
+ self.test_allowed_when_creating_private_room()
+
+ def test_can_create_with_permission_after_rejection(self):
+ """
+ After failing to publish a room with an alias as a user without publish permission,
+ retry as someone with permission, using the same alias.
+
+ This also used to fail because of the alias having been registered by the first
+ request, leaving it unavailable for any other user's new rooms.
+ """
+ self.test_denied_without_publication_permission()
+ self.test_allowed_with_publication_permission()
+
+
class TestRoomListSearchDisabled(unittest.HomeserverTestCase):
user_id = "@test:test"
|
