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

Remove outlier check for retrieving membership events #15889

Closed
wants to merge 4 commits into from
Closed

Remove outlier check for retrieving membership events #15889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2896138
Remove outlier check for retrieving membership events
jmartinesp Jul 7, 2023
31a2157
Add changelog
jmartinesp Jul 7, 2023
dbb9f90
Rename changelog to point to PR, not issue number
jmartinesp Jul 7, 2023
00728a6
Keep exception for retrieving outlier events
jmartinesp Jul 7, 2023
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
1 change: 1 addition & 0 deletions changelog.d/15889.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove outlier check for getting membership events through the `/_matrix/client/r0/rooms/{roomId}/event/{eventId}` endpoint.
Copy link
Contributor

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

Copy link
Contributor

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

18 changes: 11 additions & 7 deletions synapse/visibility.py
View file
Open in desktop
Copy link
Member

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 see m.room.member events which set their membership to join, or which change their membership from join to any other value, even if history_visibility is joined.)

Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,18 @@ async def filter_event_for_clients_with_state(
):
allowed_user_ids.discard(user_id)

# Normally these can't be seen by clients, but we make an exception for
# for membership events (eg, incoming invites, or rejections of
# said invite) for the user themselves.
if event.type == EventTypes.Member and event.state_key in allowed_user_ids:
logger.debug("Returning membership event %s", event)
return {event.state_key}
jmartinesp marked this conversation as resolved.
Show resolved Hide resolved

# We don't want to show outlier events to the client because
# we haven't figured out where they fit into the DAG yet. The
# only exception is out-of-band membership events which is
# handled above.
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
Copy link
Contributor

@MadLittleMods MadLittleMods Jul 7, 2023 •
edited
Loading

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

# said invite) for the user themselves.
if event.type == EventTypes.Member and event.state_key in allowed_user_ids:
logger.debug("Returning out-of-band-membership event %s", event)
return {event.state_key}

return set()

# First we get just the history visibility in case its shared/world-readable
Expand Down
Loading