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

Remove deprecated user_may_create_room_with_invites callback #11950

Merged
merged 6 commits into from
Feb 11, 2022
Merged
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
1 change: 1 addition & 0 deletions changelog.d/11950.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove deprecated `user_may_create_room_with_invites` spam checker callback. See the [upgrade notes](https://matrix-org.github.io/synapse/latest/upgrade.html#removal-of-user_may_create_room_with_invites) for more information.
29 changes: 19 additions & 10 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,18 @@ process, for example:
wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
```
# Upgrading to v1.(next)

# Upgrading to v1.53.0

## Dropping support for `webclient` listeners and non-HTTP(S) `web_client_location`

Per the deprecation notice in Synapse v1.51.0, listeners of type `webclient`
are no longer supported and configuring them is a now a configuration error.

Configuring a non-HTTP(S) `web_client_location` configuration is is now a
configuration error. Since the `webclient` listener is no longer supported, this
setting only applies to the root path `/` of Synapse's web server and no longer
the `/_matrix/client/` path.

## Stablisation of MSC3231

Expand Down Expand Up @@ -119,17 +130,15 @@ The new `capabilities`

are now active by default.

# Upgrading to v1.53.0

## Dropping support for `webclient` listeners and non-HTTP(S) `web_client_location`
## Removal of `user_may_create_room_with_invites`

Per the deprecation notice in Synapse v1.51.0, listeners of type `webclient`
are no longer supported and configuring them is a now a configuration error.
As announced with the release of [Synapse 1.47.0](#deprecation-of-the-user_may_create_room_with_invites-module-callback),
the deprecated `user_may_create_room_with_invites` module callback has been removed.

Configuring a non-HTTP(S) `web_client_location` configuration is is now a
configuration error. Since the `webclient` listener is no longer supported, this
setting only applies to the root path `/` of Synapse's web server and no longer
the `/_matrix/client/` path.
Modules relying on it can instead implement [`user_may_invite`](https://matrix-org.github.io/synapse/latest/modules/spam_checker_callbacks.html#user_may_invite)
and use the [`get_room_state`](https://github.com/matrix-org/synapse/blob/872f23b95fa980a61b0866c1475e84491991fa20/synapse/module_api/__init__.py#L869-L876)
module API to infer whether the invite is happening while creating a room (see [this function](https://github.com/matrix-org/synapse-domain-rule-checker/blob/e7d092dd9f2a7f844928771dbfd9fd24c2332e48/synapse_domain_rule_checker/__init__.py#L56-L89)
as an example). Alternately, modules can also implement [`on_create_room`](https://matrix-org.github.io/synapse/latest/modules/third_party_rules_callbacks.html#on_create_room).


# Upgrading to v1.52.0
Expand Down
42 changes: 0 additions & 42 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@
USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
USER_MAY_SEND_3PID_INVITE_CALLBACK = Callable[[str, str, str, str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
[str, List[str], List[Dict[str, str]]], Awaitable[bool]
]
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], Awaitable[bool]]
USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]]
CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], Awaitable[bool]]
Expand Down Expand Up @@ -174,9 +171,6 @@ def __init__(self) -> 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_invites_callbacks: List[
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = []
self._user_may_create_room_alias_callbacks: List[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = []
Expand All @@ -198,9 +192,6 @@ def register_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_invites: Optional[
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = None,
user_may_create_room_alias: Optional[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = None,
Expand Down Expand Up @@ -229,11 +220,6 @@ 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_invites is not None:
self._user_may_create_room_with_invites_callbacks.append(
user_may_create_room_with_invites,
)

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 @@ -359,34 +345,6 @@ async def user_may_create_room(self, userid: str) -> bool:

return True

async def user_may_create_room_with_invites(
self,
userid: str,
invites: List[str],
threepid_invites: List[Dict[str, str]],
) -> bool:
"""Checks if a given user may create a room with invites

If this method returns false, the creation request will be rejected.

Args:
userid: The ID of the user attempting to create a room
invites: The IDs of the Matrix users to be invited if the room creation is
allowed.
threepid_invites: The threepids to be invited if the room creation is allowed,
as a dict including a "medium" key indicating the threepid's medium (e.g.
"email") and an "address" key indicating the threepid's address (e.g.
"alice@example.com")

Returns:
True if the user may create the room, otherwise False
"""
for callback in self._user_may_create_room_with_invites_callbacks:
if await callback(userid, invites, threepid_invites) is False:
return False

return True

async def user_may_create_room_alias(
self, userid: str, room_alias: RoomAlias
) -> bool:
Expand Down
5 changes: 0 additions & 5 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,6 @@ async def create_room(

if not is_requester_admin and not (
await self.spam_checker.user_may_create_room(user_id)
and await self.spam_checker.user_may_create_room_with_invites(
user_id,
invite_list,
invite_3pid_list,
)
):
raise SynapseError(
403, "You are not permitted to create rooms", Codes.FORBIDDEN
Expand Down
5 changes: 0 additions & 5 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
CHECK_USERNAME_FOR_SPAM_CALLBACK,
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK,
USER_MAY_CREATE_ROOM_CALLBACK,
USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK,
USER_MAY_INVITE_CALLBACK,
USER_MAY_JOIN_ROOM_CALLBACK,
USER_MAY_PUBLISH_ROOM_CALLBACK,
Expand Down Expand Up @@ -217,9 +216,6 @@ 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_invites: Optional[
Copy link
Member

Choose a reason for hiding this comment

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

Should we make the parameters to register_spam_checker_callbacks keyword only via the * flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, I'd suggest opening a new issue for this.

Copy link
Member

Choose a reason for hiding this comment

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

I put up a PR at #11975

USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
] = None,
user_may_create_room_alias: Optional[
USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
] = None,
Expand All @@ -240,7 +236,6 @@ def register_spam_checker_callbacks(
user_may_invite=user_may_invite,
user_may_send_3pid_invite=user_may_send_3pid_invite,
user_may_create_room=user_may_create_room,
user_may_create_room_with_invites=user_may_create_room_with_invites,
user_may_create_room_alias=user_may_create_room_alias,
user_may_publish_room=user_may_publish_room,
check_username_for_spam=check_username_for_spam,
Expand Down
119 changes: 2 additions & 117 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"""Tests REST events for /rooms paths."""

import json
from typing import Dict, Iterable, List, Optional
from typing import Iterable, List
from unittest.mock import Mock, call
from urllib import parse as urlparse

Expand All @@ -35,7 +35,7 @@
from synapse.handlers.pagination import PurgeStatus
from synapse.rest import admin
from synapse.rest.client import account, directory, login, profile, room, sync
from synapse.types import JsonDict, Requester, RoomAlias, UserID, create_requester
from synapse.types import JsonDict, RoomAlias, UserID, create_requester
from synapse.util.stringutils import random_string

from tests import unittest
Expand Down Expand Up @@ -674,121 +674,6 @@ def test_post_room_invitees_ratelimit(self):
channel = self.make_request("POST", "/createRoom", content)
self.assertEqual(200, channel.code)

def test_spamchecker_invites(self):
"""Tests the user_may_create_room_with_invites spam checker callback."""

# Mock do_3pid_invite, so we don't fail from failing to send a 3PID invite to an
# IS.
async def do_3pid_invite(
room_id: str,
inviter: UserID,
medium: str,
address: str,
id_server: str,
requester: Requester,
txn_id: Optional[str],
id_access_token: Optional[str] = None,
) -> int:
return 0

do_3pid_invite_mock = Mock(side_effect=do_3pid_invite)
self.hs.get_room_member_handler().do_3pid_invite = do_3pid_invite_mock

# Add a mock callback for user_may_create_room_with_invites. Make it allow any
# room creation request for now.
return_value = True

async def user_may_create_room_with_invites(
user: str,
invites: List[str],
threepid_invites: List[Dict[str, str]],
) -> bool:
return return_value

callback_mock = Mock(side_effect=user_may_create_room_with_invites)
self.hs.get_spam_checker()._user_may_create_room_with_invites_callbacks.append(
callback_mock,
)

# The MXIDs we'll try to invite.
invited_mxids = [
"@alice1:red",
"@alice2:red",
"@alice3:red",
"@alice4:red",
]

# The 3PIDs we'll try to invite.
invited_3pids = [
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice1@example.com",
},
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice2@example.com",
},
{
"id_server": "example.com",
"id_access_token": "sometoken",
"medium": "email",
"address": "alice3@example.com",
},
]

# Create a room and invite the Matrix users, and check that it succeeded.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite": invited_mxids}).encode("utf8"),
)
self.assertEqual(200, channel.code)

# Check that the callback was called with the right arguments.
expected_call_args = ((self.user_id, invited_mxids, []),)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Create a room and invite the 3PIDs, and check that it succeeded.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
)
self.assertEqual(200, channel.code)

# Check that do_3pid_invite was called the right amount of time
self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))

# Check that the callback was called with the right arguments.
expected_call_args = ((self.user_id, [], invited_3pids),)
self.assertEquals(
callback_mock.call_args,
expected_call_args,
callback_mock.call_args,
)

# Now deny any room creation.
return_value = False

# Create a room and invite the 3PIDs, and check that it failed.
channel = self.make_request(
"POST",
"/createRoom",
json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
)
self.assertEqual(403, channel.code)

# Check that do_3pid_invite wasn't called this time.
self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))

def test_spam_checker_may_join_room(self):
"""Tests that the user_may_join_room spam checker callback is correctly bypassed
when creating a new room.
Expand Down