Fixes #13206 Signed-off-by: Jacek Kusnierz jacek.kusnierz@tum.detags/v1.67.0rc1
@@ -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. |
@@ -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: | |||
@@ -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 | |||
@@ -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]: | |||
@@ -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]]: | |||
@@ -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 | |||
@@ -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( | |||
@@ -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") |
@@ -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) | |||