Merge pull request #6949 from matrix-org/rav/list_room_aliases_peekable

Make room alias lists peekable

changelog.d/6949.feature

@@ -0,0 +1 @@
+Implement `GET /_matrix/client/r0/rooms/{roomId}/aliases` endpoint as per [MSC2432](https://github.com/matrix-org/matrix-doc/pull/2432).

synapse/api/auth.py

@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import logging
+from typing import Optional
 
 from six import itervalues
 
@@ -35,6 +36,7 @@ from synapse.api.errors import (
 )
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
 from synapse.config.server import is_threepid_reserved
+from synapse.events import EventBase
 from synapse.types import StateMap, UserID
 from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache
 from synapse.util.caches.lrucache import LruCache
@@ -92,20 +94,34 @@ class Auth(object):
         )
 
     @defer.inlineCallbacks
-    def check_joined_room(self, room_id, user_id, current_state=None):
+    def check_user_in_room(
-        """Check if the user is currently joined in the room
+        self,
+        room_id: str,
+        user_id: str,
+        current_state: Optional[StateMap[EventBase]] = None,
+        allow_departed_users: bool = False,
+    ):
+        """Check if the user is in the room, or was at some point.
         Args:
-            room_id(str): The room to check.
+            room_id: The room to check.
-            user_id(str): The user to check.
+
-            current_state(dict): Optional map of the current state of the room.
+            user_id: The user to check.
+
+            current_state: Optional map of the current state of the room.
                 If provided then that map is used to check whether they are a
                 member of the room. Otherwise the current membership is
                 loaded from the database.
+
+            allow_departed_users: if True, accept users that were previously
+                members but have now departed.
+
         Raises:
-            AuthError if the user is not in the room.
+            AuthError if the user is/was not in the room.
         Returns:
-            A deferred membership event for the user if the user is in
+            Deferred[Optional[EventBase]]:
-            the room.
+                Membership event for the user if the user was in the
+                room. This will be the join event if they are currently joined to
+                the room. This will be the leave event if they have left the room.
         """
         if current_state:
             member = current_state.get((EventTypes.Member, user_id), None)
@@ -113,37 +129,19 @@ class Auth(object):
             member = yield self.state.get_current_state(
                 room_id=room_id, event_type=EventTypes.Member, state_key=user_id
             )
-
-        self._check_joined_room(member, user_id, room_id)
-        return member
-
-    @defer.inlineCallbacks
-    def check_user_was_in_room(self, room_id, user_id):
-        """Check if the user was in the room at some point.
-        Args:
-            room_id(str): The room to check.
-            user_id(str): The user to check.
-        Raises:
-            AuthError if the user was never in the room.
-        Returns:
-            A deferred membership event for the user if the user was in the
-            room. This will be the join event if they are currently joined to
-            the room. This will be the leave event if they have left the room.
-        """
-        member = yield self.state.get_current_state(
-            room_id=room_id, event_type=EventTypes.Member, state_key=user_id
-        )
         membership = member.membership if member else None
 
-        if membership not in (Membership.JOIN, Membership.LEAVE):
+        if membership == Membership.JOIN:
-            raise AuthError(403, "User %s not in room %s" % (user_id, room_id))
+            return member
 
-        if membership == Membership.LEAVE:
+        # XXX this looks totally bogus. Why do we not allow users who have been banned,
+        # or those who were members previously and have been re-invited?
+        if allow_departed_users and membership == Membership.LEAVE:
             forgot = yield self.store.did_forget(user_id, room_id)
-            if forgot:
+            if not forgot:
-                raise AuthError(403, "User %s not in room %s" % (user_id, room_id))
+                return member
 
-        return member
+        raise AuthError(403, "User %s not in room %s" % (user_id, room_id))
 
     @defer.inlineCallbacks
     def check_host_in_room(self, room_id, host):
@@ -151,12 +149,6 @@ class Auth(object):
             latest_event_ids = yield self.store.is_host_joined(room_id, host)
             return latest_event_ids
 
-    def _check_joined_room(self, member, user_id, room_id):
-        if not member or member.membership != Membership.JOIN:
-            raise AuthError(
-                403, "User %s not in room %s (%s)" % (user_id, room_id, repr(member))
-            )
-
     def can_federate(self, event, auth_events):
         creation_event = auth_events.get((EventTypes.Create, ""))
 
@@ -560,7 +552,7 @@ class Auth(object):
             return True
 
         user_id = user.to_string()
-        yield self.check_joined_room(room_id, user_id)
+        yield self.check_user_in_room(room_id, user_id)
 
         # We currently require the user is a "moderator" in the room. We do this
         # by checking if they would (theoretically) be able to change the
@@ -633,10 +625,18 @@ class Auth(object):
             return query_params[0].decode("ascii")
 
     @defer.inlineCallbacks
-    def check_in_room_or_world_readable(self, room_id, user_id):
+    def check_user_in_room_or_world_readable(
+        self, room_id: str, user_id: str, allow_departed_users: bool = False
+    ):
         """Checks that the user is or was in the room or the room is world
         readable. If it isn't then an exception is raised.
 
+        Args:
+            room_id: room to check
+            user_id: user to check
+            allow_departed_users: if True, accept users that were previously
+                members but have now departed
+
         Returns:
             Deferred[tuple[str, str|None]]: Resolves to the current membership of
                 the user in the room and the membership event ID of the user. If
@@ -645,12 +645,14 @@ class Auth(object):
         """
 
         try:
-            # check_user_was_in_room will return the most recent membership
+            # check_user_in_room will return the most recent membership
             # event for the user if:
             #  * The user is a non-guest user, and was ever in the room
             #  * The user is a guest user, and has joined the room
             # else it will throw.
-            member_event = yield self.check_user_was_in_room(room_id, user_id)
+            member_event = yield self.check_user_in_room(
+                room_id, user_id, allow_departed_users=allow_departed_users
+            )
             return member_event.membership, member_event.event_id
         except AuthError:
             visibility = yield self.state.get_current_state(
@@ -662,7 +664,9 @@ class Auth(object):
             ):
                 return Membership.JOIN, None
             raise AuthError(
-                403, "Guest access not allowed", errcode=Codes.GUEST_ACCESS_FORBIDDEN
+                403,
+                "User %s not in room %s, and room previews are disabled"
+                % (user_id, room_id),
             )
 
     @defer.inlineCallbacks

synapse/handlers/directory.py

@@ -463,7 +463,9 @@ class DirectoryHandler(BaseHandler):
         # allow access to server admins and current members of the room
         is_admin = await self.auth.is_server_admin(requester.user)
         if not is_admin:
-            await self.auth.check_joined_room(room_id, requester.user.to_string())
+            await self.auth.check_user_in_room_or_world_readable(
+                room_id, requester.user.to_string()
+            )
 
         aliases = await self.store.get_aliases_for_room(room_id)
         return aliases

synapse/handlers/initial_sync.py

@@ -18,7 +18,7 @@ import logging
 from twisted.internet import defer
 
 from synapse.api.constants import EventTypes, Membership
-from synapse.api.errors import AuthError, Codes, SynapseError
+from synapse.api.errors import SynapseError
 from synapse.events.validator import EventValidator
 from synapse.handlers.presence import format_user_presence_state
 from synapse.logging.context import make_deferred_yieldable, run_in_background
@@ -274,8 +274,11 @@ class InitialSyncHandler(BaseHandler):
 
         user_id = requester.user.to_string()
 
-        membership, member_event_id = await self._check_in_room_or_world_readable(
+        (
-            room_id, user_id
+            membership,
+            member_event_id,
+        ) = await self.auth.check_user_in_room_or_world_readable(
+            room_id, user_id, allow_departed_users=True,
         )
         is_peeking = member_event_id is None
 
@@ -433,25 +436,3 @@ class InitialSyncHandler(BaseHandler):
             ret["membership"] = membership
 
         return ret
-
-    async def _check_in_room_or_world_readable(self, room_id, user_id):
-        try:
-            # check_user_was_in_room will return the most recent membership
-            # event for the user if:
-            #  * The user is a non-guest user, and was ever in the room
-            #  * The user is a guest user, and has joined the room
-            # else it will throw.
-            member_event = await self.auth.check_user_was_in_room(room_id, user_id)
-            return member_event.membership, member_event.event_id
-        except AuthError:
-            visibility = await self.state_handler.get_current_state(
-                room_id, EventTypes.RoomHistoryVisibility, ""
-            )
-            if (
-                visibility
-                and visibility.content["history_visibility"] == "world_readable"
-            ):
-                return Membership.JOIN, None
-            raise AuthError(
-                403, "Guest access not allowed", errcode=Codes.GUEST_ACCESS_FORBIDDEN
-            )

synapse/handlers/message.py

@@ -99,7 +99,9 @@ class MessageHandler(object):
         (
             membership,
             membership_event_id,
-        ) = yield self.auth.check_in_room_or_world_readable(room_id, user_id)
+        ) = yield self.auth.check_user_in_room_or_world_readable(
+            room_id, user_id, allow_departed_users=True
+        )
 
         if membership == Membership.JOIN:
             data = yield self.state.get_current_state(room_id, event_type, state_key)
@@ -177,7 +179,9 @@ class MessageHandler(object):
             (
                 membership,
                 membership_event_id,
-            ) = yield self.auth.check_in_room_or_world_readable(room_id, user_id)
+            ) = yield self.auth.check_user_in_room_or_world_readable(
+                room_id, user_id, allow_departed_users=True
+            )
 
             if membership == Membership.JOIN:
                 state_ids = yield self.store.get_filtered_current_state_ids(
@@ -216,8 +220,8 @@ class MessageHandler(object):
         if not requester.app_service:
             # We check AS auth after fetching the room membership, as it
             # requires us to pull out all joined members anyway.
-            membership, _ = yield self.auth.check_in_room_or_world_readable(
+            membership, _ = yield self.auth.check_user_in_room_or_world_readable(
-                room_id, user_id
+                room_id, user_id, allow_departed_users=True
             )
             if membership != Membership.JOIN:
                 raise NotImplementedError(

synapse/handlers/pagination.py

@@ -335,7 +335,9 @@ class PaginationHandler(object):
             (
                 membership,
                 member_event_id,
-            ) = await self.auth.check_in_room_or_world_readable(room_id, user_id)
+            ) = await self.auth.check_user_in_room_or_world_readable(
+                room_id, user_id, allow_departed_users=True
+            )
 
             if source_config.direction == "b":
                 # if we're going backwards, we might need to backfill. This

synapse/handlers/typing.py

@@ -125,7 +125,7 @@ class TypingHandler(object):
         if target_user_id != auth_user_id:
             raise AuthError(400, "Cannot set another user's typing state")
 
-        yield self.auth.check_joined_room(room_id, target_user_id)
+        yield self.auth.check_user_in_room(room_id, target_user_id)
 
         logger.debug("%s has started typing in %s", target_user_id, room_id)
 
@@ -155,7 +155,7 @@ class TypingHandler(object):
         if target_user_id != auth_user_id:
             raise AuthError(400, "Cannot set another user's typing state")
 
-        yield self.auth.check_joined_room(room_id, target_user_id)
+        yield self.auth.check_user_in_room(room_id, target_user_id)
 
         logger.debug("%s has stopped typing in %s", target_user_id, room_id)
 

synapse/rest/client/v2_alpha/relations.py

@@ -142,8 +142,8 @@ class RelationPaginationServlet(RestServlet):
     ):
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
 
-        await self.auth.check_in_room_or_world_readable(
+        await self.auth.check_user_in_room_or_world_readable(
-            room_id, requester.user.to_string()
+            room_id, requester.user.to_string(), allow_departed_users=True
         )
 
         # This gets the original event and checks that a) the event exists and
@@ -235,8 +235,8 @@ class RelationAggregationPaginationServlet(RestServlet):
     ):
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
 
-        await self.auth.check_in_room_or_world_readable(
+        await self.auth.check_user_in_room_or_world_readable(
-            room_id, requester.user.to_string()
+            room_id, requester.user.to_string(), allow_departed_users=True,
         )
 
         # This checks that a) the event exists and b) the user is allowed to
@@ -313,8 +313,8 @@ class RelationAggregationGroupPaginationServlet(RestServlet):
     async def on_GET(self, request, room_id, parent_id, relation_type, event_type, key):
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
 
-        await self.auth.check_in_room_or_world_readable(
+        await self.auth.check_user_in_room_or_world_readable(
-            room_id, requester.user.to_string()
+            room_id, requester.user.to_string(), allow_departed_users=True,
         )
 
         # This checks that a) the event exists and b) the user is allowed to

tests/handlers/test_typing.py

@@ -122,11 +122,11 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
 
         self.room_members = []
 
-        def check_joined_room(room_id, user_id):
+        def check_user_in_room(room_id, user_id):
             if user_id not in [u.to_string() for u in self.room_members]:
                 raise AuthError(401, "User is not in the room")
 
-        hs.get_auth().check_joined_room = check_joined_room
+        hs.get_auth().check_user_in_room = check_user_in_room
 
         def get_joined_hosts_for_room(room_id):
             return set(member.domain for member in self.room_members)

tests/rest/client/v1/test_rooms.py

@@ -1775,6 +1775,23 @@ class RoomAliasListTestCase(unittest.HomeserverTestCase):
         res = self._get_aliases(self.room_owner_tok)
         self.assertEqual(set(res["aliases"]), {alias1, alias2})
 
+    def test_peekable_room(self):
+        alias1 = self._random_alias()
+        self._set_alias_via_directory(alias1)
+
+        self.helper.send_state(
+            self.room_id,
+            EventTypes.RoomHistoryVisibility,
+            body={"history_visibility": "world_readable"},
+            tok=self.room_owner_tok,
+        )
+
+        self.register_user("user", "test")
+        user_tok = self.login("user", "test")
+
+        res = self._get_aliases(user_tok)
+        self.assertEqual(res["aliases"], [alias1])
+
     def _get_aliases(self, access_token: str, expected_code: int = 200) -> JsonDict:
         """Calls the endpoint under test. returns the json response object."""
         request, channel = self.make_request(