Move support for application service query parameter authorization behind a configuration option (#16017)
This commit is contained in:
parent
f0a860908b
commit
0a5f4f7665
|
@ -0,0 +1 @@
|
|||
Move support for application service query parameter authorization behind a configuration option.
|
|
@ -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
|
||||
|
|
|
@ -2848,6 +2848,20 @@ Example configuration:
|
|||
```yaml
|
||||
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`
|
||||
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue