Matrix-Mirrors/synapse-product
1
0
Fork 0
mirror of https://git.anonymousland.org/anonymousland/synapse-product.git synced 2024-10-01 08:25:44 -04:00
Code Issues Packages Projects Releases Wiki Activity

Improve performance of the register endpoint (#8009)

Browse Source
This commit is contained in:
Patrick Cloke 2020-08-06 08:09:55 -04:00 committed by GitHub
parent 079bc3c8e3
commit 66f24449dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 146 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/8009.misc Normal file
View File

@ -0,0 +1 @@
Improve the performance of the register endpoint.

4
synapse/api/errors.py
View File

@ -238,14 +238,16 @@ class InteractiveAuthIncompleteError(Exception):
(This indicates we should return a 401 with 'result' as the body) (This indicates we should return a 401 with 'result' as the body)
Attributes: Attributes:
session_id: The ID of the ongoing interactive auth session.
result: the server response to the request, which should be result: the server response to the request, which should be
passed back to the client passed back to the client
""" """
def __init__(self, result: "JsonDict"): def __init__(self, session_id: str, result: "JsonDict"):
super(InteractiveAuthIncompleteError, self).__init__( super(InteractiveAuthIncompleteError, self).__init__(
"Interactive auth not yet complete" "Interactive auth not yet complete"
) )
self.session_id = session_id
self.result = result self.result = result

19
synapse/handlers/auth.py
View File

@ -162,7 +162,7 @@ class AuthHandler(BaseHandler):
request_body: Dict[str, Any], request_body: Dict[str, Any],
clientip: str, clientip: str,
description: str, description: str,
) -> dict: ) -> Tuple[dict, str]:
""" """
Checks that the user is who they claim to be, via a UI auth. Checks that the user is who they claim to be, via a UI auth.
@ -183,9 +183,14 @@ class AuthHandler(BaseHandler):
describes the operation happening on their account. describes the operation happening on their account.
Returns: Returns:
The parameters for this request (which may A tuple of (params, session_id).
'params' contains the parameters for this request (which may
have been given only in a previous call). have been given only in a previous call).
'session_id' is the ID of this session, either passed in by the
client or assigned by this call
Raises: Raises:
InteractiveAuthIncompleteError if the client has not yet completed InteractiveAuthIncompleteError if the client has not yet completed
any of the permitted login flows any of the permitted login flows
@ -207,7 +212,7 @@ class AuthHandler(BaseHandler):
flows = [[login_type] for login_type in self._supported_ui_auth_types] flows = [[login_type] for login_type in self._supported_ui_auth_types]
try: try:
result, params, _ = await self.check_auth( result, params, session_id = await self.check_ui_auth(
flows, request, request_body, clientip, description flows, request, request_body, clientip, description
) )
except LoginError: except LoginError:
@ -230,7 +235,7 @@ class AuthHandler(BaseHandler):
if user_id != requester.user.to_string(): if user_id != requester.user.to_string():
raise AuthError(403, "Invalid auth") raise AuthError(403, "Invalid auth")
return params return params, session_id
def get_enabled_auth_types(self): def get_enabled_auth_types(self):
"""Return the enabled user-interactive authentication types """Return the enabled user-interactive authentication types
@ -240,7 +245,7 @@ class AuthHandler(BaseHandler):
""" """
return self.checkers.keys() return self.checkers.keys()
async def check_auth( async def check_ui_auth(
self, self,
flows: List[List[str]], flows: List[List[str]],
request: SynapseRequest, request: SynapseRequest,
@ -363,7 +368,7 @@ class AuthHandler(BaseHandler):
if not authdict: if not authdict:
raise InteractiveAuthIncompleteError( raise InteractiveAuthIncompleteError(
self._auth_dict_for_flows(flows, session.session_id) session.session_id, self._auth_dict_for_flows(flows, session.session_id)
) )
# check auth type currently being presented # check auth type currently being presented
@ -410,7 +415,7 @@ class AuthHandler(BaseHandler):
ret = self._auth_dict_for_flows(flows, session.session_id) ret = self._auth_dict_for_flows(flows, session.session_id)
ret["completed"] = list(creds) ret["completed"] = list(creds)
ret.update(errordict) ret.update(errordict)
raise InteractiveAuthIncompleteError(ret) raise InteractiveAuthIncompleteError(session.session_id, ret)
async def add_oob_auth( async def add_oob_auth(
self, stagetype: str, authdict: Dict[str, Any], clientip: str self, stagetype: str, authdict: Dict[str, Any], clientip: str

86
synapse/rest/client/v2_alpha/account.py
View File

@ -18,7 +18,12 @@ import logging
from http import HTTPStatus from http import HTTPStatus
from synapse.api.constants import LoginType from synapse.api.constants import LoginType
from synapse.api.errors import Codes, SynapseError, ThreepidValidationError from synapse.api.errors import (
Codes,
InteractiveAuthIncompleteError,
SynapseError,
ThreepidValidationError,
)
from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.emailconfig import ThreepidBehaviour
from synapse.http.server import finish_request, respond_with_html from synapse.http.server import finish_request, respond_with_html
from synapse.http.servlet import ( from synapse.http.servlet import (
@ -239,18 +244,12 @@ class PasswordRestServlet(RestServlet):
# we do basic sanity checks here because the auth layer will store these # we do basic sanity checks here because the auth layer will store these
# in sessions. Pull out the new password provided to us. # in sessions. Pull out the new password provided to us.
if "new_password" in body: new_password = body.pop("new_password", None)
new_password = body.pop("new_password") if new_password is not None:
if not isinstance(new_password, str) or len(new_password) > 512: if not isinstance(new_password, str) or len(new_password) > 512:
raise SynapseError(400, "Invalid password") raise SynapseError(400, "Invalid password")
self.password_policy_handler.validate_password(new_password) self.password_policy_handler.validate_password(new_password)
# If the password is valid, hash it and store it back on the body.
# This ensures that only the hashed password is handled everywhere.
if "new_password_hash" in body:
raise SynapseError(400, "Unexpected property: new_password_hash")
body["new_password_hash"] = await self.auth_handler.hash(new_password)
# there are two possibilities here. Either the user does not have an # there are two possibilities here. Either the user does not have an
# access token, and needs to do a password reset; or they have one and # access token, and needs to do a password reset; or they have one and
# need to validate their identity. # need to validate their identity.
@ -263,23 +262,49 @@ class PasswordRestServlet(RestServlet):
if self.auth.has_access_token(request): if self.auth.has_access_token(request):
requester = await self.auth.get_user_by_req(request) requester = await self.auth.get_user_by_req(request)
params = await self.auth_handler.validate_user_via_ui_auth( try:
requester, params, session_id = await self.auth_handler.validate_user_via_ui_auth(
request, requester,
body, request,
self.hs.get_ip_from_request(request), body,
"modify your account password", self.hs.get_ip_from_request(request),
) "modify your account password",
)
except InteractiveAuthIncompleteError as e:
# The user needs to provide more steps to complete auth, but
# they're not required to provide the password again.
#
# If a password is available now, hash the provided password and
# store it for later.
if new_password:
password_hash = await self.auth_handler.hash(new_password)
await self.auth_handler.set_session_data(
e.session_id, "password_hash", password_hash
)
raise
user_id = requester.user.to_string() user_id = requester.user.to_string()
else: else:
requester = None requester = None
result, params, _ = await self.auth_handler.check_auth( try:
[[LoginType.EMAIL_IDENTITY]], result, params, session_id = await self.auth_handler.check_ui_auth(
request, [[LoginType.EMAIL_IDENTITY]],
body, request,
self.hs.get_ip_from_request(request), body,
"modify your account password", self.hs.get_ip_from_request(request),
) "modify your account password",
)
except InteractiveAuthIncompleteError as e:
# The user needs to provide more steps to complete auth, but
# they're not required to provide the password again.
#
# If a password is available now, hash the provided password and
# store it for later.
if new_password:
password_hash = await self.auth_handler.hash(new_password)
await self.auth_handler.set_session_data(
e.session_id, "password_hash", password_hash
)
raise
if LoginType.EMAIL_IDENTITY in result: if LoginType.EMAIL_IDENTITY in result:
threepid = result[LoginType.EMAIL_IDENTITY] threepid = result[LoginType.EMAIL_IDENTITY]
@ -304,12 +329,21 @@ class PasswordRestServlet(RestServlet):
logger.error("Auth succeeded but no known type! %r", result.keys()) logger.error("Auth succeeded but no known type! %r", result.keys())
raise SynapseError(500, "", Codes.UNKNOWN) raise SynapseError(500, "", Codes.UNKNOWN)
assert_params_in_dict(params, ["new_password_hash"]) # If we have a password in this request, prefer it. Otherwise, there
new_password_hash = params["new_password_hash"] # must be a password hash from an earlier request.
if new_password:
password_hash = await self.auth_handler.hash(new_password)
else:
password_hash = await self.auth_handler.get_session_data(
session_id, "password_hash", None
)
if not password_hash:
raise SynapseError(400, "Missing params: password", Codes.MISSING_PARAM)
logout_devices = params.get("logout_devices", True) logout_devices = params.get("logout_devices", True)
await self._set_password_handler.set_password( await self._set_password_handler.set_password(
user_id, new_password_hash, logout_devices, requester user_id, password_hash, logout_devices, requester
) )
return 200, {} return 200, {}

108
synapse/rest/client/v2_alpha/register.py
View File

@ -24,6 +24,7 @@ import synapse.types
from synapse.api.constants import LoginType from synapse.api.constants import LoginType
from synapse.api.errors import ( from synapse.api.errors import (
Codes, Codes,
InteractiveAuthIncompleteError,
SynapseError, SynapseError,
ThreepidValidationError, ThreepidValidationError,
UnrecognizedRequestError, UnrecognizedRequestError,
@ -387,6 +388,7 @@ class RegisterRestServlet(RestServlet):
self.ratelimiter = hs.get_registration_ratelimiter() self.ratelimiter = hs.get_registration_ratelimiter()
self.password_policy_handler = hs.get_password_policy_handler() self.password_policy_handler = hs.get_password_policy_handler()
self.clock = hs.get_clock() self.clock = hs.get_clock()
self._registration_enabled = self.hs.config.enable_registration
self._registration_flows = _calculate_registration_flows( self._registration_flows = _calculate_registration_flows(
hs.config, self.auth_handler hs.config, self.auth_handler
@ -412,20 +414,8 @@ class RegisterRestServlet(RestServlet):
"Do not understand membership kind: %s" % (kind.decode("utf8"),) "Do not understand membership kind: %s" % (kind.decode("utf8"),)
) )
# we do basic sanity checks here because the auth layer will store these # Pull out the provided username and do basic sanity checks early since
# in sessions. Pull out the username/password provided to us. # the auth layer will store these in sessions.
if "password" in body:
password = body.pop("password")
if not isinstance(password, str) or len(password) > 512:
raise SynapseError(400, "Invalid password")
self.password_policy_handler.validate_password(password)
# If the password is valid, hash it and store it back on the body.
# This ensures that only the hashed password is handled everywhere.
if "password_hash" in body:
raise SynapseError(400, "Unexpected property: password_hash")
body["password_hash"] = await self.auth_handler.hash(password)
desired_username = None desired_username = None
if "username" in body: if "username" in body:
if not isinstance(body["username"], str) or len(body["username"]) > 512: if not isinstance(body["username"], str) or len(body["username"]) > 512:
@ -459,22 +449,35 @@ class RegisterRestServlet(RestServlet):
) )
return 200, result # we throw for non 200 responses return 200, result # we throw for non 200 responses
# for regular registration, downcase the provided username before # == Normal User Registration == (everyone else)
# attempting to register it. This should mean if not self._registration_enabled:
# that people who try to register with upper-case in their usernames raise SynapseError(403, "Registration has been disabled")
# don't get a nasty surprise. (Note that we treat username
# case-insenstively in login, so they are free to carry on imagining # For regular registration, convert the provided username to lowercase
# that their username is CrAzYh4cKeR if that keeps them happy) # before attempting to register it. This should mean that people who try
# to register with upper-case in their usernames don't get a nasty surprise.
#
# Note that we treat usernames case-insensitively in login, so they are
# free to carry on imagining that their username is CrAzYh4cKeR if that
# keeps them happy.
if desired_username is not None: if desired_username is not None:
desired_username = desired_username.lower() desired_username = desired_username.lower()
# == Normal User Registration == (everyone else) # Check if this account is upgrading from a guest account.
if not self.hs.config.enable_registration:
raise SynapseError(403, "Registration has been disabled")
guest_access_token = body.get("guest_access_token", None) guest_access_token = body.get("guest_access_token", None)
if "initial_device_display_name" in body and "password_hash" not in body: # Pull out the provided password and do basic sanity checks early.
#
# Note that we remove the password from the body since the auth layer
# will store the body in the session and we don't want a plaintext
# password store there.
password = body.pop("password", None)
if password is not None:
if not isinstance(password, str) or len(password) > 512:
raise SynapseError(400, "Invalid password")
self.password_policy_handler.validate_password(password)
if "initial_device_display_name" in body and password is None:
# ignore 'initial_device_display_name' if sent without # ignore 'initial_device_display_name' if sent without
# a password to work around a client bug where it sent # a password to work around a client bug where it sent
# the 'initial_device_display_name' param alone, wiping out # the 'initial_device_display_name' param alone, wiping out
@ -484,6 +487,7 @@ class RegisterRestServlet(RestServlet):
session_id = self.auth_handler.get_session_id(body) session_id = self.auth_handler.get_session_id(body)
registered_user_id = None registered_user_id = None
password_hash = None
if session_id: if session_id:
# if we get a registered user id out of here, it means we previously # if we get a registered user id out of here, it means we previously
# registered a user for this session, so we could just return the # registered a user for this session, so we could just return the
@ -492,7 +496,12 @@ class RegisterRestServlet(RestServlet):
registered_user_id = await self.auth_handler.get_session_data( registered_user_id = await self.auth_handler.get_session_data(
session_id, "registered_user_id", None session_id, "registered_user_id", None
) )
# Extract the previously-hashed password from the session.
password_hash = await self.auth_handler.get_session_data(
session_id, "password_hash", None
)
# Ensure that the username is valid.
if desired_username is not None: if desired_username is not None:
await self.registration_handler.check_username( await self.registration_handler.check_username(
desired_username, desired_username,
@ -500,20 +509,38 @@ class RegisterRestServlet(RestServlet):
assigned_user_id=registered_user_id, assigned_user_id=registered_user_id,
) )
auth_result, params, session_id = await self.auth_handler.check_auth( # Check if the user-interactive authentication flows are complete, if
self._registration_flows, # not this will raise a user-interactive auth error.
request, try:
body, auth_result, params, session_id = await self.auth_handler.check_ui_auth(
self.hs.get_ip_from_request(request), self._registration_flows,
"register a new account", request,
) body,
self.hs.get_ip_from_request(request),
"register a new account",
)
except InteractiveAuthIncompleteError as e:
# The user needs to provide more steps to complete auth.
#
# Hash the password and store it with the session since the client
# is not required to provide the password again.
#
# If a password hash was previously stored we will not attempt to
# re-hash and store it for efficiency. This assumes the password
# does not change throughout the authentication flow, but this
# should be fine since the data is meant to be consistent.
if not password_hash and password:
password_hash = await self.auth_handler.hash(password)
await self.auth_handler.set_session_data(
e.session_id, "password_hash", password_hash
)
raise
# Check that we're not trying to register a denied 3pid. # Check that we're not trying to register a denied 3pid.
# #
# the user-facing checks will probably already have happened in # the user-facing checks will probably already have happened in
# /register/email/requestToken when we requested a 3pid, but that's not # /register/email/requestToken when we requested a 3pid, but that's not
# guaranteed. # guaranteed.
if auth_result: if auth_result:
for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]: for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]:
if login_type in auth_result: if login_type in auth_result:
@ -535,12 +562,15 @@ class RegisterRestServlet(RestServlet):
# don't re-register the threepids # don't re-register the threepids
registered = False registered = False
else: else:
# NB: This may be from the auth handler and NOT from the POST # If we have a password in this request, prefer it. Otherwise, there
assert_params_in_dict(params, ["password_hash"]) # might be a password hash from an earlier request.
if password:
password_hash = await self.auth_handler.hash(password)
if not password_hash:
raise SynapseError(400, "Missing params: password", Codes.MISSING_PARAM)
desired_username = params.get("username", None) desired_username = params.get("username", None)
guest_access_token = params.get("guest_access_token", None) guest_access_token = params.get("guest_access_token", None)
new_password_hash = params.get("password_hash", None)
if desired_username is not None: if desired_username is not None:
desired_username = desired_username.lower() desired_username = desired_username.lower()
@ -582,7 +612,7 @@ class RegisterRestServlet(RestServlet):
registered_user_id = await self.registration_handler.register_user( registered_user_id = await self.registration_handler.register_user(
localpart=desired_username, localpart=desired_username,
password_hash=new_password_hash, password_hash=password_hash,
guest_access_token=guest_access_token, guest_access_token=guest_access_token,
threepid=threepid, threepid=threepid,
address=client_addr, address=client_addr,
@ -595,8 +625,8 @@ class RegisterRestServlet(RestServlet):
): ):
await self.store.upsert_monthly_active_user(registered_user_id) await self.store.upsert_monthly_active_user(registered_user_id)
# remember that we've now registered that user account, and with # Remember that the user account has been registered (and the user
# what user ID (since the user may not have specified) # ID it was registered with, since it might not have been specified).
await self.auth_handler.set_session_data( await self.auth_handler.set_session_data(
session_id, "registered_user_id", registered_user_id session_id, "registered_user_id", registered_user_id
) )

2
tests/rest/client/v2_alpha/test_register.py
View File

@ -116,8 +116,8 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
self.assertEquals(channel.result["code"], b"200", channel.result) self.assertEquals(channel.result["code"], b"200", channel.result)
self.assertDictContainsSubset(det_data, channel.json_body) self.assertDictContainsSubset(det_data, channel.json_body)
@override_config({"enable_registration": False})
def test_POST_disabled_registration(self): def test_POST_disabled_registration(self):
self.hs.config.enable_registration = False
request_data = json.dumps({"username": "kermit", "password": "monkey"}) request_data = json.dumps({"username": "kermit", "password": "monkey"})
self.auth_result = (None, {"username": "kermit", "password": "monkey"}, None) self.auth_result = (None, {"username": "kermit", "password": "monkey"}, None)