From 478840d336d7a0dc96bf6b6a48c3530db2860725 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 20 Aug 2019 14:57:45 +0100 Subject: [PATCH 1/5] Add changelog --- changelog.d/5892.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5892.misc diff --git a/changelog.d/5892.misc b/changelog.d/5892.misc new file mode 100644 index 000000000000..a915bdeed251 --- /dev/null +++ b/changelog.d/5892.misc @@ -0,0 +1 @@ +Compatibility with v2 Identity Service APIs. \ No newline at end of file From 3750d8381e31a3ed7982d590b8b48340c95136db Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 3 Sep 2019 19:53:24 +0100 Subject: [PATCH 2/5] Switch to v2 identity server api endpoints. --- changelog.d/5892.misc | 2 +- contrib/cmdclient/console.py | 5 + synapse/handlers/identity.py | 178 ++++++++++++++++++------ synapse/rest/client/v2_alpha/account.py | 13 +- 4 files changed, 147 insertions(+), 51 deletions(-) diff --git a/changelog.d/5892.misc b/changelog.d/5892.misc index a915bdeed251..939fe8c6559c 100644 --- a/changelog.d/5892.misc +++ b/changelog.d/5892.misc @@ -1 +1 @@ -Compatibility with v2 Identity Service APIs. \ No newline at end of file +Compatibility with v2 Identity Service APIs other than /lookup. \ No newline at end of file diff --git a/contrib/cmdclient/console.py b/contrib/cmdclient/console.py index af8f39c8c279..899c650b0ce7 100755 --- a/contrib/cmdclient/console.py +++ b/contrib/cmdclient/console.py @@ -268,6 +268,7 @@ def do_emailrequest(self, line): @defer.inlineCallbacks def _do_emailrequest(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/validate/email/requestToken" @@ -302,6 +303,7 @@ def do_emailvalidate(self, line): @defer.inlineCallbacks def _do_emailvalidate(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/validate/email/submitToken" @@ -330,6 +332,7 @@ def do_3pidbind(self, line): @defer.inlineCallbacks def _do_3pidbind(self, args): + # TODO: Update to use v2 Identity Service API endpoint url = self._identityServerUrl() + "/_matrix/identity/api/v1/3pid/bind" json_res = yield self.http_client.do_request( @@ -398,6 +401,7 @@ def do_invite(self, line): @defer.inlineCallbacks def _do_invite(self, roomid, userstring): if not userstring.startswith("@") and self._is_on("complete_usernames"): + # TODO: Update to use v2 Identity Service API endpoint url = self._identityServerUrl() + "/_matrix/identity/api/v1/lookup" json_res = yield self.http_client.do_request( @@ -407,6 +411,7 @@ def _do_invite(self, roomid, userstring): mxid = None if "mxid" in json_res and "signatures" in json_res: + # TODO: Update to use v2 Identity Service API endpoint url = ( self._identityServerUrl() + "/_matrix/identity/api/v1/pubkey/ed25519" diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index d199521b5878..b0ff24ccee26 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -61,21 +61,72 @@ def _should_trust_id_server(self, id_server): return False return True + def _extract_items_from_creds_dict(self, creds): + """ + Retrieve entries from a "credentials" dictionary + + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. + + Returns: + tuple(str, str, str|None): A tuple containing the client_secret, the id_server, + and the id_access_token value if available. + """ + client_secret = creds.get("client_secret") or creds.get("clientSecret") + if not client_secret: + raise SynapseError( + 400, "No client_secret in creds", errcode=Codes.MISSING_PARAM + ) + + id_server = creds.get("id_server") or creds.get("idServer") + if not id_server: + raise SynapseError( + 400, "No id_server in creds", errcode=Codes.MISSING_PARAM + ) + + id_access_token = creds.get("id_access_token") + return client_secret, id_server, id_access_token + @defer.inlineCallbacks - def threepid_from_creds(self, creds): - if "id_server" in creds: - id_server = creds["id_server"] - elif "idServer" in creds: - id_server = creds["idServer"] - else: - raise SynapseError(400, "No id_server in creds") + def threepid_from_creds(self, creds, use_v2=True): + """ + Retrieve and validate a threepid identitier from a "credentials" dictionary + + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. Required if use_v2 is true + use_v2 (bool): Whether to use v2 Identity Service API endpoints + + Returns: + Deferred[dict[str,str|int]|None]: A dictionary consisting of response params to + the /getValidated3pid endpoint of the Identity Service API, or None if the + threepid was not found + """ + client_secret, id_server, id_access_token = self._extract_items_from_creds_dict( + creds + ) + + query_params = {"sid": creds["sid"], "client_secret": client_secret} - if "client_secret" in creds: - client_secret = creds["client_secret"] - elif "clientSecret" in creds: - client_secret = creds["clientSecret"] + # Decide which API endpoint URLs and query parameters to use + if use_v2: + url = "https://%s%s" % ( + id_server, + "/_matrix/identity/v2/3pid/getValidated3pid", + ) + query_params["id_access_token"] = id_access_token else: - raise SynapseError(400, "No client_secret in creds") + url = "https://%s%s" % ( + id_server, + "/_matrix/identity/api/v1/3pid/getValidated3pid", + ) if not self._should_trust_id_server(id_server): logger.warn( @@ -85,43 +136,51 @@ def threepid_from_creds(self, creds): return None try: - data = yield self.http_client.get_json( - "https://%s%s" - % (id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid"), - {"sid": creds["sid"], "client_secret": client_secret}, - ) + data = yield self.http_client.get_json(url, query_params) + return data if "medium" in data else None except HttpResponseException as e: - logger.info("getValidated3pid failed with Matrix error: %r", e) - raise e.to_synapse_error() + if e.code != 404 or not use_v2: + # Generic failure + logger.info("getValidated3pid failed with Matrix error: %r", e) + raise e.to_synapse_error() - if "medium" in data: - return data - return None + # This identity server is too old to understand Identity Service API v2 + # Attempt v1 endpoint + logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", url) + return (yield self.threepid_from_creds(creds, use_v2=False)) @defer.inlineCallbacks - def bind_threepid(self, creds, mxid): + def bind_threepid(self, creds, mxid, use_v2=True): + """Bind a 3PID to an identity server + + Args: + creds (dict[str, str]): Dictionary of credentials that contain the following keys: + * client_secret|clientSecret: A unique secret str provided by the client + * id_server|idServer: the domain of the identity server to query + * id_access_token: The access token to authenticate to the identity + server with. Required if use_v2 is true + mxid (str): The MXID to bind the 3PID to + use_v2 (bool): Whether to use v2 Identity Service API endpoints + + Returns: + Deferred[dict]: The response from the identity server + """ logger.debug("binding threepid %r to %s", creds, mxid) - data = None - if "id_server" in creds: - id_server = creds["id_server"] - elif "idServer" in creds: - id_server = creds["idServer"] - else: - raise SynapseError(400, "No id_server in creds") + client_secret, id_server, id_access_token = self._extract_items_from_creds_dict( + creds + ) - if "client_secret" in creds: - client_secret = creds["client_secret"] - elif "clientSecret" in creds: - client_secret = creds["clientSecret"] + # Decide which API endpoint URLs to use + bind_data = {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid} + if use_v2: + bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) + bind_data["id_access_token"] = id_access_token else: - raise SynapseError(400, "No client_secret in creds") + bind_url = "https://%s/_matrix/identity/api/v1/3pid/bind" % (id_server,) try: - data = yield self.http_client.post_json_get_json( - "https://%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/bind"), - {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid}, - ) + data = yield self.http_client.post_json_get_json(bind_url, bind_data) logger.debug("bound threepid %r to %s", creds, mxid) # Remember where we bound the threepid @@ -131,9 +190,18 @@ def bind_threepid(self, creds, mxid): address=data["address"], id_server=id_server, ) + + return data + except HttpResponseException as e: + if e.code != 404 or not use_v2: + logger.error("3PID bind failed with Matrix error: %r", e) + raise e.to_synapse_error() except CodeMessageException as e: data = json.loads(e.msg) # XXX WAT? - return data + return data + + logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) + return (yield self.bind_threepid(creds, mxid, use_v2=False)) @defer.inlineCallbacks def try_unbind_threepid(self, mxid, threepid): @@ -172,13 +240,16 @@ def try_unbind_threepid(self, mxid, threepid): return changed @defer.inlineCallbacks - def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): + def try_unbind_threepid_with_id_server( + self, mxid, threepid, id_server, use_v2=True + ): """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 + use_v2 (bool): Whether to use the v2 identity service unbind API Raises: SynapseError: If we failed to contact the identity server @@ -187,7 +258,14 @@ def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): 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,) + # First attempt the v2 endpoint + if use_v2: + url = "https://%s/_matrix/identity/v2/3pid/unbind" % (id_server,) + url_bytes = "/_matrix/identity/v2/3pid/unbind".encode("ascii") + else: + url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,) + url_bytes = "/_matrix/identity/api/v1/3pid/unbind".encode("ascii") + content = { "mxid": mxid, "threepid": {"medium": threepid["medium"], "address": threepid["address"]}, @@ -199,24 +277,36 @@ def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): auth_headers = self.federation_http_client.build_auth_headers( destination=None, method="POST", - url_bytes="/_matrix/identity/api/v1/3pid/unbind".encode("ascii"), + url_bytes=url_bytes, content=content, destination_is=id_server, ) headers = {b"Authorization": auth_headers} + v1_fallback = False try: yield self.http_client.post_json_get_json(url, content, headers) changed = True except HttpResponseException as e: changed = False - if e.code in (400, 404, 501): + if e.code == 404 and use_v2: + # v2 is not supported yet, try again with v1 + v1_fallback = True + elif 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) else: logger.error("Failed to unbind threepid on identity server: %s", e) raise SynapseError(502, "Failed to contact identity server") + if v1_fallback: + logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", url) + return ( + yield self.try_unbind_threepid_with_id_server( + mxid, threepid, id_server, use_v2=False + ) + ) + yield self.store.remove_user_bound_threepid( user_id=mxid, medium=threepid["medium"], diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 0620a4d0cf83..c8f2e11e887b 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -542,15 +542,16 @@ def on_GET(self, request): def on_POST(self, request): body = parse_json_object_from_request(request) - threePidCreds = body.get("threePidCreds") - threePidCreds = body.get("three_pid_creds", threePidCreds) - if threePidCreds is None: - raise SynapseError(400, "Missing param", Codes.MISSING_PARAM) + threepid_creds = body.get("threePidCreds") or body.get("three_pid_creds") + if threepid_creds is None: + raise SynapseError( + 400, "Missing param three_pid_creds", Codes.MISSING_PARAM + ) requester = yield self.auth.get_user_by_req(request) user_id = requester.user.to_string() - threepid = yield self.identity_handler.threepid_from_creds(threePidCreds) + threepid = yield self.identity_handler.threepid_from_creds(threepid_creds) if not threepid: raise SynapseError(400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED) @@ -566,7 +567,7 @@ def on_POST(self, request): if "bind" in body and body["bind"]: logger.debug("Binding threepid %s to %s", threepid, user_id) - yield self.identity_handler.bind_threepid(threePidCreds, user_id) + yield self.identity_handler.bind_threepid(threepid_creds, user_id) return 200, {} From dd390f2f7c7203b0b84cedaa7dee6aeeaf8a6ab4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 4 Sep 2019 18:15:08 +0100 Subject: [PATCH 3/5] Apply suggestions from code review Co-Authored-By: Erik Johnston --- synapse/handlers/identity.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index b0ff24ccee26..1133fe9f3af7 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -146,7 +146,7 @@ def threepid_from_creds(self, creds, use_v2=True): # This identity server is too old to understand Identity Service API v2 # Attempt v1 endpoint - logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", url) + logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", url) return (yield self.threepid_from_creds(creds, use_v2=False)) @defer.inlineCallbacks @@ -200,7 +200,7 @@ def bind_threepid(self, creds, mxid, use_v2=True): data = json.loads(e.msg) # XXX WAT? return data - logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) + logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) return (yield self.bind_threepid(creds, mxid, use_v2=False)) @defer.inlineCallbacks @@ -300,7 +300,7 @@ def try_unbind_threepid_with_id_server( raise SynapseError(502, "Failed to contact identity server") if v1_fallback: - logger.warn("Got 404 when POSTing JSON %s, falling back to v1 URL", url) + logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", url) return ( yield self.try_unbind_threepid_with_id_server( mxid, threepid, id_server, use_v2=False From b1c766da4a1348af76564df44206e0662750b9e7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 5 Sep 2019 13:51:04 +0100 Subject: [PATCH 4/5] Don't v2 unbind, use v1 if id_access_token is not supplied --- synapse/handlers/identity.py | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 1133fe9f3af7..bb80f42bac61 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -113,6 +113,10 @@ def threepid_from_creds(self, creds, use_v2=True): creds ) + # If an id_access_token is not supplied, force usage of v1 + if id_access_token is None: + use_v2 = False + query_params = {"sid": creds["sid"], "client_secret": client_secret} # Decide which API endpoint URLs and query parameters to use @@ -171,6 +175,10 @@ def bind_threepid(self, creds, mxid, use_v2=True): creds ) + # If an id_access_token is not supplied, force usage of v1 + if id_access_token is None: + use_v2 = False + # Decide which API endpoint URLs to use bind_data = {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid} if use_v2: @@ -241,7 +249,7 @@ def try_unbind_threepid(self, mxid, threepid): @defer.inlineCallbacks def try_unbind_threepid_with_id_server( - self, mxid, threepid, id_server, use_v2=True + self, mxid, threepid, id_server ): """Removes a binding from an identity server @@ -249,7 +257,6 @@ def try_unbind_threepid_with_id_server( 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 - use_v2 (bool): Whether to use the v2 identity service unbind API Raises: SynapseError: If we failed to contact the identity server @@ -258,13 +265,8 @@ def try_unbind_threepid_with_id_server( Deferred[bool]: True on success, otherwise False if the identity server doesn't support unbinding """ - # First attempt the v2 endpoint - if use_v2: - url = "https://%s/_matrix/identity/v2/3pid/unbind" % (id_server,) - url_bytes = "/_matrix/identity/v2/3pid/unbind".encode("ascii") - else: - url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,) - url_bytes = "/_matrix/identity/api/v1/3pid/unbind".encode("ascii") + url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,) + url_bytes = "/_matrix/identity/api/v1/3pid/unbind".encode("ascii") content = { "mxid": mxid, @@ -289,24 +291,13 @@ def try_unbind_threepid_with_id_server( changed = True except HttpResponseException as e: changed = False - if e.code == 404 and use_v2: - # v2 is not supported yet, try again with v1 - v1_fallback = True - elif e.code in (400, 404, 501): + 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) else: logger.error("Failed to unbind threepid on identity server: %s", e) raise SynapseError(502, "Failed to contact identity server") - if v1_fallback: - logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", url) - return ( - yield self.try_unbind_threepid_with_id_server( - mxid, threepid, id_server, use_v2=False - ) - ) - yield self.store.remove_user_bound_threepid( user_id=mxid, medium=threepid["medium"], From c493d718f0115d9933f1c0aa33cb11e0f5fc845e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 5 Sep 2019 13:55:49 +0100 Subject: [PATCH 5/5] lint --- synapse/handlers/identity.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index bb80f42bac61..1206e1e20904 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -248,9 +248,7 @@ def try_unbind_threepid(self, mxid, threepid): return changed @defer.inlineCallbacks - def try_unbind_threepid_with_id_server( - self, mxid, threepid, id_server - ): + def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): """Removes a binding from an identity server Args: @@ -285,7 +283,6 @@ def try_unbind_threepid_with_id_server( ) headers = {b"Authorization": auth_headers} - v1_fallback = False try: yield self.http_client.post_json_get_json(url, content, headers) changed = True