Merge pull request #3667 from matrix-org/erikj/fixup_unbind

Don't fail requests to unbind 3pids for non supporting ID servers
This commit is contained in:
Erik Johnston 2018-08-15 10:32:12 +01:00 committed by GitHub
commit fef2e65d12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 79 additions and 20 deletions

1
changelog.d/3661.bugfix Normal file
View File

@ -0,0 +1 @@
Fix bug on deleting 3pid when using identity servers that don't support unbind API

View File

@ -828,12 +828,26 @@ class AuthHandler(BaseHandler):
@defer.inlineCallbacks @defer.inlineCallbacks
def delete_threepid(self, user_id, medium, address): def delete_threepid(self, user_id, medium, address):
"""Attempts to unbind the 3pid on the identity servers and deletes it
from the local database.
Args:
user_id (str)
medium (str)
address (str)
Returns:
Deferred[bool]: Returns True if successfully unbound the 3pid on
the identity server, False if identity server doesn't support the
unbind API.
"""
# 'Canonicalise' email addresses as per above # 'Canonicalise' email addresses as per above
if medium == 'email': if medium == 'email':
address = address.lower() address = address.lower()
identity_handler = self.hs.get_handlers().identity_handler identity_handler = self.hs.get_handlers().identity_handler
yield identity_handler.unbind_threepid( result = yield identity_handler.try_unbind_threepid(
user_id, user_id,
{ {
'medium': medium, 'medium': medium,
@ -841,10 +855,10 @@ class AuthHandler(BaseHandler):
}, },
) )
ret = yield self.store.user_delete_threepid( yield self.store.user_delete_threepid(
user_id, medium, address, user_id, medium, address,
) )
defer.returnValue(ret) defer.returnValue(result)
def _save_session(self, session): def _save_session(self, session):
# TODO: Persistent storage # TODO: Persistent storage

View File

@ -51,7 +51,8 @@ class DeactivateAccountHandler(BaseHandler):
erase_data (bool): whether to GDPR-erase the user's data erase_data (bool): whether to GDPR-erase the user's data
Returns: Returns:
Deferred Deferred[bool]: True if identity server supports removing
threepids, otherwise False.
""" """
# FIXME: Theoretically there is a race here wherein user resets # FIXME: Theoretically there is a race here wherein user resets
# password using threepid. # password using threepid.
@ -60,16 +61,22 @@ class DeactivateAccountHandler(BaseHandler):
# leave the user still active so they can try again. # leave the user still active so they can try again.
# Ideally we would prevent password resets and then do this in the # Ideally we would prevent password resets and then do this in the
# background thread. # background thread.
# This will be set to false if the identity server doesn't support
# unbinding
identity_server_supports_unbinding = True
threepids = yield self.store.user_get_threepids(user_id) threepids = yield self.store.user_get_threepids(user_id)
for threepid in threepids: for threepid in threepids:
try: try:
yield self._identity_handler.unbind_threepid( result = yield self._identity_handler.try_unbind_threepid(
user_id, user_id,
{ {
'medium': threepid['medium'], 'medium': threepid['medium'],
'address': threepid['address'], 'address': threepid['address'],
}, },
) )
identity_server_supports_unbinding &= result
except Exception: except Exception:
# Do we want this to be a fatal error or should we carry on? # Do we want this to be a fatal error or should we carry on?
logger.exception("Failed to remove threepid from ID server") logger.exception("Failed to remove threepid from ID server")
@ -103,6 +110,8 @@ class DeactivateAccountHandler(BaseHandler):
# parts users from rooms (if it isn't already running) # parts users from rooms (if it isn't already running)
self._start_user_parting() self._start_user_parting()
defer.returnValue(identity_server_supports_unbinding)
def _start_user_parting(self): def _start_user_parting(self):
""" """
Start the process that goes through the table of users Start the process that goes through the table of users

View File

@ -137,15 +137,19 @@ class IdentityHandler(BaseHandler):
defer.returnValue(data) defer.returnValue(data)
@defer.inlineCallbacks @defer.inlineCallbacks
def unbind_threepid(self, mxid, threepid): def try_unbind_threepid(self, mxid, threepid):
""" """Removes a binding from an identity server
Removes a binding from an identity server
Args: Args:
mxid (str): Matrix user ID of binding to be removed 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
Raises:
SynapseError: If we failed to contact the identity server
Returns: Returns:
Deferred[bool]: True on success, otherwise False Deferred[bool]: True on success, otherwise False if the identity
server doesn't support unbinding
""" """
logger.debug("unbinding threepid %r from %s", threepid, mxid) logger.debug("unbinding threepid %r from %s", threepid, mxid)
if not self.trusted_id_servers: if not self.trusted_id_servers:
@ -175,11 +179,21 @@ class IdentityHandler(BaseHandler):
content=content, content=content,
destination_is=id_server, destination_is=id_server,
) )
yield self.http_client.post_json_get_json( try:
url, yield self.http_client.post_json_get_json(
content, url,
headers, content,
) headers,
)
except HttpResponseException as e:
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) defer.returnValue(True)
@defer.inlineCallbacks @defer.inlineCallbacks

View File

@ -391,10 +391,17 @@ class DeactivateAccountRestServlet(ClientV1RestServlet):
if not is_admin: if not is_admin:
raise AuthError(403, "You are not a server admin") raise AuthError(403, "You are not a server admin")
yield self._deactivate_account_handler.deactivate_account( result = yield self._deactivate_account_handler.deactivate_account(
target_user_id, erase, target_user_id, erase,
) )
defer.returnValue((200, {})) if result:
id_server_unbind_result = "success"
else:
id_server_unbind_result = "no-support"
defer.returnValue((200, {
"id_server_unbind_result": id_server_unbind_result,
}))
class ShutdownRoomRestServlet(ClientV1RestServlet): class ShutdownRoomRestServlet(ClientV1RestServlet):

View File

@ -209,10 +209,17 @@ class DeactivateAccountRestServlet(RestServlet):
yield self.auth_handler.validate_user_via_ui_auth( yield self.auth_handler.validate_user_via_ui_auth(
requester, body, self.hs.get_ip_from_request(request), requester, body, self.hs.get_ip_from_request(request),
) )
yield self._deactivate_account_handler.deactivate_account( result = yield self._deactivate_account_handler.deactivate_account(
requester.user.to_string(), erase, requester.user.to_string(), erase,
) )
defer.returnValue((200, {})) if result:
id_server_unbind_result = "success"
else:
id_server_unbind_result = "no-support"
defer.returnValue((200, {
"id_server_unbind_result": id_server_unbind_result,
}))
class EmailThreepidRequestTokenRestServlet(RestServlet): class EmailThreepidRequestTokenRestServlet(RestServlet):
@ -364,7 +371,7 @@ class ThreepidDeleteRestServlet(RestServlet):
user_id = requester.user.to_string() user_id = requester.user.to_string()
try: try:
yield self.auth_handler.delete_threepid( ret = yield self.auth_handler.delete_threepid(
user_id, body['medium'], body['address'] user_id, body['medium'], body['address']
) )
except Exception: except Exception:
@ -374,7 +381,14 @@ class ThreepidDeleteRestServlet(RestServlet):
logger.exception("Failed to remove threepid") logger.exception("Failed to remove threepid")
raise SynapseError(500, "Failed to remove threepid") raise SynapseError(500, "Failed to remove threepid")
defer.returnValue((200, {})) if ret:
id_server_unbind_result = "success"
else:
id_server_unbind_result = "no-support"
defer.returnValue((200, {
"id_server_unbind_result": id_server_unbind_result,
}))
class WhoamiRestServlet(RestServlet): class WhoamiRestServlet(RestServlet):