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
+<https://github.com/matrix-org/synapse/blob/develop/docs/reverse_proxy.md>`_ 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
+<https://github.com/matrix-org/synapse/blob/develop/docs/workers.md>`_ 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 <https://github.com/matrix-org/synapse/blob/develop/synapse/res/templates/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 @@ class ThirdPartyEventRules(object):
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 @@ class ThirdPartyEventRules(object):
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 @@ class ThirdPartyEventRules(object):
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 @@ class ThirdPartyEventRules(object):
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 @@ class DirectoryHandler(BaseHandler):
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 @@ class DirectoryHandler(BaseHandler):
# 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 @@ class RoomCreationHandler(BaseHandler):
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 @@ __all__ = ["errors", "make_deferred_yieldable", "run_in_background", "ModuleApi"
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 @@ class ModuleApi(object):
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.constants import EventTypes, JoinRules, Membership, RoomCreatio
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
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 @@ class RoomAccessRules(object):
)
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 @@ class ModuleApiTestCase(HomeserverTestCase):
# 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 mock import Mock
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
)
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
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 @@ class ThirdPartyRulesTestCase(unittest.HomeserverTestCase):
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)
|
