element-hq / element-hq/synapse

Rework how `ThirdPartyEventRules`'s `check_event_allowed` works for replacing events

Open
#10,406 0 comments 0 reactions 0 assignees View on GitHub
P3 T-Task
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#10406](https://github.com/matrix-org/synapse/issues/10406).

---

Currently, `check_event_allowed` can return either a boolean (indicating whether an event can be allowed) or a dict (indicating that the event can be allowed, but be replaced with specific data). If it returns a dict, its content is only considered if the event is local (it's ignored in the federation code).

Making this callback return replacement data for the event feels like the wrong way to allow modules to change the content of existing events, since the function was only intended for figuring out whether an event could be allowed onto the homeserver. It's also an issue because replacing the event's data means rebuilding it with brand new auth events, which might not be possible in some cases (e.g. rejecting a local invite).

It's also an issue since the event is frozen before calling the callback, so `get_dict` will return a dict but any nested dicts (e.g. `content`) will be `frozendict`s, which isn't easily modified by a module.

For context, this feature was initially patched onto this callback to allow for a customer to build a module that hits their internal API with the json of an event and receives back whether it should allow this event, deny it, or allow it but with a different content.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.