From 1b47dedace44ec281a071da89c8f7a1bd7a1d510 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 10 Feb 2023 17:47:58 +0000 Subject: [PATCH 1/5] Break up dict arguments into their own args This is just generally cleaner, and prevent footguns of modifying dict arguments, which are passed by reference. --- synapse/handlers/identity.py | 43 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 848e46eb9ba6..eaac5fcdd0ac 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -219,28 +219,31 @@ async def bind_threepid( data = json_decoder.decode(e.msg) # XXX WAT? return data - async def try_unbind_threepid(self, mxid: str, threepid: dict) -> bool: - """Attempt to remove a 3PID from an identity server, or if one is not provided, all - identity servers we're aware the binding is present on + async def try_unbind_threepid( + self, mxid: str, medium: str, address: str, id_server: Optional[str] + ) -> bool: + """Attempt to remove a 3PID from one or more identity servers. Args: mxid: Matrix user ID of binding to be removed - threepid: Dict with medium & address of binding to be - removed, and an optional id_server. + medium: The medium of the third-party ID. + address: The address of the third-party ID. + id_server: An identity server to attempt to unbind from. If None, + attempt to remove the association from all identity servers + known to potentially have it. Raises: - SynapseError: If we failed to contact the identity server + SynapseError: If we failed to contact one or more identity servers. Returns: - True on success, otherwise False if the identity - server doesn't support unbinding (or no identity server found to - contact). + True on success, otherwise False if the identity server doesn't + support unbinding (or no identity server to contact was found). """ - if threepid.get("id_server"): - id_servers = [threepid["id_server"]] + if id_server: + id_servers = [id_server] else: id_servers = await self.store.get_id_servers_user_bound( - user_id=mxid, medium=threepid["medium"], address=threepid["address"] + mxid, medium, address ) # We don't know where to unbind, so we don't have a choice but to return @@ -250,19 +253,20 @@ async def try_unbind_threepid(self, mxid: str, threepid: dict) -> bool: changed = True for id_server in id_servers: changed &= await self.try_unbind_threepid_with_id_server( - mxid, threepid, id_server + mxid, medium, address, id_server ) return changed async def try_unbind_threepid_with_id_server( - self, mxid: str, threepid: dict, id_server: str + self, mxid: str, medium: str, address: str, id_server: str ) -> bool: """Removes a binding from an identity server Args: mxid: Matrix user ID of binding to be removed - threepid: Dict with medium & address of binding to be removed + medium: The medium of the third-party ID + address: The address of the third-party ID id_server: Identity server to unbind from Raises: @@ -286,7 +290,7 @@ async def try_unbind_threepid_with_id_server( content = { "mxid": mxid, - "threepid": {"medium": threepid["medium"], "address": threepid["address"]}, + "threepid": {"medium": medium, "address": address}, } # we abuse the federation http client to sign the request, but we have to send it @@ -319,12 +323,7 @@ async def try_unbind_threepid_with_id_server( except RequestTimedOutError: raise SynapseError(500, "Timed out contacting identity server") - await self.store.remove_user_bound_threepid( - user_id=mxid, - medium=threepid["medium"], - address=threepid["address"], - id_server=id_server, - ) + await self.store.remove_user_bound_threepid(mxid, medium, address, id_server) return changed From 1249e373dafe371d8ec84defe9448860ff39f2c9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 10 Feb 2023 17:53:40 +0000 Subject: [PATCH 2/5] Modify calling functions of 'try_unbind_threepid' --- synapse/handlers/auth.py | 5 ++--- synapse/handlers/deactivate_account.py | 7 +------ synapse/rest/client/account.py | 7 +------ 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 30f2d46c3c4a..57a6854b1e08 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -1593,9 +1593,8 @@ async def delete_threepid( if medium == "email": address = canonicalise_email(address) - identity_handler = self.hs.get_identity_handler() - result = await identity_handler.try_unbind_threepid( - user_id, {"medium": medium, "address": address, "id_server": id_server} + result = await self.hs.get_identity_handler().try_unbind_threepid( + user_id, medium, address, id_server ) await self.store.user_delete_threepid(user_id, medium, address) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index d74d135c0c50..d24f64938219 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -106,12 +106,7 @@ async def deactivate_account( for threepid in threepids: try: result = await self._identity_handler.try_unbind_threepid( - user_id, - { - "medium": threepid["medium"], - "address": threepid["address"], - "id_server": id_server, - }, + user_id, threepid["medium"], threepid["address"], id_server ) identity_server_supports_unbinding &= result except Exception: diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 4373c7366262..a54c87c31fab 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -734,12 +734,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: # Attempt to unbind the threepid from an identity server. If id_server is None, try to # unbind from all identity servers this threepid has been added to in the past result = await self.identity_handler.try_unbind_threepid( - requester.user.to_string(), - { - "address": body.address, - "medium": body.medium, - "id_server": body.id_server, - }, + requester.user.to_string(), body.address, body.medium, body.id_server ) return 200, {"id_server_unbind_result": "success" if result else "no-support"} From 26841a84cf427019d7f770c191e7a20f8f25e677 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 10 Feb 2023 17:58:11 +0000 Subject: [PATCH 3/5] changelog --- changelog.d/15053.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15053.misc diff --git a/changelog.d/15053.misc b/changelog.d/15053.misc new file mode 100644 index 000000000000..8bc4fae50ff4 --- /dev/null +++ b/changelog.d/15053.misc @@ -0,0 +1 @@ +Refactor arguments of `try_unbind_threepid` and `try_unbind_threepid_with_id_server` to not use dictionaries. \ No newline at end of file From 5604184b319700978781098937e5bb176e6f9f5c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 10 Feb 2023 23:45:21 +0000 Subject: [PATCH 4/5] Make 'try_unbind_threepid_with_id_server' private --- changelog.d/15053.misc | 2 +- synapse/handlers/identity.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog.d/15053.misc b/changelog.d/15053.misc index 8bc4fae50ff4..c27528f5c6e5 100644 --- a/changelog.d/15053.misc +++ b/changelog.d/15053.misc @@ -1 +1 @@ -Refactor arguments of `try_unbind_threepid` and `try_unbind_threepid_with_id_server` to not use dictionaries. \ No newline at end of file +Refactor arguments of `try_unbind_threepid` and `_try_unbind_threepid_with_id_server` to not use dictionaries. \ No newline at end of file diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index eaac5fcdd0ac..bf0f7acf8076 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -252,13 +252,13 @@ async def try_unbind_threepid( changed = True for id_server in id_servers: - changed &= await self.try_unbind_threepid_with_id_server( + changed &= await self._try_unbind_threepid_with_id_server( mxid, medium, address, id_server ) return changed - async def try_unbind_threepid_with_id_server( + async def _try_unbind_threepid_with_id_server( self, mxid: str, medium: str, address: str, id_server: str ) -> bool: """Removes a binding from an identity server From 2020d4dffbdf9ac2e3942b2c904915cde4039394 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 13 Feb 2023 11:33:29 +0000 Subject: [PATCH 5/5] Fix incorrect order of method arguments --- synapse/rest/client/account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index a54c87c31fab..653aa11e5091 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -734,7 +734,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: # Attempt to unbind the threepid from an identity server. If id_server is None, try to # unbind from all identity servers this threepid has been added to in the past result = await self.identity_handler.try_unbind_threepid( - requester.user.to_string(), body.address, body.medium, body.id_server + requester.user.to_string(), body.medium, body.address, body.id_server ) return 200, {"id_server_unbind_result": "success" if result else "no-support"}