From 54f9df776413a0a3f113f0698827f5a216ea8d4a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jan 2021 12:09:53 +0000 Subject: [PATCH 1/3] Remove redundant `clientip` param from UIAuth methods since this is always `request.getClientIP()`, and we have the request, we may as well elide the separate param. --- synapse/handlers/auth.py | 9 ++------- synapse/rest/client/v2_alpha/account.py | 19 +++---------------- synapse/rest/client/v2_alpha/devices.py | 12 ++---------- synapse/rest/client/v2_alpha/keys.py | 6 +----- synapse/rest/client/v2_alpha/register.py | 6 +----- 5 files changed, 9 insertions(+), 43 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index f4434673dcc6..6c2ab29dd6b4 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -284,7 +284,6 @@ async def validate_user_via_ui_auth( requester: Requester, request: SynapseRequest, request_body: Dict[str, Any], - clientip: str, description: str, ) -> Tuple[dict, Optional[str]]: """ @@ -301,8 +300,6 @@ async def validate_user_via_ui_auth( request_body: The body of the request sent by the client - clientip: The IP address of the client. - description: A human readable string to be displayed to the user that describes the operation happening on their account. @@ -351,7 +348,7 @@ async def validate_user_via_ui_auth( try: result, params, session_id = await self.check_ui_auth( - flows, request, request_body, clientip, description + flows, request, request_body, description ) except LoginError: # Update the ratelimiter to say we failed (`can_do_action` doesn't raise). @@ -426,7 +423,6 @@ async def check_ui_auth( flows: List[List[str]], request: SynapseRequest, clientdict: Dict[str, Any], - clientip: str, description: str, ) -> Tuple[dict, dict, str]: """ @@ -448,8 +444,6 @@ async def check_ui_auth( clientdict: The dictionary from the client root level, not the 'auth' key: this method prompts for auth if none is sent. - clientip: The IP address of the client. - description: A human readable string to be displayed to the user that describes the operation happening on their account. @@ -540,6 +534,7 @@ async def check_ui_auth( await self.store.set_ui_auth_clientdict(sid, clientdict) user_agent = request.get_user_agent("") + clientip = request.getClientIP() await self.store.add_user_agent_ip_to_ui_auth_session( session.session_id, user_agent, clientip diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index d837bde1d66e..7f3445fe5d67 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -189,11 +189,7 @@ async def on_POST(self, request): requester = await self.auth.get_user_by_req(request) try: params, session_id = await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "modify your account password", + requester, request, body, "modify your account password", ) except InteractiveAuthIncompleteError as e: # The user needs to provide more steps to complete auth, but @@ -215,7 +211,6 @@ async def on_POST(self, request): [[LoginType.EMAIL_IDENTITY]], request, body, - self.hs.get_ip_from_request(request), "modify your account password", ) except InteractiveAuthIncompleteError as e: @@ -309,11 +304,7 @@ async def on_POST(self, request): return 200, {} await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "deactivate your account", + requester, request, body, "deactivate your account", ) result = await self._deactivate_account_handler.deactivate_account( requester.user.to_string(), erase, id_server=body.get("id_server") @@ -695,11 +686,7 @@ async def on_POST(self, request): assert_valid_client_secret(client_secret) await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "add a third-party identifier to your account", + requester, request, body, "add a third-party identifier to your account", ) 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 af117cb27c16..314e01dfe443 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -83,11 +83,7 @@ async def on_POST(self, request): assert_params_in_dict(body, ["devices"]) await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "remove device(s) from your account", + requester, request, body, "remove device(s) from your account", ) await self.device_handler.delete_devices( @@ -133,11 +129,7 @@ async def on_DELETE(self, request, device_id): raise await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "remove a device from your account", + requester, request, body, "remove a device from your account", ) 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 b91996c7387d..a6134ead8a70 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -271,11 +271,7 @@ async def on_POST(self, request): body = parse_json_object_from_request(request) await self.auth_handler.validate_user_via_ui_auth( - requester, - request, - body, - self.hs.get_ip_from_request(request), - "add a device signing key to your account", + requester, request, body, "add a device signing key to your account", ) 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 6b5a1b710965..672bea38b696 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -513,11 +513,7 @@ async def on_POST(self, request): # not this will raise a user-interactive auth error. try: auth_result, params, session_id = await self.auth_handler.check_ui_auth( - self._registration_flows, - request, - body, - self.hs.get_ip_from_request(request), - "register a new account", + self._registration_flows, request, body, "register a new account", ) except InteractiveAuthIncompleteError as e: # The user needs to provide more steps to complete auth. From 4812821ef0bf9f9819fd3bf2ab5176da11692241 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jan 2021 12:12:24 +0000 Subject: [PATCH 2/3] Inline remaining calls to `get_ip_from_request` This thing is redundant. --- synapse/api/auth.py | 4 ++-- synapse/rest/client/v2_alpha/auth.py | 4 ++-- synapse/rest/client/v2_alpha/register.py | 2 +- synapse/server.py | 4 ---- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 48c4d7b0be2e..22ddd75e025b 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -186,7 +186,7 @@ async def get_user_by_req( AuthError if access is denied for the user in the access token """ try: - ip_addr = self.hs.get_ip_from_request(request) + ip_addr = request.getClientIP() user_agent = request.get_user_agent("") access_token = self.get_access_token_from_request(request) @@ -275,7 +275,7 @@ async def _get_appservice_user_id(self, request): return None, None if app_service.ip_range_whitelist: - ip_address = IPAddress(self.hs.get_ip_from_request(request)) + ip_address = IPAddress(request.getClientIP()) if ip_address not in app_service.ip_range_whitelist: return None, None diff --git a/synapse/rest/client/v2_alpha/auth.py b/synapse/rest/client/v2_alpha/auth.py index 9b9514632fac..149697fc23ca 100644 --- a/synapse/rest/client/v2_alpha/auth.py +++ b/synapse/rest/client/v2_alpha/auth.py @@ -128,7 +128,7 @@ async def on_POST(self, request, stagetype): authdict = {"response": response, "session": session} success = await self.auth_handler.add_oob_auth( - LoginType.RECAPTCHA, authdict, self.hs.get_ip_from_request(request) + LoginType.RECAPTCHA, authdict, request.getClientIP() ) if success: @@ -144,7 +144,7 @@ async def on_POST(self, request, stagetype): authdict = {"session": session} success = await self.auth_handler.add_oob_auth( - LoginType.TERMS, authdict, self.hs.get_ip_from_request(request) + LoginType.TERMS, authdict, request.getClientIP() ) if success: diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 672bea38b696..35e646390ecb 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -353,7 +353,7 @@ async def on_GET(self, request): 403, "Registration has been disabled", errcode=Codes.FORBIDDEN ) - ip = self.hs.get_ip_from_request(request) + ip = request.getClientIP() with self.ratelimiter.ratelimit(ip) as wait_deferred: await wait_deferred diff --git a/synapse/server.py b/synapse/server.py index a198b0eb464e..12da92b63cfe 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -283,10 +283,6 @@ def get_reactor(self) -> twisted.internet.base.ReactorBase: """ return self._reactor - def get_ip_from_request(self, request) -> str: - # X-Forwarded-For is handled by our custom request type. - return request.getClientIP() - def is_mine(self, domain_specific_string: DomainSpecificString) -> bool: return domain_specific_string.domain == self.hostname From 5e6deb8d7ecc1cfdc4dfcf6e5453364c440bb4fd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 12 Jan 2021 12:13:30 +0000 Subject: [PATCH 3/3] changelog --- changelog.d/9080.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9080.misc diff --git a/changelog.d/9080.misc b/changelog.d/9080.misc new file mode 100644 index 000000000000..3da8171f5fef --- /dev/null +++ b/changelog.d/9080.misc @@ -0,0 +1 @@ +Remove redundant `Homeserver.get_ip_from_request` method.