From 000aa89be63c27092998eca03c97eaead21404cd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 19 Aug 2021 11:12:55 -0400 Subject: [PATCH 1/7] Do not include rooms with an unknown room version in a sync response. (#10644) A user will still see this room if it is in a local cache, but it will not reappear if clearing the cache and reloading. --- changelog.d/10644.bugfix | 1 + mypy.ini | 1 + synapse/handlers/sync.py | 7 +- synapse/storage/databases/main/roommember.py | 8 +- synapse/storage/roommember.py | 1 + tests/handlers/test_sync.py | 137 +++++++++++++++++- .../replication/slave/storage/test_events.py | 1 + 7 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 changelog.d/10644.bugfix diff --git a/changelog.d/10644.bugfix b/changelog.d/10644.bugfix new file mode 100644 index 000000000000..d88a81fd82d1 --- /dev/null +++ b/changelog.d/10644.bugfix @@ -0,0 +1 @@ +Rooms with unsupported room versions are no longer returned via `/sync`. diff --git a/mypy.ini b/mypy.ini index 107f4de76ce6..90ade37b3f9a 100644 --- a/mypy.ini +++ b/mypy.ini @@ -90,6 +90,7 @@ files = tests/test_utils, tests/handlers/test_password_providers.py, tests/handlers/test_room_summary.py, + tests/handlers/test_sync.py, tests/rest/client/v1/test_login.py, tests/rest/client/v2_alpha/test_auth.py, tests/util/test_itertools.py, diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index b7b299961ff3..2203c45dcc9a 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1,5 +1,4 @@ -# Copyright 2015, 2016 OpenMarket Ltd -# Copyright 2018, 2019 New Vector Ltd +# Copyright 2015-2021 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. @@ -31,6 +30,7 @@ from synapse.api.constants import AccountDataTypes, EventTypes, Membership from synapse.api.filtering import FilterCollection +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.events import EventBase from synapse.logging.context import current_context from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, start_active_span @@ -1843,6 +1843,9 @@ async def _get_all_rooms( knocked = [] for event in room_list: + if event.room_version_id not in KNOWN_ROOM_VERSIONS: + continue + if event.membership == Membership.JOIN: room_entries.append( RoomSyncResultBuilder( diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index c2f6b9d63d70..c58a4b869072 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -386,9 +386,10 @@ def _get_rooms_for_local_user_where_membership_is_txn( ) sql = """ - SELECT room_id, e.sender, c.membership, event_id, e.stream_ordering + SELECT room_id, e.sender, c.membership, event_id, e.stream_ordering, r.room_version FROM local_current_membership AS c INNER JOIN events AS e USING (room_id, event_id) + INNER JOIN rooms AS r USING (room_id) WHERE user_id = ? AND %s @@ -397,7 +398,7 @@ def _get_rooms_for_local_user_where_membership_is_txn( ) txn.execute(sql, (user_id, *args)) - results = [RoomsForUser(**r) for r in self.db_pool.cursor_to_dict(txn)] + results = [RoomsForUser(*r) for r in txn] return results @@ -447,7 +448,8 @@ async def get_rooms_for_user_with_stream_ordering( Returns: Returns the rooms the user is in currently, along with the stream - ordering of the most recent join for that user and room. + ordering of the most recent join for that user and room, along with + the room version of the room. """ return await self.db_pool.runInteraction( "get_rooms_for_user_with_stream_ordering", diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 9fad67ce48f5..2500381b7b85 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -30,6 +30,7 @@ class RoomsForUser: membership: str event_id: str stream_ordering: int + room_version_id: str @attr.s(slots=True, frozen=True, weakref_slot=False, auto_attribs=True) diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index 84f05f6c584c..339c039914f1 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -12,9 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import Optional + +from synapse.api.constants import EventTypes, JoinRules from synapse.api.errors import Codes, ResourceLimitError from synapse.api.filtering import DEFAULT_FILTER_COLLECTION +from synapse.api.room_versions import RoomVersions from synapse.handlers.sync import SyncConfig +from synapse.rest import admin +from synapse.rest.client import knock, login, room +from synapse.server import HomeServer from synapse.types import UserID, create_requester import tests.unittest @@ -24,8 +31,14 @@ class SyncTestCase(tests.unittest.HomeserverTestCase): """Tests Sync Handler.""" - def prepare(self, reactor, clock, hs): - self.hs = hs + servlets = [ + admin.register_servlets, + knock.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor, clock, hs: HomeServer): self.sync_handler = self.hs.get_sync_handler() self.store = self.hs.get_datastore() @@ -68,12 +81,124 @@ def test_wait_for_sync_for_user_auth_blocking(self): ) self.assertEquals(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED) + def test_unknown_room_version(self): + """ + A room with an unknown room version should not break sync (and should be excluded). + """ + inviter = self.register_user("creator", "pass", admin=True) + inviter_tok = self.login("@creator:test", "pass") + + user = self.register_user("user", "pass") + tok = self.login("user", "pass") + + # Do an initial sync on a different device. + requester = create_requester(user) + initial_result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, sync_config=generate_sync_config(user, device_id="dev") + ) + ) + + # Create a room as the user. + joined_room = self.helper.create_room_as(user, tok=tok) + + # Invite the user to the room as someone else. + invite_room = self.helper.create_room_as(inviter, tok=inviter_tok) + self.helper.invite(invite_room, targ=user, tok=inviter_tok) + + knock_room = self.helper.create_room_as( + inviter, room_version=RoomVersions.V7.identifier, tok=inviter_tok + ) + self.helper.send_state( + knock_room, + EventTypes.JoinRules, + {"join_rule": JoinRules.KNOCK}, + tok=inviter_tok, + ) + channel = self.make_request( + "POST", + "/_matrix/client/r0/knock/%s" % (knock_room,), + b"{}", + tok, + ) + self.assertEquals(200, channel.code, channel.result) + + # The rooms should appear in the sync response. + result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, sync_config=generate_sync_config(user) + ) + ) + self.assertIn(joined_room, [r.room_id for r in result.joined]) + self.assertIn(invite_room, [r.room_id for r in result.invited]) + self.assertIn(knock_room, [r.room_id for r in result.knocked]) + + # Test a incremental sync (by providing a since_token). + result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config(user, device_id="dev"), + since_token=initial_result.next_batch, + ) + ) + self.assertIn(joined_room, [r.room_id for r in result.joined]) + self.assertIn(invite_room, [r.room_id for r in result.invited]) + self.assertIn(knock_room, [r.room_id for r in result.knocked]) + + # Poke the database and update the room version to an unknown one. + for room_id in (joined_room, invite_room, knock_room): + self.get_success( + self.hs.get_datastores().main.db_pool.simple_update( + "rooms", + keyvalues={"room_id": room_id}, + updatevalues={"room_version": "unknown-room-version"}, + desc="updated-room-version", + ) + ) + + # Blow away caches (supported room versions can only change due to a restart). + self.get_success( + self.store.get_rooms_for_user_with_stream_ordering.invalidate_all() + ) + self.store._get_event_cache.clear() + + # The rooms should be excluded from the sync response. + # Get a new request key. + result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, sync_config=generate_sync_config(user) + ) + ) + self.assertNotIn(joined_room, [r.room_id for r in result.joined]) + self.assertNotIn(invite_room, [r.room_id for r in result.invited]) + self.assertNotIn(knock_room, [r.room_id for r in result.knocked]) + + # The rooms should also not be in an incremental sync. + result = self.get_success( + self.sync_handler.wait_for_sync_for_user( + requester, + sync_config=generate_sync_config(user, device_id="dev"), + since_token=initial_result.next_batch, + ) + ) + self.assertNotIn(joined_room, [r.room_id for r in result.joined]) + self.assertNotIn(invite_room, [r.room_id for r in result.invited]) + self.assertNotIn(knock_room, [r.room_id for r in result.knocked]) + + +_request_key = 0 + -def generate_sync_config(user_id: str) -> SyncConfig: +def generate_sync_config( + user_id: str, device_id: Optional[str] = "device_id" +) -> SyncConfig: + """Generate a sync config (with a unique request key).""" + global _request_key + _request_key += 1 return SyncConfig( - user=UserID(user_id.split(":")[0][1:], user_id.split(":")[1]), + user=UserID.from_string(user_id), filter_collection=DEFAULT_FILTER_COLLECTION, is_guest=False, - request_key="request_key", - device_id="device_id", + request_key=("request_key", _request_key), + device_id=device_id, ) diff --git a/tests/replication/slave/storage/test_events.py b/tests/replication/slave/storage/test_events.py index 8d1b0606c469..b25a06b4271a 100644 --- a/tests/replication/slave/storage/test_events.py +++ b/tests/replication/slave/storage/test_events.py @@ -150,6 +150,7 @@ def test_invites(self): "invite", event.event_id, event.internal_metadata.stream_ordering, + RoomVersions.V1.identifier, ) ], ) From 50af1efe4be8c93ee1fd642b60ab66d32317827b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 19 Aug 2021 17:31:40 +0100 Subject: [PATCH 2/7] Extract `_resolve_state_at_missing_prevs` (#10624) This is a follow-up to #10615: it takes the code that constructs the state at a backwards extremity, and extracts it to a separate method. --- changelog.d/10624.misc | 1 + synapse/handlers/federation.py | 229 ++++++++++++++++++--------------- 2 files changed, 125 insertions(+), 105 deletions(-) create mode 100644 changelog.d/10624.misc diff --git a/changelog.d/10624.misc b/changelog.d/10624.misc new file mode 100644 index 000000000000..9a765435dbe4 --- /dev/null +++ b/changelog.d/10624.misc @@ -0,0 +1 @@ +Clean up some of the federation event authentication code for clarity. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 529d025c39b5..de86918b7bc0 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -285,12 +285,12 @@ async def on_receive_pdu( # - Fetching any missing prev events to fill in gaps in the graph # - Fetching state if we have a hole in the graph if not pdu.internal_metadata.is_outlier(): - prevs = set(pdu.prev_event_ids()) - seen = await self.store.have_events_in_timeline(prevs) - missing_prevs = prevs - seen + if sent_to_us_directly: + prevs = set(pdu.prev_event_ids()) + seen = await self.store.have_events_in_timeline(prevs) + missing_prevs = prevs - seen - if missing_prevs: - if sent_to_us_directly: + if missing_prevs: # We only backfill backwards to the min depth. min_depth = await self.get_min_depth_for_context(pdu.room_id) logger.debug("min_depth: %d", min_depth) @@ -351,106 +351,8 @@ async def on_receive_pdu( affected=pdu.event_id, ) - else: - # We don't have all of the prev_events for this event. - # - # In this case, we need to fall back to asking another server in the - # federation for the state at this event. That's ok provided we then - # resolve the state against other bits of the DAG before using it (which - # will ensure that you can't just take over a room by sending an event, - # withholding its prev_events, and declaring yourself to be an admin in - # the subsequent state request). - # - # Since we're pulling this event as a missing prev_event, then clearly - # this event is not going to become the only forward-extremity and we are - # guaranteed to resolve its state against our existing forward - # extremities, so that should be fine. - # - # XXX this really feels like it could/should be merged with the above, - # but there is an interaction with min_depth that I'm not really - # following. - logger.info( - "Event %s is missing prev_events %s: calculating state for a " - "backwards extremity", - event_id, - shortstr(missing_prevs), - ) - - # Calculate the state after each of the previous events, and - # resolve them to find the correct state at the current event. - event_map = {event_id: pdu} - try: - # Get the state of the events we know about - ours = await self.state_store.get_state_groups_ids( - room_id, seen - ) - - # state_maps is a list of mappings from (type, state_key) to event_id - state_maps: List[StateMap[str]] = list(ours.values()) - - # we don't need this any more, let's delete it. - del ours - - # Ask the remote server for the states we don't - # know about - for p in missing_prevs: - logger.info( - "Requesting state after missing prev_event %s", p - ) - - with nested_logging_context(p): - # note that if any of the missing prevs share missing state or - # auth events, the requests to fetch those events are deduped - # by the get_pdu_cache in federation_client. - remote_state = ( - await self._get_state_after_missing_prev_event( - origin, room_id, p - ) - ) - - remote_state_map = { - (x.type, x.state_key): x.event_id - for x in remote_state - } - state_maps.append(remote_state_map) - - for x in remote_state: - event_map[x.event_id] = x - - room_version = await self.store.get_room_version_id(room_id) - state_map = await self._state_resolution_handler.resolve_events_with_store( - room_id, - room_version, - state_maps, - event_map, - state_res_store=StateResolutionStore(self.store), - ) - - # We need to give _process_received_pdu the actual state events - # rather than event ids, so generate that now. - - # First though we need to fetch all the events that are in - # state_map, so we can build up the state below. - evs = await self.store.get_events( - list(state_map.values()), - get_prev_content=False, - redact_behaviour=EventRedactBehaviour.AS_IS, - ) - event_map.update(evs) - - state = [event_map[e] for e in state_map.values()] - except Exception: - logger.warning( - "Error attempting to resolve state at missing " - "prev_events", - exc_info=True, - ) - raise FederationError( - "ERROR", - 403, - "We can't get valid state history.", - affected=event_id, - ) + else: + state = await self._resolve_state_at_missing_prevs(origin, pdu) # A second round of checks for all events. Check that the event passes auth # based on `auth_events`, this allows us to assert that the event would @@ -1493,6 +1395,123 @@ async def get_event(event_id: str): event_infos, ) + async def _resolve_state_at_missing_prevs( + self, dest: str, event: EventBase + ) -> Optional[Iterable[EventBase]]: + """Calculate the state at an event with missing prev_events. + + This is used when we have pulled a batch of events from a remote server, and + still don't have all the prev_events. + + If we already have all the prev_events for `event`, this method does nothing. + + Otherwise, the missing prevs become new backwards extremities, and we fall back + to asking the remote server for the state after each missing `prev_event`, + and resolving across them. + + That's ok provided we then resolve the state against other bits of the DAG + before using it - in other words, that the received event `event` is not going + to become the only forwards_extremity in the room (which will ensure that you + can't just take over a room by sending an event, withholding its prev_events, + and declaring yourself to be an admin in the subsequent state request). + + In other words: we should only call this method if `event` has been *pulled* + as part of a batch of missing prev events, or similar. + + Params: + dest: the remote server to ask for state at the missing prevs. Typically, + this will be the server we got `event` from. + event: an event to check for missing prevs. + + Returns: + if we already had all the prev events, `None`. Otherwise, returns a list of + the events in the state at `event`. + """ + room_id = event.room_id + event_id = event.event_id + + prevs = set(event.prev_event_ids()) + seen = await self.store.have_events_in_timeline(prevs) + missing_prevs = prevs - seen + + if not missing_prevs: + return None + + logger.info( + "Event %s is missing prev_events %s: calculating state for a " + "backwards extremity", + event_id, + shortstr(missing_prevs), + ) + # Calculate the state after each of the previous events, and + # resolve them to find the correct state at the current event. + event_map = {event_id: event} + try: + # Get the state of the events we know about + ours = await self.state_store.get_state_groups_ids(room_id, seen) + + # state_maps is a list of mappings from (type, state_key) to event_id + state_maps: List[StateMap[str]] = list(ours.values()) + + # we don't need this any more, let's delete it. + del ours + + # Ask the remote server for the states we don't + # know about + for p in missing_prevs: + logger.info("Requesting state after missing prev_event %s", p) + + with nested_logging_context(p): + # note that if any of the missing prevs share missing state or + # auth events, the requests to fetch those events are deduped + # by the get_pdu_cache in federation_client. + remote_state = await self._get_state_after_missing_prev_event( + dest, room_id, p + ) + + remote_state_map = { + (x.type, x.state_key): x.event_id for x in remote_state + } + state_maps.append(remote_state_map) + + for x in remote_state: + event_map[x.event_id] = x + + room_version = await self.store.get_room_version_id(room_id) + state_map = await self._state_resolution_handler.resolve_events_with_store( + room_id, + room_version, + state_maps, + event_map, + state_res_store=StateResolutionStore(self.store), + ) + + # We need to give _process_received_pdu the actual state events + # rather than event ids, so generate that now. + + # First though we need to fetch all the events that are in + # state_map, so we can build up the state below. + evs = await self.store.get_events( + list(state_map.values()), + get_prev_content=False, + redact_behaviour=EventRedactBehaviour.AS_IS, + ) + event_map.update(evs) + + state = [event_map[e] for e in state_map.values()] + except Exception: + logger.warning( + "Error attempting to resolve state at missing prev_events", + exc_info=True, + ) + raise FederationError( + "ERROR", + 403, + "We can't get valid state history.", + affected=event_id, + ) + return state + def _sanity_check_event(self, ev: EventBase) -> None: """ Do some early sanity checks of a received event From e81d62009ee091d69d56bca702ba0edd39becb1c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 19 Aug 2021 18:05:12 +0100 Subject: [PATCH 3/7] Split `on_receive_pdu` in half (#10640) Here we split on_receive_pdu into two functions (on_receive_pdu and process_pulled_event), rather than having both cases in the same method. There's a tiny bit of overlap, but not that much. --- changelog.d/10640.misc | 1 + synapse/federation/federation_server.py | 4 +- synapse/handlers/federation.py | 236 ++++++++++++++---------- tests/test_federation.py | 10 +- 4 files changed, 142 insertions(+), 109 deletions(-) create mode 100644 changelog.d/10640.misc diff --git a/changelog.d/10640.misc b/changelog.d/10640.misc new file mode 100644 index 000000000000..9a765435dbe4 --- /dev/null +++ b/changelog.d/10640.misc @@ -0,0 +1 @@ +Clean up some of the federation event authentication code for clarity. diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index afd8f8580a2f..e1b58d40c533 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -1005,9 +1005,7 @@ async def _process_incoming_pdus_in_room_inner( async with lock: logger.info("handling received PDU: %s", event) try: - await self.handler.on_receive_pdu( - origin, event, sent_to_us_directly=True - ) + await self.handler.on_receive_pdu(origin, event) except FederationError as e: # XXX: Ideally we'd inform the remote we failed to process # the event, but we can't return an error in the transaction diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index de86918b7bc0..246df43501bc 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -203,18 +203,13 @@ def __init__(self, hs: "HomeServer"): self._ephemeral_messages_enabled = hs.config.enable_ephemeral_messages - async def on_receive_pdu( - self, origin: str, pdu: EventBase, sent_to_us_directly: bool = False - ) -> None: - """Process a PDU received via a federation /send/ transaction, or - via backfill of missing prev_events + async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: + """Process a PDU received via a federation /send/ transaction Args: origin: server which initiated the /send/ transaction. Will be used to fetch missing events or state. pdu: received PDU - sent_to_us_directly: True if this event was pushed to us; False if - we pulled it as the result of a missing prev_event. """ room_id = pdu.room_id @@ -276,8 +271,6 @@ async def on_receive_pdu( ) return None - state = None - # Check that the event passes auth based on the state at the event. This is # done for events that are to be added to the timeline (non-outliers). # @@ -285,83 +278,72 @@ async def on_receive_pdu( # - Fetching any missing prev events to fill in gaps in the graph # - Fetching state if we have a hole in the graph if not pdu.internal_metadata.is_outlier(): - if sent_to_us_directly: - prevs = set(pdu.prev_event_ids()) - seen = await self.store.have_events_in_timeline(prevs) - missing_prevs = prevs - seen - - if missing_prevs: - # We only backfill backwards to the min depth. - min_depth = await self.get_min_depth_for_context(pdu.room_id) - logger.debug("min_depth: %d", min_depth) - - if min_depth is not None and pdu.depth > min_depth: - # If we're missing stuff, ensure we only fetch stuff one - # at a time. + prevs = set(pdu.prev_event_ids()) + seen = await self.store.have_events_in_timeline(prevs) + missing_prevs = prevs - seen + + if missing_prevs: + # We only backfill backwards to the min depth. + min_depth = await self.get_min_depth_for_context(pdu.room_id) + logger.debug("min_depth: %d", min_depth) + + if min_depth is not None and pdu.depth > min_depth: + # If we're missing stuff, ensure we only fetch stuff one + # at a time. + logger.info( + "Acquiring room lock to fetch %d missing prev_events: %s", + len(missing_prevs), + shortstr(missing_prevs), + ) + with (await self._room_pdu_linearizer.queue(pdu.room_id)): logger.info( - "Acquiring room lock to fetch %d missing prev_events: %s", + "Acquired room lock to fetch %d missing prev_events", len(missing_prevs), - shortstr(missing_prevs), ) - with (await self._room_pdu_linearizer.queue(pdu.room_id)): - logger.info( - "Acquired room lock to fetch %d missing prev_events", - len(missing_prevs), + + try: + await self._get_missing_events_for_pdu( + origin, pdu, prevs, min_depth ) + except Exception as e: + raise Exception( + "Error fetching missing prev_events for %s: %s" + % (event_id, e) + ) from e - try: - await self._get_missing_events_for_pdu( - origin, pdu, prevs, min_depth - ) - except Exception as e: - raise Exception( - "Error fetching missing prev_events for %s: %s" - % (event_id, e) - ) from e - - # Update the set of things we've seen after trying to - # fetch the missing stuff - seen = await self.store.have_events_in_timeline(prevs) - missing_prevs = prevs - seen - - if not missing_prevs: - logger.info("Found all missing prev_events") - - if missing_prevs: - # since this event was pushed to us, it is possible for it to - # become the only forward-extremity in the room, and we would then - # trust its state to be the state for the whole room. This is very - # bad. Further, if the event was pushed to us, there is no excuse - # for us not to have all the prev_events. (XXX: apart from - # min_depth?) - # - # We therefore reject any such events. - logger.warning( - "Rejecting: failed to fetch %d prev events: %s", - len(missing_prevs), - shortstr(missing_prevs), - ) - raise FederationError( - "ERROR", - 403, - ( - "Your server isn't divulging details about prev_events " - "referenced in this event." - ), - affected=pdu.event_id, - ) + # Update the set of things we've seen after trying to + # fetch the missing stuff + seen = await self.store.have_events_in_timeline(prevs) + missing_prevs = prevs - seen - else: - state = await self._resolve_state_at_missing_prevs(origin, pdu) + if not missing_prevs: + logger.info("Found all missing prev_events") + + if missing_prevs: + # since this event was pushed to us, it is possible for it to + # become the only forward-extremity in the room, and we would then + # trust its state to be the state for the whole room. This is very + # bad. Further, if the event was pushed to us, there is no excuse + # for us not to have all the prev_events. (XXX: apart from + # min_depth?) + # + # We therefore reject any such events. + logger.warning( + "Rejecting: failed to fetch %d prev events: %s", + len(missing_prevs), + shortstr(missing_prevs), + ) + raise FederationError( + "ERROR", + 403, + ( + "Your server isn't divulging details about prev_events " + "referenced in this event." + ), + affected=pdu.event_id, + ) - # A second round of checks for all events. Check that the event passes auth - # based on `auth_events`, this allows us to assert that the event would - # have been allowed at some point. If an event passes this check its OK - # for it to be used as part of a returned `/state` request, as either - # a) we received the event as part of the original join and so trust it, or - # b) we'll do a state resolution with existing state before it becomes - # part of the "current state", which adds more protection. - await self._process_received_pdu(origin, pdu, state=state) + await self._process_received_pdu(origin, pdu, state=None) async def _get_missing_events_for_pdu( self, origin: str, pdu: EventBase, prevs: Set[str], min_depth: int @@ -461,24 +443,7 @@ async def _get_missing_events_for_pdu( return logger.info("Got %d prev_events", len(missing_events)) - - # We want to sort these by depth so we process them and - # tell clients about them in order. - missing_events.sort(key=lambda x: x.depth) - - for ev in missing_events: - logger.info("Handling received prev_event %s", ev) - with nested_logging_context(ev.event_id): - try: - await self.on_receive_pdu(origin, ev, sent_to_us_directly=False) - except FederationError as e: - if e.code == 403: - logger.warning( - "Received prev_event %s failed history check.", - ev.event_id, - ) - else: - raise + await self._process_pulled_events(origin, missing_events) async def _get_state_for_room( self, @@ -1395,6 +1360,81 @@ async def get_event(event_id: str): event_infos, ) + async def _process_pulled_events( + self, origin: str, events: Iterable[EventBase] + ) -> None: + """Process a batch of events we have pulled from a remote server + + Pulls in any events required to auth the events, persists the received events, + and notifies clients, if appropriate. + + Assumes the events have already had their signatures and hashes checked. + + Params: + origin: The server we received these events from + events: The received events. + """ + + # We want to sort these by depth so we process them and + # tell clients about them in order. + sorted_events = sorted(events, key=lambda x: x.depth) + + for ev in sorted_events: + with nested_logging_context(ev.event_id): + await self._process_pulled_event(origin, ev) + + async def _process_pulled_event(self, origin: str, event: EventBase) -> None: + """Process a single event that we have pulled from a remote server + + Pulls in any events required to auth the event, persists the received event, + and notifies clients, if appropriate. + + Assumes the event has already had its signatures and hashes checked. + + This is somewhat equivalent to on_receive_pdu, but applies somewhat different + logic in the case that we are missing prev_events (in particular, it just + requests the state at that point, rather than triggering a get_missing_events) - + so is appropriate when we have pulled the event from a remote server, rather + than having it pushed to us. + + Params: + origin: The server we received this event from + events: The received event + """ + logger.info("Processing pulled event %s", event) + + # these should not be outliers. + assert not event.internal_metadata.is_outlier() + + event_id = event.event_id + + existing = await self.store.get_event( + event_id, allow_none=True, allow_rejected=True + ) + if existing: + if not existing.internal_metadata.is_outlier(): + logger.info( + "Ignoring received event %s which we have already seen", + event_id, + ) + return + logger.info("De-outliering event %s", event_id) + + try: + self._sanity_check_event(event) + except SynapseError as err: + logger.warning("Event %s failed sanity check: %s", event_id, err) + return + + try: + state = await self._resolve_state_at_missing_prevs(origin, event) + await self._process_received_pdu(origin, event, state=state) + except FederationError as e: + if e.code == 403: + logger.warning("Pulled event %s failed history check.", event_id) + else: + raise + async def _resolve_state_at_missing_prevs( self, dest: str, event: EventBase ) -> Optional[Iterable[EventBase]]: @@ -1780,7 +1820,7 @@ async def _handle_queued_pdus( p, ) with nested_logging_context(p.event_id): - await self.on_receive_pdu(origin, p, sent_to_us_directly=True) + await self.on_receive_pdu(origin, p) except Exception as e: logger.warning( "Error handling queued PDU %s from %s: %s", p.event_id, origin, e diff --git a/tests/test_federation.py b/tests/test_federation.py index 3785799f46d2..348fcb72a7e0 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -85,11 +85,7 @@ def setUp(self): # Send the join, it should return None (which is not an error) self.assertEqual( - self.get_success( - self.handler.on_receive_pdu( - "test.serv", join_event, sent_to_us_directly=True - ) - ), + self.get_success(self.handler.on_receive_pdu("test.serv", join_event)), None, ) @@ -135,9 +131,7 @@ async def post_json(destination, path, data, headers=None, timeout=0): with LoggingContext("test-context"): failure = self.get_failure( - self.handler.on_receive_pdu( - "test.serv", lying_event, sent_to_us_directly=True - ), + self.handler.on_receive_pdu("test.serv", lying_event), FederationError, ) From ee3b2ac59a5645cb213b3c11c473613b80fe1e0f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 20 Aug 2021 15:47:03 +0100 Subject: [PATCH 4/7] Validate device_keys for C-S /keys/query requests (#10593) * Validate device_keys for C-S /keys/query requests Closes #10354 A small, not particularly critical fix. I'm interested in seeing if we can find a more systematic approach though. #8445 is the place for any discussion. --- changelog.d/10593.bugfix | 1 + synapse/api/errors.py | 8 +++ synapse/rest/client/keys.py | 16 ++++- tests/rest/client/v2_alpha/test_keys.py | 77 +++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 changelog.d/10593.bugfix create mode 100644 tests/rest/client/v2_alpha/test_keys.py diff --git a/changelog.d/10593.bugfix b/changelog.d/10593.bugfix new file mode 100644 index 000000000000..492e58a7a836 --- /dev/null +++ b/changelog.d/10593.bugfix @@ -0,0 +1 @@ +Reject Client-Server /keys/query requests which provide device_ids incorrectly. \ No newline at end of file diff --git a/synapse/api/errors.py b/synapse/api/errors.py index dc662bca8353..9480f448d7a5 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -147,6 +147,14 @@ def error_dict(self): return cs_error(self.msg, self.errcode) +class InvalidAPICallError(SynapseError): + """You called an existing API endpoint, but fed that endpoint + invalid or incomplete data.""" + + def __init__(self, msg: str): + super().__init__(HTTPStatus.BAD_REQUEST, msg, Codes.BAD_JSON) + + class ProxiedRequestError(SynapseError): """An error from a general matrix endpoint, eg. from a proxied Matrix API call. diff --git a/synapse/rest/client/keys.py b/synapse/rest/client/keys.py index d0d9d30d40a3..012491f59736 100644 --- a/synapse/rest/client/keys.py +++ b/synapse/rest/client/keys.py @@ -15,8 +15,9 @@ # limitations under the License. import logging +from typing import Any -from synapse.api.errors import SynapseError +from synapse.api.errors import InvalidAPICallError, SynapseError from synapse.http.servlet import ( RestServlet, parse_integer, @@ -163,6 +164,19 @@ async def on_POST(self, request): device_id = requester.device_id timeout = parse_integer(request, "timeout", 10 * 1000) body = parse_json_object_from_request(request) + + device_keys = body.get("device_keys") + if not isinstance(device_keys, dict): + raise InvalidAPICallError("'device_keys' must be a JSON object") + + def is_list_of_strings(values: Any) -> bool: + return isinstance(values, list) and all(isinstance(v, str) for v in values) + + if any(not is_list_of_strings(keys) for keys in device_keys.values()): + raise InvalidAPICallError( + "'device_keys' values must be a list of strings", + ) + result = await self.e2e_keys_handler.query_devices( body, timeout, user_id, device_id ) diff --git a/tests/rest/client/v2_alpha/test_keys.py b/tests/rest/client/v2_alpha/test_keys.py new file mode 100644 index 000000000000..80a4e728ff30 --- /dev/null +++ b/tests/rest/client/v2_alpha/test_keys.py @@ -0,0 +1,77 @@ +from http import HTTPStatus + +from synapse.api.errors import Codes +from synapse.rest import admin +from synapse.rest.client import keys, login + +from tests import unittest + + +class KeyQueryTestCase(unittest.HomeserverTestCase): + servlets = [ + keys.register_servlets, + admin.register_servlets_for_client_rest_resource, + login.register_servlets, + ] + + def test_rejects_device_id_ice_key_outside_of_list(self): + self.register_user("alice", "wonderland") + alice_token = self.login("alice", "wonderland") + bob = self.register_user("bob", "uncle") + channel = self.make_request( + "POST", + "/_matrix/client/r0/keys/query", + { + "device_keys": { + bob: "device_id1", + }, + }, + alice_token, + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertEqual( + channel.json_body["errcode"], + Codes.BAD_JSON, + channel.result, + ) + + def test_rejects_device_key_given_as_map_to_bool(self): + self.register_user("alice", "wonderland") + alice_token = self.login("alice", "wonderland") + bob = self.register_user("bob", "uncle") + channel = self.make_request( + "POST", + "/_matrix/client/r0/keys/query", + { + "device_keys": { + bob: { + "device_id1": True, + }, + }, + }, + alice_token, + ) + + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertEqual( + channel.json_body["errcode"], + Codes.BAD_JSON, + channel.result, + ) + + def test_requires_device_key(self): + """`device_keys` is required. We should complain if it's missing.""" + self.register_user("alice", "wonderland") + alice_token = self.login("alice", "wonderland") + channel = self.make_request( + "POST", + "/_matrix/client/r0/keys/query", + {}, + alice_token, + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertEqual( + channel.json_body["errcode"], + Codes.BAD_JSON, + channel.result, + ) From 7862d704fdececf5a3a10466e2e7aee81fdf10f4 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 20 Aug 2021 16:31:02 +0100 Subject: [PATCH 5/7] Follow-up: format changelog, add licence (#10593) Merged before approval; these comments from @clokep on that PR. --- changelog.d/10593.bugfix | 2 +- tests/rest/client/v2_alpha/test_keys.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/changelog.d/10593.bugfix b/changelog.d/10593.bugfix index 492e58a7a836..af910bfa4d98 100644 --- a/changelog.d/10593.bugfix +++ b/changelog.d/10593.bugfix @@ -1 +1 @@ -Reject Client-Server /keys/query requests which provide device_ids incorrectly. \ No newline at end of file +Reject Client-Server `/keys/query` requests which provide `device_ids` incorrectly. diff --git a/tests/rest/client/v2_alpha/test_keys.py b/tests/rest/client/v2_alpha/test_keys.py index 80a4e728ff30..d7fa635eae1d 100644 --- a/tests/rest/client/v2_alpha/test_keys.py +++ b/tests/rest/client/v2_alpha/test_keys.py @@ -1,3 +1,17 @@ +# Copyright 2021 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 http import HTTPStatus from synapse.api.errors import Codes From f499dc38bcdf0cec83b873923af3d5310538759e Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 20 Aug 2021 17:43:26 +0200 Subject: [PATCH 6/7] Simplify tests for the device admin rest API. (#10664) By replacing duplicated code with parameterized tests and avoiding unnecessary dumping of JSON data. --- changelog.d/10664.misc | 1 + tests/rest/admin/test_device.py | 99 ++++++--------------------------- 2 files changed, 19 insertions(+), 81 deletions(-) create mode 100644 changelog.d/10664.misc diff --git a/changelog.d/10664.misc b/changelog.d/10664.misc new file mode 100644 index 000000000000..cebd5e9a96c8 --- /dev/null +++ b/changelog.d/10664.misc @@ -0,0 +1 @@ +Simplify tests for device admin rest API. \ No newline at end of file diff --git a/tests/rest/admin/test_device.py b/tests/rest/admin/test_device.py index c4afe5c3d90b..a3679be20539 100644 --- a/tests/rest/admin/test_device.py +++ b/tests/rest/admin/test_device.py @@ -12,9 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import urllib.parse +from parameterized import parameterized + import synapse.rest.admin from synapse.api.errors import Codes from synapse.rest.client import login @@ -45,49 +46,23 @@ def prepare(self, reactor, clock, hs): self.other_user_device_id, ) - def test_no_auth(self): + @parameterized.expand(["GET", "PUT", "DELETE"]) + def test_no_auth(self, method: str): """ Try to get a device of an user without authentication. """ - channel = self.make_request("GET", self.url, b"{}") - - self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) - - channel = self.make_request("PUT", self.url, b"{}") - - self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) - - channel = self.make_request("DELETE", self.url, b"{}") + channel = self.make_request(method, self.url, b"{}") self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) - def test_requester_is_no_admin(self): + @parameterized.expand(["GET", "PUT", "DELETE"]) + def test_requester_is_no_admin(self, method: str): """ If the user is not a server admin, an error is returned. """ channel = self.make_request( - "GET", - self.url, - access_token=self.other_user_token, - ) - - self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - - channel = self.make_request( - "PUT", - self.url, - access_token=self.other_user_token, - ) - - self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - - channel = self.make_request( - "DELETE", + method, self.url, access_token=self.other_user_token, ) @@ -95,7 +70,8 @@ def test_requester_is_no_admin(self): self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) - def test_user_does_not_exist(self): + @parameterized.expand(["GET", "PUT", "DELETE"]) + def test_user_does_not_exist(self, method: str): """ Tests that a lookup for a user that does not exist returns a 404 """ @@ -105,7 +81,7 @@ def test_user_does_not_exist(self): ) channel = self.make_request( - "GET", + method, url, access_token=self.admin_user_tok, ) @@ -113,25 +89,8 @@ def test_user_does_not_exist(self): self.assertEqual(404, channel.code, msg=channel.json_body) self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - channel = self.make_request( - "PUT", - url, - access_token=self.admin_user_tok, - ) - - self.assertEqual(404, channel.code, msg=channel.json_body) - self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - - channel = self.make_request( - "DELETE", - url, - access_token=self.admin_user_tok, - ) - - self.assertEqual(404, channel.code, msg=channel.json_body) - self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) - - def test_user_is_not_local(self): + @parameterized.expand(["GET", "PUT", "DELETE"]) + def test_user_is_not_local(self, method: str): """ Tests that a lookup for a user that is not a local returns a 400 """ @@ -141,25 +100,7 @@ def test_user_is_not_local(self): ) channel = self.make_request( - "GET", - url, - access_token=self.admin_user_tok, - ) - - self.assertEqual(400, channel.code, msg=channel.json_body) - self.assertEqual("Can only lookup local users", channel.json_body["error"]) - - channel = self.make_request( - "PUT", - url, - access_token=self.admin_user_tok, - ) - - self.assertEqual(400, channel.code, msg=channel.json_body) - self.assertEqual("Can only lookup local users", channel.json_body["error"]) - - channel = self.make_request( - "DELETE", + method, url, access_token=self.admin_user_tok, ) @@ -219,12 +160,11 @@ def test_update_device_too_long_display_name(self): * (synapse.handlers.device.MAX_DEVICE_DISPLAY_NAME_LEN + 1) } - body = json.dumps(update) channel = self.make_request( "PUT", self.url, access_token=self.admin_user_tok, - content=body.encode(encoding="utf_8"), + content=update, ) self.assertEqual(400, channel.code, msg=channel.json_body) @@ -275,12 +215,11 @@ def test_update_display_name(self): Tests a normal successful update of display name """ # Set new display_name - body = json.dumps({"display_name": "new displayname"}) channel = self.make_request( "PUT", self.url, access_token=self.admin_user_tok, - content=body.encode(encoding="utf_8"), + content={"display_name": "new displayname"}, ) self.assertEqual(200, channel.code, msg=channel.json_body) @@ -529,12 +468,11 @@ def test_unknown_devices(self): """ Tests that a remove of a device that does not exist returns 200. """ - body = json.dumps({"devices": ["unknown_device1", "unknown_device2"]}) channel = self.make_request( "POST", self.url, access_token=self.admin_user_tok, - content=body.encode(encoding="utf_8"), + content={"devices": ["unknown_device1", "unknown_device2"]}, ) # Delete unknown devices returns status 200 @@ -560,12 +498,11 @@ def test_delete_devices(self): device_ids.append(str(d["device_id"])) # Delete devices - body = json.dumps({"devices": device_ids}) channel = self.make_request( "POST", self.url, access_token=self.admin_user_tok, - content=body.encode(encoding="utf_8"), + content={"devices": device_ids}, ) self.assertEqual(200, channel.code, msg=channel.json_body) From ecd823d766fecdd7e1c7163073c097a0084122e2 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 20 Aug 2021 17:50:44 +0100 Subject: [PATCH 7/7] Flatten tests/rest/client/{v1,v2_alpha} too (#10667) --- changelog.d/10667.misc | 1 + mypy.ini | 4 ++-- tests/rest/client/{v2_alpha => }/test_account.py | 0 tests/rest/client/{v2_alpha => }/test_auth.py | 2 +- .../rest/client/{v2_alpha => }/test_capabilities.py | 0 tests/rest/client/{v1 => }/test_directory.py | 0 tests/rest/client/{v1 => }/test_events.py | 0 tests/rest/client/{v2_alpha => }/test_filter.py | 0 tests/rest/client/{v2_alpha => }/test_keys.py | 0 tests/rest/client/{v1 => }/test_login.py | 2 +- .../client/{v2_alpha => }/test_password_policy.py | 0 tests/rest/client/{v1 => }/test_presence.py | 0 tests/rest/client/{v1 => }/test_profile.py | 0 tests/rest/client/{v1 => }/test_push_rule_attrs.py | 0 tests/rest/client/{v2_alpha => }/test_register.py | 0 tests/rest/client/{v2_alpha => }/test_relations.py | 0 .../rest/client/{v2_alpha => }/test_report_event.py | 0 tests/rest/client/{v1 => }/test_rooms.py | 0 .../rest/client/{v2_alpha => }/test_sendtodevice.py | 0 .../rest/client/{v2_alpha => }/test_shared_rooms.py | 0 tests/rest/client/{v2_alpha => }/test_sync.py | 0 tests/rest/client/{v1 => }/test_typing.py | 0 .../rest/client/{v2_alpha => }/test_upgrade_room.py | 0 tests/rest/client/{v1 => }/utils.py | 0 tests/rest/client/v1/__init__.py | 13 ------------- tests/rest/client/v2_alpha/__init__.py | 0 tests/unittest.py | 2 +- 27 files changed, 6 insertions(+), 18 deletions(-) create mode 100644 changelog.d/10667.misc rename tests/rest/client/{v2_alpha => }/test_account.py (100%) rename tests/rest/client/{v2_alpha => }/test_auth.py (99%) rename tests/rest/client/{v2_alpha => }/test_capabilities.py (100%) rename tests/rest/client/{v1 => }/test_directory.py (100%) rename tests/rest/client/{v1 => }/test_events.py (100%) rename tests/rest/client/{v2_alpha => }/test_filter.py (100%) rename tests/rest/client/{v2_alpha => }/test_keys.py (100%) rename tests/rest/client/{v1 => }/test_login.py (99%) rename tests/rest/client/{v2_alpha => }/test_password_policy.py (100%) rename tests/rest/client/{v1 => }/test_presence.py (100%) rename tests/rest/client/{v1 => }/test_profile.py (100%) rename tests/rest/client/{v1 => }/test_push_rule_attrs.py (100%) rename tests/rest/client/{v2_alpha => }/test_register.py (100%) rename tests/rest/client/{v2_alpha => }/test_relations.py (100%) rename tests/rest/client/{v2_alpha => }/test_report_event.py (100%) rename tests/rest/client/{v1 => }/test_rooms.py (100%) rename tests/rest/client/{v2_alpha => }/test_sendtodevice.py (100%) rename tests/rest/client/{v2_alpha => }/test_shared_rooms.py (100%) rename tests/rest/client/{v2_alpha => }/test_sync.py (100%) rename tests/rest/client/{v1 => }/test_typing.py (100%) rename tests/rest/client/{v2_alpha => }/test_upgrade_room.py (100%) rename tests/rest/client/{v1 => }/utils.py (100%) delete mode 100644 tests/rest/client/v1/__init__.py delete mode 100644 tests/rest/client/v2_alpha/__init__.py diff --git a/changelog.d/10667.misc b/changelog.d/10667.misc new file mode 100644 index 000000000000..c92846ae260a --- /dev/null +++ b/changelog.d/10667.misc @@ -0,0 +1 @@ +Flatten the `tests.synapse.rests` package by moving the contents of `v1` and `v2_alpha` into the parent. \ No newline at end of file diff --git a/mypy.ini b/mypy.ini index 90ade37b3f9a..b17872211e72 100644 --- a/mypy.ini +++ b/mypy.ini @@ -91,8 +91,8 @@ files = tests/handlers/test_password_providers.py, tests/handlers/test_room_summary.py, tests/handlers/test_sync.py, - tests/rest/client/v1/test_login.py, - tests/rest/client/v2_alpha/test_auth.py, + tests/rest/client/test_login.py, + tests/rest/client/test_auth.py, tests/util/test_itertools.py, tests/util/test_stream_change_cache.py diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/test_account.py similarity index 100% rename from tests/rest/client/v2_alpha/test_account.py rename to tests/rest/client/test_account.py diff --git a/tests/rest/client/v2_alpha/test_auth.py b/tests/rest/client/test_auth.py similarity index 99% rename from tests/rest/client/v2_alpha/test_auth.py rename to tests/rest/client/test_auth.py index cf5cfb910c8c..e2fcbdc63ac6 100644 --- a/tests/rest/client/v2_alpha/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -25,7 +25,7 @@ from tests import unittest from tests.handlers.test_oidc import HAS_OIDC -from tests.rest.client.v1.utils import TEST_OIDC_CONFIG +from tests.rest.client.utils import TEST_OIDC_CONFIG from tests.server import FakeChannel from tests.unittest import override_config, skip_unless diff --git a/tests/rest/client/v2_alpha/test_capabilities.py b/tests/rest/client/test_capabilities.py similarity index 100% rename from tests/rest/client/v2_alpha/test_capabilities.py rename to tests/rest/client/test_capabilities.py diff --git a/tests/rest/client/v1/test_directory.py b/tests/rest/client/test_directory.py similarity index 100% rename from tests/rest/client/v1/test_directory.py rename to tests/rest/client/test_directory.py diff --git a/tests/rest/client/v1/test_events.py b/tests/rest/client/test_events.py similarity index 100% rename from tests/rest/client/v1/test_events.py rename to tests/rest/client/test_events.py diff --git a/tests/rest/client/v2_alpha/test_filter.py b/tests/rest/client/test_filter.py similarity index 100% rename from tests/rest/client/v2_alpha/test_filter.py rename to tests/rest/client/test_filter.py diff --git a/tests/rest/client/v2_alpha/test_keys.py b/tests/rest/client/test_keys.py similarity index 100% rename from tests/rest/client/v2_alpha/test_keys.py rename to tests/rest/client/test_keys.py diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/test_login.py similarity index 99% rename from tests/rest/client/v1/test_login.py rename to tests/rest/client/test_login.py index eba3552b19ac..5b2243fe5205 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/test_login.py @@ -32,7 +32,7 @@ from tests import unittest from tests.handlers.test_oidc import HAS_OIDC from tests.handlers.test_saml import has_saml2 -from tests.rest.client.v1.utils import TEST_OIDC_AUTH_ENDPOINT, TEST_OIDC_CONFIG +from tests.rest.client.utils import TEST_OIDC_AUTH_ENDPOINT, TEST_OIDC_CONFIG from tests.test_utils.html_parsers import TestHtmlParser from tests.unittest import HomeserverTestCase, override_config, skip_unless diff --git a/tests/rest/client/v2_alpha/test_password_policy.py b/tests/rest/client/test_password_policy.py similarity index 100% rename from tests/rest/client/v2_alpha/test_password_policy.py rename to tests/rest/client/test_password_policy.py diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/test_presence.py similarity index 100% rename from tests/rest/client/v1/test_presence.py rename to tests/rest/client/test_presence.py diff --git a/tests/rest/client/v1/test_profile.py b/tests/rest/client/test_profile.py similarity index 100% rename from tests/rest/client/v1/test_profile.py rename to tests/rest/client/test_profile.py diff --git a/tests/rest/client/v1/test_push_rule_attrs.py b/tests/rest/client/test_push_rule_attrs.py similarity index 100% rename from tests/rest/client/v1/test_push_rule_attrs.py rename to tests/rest/client/test_push_rule_attrs.py diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/test_register.py similarity index 100% rename from tests/rest/client/v2_alpha/test_register.py rename to tests/rest/client/test_register.py diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/test_relations.py similarity index 100% rename from tests/rest/client/v2_alpha/test_relations.py rename to tests/rest/client/test_relations.py diff --git a/tests/rest/client/v2_alpha/test_report_event.py b/tests/rest/client/test_report_event.py similarity index 100% rename from tests/rest/client/v2_alpha/test_report_event.py rename to tests/rest/client/test_report_event.py diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/test_rooms.py similarity index 100% rename from tests/rest/client/v1/test_rooms.py rename to tests/rest/client/test_rooms.py diff --git a/tests/rest/client/v2_alpha/test_sendtodevice.py b/tests/rest/client/test_sendtodevice.py similarity index 100% rename from tests/rest/client/v2_alpha/test_sendtodevice.py rename to tests/rest/client/test_sendtodevice.py diff --git a/tests/rest/client/v2_alpha/test_shared_rooms.py b/tests/rest/client/test_shared_rooms.py similarity index 100% rename from tests/rest/client/v2_alpha/test_shared_rooms.py rename to tests/rest/client/test_shared_rooms.py diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/test_sync.py similarity index 100% rename from tests/rest/client/v2_alpha/test_sync.py rename to tests/rest/client/test_sync.py diff --git a/tests/rest/client/v1/test_typing.py b/tests/rest/client/test_typing.py similarity index 100% rename from tests/rest/client/v1/test_typing.py rename to tests/rest/client/test_typing.py diff --git a/tests/rest/client/v2_alpha/test_upgrade_room.py b/tests/rest/client/test_upgrade_room.py similarity index 100% rename from tests/rest/client/v2_alpha/test_upgrade_room.py rename to tests/rest/client/test_upgrade_room.py diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/utils.py similarity index 100% rename from tests/rest/client/v1/utils.py rename to tests/rest/client/utils.py diff --git a/tests/rest/client/v1/__init__.py b/tests/rest/client/v1/__init__.py deleted file mode 100644 index 5e83dba2ed6f..000000000000 --- a/tests/rest/client/v1/__init__.py +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright 2014-2016 OpenMarket Ltd -# -# 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. diff --git a/tests/rest/client/v2_alpha/__init__.py b/tests/rest/client/v2_alpha/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/unittest.py b/tests/unittest.py index 3eec9c4d5b6f..f2c90cc47b53 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -252,7 +252,7 @@ def setUp(self): reactor=self.reactor, ) - from tests.rest.client.v1.utils import RestHelper + from tests.rest.client.utils import RestHelper self.helper = RestHelper(self.hs, self.site, getattr(self, "user_id", None))