add options to require an access_token to GET /profile and /publicRooms on CS API

changelog.d/5083.feature

@@ -0,0 +1 @@
+Add an configuration option to require authentication on /publicRooms and /profile endpoints.

docs/sample_config.yaml

@@ -69,6 +69,18 @@ pid_file: DATADIR/homeserver.pid
 #
 #use_presence: false
 
+# Whether to require authentication to retrieve profile data (avatars,
+# display names) of other users through the client API. Defaults to
+# 'false'.
+#
+#require_auth_for_profile_requests: true
+
+# If set to 'true', requires authentication to access the server's
+# public rooms directory through the client API, and forbids any other
+# homeserver to fetch it via federation. Defaults to 'false'.
+#
+#restrict_public_rooms_to_local_users: true
+
 # The GC threshold parameters to pass to `gc.set_threshold`, if defined
 #
 #gc_thresholds: [700, 10, 10]

synapse/config/server.py

@@ -72,6 +72,19 @@ def read_config(self, config):
         # master, potentially causing inconsistency.
         self.enable_media_repo = config.get("enable_media_repo", True)
 
+        # Whether to require authentication to retrieve profile data (avatars,
+        # display names) of other users through the client API.
+        self.require_auth_for_profile_requests = config.get(
+            "require_auth_for_profile_requests", False,
+        )
+
+        # If set to 'True', requires authentication to access the server's
+        # public rooms directory through the client API, and forbids any other
+        # homeserver to fetch it via federation.
+        self.restrict_public_rooms_to_local_users = config.get(
+            "restrict_public_rooms_to_local_users", False,
+        )
+
         # whether to enable search. If disabled, new entries will not be inserted
         # into the search tables and they will not be indexed. Users will receive
         # errors when attempting to search for messages.
@@ -321,6 +334,18 @@ def default_config(self, server_name, data_dir_path, **kwargs):
         #
         #use_presence: false
 
+        # Whether to require authentication to retrieve profile data (avatars,
+        # display names) of other users through the client API. Defaults to
+        # 'false'.
+        #
+        #require_auth_for_profile_requests: true
+
+        # If set to 'true', requires authentication to access the server's
+        # public rooms directory through the client API, and forbids any other
+        # homeserver to fetch it via federation. Defaults to 'false'.
+        #
+        #restrict_public_rooms_to_local_users: true
+
         # The GC threshold parameters to pass to `gc.set_threshold`, if defined
         #
         #gc_thresholds: [700, 10, 10]

synapse/federation/transport/server.py

@@ -98,6 +98,7 @@ def __init__(self, hs):
         self.server_name = hs.hostname
         self.store = hs.get_datastore()
         self.federation_domain_whitelist = hs.config.federation_domain_whitelist
+        self.deny_public_rooms = hs.config.restrict_public_rooms_to_local_users
 
     # A method just so we can pass 'self' as the authenticator to the Servlets
     @defer.inlineCallbacks
@@ -133,6 +134,9 @@ def authenticate_request(self, request, content):
         ):
             raise FederationDeniedError(origin)
 
+        if b"publicRooms" in request.path and self.deny_public_rooms:
+            raise FederationDeniedError(origin)
+
         if not json_request["signatures"]:
             raise NoAuthenticationError(
                 401, "Missing Authorization headers", Codes.UNAUTHORIZED,

synapse/handlers/profile.py

@@ -53,6 +53,7 @@ def __init__(self, hs):
     @defer.inlineCallbacks
     def get_profile(self, user_id):
         target_user = UserID.from_string(user_id)
+
         if self.hs.is_mine(target_user):
             try:
                 displayname = yield self.store.get_profile_displayname(
@@ -115,7 +116,9 @@ def get_profile_from_cache(self, user_id):
             defer.returnValue(profile or {})
 
     @defer.inlineCallbacks
-    def get_displayname(self, target_user):
+    def get_displayname(self, target_user, requester=None):
+        yield self.check_profile_query_allowed(target_user, requester)
+
         if self.hs.is_mine(target_user):
             try:
                 displayname = yield self.store.get_profile_displayname(
@@ -177,7 +180,9 @@ def set_displayname(self, target_user, requester, new_displayname, by_admin=Fals
         yield self._update_join_states(requester, target_user)
 
     @defer.inlineCallbacks
-    def get_avatar_url(self, target_user):
+    def get_avatar_url(self, target_user, requester=None):
+        yield self.check_profile_query_allowed(target_user, requester)
+
         if self.hs.is_mine(target_user):
             try:
                 avatar_url = yield self.store.get_profile_avatar_url(
@@ -283,6 +288,47 @@ def _update_join_states(self, requester, target_user):
                     room_id, str(e)
                 )
 
+    @defer.inlineCallbacks
+    def check_profile_query_allowed(self, target_user, requester=None):
+        """Checks whether a profile query is allowed. If the
+        'require_auth_for_profile_requests' config flag is set to True and a
+        'requester' is provided, the query is only allowed if the two users
+        share a room.
+
+        Args:
+            target_user (UserID): The owner of the queried profile.
+            requester (None|UserID): The user querying for the profile.
+
+        Raises:
+            SynapseError(403): The two users share no room, or ne user couldn't
+                be found to be in any room the server is in, and therefore the query
+                is denied.
+        """
+        # Implementation of MSC1301: don't allow looking up profiles if the
+        # requester isn't in the same room as the target. We expect requester to
+        # be None when this function is called outside of a profile query, e.g.
+        # when building a membership event. In this case, we must allow the
+        # lookup.
+        if self.hs.config.require_auth_for_profile_requests and requester:
+            try:
+                requester_rooms = yield self.store.get_rooms_for_user(
+                    requester.to_string()
+                )
+                target_user_rooms = yield self.store.get_rooms_for_user(
+                    target_user.to_string(),
+                )
+
+                # Check if the length of the intersection between the room lists
+                # for both users is 0.
+                if not len(requester_rooms.intersection(target_user_rooms)):
+                    raise SynapseError(403, "Profile isn't available", Codes.FORBIDDEN)
+            except StoreError as e:
+                if e.code == 404:
+                    # This likely means that one of the users doesn't exist,
+                    # so we act as if we couldn't find the profile.
+                    raise SynapseError(403, "Profile isn't available", Codes.FORBIDDEN)
+                raise
+
 
 class MasterProfileHandler(BaseProfileHandler):
     PROFILE_UPDATE_MS = 60 * 1000

synapse/rest/client/v1/profile.py

@@ -31,10 +31,16 @@ def __init__(self, hs):
 
     @defer.inlineCallbacks
     def on_GET(self, request, user_id):
+        requester_user = None
+
+        if self.hs.config.require_auth_for_profile_requests:
+            requester = yield self.auth.get_user_by_req(request)
+            requester_user = requester.user
+
         user = UserID.from_string(user_id)
 
         displayname = yield self.profile_handler.get_displayname(
-            user,
+            user, requester=requester_user,
         )
 
         ret = {}
@@ -74,10 +80,16 @@ def __init__(self, hs):
 
     @defer.inlineCallbacks
     def on_GET(self, request, user_id):
+        requester_user = None
+
+        if self.hs.config.require_auth_for_profile_requests:
+            requester = yield self.auth.get_user_by_req(request)
+            requester_user = requester.user
+
         user = UserID.from_string(user_id)
 
         avatar_url = yield self.profile_handler.get_avatar_url(
-            user,
+            user, requester=requester_user,
         )
 
         ret = {}
@@ -116,13 +128,19 @@ def __init__(self, hs):
 
     @defer.inlineCallbacks
     def on_GET(self, request, user_id):
+        requester_user = None
+
+        if self.hs.config.require_auth_for_profile_requests:
+            requester = yield self.auth.get_user_by_req(request)
+            requester_user = requester.user
+
         user = UserID.from_string(user_id)
 
         displayname = yield self.profile_handler.get_displayname(
-            user,
+            user, requester=requester_user,
         )
         avatar_url = yield self.profile_handler.get_avatar_url(
-            user,
+            user, requester=requester_user,
         )
 
         ret = {}

synapse/rest/client/v1/room.py

@@ -301,6 +301,12 @@ def on_GET(self, request):
         try:
             yield self.auth.get_user_by_req(request, allow_guest=True)
         except AuthError as e:
+            # Option to allow servers to require auth when accessing
+            # /publicRooms via CS API. This is especially helpful in private
+            # federations.
+            if self.hs.config.restrict_public_rooms_to_local_users:
+                raise
+
             # We allow people to not be authed if they're just looking at our
             # room list, but require auth when we proxy the request.
             # In both cases we call the auth function, as that has the side

tests/rest/client/v1/test_profile.py

@@ -20,7 +20,7 @@
 
 import synapse.types
 from synapse.api.errors import AuthError, SynapseError
-from synapse.rest.client.v1 import profile
+from synapse.rest.client.v1 import admin, login, profile, room
 
 from tests import unittest
 
@@ -155,3 +155,74 @@ def test_set_my_avatar(self):
         self.assertEquals(mocked_set.call_args[0][0].localpart, "1234ABCD")
         self.assertEquals(mocked_set.call_args[0][1].user.localpart, "1234ABCD")
         self.assertEquals(mocked_set.call_args[0][2], "http://my.server/pic.gif")
+
+
+class ProfilesRestrictedTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        profile.register_servlets,
+        room.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+
+        config = self.default_config()
+        config.require_auth_for_profile_requests = True
+        self.hs = self.setup_test_homeserver(config=config)
+
+        return self.hs
+
+    def prepare(self, reactor, clock, hs):
+        # User owning the requested profile.
+        self.owner = self.register_user("owner", "pass")
+        self.owner_tok = self.login("owner", "pass")
+        self.profile_url = "/profile/%s" % (self.owner)
+
+        # User requesting the profile.
+        self.requester = self.register_user("requester", "pass")
+        self.requester_tok = self.login("requester", "pass")
+
+        self.room_id = self.helper.create_room_as(self.owner, tok=self.owner_tok)
+
+    def test_no_auth(self):
+        self.try_fetch_profile(401)
+
+    def test_not_in_shared_room(self):
+        self.ensure_requester_left_room()
+
+        self.try_fetch_profile(403, self.requester_tok)
+
+    def test_in_shared_room(self):
+        self.ensure_requester_left_room()
+
+        self.helper.join(
+            room=self.room_id,
+            user=self.requester,
+            tok=self.requester_tok,
+        )
+
+        self.try_fetch_profile(200, self.requester_tok)
+
+    def try_fetch_profile(self, expected_code, access_token=None):
+        request, channel = self.make_request(
+            "GET",
+            self.profile_url,
+            access_token=access_token,
+        )
+        self.render(request)
+        self.assertEqual(channel.code, expected_code, channel.result)
+
+    def ensure_requester_left_room(self):
+        try:
+            self.helper.leave(
+                room=self.room_id,
+                user=self.requester,
+                tok=self.requester_tok,
+            )
+        except AssertionError:
+            # We don't care whether the leave request didn't return a 200 (e.g.
+            # if the user isn't already in the room), because we only want to
+            # make sure the user isn't in the room.
+            pass

tests/rest/client/v1/test_rooms.py

@@ -903,3 +903,35 @@ def test_include_context(self):
         self.assertEqual(
             context["profile_info"][self.other_user_id]["displayname"], "otheruser"
         )
+
+
+class PublicRoomsRestrictedTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        admin.register_servlets,
+        room.register_servlets,
+        login.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+
+        self.url = b"/_matrix/client/r0/publicRooms"
+
+        config = self.default_config()
+        config.restrict_public_rooms_to_local_users = True
+        self.hs = self.setup_test_homeserver(config=config)
+
+        return self.hs
+
+    def test_restricted_no_auth(self):
+        request, channel = self.make_request("GET", self.url)
+        self.render(request)
+        self.assertEqual(channel.code, 401, channel.result)
+
+    def test_restricted_auth(self):
+        self.register_user("user", "pass")
+        tok = self.login("user", "pass")
+
+        request, channel = self.make_request("GET", self.url, access_token=tok)
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)