From ac7551b988465dfcc280d66f22bee427280e0589 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 27 Jul 2023 14:56:06 -0700 Subject: [PATCH 1/8] add a config option to use legacy appservice authentication --- docs/upgrade.md | 16 +++++++++++++++- docs/usage/configuration/config_documentation.md | 11 +++++++++++ synapse/config/appservice.py | 8 ++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index 5dde6c769e85..07577500318f 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -88,6 +88,21 @@ process, for example: dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb ``` +# Upgrading to v1.90.0 + +## App service query parameter authorization is now a configuration option + +Synapse v1.81.0 deprecated application service authorization via query parameters as this is +considered insecure - and from Synapse v1.71.0 forwards the application service token has also been sent via +[the `Authorization` header](https://spec.matrix.org/v1.6/application-service-api/#authorization)], making the insecure +query parameter authorization redundant. Since removing the ability to continue to use query parameters could break +backwards compatibility it has now been put behind a configuration option, `use_appservice_legacy_authorization`. +This option defaults to false, but can be activated by adding +```yaml +"use_appservice_legacy_authorization": True +``` +to your configuration. + # Upgrading to v1.89.0 ## Removal of unspecced `user` property for `/register` @@ -97,7 +112,6 @@ The standard `username` property should be used instead. See the [Application Service specification](https://spec.matrix.org/v1.7/application-service-api/#server-admin-style-permissions) for more information. - # Upgrading to v1.88.0 ## Minimum supported Python version diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 4e6fcd085acb..32f78cdff5d9 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2837,6 +2837,17 @@ Example configuration: ```yaml track_appservice_user_ips: true ``` +--- +### `use_appservice_legacy_authorization` + +Whether to send the application service access token via the `access_token` query parameter. +Defaults to false. This option is considered insecure and is not recommended. + +Example configuration: +```yaml +"use_appservice_legacy_authorization": True +``` + --- ### `macaroon_secret_key` diff --git a/synapse/config/appservice.py b/synapse/config/appservice.py index c2710fdf0410..919f81a9b716 100644 --- a/synapse/config/appservice.py +++ b/synapse/config/appservice.py @@ -43,6 +43,14 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ) self.track_appservice_user_ips = config.get("track_appservice_user_ips", False) + self.use_appservice_legacy_authorization = config.get( + "use_appservice_legacy_authorization", False + ) + if self.use_appservice_legacy_authorization: + logger.warning( + "The use of appservice legacy authorization via query params is deprecated" + " and should be considered insecure." + ) def load_appservices( From 7e12bf53c64ac0b1e70111f3bd2739a980aded77 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 27 Jul 2023 14:56:27 -0700 Subject: [PATCH 2/8] update api to check which authentication method to use --- synapse/appservice/api.py | 85 ++++++++++++++++++++++++++------------- 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 359999f68017..00043bc1a1f0 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -20,7 +20,7 @@ Dict, Iterable, List, - Mapping, + MutableMapping, Optional, Sequence, Tuple, @@ -119,6 +119,7 @@ class ApplicationServiceApi(SimpleHttpClient): def __init__(self, hs: "HomeServer"): super().__init__(hs) self.clock = hs.get_clock() + self.config = hs.config.appservice self.protocol_meta_cache: ResponseCache[Tuple[str, str]] = ResponseCache( hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS @@ -132,11 +133,16 @@ async def query_user(self, service: "ApplicationService", user_id: str) -> bool: assert service.hs_token is not None try: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}", - {"access_token": service.hs_token}, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + if self.config.use_appservice_legacy_authorization: + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}", + {"access_token": service.hs_token}, + ) + else: + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}", + headers={"Authorization": [f"Bearer {service.hs_token}"]}, + ) if response is not None: # just an empty json object return True except CodeMessageException as e: @@ -155,11 +161,16 @@ async def query_alias(self, service: "ApplicationService", alias: str) -> bool: assert service.hs_token is not None try: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", - {"access_token": service.hs_token}, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + if self.config.use_appservice_legacy_authorization: + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", + {"access_token": service.hs_token}, + ) + else: + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", + headers={"Authorization": [f"Bearer {service.hs_token}"]}, + ) if response is not None: # just an empty json object return True except CodeMessageException as e: @@ -190,15 +201,22 @@ async def query_3pe( assert service.hs_token is not None try: - args: Mapping[Any, Any] = { + args: MutableMapping[Any, Any] = { **fields, b"access_token": service.hs_token, } - 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}"]}, - ) + if self.config.use_appservice_legacy_authorization: + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}", + args=args, + ) + else: + args.pop(b"access_token", None) + 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}"]}, + ) if not isinstance(response, list): logger.warning( "query_3pe to %s returned an invalid response %r", @@ -231,11 +249,16 @@ async def _get() -> Optional[JsonDict]: # This is required by the configuration. assert service.hs_token is not None try: - info = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}", - {"access_token": service.hs_token}, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + if self.config.use_appservice_legacy_authorization: + info = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}", + {"access_token": service.hs_token}, + ) + else: + info = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}", + headers={"Authorization": [f"Bearer {service.hs_token}"]}, + ) if not _is_valid_3pe_metadata(info): logger.warning( @@ -344,12 +367,18 @@ async def push_bulk( } try: - await self.put_json( - f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}", - json_body=body, - args={"access_token": service.hs_token}, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + if self.config.use_appservice_legacy_authorization: + await self.put_json( + f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}", + json_body=body, + args={"access_token": service.hs_token}, + ) + else: + await self.put_json( + f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}", + json_body=body, + headers={"Authorization": [f"Bearer {service.hs_token}"]}, + ) if logger.isEnabledFor(logging.DEBUG): logger.debug( "push_bulk to %s succeeded! events=%s", From 35f3be5272b990caaf5a38d18801c33181256173 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 27 Jul 2023 14:56:32 -0700 Subject: [PATCH 3/8] update tests --- tests/appservice/test_api.py | 77 ++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py index 807dc2f21cf1..7aabd87e64c1 100644 --- a/tests/appservice/test_api.py +++ b/tests/appservice/test_api.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, List, Mapping, Sequence, Union +from typing import Any, List, Mapping, Optional, Sequence, Union from unittest.mock import Mock from twisted.test.proto_helpers import MemoryReactor @@ -22,6 +22,7 @@ from synapse.util import Clock from tests import unittest +from tests.unittest import override_config PROTOCOL = "myproto" TOKEN = "myastoken" @@ -39,7 +40,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: hs_token=TOKEN, ) - def test_query_3pe_authenticates_token(self) -> None: + def test_query_3pe_authenticates_token_via_header(self) -> None: """ Tests that 3pe queries to the appservice are authenticated with the appservice's token. @@ -74,11 +75,79 @@ async def get_json( 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"): + # Ensure the access token is passed as a header. + if not headers.get("Authorization"): raise RuntimeError("Access token not provided") self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"]) + self.request_url = url + if url == URL_USER: + return SUCCESS_RESULT_USER + elif url == URL_LOCATION: + return SUCCESS_RESULT_LOCATION + 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.assertEqual(self.request_url, URL_USER) + self.assertEqual(result, SUCCESS_RESULT_USER) + result = self.get_success( + self.api.query_3pe( + self.service, "location", PROTOCOL, {b"some": [b"field"]} + ) + ) + self.assertEqual(self.request_url, URL_LOCATION) + self.assertEqual(result, SUCCESS_RESULT_LOCATION) + + @override_config({"use_appservice_legacy_authorization": True}) + def test_query_3pe_authenticates_token_via_param(self) -> None: + """ + Tests that 3pe queries to the appservice are authenticated + with the appservice's token. + """ + + SUCCESS_RESULT_USER = [ + { + "protocol": PROTOCOL, + "userid": "@a:user", + "fields": { + "more": "fields", + }, + } + ] + SUCCESS_RESULT_LOCATION = [ + { + "protocol": PROTOCOL, + "alias": "#a:room", + "fields": { + "more": "fields", + }, + } + ] + + URL_USER = f"{URL}/_matrix/app/v1/thirdparty/user/{PROTOCOL}" + URL_LOCATION = f"{URL}/_matrix/app/v1/thirdparty/location/{PROTOCOL}" + + self.request_url = None + + async def get_json( + url: str, + args: Mapping[Any, Any], + headers: Optional[ + Mapping[Union[str, bytes], Sequence[Union[str, bytes]]] + ] = None, + ) -> List[JsonDict]: + # Ensure the access token is passed as a query param. + if not args.get(b"access_token"): + raise RuntimeError("Access token not provided") + self.assertEqual(args.get(b"access_token"), TOKEN) self.request_url = url if url == URL_USER: From bb3c42fd8d936dd3a965dbb8ad566b549c070a33 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 27 Jul 2023 15:07:50 -0700 Subject: [PATCH 4/8] newsfragment --- changelog.d/16017.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16017.misc diff --git a/changelog.d/16017.misc b/changelog.d/16017.misc new file mode 100644 index 000000000000..6b7244289247 --- /dev/null +++ b/changelog.d/16017.misc @@ -0,0 +1 @@ +Move support for application service query parameter authorization behind a configuration option. From 19c62893c31baba657f9f786c004c094dd87af03 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 28 Jul 2023 16:54:09 -0700 Subject: [PATCH 5/8] requested changes --- docs/upgrade.md | 2 +- .../configuration/config_documentation.md | 2 +- synapse/appservice/api.py | 85 ++++++++----------- 3 files changed, 38 insertions(+), 51 deletions(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index 07577500318f..f50a279e985a 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -99,7 +99,7 @@ query parameter authorization redundant. Since removing the ability to continue backwards compatibility it has now been put behind a configuration option, `use_appservice_legacy_authorization`. This option defaults to false, but can be activated by adding ```yaml -"use_appservice_legacy_authorization": True +use_appservice_legacy_authorization: true ``` to your configuration. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 32f78cdff5d9..58f95eff59f4 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2845,7 +2845,7 @@ Defaults to false. This option is considered insecure and is not recommended. Example configuration: ```yaml -"use_appservice_legacy_authorization": True +use_appservice_legacy_authorization: true ``` --- diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 00043bc1a1f0..8daf6b147d7e 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -133,16 +133,14 @@ async def query_user(self, service: "ApplicationService", user_id: str) -> bool: assert service.hs_token is not None try: + args = None if self.config.use_appservice_legacy_authorization: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}", - {"access_token": service.hs_token}, - ) - else: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}", - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + args = {"access_token": service.hs_token} + 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}"]}, + ) if response is not None: # just an empty json object return True except CodeMessageException as e: @@ -161,16 +159,14 @@ async def query_alias(self, service: "ApplicationService", alias: str) -> bool: assert service.hs_token is not None try: + args = None if self.config.use_appservice_legacy_authorization: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", - {"access_token": service.hs_token}, - ) - else: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + args = {"access_token": service.hs_token} + response = await self.get_json( + f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}", + args, + headers={"Authorization": [f"Bearer {service.hs_token}"]}, + ) if response is not None: # just an empty json object return True except CodeMessageException as e: @@ -205,18 +201,13 @@ async def query_3pe( **fields, b"access_token": service.hs_token, } - if self.config.use_appservice_legacy_authorization: - response = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}", - args=args, - ) - else: + if not self.config.use_appservice_legacy_authorization: args.pop(b"access_token", None) - 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}"]}, - ) + 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}"]}, + ) if not isinstance(response, list): logger.warning( "query_3pe to %s returned an invalid response %r", @@ -249,16 +240,14 @@ async def _get() -> Optional[JsonDict]: # This is required by the configuration. assert service.hs_token is not None try: + args = None if self.config.use_appservice_legacy_authorization: - info = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}", - {"access_token": service.hs_token}, - ) - else: - info = await self.get_json( - f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}", - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + args = {"access_token": service.hs_token} + 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}"]}, + ) if not _is_valid_3pe_metadata(info): logger.warning( @@ -367,18 +356,16 @@ async def push_bulk( } try: + args = None if self.config.use_appservice_legacy_authorization: - await self.put_json( - f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}", - json_body=body, - args={"access_token": service.hs_token}, - ) - else: - await self.put_json( - f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}", - json_body=body, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, - ) + args = {"access_token": service.hs_token} + + 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}"]}, + ) if logger.isEnabledFor(logging.DEBUG): logger.debug( "push_bulk to %s succeeded! events=%s", From 07fd6465708efeea3659b0acec15d4b7436aa54c Mon Sep 17 00:00:00 2001 From: Shay Date: Mon, 31 Jul 2023 12:42:55 -0700 Subject: [PATCH 6/8] Update docs/usage/configuration/config_documentation.md Co-authored-by: Patrick Cloke --- docs/usage/configuration/config_documentation.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 58f95eff59f4..bd3fdd626d30 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2840,8 +2840,11 @@ track_appservice_user_ips: true --- ### `use_appservice_legacy_authorization` -Whether to send the application service access token via the `access_token` query parameter. -Defaults to false. This option is considered insecure and is not recommended. +Whether to send the application service access tokens via the `access_token` query parameter +per older versions of the Matrix specification. Defaults to false. Set to true to enable sending +access tokens via a query parameter. + +**Enabling this option is considered insecure and is not recommended. ** Example configuration: ```yaml From 559d01c50dc2c5630a81c50b9dc9865e5aa7939e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 31 Jul 2023 12:58:07 -0700 Subject: [PATCH 7/8] requested changes --- synapse/appservice/api.py | 14 +++++++------- tests/appservice/test_api.py | 15 +++++++++++---- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 8daf6b147d7e..1380543f2c1c 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -14,13 +14,13 @@ # limitations under the License. import logging import urllib.parse +from collections.abc import Mapping from typing import ( TYPE_CHECKING, Any, Dict, Iterable, List, - MutableMapping, Optional, Sequence, Tuple, @@ -197,12 +197,12 @@ async def query_3pe( assert service.hs_token is not None try: - args: MutableMapping[Any, Any] = { - **fields, - b"access_token": service.hs_token, - } - if not self.config.use_appservice_legacy_authorization: - args.pop(b"access_token", None) + args: Mapping[Any, Any] = fields + if self.config.use_appservice_legacy_authorization: + args = { + **fields, + b"access_token": service.hs_token, + } response = await self.get_json( f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}", args=args, diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py index 7aabd87e64c1..74e3b8db0107 100644 --- a/tests/appservice/test_api.py +++ b/tests/appservice/test_api.py @@ -76,8 +76,13 @@ 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.get("Authorization"): + if not headers or not headers.get("Authorization"): raise RuntimeError("Access token not provided") + # ... and not as a query param + if args and args.get(b"access_token"): + raise RuntimeError( + "Access token should not be passed as a query param." + ) self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"]) self.request_url = url @@ -144,9 +149,11 @@ async def get_json( Mapping[Union[str, bytes], Sequence[Union[str, bytes]]] ] = None, ) -> List[JsonDict]: - # Ensure the access token is passed as a query param. - if not args.get(b"access_token"): - raise RuntimeError("Access token not provided") + # Ensure the access token is passed as a both a query param and in the headers. + if not args or not args.get(b"access_token"): + raise RuntimeError("Access token should be provided in query params.") + if not headers or not headers.get("Authorization"): + raise RuntimeError("Access token should be provided in auth headers.") self.assertEqual(args.get(b"access_token"), TOKEN) self.request_url = url From eb20b82f5dbd4815a2b3b056c66213e8865ba584 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 1 Aug 2023 11:00:54 -0700 Subject: [PATCH 8/8] requested changes --- changelog.d/{16017.misc => 16017.removal} | 0 synapse/appservice/api.py | 6 +++--- tests/appservice/test_api.py | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) rename changelog.d/{16017.misc => 16017.removal} (100%) diff --git a/changelog.d/16017.misc b/changelog.d/16017.removal similarity index 100% rename from changelog.d/16017.misc rename to changelog.d/16017.removal diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 1380543f2c1c..de7a94bf2643 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -14,17 +14,17 @@ # limitations under the License. import logging import urllib.parse -from collections.abc import Mapping from typing import ( TYPE_CHECKING, - Any, Dict, Iterable, List, + Mapping, Optional, Sequence, Tuple, TypeVar, + Union, ) from prometheus_client import Counter @@ -197,7 +197,7 @@ async def query_3pe( assert service.hs_token is not None try: - args: Mapping[Any, Any] = fields + args: Mapping[bytes, Union[List[bytes], str]] = fields if self.config.use_appservice_legacy_authorization: args = { **fields, diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py index 74e3b8db0107..3c635e3dcbdb 100644 --- a/tests/appservice/test_api.py +++ b/tests/appservice/test_api.py @@ -79,7 +79,7 @@ async def get_json( if not headers or not headers.get("Authorization"): raise RuntimeError("Access token not provided") # ... and not as a query param - if args and args.get(b"access_token"): + if b"access_token" in args: raise RuntimeError( "Access token should not be passed as a query param." ) @@ -150,12 +150,13 @@ async def get_json( ] = None, ) -> List[JsonDict]: # Ensure the access token is passed as a both a query param and in the headers. - if not args or not args.get(b"access_token"): + 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"): 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.request_url = url if url == URL_USER: return SUCCESS_RESULT_USER