diff --git a/changelog.d/5083.feature b/changelog.d/5083.feature
new file mode 100644
index 0000000000..55d114b3fe
--- /dev/null
+++ b/changelog.d/5083.feature
@@ -0,0 +1 @@
+Add an configuration option to require authentication on /publicRooms and /profile endpoints.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index b6b9da6e41..bdfc34c6bd 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -69,6 +69,20 @@ 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'. Note that profile data is also available via the federation
+# API, so this setting is of limited value if federation is enabled on
+# the server.
+#
+#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]
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 147a976485..8dce75c56a 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -72,6 +72,19 @@ class ServerConfig(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.
@@ -327,6 +340,20 @@ class ServerConfig(Config):
#
#use_presence: false
+ # Whether to require authentication to retrieve profile data (avatars,
+ # display names) of other users through the client API. Defaults to
+ # 'false'. Note that profile data is also available via the federation
+ # API, so this setting is of limited value if federation is enabled on
+ # the server.
+ #
+ #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]
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 452599e1a1..9030eb18c5 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -716,8 +716,17 @@ class PublicRoomList(BaseFederationServlet):
PATH = "/publicRooms"
+ def __init__(self, handler, authenticator, ratelimiter, server_name, deny_access):
+ super(PublicRoomList, self).__init__(
+ handler, authenticator, ratelimiter, server_name,
+ )
+ self.deny_access = deny_access
+
@defer.inlineCallbacks
def on_GET(self, origin, content, query):
+ if self.deny_access:
+ raise FederationDeniedError(origin)
+
limit = parse_integer_from_args(query, "limit", 0)
since_token = parse_string_from_args(query, "since", None)
include_all_networks = parse_boolean_from_args(
@@ -1417,6 +1426,7 @@ def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=N
authenticator=authenticator,
ratelimiter=ratelimiter,
server_name=hs.hostname,
+ deny_access=hs.config.restrict_public_rooms_to_local_users,
).register(resource)
if "group_server" in servlet_groups:
diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index a65c98ff5c..91fc718ff8 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -53,6 +53,7 @@ class BaseProfileHandler(BaseHandler):
@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(
@@ -283,6 +284,48 @@ class BaseProfileHandler(BaseHandler):
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 not self.hs.config.require_auth_for_profile_requests or not requester:
+ return
+
+ 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 room lists have no elements in common.
+ if requester_rooms.isdisjoint(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
diff --git a/synapse/rest/client/v1/profile.py b/synapse/rest/client/v1/profile.py
index a23edd8fe5..eac1966c5e 100644
--- a/synapse/rest/client/v1/profile.py
+++ b/synapse/rest/client/v1/profile.py
@@ -31,11 +31,17 @@ class ProfileDisplaynameRestServlet(ClientV1RestServlet):
@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,
- )
+ yield self.profile_handler.check_profile_query_allowed(user, requester_user)
+
+ displayname = yield self.profile_handler.get_displayname(user)
ret = {}
if displayname is not None:
@@ -74,11 +80,17 @@ class ProfileAvatarURLRestServlet(ClientV1RestServlet):
@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,
- )
+ yield self.profile_handler.check_profile_query_allowed(user, requester_user)
+
+ avatar_url = yield self.profile_handler.get_avatar_url(user)
ret = {}
if avatar_url is not None:
@@ -116,14 +128,18 @@ class ProfileRestServlet(ClientV1RestServlet):
@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,
- )
- avatar_url = yield self.profile_handler.get_avatar_url(
- user,
- )
+ yield self.profile_handler.check_profile_query_allowed(user, requester_user)
+
+ displayname = yield self.profile_handler.get_displayname(user)
+ avatar_url = yield self.profile_handler.get_avatar_url(user)
ret = {}
if displayname is not None:
diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 48da4d557f..fab04965cb 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -301,6 +301,12 @@ class PublicRoomListRestServlet(ClientV1RestServlet):
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
diff --git a/tests/rest/client/v1/test_profile.py b/tests/rest/client/v1/test_profile.py
index 1eab9c3bdb..5d13de11e6 100644
--- a/tests/rest/client/v1/test_profile.py
+++ b/tests/rest/client/v1/test_profile.py
@@ -20,7 +20,7 @@ from twisted.internet import defer
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
@@ -42,6 +42,7 @@ class ProfileTestCase(unittest.TestCase):
"set_displayname",
"get_avatar_url",
"set_avatar_url",
+ "check_profile_query_allowed",
]
)
@@ -155,3 +156,92 @@ class ProfileTestCase(unittest.TestCase):
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, access_token=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):
+ self.request_profile(
+ expected_code,
+ access_token=access_token
+ )
+
+ self.request_profile(
+ expected_code,
+ url_suffix="/displayname",
+ access_token=access_token,
+ )
+
+ self.request_profile(
+ expected_code,
+ url_suffix="/avatar_url",
+ access_token=access_token,
+ )
+
+ def request_profile(self, expected_code, url_suffix="", access_token=None):
+ request, channel = self.make_request(
+ "GET",
+ self.profile_url + url_suffix,
+ 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
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 521ac80f9a..28fbf6ae52 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -904,3 +904,35 @@ class RoomSearchTestCase(unittest.HomeserverTestCase):
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)
|
