From afe0a36920c91b76a3ef383c097b7e48f4c1f2d5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 20 Mar 2020 13:28:20 -0400 Subject: [PATCH] Handle review feedback. --- synapse/handlers/auth.py | 26 ++++++++---------------- synapse/rest/client/v2_alpha/account.py | 14 ++++++------- synapse/rest/client/v2_alpha/devices.py | 6 ++---- synapse/rest/client/v2_alpha/keys.py | 5 ++++- synapse/rest/client/v2_alpha/register.py | 3 +-- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 2f5739c7335a..d582b9571b03 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -129,8 +129,7 @@ def validate_user_via_ui_auth( requester: Requester, request_body: Dict[str, Any], clientip: str, - action_type, - action_id, + action_data, ): """ Checks that the user is who they claim to be, via a UI auth. @@ -146,6 +145,9 @@ def validate_user_via_ui_auth( clientip: The IP address of the client. + action_data: An opaque object that should be provided initially and at the + end to ensure the request is not modified during a session. + Returns: defer.Deferred[dict]: the parameters for this request (which may have been given only in a previous call). @@ -178,7 +180,7 @@ def validate_user_via_ui_auth( try: result, params, _ = yield self.check_auth( - flows, request_body, clientip, action_type, action_id + flows, request_body, clientip, action_data ) except LoginError: # Update the ratelimite to say we failed (`can_do_action` doesn't raise). @@ -222,8 +224,7 @@ def check_auth( flows: List[List[str]], clientdict: Dict[str, Any], clientip: str, - action_type, - action_id, + action_data, ): """ Takes a dictionary sent by the client in the login / registration @@ -292,14 +293,8 @@ def check_auth( # If ui_auth exists in the session this is a returning UI auth request. # Validate that none of the requested information has changed. if "ui_auth" not in session: - session["ui_auth"] = { - "action_type": action_type, - "action_id": action_id, - } - elif ( - session["ui_auth"]["action_type"] != action_type - or session["ui_auth"]["action_id"] != action_id - ): + session["ui_auth"] = action_data + elif session["ui_auth"] != action_data: raise SynapseError(403, "Foobar") if not authdict: @@ -529,10 +524,7 @@ def _invalidate_session(self, session_id) -> None: if session and "ui_auth" in session: # Set the items in the ui_auth session to sentinel values that can # never be equaled. - session["ui_auth"] = { - "action_type": object(), - "action_id": object(), - } + session["ui_auth"] = object() @defer.inlineCallbacks def get_access_token_for_user_id( diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 780f862163d6..b94066b444c7 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -237,8 +237,7 @@ async def on_POST(self, request): requester, body, self.hs.get_ip_from_request(request), - "modify_password", - "", # TODO + {"operation": "modify_password", "user": requester.user.to_string()}, ) user_id = requester.user.to_string() else: @@ -247,8 +246,7 @@ async def on_POST(self, request): [[LoginType.EMAIL_IDENTITY]], body, self.hs.get_ip_from_request(request), - "modify_password", - "", # TODO + {"operation": "modify_password"}, # TODO ) if LoginType.EMAIL_IDENTITY in result: @@ -319,8 +317,7 @@ async def on_POST(self, request): requester, body, self.hs.get_ip_from_request(request), - "deactivate", - requester.user.to_string(), + {"operation": "deactivate", "user": requester.user.to_string()}, ) result = await self._deactivate_account_handler.deactivate_account( requester.user.to_string(), erase, id_server=body.get("id_server") @@ -668,7 +665,10 @@ async def on_POST(self, request): assert_valid_client_secret(client_secret) await self.auth_handler.validate_user_via_ui_auth( - requester, body, self.hs.get_ip_from_request(request), "add_3pid", user_id + requester, + body, + self.hs.get_ip_from_request(request), + {"operation": "add_3pid", "user": user_id}, ) validation_session = await self.identity_handler.validate_threepid_session( diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index 2ef5ec19a63a..6fe508c11b3d 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -84,8 +84,7 @@ async def on_POST(self, request): requester, body, self.hs.get_ip_from_request(request), - "delete_devices", - "", # TODO + {"operation": "delete_devices", "devices": body["devices"]}, ) await self.device_handler.delete_devices( @@ -134,8 +133,7 @@ async def on_DELETE(self, request, device_id): requester, body, self.hs.get_ip_from_request(request), - "delete_device", - device_id, + {"operation": "delete_device", "device": device_id}, ) await self.device_handler.delete_device(requester.user.to_string(), device_id) diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 0e9cf8172937..07a983e0ff4d 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -263,7 +263,10 @@ async def on_POST(self, request): body = parse_json_object_from_request(request) await self.auth_handler.validate_user_via_ui_auth( - requester, body, self.hs.get_ip_from_request(request), "add_keys", user_id + requester, + body, + self.hs.get_ip_from_request(request), + {"operation": "add_keys", "user": user_id}, ) result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 950177075ec2..d916d2b1266f 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -502,8 +502,7 @@ async def on_POST(self, request): self._registration_flows, body, self.hs.get_ip_from_request(request), - "register", - "", # TODO + {"operation": "register"}, ) # Check that we're not trying to register a denied 3pid.