From 3b666c14934a15767b5922c92b136c643311fb80 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 14:07:47 +0100 Subject: [PATCH 1/5] Implement MSC2229 - allow homeservers to rebind 3pids --- synapse/rest/client/v2_alpha/account.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 785d01ea52ea..9869627b8191 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -412,12 +412,14 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): def __init__(self, hs): super(EmailThreepidRequestTokenRestServlet, self).__init__() self.hs = hs + self.auth = hs.get_auth() self.config = hs.config self.identity_handler = hs.get_handlers().identity_handler self.store = self.hs.get_datastore() @defer.inlineCallbacks def on_POST(self, request): + requester = yield self.auth.get_user_by_req(request) body = parse_json_object_from_request(request) assert_params_in_dict( body, ["id_server", "client_secret", "email", "send_attempt"] @@ -439,7 +441,8 @@ def on_POST(self, request): "email", body["email"] ) - if existing_user_id is not None: + # Allow this MSISDN to be rebound if the requester owns it + if existing_user_id != requester.user.to_string(): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestEmailToken( @@ -452,13 +455,15 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet): PATTERNS = client_patterns("/account/3pid/msisdn/requestToken$") def __init__(self, hs): - self.hs = hs super(MsisdnThreepidRequestTokenRestServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() self.store = self.hs.get_datastore() self.identity_handler = hs.get_handlers().identity_handler @defer.inlineCallbacks def on_POST(self, request): + requester = self.auth.get_user_by_req(request) body = parse_json_object_from_request(request) assert_params_in_dict( body, @@ -482,7 +487,8 @@ def on_POST(self, request): existing_user_id = yield self.store.get_user_id_by_threepid("msisdn", msisdn) - if existing_user_id is not None: + # Allow this MSISDN to be rebound if the requester owns it + if existing_user_id != requester.user.to_string(): raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestMsisdnToken( From d29e4a31f452c1f3f068e890a5147d9243e8e8b3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Sep 2019 14:09:33 +0100 Subject: [PATCH 2/5] Add changelog --- changelog.d/5996.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5996.feature diff --git a/changelog.d/5996.feature b/changelog.d/5996.feature new file mode 100644 index 000000000000..2f9068f4dc84 --- /dev/null +++ b/changelog.d/5996.feature @@ -0,0 +1 @@ +Allow users to rebind 3pids they own (MSC2229). \ No newline at end of file From 2f9d8dc5b968dd95339953a91e80c22738833bfa Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Sep 2019 15:39:52 +0100 Subject: [PATCH 3/5] Address review comments --- synapse/rest/client/v2_alpha/account.py | 52 +++++++++++++++++++++---- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 9869627b8191..34646f6c26a2 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -21,7 +21,13 @@ from twisted.internet import defer from synapse.api.constants import LoginType -from synapse.api.errors import Codes, SynapseError, ThreepidValidationError +from synapse.api.errors import ( + Codes, + SynapseError, + ThreepidValidationError, + AuthError, + InvalidClientCredentialsError, +) from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import finish_request from synapse.http.servlet import ( @@ -419,7 +425,6 @@ def __init__(self, hs): @defer.inlineCallbacks def on_POST(self, request): - requester = yield self.auth.get_user_by_req(request) body = parse_json_object_from_request(request) assert_params_in_dict( body, ["id_server", "client_secret", "email", "send_attempt"] @@ -437,12 +442,24 @@ def on_POST(self, request): Codes.THREEPID_DENIED, ) + requester = None + try: + requester = yield self.auth.get_user_by_req(request) + except (AuthError, InvalidClientCredentialsError) as e: + # No authentication provided, ignore + pass + existing_user_id = yield self.store.get_user_id_by_threepid( "email", body["email"] ) - # Allow this MSISDN to be rebound if the requester owns it - if existing_user_id != requester.user.to_string(): + # If the request is authenticated, allow this MSISDN to be rebound if the requester + # owns it. + # Otherwise, deny the request if the 3PID exists + if ( + (requester and existing_user_id != requester.user.to_string()) or + (requester is None and existing_user_id) + ): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestEmailToken( @@ -463,7 +480,6 @@ def __init__(self, hs): @defer.inlineCallbacks def on_POST(self, request): - requester = self.auth.get_user_by_req(request) body = parse_json_object_from_request(request) assert_params_in_dict( body, @@ -485,10 +501,22 @@ def on_POST(self, request): Codes.THREEPID_DENIED, ) + requester = None + try: + requester = yield self.auth.get_user_by_req(request) + except (AuthError, InvalidClientCredentialsError) as e: + # No authentication provided, ignore + pass + existing_user_id = yield self.store.get_user_id_by_threepid("msisdn", msisdn) - # Allow this MSISDN to be rebound if the requester owns it - if existing_user_id != requester.user.to_string(): + # If the request is authenticated, allow this MSISDN to be rebound if the requester + # owns it. + # Otherwise, deny the request if the 3PID exists + if ( + (requester and existing_user_id != requester.user.to_string()) or + (requester is None and existing_user_id) + ): raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestMsisdnToken( @@ -528,7 +556,6 @@ def on_POST(self, request): requester = yield self.auth.get_user_by_req(request) user_id = requester.user.to_string() - threepid = yield self.identity_handler.threepid_from_creds(threepid_creds) if not threepid: @@ -539,6 +566,15 @@ def on_POST(self, request): logger.warn("Couldn't add 3pid: invalid response from ID server") raise SynapseError(500, "Invalid response from ID Server") + existing_user_id = yield self.datastore.get_user_id_by_threepid( + threepid["medium"], threepid["address"] + ) + + # Check that the user is not trying to add an email that's already bound to another + # user + if existing_user_id != requester.user.to_string(): + raise SynapseError(400, "This 3PID is already in use", Codes.THREEPID_IN_USE) + yield self.auth_handler.add_threepid( user_id, threepid["medium"], threepid["address"], threepid["validated_at"] ) From dac83b714f6b5fd7e64c053923c67c0f662370a1 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Sep 2019 15:41:10 +0100 Subject: [PATCH 4/5] lint --- synapse/rest/client/v2_alpha/account.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 34646f6c26a2..f19321d79472 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -22,11 +22,11 @@ from synapse.api.constants import LoginType from synapse.api.errors import ( + AuthError, Codes, + InvalidClientCredentialsError, SynapseError, ThreepidValidationError, - AuthError, - InvalidClientCredentialsError, ) from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import finish_request @@ -445,7 +445,7 @@ def on_POST(self, request): requester = None try: requester = yield self.auth.get_user_by_req(request) - except (AuthError, InvalidClientCredentialsError) as e: + except (AuthError, InvalidClientCredentialsError): # No authentication provided, ignore pass @@ -456,9 +456,8 @@ def on_POST(self, request): # If the request is authenticated, allow this MSISDN to be rebound if the requester # owns it. # Otherwise, deny the request if the 3PID exists - if ( - (requester and existing_user_id != requester.user.to_string()) or - (requester is None and existing_user_id) + if (requester and existing_user_id != requester.user.to_string()) or ( + requester is None and existing_user_id ): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) @@ -504,7 +503,7 @@ def on_POST(self, request): requester = None try: requester = yield self.auth.get_user_by_req(request) - except (AuthError, InvalidClientCredentialsError) as e: + except (AuthError, InvalidClientCredentialsError): # No authentication provided, ignore pass @@ -513,9 +512,8 @@ def on_POST(self, request): # If the request is authenticated, allow this MSISDN to be rebound if the requester # owns it. # Otherwise, deny the request if the 3PID exists - if ( - (requester and existing_user_id != requester.user.to_string()) or - (requester is None and existing_user_id) + if (requester and existing_user_id != requester.user.to_string()) or ( + requester is None and existing_user_id ): raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) @@ -573,7 +571,9 @@ def on_POST(self, request): # Check that the user is not trying to add an email that's already bound to another # user if existing_user_id != requester.user.to_string(): - raise SynapseError(400, "This 3PID is already in use", Codes.THREEPID_IN_USE) + raise SynapseError( + 400, "This 3PID is already in use", Codes.THREEPID_IN_USE + ) yield self.auth_handler.add_threepid( user_id, threepid["medium"], threepid["address"], threepid["validated_at"] From ccbd9dc5ac1b15da55f996799a41a5316e8c2999 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Sep 2019 17:03:31 +0100 Subject: [PATCH 5/5] clearer logic --- synapse/rest/client/v2_alpha/account.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index f19321d79472..fb2179651e5c 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -456,10 +456,12 @@ def on_POST(self, request): # If the request is authenticated, allow this MSISDN to be rebound if the requester # owns it. # Otherwise, deny the request if the 3PID exists - if (requester and existing_user_id != requester.user.to_string()) or ( - requester is None and existing_user_id - ): - raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) + if existing_user_id: + if ( + requester is None or + (requester and existing_user_id != requester.user.to_string) + ): + raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestEmailToken( id_server, email, client_secret, send_attempt, next_link @@ -512,10 +514,12 @@ def on_POST(self, request): # If the request is authenticated, allow this MSISDN to be rebound if the requester # owns it. # Otherwise, deny the request if the 3PID exists - if (requester and existing_user_id != requester.user.to_string()) or ( - requester is None and existing_user_id - ): - raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) + if existing_user_id: + if ( + requester is None or + (requester and existing_user_id != requester.user.to_string) + ): + raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE) ret = yield self.identity_handler.requestMsisdnToken( id_server, country, phone_number, client_secret, send_attempt, next_link @@ -570,9 +574,10 @@ def on_POST(self, request): # Check that the user is not trying to add an email that's already bound to another # user - if existing_user_id != requester.user.to_string(): + if existing_user_id and existing_user_id != requester.user.to_string(): raise SynapseError( - 400, "This 3PID is already in use", Codes.THREEPID_IN_USE + 400, "This 3PID is already in use by %s" % (existing_user_id,), + Codes.THREEPID_IN_USE, ) yield self.auth_handler.add_threepid(