From 23ed267aefef87ec2937b5d01c8f3121cbb843b5 Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Sat, 2 Sep 2023 19:58:31 +0200
Subject: [PATCH 1/6] Send the opentracing span information also to appservices
---
synapse/appservice/api.py | 54 ++++++++++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 9 deletions(-)
diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index de7a94bf2643..39e184f343b8 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -40,6 +40,7 @@
from synapse.events import EventBase
from synapse.events.utils import SerializeEventConfig, serialize_event
from synapse.http.client import SimpleHttpClient, is_unknown_endpoint
+from synapse.logging import opentracing
from synapse.types import DeviceListUpdates, JsonDict, ThirdPartyInstanceID
from synapse.util.caches.response_cache import ResponseCache
@@ -136,10 +137,15 @@ async def query_user(self, service: "ApplicationService", user_id: str) -> bool:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
+
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}",
args,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
if response is not None: # just an empty json object
return True
@@ -162,10 +168,15 @@ async def query_alias(self, service: "ApplicationService", alias: str) -> bool:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
+
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}",
args,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
if response is not None: # just an empty json object
return True
@@ -203,10 +214,15 @@ async def query_3pe(
**fields,
b"access_token": service.hs_token,
}
+
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
args=args,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
if not isinstance(response, list):
logger.warning(
@@ -243,10 +259,15 @@ async def _get() -> Optional[JsonDict]:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
+
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
info = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}",
args,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
if not _is_valid_3pe_metadata(info):
@@ -280,10 +301,14 @@ async def ping(self, service: "ApplicationService", txn_id: Optional[str]) -> No
# This is required by the configuration.
assert service.hs_token is not None
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
await self.post_json_get_json(
uri=f"{service.url}{APP_SERVICE_PREFIX}/ping",
post_json={"transaction_id": txn_id},
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
async def push_bulk(
@@ -360,11 +385,15 @@ async def push_bulk(
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
await self.put_json(
f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}",
json_body=body,
args=args,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
if logger.isEnabledFor(logging.DEBUG):
logger.debug(
@@ -432,12 +461,16 @@ async def claim_client_keys(
[algorithm] * count
)
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
uri = f"{service.url}/_matrix/app/unstable/org.matrix.msc3983/keys/claim"
try:
response = await self.post_json_get_json(
uri,
body,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
except HttpResponseException as e:
# The appservice doesn't support this endpoint.
@@ -492,13 +525,16 @@ async def query_keys(
# This is required by the configuration.
assert service.hs_token is not None
-
+ headers: Dict[bytes, List[bytes]] = {
+ b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
+ }
+ opentracing.inject_header_dict(headers, check_destination=False)
uri = f"{service.url}/_matrix/app/unstable/org.matrix.msc3984/keys/query"
try:
response = await self.post_json_get_json(
uri,
query,
- headers={"Authorization": [f"Bearer {service.hs_token}"]},
+ headers=headers,
)
except HttpResponseException as e:
# The appservice doesn't support this endpoint.
From ae4f500a87258445169e1b3169f65f0570c97a5b Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Sat, 2 Sep 2023 20:03:47 +0200
Subject: [PATCH 2/6] Add changelog entry for PR 16227
---
changelog.d/16227.misc | 1 +
1 file changed, 1 insertion(+)
create mode 100644 changelog.d/16227.misc
diff --git a/changelog.d/16227.misc b/changelog.d/16227.misc
new file mode 100644
index 000000000000..510062b622c0
--- /dev/null
+++ b/changelog.d/16227.misc
@@ -0,0 +1 @@
+Add span information to requests sent to appservices. Contributed by MTRNord.
\ No newline at end of file
From 7ed9477cd7015ba14334fec9a8ea9ed207caa50b Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Sat, 2 Sep 2023 23:17:18 +0200
Subject: [PATCH 3/6] Make the changelog entry for PR 16227 a feature instead
of a misc entry
---
changelog.d/{16227.misc => 16227.feature} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename changelog.d/{16227.misc => 16227.feature} (100%)
diff --git a/changelog.d/16227.misc b/changelog.d/16227.feature
similarity index 100%
rename from changelog.d/16227.misc
rename to changelog.d/16227.feature
From bc990774fb4c02b2afdd8c56db99a6c24b44ac67 Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Mon, 4 Sep 2023 16:53:57 +0200
Subject: [PATCH 4/6] Update the test mock to work with bytestrings as headers
---
tests/appservice/test_api.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py
index 75fb5fae6b92..166d2369e93e 100644
--- a/tests/appservice/test_api.py
+++ b/tests/appservice/test_api.py
@@ -76,7 +76,7 @@ async def get_json(
headers: Mapping[Union[str, bytes], Sequence[Union[str, bytes]]],
) -> List[JsonDict]:
# Ensure the access token is passed as a header.
- if not headers or not headers.get("Authorization"):
+ if not headers or not headers.get(b"Authorization"):
raise RuntimeError("Access token not provided")
# ... and not as a query param
if b"access_token" in args:
@@ -84,7 +84,7 @@ async def get_json(
"Access token should not be passed as a query param."
)
- self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
+ self.assertEqual(headers.get(b"Authorization"), [f"Bearer {TOKEN}".encode()])
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
@@ -152,11 +152,12 @@ async def get_json(
# Ensure the access token is passed as a both a query param and in the headers.
if not args.get(b"access_token"):
raise RuntimeError("Access token should be provided in query params.")
- if not headers or not headers.get("Authorization"):
+ if not headers or not headers.get(b"Authorization"):
raise RuntimeError("Access token should be provided in auth headers.")
self.assertEqual(args.get(b"access_token"), TOKEN)
- self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
+ self.assertEqual(headers.get(b"Authorization"),
+ [f"Bearer {TOKEN}".encode()])
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
@@ -208,10 +209,11 @@ async def post_json_get_json(
headers: Mapping[Union[str, bytes], Sequence[Union[str, bytes]]],
) -> JsonDict:
# Ensure the access token is passed as both a header and query arg.
- if not headers.get("Authorization"):
+ if not headers.get(b"Authorization"):
raise RuntimeError("Access token not provided")
- self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
+ self.assertEqual(headers.get(b"Authorization"),
+ [f"Bearer {TOKEN}".encode()])
return RESPONSE
# We assign to a method, which mypy doesn't like.
From 44feccd0c287c369f755fe704bfb48aa5327114c Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Mon, 4 Sep 2023 20:09:54 +0200
Subject: [PATCH 5/6] Ensure file formatting of tests/appservice/test_api.py
---
tests/appservice/test_api.py | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py
index 166d2369e93e..366b6fd5f00d 100644
--- a/tests/appservice/test_api.py
+++ b/tests/appservice/test_api.py
@@ -84,7 +84,9 @@ async def get_json(
"Access token should not be passed as a query param."
)
- self.assertEqual(headers.get(b"Authorization"), [f"Bearer {TOKEN}".encode()])
+ self.assertEqual(
+ headers.get(b"Authorization"), [f"Bearer {TOKEN}".encode()]
+ )
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
@@ -156,8 +158,9 @@ async def get_json(
raise RuntimeError("Access token should be provided in auth headers.")
self.assertEqual(args.get(b"access_token"), TOKEN)
- self.assertEqual(headers.get(b"Authorization"),
- [f"Bearer {TOKEN}".encode()])
+ self.assertEqual(
+ headers.get(b"Authorization"), [f"Bearer {TOKEN}".encode()]
+ )
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
@@ -212,8 +215,9 @@ async def post_json_get_json(
if not headers.get(b"Authorization"):
raise RuntimeError("Access token not provided")
- self.assertEqual(headers.get(b"Authorization"),
- [f"Bearer {TOKEN}".encode()])
+ self.assertEqual(
+ headers.get(b"Authorization"), [f"Bearer {TOKEN}".encode()]
+ )
return RESPONSE
# We assign to a method, which mypy doesn't like.
From 2ac68cc2924944bc885fc2d208e94a075a556094 Mon Sep 17 00:00:00 2001
From: MTRNord <mtrnord1@gmail.com>
Date: Wed, 6 Sep 2023 16:06:40 +0200
Subject: [PATCH 6/6] Move header generation to helper method
---
synapse/appservice/api.py | 60 +++++++++++++--------------------------
1 file changed, 20 insertions(+), 40 deletions(-)
diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index 39e184f343b8..b1523be208e9 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -126,6 +126,17 @@ def __init__(self, hs: "HomeServer"):
hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS
)
+ def _get_headers(self, service: "ApplicationService") -> Dict[bytes, List[bytes]]:
+ """This makes sure we have always the auth header and opentracing headers set."""
+
+ # This is also ensured before in the functions. However this is needed to please
+ # the typechecks.
+ assert service.hs_token is not None
+
+ headers = {b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]}
+ opentracing.inject_header_dict(headers, check_destination=False)
+ return headers
+
async def query_user(self, service: "ApplicationService", user_id: str) -> bool:
if service.url is None:
return False
@@ -138,14 +149,10 @@ async def query_user(self, service: "ApplicationService", user_id: str) -> bool:
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}",
args,
- headers=headers,
+ headers=self._get_headers(service),
)
if response is not None: # just an empty json object
return True
@@ -169,14 +176,10 @@ async def query_alias(self, service: "ApplicationService", alias: str) -> bool:
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}",
args,
- headers=headers,
+ headers=self._get_headers(service),
)
if response is not None: # just an empty json object
return True
@@ -215,14 +218,10 @@ async def query_3pe(
b"access_token": service.hs_token,
}
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
args=args,
- headers=headers,
+ headers=self._get_headers(service),
)
if not isinstance(response, list):
logger.warning(
@@ -260,14 +259,10 @@ async def _get() -> Optional[JsonDict]:
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
info = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}",
args,
- headers=headers,
+ headers=self._get_headers(service),
)
if not _is_valid_3pe_metadata(info):
@@ -301,14 +296,10 @@ async def ping(self, service: "ApplicationService", txn_id: Optional[str]) -> No
# This is required by the configuration.
assert service.hs_token is not None
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
await self.post_json_get_json(
uri=f"{service.url}{APP_SERVICE_PREFIX}/ping",
post_json={"transaction_id": txn_id},
- headers=headers,
+ headers=self._get_headers(service),
)
async def push_bulk(
@@ -385,15 +376,11 @@ async def push_bulk(
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
await self.put_json(
f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}",
json_body=body,
args=args,
- headers=headers,
+ headers=self._get_headers(service),
)
if logger.isEnabledFor(logging.DEBUG):
logger.debug(
@@ -461,16 +448,12 @@ async def claim_client_keys(
[algorithm] * count
)
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
uri = f"{service.url}/_matrix/app/unstable/org.matrix.msc3983/keys/claim"
try:
response = await self.post_json_get_json(
uri,
body,
- headers=headers,
+ headers=self._get_headers(service),
)
except HttpResponseException as e:
# The appservice doesn't support this endpoint.
@@ -525,16 +508,13 @@ async def query_keys(
# This is required by the configuration.
assert service.hs_token is not None
- headers: Dict[bytes, List[bytes]] = {
- b"Authorization": [b"Bearer " + service.hs_token.encode("ascii")]
- }
- opentracing.inject_header_dict(headers, check_destination=False)
+
uri = f"{service.url}/_matrix/app/unstable/org.matrix.msc3984/keys/query"
try:
response = await self.post_json_get_json(
uri,
query,
- headers=headers,
+ headers=self._get_headers(service),
)
except HttpResponseException as e:
# The appservice doesn't support this endpoint.