Don't set new room alias before potential 403 (#10930)

Fixes: #10929 

Signed-off-by: Andrew Ferrazzutti <fair@miscworks.net>
pull/11183/head
AndrewFerr 2021-10-25 10:24:49 -04:00 committed by GitHub
parent da957a60e8
commit 4387b791e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 12 deletions

1
changelog.d/10930.bugfix Normal file
View File

@ -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.

View File

@ -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")

View File

@ -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

View File

@ -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"