0
0
Fork 1
mirror of https://mau.dev/maunium/synapse.git synced 2024-12-14 00:03:46 +01:00

Call appservices on modern paths, falling back to legacy paths. (#15317)

This uses the specced /_matrix/app/v1/... paths instead of the
"legacy" paths. If the homeserver receives an error it will retry
using the legacy path.
This commit is contained in:
Patrick Cloke 2023-04-03 13:20:32 -04:00 committed by GitHub
parent 56efa9b167
commit cf2f2934ad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 172 additions and 48 deletions

1
changelog.d/15317.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a long-standing bug that Synpase only used the [legacy appservice routes](https://spec.matrix.org/v1.6/application-service-api/#legacy-routes).

View file

@ -88,6 +88,22 @@ process, for example:
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
``` ```
# Upgrading to v1.81.0
## Application service path & authentication deprecations
Synapse now attempts the versioned appservice paths before falling back to the
[legacy paths](https://spec.matrix.org/v1.6/application-service-api/#legacy-routes).
Usage of the legacy routes should be considered deprecated.
Additionally, Synapse has supported sending the application service access token
via [the `Authorization` header](https://spec.matrix.org/v1.6/application-service-api/#authorization)
since v1.70.0. For backwards compatibility it is *also* sent as the `access_token`
query parameter. This is insecure and should be considered deprecated.
A future version of Synapse (v1.88.0 or later) will remove support for legacy
application service routes and query parameter authorization.
# Upgrading to v1.80.0 # Upgrading to v1.80.0
## Reporting events error code change ## Reporting events error code change

View file

@ -17,6 +17,8 @@ import urllib.parse
from typing import ( from typing import (
TYPE_CHECKING, TYPE_CHECKING,
Any, Any,
Awaitable,
Callable,
Dict, Dict,
Iterable, Iterable,
List, List,
@ -24,10 +26,11 @@ from typing import (
Optional, Optional,
Sequence, Sequence,
Tuple, Tuple,
TypeVar,
) )
from prometheus_client import Counter from prometheus_client import Counter
from typing_extensions import TypeGuard from typing_extensions import Concatenate, ParamSpec, TypeGuard
from synapse.api.constants import EventTypes, Membership, ThirdPartyEntityKind from synapse.api.constants import EventTypes, Membership, ThirdPartyEntityKind
from synapse.api.errors import CodeMessageException, HttpResponseException from synapse.api.errors import CodeMessageException, HttpResponseException
@ -78,7 +81,11 @@ sent_todevice_counter = Counter(
HOUR_IN_MS = 60 * 60 * 1000 HOUR_IN_MS = 60 * 60 * 1000
APP_SERVICE_PREFIX = "/_matrix/app/unstable" APP_SERVICE_PREFIX = "/_matrix/app/v1"
APP_SERVICE_UNSTABLE_PREFIX = "/_matrix/app/unstable"
P = ParamSpec("P")
R = TypeVar("R")
def _is_valid_3pe_metadata(info: JsonDict) -> bool: def _is_valid_3pe_metadata(info: JsonDict) -> bool:
@ -121,6 +128,47 @@ class ApplicationServiceApi(SimpleHttpClient):
hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS
) )
async def _send_with_fallbacks(
self,
service: "ApplicationService",
prefixes: List[str],
path: str,
func: Callable[Concatenate[str, P], Awaitable[R]],
*args: P.args,
**kwargs: P.kwargs,
) -> R:
"""
Attempt to call an application service with multiple paths, falling back
until one succeeds.
Args:
service: The appliacation service, this provides the base URL.
prefixes: A last of paths to try in order for the requests.
path: A suffix to append to each prefix.
func: The function to call, the first argument will be the full
endpoint to fetch. Other arguments are provided by args/kwargs.
Returns:
The return value of func.
"""
for i, prefix in enumerate(prefixes, start=1):
uri = f"{service.url}{prefix}{path}"
try:
return await func(uri, *args, **kwargs)
except HttpResponseException as e:
# If an error is received that is due to an unrecognised path,
# fallback to next path (if one exists). Otherwise, consider it
# a legitimate error and raise.
if i < len(prefixes) and is_unknown_endpoint(e):
continue
raise
except Exception:
# Unexpected exceptions get sent to the caller.
raise
# The function should always exit via the return or raise above this.
raise RuntimeError("Unexpected fallback behaviour. This should never be seen.")
async def query_user(self, service: "ApplicationService", user_id: str) -> bool: async def query_user(self, service: "ApplicationService", user_id: str) -> bool:
if service.url is None: if service.url is None:
return False return False
@ -128,10 +176,12 @@ class ApplicationServiceApi(SimpleHttpClient):
# This is required by the configuration. # This is required by the configuration.
assert service.hs_token is not None assert service.hs_token is not None
uri = service.url + ("/users/%s" % urllib.parse.quote(user_id))
try: try:
response = await self.get_json( response = await self._send_with_fallbacks(
uri, service,
[APP_SERVICE_PREFIX, ""],
f"/users/{urllib.parse.quote(user_id)}",
self.get_json,
{"access_token": service.hs_token}, {"access_token": service.hs_token},
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
) )
@ -140,9 +190,9 @@ class ApplicationServiceApi(SimpleHttpClient):
except CodeMessageException as e: except CodeMessageException as e:
if e.code == 404: if e.code == 404:
return False return False
logger.warning("query_user to %s received %s", uri, e.code) logger.warning("query_user to %s received %s", service.url, e.code)
except Exception as ex: except Exception as ex:
logger.warning("query_user to %s threw exception %s", uri, ex) logger.warning("query_user to %s threw exception %s", service.url, ex)
return False return False
async def query_alias(self, service: "ApplicationService", alias: str) -> bool: async def query_alias(self, service: "ApplicationService", alias: str) -> bool:
@ -152,21 +202,23 @@ class ApplicationServiceApi(SimpleHttpClient):
# This is required by the configuration. # This is required by the configuration.
assert service.hs_token is not None assert service.hs_token is not None
uri = service.url + ("/rooms/%s" % urllib.parse.quote(alias))
try: try:
response = await self.get_json( response = await self._send_with_fallbacks(
uri, service,
[APP_SERVICE_PREFIX, ""],
f"/rooms/{urllib.parse.quote(alias)}",
self.get_json,
{"access_token": service.hs_token}, {"access_token": service.hs_token},
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
) )
if response is not None: # just an empty json object if response is not None: # just an empty json object
return True return True
except CodeMessageException as e: except CodeMessageException as e:
logger.warning("query_alias to %s received %s", uri, e.code) logger.warning("query_alias to %s received %s", service.url, e.code)
if e.code == 404: if e.code == 404:
return False return False
except Exception as ex: except Exception as ex:
logger.warning("query_alias to %s threw exception %s", uri, ex) logger.warning("query_alias to %s threw exception %s", service.url, ex)
return False return False
async def query_3pe( async def query_3pe(
@ -188,25 +240,24 @@ class ApplicationServiceApi(SimpleHttpClient):
# This is required by the configuration. # This is required by the configuration.
assert service.hs_token is not None assert service.hs_token is not None
uri = "%s%s/thirdparty/%s/%s" % (
service.url,
APP_SERVICE_PREFIX,
kind,
urllib.parse.quote(protocol),
)
try: try:
args: Mapping[Any, Any] = { args: Mapping[Any, Any] = {
**fields, **fields,
b"access_token": service.hs_token, b"access_token": service.hs_token,
} }
response = await self.get_json( response = await self._send_with_fallbacks(
uri, service,
[APP_SERVICE_PREFIX, APP_SERVICE_UNSTABLE_PREFIX],
f"/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
self.get_json,
args=args, args=args,
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
) )
if not isinstance(response, list): if not isinstance(response, list):
logger.warning( logger.warning(
"query_3pe to %s returned an invalid response %r", uri, response "query_3pe to %s returned an invalid response %r",
service.url,
response,
) )
return [] return []
@ -216,12 +267,12 @@ class ApplicationServiceApi(SimpleHttpClient):
ret.append(r) ret.append(r)
else: else:
logger.warning( logger.warning(
"query_3pe to %s returned an invalid result %r", uri, r "query_3pe to %s returned an invalid result %r", service.url, r
) )
return ret return ret
except Exception as ex: except Exception as ex:
logger.warning("query_3pe to %s threw exception %s", uri, ex) logger.warning("query_3pe to %s threw exception %s", service.url, ex)
return [] return []
async def get_3pe_protocol( async def get_3pe_protocol(
@ -233,21 +284,20 @@ class ApplicationServiceApi(SimpleHttpClient):
async def _get() -> Optional[JsonDict]: async def _get() -> Optional[JsonDict]:
# This is required by the configuration. # This is required by the configuration.
assert service.hs_token is not None assert service.hs_token is not None
uri = "%s%s/thirdparty/protocol/%s" % (
service.url,
APP_SERVICE_PREFIX,
urllib.parse.quote(protocol),
)
try: try:
info = await self.get_json( info = await self._send_with_fallbacks(
uri, service,
[APP_SERVICE_PREFIX, APP_SERVICE_UNSTABLE_PREFIX],
f"/thirdparty/protocol/{urllib.parse.quote(protocol)}",
self.get_json,
{"access_token": service.hs_token}, {"access_token": service.hs_token},
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
) )
if not _is_valid_3pe_metadata(info): if not _is_valid_3pe_metadata(info):
logger.warning( logger.warning(
"query_3pe_protocol to %s did not return a valid result", uri "query_3pe_protocol to %s did not return a valid result",
service.url,
) )
return None return None
@ -260,7 +310,9 @@ class ApplicationServiceApi(SimpleHttpClient):
return info return info
except Exception as ex: except Exception as ex:
logger.warning("query_3pe_protocol to %s threw exception %s", uri, ex) logger.warning(
"query_3pe_protocol to %s threw exception %s", service.url, ex
)
return None return None
key = (service.id, protocol) key = (service.id, protocol)
@ -274,7 +326,7 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None assert service.hs_token is not None
await self.post_json_get_json( await self.post_json_get_json(
uri=service.url + "/_matrix/app/unstable/fi.mau.msc2659/ping", uri=f"{service.url}{APP_SERVICE_UNSTABLE_PREFIX}/fi.mau.msc2659/ping",
post_json={"transaction_id": txn_id}, post_json={"transaction_id": txn_id},
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
) )
@ -318,8 +370,6 @@ class ApplicationServiceApi(SimpleHttpClient):
) )
txn_id = 0 txn_id = 0
uri = service.url + ("/transactions/%s" % urllib.parse.quote(str(txn_id)))
# Never send ephemeral events to appservices that do not support it # Never send ephemeral events to appservices that do not support it
body: JsonDict = {"events": serialized_events} body: JsonDict = {"events": serialized_events}
if service.supports_ephemeral: if service.supports_ephemeral:
@ -351,8 +401,11 @@ class ApplicationServiceApi(SimpleHttpClient):
} }
try: try:
await self.put_json( await self._send_with_fallbacks(
uri=uri, service,
[APP_SERVICE_PREFIX, ""],
f"/transactions/{urllib.parse.quote(str(txn_id))}",
self.put_json,
json_body=body, json_body=body,
args={"access_token": service.hs_token}, args={"access_token": service.hs_token},
headers={"Authorization": [f"Bearer {service.hs_token}"]}, headers={"Authorization": [f"Bearer {service.hs_token}"]},
@ -360,7 +413,7 @@ class ApplicationServiceApi(SimpleHttpClient):
if logger.isEnabledFor(logging.DEBUG): if logger.isEnabledFor(logging.DEBUG):
logger.debug( logger.debug(
"push_bulk to %s succeeded! events=%s", "push_bulk to %s succeeded! events=%s",
uri, service.url,
[event.get("event_id") for event in events], [event.get("event_id") for event in events],
) )
sent_transactions_counter.labels(service.id).inc() sent_transactions_counter.labels(service.id).inc()
@ -371,7 +424,7 @@ class ApplicationServiceApi(SimpleHttpClient):
except CodeMessageException as e: except CodeMessageException as e:
logger.warning( logger.warning(
"push_bulk to %s received code=%s msg=%s", "push_bulk to %s received code=%s msg=%s",
uri, service.url,
e.code, e.code,
e.msg, e.msg,
exc_info=logger.isEnabledFor(logging.DEBUG), exc_info=logger.isEnabledFor(logging.DEBUG),
@ -379,7 +432,7 @@ class ApplicationServiceApi(SimpleHttpClient):
except Exception as ex: except Exception as ex:
logger.warning( logger.warning(
"push_bulk to %s threw exception(%s) %s args=%s", "push_bulk to %s threw exception(%s) %s args=%s",
uri, service.url,
type(ex).__name__, type(ex).__name__,
ex, ex,
ex.args, ex.args,

View file

@ -982,20 +982,21 @@ def is_unknown_endpoint(
""" """
if synapse_error is None: if synapse_error is None:
synapse_error = e.to_synapse_error() synapse_error = e.to_synapse_error()
# MSC3743 specifies that servers should return a 404 or 405 with an errcode
# Matrix v1.6 specifies that servers should return a 404 or 405 with an errcode
# of M_UNRECOGNIZED when they receive a request to an unknown endpoint or # of M_UNRECOGNIZED when they receive a request to an unknown endpoint or
# to an unknown method, respectively. # to an unknown method, respectively.
# #
# Older versions of servers don't properly handle this. This needs to be # Older versions of servers don't return proper errors, so be graceful. But,
# rather specific as some endpoints truly do return 404 errors. # also handle that some endpoints truly do return 404 errors.
return ( return (
# 404 is an unknown endpoint, 405 is a known endpoint, but unknown method. # 404 is an unknown endpoint, 405 is a known endpoint, but unknown method.
(e.code == 404 or e.code == 405) (e.code == 404 or e.code == 405)
and ( and (
# Older Dendrites returned a text body or empty body. # Consider empty body or non-JSON bodies to be unrecognised (matches
# Older Conduit returned an empty body. # older Dendrites & Conduits).
not e.response not e.response
or e.response == b"404 page not found" or not e.response.startswith(b"{")
# The proper response JSON with M_UNRECOGNIZED errcode. # The proper response JSON with M_UNRECOGNIZED errcode.
or synapse_error.errcode == Codes.UNRECOGNIZED or synapse_error.errcode == Codes.UNRECOGNIZED
) )

View file

@ -16,6 +16,7 @@ from unittest.mock import Mock
from twisted.test.proto_helpers import MemoryReactor from twisted.test.proto_helpers import MemoryReactor
from synapse.api.errors import HttpResponseException
from synapse.appservice import ApplicationService from synapse.appservice import ApplicationService
from synapse.server import HomeServer from synapse.server import HomeServer
from synapse.types import JsonDict from synapse.types import JsonDict
@ -64,8 +65,8 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
} }
] ]
URL_USER = f"{URL}/_matrix/app/unstable/thirdparty/user/{PROTOCOL}" URL_USER = f"{URL}/_matrix/app/v1/thirdparty/user/{PROTOCOL}"
URL_LOCATION = f"{URL}/_matrix/app/unstable/thirdparty/location/{PROTOCOL}" URL_LOCATION = f"{URL}/_matrix/app/v1/thirdparty/location/{PROTOCOL}"
self.request_url = None self.request_url = None
@ -106,6 +107,58 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
self.assertEqual(self.request_url, URL_LOCATION) self.assertEqual(self.request_url, URL_LOCATION)
self.assertEqual(result, SUCCESS_RESULT_LOCATION) self.assertEqual(result, SUCCESS_RESULT_LOCATION)
def test_fallback(self) -> None:
"""
Tests that the fallback to legacy URLs works.
"""
SUCCESS_RESULT_USER = [
{
"protocol": PROTOCOL,
"userid": "@a:user",
"fields": {
"more": "fields",
},
}
]
URL_USER = f"{URL}/_matrix/app/v1/thirdparty/user/{PROTOCOL}"
FALLBACK_URL_USER = f"{URL}/_matrix/app/unstable/thirdparty/user/{PROTOCOL}"
self.request_url = None
self.v1_seen = False
async def get_json(
url: str,
args: Mapping[Any, Any],
headers: Mapping[Union[str, bytes], Sequence[Union[str, bytes]]],
) -> List[JsonDict]:
# Ensure the access token is passed as both a header and query arg.
if not headers.get("Authorization") or not args.get(b"access_token"):
raise RuntimeError("Access token not provided")
self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
self.assertEqual(args.get(b"access_token"), TOKEN)
self.request_url = url
if url == URL_USER:
self.v1_seen = True
raise HttpResponseException(404, "NOT_FOUND", b"NOT_FOUND")
elif url == FALLBACK_URL_USER:
return SUCCESS_RESULT_USER
else:
raise RuntimeError(
"URL provided was invalid. This should never be seen."
)
# We assign to a method, which mypy doesn't like.
self.api.get_json = Mock(side_effect=get_json) # type: ignore[assignment]
result = self.get_success(
self.api.query_3pe(self.service, "user", PROTOCOL, {b"some": [b"field"]})
)
self.assertTrue(self.v1_seen)
self.assertEqual(self.request_url, FALLBACK_URL_USER)
self.assertEqual(result, SUCCESS_RESULT_USER)
def test_claim_keys(self) -> None: def test_claim_keys(self) -> None:
""" """
Tests that the /keys/claim response is properly parsed for missing Tests that the /keys/claim response is properly parsed for missing