diff --git a/CHANGES.md b/CHANGES.md
index 444d790e80..c7845b0e38 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,3 +1,13 @@
+Synapse 1.70.1 (2022-10-28)
+===========================
+
+Bugfixes
+--------
+
+- Fix a bug introduced in Synapse 1.70.0rc1 where the access tokens sent to application services as headers were malformed. Application services which were obtaining access tokens from query parameters were not affected. ([\#14301](https://github.com/matrix-org/synapse/issues/14301))
+- Fix room creation being rate limited too aggressively since Synapse v1.69.0. ([\#14314](https://github.com/matrix-org/synapse/issues/14314))
+
+
Synapse 1.70.0 (2022-10-26)
===========================
diff --git a/debian/changelog b/debian/changelog
index c3c1cc538a..b9b48e5fbd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+matrix-synapse-py3 (1.70.1) stable; urgency=medium
+
+ * New Synapse release 1.70.1.
+
+ -- Synapse Packaging team <packages@matrix.org> Fri, 28 Oct 2022 12:10:21 +0100
+
matrix-synapse-py3 (1.70.0) stable; urgency=medium
* New Synapse release 1.70.0.
diff --git a/pyproject.toml b/pyproject.toml
index f129b0229e..4c1c02e8e4 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -57,7 +57,7 @@ manifest-path = "rust/Cargo.toml"
[tool.poetry]
name = "matrix-synapse"
-version = "1.70.0"
+version = "1.70.1"
description = "Homeserver for the Matrix decentralised comms protocol"
authors = ["Matrix.org Team and Contributors <packages@matrix.org>"]
license = "Apache-2.0"
diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py
index 044c7d4926..511790c7c5 100644
--- a/synapse/api/ratelimiting.py
+++ b/synapse/api/ratelimiting.py
@@ -343,6 +343,7 @@ class RequestRatelimiter:
requester: Requester,
update: bool = True,
is_admin_redaction: bool = False,
+ n_actions: int = 1,
) -> None:
"""Ratelimits requests.
@@ -355,6 +356,8 @@ class RequestRatelimiter:
is_admin_redaction: Whether this is a room admin/moderator
redacting an event. If so then we may apply different
ratelimits depending on config.
+ n_actions: Multiplier for the number of actions to apply to the
+ rate limiter at once.
Raises:
LimitExceededError if the request should be ratelimited
@@ -383,7 +386,9 @@ class RequestRatelimiter:
if is_admin_redaction and self.admin_redaction_ratelimiter:
# If we have separate config for admin redactions, use a separate
# ratelimiter as to not have user_ids clash
- await self.admin_redaction_ratelimiter.ratelimit(requester, update=update)
+ await self.admin_redaction_ratelimiter.ratelimit(
+ requester, update=update, n_actions=n_actions
+ )
else:
# Override rate and burst count per-user
await self.request_ratelimiter.ratelimit(
@@ -391,4 +396,5 @@ class RequestRatelimiter:
rate_hz=messages_per_second,
burst_count=burst_count,
update=update,
+ n_actions=n_actions,
)
diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index fbac4375b0..60774b240d 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -123,7 +123,7 @@ class ApplicationServiceApi(SimpleHttpClient):
response = await self.get_json(
uri,
{"access_token": service.hs_token},
- headers={"Authorization": f"Bearer {service.hs_token}"},
+ headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
return True
@@ -147,7 +147,7 @@ class ApplicationServiceApi(SimpleHttpClient):
response = await self.get_json(
uri,
{"access_token": service.hs_token},
- headers={"Authorization": f"Bearer {service.hs_token}"},
+ headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
return True
@@ -190,7 +190,9 @@ class ApplicationServiceApi(SimpleHttpClient):
b"access_token": service.hs_token,
}
response = await self.get_json(
- uri, args=args, headers={"Authorization": f"Bearer {service.hs_token}"}
+ uri,
+ args=args,
+ headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if not isinstance(response, list):
logger.warning(
@@ -230,7 +232,7 @@ class ApplicationServiceApi(SimpleHttpClient):
info = await self.get_json(
uri,
{"access_token": service.hs_token},
- headers={"Authorization": f"Bearer {service.hs_token}"},
+ headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if not _is_valid_3pe_metadata(info):
@@ -327,7 +329,7 @@ class ApplicationServiceApi(SimpleHttpClient):
uri=uri,
json_body=body,
args={"access_token": service.hs_token},
- headers={"Authorization": f"Bearer {service.hs_token}"},
+ headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if logger.isEnabledFor(logging.DEBUG):
logger.debug(
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index de97886ea9..f10cfca073 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -557,7 +557,6 @@ class RoomCreationHandler:
invite_list=[],
initial_state=initial_state,
creation_content=creation_content,
- ratelimit=False,
)
# Transfer membership events
@@ -751,6 +750,10 @@ class RoomCreationHandler:
)
if ratelimit:
+ # Rate limit once in advance, but don't rate limit the individual
+ # events in the room — room creation isn't atomic and it's very
+ # janky if half the events in the initial state don't make it because
+ # of rate limiting.
await self.request_ratelimiter.ratelimit(requester)
room_version_id = config.get(
@@ -911,7 +914,6 @@ class RoomCreationHandler:
room_alias=room_alias,
power_level_content_override=power_level_content_override,
creator_join_profile=creator_join_profile,
- ratelimit=ratelimit,
)
if "name" in config:
@@ -1035,7 +1037,6 @@ class RoomCreationHandler:
room_alias: Optional[RoomAlias] = None,
power_level_content_override: Optional[JsonDict] = None,
creator_join_profile: Optional[JsonDict] = None,
- ratelimit: bool = True,
) -> Tuple[int, str, int]:
"""Sends the initial events into a new room. Sends the room creation, membership,
and power level events into the room sequentially, then creates and batches up the
@@ -1044,6 +1045,8 @@ class RoomCreationHandler:
`power_level_content_override` doesn't apply when initial state has
power level state event content.
+ Rate limiting should already have been applied by this point.
+
Returns:
A tuple containing the stream ID, event ID and depth of the last
event sent to the room.
@@ -1123,7 +1126,7 @@ class RoomCreationHandler:
creator.user,
room_id,
"join",
- ratelimit=ratelimit,
+ ratelimit=False,
content=creator_join_profile,
new_room=True,
prev_event_ids=[last_sent_event_id],
@@ -1248,7 +1251,10 @@ class RoomCreationHandler:
events_to_send.append((encryption_event, encryption_context))
last_event = await self.event_creation_handler.handle_new_client_event(
- creator, events_to_send, ignore_shadow_ban=True
+ creator,
+ events_to_send,
+ ignore_shadow_ban=True,
+ ratelimit=False,
)
assert last_event.internal_metadata.stream_ordering is not None
return last_event.internal_metadata.stream_ordering, last_event.event_id, depth
diff --git a/tests/appservice/test_api.py b/tests/appservice/test_api.py
index 11008ac1fb..89ee79396f 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
+from typing import Any, List, Mapping, Sequence, Union
from unittest.mock import Mock
from twisted.test.proto_helpers import MemoryReactor
@@ -70,13 +70,15 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
self.request_url = None
async def get_json(
- url: str, args: Mapping[Any, Any], headers: Mapping[Any, Any]
+ url: str,
+ 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"):
raise RuntimeError("Access token not provided")
- self.assertEqual(headers.get("Authorization"), f"Bearer {TOKEN}")
+ self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
self.assertEqual(args.get(b"access_token"), TOKEN)
self.request_url = url
if url == URL_USER:
diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index 716366eb90..1084d4ad9d 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -54,6 +54,7 @@ from tests.http.server._base import make_request_with_cancellation_test
from tests.storage.test_stream import PaginationTestCase
from tests.test_utils import make_awaitable
from tests.test_utils.event_injection import create_event
+from tests.unittest import override_config
PATH_PREFIX = b"/_matrix/client/api/v1"
@@ -871,6 +872,41 @@ class RoomsCreateTestCase(RoomBase):
self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body)
self.assertEqual(join_mock.call_count, 0)
+ def _create_basic_room(self) -> Tuple[int, object]:
+ """
+ Tries to create a basic room and returns the response code.
+ """
+ channel = self.make_request(
+ "POST",
+ "/createRoom",
+ {},
+ )
+ return channel.code, channel.json_body
+
+ @override_config(
+ {
+ "rc_message": {"per_second": 0.2, "burst_count": 10},
+ }
+ )
+ def test_room_creation_ratelimiting(self) -> None:
+ """
+ Regression test for #14312, where ratelimiting was made too strict.
+ Clients should be able to create 10 rooms in a row
+ without hitting rate limits, using default rate limit config.
+ (We override rate limiting config back to its default value.)
+
+ To ensure we don't make ratelimiting too generous accidentally,
+ also check that we can't create an 11th room.
+ """
+
+ for _ in range(10):
+ code, json_body = self._create_basic_room()
+ self.assertEqual(code, HTTPStatus.OK, json_body)
+
+ # The 6th room hits the rate limit.
+ code, json_body = self._create_basic_room()
+ self.assertEqual(code, HTTPStatus.TOO_MANY_REQUESTS, json_body)
+
class RoomTopicTestCase(RoomBase):
"""Tests /rooms/$room_id/topic REST events."""
@@ -1390,10 +1426,22 @@ class RoomJoinRatelimitTestCase(RoomBase):
)
def test_join_local_ratelimit(self) -> None:
"""Tests that local joins are actually rate-limited."""
- for _ in range(3):
- self.helper.create_room_as(self.user_id)
+ # Create 4 rooms
+ room_ids = [
+ self.helper.create_room_as(self.user_id, is_public=True) for _ in range(4)
+ ]
+
+ joiner_user_id = self.register_user("joiner", "secret")
+ # Now make a new user try to join some of them.
- self.helper.create_room_as(self.user_id, expect_code=429)
+ # The user can join 3 rooms
+ for room_id in room_ids[0:3]:
+ self.helper.join(room_id, joiner_user_id)
+
+ # But the user cannot join a 4th room
+ self.helper.join(
+ room_ids[3], joiner_user_id, expect_code=HTTPStatus.TOO_MANY_REQUESTS
+ )
@unittest.override_config(
{"rc_joins": {"local": {"per_second": 0.5, "burst_count": 3}}}
|
