diff --git a/UPGRADE.rst b/UPGRADE.rst index 6492fa011f..b2069a0d26 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -75,6 +75,70 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb +Upgrading to v1.21.0 +==================== + +Forwarding ``/_synapse/client`` through your reverse proxy +---------------------------------------------------------- + +The `reverse proxy documentation +`_ has been updated +to include reverse proxy directives for ``/_synapse/client/*`` endpoints. As the user password +reset flow now uses endpoints under this prefix, **you must update your reverse proxy +configurations for user password reset to work**. + +Additionally, note that the `Synapse worker documentation +`_ has been updated to + state that the ``/_synapse/client/password_reset/email/submit_token`` endpoint can be handled +by all workers. If you make use of Synapse's worker feature, please update your reverse proxy +configuration to reflect this change. + +New HTML templates +------------------ + +A new HTML template, +`password_reset_confirmation.html `_, +has been added to the ``synapse/res/templates`` directory. If you are using a +custom template directory, you may want to copy the template over and modify it. + +Note that as of v1.20.0, templates do not need to be included in custom template +directories for Synapse to start. The default templates will be used if a custom +template cannot be found. + +This page will appear to the user after clicking a password reset link that has +been emailed to them. + +To complete password reset, the page must include a way to make a `POST` +request to +``/_synapse/client/password_reset/{medium}/submit_token`` +with the query parameters from the original link, presented as a URL-encoded form. See the file +itself for more details. + +Updated Single Sign-on HTML Templates +------------------------------------- + +The ``saml_error.html`` template was removed from Synapse and replaced with the +``sso_error.html`` template. If your Synapse is configured to use SAML and a +custom ``sso_redirect_confirm_template_dir`` configuration then any customisations +of the ``saml_error.html`` template will need to be merged into the ``sso_error.html`` +template. These templates are similar, but the parameters are slightly different: + +* The ``msg`` parameter should be renamed to ``error_description``. +* There is no longer a ``code`` parameter for the response code. +* A string ``error`` parameter is available that includes a short hint of why a + user is seeing the error page. + +ThirdPartyEventRules breaking changes +------------------------------------- + +This release introduces a backwards-incompatible change to modules making use of +`ThirdPartyEventRules` in Synapse. + +The `http_client` argument is no longer passed to modules as they are initialised. Instead, +modules are expected to make use of the `http_client` property on the `ModuleApi` class. +Modules are now passed a `module_api` argument during initialisation, which is an instance of +`ModuleApi`. + Upgrading to v1.18.0 ==================== diff --git a/changelog.d/63.feature b/changelog.d/63.feature new file mode 100644 index 0000000000..b45f38fa94 --- /dev/null +++ b/changelog.d/63.feature @@ -0,0 +1 @@ +Make AccessRules use the public rooms directory instead of checking a room's join rules on rule change. diff --git a/synapse/events/third_party_rules.py b/synapse/events/third_party_rules.py index 459132d388..deb0d865b5 100644 --- a/synapse/events/third_party_rules.py +++ b/synapse/events/third_party_rules.py @@ -12,8 +12,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import Callable -from twisted.internet import defer +from synapse.events import EventBase +from synapse.module_api import ModuleApi +from synapse.types import StateMap class ThirdPartyEventRules(object): @@ -36,11 +39,10 @@ def __init__(self, hs): if module is not None: self.third_party_rules = module( - config=config, http_client=hs.get_simple_http_client() + config=config, module_api=ModuleApi(hs, hs.get_auth_handler()), ) - @defer.inlineCallbacks - def check_event_allowed(self, event, context): + async def check_event_allowed(self, event, context): """Check if a provided event should be allowed in the given context. Args: @@ -53,18 +55,17 @@ def check_event_allowed(self, event, context): if self.third_party_rules is None: return True - prev_state_ids = yield context.get_prev_state_ids() + prev_state_ids = await context.get_prev_state_ids() # Retrieve the state events from the database. state_events = {} for key, event_id in prev_state_ids.items(): - state_events[key] = yield self.store.get_event(event_id, allow_none=True) + state_events[key] = await self.store.get_event(event_id, allow_none=True) - ret = yield self.third_party_rules.check_event_allowed(event, state_events) + ret = await self.third_party_rules.check_event_allowed(event, state_events) return ret - @defer.inlineCallbacks - def on_create_room(self, requester, config, is_requester_admin): + async def on_create_room(self, requester, config, is_requester_admin): """Intercept requests to create room to allow, deny or update the request config. @@ -80,13 +81,12 @@ def on_create_room(self, requester, config, is_requester_admin): if self.third_party_rules is None: return True - ret = yield self.third_party_rules.on_create_room( + ret = await self.third_party_rules.on_create_room( requester, config, is_requester_admin ) return ret - @defer.inlineCallbacks - def check_threepid_can_be_invited(self, medium, address, room_id): + async def check_threepid_can_be_invited(self, medium, address, room_id): """Check if a provided 3PID can be invited in the given room. Args: @@ -101,14 +101,51 @@ def check_threepid_can_be_invited(self, medium, address, room_id): if self.third_party_rules is None: return True - state_ids = yield self.store.get_filtered_current_state_ids(room_id) - room_state_events = yield self.store.get_events(state_ids.values()) + state_events = await self._get_state_map_for_room(room_id) + + ret = await self.third_party_rules.check_threepid_can_be_invited( + medium, address, state_events + ) + return ret + + async def check_visibility_can_be_modified( + self, room_id: str, new_visibility: str + ) -> bool: + """Check if a room is allowed to be published to, or removed from, the public room + list. + + Args: + room_id: The ID of the room. + new_visibility: The new visibility state. Either "public" or "private". + + Returns: + True if the room's visibility can be modified, False if not. + """ + if self.third_party_rules is None: + return True + + check_func = getattr(self.third_party_rules, "check_visibility_can_be_modified") + if not check_func or not isinstance(check_func, Callable): + return True + + state_events = await self._get_state_map_for_room(room_id) + + return await check_func(room_id, state_events, new_visibility) + + async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]: + """Given a room ID, return the state events of that room. + + Args: + room_id: The ID of the room. + + Returns: + A dict mapping (event type, state key) to state event. + """ + state_ids = await self.store.get_filtered_current_state_ids(room_id) + room_state_events = await self.store.get_events(state_ids.values()) state_events = {} for key, event_id in state_ids.items(): state_events[key] = room_state_events[event_id] - ret = yield self.third_party_rules.check_threepid_can_be_invited( - medium, address, state_events - ) - return ret + return state_events diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 79a2df6201..af9936f7e2 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -45,6 +45,7 @@ def __init__(self, hs): self.config = hs.config self.enable_room_list_search = hs.config.enable_room_list_search self.require_membership = hs.config.require_membership_for_aliases + self.third_party_event_rules = hs.get_third_party_event_rules() self.federation = hs.get_federation_client() hs.get_federation_registry().register_query_handler( @@ -448,6 +449,15 @@ async def edit_published_room_list( # per alias creation rule? raise SynapseError(403, "Not allowed to publish room") + # Check if publishing is blocked by a third party module + allowed_by_third_party_rules = await ( + self.third_party_event_rules.check_visibility_can_be_modified( + room_id, visibility + ) + ) + if not allowed_by_third_party_rules: + raise SynapseError(403, "Not allowed to publish room") + await self.store.set_room_is_public(room_id, making_public) async def edit_published_appservice_room_list( diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 6350cae0f2..4ab229963a 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -660,6 +660,15 @@ async def create_room( creator_id=user_id, is_public=is_public, room_version=room_version, ) + # Check whether this visibility value is blocked by a third party module + allowed_by_third_party_rules = await ( + self.third_party_event_rules.check_visibility_can_be_modified( + room_id, visibility + ) + ) + if not allowed_by_third_party_rules: + raise SynapseError(403, "Room visibility value not allowed.") + directory_handler = self.hs.get_handlers().directory_handler if room_alias: await directory_handler.create_association( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index a7849cefa5..0861e0cfff 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -14,13 +14,18 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import TYPE_CHECKING from twisted.internet import defer +from synapse.http.client import SimpleHttpClient from synapse.http.site import SynapseRequest from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.types import UserID +if TYPE_CHECKING: + from synapse.server import HomeServer + """ This package defines the 'stable' API which can be used by extension modules which are loaded into Synapse. @@ -31,6 +36,50 @@ logger = logging.getLogger(__name__) +class PublicRoomListManager: + """Contains methods for adding to, removing from and querying whether a room + is in the public room list. + + Args: + hs: The Homeserver object + """ + + def __init__(self, hs: "HomeServer"): + self._store = hs.get_datastore() + + async def room_is_in_public_room_list(self, room_id: str) -> bool: + """Checks whether a room is in the public room list. + + Args: + room_id: The ID of the room. + + Returns: + Whether the room is in the public room list. Returns False if the room does + not exist. + """ + room = await self._store.get_room(room_id) + if not room: + return False + + return room.get("is_public", False) + + async def add_room_to_public_room_list(self, room_id: str) -> None: + """Publishes a room to the public room list. + + Args: + room_id: The ID of the room. + """ + await self._store.set_room_is_public(room_id, True) + + async def remove_room_from_public_room_list(self, room_id: str) -> None: + """Removes a room from the public room list. + + Args: + room_id: The ID of the room. + """ + await self._store.set_room_is_public(room_id, False) + + class ModuleApi(object): """A proxy object that gets passed to various plugin modules so they can register new users etc if necessary. @@ -43,6 +92,9 @@ def __init__(self, hs, auth_handler): self._auth = hs.get_auth() self._auth_handler = auth_handler + self.http_client = hs.get_simple_http_client() # type: SimpleHttpClient + self.public_room_list_manager = PublicRoomListManager(hs) + def get_user_by_req(self, req, allow_guest=False): """Check the access_token provided for a request diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index bf3b9adb56..a8b21048ec 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -21,7 +21,7 @@ from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.events import EventBase -from synapse.http.client import SimpleHttpClient +from synapse.module_api import ModuleApi from synapse.types import Requester, StateMap, get_domain_from_id ACCESS_RULES_TYPE = "im.vector.room.access_rules" @@ -74,10 +74,11 @@ class RoomAccessRules(object): Don't forget to consider if you can invite users from your own domain. """ - def __init__(self, config: Dict, http_client: SimpleHttpClient): - self.http_client = http_client - + def __init__( + self, config: Dict, module_api: ModuleApi, + ): self.id_server = config["id_server"] + self.module_api = module_api self.domains_forbidden_when_restricted = config.get( "domains_forbidden_when_restricted", [] @@ -101,7 +102,7 @@ def parse_config(config: Dict) -> Dict: return config - def on_create_room( + async def on_create_room( self, requester: Requester, config: Dict, is_requester_admin: bool, ) -> bool: """Implements synapse.events.ThirdPartyEventRules.on_create_room. @@ -176,12 +177,13 @@ def on_create_room( access_rule = default_rule - # Check that the preset or the join rule in use is compatible with the access - # rule, whether it's a user-defined one or the default one (i.e. if it involves - # a "public" join rule, the access rule must be "restricted"). + # Check that the preset in use is compatible with the access rule, whether it's + # user-defined or the default. + # + # Direct rooms may not have their join_rules set to JoinRules.PUBLIC. if ( join_rule == JoinRules.PUBLIC or preset == RoomCreationPreset.PUBLIC_CHAT - ) and access_rule != AccessRules.RESTRICTED: + ) and access_rule == AccessRules.DIRECT: raise SynapseError(400, "Invalid access rule") # Check if the creator can override values for the power levels. @@ -280,7 +282,7 @@ def check_threepid_can_be_invited( return False # Get the HS this address belongs to from the identity server. - res = yield self.http_client.get_json( + res = yield self.module_api.http_client.get_json( "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), {"medium": medium, "address": address}, ) @@ -293,7 +295,7 @@ def check_threepid_can_be_invited( return True - def check_event_allowed( + async def check_event_allowed( self, event: EventBase, state_events: StateMap[EventBase], ) -> bool: """Implements synapse.events.ThirdPartyEventRules.check_event_allowed. @@ -310,7 +312,7 @@ def check_event_allowed( True if the event can be allowed, False otherwise. """ if event.type == ACCESS_RULES_TYPE: - return self._on_rules_change(event, state_events) + return await self._on_rules_change(event, state_events) # We need to know the rule to apply when processing the event types below. rule = self._get_rule_from_state(state_events) @@ -337,17 +339,47 @@ def check_event_allowed( return True - def _on_rules_change( - self, event: EventBase, state_events: StateMap[EventBase], + async def check_visibility_can_be_modified( + self, room_id: str, state_events: StateMap[EventBase], new_visibility: str ) -> bool: - """Implement the checks and behaviour specified on allowing or forbidding a new - im.vector.room.access_rules event. + """Implements + synapse.events.ThirdPartyEventRules.check_visibility_can_be_modified + + Determines whether a room can be published, or removed from, the public room + list. A room is published if its visibility is set to "public". Otherwise, + its visibility is "private". A room with access rule other than "restricted" + may not be published. Args: - event: The event to check. + room_id: The ID of the room. state_events: A dict mapping (event type, state key) to state event. - State events in the room before the event was sent. + State events in the room. + new_visibility: The new visibility state. Either "public" or "private". + + Returns: + Whether the room is allowed to be published to, or removed from, the public + rooms directory. + """ + # We need to know the rule to apply when processing the event types below. + rule = self._get_rule_from_state(state_events) + + # Allow adding a room to the public rooms list only if it is restricted + if new_visibility == "public": + return rule == AccessRules.RESTRICTED + + # By default a room is created as "restricted", meaning it is allowed to be + # published to the public rooms directory. + return True + async def _on_rules_change( + self, event: EventBase, state_events: StateMap[EventBase] + ): + """Checks whether an im.vector.room.access_rules event is forbidden or allowed. + + Args: + event: The im.vector.room.access_rules event. + state_events: A dict mapping (event type, state key) to state event. + State events in the room before the event was sent. Returns: True if the event can be allowed, False otherwise. """ @@ -357,12 +389,6 @@ def _on_rules_change( if new_rule not in VALID_ACCESS_RULES: return False - # We must not allow rooms with the "public" join rule to be given any other access - # rule than "restricted". - join_rule = self._get_join_rule_from_state(state_events) - if join_rule == JoinRules.PUBLIC and new_rule != AccessRules.RESTRICTED: - return False - # Make sure we don't apply "direct" if the room has more than two members. if new_rule == AccessRules.DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( @@ -372,6 +398,14 @@ def _on_rules_change( if len(existing_members) > 2 or len(threepid_tokens) > 1: return False + if new_rule != AccessRules.RESTRICTED: + # Block this change if this room is currently listed in the public rooms + # directory + if await self.module_api.public_room_list_manager.room_is_in_public_room_list( + event.room_id + ): + return False + prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) # Now that we know the new rule doesn't break the "direct" case, we can allow any @@ -404,7 +438,7 @@ def _on_membership_or_invite( if rule == AccessRules.RESTRICTED: ret = self._on_membership_or_invite_restricted(event) elif rule == AccessRules.UNRESTRICTED: - ret = self._on_membership_or_invite_unrestricted() + ret = self._on_membership_or_invite_unrestricted(event, state_events) elif rule == AccessRules.DIRECT: ret = self._on_membership_or_invite_direct(event, state_events) else: @@ -442,14 +476,26 @@ def _on_membership_or_invite_restricted(self, event: EventBase) -> bool: invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted - def _on_membership_or_invite_unrestricted(self) -> bool: + def _on_membership_or_invite_unrestricted( + self, event: EventBase, state_events: StateMap[EventBase] + ) -> bool: """Implements the checks and behaviour specified for the "unrestricted" rule. - "unrestricted" currently means that every event is allowed. + "unrestricted" currently means that forbidden users cannot join without an invite. Returns: True if the event can be allowed, False otherwise. """ + # If this is a join from a forbidden user and they don't have an invite to the + # room, then deny it + if event.type == EventTypes.Member and event.membership == Membership.JOIN: + # Check if this user is from a forbidden server + target_domain = get_domain_from_id(event.state_key) + if target_domain in self.domains_forbidden_when_restricted: + # If so, they'll need an invite to join this room. Check if one exists + if not self._user_is_invited_to_room(event.state_key, state_events): + return False + return True def _on_membership_or_invite_direct( @@ -569,21 +615,10 @@ def _is_power_level_content_allowed( return True def _on_join_rule_change(self, event: EventBase, rule: str) -> bool: - """Check whether a join rule change is allowed. A join rule change is always - allowed unless the new join rule is "public" and the current access rule isn't - "restricted". - - The rationale is that external users (those whose server would be denied access - to rooms enforcing the "restricted" access rule) should always rely on non- - external users for access to rooms, therefore they shouldn't be able to access - rooms that don't require an invite to be joined. - - Note that we currently rely on the default access rule being "restricted": during - room creation, the m.room.join_rules event will be sent *before* the - im.vector.room.access_rules one, so the access rule that will be considered here - in this case will be the default "restricted" one. This is fine since the - "restricted" access rule allows any value for the join rule, but we should keep - that in mind if we need to change the default access rule in the future. + """Check whether a join rule change is allowed. + + A join rule change is always allowed unless the new join rule is "public" and + the current access rule is "direct". Args: event: The event to check. @@ -593,7 +628,7 @@ def _on_join_rule_change(self, event: EventBase, rule: str) -> bool: Whether the change is allowed. """ if event.content.get("join_rule") == JoinRules.PUBLIC: - return rule == AccessRules.RESTRICTED + return rule != AccessRules.DIRECT return True @@ -717,3 +752,28 @@ def _is_invite_from_threepid(invite: EventBase, threepid_invite_token: str) -> b ) return token == threepid_invite_token + + def _user_is_invited_to_room( + self, user_id: str, state_events: StateMap[EventBase] + ) -> bool: + """Checks whether a given user has been invited to a room + + A user has an invite for a room if its state contains a `m.room.member` + event with membership "invite" and their user ID as the state key. + + Args: + user_id: The user to check. + state_events: The state events from the room. + + Returns: + True if the user has been invited to the room, or False if they haven't. + """ + for (event_type, state_key), state_event in state_events.items(): + if ( + event_type == EventTypes.Member + and state_key == user_id + and state_event.membership == Membership.INVITE + ): + return True + + return False diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 807cd65dd6..2d10931b33 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -12,13 +12,20 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - from synapse.module_api import ModuleApi +from synapse.rest import admin +from synapse.rest.client.v1 import login, room from tests.unittest import HomeserverTestCase class ModuleApiTestCase(HomeserverTestCase): + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + def prepare(self, reactor, clock, homeserver): self.store = homeserver.get_datastore() self.module_api = ModuleApi(homeserver, homeserver.get_auth_handler()) @@ -52,3 +59,50 @@ def test_can_register_user(self): # Check that the displayname was assigned displayname = self.get_success(self.store.get_profile_displayname("bob")) self.assertEqual(displayname, "Bobberino") + + def test_public_rooms(self): + """Tests that a room can be added and removed from the public rooms list, + as well as have its public rooms directory state queried. + """ + # Create a user and room to play with + user_id = self.register_user("kermit", "monkey") + tok = self.login("kermit", "monkey") + room_id = self.helper.create_room_as(user_id, tok=tok) + + # The room should not currently be in the public rooms directory + is_in_public_rooms = self.get_success( + self.module_api.public_room_list_manager.room_is_in_public_room_list( + room_id + ) + ) + self.assertFalse(is_in_public_rooms) + + # Let's try adding it to the public rooms directory + self.get_success( + self.module_api.public_room_list_manager.add_room_to_public_room_list( + room_id + ) + ) + + # And checking whether it's in there... + is_in_public_rooms = self.get_success( + self.module_api.public_room_list_manager.room_is_in_public_room_list( + room_id + ) + ) + self.assertTrue(is_in_public_rooms) + + # Let's remove it again + self.get_success( + self.module_api.public_room_list_manager.remove_room_from_public_room_list( + room_id + ) + ) + + # Should be gone + is_in_public_rooms = self.get_success( + self.module_api.public_room_list_manager.room_is_in_public_room_list( + room_id + ) + ) + self.assertFalse(is_in_public_rooms) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index fbb482f05b..ae59f8f911 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -20,14 +20,15 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset +from synapse.api.constants import EventTypes, JoinRules, Membership, RoomCreationPreset from synapse.rest import admin -from synapse.rest.client.v1 import login, room +from synapse.rest.client.v1 import directory, login, room from synapse.third_party_rules.access_rules import ( ACCESS_RULES_TYPE, AccessRules, RoomAccessRules, ) +from synapse.types import create_requester from tests import unittest @@ -38,6 +39,7 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): admin.register_servlets, login.register_servlets, room.register_servlets, + directory.register_servlets, ] def make_homeserver(self, reactor, clock): @@ -99,6 +101,8 @@ def post_json_get_json(uri, post_json, args={}, headers=None): mock_http_client ) + self.third_party_event_rules = self.hs.get_third_party_event_rules() + return self.hs def prepare(self, reactor, clock, homeserver): @@ -200,8 +204,8 @@ def test_existing_room_can_change_power_levels(self): ) def test_public_room(self): - """Tests that it's not possible to have a room with the public join rule and an - access rule that's not restricted. + """Tests that it's only possible to have a room listed in the public room list + if the access rule is restricted. """ # Creating a room with the public_chat preset should succeed and set the access # rule to restricted. @@ -224,6 +228,26 @@ def test_public_room(self): self.current_rule_in_room(init_state_room_id), AccessRules.RESTRICTED ) + # List preset_room_id in the public room list + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/directory/list/room/%s" % (preset_room_id,), + {"visibility": "public"}, + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + + # List init_state_room_id in the public room list + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/directory/list/room/%s" % (init_state_room_id,), + {"visibility": "public"}, + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + # Changing access rule to unrestricted should fail. self.change_rule_in_room( preset_room_id, AccessRules.UNRESTRICTED, expected_code=403 @@ -238,54 +262,25 @@ def test_public_room(self): init_state_room_id, AccessRules.DIRECT, expected_code=403 ) - # Changing join rule to public in an unrestricted room should fail. - self.change_join_rule_in_room( - self.unrestricted_room, JoinRules.PUBLIC, expected_code=403 - ) - # Changing join rule to public in an direct room should fail. - self.change_join_rule_in_room( - self.direct_rooms[0], JoinRules.PUBLIC, expected_code=403 - ) - - # Creating a new room with the public_chat preset and an access rule that isn't - # restricted should fail. - self.create_room( - preset=RoomCreationPreset.PUBLIC_CHAT, - rule=AccessRules.UNRESTRICTED, - expected_code=400, - ) + # Creating a new room with the public_chat preset and an access rule of direct + # should fail. self.create_room( preset=RoomCreationPreset.PUBLIC_CHAT, rule=AccessRules.DIRECT, expected_code=400, ) - # Creating a room with the public join rule in its initial state and an access - # rule that isn't restricted should fail. - self.create_room( - initial_state=[ - { - "type": "m.room.join_rules", - "content": {"join_rule": JoinRules.PUBLIC}, - } - ], - rule=AccessRules.UNRESTRICTED, - expected_code=400, - ) - self.create_room( - initial_state=[ - { - "type": "m.room.join_rules", - "content": {"join_rule": JoinRules.PUBLIC}, - } - ], - rule=AccessRules.DIRECT, - expected_code=400, + # Changing join rule to public in an direct room should fail. + self.change_join_rule_in_room( + self.direct_rooms[0], JoinRules.PUBLIC, expected_code=403 ) def test_restricted(self): """Tests that in restricted mode we're unable to invite users from blacklisted servers but can invite other users. + + Also tests that the room can be published to, and removed from, the public room + list. """ # We can't invite a user from a forbidden HS. self.helper.invite( @@ -320,14 +315,33 @@ def test_restricted(self): expected_code=200, ) + # We are allowed to publish the room to the public room list + url = "/_matrix/client/r0/directory/list/room/%s" % self.restricted_room + data = {"visibility": "public"} + + request, channel = self.make_request("PUT", url, data, access_token=self.tok) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + + # We are allowed to remove the room from the public room list + url = "/_matrix/client/r0/directory/list/room/%s" % self.restricted_room + data = {"visibility": "private"} + + request, channel = self.make_request("PUT", url, data, access_token=self.tok) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + def test_direct(self): """Tests that, in direct mode, other users than the initial two can't be invited, but the following scenario works: * invited user joins the room * invited user leaves the room * room creator re-invites invited user - Also tests that a user from a HS that's in the list of forbidden domains (to use + + Tests that a user from a HS that's in the list of forbidden domains (to use in restricted mode) can be invited. + + Tests that the room cannot be published to the public room list. """ not_invited_user = "@not_invited:forbidden_domain" @@ -412,10 +426,20 @@ def test_direct(self): self.hs.config.rc_third_party_invite.burst_count = burst self.hs.config.rc_third_party_invite.per_second = per_second + # We can't publish the room to the public room list + url = "/_matrix/client/r0/directory/list/room/%s" % self.direct_rooms[0] + data = {"visibility": "public"} + + request, channel = self.make_request("PUT", url, data, access_token=self.tok) + self.render(request) + self.assertEqual(channel.code, 403, channel.result) + def test_unrestricted(self): """Tests that, in unrestricted mode, we can invite whoever we want, but we can only change the power level of users that wouldn't be forbidden in restricted mode. + + Tests that the room cannot be published to the public room list. """ # We can invite self.helper.invite( @@ -482,6 +506,14 @@ def test_unrestricted(self): expect_code=403, ) + # We can't publish the room to the public room list + url = "/_matrix/client/r0/directory/list/room/%s" % self.unrestricted_room + data = {"visibility": "public"} + + request, channel = self.make_request("PUT", url, data, access_token=self.tok) + self.render(request) + self.assertEqual(channel.code, 403, channel.result) + def test_change_rules(self): """Tests that we can only change the current rule from restricted to unrestricted. @@ -526,6 +558,30 @@ def test_change_rules(self): expected_code=403, ) + # We can't publish a room to the public room list and then change its rule to + # unrestricted + + # Create a restricted room + test_room_id = self.create_room(rule=AccessRules.RESTRICTED) + + # Publish the room to the public room list + url = "/_matrix/client/r0/directory/list/room/%s" % test_room_id + data = {"visibility": "public"} + + request, channel = self.make_request("PUT", url, data, access_token=self.tok) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + + # Attempt to switch the room to "unrestricted" + self.change_rule_in_room( + room_id=test_room_id, new_rule=AccessRules.UNRESTRICTED, expected_code=403 + ) + + # Attempt to switch the room to "direct" + self.change_rule_in_room( + room_id=test_room_id, new_rule=AccessRules.DIRECT, expected_code=403 + ) + def test_change_room_avatar(self): """Tests that changing the room avatar is always allowed unless the room is a direct chat, in which case it's forbidden. @@ -670,6 +726,119 @@ def test_revoke_3pid_invite_direct(self): tok=self.tok, ) + def test_check_event_allowed(self): + """Tests that RoomAccessRules.check_event_allowed behaves accordingly. + + It tests that: + * forbidden users cannot join restricted rooms. + * forbidden users can only join unrestricted rooms if they have an invite. + """ + event_creator = self.hs.get_event_creation_handler() + + # Test that forbidden users cannot join restricted rooms + requester = create_requester(self.user_id) + allowed_requester = create_requester("@user:allowed_domain") + forbidden_requester = create_requester("@user:forbidden_domain") + + # Create a join event for a forbidden user + forbidden_join_event, forbidden_join_event_context = self.get_success( + event_creator.create_event( + forbidden_requester, + { + "type": EventTypes.Member, + "room_id": self.restricted_room, + "sender": forbidden_requester.user.to_string(), + "content": {"membership": Membership.JOIN}, + "state_key": forbidden_requester.user.to_string(), + }, + ) + ) + + # Create a join event for an allowed user + allowed_join_event, allowed_join_event_context = self.get_success( + event_creator.create_event( + allowed_requester, + { + "type": EventTypes.Member, + "room_id": self.restricted_room, + "sender": allowed_requester.user.to_string(), + "content": {"membership": Membership.JOIN}, + "state_key": allowed_requester.user.to_string(), + }, + ) + ) + + # Assert a join event from a forbidden user to a restricted room is rejected + can_join = self.get_success( + self.third_party_event_rules.check_event_allowed( + forbidden_join_event, forbidden_join_event_context + ) + ) + self.assertFalse(can_join) + + # But a join event from an non-forbidden user to a restricted room is allowed + can_join = self.get_success( + self.third_party_event_rules.check_event_allowed( + allowed_join_event, allowed_join_event_context + ) + ) + self.assertTrue(can_join) + + # Test that forbidden users can only join unrestricted rooms if they have an invite + + # Recreate the forbidden join event for the unrestricted room instead + forbidden_join_event, forbidden_join_event_context = self.get_success( + event_creator.create_event( + forbidden_requester, + { + "type": EventTypes.Member, + "room_id": self.unrestricted_room, + "sender": forbidden_requester.user.to_string(), + "content": {"membership": Membership.JOIN}, + "state_key": forbidden_requester.user.to_string(), + }, + ) + ) + + # A forbidden user without an invite should not be able to join an unrestricted room + can_join = self.get_success( + self.third_party_event_rules.check_event_allowed( + forbidden_join_event, forbidden_join_event_context + ) + ) + self.assertFalse(can_join) + + # However, if we then invite this user... + self.helper.invite( + room=self.unrestricted_room, + src=requester.user.to_string(), + targ=forbidden_requester.user.to_string(), + tok=self.tok, + ) + + # And create another join event, making sure that its context states it's coming + # in after the above invite was made... + forbidden_join_event, forbidden_join_event_context = self.get_success( + event_creator.create_event( + forbidden_requester, + { + "type": EventTypes.Member, + "room_id": self.unrestricted_room, + "sender": forbidden_requester.user.to_string(), + "content": {"membership": Membership.JOIN}, + "state_key": forbidden_requester.user.to_string(), + }, + ) + ) + + # Then the forbidden user should be able to join! + can_join = self.get_success( + self.third_party_event_rules.check_event_allowed( + forbidden_join_event, forbidden_join_event_context + ) + ) + self.assertTrue(can_join) + def create_room( self, direct=False, diff --git a/tests/rest/client/third_party_rules.py b/tests/rest/client/third_party_rules.py index 7167fc56b6..9b79e9de6e 100644 --- a/tests/rest/client/third_party_rules.py +++ b/tests/rest/client/third_party_rules.py @@ -12,18 +12,23 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - from synapse.rest import admin from synapse.rest.client.v1 import login, room +from synapse.types import Requester from tests import unittest class ThirdPartyRulesTestModule(object): - def __init__(self, config): + def __init__(self, config, *args, **kwargs): pass - def check_event_allowed(self, event, context): + async def on_create_room( + self, requester: Requester, config: dict, is_requester_admin: bool + ): + return True + + async def check_event_allowed(self, event, context): if event.type == "foo.bar.forbidden": return False else: @@ -51,29 +56,31 @@ def make_homeserver(self, reactor, clock): self.hs = self.setup_test_homeserver(config=config) return self.hs + def prepare(self, reactor, clock, homeserver): + # Create a user and room to play with during the tests + self.user_id = self.register_user("kermit", "monkey") + self.tok = self.login("kermit", "monkey") + + self.room_id = self.helper.create_room_as(self.user_id, tok=self.tok) + def test_third_party_rules(self): """Tests that a forbidden event is forbidden from being sent, but an allowed one can be sent. """ - user_id = self.register_user("kermit", "monkey") - tok = self.login("kermit", "monkey") - - room_id = self.helper.create_room_as(user_id, tok=tok) - request, channel = self.make_request( "PUT", - "/_matrix/client/r0/rooms/%s/send/foo.bar.allowed/1" % room_id, + "/_matrix/client/r0/rooms/%s/send/foo.bar.allowed/1" % self.room_id, {}, - access_token=tok, + access_token=self.tok, ) self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) request, channel = self.make_request( "PUT", - "/_matrix/client/r0/rooms/%s/send/foo.bar.forbidden/1" % room_id, + "/_matrix/client/r0/rooms/%s/send/foo.bar.forbidden/1" % self.room_id, {}, - access_token=tok, + access_token=self.tok, ) self.render(request) self.assertEquals(channel.result["code"], b"403", channel.result)