Refactor checking restricted join rules (#10007)

To be more consistent with similar code. The check now automatically
raises an AuthError instead of passing back a boolean. It also absorbs
some shared logic between callers.

4 files changed, 50 insertions, 51 deletions

diff --git a/changelog.d/10007.feature b/changelog.d/10007.feature
new file mode 100644
index 0000000000..2c655350c0
--- /dev/null
+++ b/changelog.d/10007.feature
@@ -0,0 +1 @@
+Experimental support to allow a user who could join a restricted room to view it in the spaces summary.
diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py
index eff639f407..5b2fe103e7 100644
--- a/synapse/handlers/event_auth.py
+++ b/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, 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,44 +31,58 @@ 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.
 
-        Returns:
-            True if the user can join the room, false otherwise.
+        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 only applies to room versions which support the new join rule.
         if not room_version.msc3083_join_rules:
-            return True
+            return
 
         # 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
 
         # 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
+            return
 
         # 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
+            allowed_spaces = ()
 
         # Get the list of joined rooms and see if there's an overlap.
-        joined_rooms = await self._store.get_rooms_for_user(user_id)
+        if allowed_spaces:
+            joined_rooms = await self._store.get_rooms_for_user(user_id)
+        else:
+            joined_rooms = ()
 
         # Pull out the other room IDs, invalid data gets filtered.
         for space in allowed_spaces:
@@ -80,7 +96,10 @@ class EventAuthHandler:
             # The user was joined to one of the spaces specified, they can join
             # this room!
             if space_id in joined_rooms:
-                return True
+                return
 
         # The user was not in any of the required spaces.
-        return False
+        raise AuthError(
+            403,
+            "You do not belong to any of the required spaces to join this room.",
+        )
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 798ed75b30..678f6b7707 100644
--- a/synapse/handlers/federation.py
+++ b/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)
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 9a092da715..d6fc43e798 100644
--- a/synapse/handlers/room_member.py
+++ b/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.