From 4ce615e58cee458cf51f2421aeeb575ffe44bb25 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 6 Sep 2023 16:55:00 +0100 Subject: [PATCH 1/3] Refactor: Merge the two except blocks --- synapse/rest/client/account.py | 100 +++++++++++++++------------------ 1 file changed, 44 insertions(+), 56 deletions(-) diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 679ab9f266c7..27097294ff4e 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -179,76 +179,64 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: # # In the second case, we require a password to confirm their identity. - requester = None - if self.auth.has_access_token(request): - requester = await self.auth.get_user_by_req(request) - try: + try: + requester = None + if self.auth.has_access_token(request): + requester = await self.auth.get_user_by_req(request) params, session_id = await self.auth_handler.validate_user_via_ui_auth( requester, request, body.dict(exclude_unset=True), "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: - new_password_hash = await self.auth_handler.hash(new_password) - await self.auth_handler.set_session_data( - e.session_id, - UIAuthSessionDataConstants.PASSWORD_HASH, - new_password_hash, - ) - raise - user_id = requester.user.to_string() - else: - try: + user_id = requester.user.to_string() + else: result, params, session_id = await self.auth_handler.check_ui_auth( [[LoginType.EMAIL_IDENTITY]], request, body.dict(exclude_unset=True), "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: - new_password_hash = await self.auth_handler.hash(new_password) - await self.auth_handler.set_session_data( - e.session_id, - UIAuthSessionDataConstants.PASSWORD_HASH, - new_password_hash, + + if LoginType.EMAIL_IDENTITY in result: + threepid = result[LoginType.EMAIL_IDENTITY] + if "medium" not in threepid or "address" not in threepid: + raise SynapseError(500, "Malformed threepid") + if threepid["medium"] == "email": + # For emails, canonicalise the address. + # We store all email addresses canonicalised in the DB. + # (See add_threepid in synapse/handlers/auth.py) + try: + threepid["address"] = validate_email(threepid["address"]) + except ValueError as e: + raise SynapseError(400, str(e)) + # if using email, we must know about the email they're authing with! + threepid_user_id = await self.datastore.get_user_id_by_threepid( + threepid["medium"], threepid["address"] ) - raise - - if LoginType.EMAIL_IDENTITY in result: - threepid = result[LoginType.EMAIL_IDENTITY] - if "medium" not in threepid or "address" not in threepid: - raise SynapseError(500, "Malformed threepid") - if threepid["medium"] == "email": - # For emails, canonicalise the address. - # We store all email addresses canonicalised in the DB. - # (See add_threepid in synapse/handlers/auth.py) - try: - threepid["address"] = validate_email(threepid["address"]) - except ValueError as e: - raise SynapseError(400, str(e)) - # if using email, we must know about the email they're authing with! - threepid_user_id = await self.datastore.get_user_id_by_threepid( - threepid["medium"], threepid["address"] + if not threepid_user_id: + raise SynapseError( + 404, "Email address not found", Codes.NOT_FOUND + ) + user_id = threepid_user_id + else: + logger.error("Auth succeeded but no known type! %r", result.keys()) + raise SynapseError(500, "", Codes.UNKNOWN) + + 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: + new_password_hash = await self.auth_handler.hash(new_password) + await self.auth_handler.set_session_data( + e.session_id, + UIAuthSessionDataConstants.PASSWORD_HASH, + new_password_hash, ) - if not threepid_user_id: - raise SynapseError(404, "Email address not found", Codes.NOT_FOUND) - user_id = threepid_user_id - else: - logger.error("Auth succeeded but no known type! %r", result.keys()) - raise SynapseError(500, "", Codes.UNKNOWN) + raise # If we have a password in this request, prefer it. Otherwise, use the # password hash from an earlier request. From 2a4b0d07f9956e5d11d62e2107d1f0e1ac5629f3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 6 Sep 2023 16:56:12 +0100 Subject: [PATCH 2/3] Don't keep hashing the password --- synapse/rest/client/account.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 27097294ff4e..196b292890dc 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -228,14 +228,24 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: # 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: - new_password_hash = await self.auth_handler.hash(new_password) - await self.auth_handler.set_session_data( - e.session_id, - UIAuthSessionDataConstants.PASSWORD_HASH, - new_password_hash, - ) + # store it for later. We only do this if we don't already have the + # password hash stored, to avoid repeatedly hashing the password. + + if not new_password: + raise + + existing_session_password_hash = await self.auth_handler.get_session_data( + e.session_id, UIAuthSessionDataConstants.PASSWORD_HASH, None + ) + if existing_session_password_hash: + raise + + new_password_hash = await self.auth_handler.hash(new_password) + await self.auth_handler.set_session_data( + e.session_id, + UIAuthSessionDataConstants.PASSWORD_HASH, + new_password_hash, + ) raise # If we have a password in this request, prefer it. Otherwise, use the @@ -243,9 +253,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: if new_password: password_hash: Optional[str] = await self.auth_handler.hash(new_password) elif session_id is not None: - password_hash = await self.auth_handler.get_session_data( - session_id, UIAuthSessionDataConstants.PASSWORD_HASH, None - ) + password_hash = existing_session_password_hash else: # UI validation was skipped, but the request did not include a new # password. From 102113fc9fac32201ca6dbc0dfbf92dc60904d69 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 6 Sep 2023 17:01:27 +0100 Subject: [PATCH 3/3] Newsifle --- changelog.d/16264.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16264.misc diff --git a/changelog.d/16264.misc b/changelog.d/16264.misc new file mode 100644 index 000000000000..a744434bef06 --- /dev/null +++ b/changelog.d/16264.misc @@ -0,0 +1 @@ +Reduce CPU overhead of change password endpoint.