From 0f6487e09067068f29f400167c34b11d56504427 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 11 Jul 2022 11:34:29 +0200
Subject: [PATCH 01/21] Drop v1 lookup

Signed-off-by: Jacek Kusnierz <jacek.kusnierz@tum.de>
---
 synapse/handlers/identity.py | 63 +++++++-----------------------------
 1 file changed, 11 insertions(+), 52 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 9bca2bc4b24e..d0c32e883900 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -629,62 +629,21 @@ async def lookup_3pid(
         Returns:
             the matrix ID of the 3pid, or None if it is not recognized.
         """
-        if id_access_token is not None:
-            try:
-                results = await self._lookup_3pid_v2(
-                    id_server, id_access_token, medium, address
-                )
-                return results
-
-            except Exception as e:
-                # Catch HttpResponseExcept for a non-200 response code
-                # Check if this identity server does not know about v2 lookups
-                if isinstance(e, HttpResponseException) and e.code == 404:
-                    # This is an old identity server that does not yet support v2 lookups
-                    logger.warning(
-                        "Attempted v2 lookup on v1 identity server %s. Falling "
-                        "back to v1",
-                        id_server,
-                    )
-                else:
-                    logger.warning("Error when looking up hashing details: %s", e)
-                    return None
-
-        return await self._lookup_3pid_v1(id_server, medium, address)
-
-    async def _lookup_3pid_v1(
-        self, id_server: str, medium: str, address: str
-    ) -> Optional[str]:
-        """Looks up a 3pid in the passed identity server using v1 lookup.
-
-        Args:
-            id_server: The server name (including port, if required)
-                of the identity server to use.
-            medium: The type of the third party identifier (e.g. "email").
-            address: The third party identifier (e.g. "foo@example.com").
+        if id_access_token is None:
+            raise SynapseError(
+                400, "id_access_token is required", errcode=Codes.MISSING_PARAM
+            )
 
-        Returns:
-            the matrix ID of the 3pid, or None if it is not recognized.
-        """
         try:
-            data = await self.blacklisting_http_client.get_json(
-                "%s%s/_matrix/identity/api/v1/lookup" % (id_server_scheme, id_server),
-                {"medium": medium, "address": address},
+            results = await self._lookup_3pid(
+                id_server, id_access_token, medium, address
             )
+            return results
+        except Exception as e:
+            logger.warning("Error when looking up hashing details: %s", e)
+            return None
 
-            if "mxid" in data:
-                # note: we used to verify the identity server's signature here, but no longer
-                # require or validate it. See the following for context:
-                # https://github.com/matrix-org/synapse/issues/5253#issuecomment-666246950
-                return data["mxid"]
-        except RequestTimedOutError:
-            raise SynapseError(500, "Timed out contacting identity server")
-        except OSError as e:
-            logger.warning("Error from v1 identity server lookup: %s" % (e,))
-
-        return None
-
-    async def _lookup_3pid_v2(
+    async def _lookup_3pid(
         self, id_server: str, id_access_token: str, medium: str, address: str
     ) -> Optional[str]:
         """Looks up a 3pid in the passed identity server using v2 lookup.

From ef1c7940e5a88913959c92ac03c8ab8537d998da Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 11 Jul 2022 11:40:00 +0200
Subject: [PATCH 02/21] Add changelog

Signed-off-by: Jacek Kusnierz <jacek.kusnierz@tum.de>
---
 changelog.d/13241.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/13241.misc

diff --git a/changelog.d/13241.misc b/changelog.d/13241.misc
new file mode 100644
index 000000000000..44fd7d4957aa
--- /dev/null
+++ b/changelog.d/13241.misc
@@ -0,0 +1 @@
+Drop v1 3PID lookup.
\ No newline at end of file

From e9c789a03b22b6d00126d7006abaf3c4f9623ceb Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 11 Jul 2022 12:29:39 +0200
Subject: [PATCH 03/21] Address code review

---
 changelog.d/13241.misc          |  1 -
 changelog.d/13241.removal       |  1 +
 synapse/handlers/identity.py    | 56 ++++++++++++++-------------------
 synapse/handlers/room.py        |  2 +-
 synapse/handlers/room_member.py |  6 ++--
 synapse/rest/client/room.py     |  4 +--
 6 files changed, 30 insertions(+), 40 deletions(-)
 delete mode 100644 changelog.d/13241.misc
 create mode 100644 changelog.d/13241.removal

diff --git a/changelog.d/13241.misc b/changelog.d/13241.misc
deleted file mode 100644
index 44fd7d4957aa..000000000000
--- a/changelog.d/13241.misc
+++ /dev/null
@@ -1 +0,0 @@
-Drop v1 3PID lookup.
\ No newline at end of file
diff --git a/changelog.d/13241.removal b/changelog.d/13241.removal
new file mode 100644
index 000000000000..78d714090f02
--- /dev/null
+++ b/changelog.d/13241.removal
@@ -0,0 +1 @@
+Drop support for calling `/_matrix/client/v3/rooms/{roomId}/invite` without an `id_access_token`.
\ No newline at end of file
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index d0c32e883900..ad944c165af4 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -610,11 +610,7 @@ async def proxy_msisdn_submit_token(
             raise SynapseError(400, "Error contacting the identity server")
 
     async def lookup_3pid(
-        self,
-        id_server: str,
-        medium: str,
-        address: str,
-        id_access_token: Optional[str] = None,
+        self, id_server: str, medium: str, address: str, id_access_token: str
     ) -> Optional[str]:
         """Looks up a 3pid in the passed identity server.
 
@@ -629,13 +625,9 @@ async def lookup_3pid(
         Returns:
             the matrix ID of the 3pid, or None if it is not recognized.
         """
-        if id_access_token is None:
-            raise SynapseError(
-                400, "id_access_token is required", errcode=Codes.MISSING_PARAM
-            )
 
         try:
-            results = await self._lookup_3pid(
+            results = await self._lookup_3pid_v2(
                 id_server, id_access_token, medium, address
             )
             return results
@@ -643,7 +635,7 @@ async def lookup_3pid(
             logger.warning("Error when looking up hashing details: %s", e)
             return None
 
-    async def _lookup_3pid(
+    async def _lookup_3pid_v2(
         self, id_server: str, id_access_token: str, medium: str, address: str
     ) -> Optional[str]:
         """Looks up a 3pid in the passed identity server using v2 lookup.
@@ -770,7 +762,7 @@ async def ask_id_server_for_third_party_invite(
         room_type: Optional[str],
         inviter_display_name: str,
         inviter_avatar_url: str,
-        id_access_token: Optional[str] = None,
+        id_access_token: str,
     ) -> Tuple[str, List[Dict[str, str]], Dict[str, str], str]:
         """
         Asks an identity server for a third party invite.
@@ -791,7 +783,7 @@ async def ask_id_server_for_third_party_invite(
             inviter_display_name: The current display name of the
                 inviter.
             inviter_avatar_url: The URL of the inviter's avatar.
-            id_access_token (str|None): The access token to authenticate to the identity
+            id_access_token (str): The access token to authenticate to the identity
                 server with
 
         Returns:
@@ -823,30 +815,28 @@ async def ask_id_server_for_third_party_invite(
             invite_config["org.matrix.web_client_location"] = self._web_client_location
 
         # Add the identity service access token to the JSON body and use the v2
-        # Identity Service endpoints if id_access_token is present
+        # Identity Service endpoints
         data = None
         base_url = "%s%s/_matrix/identity" % (id_server_scheme, id_server)
 
-        if id_access_token:
-            key_validity_url = "%s%s/_matrix/identity/v2/pubkey/isvalid" % (
-                id_server_scheme,
-                id_server,
-            )
+        key_validity_url = "%s%s/_matrix/identity/v2/pubkey/isvalid" % (
+            id_server_scheme,
+            id_server,
+        )
 
-            # Attempt a v2 lookup
-            url = base_url + "/v2/store-invite"
-            try:
-                data = await self.blacklisting_http_client.post_json_get_json(
-                    url,
-                    invite_config,
-                    {"Authorization": create_id_access_token_header(id_access_token)},
-                )
-            except RequestTimedOutError:
-                raise SynapseError(500, "Timed out contacting identity server")
-            except HttpResponseException as e:
-                if e.code != 404:
-                    logger.info("Failed to POST %s with JSON: %s", url, e)
-                    raise e
+        url = base_url + "/v2/store-invite"
+        try:
+            data = await self.blacklisting_http_client.post_json_get_json(
+                url,
+                invite_config,
+                {"Authorization": create_id_access_token_header(id_access_token)},
+            )
+        except RequestTimedOutError:
+            raise SynapseError(500, "Timed out contacting identity server")
+        except HttpResponseException as e:
+            if e.code != 404:
+                logger.info("Failed to POST %s with JSON: %s", url, e)
+                raise e
 
         if data is None:
             key_validity_url = "%s%s/_matrix/identity/api/v1/pubkey/isvalid" % (
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 44f808457940..307ee2076751 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -955,7 +955,7 @@ async def create_room(
 
         for invite_3pid in invite_3pid_list:
             id_server = invite_3pid["id_server"]
-            id_access_token = invite_3pid.get("id_access_token")  # optional
+            id_access_token = invite_3pid["id_access_token"]
             address = invite_3pid["address"]
             medium = invite_3pid["medium"]
             # Note that do_3pid_invite can raise a  ShadowBanError, but this was
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index a1d8875dd874..0fec55851f0b 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1313,7 +1313,7 @@ async def do_3pid_invite(
         id_server: str,
         requester: Requester,
         txn_id: Optional[str],
-        id_access_token: Optional[str] = None,
+        id_access_token: str,
     ) -> int:
         """Invite a 3PID to a room.
 
@@ -1326,7 +1326,7 @@ async def do_3pid_invite(
             requester: The user making the request.
             txn_id: The transaction ID this is part of, or None if this is not
                 part of a transaction.
-            id_access_token: The optional identity server access token.
+            id_access_token: Identity server access token.
 
         Returns:
              The new stream ID.
@@ -1411,7 +1411,7 @@ async def _make_and_store_3pid_invite(
         room_id: str,
         user: UserID,
         txn_id: Optional[str],
-        id_access_token: Optional[str] = None,
+        id_access_token: str,
     ) -> int:
         room_state = await self._storage_controllers.state.get_current_state(
             room_id,
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 2f513164cb84..71080e076851 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -870,7 +870,7 @@ async def on_POST(
                     content["id_server"],
                     requester,
                     txn_id,
-                    content.get("id_access_token"),
+                    content["id_access_token"],
                 )
             except ShadowBanError:
                 # Pretend the request succeeded.
@@ -908,7 +908,7 @@ async def on_POST(
         return 200, return_value
 
     def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
-        for key in {"id_server", "medium", "address"}:
+        for key in {"id_server", "medium", "address", "id_access_token"}:
             if key not in content:
                 return False
         return True

From e354ec681adba42cfb5ddb1235dcc2cb33e23a06 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 11 Jul 2022 14:20:58 +0200
Subject: [PATCH 04/21] Repair tests PoC.

Signed-off-by: Jacek Kusnierz <jacek.kusnierz@tum.de>
---
 tests/rest/client/test_identity.py      | 13 +++++++++++++
 tests/rest/client/test_shadow_banned.py |  7 ++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/tests/rest/client/test_identity.py b/tests/rest/client/test_identity.py
index 299b9d21e24b..c98b8102d301 100644
--- a/tests/rest/client/test_identity.py
+++ b/tests/rest/client/test_identity.py
@@ -55,6 +55,7 @@ def test_3pid_lookup_disabled(self) -> None:
             "id_server": "testis",
             "medium": "email",
             "address": "test@example.com",
+            "id_access_token": tok,
         }
         request_data = json.dumps(params)
         request_url = ("/rooms/%s/invite" % (room_id)).encode("ascii")
@@ -62,3 +63,15 @@ def test_3pid_lookup_disabled(self) -> None:
             b"POST", request_url, request_data, access_token=tok
         )
         self.assertEqual(channel.code, HTTPStatus.FORBIDDEN, channel.result)
+
+
+# {'version': b'1.1', 'code': b'400', 'reason': b'Bad Request',
+# 'headers': [
+#     (b'Server', b'1'), (b'Date', b'Mon,
+#         11 Jul 2022 10: 58: 19 GMT'),
+#         (b'Content-Type', b'application/json'), (b'Cache-Control', b'no-cache, no-store, must-revalidate'), (b'Access-Control-Allow-Origin', b'*'), (b'Access-Control-Allow-Methods', b'GET, HEAD, POST, PUT, DELETE, OPTIONS'), (b'Access-Control-Allow-Headers', b'X-Requested-With, Content-Type, Authorization, Date')
+#     ], 'body': b'{
+#         "errcode": "M_MISSING_PARAM",
+#         "error":"Missing params: [\'user_id\']"
+#     }', 'done': True
+# }
diff --git a/tests/rest/client/test_shadow_banned.py b/tests/rest/client/test_shadow_banned.py
index d9bd8c4a281d..21609231359a 100644
--- a/tests/rest/client/test_shadow_banned.py
+++ b/tests/rest/client/test_shadow_banned.py
@@ -97,7 +97,12 @@ def test_invite_3pid(self) -> None:
         channel = self.make_request(
             "POST",
             "/rooms/%s/invite" % (room_id,),
-            {"id_server": "test", "medium": "email", "address": "test@test.test"},
+            {
+                "id_server": "test",
+                "medium": "email",
+                "address": "test@test.test",
+                "id_access_token": self.banned_access_token,
+            },
             access_token=self.banned_access_token,
         )
         self.assertEqual(200, channel.code, channel.result)

From eb5ba09713efb8e5f5188c0e38b274327149aa2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jacek=20Ku=C5=9Bnierz?= <kusnierz@protonmail.com>
Date: Tue, 12 Jul 2022 11:46:26 +0200
Subject: [PATCH 05/21] Update changelog.d/13241.removal

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 changelog.d/13241.removal | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/13241.removal b/changelog.d/13241.removal
index 78d714090f02..60b0e7969cd2 100644
--- a/changelog.d/13241.removal
+++ b/changelog.d/13241.removal
@@ -1 +1 @@
-Drop support for calling `/_matrix/client/v3/rooms/{roomId}/invite` without an `id_access_token`.
\ No newline at end of file
+Drop support for calling `/_matrix/client/v3/rooms/{roomId}/invite` without an `id_access_token`, which was not permitted by the spec. Contributed by @Vetchu.
\ No newline at end of file

From 779e78c7cfcefdaecbbbb8f36d4f617555ed244b Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Thu, 21 Jul 2022 17:43:46 +0200
Subject: [PATCH 06/21] Add tests, address concerns

---
 synapse/handlers/identity.py    |  4 +---
 synapse/handlers/room_member.py |  2 +-
 tests/rest/client/test_rooms.py | 31 +++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 263b3667edee..a6ad718933f1 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -762,9 +762,7 @@ async def ask_id_server_for_third_party_invite(
         except RequestTimedOutError:
             raise SynapseError(500, "Timed out contacting identity server")
         except HttpResponseException as e:
-            if e.code != 404:
-                logger.info("Failed to POST %s with JSON: %s", url, e)
-                raise e
+            logger.info("Failed to POST %s with JSON: %s", url, e)
 
         if data is None:
             key_validity_url = "%s%s/_matrix/identity/api/v1/pubkey/isvalid" % (
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index c6bc3c1edf78..ffef06887fdd 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1381,7 +1381,7 @@ async def do_3pid_invite(
         txn_id: Optional[str],
         id_access_token: str,
         prev_event_ids: Optional[List[str]] = None,
-        depth: Optional[int] = None
+        depth: Optional[int] = None,
     ) -> Tuple[str, int]:
         """Invite a 3PID to a room.
 
diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index c45cb3209025..2d2a5b962886 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3462,3 +3462,34 @@ def test_threepid_invite_spamcheck(self) -> None:
 
         # Also check that it stopped before calling _make_and_store_3pid_invite.
         make_invite_mock.assert_called_once()
+
+    def test_400_without_id_access_token(self) -> None:
+        """
+        Test allowing/blocking threepid invites with a spam-check module.
+
+        In this test, we use the more recent API in which callbacks return a `Union[Codes, Literal["NOT_SPAM"]]`."""
+        make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0)))
+        self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock
+        self.hs.get_identity_handler().lookup_3pid = Mock(
+            return_value=make_awaitable(None),
+        )
+
+        mock = Mock(
+            return_value=make_awaitable(synapse.module_api.NOT_SPAM),
+            spec=lambda *x: None,
+        )
+        self.hs.get_spam_checker()._user_may_send_3pid_invite_callbacks.append(mock)
+
+        # Send a 3PID invite into the room and check that it succeeded.
+        email_to_invite = "teresa@example.com"
+        channel = self.make_request(
+            method="POST",
+            path="/rooms/" + self.room_id + "/invite",
+            content={
+                "id_server": "example.com",
+                "medium": "email",
+                "address": email_to_invite,
+            },
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.code, 400)

From 5668dcda1978da33ce0c0e7956edf12fb5f427ea Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Fri, 22 Jul 2022 15:20:19 +0200
Subject: [PATCH 07/21] minify test

---
 tests/rest/client/test_rooms.py | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index 2d2a5b962886..fd6ba4a942a3 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3464,31 +3464,13 @@ def test_threepid_invite_spamcheck(self) -> None:
         make_invite_mock.assert_called_once()
 
     def test_400_without_id_access_token(self) -> None:
-        """
-        Test allowing/blocking threepid invites with a spam-check module.
-
-        In this test, we use the more recent API in which callbacks return a `Union[Codes, Literal["NOT_SPAM"]]`."""
-        make_invite_mock = Mock(return_value=make_awaitable((Mock(event_id="abc"), 0)))
-        self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock
-        self.hs.get_identity_handler().lookup_3pid = Mock(
-            return_value=make_awaitable(None),
-        )
-
-        mock = Mock(
-            return_value=make_awaitable(synapse.module_api.NOT_SPAM),
-            spec=lambda *x: None,
-        )
-        self.hs.get_spam_checker()._user_may_send_3pid_invite_callbacks.append(mock)
-
-        # Send a 3PID invite into the room and check that it succeeded.
-        email_to_invite = "teresa@example.com"
         channel = self.make_request(
             method="POST",
             path="/rooms/" + self.room_id + "/invite",
             content={
                 "id_server": "example.com",
                 "medium": "email",
-                "address": email_to_invite,
+                "address": "teresa@example.com"
             },
             access_token=self.tok,
         )

From 6e908e42604cd4631b3e812501a05e8668ae869e Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 25 Jul 2022 09:33:40 +0200
Subject: [PATCH 08/21] missing comma

---
 tests/rest/client/test_rooms.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index fd6ba4a942a3..35859bcddc69 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3470,7 +3470,7 @@ def test_400_without_id_access_token(self) -> None:
             content={
                 "id_server": "example.com",
                 "medium": "email",
-                "address": "teresa@example.com"
+                "address": "teresa@example.com",
             },
             access_token=self.tok,
         )

From 669827995b7430d8412c642a0bb628c6114e939c Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 25 Jul 2022 20:36:55 +0200
Subject: [PATCH 09/21] Address comments

---
 synapse/handlers/identity.py       | 35 +++++-------------------------
 tests/rest/client/test_identity.py | 14 ------------
 tests/rest/client/test_rooms.py    |  7 +++++-
 3 files changed, 12 insertions(+), 44 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index a6ad718933f1..eb7070f874da 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -763,45 +763,22 @@ async def ask_id_server_for_third_party_invite(
             raise SynapseError(500, "Timed out contacting identity server")
         except HttpResponseException as e:
             logger.info("Failed to POST %s with JSON: %s", url, e)
-
         if data is None:
-            key_validity_url = "%s%s/_matrix/identity/api/v1/pubkey/isvalid" % (
-                id_server_scheme,
-                id_server,
-            )
-            url = base_url + "/api/v1/store-invite"
-
+            # Some identity servers may only support application/x-www-form-urlencoded
+            # types. This is especially true with old instances of Sydent, see
+            # https://github.com/matrix-org/sydent/pull/170
             try:
-                data = await self.blacklisting_http_client.post_json_get_json(
+                data = await self.blacklisting_http_client.post_urlencoded_get_json(
                     url, invite_config
                 )
-            except RequestTimedOutError:
-                raise SynapseError(500, "Timed out contacting identity server")
             except HttpResponseException as e:
                 logger.warning(
-                    "Error trying to call /store-invite on %s%s: %s",
+                    "Error calling /store-invite on %s%s with fallback " "encoding: %s",
                     id_server_scheme,
                     id_server,
                     e,
                 )
-
-            if data is None:
-                # Some identity servers may only support application/x-www-form-urlencoded
-                # types. This is especially true with old instances of Sydent, see
-                # https://github.com/matrix-org/sydent/pull/170
-                try:
-                    data = await self.blacklisting_http_client.post_urlencoded_get_json(
-                        url, invite_config
-                    )
-                except HttpResponseException as e:
-                    logger.warning(
-                        "Error calling /store-invite on %s%s with fallback "
-                        "encoding: %s",
-                        id_server_scheme,
-                        id_server,
-                        e,
-                    )
-                    raise e
+                raise e
 
         # TODO: Check for success
         token = data["token"]
diff --git a/tests/rest/client/test_identity.py b/tests/rest/client/test_identity.py
index 0a7110f3929f..b0c821574466 100644
--- a/tests/rest/client/test_identity.py
+++ b/tests/rest/client/test_identity.py
@@ -25,7 +25,6 @@
 
 
 class IdentityTestCase(unittest.HomeserverTestCase):
-
     servlets = [
         synapse.rest.admin.register_servlets_for_client_rest_resource,
         room.register_servlets,
@@ -33,7 +32,6 @@ class IdentityTestCase(unittest.HomeserverTestCase):
     ]
 
     def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
-
         config = self.default_config()
         config["enable_3pid_lookup"] = False
         self.hs = self.setup_test_homeserver(config=config)
@@ -61,15 +59,3 @@ def test_3pid_lookup_disabled(self) -> None:
             b"POST", request_url, request_data, access_token=tok
         )
         self.assertEqual(channel.code, HTTPStatus.FORBIDDEN, channel.result)
-
-
-# {'version': b'1.1', 'code': b'400', 'reason': b'Bad Request',
-# 'headers': [
-#     (b'Server', b'1'), (b'Date', b'Mon,
-#         11 Jul 2022 10: 58: 19 GMT'),
-#         (b'Content-Type', b'application/json'), (b'Cache-Control', b'no-cache, no-store, must-revalidate'), (b'Access-Control-Allow-Origin', b'*'), (b'Access-Control-Allow-Methods', b'GET, HEAD, POST, PUT, DELETE, OPTIONS'), (b'Access-Control-Allow-Headers', b'X-Requested-With, Content-Type, Authorization, Date')
-#     ], 'body': b'{
-#         "errcode": "M_MISSING_PARAM",
-#         "error":"Missing params: [\'user_id\']"
-#     }', 'done': True
-# }
diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index 35859bcddc69..3f29fe477daf 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3463,7 +3463,11 @@ def test_threepid_invite_spamcheck(self) -> None:
         # Also check that it stopped before calling _make_and_store_3pid_invite.
         make_invite_mock.assert_called_once()
 
-    def test_400_without_id_access_token(self) -> None:
+    def test_400_missing_param_without_id_access_token(self) -> None:
+        """
+        Test checking that invite returns 400 missing param
+        if we do not include id_access_token.
+        """
         channel = self.make_request(
             method="POST",
             path="/rooms/" + self.room_id + "/invite",
@@ -3475,3 +3479,4 @@ def test_400_without_id_access_token(self) -> None:
             access_token=self.tok,
         )
         self.assertEqual(channel.code, 400)
+        self.assertEqual(channel.json_body["errcode"], "M_MISSING_PARAM")

From 9a042346666d0924647e1b7af1d2c95c09c2e863 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jacek=20Ku=C5=9Bnierz?= <kusnierz@protonmail.com>
Date: Mon, 15 Aug 2022 14:58:59 +0200
Subject: [PATCH 10/21] Update tests/rest/client/test_rooms.py

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 tests/rest/client/test_rooms.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index 61a535a82d54..c7eb88d33f3d 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3464,7 +3464,7 @@ def test_threepid_invite_spamcheck(self) -> None:
 
     def test_400_missing_param_without_id_access_token(self) -> None:
         """
-        Test checking that invite returns 400 missing param
+        Test that a 3pid invite request returns 400 M_MISSING_PARAM
         if we do not include id_access_token.
         """
         channel = self.make_request(

From e115f60d7bbd60a2badc6b8f61d2c81a67cc7bc9 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 15:20:39 +0200
Subject: [PATCH 11/21] addressed the comments

Signed-off-by: Jacek Kusnierz <jacek.kusnierz@tum.de>
---
 synapse/handlers/identity.py              | 19 +------------------
 synapse/rest/client/room.py               | 14 ++++++++++----
 synapse/rest/media/v1/media_repository.py |  1 -
 tests/rest/client/test_shadow_banned.py   |  2 +-
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 145988e82498..e4a9a8f02e3a 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -799,14 +799,13 @@ async def ask_id_server_for_third_party_invite(
         # Add the identity service access token to the JSON body and use the v2
         # Identity Service endpoints
         data = None
-        base_url = "%s%s/_matrix/identity" % (id_server_scheme, id_server)
 
         key_validity_url = "%s%s/_matrix/identity/v2/pubkey/isvalid" % (
             id_server_scheme,
             id_server,
         )
 
-        url = base_url + "/v2/store-invite"
+        url = "%s%s/_matrix/identity/v2/store-invite" % (id_server_scheme, id_server)
         try:
             data = await self.blacklisting_http_client.post_json_get_json(
                 url,
@@ -817,22 +816,6 @@ async def ask_id_server_for_third_party_invite(
             raise SynapseError(500, "Timed out contacting identity server")
         except HttpResponseException as e:
             logger.info("Failed to POST %s with JSON: %s", url, e)
-        if data is None:
-            # Some identity servers may only support application/x-www-form-urlencoded
-            # types. This is especially true with old instances of Sydent, see
-            # https://github.com/matrix-org/sydent/pull/170
-            try:
-                data = await self.blacklisting_http_client.post_urlencoded_get_json(
-                    url, invite_config
-                )
-            except HttpResponseException as e:
-                logger.warning(
-                    "Error calling /store-invite on %s%s with fallback " "encoding: %s",
-                    id_server_scheme,
-                    id_server,
-                    e,
-                )
-                raise e
 
         # TODO: Check for success
         token = data["token"]
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 71080e076851..656fcf9a20c0 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -908,10 +908,16 @@ async def on_POST(
         return 200, return_value
 
     def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
-        for key in {"id_server", "medium", "address", "id_access_token"}:
-            if key not in content:
-                return False
-        return True
+        # if the request has medium and address keys, we should treat it as a 3pid invite.
+        if all(key in content for key in ("medium", "address")):
+            if all(key in content for key in ("id_server", "id_access_token")):
+                return True
+            # if it does not have id_server or id_access_token, we should treat that as an error.
+            raise SynapseError(
+                400,
+                "`id_server` and `id_access_token` are required when doing 3pid invite",
+            )
+        return False
 
     def on_PUT(
         self, request: SynapseRequest, room_id: str, membership_action: str, txn_id: str
diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py
index 7435fd9130c5..9dd3c8d4bbd4 100644
--- a/synapse/rest/media/v1/media_repository.py
+++ b/synapse/rest/media/v1/media_repository.py
@@ -64,7 +64,6 @@
 
 logger = logging.getLogger(__name__)
 
-
 # How often to run the background job to update the "recently accessed"
 # attribute of local and remote media.
 UPDATE_RECENTLY_ACCESSED_TS = 60 * 1000  # 1 minute
diff --git a/tests/rest/client/test_shadow_banned.py b/tests/rest/client/test_shadow_banned.py
index 21609231359a..f8301730599c 100644
--- a/tests/rest/client/test_shadow_banned.py
+++ b/tests/rest/client/test_shadow_banned.py
@@ -101,7 +101,7 @@ def test_invite_3pid(self) -> None:
                 "id_server": "test",
                 "medium": "email",
                 "address": "test@test.test",
-                "id_access_token": self.banned_access_token,
+                "id_access_token": "anytoken",
             },
             access_token=self.banned_access_token,
         )

From f2a74b2cb6d669e2d836bc155afb17a7d4407438 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 15:30:55 +0200
Subject: [PATCH 12/21] add missing check

---
 synapse/handlers/identity.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index e4a9a8f02e3a..f64fbfb570d9 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -817,7 +817,11 @@ async def ask_id_server_for_third_party_invite(
         except HttpResponseException as e:
             logger.info("Failed to POST %s with JSON: %s", url, e)
 
-        # TODO: Check for success
+        if not (data and data.get("token") and data.get("display_name")):
+            raise SynapseError(
+                500, "No data with token was received from identity server"
+            )
+
         token = data["token"]
         public_keys = data.get("public_keys", [])
         if "public_key" in data:

From f9600bbe08e9cd6f1f3db8c352b98efafa4e437b Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 16:53:19 +0200
Subject: [PATCH 13/21] fixed test

---
 synapse/handlers/room.py    |  2 ++
 synapse/rest/client/room.py | 28 +++++++++++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 742ca48fbca3..6d3daf18524a 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -64,6 +64,7 @@
 from synapse.handlers.relations import BundledAggregations
 from synapse.module_api import NOT_SPAM
 from synapse.rest.admin._base import assert_user_is_admin
+from synapse.rest.client.room import has_3pid_invite_keys
 from synapse.storage.state import StateFilter
 from synapse.streams import EventSource
 from synapse.types import (
@@ -978,6 +979,7 @@ async def create_room(
                 depth += 1
 
         for invite_3pid in invite_3pid_list:
+            assert has_3pid_invite_keys(invite_3pid)
             id_server = invite_3pid["id_server"]
             id_access_token = invite_3pid["id_access_token"]
             address = invite_3pid["address"]
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 656fcf9a20c0..536a68956ad1 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -61,6 +61,20 @@
 logger = logging.getLogger(__name__)
 
 
+def has_3pid_invite_keys(content: JsonDict) -> bool:
+    # if the request has medium and address keys, we should treat it as a 3pid invite.
+    if all(key in content for key in ("medium", "address")):
+        if all(key in content for key in ("id_server", "id_access_token")):
+            return True
+        # if it does not have id_server or id_access_token, we should treat that as an error.
+        raise SynapseError(
+            400,
+            "`id_server` and `id_access_token` are required when doing 3pid invite",
+            Codes.MISSING_PARAM,
+        )
+    return False
+
+
 class TransactionRestServlet(RestServlet):
     def __init__(self, hs: "HomeServer"):
         super().__init__()
@@ -860,7 +874,7 @@ async def on_POST(
             # cheekily send invalid bodies.
             content = {}
 
-        if membership_action == "invite" and self._has_3pid_invite_keys(content):
+        if membership_action == "invite" and has_3pid_invite_keys(content):
             try:
                 await self.room_member_handler.do_3pid_invite(
                     room_id,
@@ -907,18 +921,6 @@ async def on_POST(
 
         return 200, return_value
 
-    def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
-        # if the request has medium and address keys, we should treat it as a 3pid invite.
-        if all(key in content for key in ("medium", "address")):
-            if all(key in content for key in ("id_server", "id_access_token")):
-                return True
-            # if it does not have id_server or id_access_token, we should treat that as an error.
-            raise SynapseError(
-                400,
-                "`id_server` and `id_access_token` are required when doing 3pid invite",
-            )
-        return False
-
     def on_PUT(
         self, request: SynapseRequest, room_id: str, membership_action: str, txn_id: str
     ) -> Awaitable[Tuple[int, JsonDict]]:

From 0073118ada7771502968e5296daa21bc13543d4a Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 17:24:19 +0200
Subject: [PATCH 14/21] catch missing param for 3pid

---
 synapse/handlers/room.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 6d3daf18524a..3d2e6188ccc3 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -370,7 +370,9 @@ async def _update_upgraded_room_pls(
         # required PL above that.
 
         pl_content = copy_and_fixup_power_levels_contents(old_room_pl_state.content)
-        users_default: int = pl_content.get("users_default", 0)  # type: ignore[assignment]
+        users_default: int = pl_content.get(
+            "users_default", 0
+        )  # type: ignore[assignment]
         restricted_level = max(users_default + 1, 50)
 
         updated = False
@@ -979,7 +981,11 @@ async def create_room(
                 depth += 1
 
         for invite_3pid in invite_3pid_list:
-            assert has_3pid_invite_keys(invite_3pid)
+            try:
+                assert has_3pid_invite_keys(invite_3pid)
+            except Codes.MISSING_PARAM as e:
+                logger.error(e, invite_3pid)
+                continue
             id_server = invite_3pid["id_server"]
             id_access_token = invite_3pid["id_access_token"]
             address = invite_3pid["address"]

From ebcdbba20c6e24df065f01223999341cc51c75a7 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 17:28:12 +0200
Subject: [PATCH 15/21] synapseerror

---
 synapse/handlers/room.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 3d2e6188ccc3..dc49b87d9393 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -983,7 +983,7 @@ async def create_room(
         for invite_3pid in invite_3pid_list:
             try:
                 assert has_3pid_invite_keys(invite_3pid)
-            except Codes.MISSING_PARAM as e:
+            except SynapseError as e:
                 logger.error(e, invite_3pid)
                 continue
             id_server = invite_3pid["id_server"]

From b97b27fc4de4c9b3f0ae2b34fdd806c3321cf29d Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Mon, 15 Aug 2022 20:47:16 +0200
Subject: [PATCH 16/21] validate earlier

---
 synapse/handlers/room.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index dc49b87d9393..293bd096159f 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -735,6 +735,14 @@ async def create_room(
         invite_3pid_list = config.get("invite_3pid", [])
         invite_list = config.get("invite", [])
 
+        # validate each entry
+        for invite_3pid in invite_3pid_list:
+            try:
+                assert has_3pid_invite_keys(invite_3pid)
+            except SynapseError as e:
+                logger.error(e, invite_3pid)
+                invite_3pid_list.remove(invite_list)
+
         if not is_requester_admin:
             spam_check = await self.spam_checker.user_may_create_room(user_id)
             if spam_check != NOT_SPAM:
@@ -981,11 +989,6 @@ async def create_room(
                 depth += 1
 
         for invite_3pid in invite_3pid_list:
-            try:
-                assert has_3pid_invite_keys(invite_3pid)
-            except SynapseError as e:
-                logger.error(e, invite_3pid)
-                continue
             id_server = invite_3pid["id_server"]
             id_access_token = invite_3pid["id_access_token"]
             address = invite_3pid["address"]

From 21d9be120559b4e49cfa3f3b0fe5d6eb22f4c816 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Tue, 23 Aug 2022 16:47:12 +0200
Subject: [PATCH 17/21] Next iteration

Signed-off-by: Jacek Kusnierz <jacek.kusnierz@tum.de>
---
 synapse/handlers/identity.py |  5 ---
 synapse/handlers/room.py     | 20 +++++++----
 synapse/rest/client/room.py  | 65 +++++++++++++++++-------------------
 3 files changed, 43 insertions(+), 47 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index cf9f263ff8dc..8c79ca034546 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -763,11 +763,6 @@ async def ask_id_server_for_third_party_invite(
         except HttpResponseException as e:
             logger.info("Failed to POST %s with JSON: %s", url, e)
 
-        if not (data and data.get("token") and data.get("display_name")):
-            raise SynapseError(
-                500, "No data with token was received from identity server"
-            )
-
         token = data["token"]
         public_keys = data.get("public_keys", [])
         if "public_key" in data:
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 92774d8f80c3..cce4dabea0bc 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -64,7 +64,6 @@
 from synapse.handlers.relations import BundledAggregations
 from synapse.module_api import NOT_SPAM
 from synapse.rest.admin._base import assert_user_is_admin
-from synapse.rest.client.room import has_3pid_invite_keys
 from synapse.storage.state import StateFilter
 from synapse.streams import EventSource
 from synapse.types import (
@@ -681,6 +680,12 @@ async def _move_aliases_to_new_room(
             # we returned the new room to the client at this point.
             logger.error("Unable to send updated alias events in new room: %s", e)
 
+    def has_all_3pid_keys(self, invite_3pid: dict) -> bool:
+        return all(
+            key in invite_3pid
+            for key in ("medium", "address", "id_server", "id_access_token")
+        )
+
     async def create_room(
         self,
         requester: Requester,
@@ -735,13 +740,14 @@ async def create_room(
         invite_3pid_list = config.get("invite_3pid", [])
         invite_list = config.get("invite", [])
 
-        # validate each entry
+        # validate each entry for correctness
         for invite_3pid in invite_3pid_list:
-            try:
-                assert has_3pid_invite_keys(invite_3pid)
-            except SynapseError as e:
-                logger.error(e, invite_3pid)
-                invite_3pid_list.remove(invite_list)
+            if not self.has_all_3pid_keys(invite_3pid):
+                raise SynapseError(
+                    400,
+                    f"`id_server` and `id_access_token` are required when doing 3pid invite, caused by {invite_3pid}",
+                    Codes.MISSING_PARAM,
+                )
 
         if not is_requester_admin:
             spam_check = await self.spam_checker.user_may_create_room(user_id)
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 3e2958929c59..13d12bb5785b 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -65,20 +65,6 @@
 logger = logging.getLogger(__name__)
 
 
-def has_3pid_invite_keys(content: JsonDict) -> bool:
-    # if the request has medium and address keys, we should treat it as a 3pid invite.
-    if all(key in content for key in ("medium", "address")):
-        if all(key in content for key in ("id_server", "id_access_token")):
-            return True
-        # if it does not have id_server or id_access_token, we should treat that as an error.
-        raise SynapseError(
-            400,
-            "`id_server` and `id_access_token` are required when doing 3pid invite",
-            Codes.MISSING_PARAM,
-        )
-    return False
-
-
 class _RoomSize(Enum):
     """
     Enum to differentiate sizes of rooms. This is a pretty good approximation
@@ -930,6 +916,9 @@ def __init__(self, hs: "HomeServer"):
         self.room_member_handler = hs.get_room_member_handler()
         self.auth = hs.get_auth()
 
+    def has_3pid_invite_keys(self, content: JsonDict) -> bool:
+        return all(key in content for key in ("medium", "address"))
+
     def register(self, http_server: HttpServer) -> None:
         # /rooms/$roomid/[invite|join|leave]
         PATTERNS = (
@@ -960,22 +949,29 @@ async def on_POST(
             # cheekily send invalid bodies.
             content = {}
 
-        if membership_action == "invite" and has_3pid_invite_keys(content):
-            try:
-                await self.room_member_handler.do_3pid_invite(
-                    room_id,
-                    requester.user,
-                    content["medium"],
-                    content["address"],
-                    content["id_server"],
-                    requester,
-                    txn_id,
-                    content["id_access_token"],
+        if self.has_3pid_invite_keys(content):
+            if not all(key in content for key in ("id_server", "id_access_token")):
+                raise SynapseError(
+                    400,
+                    "`id_server` and `id_access_token` are required when doing 3pid invite",
+                    Codes.MISSING_PARAM,
                 )
-            except ShadowBanError:
-                # Pretend the request succeeded.
-                pass
-            return 200, {}
+            if membership_action == "invite":
+                try:
+                    await self.room_member_handler.do_3pid_invite(
+                        room_id,
+                        requester.user,
+                        content["medium"],
+                        content["address"],
+                        content["id_server"],
+                        requester,
+                        txn_id,
+                        content["id_access_token"],
+                    )
+                except ShadowBanError:
+                    # Pretend the request succeeded.
+                    pass
+                return 200, {}
 
         target = requester.user
         if membership_action in ["invite", "ban", "unban", "kick"]:
@@ -1128,12 +1124,11 @@ async def on_PUT(
 
 class RoomAliasListServlet(RestServlet):
     PATTERNS = [
-                   re.compile(
-                       r"^/_matrix/client/unstable/org\.matrix\.msc2432"
-                       r"/rooms/(?P<room_id>[^/]*)/aliases"
-                   ),
-               ] + list(
-        client_patterns("/rooms/(?P<room_id>[^/]*)/aliases$", unstable=False))
+        re.compile(
+            r"^/_matrix/client/unstable/org\.matrix\.msc2432"
+            r"/rooms/(?P<room_id>[^/]*)/aliases"
+        ),
+    ] + list(client_patterns("/rooms/(?P<room_id>[^/]*)/aliases$", unstable=False))
 
     def __init__(self, hs: "HomeServer"):
         super().__init__()

From caeee4d46efc198e6103888c709be14295d79318 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Tue, 23 Aug 2022 16:53:08 +0200
Subject: [PATCH 18/21] allow exception to go up

---
 synapse/handlers/identity.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 8c79ca034546..93d09e993961 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -760,8 +760,6 @@ async def ask_id_server_for_third_party_invite(
             )
         except RequestTimedOutError:
             raise SynapseError(500, "Timed out contacting identity server")
-        except HttpResponseException as e:
-            logger.info("Failed to POST %s with JSON: %s", url, e)
 
         token = data["token"]
         public_keys = data.get("public_keys", [])

From 3dfd8fb7ad23389132d4da13c31ac89fbe16d031 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Thu, 25 Aug 2022 19:28:05 +0200
Subject: [PATCH 19/21] beautify code

---
 synapse/handlers/room.py    | 8 +++-----
 synapse/rest/client/room.py | 4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index cce4dabea0bc..e4be50275797 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -369,9 +369,7 @@ async def _update_upgraded_room_pls(
         # required PL above that.
 
         pl_content = copy_and_fixup_power_levels_contents(old_room_pl_state.content)
-        users_default: int = pl_content.get(
-            "users_default", 0
-        )  # type: ignore[assignment]
+        users_default: int = pl_content.get("users_default", 0)  # type: ignore[assignment]
         restricted_level = max(users_default + 1, 50)
 
         updated = False
@@ -680,7 +678,7 @@ async def _move_aliases_to_new_room(
             # we returned the new room to the client at this point.
             logger.error("Unable to send updated alias events in new room: %s", e)
 
-    def has_all_3pid_keys(self, invite_3pid: dict) -> bool:
+    def _has_all_3pid_keys(self, invite_3pid: dict) -> bool:
         return all(
             key in invite_3pid
             for key in ("medium", "address", "id_server", "id_access_token")
@@ -742,7 +740,7 @@ async def create_room(
 
         # validate each entry for correctness
         for invite_3pid in invite_3pid_list:
-            if not self.has_all_3pid_keys(invite_3pid):
+            if not self._has_all_3pid_keys(invite_3pid):
                 raise SynapseError(
                     400,
                     f"`id_server` and `id_access_token` are required when doing 3pid invite, caused by {invite_3pid}",
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 13d12bb5785b..4b3c6ebe95ca 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -916,7 +916,7 @@ def __init__(self, hs: "HomeServer"):
         self.room_member_handler = hs.get_room_member_handler()
         self.auth = hs.get_auth()
 
-    def has_3pid_invite_keys(self, content: JsonDict) -> bool:
+    def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
         return all(key in content for key in ("medium", "address"))
 
     def register(self, http_server: HttpServer) -> None:
@@ -949,7 +949,7 @@ async def on_POST(
             # cheekily send invalid bodies.
             content = {}
 
-        if self.has_3pid_invite_keys(content):
+        if self._has_3pid_invite_keys(content):
             if not all(key in content for key in ("id_server", "id_access_token")):
                 raise SynapseError(
                     400,

From 5b237aa131e452be6129ecb6b2a0ca5fc5942364 Mon Sep 17 00:00:00 2001
From: Jacek Kusnierz <jacek.kusnierz@tum.de>
Date: Fri, 26 Aug 2022 14:24:35 +0200
Subject: [PATCH 20/21] hopefully addresses the comments

---
 synapse/handlers/room.py    | 21 +++++++++----------
 synapse/rest/client/room.py | 42 ++++++++++++++++++-------------------
 2 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index e4be50275797..2883f3e53cab 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -19,6 +19,7 @@
 import random
 import string
 from collections import OrderedDict
+from http import HTTPStatus
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -678,12 +679,6 @@ async def _move_aliases_to_new_room(
             # we returned the new room to the client at this point.
             logger.error("Unable to send updated alias events in new room: %s", e)
 
-    def _has_all_3pid_keys(self, invite_3pid: dict) -> bool:
-        return all(
-            key in invite_3pid
-            for key in ("medium", "address", "id_server", "id_access_token")
-        )
-
     async def create_room(
         self,
         requester: Requester,
@@ -711,8 +706,8 @@ async def create_room(
                 was, requested, `room_alias`. Secondly, the stream_id of the
                 last persisted event.
         Raises:
-            SynapseError if the room ID couldn't be stored, or something went
-            horribly wrong.
+            SynapseError if the room ID couldn't be stored, 3pid invitation config
+            validation failed, or something went horribly wrong.
             ResourceLimitError if server is blocked to some resource being
             exceeded
         """
@@ -740,10 +735,14 @@ async def create_room(
 
         # validate each entry for correctness
         for invite_3pid in invite_3pid_list:
-            if not self._has_all_3pid_keys(invite_3pid):
+            if not all(
+                key in invite_3pid
+                for key in ("medium", "address", "id_server", "id_access_token")
+            ):
                 raise SynapseError(
-                    400,
-                    f"`id_server` and `id_access_token` are required when doing 3pid invite, caused by {invite_3pid}",
+                    HTTPStatus.BAD_REQUEST,
+                    "all of: `medium`, `address`, `id_server` and `id_access_token` "
+                    f"are required when doing 3pid invite, caused by {invite_3pid}",
                     Codes.MISSING_PARAM,
                 )
 
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 4b3c6ebe95ca..c05a0b718c1c 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -17,6 +17,7 @@
 import logging
 import re
 from enum import Enum
+from http import HTTPStatus
 from typing import TYPE_CHECKING, Awaitable, Dict, List, Optional, Tuple
 from urllib import parse as urlparse
 
@@ -916,9 +917,6 @@ def __init__(self, hs: "HomeServer"):
         self.room_member_handler = hs.get_room_member_handler()
         self.auth = hs.get_auth()
 
-    def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
-        return all(key in content for key in ("medium", "address"))
-
     def register(self, http_server: HttpServer) -> None:
         # /rooms/$roomid/[invite|join|leave]
         PATTERNS = (
@@ -949,29 +947,31 @@ async def on_POST(
             # cheekily send invalid bodies.
             content = {}
 
-        if self._has_3pid_invite_keys(content):
+        if membership_action == "invite" and all(
+            key in content for key in ("medium", "address")
+        ):
             if not all(key in content for key in ("id_server", "id_access_token")):
                 raise SynapseError(
-                    400,
+                    HTTPStatus.BAD_REQUEST,
                     "`id_server` and `id_access_token` are required when doing 3pid invite",
                     Codes.MISSING_PARAM,
                 )
-            if membership_action == "invite":
-                try:
-                    await self.room_member_handler.do_3pid_invite(
-                        room_id,
-                        requester.user,
-                        content["medium"],
-                        content["address"],
-                        content["id_server"],
-                        requester,
-                        txn_id,
-                        content["id_access_token"],
-                    )
-                except ShadowBanError:
-                    # Pretend the request succeeded.
-                    pass
-                return 200, {}
+
+            try:
+                await self.room_member_handler.do_3pid_invite(
+                    room_id,
+                    requester.user,
+                    content["medium"],
+                    content["address"],
+                    content["id_server"],
+                    requester,
+                    txn_id,
+                    content["id_access_token"],
+                )
+            except ShadowBanError:
+                # Pretend the request succeeded.
+                pass
+            return 200, {}
 
         target = requester.user
         if membership_action in ["invite", "ban", "unban", "kick"]:

From 27951c5a53225843e68defcae5da4cd3d61a683b Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 31 Aug 2022 12:34:11 +0100
Subject: [PATCH 21/21] Update synapse/handlers/room.py

---
 synapse/handlers/room.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 2883f3e53cab..a28e0194425e 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -741,8 +741,8 @@ async def create_room(
             ):
                 raise SynapseError(
                     HTTPStatus.BAD_REQUEST,
-                    "all of: `medium`, `address`, `id_server` and `id_access_token` "
-                    f"are required when doing 3pid invite, caused by {invite_3pid}",
+                    "all of `medium`, `address`, `id_server` and `id_access_token` "
+                    "are required when making a 3pid invite",
                     Codes.MISSING_PARAM,
                 )