From 7212a681ecb624c3fdad640779c67cb70b9302b5 Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:09:06 +0200 Subject: [PATCH 1/7] Remove v1 3pid bind Signed-off-by: Jacek Kusnierz --- synapse/handlers/identity.py | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 9bca2bc4b24e..6fc503b4b002 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -164,7 +164,6 @@ async def bind_threepid( mxid: str, id_server: str, id_access_token: Optional[str] = None, - use_v2: bool = True, ) -> JsonDict: """Bind a 3PID to an identity server @@ -175,10 +174,10 @@ async def bind_threepid( id_server: The domain of the identity server to query id_access_token: The access token to authenticate to the identity server with, if necessary. Required if use_v2 is true - use_v2: Whether to use v2 Identity Service API endpoints. Defaults to True Raises: SynapseError: On any of the following conditions + - no id_access_token was supplied - the supplied id_server is not a valid identity server name - we failed to contact the supplied identity server @@ -187,9 +186,10 @@ async def bind_threepid( """ logger.debug("Proxying threepid bind request for %s to %s", mxid, id_server) - # If an id_access_token is not supplied, force usage of v1 if id_access_token is None: - use_v2 = False + raise SynapseError( + 400, "id_access_token is required", errcode=Codes.MISSING_PARAM + ) if not valid_id_server_location(id_server): raise SynapseError( @@ -198,13 +198,9 @@ async def bind_threepid( ) # Decide which API endpoint URLs to use - headers = {} bind_data = {"sid": sid, "client_secret": client_secret, "mxid": mxid} - if use_v2: - bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) - headers["Authorization"] = create_id_access_token_header(id_access_token) # type: ignore - else: - bind_url = "https://%s/_matrix/identity/api/v1/3pid/bind" % (id_server,) + bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) + headers = {"Authorization": create_id_access_token_header(id_access_token)} try: # Use the blacklisting http client as this call is only to identity servers @@ -223,7 +219,7 @@ async def bind_threepid( return data except HttpResponseException as e: - if e.code != 404 or not use_v2: + if e.code != 404: logger.error("3PID bind failed with Matrix error: %r", e) raise e.to_synapse_error() except RequestTimedOutError: @@ -231,12 +227,7 @@ async def bind_threepid( except CodeMessageException as e: data = json_decoder.decode(e.msg) # XXX WAT? return data - - logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) - res = await self.bind_threepid( - client_secret, sid, mxid, id_server, id_access_token, use_v2=False - ) - return res + return {} 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 From 1084c5d4261f857c589f81d0f999ed4bfa86e4e9 Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:13:40 +0200 Subject: [PATCH 2/7] Add changelog --- changelog.d/13239.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13239.misc diff --git a/changelog.d/13239.misc b/changelog.d/13239.misc new file mode 100644 index 000000000000..a3a9f3cf5e89 --- /dev/null +++ b/changelog.d/13239.misc @@ -0,0 +1 @@ +Drop v1 3pid bind support. \ No newline at end of file From c55b3ec675a28a448dc549772f2aa8f1011bc814 Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:48:49 +0200 Subject: [PATCH 3/7] Correction according to code review Signed-off-by: Jacek Kusnierz --- changelog.d/13239.misc | 2 +- synapse/handlers/identity.py | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/changelog.d/13239.misc b/changelog.d/13239.misc index a3a9f3cf5e89..1ab07b7eb151 100644 --- a/changelog.d/13239.misc +++ b/changelog.d/13239.misc @@ -1 +1 @@ -Drop v1 3pid bind support. \ No newline at end of file +Drop support for calling `/_matrix/client/v3/account/3pid/bind` without an `id_access_token`. \ No newline at end of file diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 6fc503b4b002..25ce58154c64 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -163,7 +163,7 @@ async def bind_threepid( sid: str, mxid: str, id_server: str, - id_access_token: Optional[str] = None, + id_access_token: str, ) -> JsonDict: """Bind a 3PID to an identity server @@ -173,11 +173,10 @@ async def bind_threepid( mxid: The MXID to bind the 3PID to id_server: The domain of the identity server to query id_access_token: The access token to authenticate to the identity - server with, if necessary. Required if use_v2 is true + server with Raises: SynapseError: On any of the following conditions - - no id_access_token was supplied - the supplied id_server is not a valid identity server name - we failed to contact the supplied identity server @@ -186,18 +185,12 @@ async def bind_threepid( """ logger.debug("Proxying threepid bind request for %s to %s", mxid, id_server) - if id_access_token is None: - raise SynapseError( - 400, "id_access_token is required", errcode=Codes.MISSING_PARAM - ) - if not valid_id_server_location(id_server): raise SynapseError( 400, "id_server must be a valid hostname with optional port and path components", ) - # Decide which API endpoint URLs to use bind_data = {"sid": sid, "client_secret": client_secret, "mxid": mxid} bind_url = "https://%s/_matrix/identity/v2/3pid/bind" % (id_server,) headers = {"Authorization": create_id_access_token_header(id_access_token)} @@ -219,15 +212,13 @@ async def bind_threepid( return data except HttpResponseException as e: - if e.code != 404: - logger.error("3PID bind failed with Matrix error: %r", e) - raise e.to_synapse_error() + logger.error("3PID bind failed with Matrix error: %r", e) + raise e.to_synapse_error() except RequestTimedOutError: raise SynapseError(500, "Timed out contacting identity server") except CodeMessageException as e: data = json_decoder.decode(e.msg) # XXX WAT? return data - return {} 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 From f8baf960333368a41005e99719f7015d9aec743a Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:51:09 +0200 Subject: [PATCH 4/7] Missed one comment --- 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 bdc4a9c0683d..bf444869f864 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -746,7 +746,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: assert_params_in_dict(body, ["id_server", "sid", "client_secret"]) id_server = body["id_server"] sid = body["sid"] - id_access_token = body.get("id_access_token") # optional + id_access_token = body["id_access_token"] client_secret = body["client_secret"] assert_valid_client_secret(client_secret) From df50b11a204b1f62363866614536d61784c3fa30 Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:52:29 +0200 Subject: [PATCH 5/7] Changelog Misc->Removal --- changelog.d/{13239.misc => 13239.removal} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{13239.misc => 13239.removal} (100%) diff --git a/changelog.d/13239.misc b/changelog.d/13239.removal similarity index 100% rename from changelog.d/13239.misc rename to changelog.d/13239.removal From 026c583b18fbce3a3c9b0d1391d2b5f0a530d5cb Mon Sep 17 00:00:00 2001 From: Jacek Kusnierz Date: Mon, 11 Jul 2022 11:54:35 +0200 Subject: [PATCH 6/7] Assert id token in params --- synapse/rest/client/account.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index bf444869f864..f179fb2606cd 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -743,7 +743,9 @@ def __init__(self, hs: "HomeServer"): async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: body = parse_json_object_from_request(request) - assert_params_in_dict(body, ["id_server", "sid", "client_secret"]) + assert_params_in_dict( + body, ["id_server", "sid", "id_access_token", "client_secret"] + ) id_server = body["id_server"] sid = body["sid"] id_access_token = body["id_access_token"] From 7eefc79a953242febaba934dabb9d3f74e15a786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacek=20Ku=C5=9Bnierz?= Date: Tue, 12 Jul 2022 11:45:19 +0200 Subject: [PATCH 7/7] Update changelog.d/13239.removal Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- changelog.d/13239.removal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/13239.removal b/changelog.d/13239.removal index 1ab07b7eb151..8f6045176d44 100644 --- a/changelog.d/13239.removal +++ b/changelog.d/13239.removal @@ -1 +1 @@ -Drop support for calling `/_matrix/client/v3/account/3pid/bind` without an `id_access_token`. \ No newline at end of file +Drop support for calling `/_matrix/client/v3/account/3pid/bind` without an `id_access_token`, which was not permitted by the spec. Contributed by @Vetchu. \ No newline at end of file