diff --git a/changelog.d/13241.removal b/changelog.d/13241.removal
new file mode 100644
index 0000000000..60b0e7969c
--- /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`, which was not permitted by the spec. Contributed by @Vetchu.
\ No newline at end of file
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 9571d461c8..93d09e9939 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -538,11 +538,7 @@ class IdentityHandler:
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.
@@ -557,60 +553,15 @@ class IdentityHandler:
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").
-
- 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_v2(
+ id_server, id_access_token, medium, address
)
-
- 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
+ return results
+ except Exception as e:
+ logger.warning("Error when looking up hashing details: %s", e)
+ return None
async def _lookup_3pid_v2(
self, id_server: str, id_access_token: str, medium: str, address: str
@@ -739,7 +690,7 @@ class IdentityHandler:
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.
@@ -760,7 +711,7 @@ class IdentityHandler:
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:
@@ -792,71 +743,24 @@ class IdentityHandler:
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
-
- if data is None:
- key_validity_url = "%s%s/_matrix/identity/api/v1/pubkey/isvalid" % (
- id_server_scheme,
- id_server,
+ 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,
+ invite_config,
+ {"Authorization": create_id_access_token_header(id_access_token)},
)
- url = base_url + "/api/v1/store-invite"
-
- try:
- data = await self.blacklisting_http_client.post_json_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",
- 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
-
- # TODO: Check for success
+ except RequestTimedOutError:
+ raise SynapseError(500, "Timed out contacting 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 f64a8690a5..33e9a87002 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -19,6 +19,7 @@ import math
import random
import string
from collections import OrderedDict
+from http import HTTPStatus
from typing import (
TYPE_CHECKING,
Any,
@@ -704,8 +705,8 @@ class RoomCreationHandler:
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
"""
@@ -731,6 +732,19 @@ class RoomCreationHandler:
invite_3pid_list = config.get("invite_3pid", [])
invite_list = config.get("invite", [])
+ # validate each entry for correctness
+ for invite_3pid in invite_3pid_list:
+ if not all(
+ key in invite_3pid
+ for key in ("medium", "address", "id_server", "id_access_token")
+ ):
+ raise SynapseError(
+ HTTPStatus.BAD_REQUEST,
+ "all of `medium`, `address`, `id_server` and `id_access_token` "
+ "are required when making a 3pid invite",
+ Codes.MISSING_PARAM,
+ )
+
if not is_requester_admin:
spam_check = await self.spam_checker.user_may_create_room(user_id)
if spam_check != NOT_SPAM:
@@ -978,7 +992,7 @@ class RoomCreationHandler:
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 e726997d83..5d4adf5bfd 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1382,7 +1382,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
id_server: str,
requester: Requester,
txn_id: Optional[str],
- id_access_token: Optional[str] = None,
+ id_access_token: str,
prev_event_ids: Optional[List[str]] = None,
depth: Optional[int] = None,
) -> Tuple[str, int]:
@@ -1397,7 +1397,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
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.
depth: Override the depth used to order the event in the DAG.
prev_event_ids: The event IDs to use as the prev events
Should normally be set to None, which will cause the depth to be calculated
@@ -1494,7 +1494,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
room_id: str,
user: UserID,
txn_id: Optional[str],
- id_access_token: Optional[str] = None,
+ id_access_token: str,
prev_event_ids: Optional[List[str]] = None,
depth: Optional[int] = None,
) -> Tuple[EventBase, int]:
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 0e2834008e..0bca012535 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
@@ -947,7 +948,16 @@ class RoomMembershipRestServlet(TransactionRestServlet):
# cheekily send invalid bodies.
content = {}
- if membership_action == "invite" and 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(
+ HTTPStatus.BAD_REQUEST,
+ "`id_server` and `id_access_token` are required when doing 3pid invite",
+ Codes.MISSING_PARAM,
+ )
+
try:
await self.room_member_handler.do_3pid_invite(
room_id,
@@ -957,7 +967,7 @@ class RoomMembershipRestServlet(TransactionRestServlet):
content["id_server"],
requester,
txn_id,
- content.get("id_access_token"),
+ content["id_access_token"],
)
except ShadowBanError:
# Pretend the request succeeded.
@@ -994,12 +1004,6 @@ class RoomMembershipRestServlet(TransactionRestServlet):
return 200, return_value
- def _has_3pid_invite_keys(self, content: JsonDict) -> bool:
- for key in {"id_server", "medium", "address"}:
- if key not in content:
- return False
- return True
-
def on_PUT(
self, request: SynapseRequest, room_id: str, membership_action: str, txn_id: str
) -> Awaitable[Tuple[int, JsonDict]]:
diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py
index 7435fd9130..9dd3c8d4bb 100644
--- a/synapse/rest/media/v1/media_repository.py
+++ b/synapse/rest/media/v1/media_repository.py
@@ -64,7 +64,6 @@ if TYPE_CHECKING:
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_identity.py b/tests/rest/client/test_identity.py
index dc17c9d113..b0c8215744 100644
--- a/tests/rest/client/test_identity.py
+++ b/tests/rest/client/test_identity.py
@@ -25,7 +25,6 @@ from tests import unittest
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)
@@ -54,6 +52,7 @@ class IdentityTestCase(unittest.HomeserverTestCase):
"id_server": "testis",
"medium": "email",
"address": "test@example.com",
+ "id_access_token": tok,
}
request_url = ("/rooms/%s/invite" % (room_id)).encode("ascii")
channel = self.make_request(
diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index aa2f578441..c7eb88d33f 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -3461,3 +3461,21 @@ class ThreepidInviteTestCase(unittest.HomeserverTestCase):
# Also check that it stopped before calling _make_and_store_3pid_invite.
make_invite_mock.assert_called_once()
+
+ def test_400_missing_param_without_id_access_token(self) -> None:
+ """
+ Test that a 3pid invite request returns 400 M_MISSING_PARAM
+ if we do not include id_access_token.
+ """
+ channel = self.make_request(
+ method="POST",
+ path="/rooms/" + self.room_id + "/invite",
+ content={
+ "id_server": "example.com",
+ "medium": "email",
+ "address": "teresa@example.com",
+ },
+ access_token=self.tok,
+ )
+ self.assertEqual(channel.code, 400)
+ self.assertEqual(channel.json_body["errcode"], "M_MISSING_PARAM")
diff --git a/tests/rest/client/test_shadow_banned.py b/tests/rest/client/test_shadow_banned.py
index c50f034b34..c807a37bc2 100644
--- a/tests/rest/client/test_shadow_banned.py
+++ b/tests/rest/client/test_shadow_banned.py
@@ -97,7 +97,12 @@ class RoomTestCase(_ShadowBannedBase):
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": "anytoken",
+ },
access_token=self.banned_access_token,
)
self.assertEqual(200, channel.code, channel.result)
|
