-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Remove outlier check for retrieving membership events #15889
Remove outlier check for retrieving membership events #15889
Conversation
Signed-off-by: Jorge Martín Espinosa <jorgem@element.io>
@@ -207,15 +207,12 @@ async def filter_event_for_clients_with_state( | |||
): | |||
allowed_user_ids.discard(user_id) | |||
|
|||
if event.internal_metadata.outlier: | |||
# Normally these can't be seen by clients, but we make an exception for | |||
# for out-of-band membership events (eg, incoming invites, or rejections of |
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.
For reference, out-of-band membership events has some docs written here: docs/development/room-dag-concepts.md#out-of-band-membership-events
We might want to better clarify it once we figure out what we want/can do. If we need a new label for the "membership events for the user themselves" we should also figure that out
@@ -0,0 +1 @@ | |||
Remove outlier check for getting membership events through the `/_matrix/client/r0/rooms/{roomId}/event/{eventId}` endpoint. |
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.
We should probably create a spec issue to track clarifying the out-of-band membership behavior, https://github.com/matrix-org/matrix-spec/issues
@@ -0,0 +1 @@ | |||
Remove outlier check for getting membership events through the `/_matrix/client/r0/rooms/{roomId}/event/{eventId}` endpoint. |
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.
This PR probably needs more eyes on whether we want to do this so I've put it back in the review queue
Co-authored-by: Eric Eastwood <madlittlemods@gmail.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'm failing to see how this fixes the bug -- is this saying that a user should always be allowed to see their own membership event where previously we would only check this if it was an outlier?
I'm guessing it might not be an outlier if there are other users in the room, but the user in question hasn't joined yet?
I'm really not sure this matches what the spec says:
Likewise, for the user’s own
m.room.member
events, the user should be allowed to see the event if their membership before or after the event would allow them to see it. (For example, a user can always seem.room.member
events which set their membership tojoin
, or which change their membership fromjoin
to any other value, even ifhistory_visibility
isjoined
.)
@jmartinesp Is this still wanted? There's a few pending questions above to answer! |
Actually, I don't think this is needed anymore. I'll close it now. |
Pull Request Checklist
EventStore
toEventWorkerStore
.".code blocks
.(run the linters)
Changes
Removed the check for the membership events for the current user being outliers when retrieving them from the
/_matrix/client/r0/rooms/{roomId}/event/{eventId}
endpoint.Why
Fixes #15246.
This is specifically being spurred on to support Element X notifications for invites (element-hq/element-meta#1094, matrix-org/matrix-rust-sdk#2179)
It seems like the exception done in that check could have no reason to be applied only to out-of-band membership events (
outlier
), according to this comment.