From f5b63716a52d2286aa67c7a103602ee23a506560 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 26 Feb 2021 14:24:23 -0500 Subject: [PATCH 1/6] Convert RoomID to string. --- synapse/rest/admin/rooms.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 1a3a36f6cf0c..4cdc5c23f163 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -363,7 +363,7 @@ async def on_POST( raise NotFoundError("User not found") if RoomID.is_valid(room_identifier): - room_id = room_identifier + resolved_room_id = room_identifier try: remote_room_hosts = [ x.decode("ascii") for x in request.args[b"server_name"] @@ -374,6 +374,7 @@ async def on_POST( handler = self.room_member_handler room_alias = RoomAlias.from_string(room_identifier) room_id, remote_room_hosts = await handler.lookup_room_alias(room_alias) + resolved_room_id = room_id.to_string() else: raise SynapseError( 400, "%s was not legal room ID or room alias" % (room_identifier,) @@ -384,7 +385,7 @@ async def on_POST( ) # send invite if room has "JoinRules.INVITE" - room_state = await self.state_handler.get_current_state(room_id) + room_state = await self.state_handler.get_current_state(resolved_room_id) join_rules_event = room_state.get((EventTypes.JoinRules, "")) if join_rules_event: if not (join_rules_event.content.get("join_rule") == JoinRules.PUBLIC): @@ -394,7 +395,7 @@ async def on_POST( await self.room_member_handler.update_membership( requester=requester, target=fake_requester.user, - room_id=room_id, + room_id=resolved_room_id, action="invite", remote_room_hosts=remote_room_hosts, ratelimit=False, @@ -403,13 +404,13 @@ async def on_POST( await self.room_member_handler.update_membership( requester=fake_requester, target=fake_requester.user, - room_id=room_id, + room_id=resolved_room_id, action="join", remote_room_hosts=remote_room_hosts, ratelimit=False, ) - return 200, {"room_id": room_id} + return 200, {"room_id": resolved_room_id} class MakeRoomAdminRestServlet(RestServlet): From 7fd82fa857ad4610045212b18b94cea66ea3560e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 26 Feb 2021 14:32:31 -0500 Subject: [PATCH 2/6] Make a similar change in other spots. --- synapse/rest/admin/rooms.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 4cdc5c23f163..1d725458fe5e 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -442,11 +442,11 @@ async def on_POST(self, request, room_identifier): # Resolve to a room ID, if necessary. if RoomID.is_valid(room_identifier): - room_id = room_identifier + resolved_room_id = room_identifier elif RoomAlias.is_valid(room_identifier): room_alias = RoomAlias.from_string(room_identifier) room_id, _ = await self.room_member_handler.lookup_room_alias(room_alias) - room_id = room_id.to_string() + resolved_room_id = room_id.to_string() else: raise SynapseError( 400, "%s was not legal room ID or room alias" % (room_identifier,) @@ -456,7 +456,7 @@ async def on_POST(self, request, room_identifier): user_to_add = content.get("user_id", requester.user.to_string()) # Figure out which local users currently have power in the room, if any. - room_state = await self.state_handler.get_current_state(room_id) + room_state = await self.state_handler.get_current_state(resolved_room_id) if not room_state: raise SynapseError(400, "Server not in room") @@ -517,7 +517,7 @@ async def on_POST(self, request, room_identifier): "sender": admin_user_id, "type": EventTypes.PowerLevels, "state_key": "", - "room_id": room_id, + "room_id": resolved_room_id, }, ) except AuthError: @@ -550,7 +550,7 @@ async def on_POST(self, request, room_identifier): await self.room_member_handler.update_membership( fake_requester, target=UserID.from_string(user_to_add), - room_id=room_id, + room_id=resolved_room_id, action=Membership.INVITE, ) From a4a9669844c595e6a87321d39661f74ab767857a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 26 Feb 2021 14:48:56 -0500 Subject: [PATCH 3/6] Add missing type hints. --- synapse/rest/admin/rooms.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 1d725458fe5e..ac1e9a340032 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -435,7 +435,9 @@ def __init__(self, hs: "HomeServer"): self.state_handler = hs.get_state_handler() self.is_mine_id = hs.is_mine_id - async def on_POST(self, request, room_identifier): + async def on_POST( + self, request: SynapseRequest, room_identifier: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) content = parse_json_object_from_request(request, allow_empty_body=True) @@ -595,7 +597,9 @@ async def resolve_room_id(self, room_identifier: str) -> str: ) return resolved_room_id - async def on_DELETE(self, request, room_identifier): + async def on_DELETE( + self, request: SynapseRequest, room_identifier: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) @@ -604,7 +608,9 @@ async def on_DELETE(self, request, room_identifier): deleted_count = await self.store.delete_forward_extremities_for_room(room_id) return 200, {"deleted": deleted_count} - async def on_GET(self, request, room_identifier): + async def on_GET( + self, request: SynapseRequest, room_identifier: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) @@ -624,14 +630,16 @@ class RoomEventContextServlet(RestServlet): PATTERNS = admin_patterns("/rooms/(?P[^/]*)/context/(?P[^/]*)$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer"): super().__init__() self.clock = hs.get_clock() self.room_context_handler = hs.get_room_context_handler() self._event_serializer = hs.get_event_client_serializer() self.auth = hs.get_auth() - async def on_GET(self, request, room_id, event_id): + async def on_GET( + self, request: SynapseRequest, room_id: str, event_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request, allow_guest=False) await assert_user_is_admin(self.auth, requester.user) From 0f8fdb633c9620457d7dd71540cc3b4a051d9d2e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 26 Feb 2021 14:54:14 -0500 Subject: [PATCH 4/6] Share code to resolve room alias. --- synapse/rest/admin/rooms.py | 129 +++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 61 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index ac1e9a340032..f2c42a0f30a4 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -44,6 +44,48 @@ logger = logging.getLogger(__name__) +class ResolveRoomIdMixin: + def __init__(self, hs: "HomeServer"): + self.room_member_handler = hs.get_room_member_handler() + + async def resolve_room_id( + self, room_identifier: str, remote_room_hosts: Optional[List[str]] = None + ) -> Tuple[str, Optional[List[str]]]: + """ + Resolve a room identifier to a room ID, if necessary. + + This also performanes checks to ensure the room ID is of the proper form. + + Args: + room_identifier: The room ID or alias. + remote_room_hosts: The potential remote room hosts to use. + + Returns: + The resolved room ID. + + Raises: + SynapseError if the room ID is of the wrong form. + """ + if RoomID.is_valid(room_identifier): + resolved_room_id = room_identifier + elif RoomAlias.is_valid(room_identifier): + room_alias = RoomAlias.from_string(room_identifier) + ( + room_id, + remote_room_hosts, + ) = await self.room_member_handler.lookup_room_alias(room_alias) + resolved_room_id = room_id.to_string() + else: + raise SynapseError( + 400, "%s was not legal room ID or room alias" % (room_identifier,) + ) + if not resolved_room_id: + raise SynapseError( + 400, "Unknown room ID or room alias %s" % room_identifier + ) + return resolved_room_id, remote_room_hosts + + class ShutdownRoomRestServlet(RestServlet): """Shuts down a room by removing all local users from the room and blocking all future invites and joins to the room. Any local aliases will be repointed @@ -334,14 +376,14 @@ async def on_GET( return 200, ret -class JoinRoomAliasServlet(RestServlet): +class JoinRoomAliasServlet(ResolveRoomIdMixin, RestServlet): PATTERNS = admin_patterns("/join/(?P[^/]*)") def __init__(self, hs: "HomeServer"): + super().__init__(hs) self.hs = hs self.auth = hs.get_auth() - self.room_member_handler = hs.get_room_member_handler() self.admin_handler = hs.get_admin_handler() self.state_handler = hs.get_state_handler() @@ -362,30 +404,23 @@ async def on_POST( if not await self.admin_handler.get_user(target_user): raise NotFoundError("User not found") - if RoomID.is_valid(room_identifier): - resolved_room_id = room_identifier - try: - remote_room_hosts = [ - x.decode("ascii") for x in request.args[b"server_name"] - ] # type: Optional[List[str]] - except Exception: - remote_room_hosts = None - elif RoomAlias.is_valid(room_identifier): - handler = self.room_member_handler - room_alias = RoomAlias.from_string(room_identifier) - room_id, remote_room_hosts = await handler.lookup_room_alias(room_alias) - resolved_room_id = room_id.to_string() - else: - raise SynapseError( - 400, "%s was not legal room ID or room alias" % (room_identifier,) - ) + # Get the room ID from the identifier. + try: + remote_room_hosts = [ + x.decode("ascii") for x in request.args[b"server_name"] + ] # type: Optional[List[str]] + except Exception: + remote_room_hosts = None + room_id, remote_room_hosts = await self.resolve_room_id( + room_identifier, remote_room_hosts + ) fake_requester = create_requester( target_user, authenticated_entity=requester.authenticated_entity ) # send invite if room has "JoinRules.INVITE" - room_state = await self.state_handler.get_current_state(resolved_room_id) + room_state = await self.state_handler.get_current_state(room_id) join_rules_event = room_state.get((EventTypes.JoinRules, "")) if join_rules_event: if not (join_rules_event.content.get("join_rule") == JoinRules.PUBLIC): @@ -395,7 +430,7 @@ async def on_POST( await self.room_member_handler.update_membership( requester=requester, target=fake_requester.user, - room_id=resolved_room_id, + room_id=room_id, action="invite", remote_room_hosts=remote_room_hosts, ratelimit=False, @@ -404,16 +439,16 @@ async def on_POST( await self.room_member_handler.update_membership( requester=fake_requester, target=fake_requester.user, - room_id=resolved_room_id, + room_id=room_id, action="join", remote_room_hosts=remote_room_hosts, ratelimit=False, ) - return 200, {"room_id": resolved_room_id} + return 200, {"room_id": room_id} -class MakeRoomAdminRestServlet(RestServlet): +class MakeRoomAdminRestServlet(ResolveRoomIdMixin, RestServlet): """Allows a server admin to get power in a room if a local user has power in a room. Will also invite the user if they're not in the room and it's a private room. Can specify another user (rather than the admin user) to be @@ -428,9 +463,9 @@ class MakeRoomAdminRestServlet(RestServlet): PATTERNS = admin_patterns("/rooms/(?P[^/]*)/make_room_admin") def __init__(self, hs: "HomeServer"): + super().__init__(hs) self.hs = hs self.auth = hs.get_auth() - self.room_member_handler = hs.get_room_member_handler() self.event_creation_handler = hs.get_event_creation_handler() self.state_handler = hs.get_state_handler() self.is_mine_id = hs.is_mine_id @@ -442,23 +477,13 @@ async def on_POST( await assert_user_is_admin(self.auth, requester.user) content = parse_json_object_from_request(request, allow_empty_body=True) - # Resolve to a room ID, if necessary. - if RoomID.is_valid(room_identifier): - resolved_room_id = room_identifier - elif RoomAlias.is_valid(room_identifier): - room_alias = RoomAlias.from_string(room_identifier) - room_id, _ = await self.room_member_handler.lookup_room_alias(room_alias) - resolved_room_id = room_id.to_string() - else: - raise SynapseError( - 400, "%s was not legal room ID or room alias" % (room_identifier,) - ) + room_id, _ = await self.resolve_room_id(room_identifier) # Which user to grant room admin rights to. user_to_add = content.get("user_id", requester.user.to_string()) # Figure out which local users currently have power in the room, if any. - room_state = await self.state_handler.get_current_state(resolved_room_id) + room_state = await self.state_handler.get_current_state(room_id) if not room_state: raise SynapseError(400, "Server not in room") @@ -519,7 +544,7 @@ async def on_POST( "sender": admin_user_id, "type": EventTypes.PowerLevels, "state_key": "", - "room_id": resolved_room_id, + "room_id": room_id, }, ) except AuthError: @@ -552,14 +577,14 @@ async def on_POST( await self.room_member_handler.update_membership( fake_requester, target=UserID.from_string(user_to_add), - room_id=resolved_room_id, + room_id=room_id, action=Membership.INVITE, ) return 200, {} -class ForwardExtremitiesRestServlet(RestServlet): +class ForwardExtremitiesRestServlet(ResolveRoomIdMixin, RestServlet): """Allows a server admin to get or clear forward extremities. Clearing does not require restarting the server. @@ -574,36 +599,18 @@ class ForwardExtremitiesRestServlet(RestServlet): PATTERNS = admin_patterns("/rooms/(?P[^/]*)/forward_extremities") def __init__(self, hs: "HomeServer"): + super().__init__(hs) self.hs = hs self.auth = hs.get_auth() - self.room_member_handler = hs.get_room_member_handler() self.store = hs.get_datastore() - async def resolve_room_id(self, room_identifier: str) -> str: - """Resolve to a room ID, if necessary.""" - if RoomID.is_valid(room_identifier): - resolved_room_id = room_identifier - elif RoomAlias.is_valid(room_identifier): - room_alias = RoomAlias.from_string(room_identifier) - room_id, _ = await self.room_member_handler.lookup_room_alias(room_alias) - resolved_room_id = room_id.to_string() - else: - raise SynapseError( - 400, "%s was not legal room ID or room alias" % (room_identifier,) - ) - if not resolved_room_id: - raise SynapseError( - 400, "Unknown room ID or room alias %s" % room_identifier - ) - return resolved_room_id - async def on_DELETE( self, request: SynapseRequest, room_identifier: str ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - room_id = await self.resolve_room_id(room_identifier) + room_id, _ = await self.resolve_room_id(room_identifier) deleted_count = await self.store.delete_forward_extremities_for_room(room_id) return 200, {"deleted": deleted_count} @@ -614,7 +621,7 @@ async def on_GET( requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - room_id = await self.resolve_room_id(room_identifier) + room_id, _ = await self.resolve_room_id(room_identifier) extremities = await self.store.get_forward_extremities_for_room(room_id) return 200, {"count": len(extremities), "results": extremities} From cb4dd2ae7cd08b65f0b8e9923c089c02bbd396cb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 26 Feb 2021 14:59:36 -0500 Subject: [PATCH 5/6] Newsfragment --- changelog.d/9506.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9506.bugfix diff --git a/changelog.d/9506.bugfix b/changelog.d/9506.bugfix new file mode 100644 index 000000000000..8ff9ae122e3e --- /dev/null +++ b/changelog.d/9506.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.25.0: `ValueError: Attempted to iterate a RoomID` when attempting to use `/_synapse/admin/join/` with a room alias. From f55400e238001fd4cf799b30d5955af86008661d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Mar 2021 07:20:47 -0500 Subject: [PATCH 6/6] Clarify changelog. Co-authored-by: Erik Johnston --- changelog.d/9506.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9506.bugfix b/changelog.d/9506.bugfix index 8ff9ae122e3e..cc0d410e0f9f 100644 --- a/changelog.d/9506.bugfix +++ b/changelog.d/9506.bugfix @@ -1 +1 @@ -Fix a bug introduced in v1.25.0: `ValueError: Attempted to iterate a RoomID` when attempting to use `/_synapse/admin/join/` with a room alias. +Fix a bug introduced in v1.25.0 where `/_synapse/admin/join/` would fail when given a room alias.