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

Commit

Permalink
Add third_party module callbacks to check if a user can delete a room…
Browse files Browse the repository at this point in the history
… and deactivate a user (#12028)

* Add check_can_deactivate_user

* Add check_can_shutdown_rooms

* Documentation

* callbacks, not functions

* Various suggested tweaks

* Add tests for test_check_can_shutdown_room and test_check_can_deactivate_user

* Update check_can_deactivate_user to not take a Requester

* Fix check_can_shutdown_room docs

* Renegade and use `by_admin` instead of `admin_user_id`

* fix lint

* Update docs/modules/third_party_rules_callbacks.md

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

* Update docs/modules/third_party_rules_callbacks.md

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

* Update docs/modules/third_party_rules_callbacks.md

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

* Update docs/modules/third_party_rules_callbacks.md

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>
  • Loading branch information
Half-Shot and babolivier authored Mar 9, 2022
1 parent 690cb4f commit 15382b1
Show file tree
Hide file tree
Showing 8 changed files with 254 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog.d/12028.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add third-party rules rules callbacks `check_can_shutdown_room` and `check_can_deactivate_user`.
43 changes: 43 additions & 0 deletions docs/modules/third_party_rules_callbacks.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,49 @@ deny an incoming event, see [`check_event_for_spam`](spam_checker_callbacks.md#c

If multiple modules implement this callback, Synapse runs them all in order.

### `check_can_shutdown_room`

_First introduced in Synapse v1.55.0_

```python
async def check_can_shutdown_room(
user_id: str, room_id: str,
) -> bool:
```

Called when an admin user requests the shutdown of a room. The module must return a
boolean indicating whether the shutdown can go through. If the callback returns `False`,
the shutdown will not proceed and the caller will see a `M_FORBIDDEN` error.

If multiple modules implement this callback, they will be considered in order. If a
callback returns `True`, Synapse falls through to the next one. The value of the first
callback that does not return `True` will be used. If this happens, Synapse will not call
any of the subsequent implementations of this callback.

### `check_can_deactivate_user`

_First introduced in Synapse v1.55.0_

```python
async def check_can_deactivate_user(
user_id: str, by_admin: bool,
) -> bool:
```

Called when the deactivation of a user is requested. User deactivation can be
performed by an admin or the user themselves, so developers are encouraged to check the
requester when implementing this callback. The module must return a
boolean indicating whether the deactivation can go through. If the callback returns `False`,
the deactivation will not proceed and the caller will see a `M_FORBIDDEN` error.

The module is passed two parameters, `user_id` which is the ID of the user being deactivated, and `by_admin` which is `True` if the request is made by a serve admin, and `False` otherwise.

If multiple modules implement this callback, they will be considered in order. If a
callback returns `True`, Synapse falls through to the next one. The value of the first
callback that does not return `True` will be used. If this happens, Synapse will not call
any of the subsequent implementations of this callback.


### `on_profile_update`

_First introduced in Synapse v1.54.0_
Expand Down
55 changes: 55 additions & 0 deletions synapse/events/third_party_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
[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_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]

Expand Down Expand Up @@ -157,6 +159,12 @@ def __init__(self, hs: "HomeServer"):
CHECK_VISIBILITY_CAN_BE_MODIFIED_CALLBACK
] = []
self._on_new_event_callbacks: List[ON_NEW_EVENT_CALLBACK] = []
self._check_can_shutdown_room_callbacks: List[
CHECK_CAN_SHUTDOWN_ROOM_CALLBACK
] = []
self._check_can_deactivate_user_callbacks: List[
CHECK_CAN_DEACTIVATE_USER_CALLBACK
] = []
self._on_profile_update_callbacks: List[ON_PROFILE_UPDATE_CALLBACK] = []
self._on_user_deactivation_status_changed_callbacks: List[
ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
Expand All @@ -173,6 +181,8 @@ def register_third_party_rules_callbacks(
CHECK_VISIBILITY_CAN_BE_MODIFIED_CALLBACK
] = None,
on_new_event: Optional[ON_NEW_EVENT_CALLBACK] = None,
check_can_shutdown_room: Optional[CHECK_CAN_SHUTDOWN_ROOM_CALLBACK] = None,
check_can_deactivate_user: Optional[CHECK_CAN_DEACTIVATE_USER_CALLBACK] = None,
on_profile_update: Optional[ON_PROFILE_UPDATE_CALLBACK] = None,
on_user_deactivation_status_changed: Optional[
ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
Expand All @@ -198,6 +208,11 @@ def register_third_party_rules_callbacks(
if on_new_event is not None:
self._on_new_event_callbacks.append(on_new_event)

if check_can_shutdown_room is not None:
self._check_can_shutdown_room_callbacks.append(check_can_shutdown_room)

if check_can_deactivate_user is not None:
self._check_can_deactivate_user_callbacks.append(check_can_deactivate_user)
if on_profile_update is not None:
self._on_profile_update_callbacks.append(on_profile_update)

Expand Down Expand Up @@ -369,6 +384,46 @@ 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:
"""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.
room_id: The ID of the room.
"""
for callback in self._check_can_shutdown_room_callbacks:
try:
if await callback(user_id, room_id) is False:
return False
except Exception as e:
logger.exception(
"Failed to run module API callback %s: %s", callback, e
)
return True

async def check_can_deactivate_user(
self,
user_id: str,
by_admin: bool,
) -> bool:
"""Intercept requests to deactivate a user. If `False` is returned, the
user should not be deactivated.
Args:
requester
user_id: The ID of the room.
"""
for callback in self._check_can_deactivate_user_callbacks:
try:
if await callback(user_id, by_admin) is False:
return False
except Exception as e:
logger.exception(
"Failed to run module API callback %s: %s", callback, e
)
return True

async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]:
"""Given a room ID, return the state events of that room.
Expand Down
12 changes: 11 additions & 1 deletion synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from synapse.api.errors import SynapseError
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.types import Requester, UserID, create_requester
from synapse.types import Codes, Requester, UserID, create_requester

if TYPE_CHECKING:
from synapse.server import HomeServer
Expand All @@ -42,6 +42,7 @@ def __init__(self, hs: "HomeServer"):

# Flag that indicates whether the process to part users from rooms is running
self._user_parter_running = False
self._third_party_rules = hs.get_third_party_event_rules()

# Start the user parter loop so it can resume parting users from rooms where
# it left off (if it has work left to do).
Expand Down Expand Up @@ -74,6 +75,15 @@ async def deactivate_account(
Returns:
True if identity server supports removing threepids, otherwise False.
"""

# Check if this user can be deactivated
if not await self._third_party_rules.check_can_deactivate_user(
user_id, by_admin
):
raise SynapseError(
403, "Deactivation of this user is forbidden", Codes.FORBIDDEN
)

# FIXME: Theoretically there is a race here wherein user resets
# password using threepid.

Expand Down
8 changes: 8 additions & 0 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,7 @@ def __init__(self, hs: "HomeServer"):
self.room_member_handler = hs.get_room_member_handler()
self._room_creation_handler = hs.get_room_creation_handler()
self._replication = hs.get_replication_data_handler()
self._third_party_rules = hs.get_third_party_event_rules()
self.event_creation_handler = hs.get_event_creation_handler()
self.store = hs.get_datastores().main

Expand Down Expand Up @@ -1548,6 +1549,13 @@ async def shutdown_room(
if not RoomID.is_valid(room_id):
raise SynapseError(400, "%s is not a legal room ID" % (room_id,))

if not await self._third_party_rules.check_can_shutdown_room(
requester_user_id, room_id
):
raise SynapseError(
403, "Shutdown of this room is forbidden", Codes.FORBIDDEN
)

# Action the block first (even if the room doesn't exist yet)
if block:
# This will work even if the room is already blocked, but that is
Expand Down
6 changes: 6 additions & 0 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
USER_MAY_SEND_3PID_INVITE_CALLBACK,
)
from synapse.events.third_party_rules import (
CHECK_CAN_DEACTIVATE_USER_CALLBACK,
CHECK_CAN_SHUTDOWN_ROOM_CALLBACK,
CHECK_EVENT_ALLOWED_CALLBACK,
CHECK_THREEPID_CAN_BE_INVITED_CALLBACK,
CHECK_VISIBILITY_CAN_BE_MODIFIED_CALLBACK,
Expand Down Expand Up @@ -283,6 +285,8 @@ def register_third_party_rules_callbacks(
CHECK_VISIBILITY_CAN_BE_MODIFIED_CALLBACK
] = None,
on_new_event: Optional[ON_NEW_EVENT_CALLBACK] = None,
check_can_shutdown_room: Optional[CHECK_CAN_SHUTDOWN_ROOM_CALLBACK] = None,
check_can_deactivate_user: Optional[CHECK_CAN_DEACTIVATE_USER_CALLBACK] = None,
on_profile_update: Optional[ON_PROFILE_UPDATE_CALLBACK] = None,
on_user_deactivation_status_changed: Optional[
ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
Expand All @@ -298,6 +302,8 @@ def register_third_party_rules_callbacks(
check_threepid_can_be_invited=check_threepid_can_be_invited,
check_visibility_can_be_modified=check_visibility_can_be_modified,
on_new_event=on_new_event,
check_can_shutdown_room=check_can_shutdown_room,
check_can_deactivate_user=check_can_deactivate_user,
on_profile_update=on_profile_update,
on_user_deactivation_status_changed=on_user_deactivation_status_changed,
)
Expand Down
9 changes: 9 additions & 0 deletions synapse/rest/admin/rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def __init__(self, hs: "HomeServer"):
self._auth = hs.get_auth()
self._store = hs.get_datastores().main
self._pagination_handler = hs.get_pagination_handler()
self._third_party_rules = hs.get_third_party_event_rules()

async def on_DELETE(
self, request: SynapseRequest, room_id: str
Expand Down Expand Up @@ -106,6 +107,14 @@ async def on_DELETE(
HTTPStatus.BAD_REQUEST, "%s is not a legal room ID" % (room_id,)
)

# Check this here, as otherwise we'll only fail after the background job has been started.
if not await self._third_party_rules.check_can_shutdown_room(
requester.user.to_string(), room_id
):
raise SynapseError(
403, "Shutdown of this room is forbidden", Codes.FORBIDDEN
)

delete_id = self._pagination_handler.start_shutdown_and_purge_room(
room_id=room_id,
new_room_user_id=content.get("new_room_user_id"),
Expand Down
121 changes: 121 additions & 0 deletions tests/rest/client/test_third_party_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,3 +775,124 @@ def test_on_user_deactivation_status_changed_admin(self) -> None:
self.assertEqual(args[0], user_id)
self.assertFalse(args[1])
self.assertTrue(args[2])

def test_check_can_deactivate_user(self) -> None:
"""Tests that the on_user_deactivation_status_changed module callback is called
correctly when processing a user's deactivation.
"""
# Register a mocked callback.
deactivation_mock = Mock(return_value=make_awaitable(False))
third_party_rules = self.hs.get_third_party_event_rules()
third_party_rules._check_can_deactivate_user_callbacks.append(
deactivation_mock,
)

# Register a user that we'll deactivate.
user_id = self.register_user("altan", "password")
tok = self.login("altan", "password")

# Deactivate that user.
channel = self.make_request(
"POST",
"/_matrix/client/v3/account/deactivate",
{
"auth": {
"type": LoginType.PASSWORD,
"password": "password",
"identifier": {
"type": "m.id.user",
"user": user_id,
},
},
"erase": True,
},
access_token=tok,
)

# Check that the deactivation was blocked
self.assertEqual(channel.code, 403, channel.json_body)

# Check that the mock was called once.
deactivation_mock.assert_called_once()
args = deactivation_mock.call_args[0]

# Check that the mock was called with the right user ID
self.assertEqual(args[0], user_id)

# Check that the request was not made by an admin
self.assertEqual(args[1], False)

def test_check_can_deactivate_user_admin(self) -> None:
"""Tests that the on_user_deactivation_status_changed module callback is called
correctly when processing a user's deactivation triggered by a server admin.
"""
# Register a mocked callback.
deactivation_mock = Mock(return_value=make_awaitable(False))
third_party_rules = self.hs.get_third_party_event_rules()
third_party_rules._check_can_deactivate_user_callbacks.append(
deactivation_mock,
)

# Register an admin user.
self.register_user("admin", "password", admin=True)
admin_tok = self.login("admin", "password")

# Register a user that we'll deactivate.
user_id = self.register_user("altan", "password")

# Deactivate the user.
channel = self.make_request(
"PUT",
"/_synapse/admin/v2/users/%s" % user_id,
{"deactivated": True},
access_token=admin_tok,
)

# Check that the deactivation was blocked
self.assertEqual(channel.code, 403, channel.json_body)

# Check that the mock was called once.
deactivation_mock.assert_called_once()
args = deactivation_mock.call_args[0]

# Check that the mock was called with the right user ID
self.assertEqual(args[0], user_id)

# Check that the mock was made by an admin
self.assertEqual(args[1], True)

def test_check_can_shutdown_room(self) -> None:
"""Tests that the check_can_shutdown_room module callback is called
correctly when processing an admin's shutdown room request.
"""
# Register a mocked callback.
shutdown_mock = Mock(return_value=make_awaitable(False))
third_party_rules = self.hs.get_third_party_event_rules()
third_party_rules._check_can_shutdown_room_callbacks.append(
shutdown_mock,
)

# Register an admin user.
admin_user_id = self.register_user("admin", "password", admin=True)
admin_tok = self.login("admin", "password")

# Shutdown the room.
channel = self.make_request(
"DELETE",
"/_synapse/admin/v2/rooms/%s" % self.room_id,
{},
access_token=admin_tok,
)

# Check that the shutdown was blocked
self.assertEqual(channel.code, 403, channel.json_body)

# Check that the mock was called once.
shutdown_mock.assert_called_once()
args = shutdown_mock.call_args[0]

# Check that the mock was called with the right user ID
self.assertEqual(args[0], admin_user_id)

# Check that the mock was called with the right room ID
self.assertEqual(args[1], self.room_id)

0 comments on commit 15382b1

Please sign in to comment.