add options to require an access_token to GET /profile and /publicRooms on CS API (#5083)
This commit adds two config options: * `restrict_public_rooms_to_local_users` Requires auth to fetch the public rooms directory through the CS API and disables fetching it through the federation API. * `require_auth_for_profile_requests` When set to `true`, requires that requests to `/profile` over the CS API are authenticated, and only returns the user's profile if the requester shares a room with the profile's owner, as per MSC1301. MSC1301 also specifies a behaviour for federation (only returning the profile if the server asking for it shares a room with the profile's owner), but that's currently really non-trivial to do in a not too expensive way. Next step is writing down a MSC that allows a HS to specify which user sent the profile query. In this implementation, Synapse won't send a profile query over federation if it doesn't believe it already shares a room with the profile's owner, though. Groups have been intentionally omitted from this commit.pull/5154/head
Matthew Hodgson
Brendan Abolivier
parent
c8c069db92
commit
c0e0740bef
|
|
@ -0,0 +1 @@
|
|||
Add an configuration option to require authentication on /publicRooms and /profile endpoints.
|
||||
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue