From 1666c0696a8891d39fd308e38e1b9b0042cabe34 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Apr 2019 10:16:13 +0100 Subject: Track IS used to bind 3PIDs This will then be used to know which IS to default to when unbinding the threepid. --- synapse/handlers/identity.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'synapse/handlers/identity.py') diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 39184f0e22..cc480301ac 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -132,6 +132,14 @@ class IdentityHandler(BaseHandler): } ) logger.debug("bound threepid %r to %s", creds, mxid) + + # Remember where we bound the threepid + yield self.store.add_user_bound_threepid( + user_id=mxid, + medium=data["medium"], + address=data["address"], + id_server=id_server, + ) except CodeMessageException as e: data = json.loads(e.msg) # XXX WAT? defer.returnValue(data) @@ -188,6 +196,13 @@ class IdentityHandler(BaseHandler): content, headers, ) + + yield self.store.remove_user_bound_threepid( + user_id=mxid, + medium=threepid["medium"], + address=threepid["address"], + id_server=id_server, + ) except HttpResponseException as e: if e.code in (400, 404, 501,): # The remote server probably doesn't support unbinding (yet) -- cgit 1.5.1 From 9fbbc3d9e54dbe0a1ce71907b4844ba41cec741b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Apr 2019 10:18:40 +0100 Subject: For unbind poke IS used during binding of 3PID This changes the behaviour from using the server specified trusted identity server to using the IS that used during the binding of the 3PID, if known. This is the behaviour specified by MSC1915. --- synapse/handlers/identity.py | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) (limited to 'synapse/handlers/identity.py') diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index cc480301ac..765b16d1d9 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -159,21 +159,47 @@ class IdentityHandler(BaseHandler): Deferred[bool]: True on success, otherwise False if the identity server doesn't support unbinding """ - logger.debug("unbinding threepid %r from %s", threepid, mxid) - if not self.trusted_id_servers: - logger.warn("Can't unbind threepid: no trusted ID servers set in config") + id_servers = yield self.store.get_id_servers_user_bound( + user_id=mxid, + medium=threepid["medium"], + address=threepid["address"], + ) + + # We don't know where to unbind, so we don't have a choice but to return + if not id_servers: defer.returnValue(False) - # We don't track what ID server we added 3pids on (perhaps we ought to) - # but we assume that any of the servers in the trusted list are in the - # same ID server federation, so we can pick any one of them to send the - # deletion request to. - id_server = next(iter(self.trusted_id_servers)) + changed = True + for id_server in id_servers: + changed &= yield self.try_unbind_threepid_with_id_server( + mxid, threepid, id_server, + ) + + defer.returnValue(changed) + + @defer.inlineCallbacks + def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): + """Removes a binding from an identity server + + Args: + mxid (str): Matrix user ID of binding to be removed + threepid (dict): Dict with medium & address of binding to be removed + id_server (str): Identity server to unbind from + + Raises: + SynapseError: If we failed to contact the identity server + Returns: + Deferred[bool]: True on success, otherwise False if the identity + server doesn't support unbinding + """ url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,) content = { "mxid": mxid, - "threepid": threepid, + "threepid": { + "medium": threepid["medium"], + "address": threepid["address"], + }, } # we abuse the federation http client to sign the request, but we have to send it -- cgit 1.5.1 From 057715aaa2b143843692ebe5fd137fed6e5d671c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Apr 2019 10:21:12 +0100 Subject: Allowing specifying IS to use in unbind API. By default the homeserver will use the identity server used during the binding of the 3PID to unbind the 3PID. However, we need to allow clients to explicitly ask the homeserver to unbind via a particular identity server, for the case where the 3PID was bound out of band from the homeserver. Implements MSC915. --- synapse/handlers/auth.py | 7 ++++++- synapse/handlers/deactivate_account.py | 5 ++++- synapse/handlers/identity.py | 13 ++++++++----- synapse/rest/client/v2_alpha/account.py | 3 ++- 4 files changed, 20 insertions(+), 8 deletions(-) (limited to 'synapse/handlers/identity.py') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 4544de821d..aa5d89a9ac 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -912,7 +912,7 @@ class AuthHandler(BaseHandler): ) @defer.inlineCallbacks - def delete_threepid(self, user_id, medium, address): + def delete_threepid(self, user_id, medium, address, id_server=None): """Attempts to unbind the 3pid on the identity servers and deletes it from the local database. @@ -920,6 +920,10 @@ class AuthHandler(BaseHandler): user_id (str) medium (str) address (str) + id_server (str|None): Use the given identity server when unbinding + any threepids. If None then will attempt to unbind using the + identity server specified when binding (if known). + Returns: Deferred[bool]: Returns True if successfully unbound the 3pid on @@ -937,6 +941,7 @@ class AuthHandler(BaseHandler): { 'medium': medium, 'address': address, + 'id_server': id_server, }, ) diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py index 97d3f31d98..101879f89f 100644 --- a/synapse/handlers/deactivate_account.py +++ b/synapse/handlers/deactivate_account.py @@ -43,12 +43,15 @@ class DeactivateAccountHandler(BaseHandler): hs.get_reactor().callWhenRunning(self._start_user_parting) @defer.inlineCallbacks - def deactivate_account(self, user_id, erase_data): + def deactivate_account(self, user_id, erase_data, id_server=None): """Deactivate a user's account Args: user_id (str): ID of user to be deactivated erase_data (bool): whether to GDPR-erase the user's data + id_server (str|None): Use the given identity server when unbinding + any threepids. If None then will attempt to unbind using the + identity server specified when binding (if known). Returns: Deferred[bool]: True if identity server supports removing diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 765b16d1d9..4c127ba125 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -159,11 +159,14 @@ class IdentityHandler(BaseHandler): Deferred[bool]: True on success, otherwise False if the identity server doesn't support unbinding """ - id_servers = yield self.store.get_id_servers_user_bound( - user_id=mxid, - medium=threepid["medium"], - address=threepid["address"], - ) + if threepid.get("id_server"): + id_servers = [threepid["id_server"]] + else: + id_servers = yield self.store.get_id_servers_user_bound( + user_id=mxid, + medium=threepid["medium"], + address=threepid["address"], + ) # We don't know where to unbind, so we don't have a choice but to return if not id_servers: diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 37b32dd37b..50a434a507 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -215,6 +215,7 @@ class DeactivateAccountRestServlet(RestServlet): ) result = yield self._deactivate_account_handler.deactivate_account( requester.user.to_string(), erase, + id_server=body.get("id_server"), ) if result: id_server_unbind_result = "success" @@ -380,7 +381,7 @@ class ThreepidDeleteRestServlet(RestServlet): try: ret = yield self.auth_handler.delete_threepid( - user_id, body['medium'], body['address'] + user_id, body['medium'], body['address'], body.get("id_server"), ) except Exception: # NB. This endpoint should succeed if there is nothing to -- cgit 1.5.1 From c75e2017f16bf18127f4f70f69100b8343a139c4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Apr 2019 11:15:19 +0100 Subject: Fixup docstrings --- synapse/handlers/identity.py | 6 ++++-- synapse/storage/registration.py | 6 +++--- synapse/storage/schema/delta/53/user_threepid_id.sql | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) (limited to 'synapse/handlers/identity.py') diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 4c127ba125..3eb112bf9d 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -150,14 +150,16 @@ class IdentityHandler(BaseHandler): Args: mxid (str): Matrix user ID of binding to be removed - threepid (dict): Dict with medium & address of binding to be removed + threepid (dict): Dict with medium & address of binding to be + removed, and an optional id_server. Raises: SynapseError: If we failed to contact the identity server Returns: Deferred[bool]: True on success, otherwise False if the identity - server doesn't support unbinding + server doesn't support unbinding (or no identity server found to + contact). """ if threepid.get("id_server"): id_servers = [threepid["id_server"]] diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 4d64107e16..8ada655725 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -358,9 +358,9 @@ class RegistrationWorkerStore(SQLBaseStore): ) def remove_user_bound_threepid(self, user_id, medium, address, id_server): - """The server proxied a bind request to the given identity server on - behalf of the given user. We need to remember this in case the user - asks us to unbind the threepid. + """The server proxied an unbind request to the given identity server on + behalf of the given user, so we remove the mapping of threepid to + identity server. Args: user_id (str) diff --git a/synapse/storage/schema/delta/53/user_threepid_id.sql b/synapse/storage/schema/delta/53/user_threepid_id.sql index 6c0b7ec0f7..80c2c573b6 100644 --- a/synapse/storage/schema/delta/53/user_threepid_id.sql +++ b/synapse/storage/schema/delta/53/user_threepid_id.sql @@ -13,8 +13,8 @@ * limitations under the License. */ --- Tracks when -CREATE TABlE user_threepid_id_server ( +-- Tracks which identity server a user bound their threepid via. +CREATE TABLE user_threepid_id_server ( user_id TEXT NOT NULL, medium TEXT NOT NULL, address TEXT NOT NULL, -- cgit 1.5.1 From 24232514bfdd476b037052889d81a6651062446d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Apr 2019 11:16:57 +0100 Subject: Remove threepid binding if id server returns 400/404/501 --- synapse/handlers/identity.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'synapse/handlers/identity.py') diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 3eb112bf9d..22469486d7 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -227,23 +227,24 @@ class IdentityHandler(BaseHandler): content, headers, ) - - yield self.store.remove_user_bound_threepid( - user_id=mxid, - medium=threepid["medium"], - address=threepid["address"], - id_server=id_server, - ) + changed = True except HttpResponseException as e: + changed = False if e.code in (400, 404, 501,): # The remote server probably doesn't support unbinding (yet) logger.warn("Received %d response while unbinding threepid", e.code) - defer.returnValue(False) else: logger.error("Failed to unbind threepid on identity server: %s", e) raise SynapseError(502, "Failed to contact identity server") - defer.returnValue(True) + yield self.store.remove_user_bound_threepid( + user_id=mxid, + medium=threepid["medium"], + address=threepid["address"], + id_server=id_server, + ) + + defer.returnValue(changed) @defer.inlineCallbacks def requestEmailToken(self, id_server, email, client_secret, send_attempt, **kwargs): -- cgit 1.5.1