Merge pull request #8535 from matrix-org/rav/third_party_events_updates

Support modifying event content from ThirdPartyRules modules
This commit is contained in:
Richard van der Hoff 2020-10-15 20:55:41 +01:00 committed by GitHub
commit 5649669c3c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 118 additions and 81 deletions

1
changelog.d/8535.feature Normal file
View file

@ -0,0 +1 @@
Support modifying event content in `ThirdPartyRules` modules.

View file

@ -312,6 +312,12 @@ class EventBase(metaclass=abc.ABCMeta):
""" """
return [e for e, _ in self.auth_events] return [e for e, _ in self.auth_events]
def freeze(self):
"""'Freeze' the event dict, so it cannot be modified by accident"""
# this will be a no-op if the event dict is already frozen.
self._dict = freeze(self._dict)
class FrozenEvent(EventBase): class FrozenEvent(EventBase):
format_version = EventFormatVersions.V1 # All events of this type are V1 format_version = EventFormatVersions.V1 # All events of this type are V1

View file

@ -12,7 +12,8 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Callable
from typing import Callable, Union
from synapse.events import EventBase from synapse.events import EventBase
from synapse.events.snapshot import EventContext from synapse.events.snapshot import EventContext
@ -44,15 +45,20 @@ class ThirdPartyEventRules:
async def check_event_allowed( async def check_event_allowed(
self, event: EventBase, context: EventContext self, event: EventBase, context: EventContext
) -> bool: ) -> Union[bool, dict]:
"""Check if a provided event should be allowed in the given context. """Check if a provided event should be allowed in the given context.
The module can return:
* True: the event is allowed.
* False: the event is not allowed, and should be rejected with M_FORBIDDEN.
* a dict: replacement event data.
Args: Args:
event: The event to be checked. event: The event to be checked.
context: The context of the event. context: The context of the event.
Returns: Returns:
True if the event should be allowed, False if not. The result from the ThirdPartyRules module, as above
""" """
if self.third_party_rules is None: if self.third_party_rules is None:
return True return True
@ -63,9 +69,10 @@ class ThirdPartyEventRules:
events = await self.store.get_events(prev_state_ids.values()) events = await self.store.get_events(prev_state_ids.values())
state_events = {(ev.type, ev.state_key): ev for ev in events.values()} state_events = {(ev.type, ev.state_key): ev for ev in events.values()}
# The module can modify the event slightly if it wants, but caution should be # Ensure that the event is frozen, to make sure that the module is not tempted
# exercised, and it's likely to go very wrong if applied to events received over # to try to modify it. Any attempt to modify it at this point will invalidate
# federation. # the hashes and signatures.
event.freeze()
return await self.third_party_rules.check_event_allowed(event, state_events) return await self.third_party_rules.check_event_allowed(event, state_events)

View file

@ -1507,18 +1507,9 @@ class FederationHandler(BaseHandler):
event, context = await self.event_creation_handler.create_new_client_event( event, context = await self.event_creation_handler.create_new_client_event(
builder=builder builder=builder
) )
except AuthError as e: except SynapseError as e:
logger.warning("Failed to create join to %s because %s", room_id, e) logger.warning("Failed to create join to %s because %s", room_id, e)
raise e raise
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.info("Creation of join %s forbidden by third-party rules", event)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
# The remote hasn't signed it yet, obviously. We'll do the full checks # The remote hasn't signed it yet, obviously. We'll do the full checks
# when we get the event back in `on_send_join_request` # when we get the event back in `on_send_join_request`
@ -1567,15 +1558,6 @@ class FederationHandler(BaseHandler):
context = await self._handle_new_event(origin, event) context = await self._handle_new_event(origin, event)
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.info("Sending of join %s forbidden by third-party rules", event)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
logger.debug( logger.debug(
"on_send_join_request: After _handle_new_event: %s, sigs: %s", "on_send_join_request: After _handle_new_event: %s, sigs: %s",
event.event_id, event.event_id,
@ -1748,15 +1730,6 @@ class FederationHandler(BaseHandler):
builder=builder builder=builder
) )
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.warning("Creation of leave %s forbidden by third-party rules", event)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
try: try:
# The remote hasn't signed it yet, obviously. We'll do the full checks # The remote hasn't signed it yet, obviously. We'll do the full checks
# when we get the event back in `on_send_leave_request` # when we get the event back in `on_send_leave_request`
@ -1789,16 +1762,7 @@ class FederationHandler(BaseHandler):
event.internal_metadata.outlier = False event.internal_metadata.outlier = False
context = await self._handle_new_event(origin, event) await self._handle_new_event(origin, event)
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.info("Sending of leave %s forbidden by third-party rules", event)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
logger.debug( logger.debug(
"on_send_leave_request: After _handle_new_event: %s, sigs: %s", "on_send_leave_request: After _handle_new_event: %s, sigs: %s",
@ -2694,18 +2658,6 @@ class FederationHandler(BaseHandler):
builder=builder builder=builder
) )
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.info(
"Creation of threepid invite %s forbidden by third-party rules",
event,
)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
event, context = await self.add_display_name_to_third_party_invite( event, context = await self.add_display_name_to_third_party_invite(
room_version, event_dict, event, context room_version, event_dict, event, context
) )
@ -2756,18 +2708,6 @@ class FederationHandler(BaseHandler):
event, context = await self.event_creation_handler.create_new_client_event( event, context = await self.event_creation_handler.create_new_client_event(
builder=builder builder=builder
) )
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.warning(
"Exchange of threepid invite %s forbidden by third-party rules", event
)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
event, context = await self.add_display_name_to_third_party_invite( event, context = await self.add_display_name_to_third_party_invite(
room_version, event_dict, event, context room_version, event_dict, event, context
) )

View file

@ -811,6 +811,23 @@ class EventCreationHandler:
if requester: if requester:
context.app_service = requester.app_service context.app_service = requester.app_service
third_party_result = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not third_party_result:
logger.info(
"Event %s forbidden by third-party rules", event,
)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
elif isinstance(third_party_result, dict):
# the third-party rules want to replace the event. We'll need to build a new
# event.
event, context = await self._rebuild_event_after_third_party_rules(
third_party_result, event
)
self.validator.validate_new(event, self.config) self.validator.validate_new(event, self.config)
# If this event is an annotation then we check that that the sender # If this event is an annotation then we check that that the sender
@ -897,14 +914,6 @@ class EventCreationHandler:
else: else:
room_version = await self.store.get_room_version_id(event.room_id) room_version = await self.store.get_room_version_id(event.room_id)
event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
if event.internal_metadata.is_out_of_band_membership(): if event.internal_metadata.is_out_of_band_membership():
# the only sort of out-of-band-membership events we expect to see here # the only sort of out-of-band-membership events we expect to see here
# are invite rejections we have generated ourselves. # are invite rejections we have generated ourselves.
@ -1307,3 +1316,57 @@ class EventCreationHandler:
room_id, room_id,
) )
del self._rooms_to_exclude_from_dummy_event_insertion[room_id] del self._rooms_to_exclude_from_dummy_event_insertion[room_id]
async def _rebuild_event_after_third_party_rules(
self, third_party_result: dict, original_event: EventBase
) -> Tuple[EventBase, EventContext]:
# the third_party_event_rules want to replace the event.
# we do some basic checks, and then return the replacement event and context.
# Construct a new EventBuilder and validate it, which helps with the
# rest of these checks.
try:
builder = self.event_builder_factory.for_room_version(
original_event.room_version, third_party_result
)
self.validator.validate_builder(builder)
except SynapseError as e:
raise Exception(
"Third party rules module created an invalid event: " + e.msg,
)
immutable_fields = [
# changing the room is going to break things: we've already checked that the
# room exists, and are holding a concurrency limiter token for that room.
# Also, we might need to use a different room version.
"room_id",
# changing the type or state key might work, but we'd need to check that the
# calling functions aren't making assumptions about them.
"type",
"state_key",
]
for k in immutable_fields:
if getattr(builder, k, None) != original_event.get(k):
raise Exception(
"Third party rules module created an invalid event: "
"cannot change field " + k
)
# check that the new sender belongs to this HS
if not self.hs.is_mine_id(builder.sender):
raise Exception(
"Third party rules module created an invalid event: "
"invalid sender " + builder.sender
)
# copy over the original internal metadata
for k, v in original_event.internal_metadata.get_dict().items():
setattr(builder.internal_metadata, k, v)
event = await builder.build(prev_event_ids=original_event.prev_event_ids())
# we rebuild the event context, to be on the safe side. If nothing else,
# delta_ids might need an update.
context = await self.state.compute_event_context(event)
return event, context

View file

@ -114,16 +114,36 @@ class ThirdPartyRulesTestCase(unittest.HomeserverTestCase):
self.render(request) self.render(request)
self.assertEquals(channel.result["code"], b"403", channel.result) self.assertEquals(channel.result["code"], b"403", channel.result)
def test_modify_event(self): def test_cannot_modify_event(self):
"""Tests that the module can successfully tweak an event before it is persisted. """cannot accidentally modify an event before it is persisted"""
"""
# first patch the event checker so that it will modify the event # first patch the event checker so that it will try to modify the event
async def check(ev: EventBase, state): async def check(ev: EventBase, state):
ev.content = {"x": "y"} ev.content = {"x": "y"}
return True return True
current_rules_module().check_event_allowed = check current_rules_module().check_event_allowed = check
# now send the event
request, channel = self.make_request(
"PUT",
"/_matrix/client/r0/rooms/%s/send/modifyme/1" % self.room_id,
{"x": "x"},
access_token=self.tok,
)
self.render(request)
self.assertEqual(channel.result["code"], b"500", channel.result)
def test_modify_event(self):
"""The module can return a modified version of the event"""
# first patch the event checker so that it will modify the event
async def check(ev: EventBase, state):
d = ev.get_dict()
d["content"] = {"x": "y"}
return d
current_rules_module().check_event_allowed = check
# now send the event # now send the event
request, channel = self.make_request( request, channel = self.make_request(
"PUT", "PUT",