From 558850de609b10ad66fd1a61998bdff21c706d19 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 12:01:45 -0500 Subject: [PATCH 01/11] Properly wait for users to leave down sync Same fix as applied in https://github.com/matrix-org/complement/pull/808 --- tests/restricted_rooms_test.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/restricted_rooms_test.go b/tests/restricted_rooms_test.go index 1fd28588..59f93c00 100644 --- a/tests/restricted_rooms_test.go +++ b/tests/restricted_rooms_test.go @@ -121,14 +121,7 @@ func checkRestrictedRoom(t *testing.T, deployment complement.Deployment, alice * // Wait until Alice sees Bob leave the allowed room. This ensures that Alice's HS // has processed the leave before Bob tries rejoining, so that it rejects his // attempt to join the room. - alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - allowed_room, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != bob.UserID { - return false - } - - return ev.Get("content").Get("membership").Str == "leave" - })) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, allowed_room)) res := bob.JoinRoom(t, room, []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), From 8656464b77554e679f26cdfdaa3dafddb6548831 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 12:02:39 -0500 Subject: [PATCH 02/11] Better wait for correct membership event --- tests/restricted_rooms_test.go | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/tests/restricted_rooms_test.go b/tests/restricted_rooms_test.go index 59f93c00..e7cf808c 100644 --- a/tests/restricted_rooms_test.go +++ b/tests/restricted_rooms_test.go @@ -290,18 +290,7 @@ func doTestRestrictedRoomsRemoteJoinLocalUser(t *testing.T, roomVersion string, // Ensure that the join comes down sync on hs2. Note that we want to ensure hs2 // accepted the event. - charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - room, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { - return false - } - must.Equal(t, ev.Get("sender").Str, bob.UserID, "Bob should have joined by himself") - must.Equal(t, ev.Get("content").Get("membership").Str, "join", "Bob failed to join the room") - - return true - }, - )) + charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, room)) // Raise the power level so that users on hs1 can invite people and then leave // the room. @@ -403,10 +392,9 @@ func doTestRestrictedRoomsRemoteJoinFailOver(t *testing.T, roomVersion string, j bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { + if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID || ev.Get("content").Get("membership").Str != "join" { return false } - must.Equal(t, ev.Get("content").Get("membership").Str, "join", "Charlie failed to join the room") must.Equal(t, ev.Get("content").Get("join_authorised_via_users_server").Str, alice.UserID, "Join authorised via incorrect server") return true @@ -459,11 +447,10 @@ func doTestRestrictedRoomsRemoteJoinFailOver(t *testing.T, roomVersion string, j bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( room, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID { + if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID || ev.Get("content").Get("membership").Str != "join" { return false } must.MatchGJSON(t, ev, - match.JSONKeyEqual("content.membership", "join"), match.JSONKeyEqual("content.join_authorised_via_users_server", alice.UserID), ) return true From 8788c3c26763b9bbb057b26dd43c8a7480dcb9f1 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 13:53:22 -0500 Subject: [PATCH 03/11] Introduce `SyncBannedFrom` --- client/sync.go | 136 +++++++++--------- tests/csapi/apidoc_room_members_test.go | 27 +--- ...federation_room_join_partial_state_test.go | 4 +- 3 files changed, 73 insertions(+), 94 deletions(-) diff --git a/client/sync.go b/client/sync.go index 1916ce22..6c8cb976 100644 --- a/client/sync.go +++ b/client/sync.go @@ -269,95 +269,97 @@ func SyncPresenceHas(fromUser string, expectedPresence *string, checks ...func(g } } -// Checks that `userID` gets invited to `roomID`. -// -// This checks different parts of the /sync response depending on the client making the request. -// If the client is also the person being invited to the room then the 'invite' block will be inspected. -// If the client is different to the person being invited then the 'join' block will be inspected. -func SyncInvitedTo(userID, roomID string) SyncCheckOpt { - return func(clientUserID string, topLevelSyncJSON gjson.Result) error { - // two forms which depend on what the client user is: - // - passively viewing an invite for a room you're joined to (timeline events) - // - actively being invited to a room. - if clientUserID == userID { - // active - err := checkArrayElements( - topLevelSyncJSON, "rooms.invite."+GjsonEscape(roomID)+".invite_state.events", - func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "invite" - }, - ) - if err != nil { - return fmt.Errorf("SyncInvitedTo(%s): %s", roomID, err) - } - return nil - } - // passive - return SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "invite" - })(clientUserID, topLevelSyncJSON) - } -} - -// Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. -// -// Additional checks can be passed to narrow down the check, all must pass. -func SyncJoinedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { - checkJoined := func(ev gjson.Result) bool { - if ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "join" { +func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Result) bool) SyncCheckOpt { + checkMembership := func(ev gjson.Result) bool { + if ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == membership { for _, check := range checks { if !check(ev) { // short-circuit, bail early return false } } - // passed both basic join check and all other checks + // passed both basic membership check and all other checks return true } return false } return func(clientUserID string, topLevelSyncJSON gjson.Result) error { - // Check both the timeline and the state events for the join event - // since on initial sync, the state events may only be in - // .state.events. + // Check both the timeline and the state events for the membership event since on + // initial sync, the state events may only be in state. Additionally, state only + // covers the "updates for the room up to the start of the timeline." + + roomTypeKey := "" + if membership == "join" { + roomTypeKey = "join" + } else if membership == "leave" || membership == "ban" { + roomTypeKey = "leave" + } else if membership == "invite" { + roomTypeKey = "invite" + } else if membership == "knock" { + roomTypeKey = "knock" + } else { + return fmt.Errorf("syncMembershipIn(%s): unknown membership: %s", roomID, membership) + } + + // Check the state + stateKey := "" + if membership == "join" || membership == "leave" || membership == "ban" { + stateKey = "state" + } else if membership == "invite" { + stateKey = "invite_state" + } else if membership == "knock" { + stateKey = "knock_state" + } else { + return fmt.Errorf("syncMembershipIn(%s): unknown membership: %s", roomID, membership) + } firstErr := checkArrayElements( - topLevelSyncJSON, "rooms.join."+GjsonEscape(roomID)+".timeline.events", checkJoined, + topLevelSyncJSON, "rooms."+roomTypeKey+"."+GjsonEscape(roomID)+"."+stateKey+".events", checkMembership, ) if firstErr == nil { return nil } - secondErr := checkArrayElements( - topLevelSyncJSON, "rooms.join."+GjsonEscape(roomID)+".state.events", checkJoined, - ) - if secondErr == nil { - return nil + // Check the timeline (only available for join/leave/ban) + var secondErr error + if membership == "join" || membership == "leave" || membership == "ban" { + secondErr = checkArrayElements( + topLevelSyncJSON, "rooms."+roomTypeKey+"."+GjsonEscape(roomID)+".timeline.events", checkMembership, + ) + if secondErr == nil { + return nil + } } - return fmt.Errorf("SyncJoinedTo(%s): %s & %s", roomID, firstErr, secondErr) + + return fmt.Errorf("syncMembershipIn(%s): %s & %s", roomID, firstErr, secondErr) } } -// Check that `userID` is leaving `roomID` by inspecting the timeline for a membership event, or witnessing `roomID` in `rooms.leave` +// Checks that `userID` gets invited to `roomID`. +// +// This checks different parts of the /sync response depending on the client making the request. +// If the client is also the person being invited to the room then we will look for an invite in the state block. +// If the client is different to the person being invited then the timeline will be inspected for invite events. +func SyncInvitedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { + return syncMembershipIn(userID, roomID, "invite") +} + +// Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. +// +// Additional checks can be passed to narrow down the check, all must pass. +func SyncJoinedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { + return syncMembershipIn(userID, roomID, "join", checks...) +} + +// Check that `userID` has left the `roomID` // Note: This will not work properly with initial syncs, see https://github.com/matrix-org/matrix-doc/issues/3537 -func SyncLeftFrom(userID, roomID string) SyncCheckOpt { - return func(clientUserID string, topLevelSyncJSON gjson.Result) error { - // two forms which depend on what the client user is: - // - passively viewing a membership for a room you're joined in - // - actively leaving the room - if clientUserID == userID { - // active - events := topLevelSyncJSON.Get("rooms.leave." + GjsonEscape(roomID)) - if !events.Exists() { - return fmt.Errorf("no leave section for room %s", roomID) - } else { - return nil - } - } - // passive - return SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "leave" - })(clientUserID, topLevelSyncJSON) - } +func SyncLeftFrom(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { + return syncMembershipIn(userID, roomID, "leave", checks...) +} + +// Check that `userID` is banned from the `roomID` +// Note: This will not work properly with initial syncs, see https://github.com/matrix-org/matrix-doc/issues/3537 +func SyncBannedFrom(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { + return syncMembershipIn(userID, roomID, "ban", checks...) } // Calls the `check` function for each global account data event, and returns with success if the diff --git a/tests/csapi/apidoc_room_members_test.go b/tests/csapi/apidoc_room_members_test.go index 41c15ef8..adc2e094 100644 --- a/tests/csapi/apidoc_room_members_test.go +++ b/tests/csapi/apidoc_room_members_test.go @@ -79,16 +79,7 @@ func TestRoomMembers(t *testing.T) { }, }) - bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - roomID, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { - return false - } - must.Equal(t, ev.Get("content").Get("membership").Str, "join", "Bob failed to join the room") - return true - }, - )) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID)) }) // sytest: Test that we can be reinvited to a room we created t.Run("Test that we can be reinvited to a room we created", func(t *testing.T) { @@ -122,14 +113,7 @@ func TestRoomMembers(t *testing.T) { alice.MustLeaveRoom(t, roomID) // Wait until alice has left the room - bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - roomID, - func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && - ev.Get("content.membership").Str == "leave" && - ev.Get("state_key").Str == alice.UserID - }, - )) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(alice.UserID, roomID)) bob.MustInviteRoom(t, roomID, alice.UserID) since := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(alice.UserID, roomID)) @@ -203,12 +187,7 @@ func TestRoomMembers(t *testing.T) { }) res := alice.Do(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "ban"}, banBody) must.MatchResponse(t, res, match.HTTPResponse{StatusCode: 200}) - alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != bob.UserID { - return false - } - return ev.Get("content.membership").Str == "ban" - })) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncBannedFrom(bob.UserID, roomID)) // verify bob is banned content := alice.MustGetStateEventContent(t, roomID, "m.room.member", bob.UserID) must.MatchGJSON(t, content, match.JSONKeyEqual("membership", "ban")) diff --git a/tests/msc3902/federation_room_join_partial_state_test.go b/tests/msc3902/federation_room_join_partial_state_test.go index 48b86814..9e11b163 100644 --- a/tests/msc3902/federation_room_join_partial_state_test.go +++ b/tests/msc3902/federation_room_join_partial_state_test.go @@ -3963,9 +3963,7 @@ func TestPartialStateJoin(t *testing.T) { aliceNextBatch = alice.MustSyncUntil( t, client.SyncReq{Since: aliceNextBatch, Filter: buildLazyLoadingSyncFilter(nil)}, - // TODO: introduce a SyncBannedFrom which checks the membership of the - // leave event - client.SyncLeftFrom(alice.UserID, serverRoom.RoomID), + client.SyncBannedFrom(alice.UserID, serverRoom.RoomID), ) t.Log("Alice tries to rejoin...") From a820988fee0bbaf945eba8b30f0bd0e373ced53e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 14:19:28 -0500 Subject: [PATCH 04/11] Tighten up `syncMembershipIn` --- client/sync.go | 80 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/client/sync.go b/client/sync.go index 6c8cb976..c97ce0f6 100644 --- a/client/sync.go +++ b/client/sync.go @@ -269,6 +269,14 @@ func SyncPresenceHas(fromUser string, expectedPresence *string, checks ...func(g } } +// syncMembershipIn checks that `userID` has `membership` in `roomID`, with optional +// extra checks on the found membership event. +// +// This can be used to passively observe another user's membership changes in a room +// although we assume that the observing client is joined to the room. +// +// Note: This will not work properly with leave/ban membership for initial syncs, see +// https://github.com/matrix-org/matrix-doc/issues/3537 func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Result) bool) SyncCheckOpt { checkMembership := func(ev gjson.Result) bool { if ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == membership { @@ -288,30 +296,46 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re // initial sync, the state events may only be in state. Additionally, state only // covers the "updates for the room up to the start of the timeline." - roomTypeKey := "" - if membership == "join" { - roomTypeKey = "join" - } else if membership == "leave" || membership == "ban" { - roomTypeKey = "leave" - } else if membership == "invite" { - roomTypeKey = "invite" - } else if membership == "knock" { - roomTypeKey = "knock" - } else { - return fmt.Errorf("syncMembershipIn(%s): unknown membership: %s", roomID, membership) + // We assume the passively observing client user is joined to the room + roomTypeKey := "join" + // Otherwise, if the client is the user whose membership we are checking, we need to + // pick the correct room type JSON key based on the membership being checked. + if clientUserID == userID { + if membership == "join" { + roomTypeKey = "join" + } else if membership == "leave" || membership == "ban" { + roomTypeKey = "leave" + } else if membership == "invite" { + roomTypeKey = "invite" + } else if membership == "knock" { + roomTypeKey = "knock" + } else { + return fmt.Errorf("syncMembershipIn(%s, %s): unknown membership: %s", roomID, membership, membership) + } } - // Check the state - stateKey := "" - if membership == "join" || membership == "leave" || membership == "ban" { - stateKey = "state" - } else if membership == "invite" { - stateKey = "invite_state" - } else if membership == "knock" { - stateKey = "knock_state" - } else { - return fmt.Errorf("syncMembershipIn(%s): unknown membership: %s", roomID, membership) + // We assume the passively observing client user is joined to the room (`rooms.join..state`) + stateKey := "state" + // Otherwise, if the client is the user whose membership we are checking, + // we need to pick the correct JSON key based on the membership being checked. + if clientUserID == userID { + if membership == "join" || membership == "leave" || membership == "ban" { + stateKey = "state" + } else if membership == "invite" { + stateKey = "invite_state" + } else if membership == "knock" { + stateKey = "knock_state" + } else { + return fmt.Errorf("syncMembershipIn(%s, %s): unknown membership: %s", roomID, membership, membership) + } } + + // Check the state first as it's a better source of truth than the `timeline`. + // + // FIXME: Ideally, we'd use something like `state_after` to get the actual current + // state in the room instead of us assuming that no state resets/conflicts happen + // when we apply state from the `timeline` on top of the `state`. But `state_after` + // is gated behind a sync request parameter which we can't control here. firstErr := checkArrayElements( topLevelSyncJSON, "rooms."+roomTypeKey+"."+GjsonEscape(roomID)+"."+stateKey+".events", checkMembership, ) @@ -319,9 +343,17 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re return nil } - // Check the timeline (only available for join/leave/ban) + // Check the timeline + // + // This is also important to differentiate between leave/ban because those both + // appear in the `leave` `roomTypeKey` and we need to specifically check the + // timeline for the membership event to differentiate them. var secondErr error - if membership == "join" || membership == "leave" || membership == "ban" { + // We assume the passively observing client user is joined to the room + if clientUserID != userID || + // Otherwise, if the client is the user whose membership we are checking, + // `timeline` is only available for join/leave/ban memberships. + membership == "join" || membership == "leave" || membership == "ban" { secondErr = checkArrayElements( topLevelSyncJSON, "rooms."+roomTypeKey+"."+GjsonEscape(roomID)+".timeline.events", checkMembership, ) @@ -330,7 +362,7 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re } } - return fmt.Errorf("syncMembershipIn(%s): %s & %s", roomID, firstErr, secondErr) + return fmt.Errorf("syncMembershipIn(%s, %s): %s & %s - %s", roomID, membership, firstErr, secondErr, topLevelSyncJSON) } } From cca43680415cd184d966f64aa7cd1f10b2c3c450 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 14:32:12 -0500 Subject: [PATCH 05/11] Replace more `SyncTimelineHas` usage --- tests/restricted_rooms_test.go | 35 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/tests/restricted_rooms_test.go b/tests/restricted_rooms_test.go index e7cf808c..8bb8406d 100644 --- a/tests/restricted_rooms_test.go +++ b/tests/restricted_rooms_test.go @@ -389,17 +389,12 @@ func doTestRestrictedRoomsRemoteJoinFailOver(t *testing.T, roomVersion string, j }) // Double check that the join was authorised via hs1. - bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - room, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID || ev.Get("content").Get("membership").Str != "join" { - return false - } - must.Equal(t, ev.Get("content").Get("join_authorised_via_users_server").Str, alice.UserID, "Join authorised via incorrect server") - - return true - }, - )) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(charlie.UserID, room, func(ev gjson.Result) bool { + must.MatchGJSON(t, ev, + match.JSONKeyEqual("content.join_authorised_via_users_server", alice.UserID), + ) + return true + })) // Bump the power-level of bob. t.Logf("%s allows %s to send invites.", alice.UserID, bob.UserID) @@ -444,16 +439,10 @@ func doTestRestrictedRoomsRemoteJoinFailOver(t *testing.T, roomVersion string, j }) // Double check that the join was authorised via hs1. - bob.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas( - room, - func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("state_key").Str != charlie.UserID || ev.Get("content").Get("membership").Str != "join" { - return false - } - must.MatchGJSON(t, ev, - match.JSONKeyEqual("content.join_authorised_via_users_server", alice.UserID), - ) - return true - }, - )) + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(charlie.UserID, room, func(ev gjson.Result) bool { + must.MatchGJSON(t, ev, + match.JSONKeyEqual("content.join_authorised_via_users_server", alice.UserID), + ) + return true + })) } From 2ec2fedec9738b7ff07b4cbeea2a7e8c47ecdd1f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 14:36:46 -0500 Subject: [PATCH 06/11] Replace `SyncTimelineHas` in `TestFederationRoomsInvite` --- tests/federation_rooms_invite_test.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/tests/federation_rooms_invite_test.go b/tests/federation_rooms_invite_test.go index c5200d8d..d5386014 100644 --- a/tests/federation_rooms_invite_test.go +++ b/tests/federation_rooms_invite_test.go @@ -227,22 +227,15 @@ func TestFederationRoomsInvite(t *testing.T) { "is_direct": true, }) bob.MustJoinRoom(t, roomID, []spec.ServerName{}) - bob.MustSyncUntil(t, client.SyncReq{}, - client.SyncTimelineHas(roomID, func(result gjson.Result) bool { - // We expect a membership event .. - if result.Get("type").Str != spec.MRoomMember { - return false - } - // .. for Bob - if result.Get("state_key").Str != bob.UserID { - return false - } - // Check that we've got tbe expected is_idrect flag - return result.Get("unsigned.prev_content.membership").Str == "invite" && - result.Get("unsigned.prev_content.is_direct").Bool() == true && - result.Get("unsigned.prev_sender").Str == alice.UserID - }), - ) + + bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, roomID, func(ev gjson.Result) bool { + must.MatchGJSON(t, ev, + match.JSONKeyEqual("unsigned.prev_content.membership", "invite"), + match.JSONKeyEqual("unsigned.prev_content.is_direct", true), + match.JSONKeyEqual("unsigned.prev_sender", alice.UserID), + ) + return true + })) }) }) } From 3c93edbd47cecf74c3bcaafdf6f0c4e2d108b321 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 14:58:58 -0500 Subject: [PATCH 07/11] Replace `SyncTimelineHas` in knocking tests --- client/sync.go | 17 ++++++++--- tests/csapi/rooms_state_test.go | 12 ++++---- tests/knocking_test.go | 52 +++++---------------------------- 3 files changed, 26 insertions(+), 55 deletions(-) diff --git a/client/sync.go b/client/sync.go index c97ce0f6..d5535234 100644 --- a/client/sync.go +++ b/client/sync.go @@ -368,11 +368,16 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re // Checks that `userID` gets invited to `roomID`. // -// This checks different parts of the /sync response depending on the client making the request. -// If the client is also the person being invited to the room then we will look for an invite in the state block. -// If the client is different to the person being invited then the timeline will be inspected for invite events. +// Additional checks can be passed to narrow down the check, all must pass. func SyncInvitedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { - return syncMembershipIn(userID, roomID, "invite") + return syncMembershipIn(userID, roomID, "invite", checks...) +} + +// Checks that `userID` has knocked on `roomID`. +// +// Additional checks can be passed to narrow down the check, all must pass. +func SyncKnockedOn(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { + return syncMembershipIn(userID, roomID, "knock", checks...) } // Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. @@ -384,12 +389,16 @@ func SyncJoinedTo(userID, roomID string, checks ...func(gjson.Result) bool) Sync // Check that `userID` has left the `roomID` // Note: This will not work properly with initial syncs, see https://github.com/matrix-org/matrix-doc/issues/3537 +// +// Additional checks can be passed to narrow down the check, all must pass. func SyncLeftFrom(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { return syncMembershipIn(userID, roomID, "leave", checks...) } // Check that `userID` is banned from the `roomID` // Note: This will not work properly with initial syncs, see https://github.com/matrix-org/matrix-doc/issues/3537 +// +// Additional checks can be passed to narrow down the check, all must pass. func SyncBannedFrom(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { return syncMembershipIn(userID, roomID, "ban", checks...) } diff --git a/tests/csapi/rooms_state_test.go b/tests/csapi/rooms_state_test.go index 05a482e9..2158cae1 100644 --- a/tests/csapi/rooms_state_test.go +++ b/tests/csapi/rooms_state_test.go @@ -13,6 +13,7 @@ import ( "github.com/matrix-org/complement/b" "github.com/matrix-org/complement/client" "github.com/matrix-org/complement/helpers" + "github.com/matrix-org/complement/match" "github.com/matrix-org/complement/must" ) @@ -46,13 +47,10 @@ func TestRoomCreationReportsEventsToMyself(t *testing.T) { t.Run("Room creation reports m.room.member to myself", func(t *testing.T) { t.Parallel() - alice.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" { - return false - } - must.Equal(t, ev.Get("sender").Str, alice.UserID, "wrong sender") - must.Equal(t, ev.Get("state_key").Str, alice.UserID, "wrong state_key") - must.Equal(t, ev.Get("content").Get("membership").Str, "join", "wrong content.membership") + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, roomID, func(ev gjson.Result) bool { + must.MatchGJSON(t, ev, + match.JSONKeyEqual("sender", alice.UserID), + ) return true })) }) diff --git a/tests/knocking_test.go b/tests/knocking_test.go index 7c1bc98b..7eb625cf 100644 --- a/tests/knocking_test.go +++ b/tests/knocking_test.go @@ -163,12 +163,10 @@ func knockingBetweenTwoUsersTest( } } - inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - if ev.Get("type").Str != "m.room.member" || ev.Get("sender").Str != knockingUser.UserID { - return false - } - must.Equal(t, ev.Get("content").Get("reason").Str, testKnockReason, "incorrect reason for knock") - must.Equal(t, ev.Get("content").Get("membership").Str, "knock", "incorrect membership for knocking user") + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncKnockedOn(knockingUser.UserID, roomID, func(ev gjson.Result) bool { + must.MatchGJSON(t, ev, + match.JSONKeyEqual("content.reason", testKnockReason), + ) return true })) }) @@ -224,11 +222,7 @@ func knockingBetweenTwoUsersTest( ) // Wait until the leave membership event has come down sync - inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str != "m.room.member" || - ev.Get("state_key").Str != knockingUser.UserID || - ev.Get("content").Get("membership").Str != "leave" - })) + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(knockingUser.UserID, roomID)) // Knock again mustKnockOnRoomSynced(t, knockingUser, roomID, "Pleeease let me in?", []spec.ServerName{ @@ -258,11 +252,7 @@ func knockingBetweenTwoUsersTest( inRoomUser.MustInviteRoom(t, roomID, knockingUser.UserID) // Wait until the invite membership event has come down sync - inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str != "m.room.member" || - ev.Get("state_key").Str != knockingUser.UserID || - ev.Get("content").Get("membership").Str != "invite" - })) + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(knockingUser.UserID, roomID)) }) t.Run("A user cannot knock on a room they are already invited to", func(t *testing.T) { @@ -299,11 +289,7 @@ func knockingBetweenTwoUsersTest( ) // Wait until the ban membership event has come down sync - inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str != "m.room.member" || - ev.Get("state_key").Str != knockingUser.UserID || - ev.Get("content").Get("membership").Str != "ban" - })) + inRoomUser.MustSyncUntil(t, client.SyncReq{}, client.SyncBannedFrom(knockingUser.UserID, roomID)) knockOnRoomWithStatus(t, knockingUser, roomID, "I didn't mean it!", []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), @@ -311,28 +297,6 @@ func knockingBetweenTwoUsersTest( }) } -func syncKnockedOn(userID, roomID string) client.SyncCheckOpt { - return func(clientUserID string, topLevelSyncJSON gjson.Result) error { - // two forms which depend on what the client user is: - // - passively viewing a membership for a room you're joined in - // - actively leaving the room - if clientUserID == userID { - events := topLevelSyncJSON.Get("rooms.knock." + client.GjsonEscape(roomID) + ".knock_state.events") - if events.Exists() && events.IsArray() { - // We don't currently define any required state event types to be sent. - // If we've reached this point, then an entry for this room was found - return nil - } - return fmt.Errorf("no knock section for room %s", roomID) - } - - // passive - return client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { - return ev.Get("type").Str == "m.room.member" && ev.Get("state_key").Str == userID && ev.Get("content.membership").Str == "knock" - })(clientUserID, topLevelSyncJSON) - } -} - // mustKnockOnRoomSynced will knock on a given room on the behalf of a user, and block until the knock has persisted. // serverNames should be populated if knocking on a room that the user's homeserver isn't currently a part of. // Fails the test if the knock response does not return a 200 status code. @@ -344,7 +308,7 @@ func mustKnockOnRoomSynced(t *testing.T, c *client.CSAPI, roomID, reason string, knockOnRoomWithStatus(t, c, roomID, reason, serverNames, 200) // The knock should have succeeded. Block until we see the knock appear down sync - c.MustSyncUntil(t, client.SyncReq{}, syncKnockedOn(c.UserID, roomID)) + c.MustSyncUntil(t, client.SyncReq{}, client.SyncKnockedOn(c.UserID, roomID)) } // knockOnRoomWithStatus will knock on a given room on the behalf of a user. From ab61fd353109862cd97f967f946040a96a3d4491 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 15:25:54 -0500 Subject: [PATCH 08/11] Align docstrings --- client/sync.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/sync.go b/client/sync.go index d5535234..2389ed08 100644 --- a/client/sync.go +++ b/client/sync.go @@ -366,21 +366,21 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re } } -// Checks that `userID` gets invited to `roomID`. +// Checks that `userID` gets invited to `roomID` // // Additional checks can be passed to narrow down the check, all must pass. func SyncInvitedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { return syncMembershipIn(userID, roomID, "invite", checks...) } -// Checks that `userID` has knocked on `roomID`. +// Checks that `userID` has knocked on `roomID` // // Additional checks can be passed to narrow down the check, all must pass. func SyncKnockedOn(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { return syncMembershipIn(userID, roomID, "knock", checks...) } -// Check that `userID` gets joined to `roomID` by inspecting the join timeline for a membership event. +// Check that `userID` gets joined to `roomID` // // Additional checks can be passed to narrow down the check, all must pass. func SyncJoinedTo(userID, roomID string, checks ...func(gjson.Result) bool) SyncCheckOpt { From 9547994dd0221081ec528eab71da83379e2c6095 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 27 Oct 2025 15:28:58 -0500 Subject: [PATCH 09/11] Use correct assertion now that we differentiate leave/ban --- tests/federation_room_ban_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/federation_room_ban_test.go b/tests/federation_room_ban_test.go index 9a3ff479..29d3d53c 100644 --- a/tests/federation_room_ban_test.go +++ b/tests/federation_room_ban_test.go @@ -30,7 +30,7 @@ func TestUnbanViaInvite(t *testing.T) { bob.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "ban"}, client.WithJSONBody(t, map[string]interface{}{ "user_id": alice.UserID, })) - alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(alice.UserID, roomID)) + alice.MustSyncUntil(t, client.SyncReq{}, client.SyncBannedFrom(alice.UserID, roomID)) // Unban Alice bob.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "unban"}, client.WithJSONBody(t, map[string]interface{}{ From 9cc6af9c5defbce555aece6c3191a818d6f9ea85 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 10 Nov 2025 14:24:08 -0600 Subject: [PATCH 10/11] Make it more clear that this is an additional use case --- client/sync.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/sync.go b/client/sync.go index 2389ed08..b5dcad4e 100644 --- a/client/sync.go +++ b/client/sync.go @@ -272,8 +272,8 @@ func SyncPresenceHas(fromUser string, expectedPresence *string, checks ...func(g // syncMembershipIn checks that `userID` has `membership` in `roomID`, with optional // extra checks on the found membership event. // -// This can be used to passively observe another user's membership changes in a room -// although we assume that the observing client is joined to the room. +// This can be also used to passively observe another user's membership changes in a +// room although we assume that the observing client is joined to the room. // // Note: This will not work properly with leave/ban membership for initial syncs, see // https://github.com/matrix-org/matrix-doc/issues/3537 From 680e22a6c200a3066fdf682ab0f52423c8537567 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 10 Nov 2025 14:31:15 -0600 Subject: [PATCH 11/11] Make the `timeline` logic more straight-forward --- client/sync.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/client/sync.go b/client/sync.go index b5dcad4e..7939d944 100644 --- a/client/sync.go +++ b/client/sync.go @@ -5,6 +5,7 @@ import ( "net/http" "net/url" "reflect" + "slices" "sort" "strings" "time" @@ -349,11 +350,11 @@ func syncMembershipIn(userID, roomID, membership string, checks ...func(gjson.Re // appear in the `leave` `roomTypeKey` and we need to specifically check the // timeline for the membership event to differentiate them. var secondErr error - // We assume the passively observing client user is joined to the room - if clientUserID != userID || - // Otherwise, if the client is the user whose membership we are checking, - // `timeline` is only available for join/leave/ban memberships. - membership == "join" || membership == "leave" || membership == "ban" { + // The `timeline` is only available for join/leave/ban memberships. + if slices.Contains([]string{"join", "leave", "ban"}, membership) || + // We assume the passively observing client user is joined to the room (therefore + // has `timeline`). + clientUserID != userID { secondErr = checkArrayElements( topLevelSyncJSON, "rooms."+roomTypeKey+"."+GjsonEscape(roomID)+".timeline.events", checkMembership, )