Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add authentication to thirdparty bridge APIs #12746

Merged
merged 11 commits into from
May 24, 2022
Merged
1 change: 1 addition & 0 deletions changelog.d/12746.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Always send an `access_token` in `/thirdparty/` requests to appservices, as required by the [Matrix specification](https://spec.matrix.org/v1.1/application-service-api/#third-party-networks).
15 changes: 12 additions & 3 deletions synapse/appservice/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
import logging
import urllib.parse
from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Mapping, Optional, Tuple

from prometheus_client import Counter
from typing_extensions import TypeGuard
Expand Down Expand Up @@ -155,14 +155,21 @@ async def query_3pe(
if service.url is None:
return []

# This is required by the configuration.
assert service.hs_token is not None

uri = "%s%s/thirdparty/%s/%s" % (
service.url,
APP_SERVICE_PREFIX,
kind,
urllib.parse.quote(protocol),
)
try:
response = await self.get_json(uri, fields)
args: Mapping[Any, Any] = {
**fields,
b"access_token": service.hs_token,
}
response = await self.get_json(uri, args=args)
if not isinstance(response, list):
logger.warning(
"query_3pe to %s returned an invalid response %r", uri, response
Expand Down Expand Up @@ -190,13 +197,15 @@ async def get_3pe_protocol(
return {}

async def _get() -> Optional[JsonDict]:
# This is required by the configuration.
assert service.hs_token is not None
uri = "%s%s/thirdparty/protocol/%s" % (
service.url,
APP_SERVICE_PREFIX,
urllib.parse.quote(protocol),
)
try:
info = await self.get_json(uri)
info = await self.get_json(uri, {"access_token": service.hs_token})

if not _is_valid_3pe_metadata(info):
logger.warning(
Expand Down
98 changes: 98 additions & 0 deletions tests/appservice/test_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Copyright 2022 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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
from unittest.mock import Mock

from twisted.test.proto_helpers import MemoryReactor

from synapse.appservice import ApplicationService
from synapse.appservice.api import ApplicationServiceApi
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util import Clock

from tests import unittest

PROTOCOL = "myproto"
TOKEN = "myastoken"
URL = "http://mytestservice"


class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
self.api = ApplicationServiceApi(hs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.api = ApplicationServiceApi(hs)
self.api = hs.get_application_service_api()

It probably doesn't make that much of a difference, but it's more consistent with how we access this kind of objects in tests.

self.service = ApplicationService(
id="unique_identifier",
sender="@as:test",
url=URL,
token="unused",
hs_token=TOKEN,
hostname="myserver",
)

def test_query_3pe_authenticates_token(self):
babolivier marked this conversation as resolved.
Show resolved Hide resolved

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/unstable/thirdparty/user/{PROTOCOL}"
URL_LOCATION = f"{URL}/_matrix/app/unstable/thirdparty/location/{PROTOCOL}"

self.request_url = None

async def get_json(url: str, args: Mapping[Any, Any]) -> List[JsonDict]:
if not args.get(b"access_token"):
raise Exception("Access token not provided")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise Exception("Access token not provided")
raise RuntimeError("Access token not provided")

For consistency


self.assertEqual(args.get(b"access_token"), TOKEN)
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
elif url == URL_LOCATION:
return SUCCESS_RESULT_LOCATION
else:
self.fail("URL provided was invalid")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is now a bit unnecessary since raising the RuntimeError will automatically fail the test.

raise RuntimeError("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)