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

Allow ThirdPartyEventRules modules to manipulate public room state #8292

Merged
merged 23 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
68391e4
Add a store method for checking whether a room is in the public dir
anoadragon453 Sep 10, 2020
0503eee
Allow ThirdPartyEventRules modules to block publishing a room
anoadragon453 Sep 10, 2020
9eae87d
Query ThirdPartyEventRules about public rooms during room creation
anoadragon453 Sep 10, 2020
833883e
Query ThirdPartyEventRules while trying to add to the public room dir
anoadragon453 Sep 10, 2020
8690c86
Add tests
anoadragon453 Sep 11, 2020
6eae2c1
Changelog
anoadragon453 Sep 11, 2020
431e3e3
Replace verbose type with StateMap
anoadragon453 Sep 11, 2020
529863a
Remove get_room_is_public and use get_room instead
anoadragon453 Sep 11, 2020
699fa5b
Address review comments
anoadragon453 Sep 11, 2020
f5b6246
Axe the PublicRoomsManager and switch to passing the ModuleApi
anoadragon453 Sep 11, 2020
e768fae
Move PublicRoomsManager tests to ModuleApi
anoadragon453 Sep 11, 2020
04ab21c
Create a PublicRoomListManager as part of ModuleApi
anoadragon453 Sep 14, 2020
06deecf
Backwards incompatibility warning for ThirdPartyEventRules in UPGRADE…
anoadragon453 Sep 14, 2020
6ed082d
Some minor fixes
anoadragon453 Sep 14, 2020
83a20a2
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/thi…
anoadragon453 Sep 14, 2020
0f4c1bb
Update unit tests
anoadragon453 Sep 14, 2020
3bde820
Fix call to old method name
anoadragon453 Sep 14, 2020
8d9b166
Update UPGRADE.rst, remove extra entry, convert to rst
anoadragon453 Sep 28, 2020
1ff7df9
Move PublicRoomListManager below ModuleApi
anoadragon453 Sep 28, 2020
e5b2d20
Remove hs param from the PublicRoomListManager docstring
anoadragon453 Sep 28, 2020
5730d17
Expose http_client and public_room_list_manager as properties
anoadragon453 Sep 28, 2020
d46396d
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/thi…
anoadragon453 Oct 5, 2020
49e71a8
Move upgrade notes comment from v1.21.0 to v1.22.0
anoadragon453 Oct 5, 2020
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
24 changes: 24 additions & 0 deletions UPGRADE.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
Upgrading to v1.20.0
====================

Shared rooms endpoint (MSC2666)
-------------------------------

This release contains a new unstable endpoint `/_matrix/client/unstable/uk.half-shot.msc2666/user/shared_rooms/.*`
for fetching rooms one user has in common with another. This feature requires the
`update_user_directory` config flag to be `True`. If you are you are using a `synapse.app.user_dir`
worker, requests to this endpoint must be handled by that worker.
See `docs/workers.md <docs/workers.md>`_ for more details.


anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
Upgrading Synapse
=================

Expand Down Expand Up @@ -128,6 +141,17 @@ template. These templates are similar, but the parameters are slightly different
* A string ``error`` parameter is available that includes a short hint of why a
user is seeing the error page.

ThirdPartyEventRules breaking changes
-------------------------------------

This release introduces a backwards-incompatible change to modules making use of
`ThirdPartyEventRules` in Synapse.

The `http_client` argument is no longer passed to modules as they are initialised. Instead,
modules are expected to make use of the `http_client` property on the `ModuleApi` class.
Modules are now passed a `module_api` argument during initialisation, which is an instance of
`ModuleApi`.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

Upgrading to v1.18.0
====================

Expand Down
1 change: 1 addition & 0 deletions changelog.d/8292.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow `ThirdPartyEventRules` modules to query and manipulate whether a room is in the public rooms directory.
51 changes: 45 additions & 6 deletions synapse/events/third_party_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Callable

from synapse.events import EventBase
from synapse.events.snapshot import EventContext
from synapse.types import Requester
from synapse.module_api import ModuleApi
from synapse.types import Requester, StateMap


class ThirdPartyEventRules:
Expand All @@ -38,7 +40,7 @@ def __init__(self, hs):

if module is not None:
self.third_party_rules = module(
config=config, http_client=hs.get_simple_http_client()
config=config, module_api=ModuleApi(hs, hs.get_auth_handler()),
richvdh marked this conversation as resolved.
Show resolved Hide resolved
)

async def check_event_allowed(
Expand Down Expand Up @@ -106,14 +108,51 @@ async def check_threepid_can_be_invited(
if self.third_party_rules is None:
return True

state_events = await self._get_state_map_for_room(room_id)

ret = await self.third_party_rules.check_threepid_can_be_invited(
medium, address, state_events
)
return ret

async def check_visibility_can_be_modified(
self, room_id: str, new_visibility: str
) -> bool:
"""Check if a room is allowed to be published to, or removed from, the public room
list.

Args:
room_id: The ID of the room.
new_visibility: The new visibility state. Either "public" or "private".

Returns:
True if the room's visibility can be modified, False if not.
"""
if self.third_party_rules is None:
return True

check_func = getattr(self.third_party_rules, "check_visibility_can_be_modified")
if not check_func or not isinstance(check_func, Callable):
return True

state_events = await self._get_state_map_for_room(room_id)

return await check_func(room_id, state_events, new_visibility)

async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]:
"""Given a room ID, return the state events of that room.

Args:
room_id: The ID of the room.

Returns:
A dict mapping (event type, state key) to state event.
"""
state_ids = await self.store.get_filtered_current_state_ids(room_id)
room_state_events = await self.store.get_events(state_ids.values())

state_events = {}
for key, event_id in state_ids.items():
state_events[key] = room_state_events[event_id]

ret = await self.third_party_rules.check_threepid_can_be_invited(
medium, address, state_events
)
return ret
return state_events
10 changes: 10 additions & 0 deletions synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def __init__(self, hs):
self.config = hs.config
self.enable_room_list_search = hs.config.enable_room_list_search
self.require_membership = hs.config.require_membership_for_aliases
self.third_party_event_rules = hs.get_third_party_event_rules()

self.federation = hs.get_federation_client()
hs.get_federation_registry().register_query_handler(
Expand Down Expand Up @@ -454,6 +455,15 @@ async def edit_published_room_list(
# per alias creation rule?
raise SynapseError(403, "Not allowed to publish room")

# Check if publishing is blocked by a third party module
allowed_by_third_party_rules = await (
self.third_party_event_rules.check_visibility_can_be_modified(
room_id, visibility
)
)
if not allowed_by_third_party_rules:
raise SynapseError(403, "Not allowed to publish room")

await self.store.set_room_is_public(room_id, making_public)

async def edit_published_appservice_room_list(
Expand Down
9 changes: 9 additions & 0 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,15 @@ async def create_room(
creator_id=user_id, is_public=is_public, room_version=room_version,
)

# Check whether this visibility value is blocked by a third party module
allowed_by_third_party_rules = await (
self.third_party_event_rules.check_visibility_can_be_modified(
richvdh marked this conversation as resolved.
Show resolved Hide resolved
room_id, visibility
)
)
if not allowed_by_third_party_rules:
raise SynapseError(403, "Room visibility value not allowed.")

directory_handler = self.hs.get_handlers().directory_handler
if room_alias:
await directory_handler.create_association(
Expand Down
52 changes: 52 additions & 0 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING

from twisted.internet import defer

from synapse.http.client import SimpleHttpClient
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.types import UserID

if TYPE_CHECKING:
from synapse.server import HomeServer

"""
This package defines the 'stable' API which can be used by extension modules which
are loaded into Synapse.
Expand All @@ -31,6 +36,50 @@
logger = logging.getLogger(__name__)


class PublicRoomListManager:
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
"""Contains methods for adding to, removing from and querying whether a room
is in the public room list.

Args:
hs: The Homeserver object
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
"""

def __init__(self, hs: "HomeServer"):
self._store = hs.get_datastore()

async def room_is_in_public_room_list(self, room_id: str) -> bool:
"""Checks whether a room is in the public room list.

Args:
room_id: The ID of the room.

Returns:
Whether the room is in the public room list. Returns False if the room does
not exist.
"""
room = await self._store.get_room(room_id)
if not room:
return False

return room.get("is_public", False)

async def add_room_to_public_room_list(self, room_id: str) -> None:
"""Publishes a room to the public room list.

Args:
room_id: The ID of the room.
"""
await self._store.set_room_is_public(room_id, True)

async def remove_room_from_public_room_list(self, room_id: str) -> None:
"""Removes a room from the public room list.

Args:
room_id: The ID of the room.
"""
await self._store.set_room_is_public(room_id, False)


class ModuleApi:
"""A proxy object that gets passed to various plugin modules so they
can register new users etc if necessary.
Expand All @@ -43,6 +92,9 @@ def __init__(self, hs, auth_handler):
self._auth = hs.get_auth()
self._auth_handler = auth_handler

self.http_client = hs.get_simple_http_client() # type: SimpleHttpClient
self.public_room_list_manager = PublicRoomListManager(hs)
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend exposing these as @property methods, so that they can have some docstring describing what they do

Copy link
Member Author

Choose a reason for hiding this comment

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

I have done so in 5730d17, though I'm not sure if the contents of the docstrings were entirely what you were looking for.

Copy link
Member

Choose a reason for hiding this comment

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

they look good to me


def get_user_by_req(self, req, allow_guest=False):
"""Check the access_token provided for a request

Expand Down
56 changes: 55 additions & 1 deletion tests/module_api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,20 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from synapse.module_api import ModuleApi
from synapse.rest import admin
from synapse.rest.client.v1 import login, room

from tests.unittest import HomeserverTestCase


class ModuleApiTestCase(HomeserverTestCase):
servlets = [
admin.register_servlets,
login.register_servlets,
room.register_servlets,
]

def prepare(self, reactor, clock, homeserver):
self.store = homeserver.get_datastore()
self.module_api = ModuleApi(homeserver, homeserver.get_auth_handler())
Expand Down Expand Up @@ -52,3 +59,50 @@ def test_can_register_user(self):
# Check that the displayname was assigned
displayname = self.get_success(self.store.get_profile_displayname("bob"))
self.assertEqual(displayname, "Bobberino")

def test_public_rooms(self):
"""Tests that a room can be added and removed from the public rooms list,
as well as have its public rooms directory state queried.
"""
# Create a user and room to play with
user_id = self.register_user("kermit", "monkey")
tok = self.login("kermit", "monkey")
room_id = self.helper.create_room_as(user_id, tok=tok)

# The room should not currently be in the public rooms directory
is_in_public_rooms = self.get_success(
self.module_api.public_room_list_manager.room_is_in_public_room_list(
room_id
)
)
self.assertFalse(is_in_public_rooms)

# Let's try adding it to the public rooms directory
self.get_success(
self.module_api.public_room_list_manager.add_room_to_public_room_list(
room_id
)
)

# And checking whether it's in there...
is_in_public_rooms = self.get_success(
self.module_api.public_room_list_manager.room_is_in_public_room_list(
room_id
)
)
self.assertTrue(is_in_public_rooms)

# Let's remove it again
self.get_success(
self.module_api.public_room_list_manager.remove_room_from_public_room_list(
room_id
)
)

# Should be gone
is_in_public_rooms = self.get_success(
self.module_api.public_room_list_manager.room_is_in_public_room_list(
room_id
)
)
self.assertFalse(is_in_public_rooms)
31 changes: 19 additions & 12 deletions tests/rest/client/third_party_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,23 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from synapse.rest import admin
from synapse.rest.client.v1 import login, room
from synapse.types import Requester

from tests import unittest


class ThirdPartyRulesTestModule:
def __init__(self, config):
def __init__(self, config, *args, **kwargs):
pass

def check_event_allowed(self, event, context):
async def on_create_room(
self, requester: Requester, config: dict, is_requester_admin: bool
):
return True

async def check_event_allowed(self, event, context):
if event.type == "foo.bar.forbidden":
return False
else:
Expand Down Expand Up @@ -51,29 +56,31 @@ def make_homeserver(self, reactor, clock):
self.hs = self.setup_test_homeserver(config=config)
return self.hs

def prepare(self, reactor, clock, homeserver):
# Create a user and room to play with during the tests
self.user_id = self.register_user("kermit", "monkey")
self.tok = self.login("kermit", "monkey")

self.room_id = self.helper.create_room_as(self.user_id, tok=self.tok)

def test_third_party_rules(self):
"""Tests that a forbidden event is forbidden from being sent, but an allowed one
can be sent.
"""
user_id = self.register_user("kermit", "monkey")
tok = self.login("kermit", "monkey")

room_id = self.helper.create_room_as(user_id, tok=tok)

request, channel = self.make_request(
"PUT",
"/_matrix/client/r0/rooms/%s/send/foo.bar.allowed/1" % room_id,
"/_matrix/client/r0/rooms/%s/send/foo.bar.allowed/1" % self.room_id,
{},
access_token=tok,
access_token=self.tok,
)
self.render(request)
self.assertEquals(channel.result["code"], b"200", channel.result)

request, channel = self.make_request(
"PUT",
"/_matrix/client/r0/rooms/%s/send/foo.bar.forbidden/1" % room_id,
"/_matrix/client/r0/rooms/%s/send/foo.bar.forbidden/1" % self.room_id,
{},
access_token=tok,
access_token=self.tok,
)
self.render(request)
self.assertEquals(channel.result["code"], b"403", channel.result)