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

Support modifying event content from ThirdPartyRules modules #8535

Merged
merged 5 commits into from
Oct 15, 2020
Merged

Support modifying event content from ThirdPartyRules modules #8535

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d59378d
Remove redundant calls to third_party_rules in `on_send_{join,leave}`
richvdh Oct 13, 2020
123711e
Move third_party_rules check to event creation time
richvdh Oct 13, 2020
617e8a4
Allow ThirdPartyRules modules to replace event content
richvdh Oct 13, 2020
898196f
guard against accidental modification
richvdh Oct 13, 2020
091e948
changelog
richvdh Oct 13, 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
66 changes: 3 additions & 63 deletions synapse/handlers/federation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1507,18 +1507,9 @@ async def on_make_join_request(
event, context = await self.event_creation_handler.create_new_client_event(
builder=builder
)
except AuthError as e:
except SynapseError as e:
Copy link
Member Author

@richvdh richvdh Oct 13, 2020

Choose a reason for hiding this comment

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

I'm changing this because create_new_client_event will now raise a SynapseError if the third_party_rules check blocks the event.

logger.warning("Failed to create join to %s because %s", room_id, e)
raise e

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
)
raise

# 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`
Expand Down Expand Up @@ -1567,15 +1558,6 @@ async def on_send_join_request(self, origin, pdu):

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(
"on_send_join_request: After _handle_new_event: %s, sigs: %s",
event.event_id,
Expand Down Expand Up @@ -1748,15 +1730,6 @@ async def on_make_leave_request(
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:
# 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`
Expand Down Expand Up @@ -1789,16 +1762,7 @@ async def on_send_leave_request(self, origin, pdu):

event.internal_metadata.outlier = False

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 leave %s forbidden by third-party rules", event)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)
await self._handle_new_event(origin, event)

logger.debug(
"on_send_leave_request: After _handle_new_event: %s, sigs: %s",
Expand Down Expand Up @@ -2694,18 +2658,6 @@ async def exchange_third_party_invite(
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
)

Comment on lines -2697 to -2708
Copy link
Member Author

@richvdh richvdh Oct 13, 2020

Choose a reason for hiding this comment

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

afaict, this was redundant, because we later call member_handler.send_membership_event, which then calls handle_new_client_event, which was also doing this check. But in any case, it's doubly redundant now.

event, context = await self.add_display_name_to_third_party_invite(
room_version, event_dict, event, context
)
Expand Down Expand Up @@ -2756,18 +2708,6 @@ async def on_exchange_third_party_invite_request(
event, context = await self.event_creation_handler.create_new_client_event(
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
)

Comment on lines -2759 to -2770
Copy link
Member Author

@richvdh richvdh Oct 13, 2020

Choose a reason for hiding this comment

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

(ditto)

event, context = await self.add_display_name_to_third_party_invite(
room_version, event_dict, event, context
)
Expand Down
19 changes: 11 additions & 8 deletions synapse/handlers/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,17 @@ async def create_new_client_event(
if requester:
context.app_service = requester.app_service

event_allowed = await self.third_party_event_rules.check_event_allowed(
event, context
)
if not event_allowed:
logger.info(
"Event %s forbidden by third-party rules", event,
)
raise SynapseError(
403, "This event is not allowed in this context", Codes.FORBIDDEN
)

self.validator.validate_new(event, self.config)

# If this event is an annotation then we check that that the sender
Expand Down Expand Up @@ -881,14 +892,6 @@ async def handle_new_client_event(
else:
room_version = await self.store.get_room_version_id(event.room_id)

event_allowed = await self.third_party_event_rules.check_event_allowed(
Copy link
Member Author

@richvdh richvdh Oct 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this call path is somewhat less obvious than the others, since there are 6 callers of handle_new_client_event. They are:

  • EventCreationHandler._send_dummy_event_for_room
  • EventCreationHandler.create_and_send_nonmember_event
  • RoomCreationHandler._upgrade_room
  • RoomMemberHandler._local_membership_update
  • RoomMemberHandler.send_membership_event, which in turn is called by FederationHandler.exchange_third_party_invite and FederationHandler.on_exchange_third_party_invite_request

All the above create the event by calling EventCreationHandler.create_event, and thence create_new_client_event.

  • RoomMemberMasterHandler._locally_reject_invite

This one is brought in line by #8537.

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():
# the only sort of out-of-band-membership events we expect to see here
# are invite rejections we have generated ourselves.
Expand Down