From dfb8bef58a35e6edeff3f3f427dfc151f1f42703 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 25 Jul 2023 11:12:14 +0100 Subject: [PATCH 1/4] Allow user_id to be optional for room deletion --- synapse/handlers/pagination.py | 10 ++++++++-- synapse/handlers/room.py | 5 ++++- .../callbacks/third_party_event_rules_callbacks.py | 9 +++++++-- synapse/storage/databases/main/room.py | 4 ++-- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 19b8728db9b9..269f860333d4 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -700,7 +700,7 @@ async def _shutdown_and_purge_room( self, delete_id: str, room_id: str, - requester_user_id: str, + requester_user_id: Optional[str], new_room_user_id: Optional[str] = None, new_room_name: Optional[str] = None, message: Optional[str] = None, @@ -719,6 +719,9 @@ async def _shutdown_and_purge_room( requester_user_id: User who requested the action. Will be recorded as putting the room on the blocking list. + If None, the action was not manually requested but instead + triggered automatically, e.g. through a Synapse module + or some other policy. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be @@ -803,7 +806,7 @@ def clear_delete() -> None: def start_shutdown_and_purge_room( self, room_id: str, - requester_user_id: str, + requester_user_id: Optional[str], new_room_user_id: Optional[str] = None, new_room_name: Optional[str] = None, message: Optional[str] = None, @@ -818,6 +821,9 @@ def start_shutdown_and_purge_room( requester_user_id: User who requested the action and put the room on the blocking list. + If None, the action was not manually requested but instead + triggered automatically, e.g. through a Synapse module + or some other policy. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 0513e28aabe1..ac9fd6ee6f3e 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1787,7 +1787,7 @@ def __init__(self, hs: "HomeServer"): async def shutdown_room( self, room_id: str, - requester_user_id: str, + requester_user_id: Optional[str], new_room_user_id: Optional[str] = None, new_room_name: Optional[str] = None, message: Optional[str] = None, @@ -1811,6 +1811,9 @@ async def shutdown_room( requester_user_id: User who requested the action and put the room on the blocking list. + If None, the action was not manually requested but instead + triggered automatically, e.g. through a Synapse module + or some other policy. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py index 911f37ba4253..2e849df5b90e 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py @@ -429,12 +429,17 @@ async def on_new_event(self, event_id: str) -> None: "Failed to run module API callback %s: %s", callback, e ) - async def check_can_shutdown_room(self, user_id: str, room_id: str) -> bool: + async def check_can_shutdown_room( + self, user_id: Optional[str], room_id: str + ) -> bool: """Intercept requests to shutdown a room. If `False` is returned, the room must not be shut down. Args: - requester: The ID of the user requesting the shutdown. + user_id: The ID of the user requesting the shutdown. + If no user ID is supplied, then the room is being shut down through + some mechanism other than a user's request, e.g. through a module's + request. room_id: The ID of the room. """ for callback in self._check_can_shutdown_room_callbacks: diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 830658f328b4..f7a372746946 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -2373,7 +2373,7 @@ async def add_event_report( ) return next_id - async def block_room(self, room_id: str, user_id: str) -> None: + async def block_room(self, room_id: str, user_id: Optional[str]) -> None: """Marks the room as blocked. Can be called multiple times (though we'll only track the last user to @@ -2389,7 +2389,7 @@ async def block_room(self, room_id: str, user_id: str) -> None: table="blocked_rooms", keyvalues={"room_id": room_id}, values={}, - insertion_values={"user_id": user_id}, + insertion_values={"user_id": user_id or ""}, desc="block_room", ) await self.db_pool.runInteraction( From f368862198c866ca1b061a0d2b8d8eb65f462daa Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 25 Jul 2023 11:12:27 +0100 Subject: [PATCH 2/4] Add module API method to delete a room --- synapse/module_api/__init__.py | 13 +++++++++++++ .../callbacks/third_party_event_rules_callbacks.py | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 95f780011153..4949e61bf363 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -1677,6 +1677,19 @@ async def create_room( room_alias_str = room_alias.to_string() if room_alias else None return room_id, room_alias_str + async def delete_room(self, room_id: str) -> None: + """ + Schedules the deletion of a room from Synapse's database. + + If the room is already being deleted, this method does nothing. + This method does not wait for the room to be deleted. + + Added in Synapse v1.89.0. + """ + # Future extensions to this method might want to e.g. allow use of `force_purge`. + # TODO In the future we should make sure this is persistent. + self._hs.get_pagination_handler().start_shutdown_and_purge_room(room_id, None) + async def set_displayname( self, user_id: UserID, diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py index 2e849df5b90e..ecaeef35118c 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py @@ -40,7 +40,7 @@ [str, StateMap[EventBase], str], Awaitable[bool] ] ON_NEW_EVENT_CALLBACK = Callable[[EventBase, StateMap[EventBase]], Awaitable] -CHECK_CAN_SHUTDOWN_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]] +CHECK_CAN_SHUTDOWN_ROOM_CALLBACK = Callable[[Optional[str], str], Awaitable[bool]] CHECK_CAN_DEACTIVATE_USER_CALLBACK = Callable[[str, bool], Awaitable[bool]] ON_PROFILE_UPDATE_CALLBACK = Callable[[str, ProfileInfo, bool, bool], Awaitable] ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK = Callable[[str, bool, bool], Awaitable] From c1c84abc9c31c7d0558efd19e8b6bb9dee425218 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 25 Jul 2023 11:13:48 +0100 Subject: [PATCH 3/4] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/15997.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15997.misc diff --git a/changelog.d/15997.misc b/changelog.d/15997.misc new file mode 100644 index 000000000000..94768c3cb82d --- /dev/null +++ b/changelog.d/15997.misc @@ -0,0 +1 @@ +Allow modules to delete rooms. \ No newline at end of file From fd69d40143f80b000c564fb5498b2e421671f78b Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Fri, 11 Aug 2023 15:43:48 +0100 Subject: [PATCH 4/4] Don't worry about the case block=True && requester_user_id is None --- synapse/handlers/pagination.py | 2 ++ synapse/handlers/room.py | 5 +++++ synapse/storage/databases/main/room.py | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 8914c5a4e2ee..1ffa45ed36d7 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -735,6 +735,7 @@ async def _shutdown_and_purge_room( If None, the action was not manually requested but instead triggered automatically, e.g. through a Synapse module or some other policy. + MUST NOT be None if block=True. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be @@ -839,6 +840,7 @@ def start_shutdown_and_purge_room( If None, the action was not manually requested but instead triggered automatically, e.g. through a Synapse module or some other policy. + MUST NOT be None if block=True. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index ac9fd6ee6f3e..7a762c851101 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1814,6 +1814,7 @@ async def shutdown_room( If None, the action was not manually requested but instead triggered automatically, e.g. through a Synapse module or some other policy. + MUST NOT be None if block=True. new_room_user_id: If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be @@ -1866,6 +1867,10 @@ async def shutdown_room( # Action the block first (even if the room doesn't exist yet) if block: + if requester_user_id is None: + raise ValueError( + "shutdown_room: block=True not allowed when requester_user_id is None." + ) # This will work even if the room is already blocked, but that is # desirable in case the first attempt at blocking the room failed below. await self.store.block_room(room_id, requester_user_id) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index d6d5a35d40b2..719e11aea61d 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -2373,7 +2373,7 @@ async def add_event_report( ) return next_id - async def block_room(self, room_id: str, user_id: Optional[str]) -> None: + async def block_room(self, room_id: str, user_id: str) -> None: """Marks the room as blocked. Can be called multiple times (though we'll only track the last user to @@ -2389,7 +2389,7 @@ async def block_room(self, room_id: str, user_id: Optional[str]) -> None: table="blocked_rooms", keyvalues={"room_id": room_id}, values={}, - insertion_values={"user_id": user_id or ""}, + insertion_values={"user_id": user_id}, desc="block_room", ) await self.db_pool.runInteraction(