Merge remote-tracking branch 'upstream/release-v1.35'

synapse/handlers/account_validity.py

@@ -15,12 +15,9 @@
 import email.mime.multipart
 import email.utils
 import logging
-from email.mime.multipart import MIMEMultipart
-from email.mime.text import MIMEText
 from typing import TYPE_CHECKING, List, Optional, Tuple
 
 from synapse.api.errors import StoreError, SynapseError
-from synapse.logging.context import make_deferred_yieldable
 from synapse.metrics.background_process_metrics import wrap_as_background_process
 from synapse.types import UserID
 from synapse.util import stringutils
@@ -36,9 +33,11 @@ class AccountValidityHandler:
         self.hs = hs
         self.config = hs.config
         self.store = self.hs.get_datastore()
-        self.sendmail = self.hs.get_sendmail()
+        self.send_email_handler = self.hs.get_send_email_handler()
         self.clock = self.hs.get_clock()
 
+        self._app_name = self.hs.config.email_app_name
+
         self._account_validity_enabled = (
             hs.config.account_validity.account_validity_enabled
         )
@@ -63,23 +62,10 @@ class AccountValidityHandler:
             self._template_text = (
                 hs.config.account_validity.account_validity_template_text
             )
-            account_validity_renew_email_subject = (
+            self._renew_email_subject = (
                 hs.config.account_validity.account_validity_renew_email_subject
             )
 
-            try:
-                app_name = hs.config.email_app_name
-
-                self._subject = account_validity_renew_email_subject % {"app": app_name}
-
-                self._from_string = hs.config.email_notif_from % {"app": app_name}
-            except Exception:
-                # If substitution failed, fall back to the bare strings.
-                self._subject = account_validity_renew_email_subject
-                self._from_string = hs.config.email_notif_from
-
-            self._raw_from = email.utils.parseaddr(self._from_string)[1]
-
             # Check the renewal emails to send and send them every 30min.
             if hs.config.run_background_tasks:
                 self.clock.looping_call(self._send_renewal_emails, 30 * 60 * 1000)
@@ -159,38 +145,17 @@ class AccountValidityHandler:
         }
 
         html_text = self._template_html.render(**template_vars)
-        html_part = MIMEText(html_text, "html", "utf8")
-
         plain_text = self._template_text.render(**template_vars)
-        text_part = MIMEText(plain_text, "plain", "utf8")
 
         for address in addresses:
             raw_to = email.utils.parseaddr(address)[1]
 
-            multipart_msg = MIMEMultipart("alternative")
-            multipart_msg["Subject"] = self._subject
-            multipart_msg["From"] = self._from_string
-            multipart_msg["To"] = address
-            multipart_msg["Date"] = email.utils.formatdate()
-            multipart_msg["Message-ID"] = email.utils.make_msgid()
-            multipart_msg.attach(text_part)
-            multipart_msg.attach(html_part)
-
-            logger.info("Sending renewal email to %s", address)
-
-            await make_deferred_yieldable(
-                self.sendmail(
-                    self.hs.config.email_smtp_host,
-                    self._raw_from,
-                    raw_to,
-                    multipart_msg.as_string().encode("utf8"),
-                    reactor=self.hs.get_reactor(),
-                    port=self.hs.config.email_smtp_port,
-                    requireAuthentication=self.hs.config.email_smtp_user is not None,
-                    username=self.hs.config.email_smtp_user,
-                    password=self.hs.config.email_smtp_pass,
-                    requireTransportSecurity=self.hs.config.require_transport_security,
-                )
+            await self.send_email_handler.send_email(
+                email_address=raw_to,
+                subject=self._renew_email_subject,
+                app_name=self._app_name,
+                html=html_text,
+                text=plain_text,
             )
 
         await self.store.set_renewal_mail_status(user_id=user_id, email_sent=True)

synapse/handlers/event_auth.py

@@ -11,10 +11,12 @@
 # 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 TYPE_CHECKING
+from typing import TYPE_CHECKING, Collection, Optional
 
-from synapse.api.constants import EventTypes, JoinRules
+from synapse.api.constants import EventTypes, JoinRules, Membership
+from synapse.api.errors import AuthError
 from synapse.api.room_versions import RoomVersion
+from synapse.events import EventBase
 from synapse.types import StateMap
 
 if TYPE_CHECKING:
@@ -29,46 +31,104 @@ class EventAuthHandler:
     def __init__(self, hs: "HomeServer"):
         self._store = hs.get_datastore()
 
-    async def can_join_without_invite(
-        self, state_ids: StateMap[str], room_version: RoomVersion, user_id: str
-    ) -> bool:
+    async def check_restricted_join_rules(
+        self,
+        state_ids: StateMap[str],
+        room_version: RoomVersion,
+        user_id: str,
+        prev_member_event: Optional[EventBase],
+    ) -> None:
         """
-        Check whether a user can join a room without an invite.
+        Check whether a user can join a room without an invite due to restricted join rules.
 
         When joining a room with restricted joined rules (as defined in MSC3083),
-        the membership of spaces must be checked during join.
+        the membership of spaces must be checked during a room join.
 
         Args:
             state_ids: The state of the room as it currently is.
             room_version: The room version of the room being joined.
             user_id: The user joining the room.
+            prev_member_event: The current membership event for this user.
+
+        Raises:
+            AuthError if the user cannot join the room.
+        """
+        # If the member is invited or currently joined, then nothing to do.
+        if prev_member_event and (
+            prev_member_event.membership in (Membership.JOIN, Membership.INVITE)
+        ):
+            return
+
+        # This is not a room with a restricted join rule, so we don't need to do the
+        # restricted room specific checks.
+        #
+        # Note: We'll be applying the standard join rule checks later, which will
+        # catch the cases of e.g. trying to join private rooms without an invite.
+        if not await self.has_restricted_join_rules(state_ids, room_version):
+            return
+
+        # Get the spaces which allow access to this room and check if the user is
+        # in any of them.
+        allowed_spaces = await self.get_spaces_that_allow_join(state_ids)
+        if not await self.is_user_in_rooms(allowed_spaces, user_id):
+            raise AuthError(
+                403,
+                "You do not belong to any of the required spaces to join this room.",
+            )
+
+    async def has_restricted_join_rules(
+        self, state_ids: StateMap[str], room_version: RoomVersion
+    ) -> bool:
+        """
+        Return if the room has the proper join rules set for access via spaces.
+
+        Args:
+            state_ids: The state of the room as it currently is.
+            room_version: The room version of the room to query.
 
         Returns:
-            True if the user can join the room, false otherwise.
+            True if the proper room version and join rules are set for restricted access.
         """
         # This only applies to room versions which support the new join rule.
         if not room_version.msc3083_join_rules:
-            return True
+            return False
 
         # If there's no join rule, then it defaults to invite (so this doesn't apply).
         join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None)
         if not join_rules_event_id:
-            return True
+            return False
+
+        # If the join rule is not restricted, this doesn't apply.
+        join_rules_event = await self._store.get_event(join_rules_event_id)
+        return join_rules_event.content.get("join_rule") == JoinRules.MSC3083_RESTRICTED
+
+    async def get_spaces_that_allow_join(
+        self, state_ids: StateMap[str]
+    ) -> Collection[str]:
+        """
+        Generate a list of spaces which allow access to a room.
+
+        Args:
+            state_ids: The state of the room as it currently is.
+
+        Returns:
+            A collection of spaces which provide membership to the room.
+        """
+        # If there's no join rule, then it defaults to invite (so this doesn't apply).
+        join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None)
+        if not join_rules_event_id:
+            return ()
 
         # If the join rule is not restricted, this doesn't apply.
         join_rules_event = await self._store.get_event(join_rules_event_id)
-        if join_rules_event.content.get("join_rule") != JoinRules.MSC3083_RESTRICTED:
-            return True
 
         # If allowed is of the wrong form, then only allow invited users.
         allowed_spaces = join_rules_event.content.get("allow", [])
         if not isinstance(allowed_spaces, list):
-            return False
-
-        # Get the list of joined rooms and see if there's an overlap.
-        joined_rooms = await self._store.get_rooms_for_user(user_id)
+            return ()
 
         # Pull out the other room IDs, invalid data gets filtered.
+        result = []
         for space in allowed_spaces:
             if not isinstance(space, dict):
                 continue
@@ -77,10 +137,31 @@ class EventAuthHandler:
             if not isinstance(space_id, str):
                 continue
 
-            # The user was joined to one of the spaces specified, they can join
-            # this room!
-            if space_id in joined_rooms:
+            result.append(space_id)
+
+        return result
+
+    async def is_user_in_rooms(self, room_ids: Collection[str], user_id: str) -> bool:
+        """
+        Check whether a user is a member of any of the provided rooms.
+
+        Args:
+            room_ids: The rooms to check for membership.
+            user_id: The user to check.
+
+        Returns:
+            True if the user is in any of the rooms, false otherwise.
+        """
+        if not room_ids:
+            return False
+
+        # Get the list of joined rooms and see if there's an overlap.
+        joined_rooms = await self._store.get_rooms_for_user(user_id)
+
+        # Check each room and see if the user is in it.
+        for room_id in room_ids:
+            if room_id in joined_rooms:
                 return True
 
-        # The user was not in any of the required spaces.
+        # The user was not in any of the rooms.
         return False

synapse/handlers/federation.py

@@ -1668,28 +1668,17 @@ class FederationHandler(BaseHandler):
         # Check if the user is already in the room or invited to the room.
         user_id = event.state_key
         prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None)
-        newly_joined = True
-        user_is_invited = False
+        prev_member_event = None
         if prev_member_event_id:
             prev_member_event = await self.store.get_event(prev_member_event_id)
-            newly_joined = prev_member_event.membership != Membership.JOIN
-            user_is_invited = prev_member_event.membership == Membership.INVITE
 
-        # If the member is not already in the room, and not invited, check if
-        # they should be allowed access via membership in a space.
-        if (
-            newly_joined
-            and not user_is_invited
-            and not await self._event_auth_handler.can_join_without_invite(
-                prev_state_ids,
-                event.room_version,
-                user_id,
-            )
-        ):
-            raise AuthError(
-                403,
-                "You do not belong to any of the required spaces to join this room.",
-            )
+        # Check if the member should be allowed access via membership in a space.
+        await self._event_auth_handler.check_restricted_join_rules(
+            prev_state_ids,
+            event.room_version,
+            user_id,
+            prev_member_event,
+        )
 
         # Persist the event.
         await self._auth_and_persist_event(origin, event, context)

synapse/handlers/presence.py

@@ -222,9 +222,21 @@ class BasePresenceHandler(abc.ABC):
 
     @abc.abstractmethod
     async def set_state(
-        self, target_user: UserID, state: JsonDict, ignore_status_msg: bool = False
+        self,
+        target_user: UserID,
+        state: JsonDict,
+        ignore_status_msg: bool = False,
+        force_notify: bool = False,
     ) -> None:
-        """Set the presence state of the user. """
+        """Set the presence state of the user.
+
+        Args:
+            target_user: The ID of the user to set the presence state of.
+            state: The presence state as a JSON dictionary.
+            ignore_status_msg: True to ignore the "status_msg" field of the `state` dict.
+                If False, the user's current status will be updated.
+            force_notify: Whether to force notification of the update to clients.
+        """
 
     @abc.abstractmethod
     async def bump_presence_active_time(self, user: UserID):
@@ -296,6 +308,51 @@ class BasePresenceHandler(abc.ABC):
         for destinations, states in hosts_and_states:
             self._federation.send_presence_to_destinations(states, destinations)
 
+    async def send_full_presence_to_users(self, user_ids: Collection[str]):
+        """
+        Adds to the list of users who should receive a full snapshot of presence
+        upon their next sync. Note that this only works for local users.
+
+        Then, grabs the current presence state for a given set of users and adds it
+        to the top of the presence stream.
+
+        Args:
+            user_ids: The IDs of the local users to send full presence to.
+        """
+        # Retrieve one of the users from the given set
+        if not user_ids:
+            raise Exception(
+                "send_full_presence_to_users must be called with at least one user"
+            )
+        user_id = next(iter(user_ids))
+
+        # Mark all users as receiving full presence on their next sync
+        await self.store.add_users_to_send_full_presence_to(user_ids)
+
+        # Add a new entry to the presence stream. Since we use stream tokens to determine whether a
+        # local user should receive a full snapshot of presence when they sync, we need to bump the
+        # presence stream so that subsequent syncs with no presence activity in between won't result
+        # in the client receiving multiple full snapshots of presence.
+        #
+        # If we bump the stream ID, then the user will get a higher stream token next sync, and thus
+        # correctly won't receive a second snapshot.
+
+        # Get the current presence state for one of the users (defaults to offline if not found)
+        current_presence_state = await self.get_state(UserID.from_string(user_id))
+
+        # Convert the UserPresenceState object into a serializable dict
+        state = {
+            "presence": current_presence_state.state,
+            "status_message": current_presence_state.status_msg,
+        }
+
+        # Copy the presence state to the tip of the presence stream.
+
+        # We set force_notify=True here so that this presence update is guaranteed to
+        # increment the presence stream ID (which resending the current user's presence
+        # otherwise would not do).
+        await self.set_state(UserID.from_string(user_id), state, force_notify=True)
+
 
 class _NullContextManager(ContextManager[None]):
     """A context manager which does nothing."""
@@ -480,8 +537,17 @@ class WorkerPresenceHandler(BasePresenceHandler):
         target_user: UserID,
         state: JsonDict,
         ignore_status_msg: bool = False,
+        force_notify: bool = False,
     ) -> None:
-        """Set the presence state of the user."""
+        """Set the presence state of the user.
+
+        Args:
+            target_user: The ID of the user to set the presence state of.
+            state: The presence state as a JSON dictionary.
+            ignore_status_msg: True to ignore the "status_msg" field of the `state` dict.
+                If False, the user's current status will be updated.
+            force_notify: Whether to force notification of the update to clients.
+        """
         presence = state["presence"]
 
         valid_presence = (
@@ -508,6 +574,7 @@ class WorkerPresenceHandler(BasePresenceHandler):
             user_id=user_id,
             state=state,
             ignore_status_msg=ignore_status_msg,
+            force_notify=force_notify,
         )
 
     async def bump_presence_active_time(self, user: UserID) -> None:
@@ -677,13 +744,19 @@ class PresenceHandler(BasePresenceHandler):
                 [self.user_to_current_state[user_id] for user_id in unpersisted]
             )
 
-    async def _update_states(self, new_states: Iterable[UserPresenceState]) -> None:
+    async def _update_states(
+        self, new_states: Iterable[UserPresenceState], force_notify: bool = False
+    ) -> None:
         """Updates presence of users. Sets the appropriate timeouts. Pokes
         the notifier and federation if and only if the changed presence state
         should be sent to clients/servers.
 
         Args:
             new_states: The new user presence state updates to process.
+            force_notify: Whether to force notifying clients of this presence state update,
+                even if it doesn't change the state of a user's presence (e.g online -> online).
+                This is currently used to bump the max presence stream ID without changing any
+                user's presence (see PresenceHandler.add_users_to_send_full_presence_to).
         """
         now = self.clock.time_msec()
 
@@ -720,6 +793,9 @@ class PresenceHandler(BasePresenceHandler):
                     now=now,
                 )
 
+                if force_notify:
+                    should_notify = True
+
                 self.user_to_current_state[user_id] = new_state
 
                 if should_notify:
@@ -1058,9 +1134,21 @@ class PresenceHandler(BasePresenceHandler):
             await self._update_states(updates)
 
     async def set_state(
-        self, target_user: UserID, state: JsonDict, ignore_status_msg: bool = False
+        self,
+        target_user: UserID,
+        state: JsonDict,
+        ignore_status_msg: bool = False,
+        force_notify: bool = False,
     ) -> None:
-        """Set the presence state of the user."""
+        """Set the presence state of the user.
+
+        Args:
+            target_user: The ID of the user to set the presence state of.
+            state: The presence state as a JSON dictionary.
+            ignore_status_msg: True to ignore the "status_msg" field of the `state` dict.
+                If False, the user's current status will be updated.
+            force_notify: Whether to force notification of the update to clients.
+        """
         status_msg = state.get("status_msg", None)
         presence = state["presence"]
 
@@ -1091,7 +1179,9 @@ class PresenceHandler(BasePresenceHandler):
         ):
             new_fields["last_active_ts"] = self.clock.time_msec()
 
-        await self._update_states([prev_state.copy_and_replace(**new_fields)])
+        await self._update_states(
+            [prev_state.copy_and_replace(**new_fields)], force_notify=force_notify
+        )
 
     async def is_visible(self, observed_user: UserID, observer_user: UserID) -> bool:
         """Returns whether a user can see another user's presence."""
@@ -1389,11 +1479,10 @@ class PresenceEventSource:
         #
         #   Presence -> Notifier -> PresenceEventSource -> Presence
         #
-        # Same with get_module_api, get_presence_router
+        # Same with get_presence_router:
         #
         #   AuthHandler -> Notifier -> PresenceEventSource -> ModuleApi -> AuthHandler
         self.get_presence_handler = hs.get_presence_handler
-        self.get_module_api = hs.get_module_api
         self.get_presence_router = hs.get_presence_router
         self.clock = hs.get_clock()
         self.store = hs.get_datastore()
@@ -1424,16 +1513,21 @@ class PresenceEventSource:
         stream_change_cache = self.store.presence_stream_cache
 
         with Measure(self.clock, "presence.get_new_events"):
-            if user_id in self.get_module_api()._send_full_presence_to_local_users:
-                # This user has been specified by a module to receive all current, online
-                # user presence. Removing from_key and setting include_offline to false
-                # will do effectively this.
-                from_key = None
-                include_offline = False
-
             if from_key is not None:
                 from_key = int(from_key)
 
+                # Check if this user should receive all current, online user presence. We only
+                # bother to do this if from_key is set, as otherwise the user will receive all
+                # user presence anyways.
+                if await self.store.should_user_receive_full_presence_with_token(
+                    user_id, from_key
+                ):
+                    # This user has been specified by a module to receive all current, online
+                    # user presence. Removing from_key and setting include_offline to false
+                    # will do effectively this.
+                    from_key = None
+                    include_offline = False
+
             max_token = self.store.get_current_presence_token()
             if from_key == max_token:
                 # This is necessary as due to the way stream ID generators work
@@ -1467,12 +1561,6 @@ class PresenceEventSource:
                     user_id, include_offline, from_key
                 )
 
-                # Remove the user from the list of users to receive all presence
-                if user_id in self.get_module_api()._send_full_presence_to_local_users:
-                    self.get_module_api()._send_full_presence_to_local_users.remove(
-                        user_id
-                    )
-
                 return presence_updates, max_token
 
             # Make mypy happy. users_interested_in should now be a set
@@ -1522,10 +1610,6 @@ class PresenceEventSource:
             )
             presence_updates = list(users_to_state.values())
 
-        # Remove the user from the list of users to receive all presence
-        if user_id in self.get_module_api()._send_full_presence_to_local_users:
-            self.get_module_api()._send_full_presence_to_local_users.remove(user_id)
-
         if not include_offline:
             # Filter out offline presence states
             presence_updates = self._filter_offline_presence_state(presence_updates)

synapse/handlers/room_member.py

@@ -260,25 +260,15 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
 
         if event.membership == Membership.JOIN:
             newly_joined = True
-            user_is_invited = False
+            prev_member_event = None
             if prev_member_event_id:
                 prev_member_event = await self.store.get_event(prev_member_event_id)
                 newly_joined = prev_member_event.membership != Membership.JOIN
-                user_is_invited = prev_member_event.membership == Membership.INVITE
 
-            # If the member is not already in the room and is not accepting an invite,
-            # check if they should be allowed access via membership in a space.
-            if (
-                newly_joined
-                and not user_is_invited
-                and not await self.event_auth_handler.can_join_without_invite(
-                    prev_state_ids, event.room_version, user_id
-                )
-            ):
-                raise AuthError(
-                    403,
-                    "You do not belong to any of the required spaces to join this room.",
-                )
+            # Check if the member should be allowed access via membership in a space.
+            await self.event_auth_handler.check_restricted_join_rules(
+                prev_state_ids, event.room_version, user_id, prev_member_event
+            )
 
             # Only rate-limit if the user actually joined the room, otherwise we'll end
             # up blocking profile updates.

synapse/handlers/send_email.py

@@ -0,0 +1,98 @@
+# Copyright 2021 The Matrix.org C.I.C. Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+import email.utils
+import logging
+from email.mime.multipart import MIMEMultipart
+from email.mime.text import MIMEText
+from typing import TYPE_CHECKING
+
+from synapse.logging.context import make_deferred_yieldable
+
+if TYPE_CHECKING:
+    from synapse.server import HomeServer
+
+logger = logging.getLogger(__name__)
+
+
+class SendEmailHandler:
+    def __init__(self, hs: "HomeServer"):
+        self.hs = hs
+
+        self._sendmail = hs.get_sendmail()
+        self._reactor = hs.get_reactor()
+
+        self._from = hs.config.email.email_notif_from
+        self._smtp_host = hs.config.email.email_smtp_host
+        self._smtp_port = hs.config.email.email_smtp_port
+        self._smtp_user = hs.config.email.email_smtp_user
+        self._smtp_pass = hs.config.email.email_smtp_pass
+        self._require_transport_security = hs.config.email.require_transport_security
+
+    async def send_email(
+        self,
+        email_address: str,
+        subject: str,
+        app_name: str,
+        html: str,
+        text: str,
+    ) -> None:
+        """Send a multipart email with the given information.
+
+        Args:
+            email_address: The address to send the email to.
+            subject: The email's subject.
+            app_name: The app name to include in the From header.
+            html: The HTML content to include in the email.
+            text: The plain text content to include in the email.
+        """
+        try:
+            from_string = self._from % {"app": app_name}
+        except (KeyError, TypeError):
+            from_string = self._from
+
+        raw_from = email.utils.parseaddr(from_string)[1]
+        raw_to = email.utils.parseaddr(email_address)[1]
+
+        if raw_to == "":
+            raise RuntimeError("Invalid 'to' address")
+
+        html_part = MIMEText(html, "html", "utf8")
+        text_part = MIMEText(text, "plain", "utf8")
+
+        multipart_msg = MIMEMultipart("alternative")
+        multipart_msg["Subject"] = subject
+        multipart_msg["From"] = from_string
+        multipart_msg["To"] = email_address
+        multipart_msg["Date"] = email.utils.formatdate()
+        multipart_msg["Message-ID"] = email.utils.make_msgid()
+        multipart_msg.attach(text_part)
+        multipart_msg.attach(html_part)
+
+        logger.info("Sending email to %s" % email_address)
+
+        await make_deferred_yieldable(
+            self._sendmail(
+                self._smtp_host,
+                raw_from,
+                raw_to,
+                multipart_msg.as_string().encode("utf8"),
+                reactor=self._reactor,
+                port=self._smtp_port,
+                requireAuthentication=self._smtp_user is not None,
+                username=self._smtp_user,
+                password=self._smtp_pass,
+                requireTransportSecurity=self._require_transport_security,
+            )
+        )

synapse/handlers/space_summary.py

@@ -16,11 +16,16 @@ import itertools
 import logging
 import re
 from collections import deque
-from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple, cast
+from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Set, Tuple
 
 import attr
 
-from synapse.api.constants import EventContentFields, EventTypes, HistoryVisibility
+from synapse.api.constants import (
+    EventContentFields,
+    EventTypes,
+    HistoryVisibility,
+    Membership,
+)
 from synapse.api.errors import AuthError
 from synapse.events import EventBase
 from synapse.events.utils import format_event_for_client_v2
@@ -32,7 +37,6 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 # number of rooms to return. We'll stop once we hit this limit.
-# TODO: allow clients to reduce this with a request param.
 MAX_ROOMS = 50
 
 # max number of events to return per room.
@@ -46,8 +50,7 @@ class SpaceSummaryHandler:
     def __init__(self, hs: "HomeServer"):
         self._clock = hs.get_clock()
         self._auth = hs.get_auth()
-        self._room_list_handler = hs.get_room_list_handler()
-        self._state_handler = hs.get_state_handler()
+        self._event_auth_handler = hs.get_event_auth_handler()
         self._store = hs.get_datastore()
         self._event_serializer = hs.get_event_client_serializer()
         self._server_name = hs.hostname
@@ -112,28 +115,88 @@ class SpaceSummaryHandler:
             max_children = max_rooms_per_space if processed_rooms else None
 
             if is_in_room:
-                rooms, events = await self._summarize_local_room(
-                    requester, room_id, suggested_only, max_children
+                room, events = await self._summarize_local_room(
+                    requester, None, room_id, suggested_only, max_children
                 )
+
+                logger.debug(
+                    "Query of local room %s returned events %s",
+                    room_id,
+                    ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
+                )
+
+                if room:
+                    rooms_result.append(room)
             else:
-                rooms, events = await self._summarize_remote_room(
+                fed_rooms, fed_events = await self._summarize_remote_room(
                     queue_entry,
                     suggested_only,
                     max_children,
                     exclude_rooms=processed_rooms,
                 )
 
-            logger.debug(
-                "Query of %s returned rooms %s, events %s",
-                queue_entry.room_id,
-                [room.get("room_id") for room in rooms],
-                ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in events],
-            )
+                # The results over federation might include rooms that the we,
+                # as the requesting server, are allowed to see, but the requesting
+                # user is not permitted see.
+                #
+                # Filter the returned results to only what is accessible to the user.
+                room_ids = set()
+                events = []
+                for room in fed_rooms:
+                    fed_room_id = room.get("room_id")
+                    if not fed_room_id or not isinstance(fed_room_id, str):
+                        continue
 
-            rooms_result.extend(rooms)
+                    # The room should only be included in the summary if:
+                    #     a. the user is in the room;
+                    #     b. the room is world readable; or
+                    #     c. the user is in a space that has been granted access to
+                    #        the room.
+                    #
+                    # Note that we know the user is not in the root room (which is
+                    # why the remote call was made in the first place), but the user
+                    # could be in one of the children rooms and we just didn't know
+                    # about the link.
+                    include_room = room.get("world_readable") is True
 
-            # any rooms returned don't need visiting again
-            processed_rooms.update(cast(str, room.get("room_id")) for room in rooms)
+                    # Check if the user is a member of any of the allowed spaces
+                    # from the response.
+                    allowed_spaces = room.get("allowed_spaces")
+                    if (
+                        not include_room
+                        and allowed_spaces
+                        and isinstance(allowed_spaces, list)
+                    ):
+                        include_room = await self._event_auth_handler.is_user_in_rooms(
+                            allowed_spaces, requester
+                        )
+
+                    # Finally, if this isn't the requested room, check ourselves
+                    # if we can access the room.
+                    if not include_room and fed_room_id != queue_entry.room_id:
+                        include_room = await self._is_room_accessible(
+                            fed_room_id, requester, None
+                        )
+
+                    # The user can see the room, include it!
+                    if include_room:
+                        rooms_result.append(room)
+                        room_ids.add(fed_room_id)
+
+                    # All rooms returned don't need visiting again (even if the user
+                    # didn't have access to them).
+                    processed_rooms.add(fed_room_id)
+
+                for event in fed_events:
+                    if event.get("room_id") in room_ids:
+                        events.append(event)
+
+                logger.debug(
+                    "Query of %s returned rooms %s, events %s",
+                    room_id,
+                    [room.get("room_id") for room in fed_rooms],
+                    ["%s->%s" % (ev["room_id"], ev["state_key"]) for ev in fed_events],
+                )
 
             # the room we queried may or may not have been returned, but don't process
             # it again, anyway.
@@ -159,10 +222,16 @@ class SpaceSummaryHandler:
                 )
                 processed_events.add(ev_key)
 
+        # Before returning to the client, remove the allowed_spaces key for any
+        # rooms.
+        for room in rooms_result:
+            room.pop("allowed_spaces", None)
+
         return {"rooms": rooms_result, "events": events_result}
 
     async def federation_space_summary(
         self,
+        origin: str,
         room_id: str,
         suggested_only: bool,
         max_rooms_per_space: Optional[int],
@@ -172,6 +241,8 @@ class SpaceSummaryHandler:
         Implementation of the space summary Federation API
 
         Args:
+            origin: The server requesting the spaces summary.
+
             room_id: room id to start the summary at
 
             suggested_only: whether we should only return children with the "suggested"
@@ -206,14 +277,15 @@ class SpaceSummaryHandler:
 
             logger.debug("Processing room %s", room_id)
 
-            rooms, events = await self._summarize_local_room(
-                None, room_id, suggested_only, max_rooms_per_space
+            room, events = await self._summarize_local_room(
+                None, origin, room_id, suggested_only, max_rooms_per_space
             )
 
             processed_rooms.add(room_id)
 
-            rooms_result.extend(rooms)
-            events_result.extend(events)
+            if room:
+                rooms_result.append(room)
+                events_result.extend(events)
 
             # add any children to the queue
             room_queue.extend(edge_event["state_key"] for edge_event in events)
@@ -223,19 +295,27 @@ class SpaceSummaryHandler:
     async def _summarize_local_room(
         self,
         requester: Optional[str],
+        origin: Optional[str],
         room_id: str,
         suggested_only: bool,
         max_children: Optional[int],
-    ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]:
+    ) -> Tuple[Optional[JsonDict], Sequence[JsonDict]]:
         """
         Generate a room entry and a list of event entries for a given room.
 
         Args:
-            requester: The requesting user, or None if this is over federation.
+            requester:
+                The user requesting the summary, if it is a local request. None
+                if this is a federation request.
+            origin:
+                The server requesting the summary, if it is a federation request.
+                None if this is a local request.
             room_id: The room ID to summarize.
             suggested_only: True if only suggested children should be returned.
                 Otherwise, all children are returned.
-            max_children: The maximum number of children to return for this node.
+            max_children:
+                The maximum number of children rooms to include. This is capped
+                to a server-set limit.
 
         Returns:
             A tuple of:
@@ -244,8 +324,8 @@ class SpaceSummaryHandler:
                 An iterable of the sorted children events. This may be limited
                 to a maximum size or may include all children.
         """
-        if not await self._is_room_accessible(room_id, requester):
-            return (), ()
+        if not await self._is_room_accessible(room_id, requester, origin):
+            return None, ()
 
         room_entry = await self._build_room_entry(room_id)
 
@@ -269,7 +349,7 @@ class SpaceSummaryHandler:
                     event_format=format_event_for_client_v2,
                 )
             )
-        return (room_entry,), events_result
+        return room_entry, events_result
 
     async def _summarize_remote_room(
         self,
@@ -278,6 +358,26 @@ class SpaceSummaryHandler:
         max_children: Optional[int],
         exclude_rooms: Iterable[str],
     ) -> Tuple[Sequence[JsonDict], Sequence[JsonDict]]:
+        """
+        Request room entries and a list of event entries for a given room by querying a remote server.
+
+        Args:
+            room: The room to summarize.
+            suggested_only: True if only suggested children should be returned.
+                Otherwise, all children are returned.
+            max_children:
+                The maximum number of children rooms to include. This is capped
+                to a server-set limit.
+            exclude_rooms:
+                Rooms IDs which do not need to be summarized.
+
+        Returns:
+            A tuple of:
+                An iterable of rooms.
+
+                An iterable of the sorted children events. This may be limited
+                to a maximum size or may include all children.
+        """
         room_id = room.room_id
         logger.info("Requesting summary for %s via %s", room_id, room.via)
 
@@ -309,27 +409,93 @@ class SpaceSummaryHandler:
             or ev.event_type == EventTypes.SpaceChild
         )
 
-    async def _is_room_accessible(self, room_id: str, requester: Optional[str]) -> bool:
-        # if we have an authenticated requesting user, first check if they are in the
-        # room
+    async def _is_room_accessible(
+        self, room_id: str, requester: Optional[str], origin: Optional[str]
+    ) -> bool:
+        """
+        Calculate whether the room should be shown in the spaces summary.
+
+        It should be included if:
+
+        * The requester is joined or invited to the room.
+        * The requester can join without an invite (per MSC3083).
+        * The origin server has any user that is joined or invited to the room.
+        * The history visibility is set to world readable.
+
+        Args:
+            room_id: The room ID to summarize.
+            requester:
+                The user requesting the summary, if it is a local request. None
+                if this is a federation request.
+            origin:
+                The server requesting the summary, if it is a federation request.
+                None if this is a local request.
+
+        Returns:
+             True if the room should be included in the spaces summary.
+        """
+        state_ids = await self._store.get_current_state_ids(room_id)
+
+        # If there's no state for the room, it isn't known.
+        if not state_ids:
+            logger.info("room %s is unknown, omitting from summary", room_id)
+            return False
+
+        room_version = await self._store.get_room_version(room_id)
+
+        # if we have an authenticated requesting user, first check if they are able to view
+        # stripped state in the room.
         if requester:
+            member_event_id = state_ids.get((EventTypes.Member, requester), None)
+
+            # If they're in the room they can see info on it.
+            member_event = None
+            if member_event_id:
+                member_event = await self._store.get_event(member_event_id)
+                if member_event.membership in (Membership.JOIN, Membership.INVITE):
+                    return True
+
+            # Otherwise, check if they should be allowed access via membership in a space.
             try:
-                await self._auth.check_user_in_room(room_id, requester)
-                return True
+                await self._event_auth_handler.check_restricted_join_rules(
+                    state_ids, room_version, requester, member_event
+                )
             except AuthError:
+                # The user doesn't have access due to spaces, but might have access
+                # another way. Keep trying.
                 pass
+            else:
+                return True
+
+        # If this is a request over federation, check if the host is in the room or
+        # is in one of the spaces specified via the join rules.
+        elif origin:
+            if await self._auth.check_host_in_room(room_id, origin):
+                return True
+
+            # Alternately, if the host has a user in any of the spaces specified
+            # for access, then the host can see this room (and should do filtering
+            # if the requester cannot see it).
+            if await self._event_auth_handler.has_restricted_join_rules(
+                state_ids, room_version
+            ):
+                allowed_spaces = (
+                    await self._event_auth_handler.get_spaces_that_allow_join(state_ids)
+                )
+                for space_id in allowed_spaces:
+                    if await self._auth.check_host_in_room(space_id, origin):
+                        return True
 
         # otherwise, check if the room is peekable
-        hist_vis_ev = await self._state_handler.get_current_state(
-            room_id, EventTypes.RoomHistoryVisibility, ""
-        )
-        if hist_vis_ev:
+        hist_vis_event_id = state_ids.get((EventTypes.RoomHistoryVisibility, ""), None)
+        if hist_vis_event_id:
+            hist_vis_ev = await self._store.get_event(hist_vis_event_id)
             hist_vis = hist_vis_ev.content.get("history_visibility")
             if hist_vis == HistoryVisibility.WORLD_READABLE:
                 return True
 
         logger.info(
-            "room %s is unpeekable and user %s is not a member, omitting from summary",
+            "room %s is unpeekable and user %s is not a member / not allowed to join, omitting from summary",
             room_id,
             requester,
         )
@@ -354,6 +520,15 @@ class SpaceSummaryHandler:
         if not room_type:
             room_type = create_event.content.get(EventContentFields.MSC1772_ROOM_TYPE)
 
+        room_version = await self._store.get_room_version(room_id)
+        allowed_spaces = None
+        if await self._event_auth_handler.has_restricted_join_rules(
+            current_state_ids, room_version
+        ):
+            allowed_spaces = await self._event_auth_handler.get_spaces_that_allow_join(
+                current_state_ids
+            )
+
         entry = {
             "room_id": stats["room_id"],
             "name": stats["name"],
@@ -367,6 +542,7 @@ class SpaceSummaryHandler:
             "guest_can_join": stats["guest_access"] == "can_join",
             "creation_ts": create_event.origin_server_ts,
             "room_type": room_type,
+            "allowed_spaces": allowed_spaces,
         }
 
         # Filter out Nones – rather omit the field altogether
@@ -430,8 +606,8 @@ def _is_suggested_child_event(edge_event: EventBase) -> bool:
     return False
 
 
-# Order may only contain characters in the range of \x20 (space) to \x7F (~).
-_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7F]")
+# Order may only contain characters in the range of \x20 (space) to \x7E (~) inclusive.
+_INVALID_ORDER_CHARS_RE = re.compile(r"[^\x20-\x7E]")
 
 
 def _child_events_comparison_key(child: EventBase) -> Tuple[bool, Optional[str], str]: