-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Make label-based filtering support edits #6374
Conversation
…ecent edit of the same event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before I dig any more deeply into this:
what is the permissions model here? it looks like I could generate an edit event which changes the labels on an event sent by someone else? (Edit: I'm going to take this to the MSC)
Edit edit: I've spent the last hour chasing MSCs and concluded that MSC1849 doesn't really discuss the permission model, but synapse does do some basic permissions checks. It does so by only checking for edits when it serves them to the client, and checking that the senders match. We'll need to do the equivalent here.
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think this approach is somewhat flawed.
# Make sure that the sender of the edit match the original event's sender, | ||
# otherwise ignore this edit. We do this here because _store_labels_txn and | ||
# the background update converge here. | ||
original_sender = self.simple_select_one_onecol_txn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if the update arrives before the original event?
This is the wrong approach for handling edits, in that we can't ensure that updates arrive in the right order, or that updates even arrive after the original event. A correct implementation would be calculating the list of labels when processing edits when the client requests events. |
Support the latest changes on MSC2326 wrt edits.
The summary is that, when an edit of an event with labels comes through (whether it changes the list of labels or not), we need to:
To do this, every time we process an edit, we:
Blocked on #6329 because I plan on iterating on its changes to add tests for this one.