diff --git a/changelog.d/16017.removal b/changelog.d/16017.removal
new file mode 100644
index 0000000000..6b72442892
--- /dev/null
+++ b/changelog.d/16017.removal
@@ -0,0 +1 @@
+Move support for application service query parameter authorization behind a configuration option.
diff --git a/docs/upgrade.md b/docs/upgrade.md
index 5dde6c769e..f50a279e98 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 c32608da2b..2987c9332d 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -2849,6 +2849,20 @@ Example configuration:
track_appservice_user_ips: true
```
---
+### `use_appservice_legacy_authorization`
+
+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
+use_appservice_legacy_authorization: true
+```
+
+---
### `macaroon_secret_key`
A secret which is used to sign
diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index 359999f680..de7a94bf26 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -16,7 +16,6 @@ import logging
import urllib.parse
from typing import (
TYPE_CHECKING,
- Any,
Dict,
Iterable,
List,
@@ -25,6 +24,7 @@ from typing import (
Sequence,
Tuple,
TypeVar,
+ Union,
)
from prometheus_client import Counter
@@ -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,9 +133,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
+ args = None
+ if self.config.use_appservice_legacy_authorization:
+ args = {"access_token": service.hs_token}
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}",
- {"access_token": service.hs_token},
+ args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
@@ -155,9 +159,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
+ args = None
+ if self.config.use_appservice_legacy_authorization:
+ args = {"access_token": service.hs_token}
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}",
- {"access_token": service.hs_token},
+ args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
@@ -190,10 +197,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
- args: Mapping[Any, Any] = {
- **fields,
- b"access_token": service.hs_token,
- }
+ args: Mapping[bytes, Union[List[bytes], str]] = 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,
@@ -231,9 +240,12 @@ class ApplicationServiceApi(SimpleHttpClient):
# This is required by the configuration.
assert service.hs_token is not None
try:
+ args = None
+ if self.config.use_appservice_legacy_authorization:
+ args = {"access_token": service.hs_token}
info = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}",
- {"access_token": service.hs_token},
+ args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
@@ -344,10 +356,14 @@ class ApplicationServiceApi(SimpleHttpClient):
}
try:
+ args = None
+ if self.config.use_appservice_legacy_authorization:
+ 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={"access_token": service.hs_token},
+ args=args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if logger.isEnabledFor(logging.DEBUG):
diff --git a/synapse/config/appservice.py b/synapse/config/appservice.py
index c2710fdf04..919f81a9b7 100644
--- a/synapse/config/appservice.py
+++ b/synapse/config/appservice.py
@@ -43,6 +43,14 @@ class AppServiceConfig(Config):
)
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(
diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py
index 807dc2f21c..3c635e3dcb 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.types import JsonDict
from synapse.util import Clock
from tests import unittest
+from tests.unittest import override_config
PROTOCOL = "myproto"
TOKEN = "myastoken"
@@ -39,7 +40,7 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
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,12 +75,88 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
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 or not headers.get("Authorization"):
raise RuntimeError("Access token not provided")
+ # ... and not as a query param
+ if b"access_token" in args:
+ raise RuntimeError(
+ "Access token should not be passed as a query param."
+ )
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 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"):
+ 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
|
