Skip to content

Add user_may_create_room_with_visibility spamchecker module API callback #18455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions docs/modules/spam_checker_callbacks.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,27 @@ The value of the first callback that does not return `synapse.module_api.NOT_SPA
be used. If this happens, Synapse will not call any of the subsequent implementations of
this callback.

### `user_may_create_room_with_visibility`

_First introduced in Synapse vX.X.X_

```python
async def user_may_create_room_with_visibility(user_id: str, visibility: str) -> Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes"]
```

Called when processing a room creation request or updating join rules for an existing room.

The callback must return one of:
- `synapse.module_api.NOT_SPAM`, to allow the operation. Other callbacks may still
decide to reject it.
- `synapse.module_api.errors.Codes` to reject the operation with an error code. In case
of doubt, `synapse.module_api.errors.Codes.FORBIDDEN` is a good error code.

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


### `user_may_create_room_alias`
Expand Down
1 change: 1 addition & 0 deletions docs/spam_checker.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ well as some specific methods:

* `user_may_invite`
* `user_may_create_room`
* `user_may_create_room_with_visibility`
* `user_may_create_room_alias`
* `user_may_publish_room`
* `check_username_for_spam`
Expand Down
18 changes: 15 additions & 3 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,10 @@ async def create_room(
Codes.MISSING_PARAM,
)

# The spec says rooms should default to private visibility if
# `visibility` is not specified.
visibility = config.get("visibility", "private")

if not is_requester_admin:
spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
user_id
Expand All @@ -795,6 +799,17 @@ async def create_room(
errcode=spam_check[0],
additional_fields=spam_check[1],
)
spam_check = await self._spam_checker_module_callbacks.user_may_create_room_with_visibility(
user_id,
visibility,
)
if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
raise SynapseError(
403,
"You are not permitted to create rooms with visibility {visibility}",
errcode=spam_check[0],
additional_fields=spam_check[1],
)

if ratelimit:
# Rate limit once in advance, but don't rate limit the individual
Expand Down Expand Up @@ -887,9 +902,6 @@ async def create_room(
% (user_id,),
)

# The spec says rooms should default to private visibility if
# `visibility` is not specified.
visibility = config.get("visibility", "private")
is_public = visibility == "public"

self._validate_room_config(config, visibility)
Expand Down
5 changes: 5 additions & 0 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
SHOULD_DROP_FEDERATED_EVENT_CALLBACK,
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK,
USER_MAY_CREATE_ROOM_CALLBACK,
USER_MAY_CREATE_ROOM_WITH_VISIBILITY_CALLBACK,
USER_MAY_INVITE_CALLBACK,
USER_MAY_JOIN_ROOM_CALLBACK,
USER_MAY_PUBLISH_ROOM_CALLBACK,
Expand Down Expand Up @@ -307,6 +308,9 @@ def register_spam_checker_callbacks(
user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
user_may_send_3pid_invite: Optional[USER_MAY_SEND_3PID_INVITE_CALLBACK] = None,
user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
user_may_create_room_with_visibility: Optional[
USER_MAY_CREATE_ROOM_WITH_VISIBILITY_CALLBACK
] = None,
user_may_create_room_alias: Optional[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = None,
Expand Down Expand Up @@ -335,6 +339,7 @@ def register_spam_checker_callbacks(
check_registration_for_spam=check_registration_for_spam,
check_media_file_for_spam=check_media_file_for_spam,
check_login_for_spam=check_login_for_spam,
user_may_create_room_with_visibility=user_may_create_room_with_visibility,
)

def register_account_validity_callbacks(
Expand Down
43 changes: 43 additions & 0 deletions synapse/module_api/callbacks/spamchecker_callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@
]
],
]
USER_MAY_CREATE_ROOM_WITH_VISIBILITY_CALLBACK = Callable[
[str, str],
Awaitable[
Union[
Literal["NOT_SPAM"],
Codes,
]
],
]
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[
[str, RoomAlias],
Awaitable[
Expand Down Expand Up @@ -332,6 +341,9 @@ def __init__(self, hs: "synapse.server.HomeServer") -> None:
USER_MAY_SEND_3PID_INVITE_CALLBACK
] = []
self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = []
self._user_may_create_room_with_visibility_callbacks: List[
USER_MAY_CREATE_ROOM_WITH_VISIBILITY_CALLBACK
] = []
self._user_may_create_room_alias_callbacks: List[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = []
Expand Down Expand Up @@ -367,6 +379,9 @@ def register_callbacks(
] = None,
check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None,
check_login_for_spam: Optional[CHECK_LOGIN_FOR_SPAM_CALLBACK] = None,
user_may_create_room_with_visibility: Optional[
USER_MAY_CREATE_ROOM_WITH_VISIBILITY_CALLBACK
] = None,
) -> None:
"""Register callbacks from module for each hook."""
if check_event_for_spam is not None:
Expand All @@ -391,6 +406,11 @@ def register_callbacks(
if user_may_create_room is not None:
self._user_may_create_room_callbacks.append(user_may_create_room)

if user_may_create_room_with_visibility is not None:
self._user_may_create_room_with_visibility_callbacks.append(
user_may_create_room_with_visibility,
)

if user_may_create_room_alias is not None:
self._user_may_create_room_alias_callbacks.append(
user_may_create_room_alias,
Expand Down Expand Up @@ -653,6 +673,29 @@ async def user_may_create_room(

return self.NOT_SPAM

async def user_may_create_room_with_visibility(
self, userid: str, visibility: str
) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
"""Checks if a given user may create a room with a given visibility
Args:
userid: The ID of the user attempting to create a room
visibility: The visibility of the room to be created
"""
for callback in self._user_may_create_room_with_visibility_callbacks:
with Measure(self.clock, f"{callback.__module__}.{callback.__qualname__}"):
res = await delay_cancellation(callback(userid, visibility))
if res is self.NOT_SPAM:
continue
elif isinstance(res, synapse.api.errors.Codes):
return res, {}
else:
logger.warning(
"Module returned invalid value, rejecting room creation as spam"
)
return synapse.api.errors.Codes.FORBIDDEN, {}

return self.NOT_SPAM

async def user_may_create_room_alias(
self, userid: str, room_alias: RoomAlias
) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
Expand Down
15 changes: 15 additions & 0 deletions synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ def __init__(self, hs: "HomeServer"):
self.delayed_events_handler = hs.get_delayed_events_handler()
self.auth = hs.get_auth()
self._max_event_delay_ms = hs.config.server.max_event_delay_ms
self._spam_checker_module_callbacks = hs.get_module_api_callbacks().spam_checker

def register(self, http_server: HttpServer) -> None:
# /rooms/$roomid/state/$eventtype
Expand Down Expand Up @@ -289,6 +290,20 @@ async def on_PUT(

content = parse_json_object_from_request(request)

if event_type == EventTypes.JoinRules:
visibility = "public" # XXXTODO: determine visibility from content
spam_check = await self._spam_checker_module_callbacks.user_may_create_room_with_visibility(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be preferable from a module API point of view to separate room creation from changing visibility.

For changing visibility it might make sense to be:

user_may_change_visibility(user_id: str, room_id: str, old_visibility: str, new_visibility: str)

But, that might also be over complicating things.

requester.user.to_string(), visibility
)

if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
raise SynapseError(
403,
"You are not permitted to change room visibility to {visibility}",
errcode=spam_check[0],
additional_fields=spam_check[1],
)

origin_server_ts = None
if requester.app_service:
origin_server_ts = parse_integer(request, "ts")
Expand Down
Loading