mirror of
https://git.anonymousland.org/anonymousland/synapse-product.git
synced 2024-12-17 08:44:19 -05:00
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.
This commit is contained in:
parent
4d6d38ac2f
commit
7ae559944a
1
changelog.d/11392.bugfix
Normal file
1
changelog.d/11392.bugfix
Normal file
@ -0,0 +1 @@
|
|||||||
|
Fix a bug introduced in v1.13.0 where creating and publishing a room could cause errors if `room_list_publication_rules` is configured.
|
@ -1,4 +1,5 @@
|
|||||||
# Copyright 2018 New Vector Ltd
|
# Copyright 2018 New Vector Ltd
|
||||||
|
# Copyright 2021 Matrix.org Foundation C.I.C.
|
||||||
#
|
#
|
||||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
# you may not use this file except in compliance with 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
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
from typing import List
|
||||||
|
|
||||||
|
from synapse.types import JsonDict
|
||||||
from synapse.util import glob_to_regex
|
from synapse.util import glob_to_regex
|
||||||
|
|
||||||
from ._base import Config, ConfigError
|
from ._base import Config, ConfigError
|
||||||
@ -20,7 +24,7 @@ from ._base import Config, ConfigError
|
|||||||
class RoomDirectoryConfig(Config):
|
class RoomDirectoryConfig(Config):
|
||||||
section = "roomdirectory"
|
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)
|
self.enable_room_list_search = config.get("enable_room_list_search", True)
|
||||||
|
|
||||||
alias_creation_rules = config.get("alias_creation_rules")
|
alias_creation_rules = config.get("alias_creation_rules")
|
||||||
@ -47,7 +51,7 @@ class RoomDirectoryConfig(Config):
|
|||||||
_RoomDirectoryRule("room_list_publication_rules", {"action": "allow"})
|
_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 """
|
return """
|
||||||
# Uncomment to disable searching the public room list. When disabled
|
# Uncomment to disable searching the public room list. When disabled
|
||||||
# blocks searching local and remote room lists for local and remote
|
# blocks searching local and remote room lists for local and remote
|
||||||
@ -113,16 +117,16 @@ class RoomDirectoryConfig(Config):
|
|||||||
# action: allow
|
# 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
|
"""Checks if the given user is allowed to create the given alias
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id (str)
|
user_id: The user to check.
|
||||||
room_id (str)
|
room_id: The room ID for the alias.
|
||||||
alias (str)
|
alias: The alias being created.
|
||||||
|
|
||||||
Returns:
|
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:
|
for rule in self._alias_creation_rules:
|
||||||
if rule.matches(user_id, room_id, [alias]):
|
if rule.matches(user_id, room_id, [alias]):
|
||||||
@ -130,16 +134,18 @@ class RoomDirectoryConfig(Config):
|
|||||||
|
|
||||||
return False
|
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
|
"""Checks if the given user is allowed to publish the room
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id (str)
|
user_id: The user ID publishing the room.
|
||||||
room_id (str)
|
room_id: The room being published.
|
||||||
aliases (list[str]): any local aliases associated with the room
|
aliases: any local aliases associated with the room
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
boolean: True if user can publish room
|
True if user can publish room
|
||||||
"""
|
"""
|
||||||
for rule in self._room_list_publication_rules:
|
for rule in self._room_list_publication_rules:
|
||||||
if rule.matches(user_id, room_id, aliases):
|
if rule.matches(user_id, room_id, aliases):
|
||||||
@ -153,11 +159,11 @@ class _RoomDirectoryRule:
|
|||||||
creating an alias or publishing a room.
|
creating an alias or publishing a room.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, option_name, rule):
|
def __init__(self, option_name: str, rule: JsonDict):
|
||||||
"""
|
"""
|
||||||
Args:
|
Args:
|
||||||
option_name (str): Name of the config option this rule belongs to
|
option_name: Name of the config option this rule belongs to
|
||||||
rule (dict): The rule as specified in the config
|
rule: The rule as specified in the config
|
||||||
"""
|
"""
|
||||||
|
|
||||||
action = rule["action"]
|
action = rule["action"]
|
||||||
@ -181,18 +187,18 @@ class _RoomDirectoryRule:
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise ConfigError("Failed to parse glob into regex") from 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.
|
"""Tests if this rule matches the given user_id, room_id and aliases.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user_id (str)
|
user_id: The user ID to check.
|
||||||
room_id (str)
|
room_id: The room ID to check.
|
||||||
aliases (list[str]): The associated aliases to the room. Will be a
|
aliases: The associated aliases to the room. Will be a single element
|
||||||
single element for testing alias creation, and can be empty for
|
for testing alias creation, and can be empty for testing room
|
||||||
testing room publishing.
|
publishing.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
boolean
|
True if the rule matches.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
# Note: The regexes are anchored at both ends
|
# Note: The regexes are anchored at both ends
|
||||||
|
@ -775,8 +775,11 @@ class RoomCreationHandler:
|
|||||||
raise SynapseError(403, "Room visibility value not allowed.")
|
raise SynapseError(403, "Room visibility value not allowed.")
|
||||||
|
|
||||||
if is_public:
|
if is_public:
|
||||||
|
room_aliases = []
|
||||||
|
if room_alias:
|
||||||
|
room_aliases.append(room_alias.to_string())
|
||||||
if not self.config.roomdirectory.is_publishing_room_allowed(
|
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
|
# Let's just return a generic message, as there may be all sorts of
|
||||||
# reasons why we said no. TODO: Allow configurable error messages
|
# reasons why we said no. TODO: Allow configurable error messages
|
||||||
|
@ -1,4 +1,5 @@
|
|||||||
# Copyright 2014-2016 OpenMarket Ltd
|
# Copyright 2014-2016 OpenMarket Ltd
|
||||||
|
# Copyright 2021 Matrix.org Foundation C.I.C.
|
||||||
#
|
#
|
||||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
# you may not use this file except in compliance with the License.
|
# you may not use this file except in compliance with the License.
|
||||||
@ -12,13 +13,11 @@
|
|||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
|
||||||
from unittest.mock import Mock
|
from unittest.mock import Mock
|
||||||
|
|
||||||
import synapse.api.errors
|
import synapse.api.errors
|
||||||
import synapse.rest.admin
|
import synapse.rest.admin
|
||||||
from synapse.api.constants import EventTypes
|
from synapse.api.constants import EventTypes
|
||||||
from synapse.config.room_directory import RoomDirectoryConfig
|
|
||||||
from synapse.rest.client import directory, login, room
|
from synapse.rest.client import directory, login, room
|
||||||
from synapse.types import RoomAlias, create_requester
|
from synapse.types import RoomAlias, create_requester
|
||||||
|
|
||||||
@ -394,22 +393,15 @@ class TestCreateAliasACL(unittest.HomeserverTestCase):
|
|||||||
|
|
||||||
servlets = [directory.register_servlets, room.register_servlets]
|
servlets = [directory.register_servlets, room.register_servlets]
|
||||||
|
|
||||||
def prepare(self, reactor, clock, hs):
|
def default_config(self):
|
||||||
# We cheekily override the config to add custom alias creation rules
|
config = super().default_config()
|
||||||
config = {}
|
|
||||||
|
# Add custom alias creation rules to the config.
|
||||||
config["alias_creation_rules"] = [
|
config["alias_creation_rules"] = [
|
||||||
{"user_id": "*", "alias": "#unofficial_*", "action": "allow"}
|
{"user_id": "*", "alias": "#unofficial_*", "action": "allow"}
|
||||||
]
|
]
|
||||||
config["room_list_publication_rules"] = []
|
|
||||||
|
|
||||||
rd_config = RoomDirectoryConfig()
|
return config
|
||||||
rd_config.read_config(config)
|
|
||||||
|
|
||||||
self.hs.config.roomdirectory.is_alias_creation_allowed = (
|
|
||||||
rd_config.is_alias_creation_allowed
|
|
||||||
)
|
|
||||||
|
|
||||||
return hs
|
|
||||||
|
|
||||||
def test_denied(self):
|
def test_denied(self):
|
||||||
room_id = self.helper.create_room_as(self.user_id)
|
room_id = self.helper.create_room_as(self.user_id)
|
||||||
@ -417,7 +409,7 @@ class TestCreateAliasACL(unittest.HomeserverTestCase):
|
|||||||
channel = self.make_request(
|
channel = self.make_request(
|
||||||
"PUT",
|
"PUT",
|
||||||
b"directory/room/%23test%3Atest",
|
b"directory/room/%23test%3Atest",
|
||||||
('{"room_id":"%s"}' % (room_id,)).encode("ascii"),
|
{"room_id": room_id},
|
||||||
)
|
)
|
||||||
self.assertEquals(403, channel.code, channel.result)
|
self.assertEquals(403, channel.code, channel.result)
|
||||||
|
|
||||||
@ -427,14 +419,35 @@ class TestCreateAliasACL(unittest.HomeserverTestCase):
|
|||||||
channel = self.make_request(
|
channel = self.make_request(
|
||||||
"PUT",
|
"PUT",
|
||||||
b"directory/room/%23unofficial_test%3Atest",
|
b"directory/room/%23unofficial_test%3Atest",
|
||||||
('{"room_id":"%s"}' % (room_id,)).encode("ascii"),
|
{"room_id": room_id},
|
||||||
)
|
)
|
||||||
self.assertEquals(200, channel.code, channel.result)
|
self.assertEquals(200, channel.code, channel.result)
|
||||||
|
|
||||||
|
def test_denied_during_creation(self):
|
||||||
|
"""A room alias that is not allowed should be rejected during creation."""
|
||||||
|
# Invalid room alias.
|
||||||
|
self.helper.create_room_as(
|
||||||
|
self.user_id,
|
||||||
|
expect_code=403,
|
||||||
|
extra_content={"room_alias_name": "foo"},
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_allowed_during_creation(self):
|
||||||
|
"""A valid room alias should be allowed during creation."""
|
||||||
|
room_id = self.helper.create_room_as(
|
||||||
|
self.user_id,
|
||||||
|
extra_content={"room_alias_name": "unofficial_test"},
|
||||||
|
)
|
||||||
|
|
||||||
|
channel = self.make_request(
|
||||||
|
"GET",
|
||||||
|
b"directory/room/%23unofficial_test%3Atest",
|
||||||
|
)
|
||||||
|
self.assertEquals(200, channel.code, channel.result)
|
||||||
|
self.assertEquals(channel.json_body["room_id"], room_id)
|
||||||
|
|
||||||
|
|
||||||
class TestCreatePublishedRoomACL(unittest.HomeserverTestCase):
|
class TestCreatePublishedRoomACL(unittest.HomeserverTestCase):
|
||||||
data = {"room_alias_name": "unofficial_test"}
|
|
||||||
|
|
||||||
servlets = [
|
servlets = [
|
||||||
synapse.rest.admin.register_servlets_for_client_rest_resource,
|
synapse.rest.admin.register_servlets_for_client_rest_resource,
|
||||||
login.register_servlets,
|
login.register_servlets,
|
||||||
@ -443,28 +456,31 @@ class TestCreatePublishedRoomACL(unittest.HomeserverTestCase):
|
|||||||
]
|
]
|
||||||
hijack_auth = False
|
hijack_auth = False
|
||||||
|
|
||||||
|
data = {"room_alias_name": "unofficial_test"}
|
||||||
|
allowed_localpart = "allowed"
|
||||||
|
|
||||||
|
def default_config(self):
|
||||||
|
config = super().default_config()
|
||||||
|
|
||||||
|
# Add custom room list publication rules to the config.
|
||||||
|
config["room_list_publication_rules"] = [
|
||||||
|
{
|
||||||
|
"user_id": "@" + self.allowed_localpart + "*",
|
||||||
|
"alias": "#unofficial_*",
|
||||||
|
"action": "allow",
|
||||||
|
},
|
||||||
|
{"user_id": "*", "alias": "*", "action": "deny"},
|
||||||
|
]
|
||||||
|
|
||||||
|
return config
|
||||||
|
|
||||||
def prepare(self, reactor, clock, hs):
|
def prepare(self, reactor, clock, hs):
|
||||||
self.allowed_user_id = self.register_user("allowed", "pass")
|
self.allowed_user_id = self.register_user(self.allowed_localpart, "pass")
|
||||||
self.allowed_access_token = self.login("allowed", "pass")
|
self.allowed_access_token = self.login(self.allowed_localpart, "pass")
|
||||||
|
|
||||||
self.denied_user_id = self.register_user("denied", "pass")
|
self.denied_user_id = self.register_user("denied", "pass")
|
||||||
self.denied_access_token = self.login("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
|
return hs
|
||||||
|
|
||||||
def test_denied_without_publication_permission(self):
|
def test_denied_without_publication_permission(self):
|
||||||
@ -505,10 +521,23 @@ class TestCreatePublishedRoomACL(unittest.HomeserverTestCase):
|
|||||||
self.allowed_user_id,
|
self.allowed_user_id,
|
||||||
tok=self.allowed_access_token,
|
tok=self.allowed_access_token,
|
||||||
extra_content=self.data,
|
extra_content=self.data,
|
||||||
is_public=False,
|
is_public=True,
|
||||||
expect_code=200,
|
expect_code=200,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_denied_publication_with_invalid_alias(self):
|
||||||
|
"""
|
||||||
|
Try to create a room, register an alias for it, and publish it,
|
||||||
|
as a user WITH permission to publish rooms.
|
||||||
|
"""
|
||||||
|
self.helper.create_room_as(
|
||||||
|
self.allowed_user_id,
|
||||||
|
tok=self.allowed_access_token,
|
||||||
|
extra_content={"room_alias_name": "foo"},
|
||||||
|
is_public=True,
|
||||||
|
expect_code=403,
|
||||||
|
)
|
||||||
|
|
||||||
def test_can_create_as_private_room_after_rejection(self):
|
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,
|
After failing to publish a room with an alias as a user without publish permission,
|
||||||
|
Loading…
Reference in New Issue
Block a user