From c5adad180c1226193c3dd485e49bfaf8f84eb2e2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 28 Apr 2021 08:16:14 -0400 Subject: [PATCH 1/9] Add a test for restricted rooms appearing in the spaces summary. --- tests/msc3083_test.go | 94 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 3ddb0449..20151cd6 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -1,4 +1,4 @@ -// +build msc3083 +// +build msc2946,msc3083 // Tests MSC3083, an experimental feature for joining restricted rooms based on // membership in a space. @@ -12,6 +12,9 @@ import ( "github.com/matrix-org/complement/internal/b" "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/docker" + "github.com/matrix-org/complement/internal/match" + "github.com/matrix-org/complement/internal/must" + "github.com/tidwall/gjson" ) var ( @@ -159,3 +162,92 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { // Execute the checks. CheckRestrictedRoom(t, alice, bob, space, room) } + +// Tests that MSC2946 works for a restricted room. +// +// Create a space with a room in it that has join rules restricted to membership +// in that space. +// +// The user should be unable to see the room in the spaces summary unless they +// are a member of the space. +func TestRestrictedRoomsSpacesSummary(t *testing.T) { + deployment := Deploy(t, "msc2946", b.BlueprintOneToOneRoom) + defer deployment.Destroy(t) + + // Create the rooms + alice := deployment.Client(t, "hs1", "@alice:hs1") + space := alice.CreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": "Space", + // World readable to allow peeking without joining. + "initial_state": []map[string]interface{}{ + { + "type": "m.room.history_visibility", + "state_key": "", + "content": map[string]interface{}{ + "history_visibility": "world_readable", + }, + }, + }, + }) + // The room is an unstable room version which supports the restricted join_rule. + room := alice.CreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": "Room", + "room_version": "org.matrix.msc3083", + "initial_state": []map[string]interface{}{ + { + "type": "m.room.join_rules", + "state_key": "", + "content": map[string]interface{}{ + "join_rule": "restricted", + "allow": []map[string]interface{}{ + { + "space": &space, + "via": []string{"hs1"}, + }, + }, + }, + }, + }, + }) + alice.SendEventSynced(t, space, b.Event{ + Type: "org.matrix.msc1772.space.child", + StateKey: &room, + Content: map[string]interface{}{ + "via": []string{"hs1"}, + }, + }) + + t.Logf("Space: %s", space) + t.Logf("Room: %s", room) + + // Create a second user on the same homeserver. + bob := deployment.Client(t, "hs1", "@bob:hs1") + + // Querying the space returns only the space, as the room is restricted. + res := bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) + + // Join the space, and now the restricted room should appear. + bob.JoinRoom(t, space, []string{"hs1"}) + + res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, room, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) +} From bfd9cd93716b69496a2046dd6ba6d15054fbd9f5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 13 May 2021 10:02:56 -0400 Subject: [PATCH 2/9] Add tests for returning restricted rooms for the spaces summary over federation. --- tests/msc3083_test.go | 112 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 20151cd6..e1f14043 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -251,3 +251,115 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { }, }) } + +// Tests that MSC2946 works over federation for a restricted room. +// +// Create a space with a room in it that has join rules restricted to membership +// in that space. The space and room are on different homeservers. +// +// The user should be unable to see the room in the spaces summary unless they +// are a member of the space. +func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { + deployment := Deploy(t, "msc2946", b.BlueprintFederationTwoLocalOneRemote) + defer deployment.Destroy(t) + + // Create the rooms + alice := deployment.Client(t, "hs1", "@alice:hs1") + bob := deployment.Client(t, "hs1", "@bob:hs1") + space := alice.CreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "initial_state": []map[string]interface{}{ + { + "type": "m.room.history_visibility", + "state_key": "", + "content": map[string]string{ + "history_visibility": "world_readable", + }, + }, + }, + }) + + // The room is an unstable room version which supports the restricted join_rule + // and is created on hs2. + charlie := deployment.Client(t, "hs2", "@charlie:hs2") + room := charlie.CreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": "Room", + "room_version": "org.matrix.msc3083", + "initial_state": []map[string]interface{}{ + { + "type": "m.room.join_rules", + "state_key": "", + "content": map[string]interface{}{ + "join_rule": "restricted", + "allow": []map[string]interface{}{ + { + "space": &space, + "via": []string{"hs1"}, + }, + }, + }, + }, + }, + }) + + // create the link (this doesn't really make sense since how would alice know + // about the room? but it works for testing) + alice.SendEventSynced(t, space, b.Event{ + Type: spaceChildEventType, + StateKey: &room, + Content: map[string]interface{}{ + "via": []string{"hs2"}, + }, + }) + + // The room appears for no one at first since hs2 doesn't know about who is in ss1. + res := alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) + + res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) + + // Charlie joins ss1 and now hs2 knows that alice is in it. + charlie.JoinRoom(t, space, []string{"hs1"}) + + // The restricted room should appear for alice (who is in the space). + res = alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, room, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) + + // Bob still doesn't know about the room. + res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", []interface{}{ + space, + }, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) +} From d1e32cb1448094bd8f40e599f845a31ee45e3dcb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 13 May 2021 11:31:31 -0400 Subject: [PATCH 3/9] Abstract some code. --- tests/msc3083_test.go | 83 ++++++++++--------------------------------- 1 file changed, 19 insertions(+), 64 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index e1f14043..eeb89dda 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -163,6 +163,19 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { CheckRestrictedRoom(t, alice, bob, space, room) } +// Request the room summary and ensure the expected rooms are in the response. +func RequestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, expected_rooms []interface{}) { + // The room appears for no one at first since hs2 doesn't know about who is in ss1. + res := user.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONCheckOff("rooms", expected_rooms, func(r gjson.Result) interface{} { + return r.Get("room_id").Str + }, nil), + }, + }) +} + // Tests that MSC2946 works for a restricted room. // // Create a space with a room in it that has join rules restricted to membership @@ -226,30 +239,11 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { bob := deployment.Client(t, "hs1", "@bob:hs1") // Querying the space returns only the space, as the room is restricted. - res := bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) + RequestAndAssertSummary(t, bob, space, []interface{}{space}) // Join the space, and now the restricted room should appear. bob.JoinRoom(t, space, []string{"hs1"}) - - res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, room, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) + RequestAndAssertSummary(t, bob, space, []interface{}{space, room}) } // Tests that MSC2946 works over federation for a restricted room. @@ -314,52 +308,13 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { }) // The room appears for no one at first since hs2 doesn't know about who is in ss1. - res := alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) - - res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) + RequestAndAssertSummary(t, alice, space, []interface{}{space}) + RequestAndAssertSummary(t, bob, space, []interface{}{space}) // Charlie joins ss1 and now hs2 knows that alice is in it. charlie.JoinRoom(t, space, []string{"hs1"}) // The restricted room should appear for alice (who is in the space). - res = alice.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, room, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) - - // Bob still doesn't know about the room. - res = bob.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONCheckOff("rooms", []interface{}{ - space, - }, func(r gjson.Result) interface{} { - return r.Get("room_id").Str - }, nil), - }, - }) + RequestAndAssertSummary(t, alice, space, []interface{}{space, room}) + RequestAndAssertSummary(t, bob, space, []interface{}{space}) } From 63a30217d5b8e05e5c2a645414df2b873f50c72a Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 14 May 2021 15:20:58 +0100 Subject: [PATCH 4/9] Remove namespace --- tests/msc3083_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index d4ea7bdb..d290da41 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -184,7 +184,7 @@ func RequestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, exp // The user should be unable to see the room in the spaces summary unless they // are a member of the space. func TestRestrictedRoomsSpacesSummary(t *testing.T) { - deployment := Deploy(t, "msc2946", b.BlueprintOneToOneRoom) + deployment := Deploy(t, b.BlueprintOneToOneRoom) defer deployment.Destroy(t) // Create the rooms @@ -254,7 +254,7 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { // The user should be unable to see the room in the spaces summary unless they // are a member of the space. func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { - deployment := Deploy(t, "msc2946", b.BlueprintFederationTwoLocalOneRemote) + deployment := Deploy(t, b.BlueprintFederationTwoLocalOneRemote) defer deployment.Destroy(t) // Create the rooms From 32532b00922fcf18f0b49b4d2a79526cdfdd7b3a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 17 May 2021 09:55:38 -0400 Subject: [PATCH 5/9] Remove incorrect comment. --- tests/msc3083_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index d290da41..7dc74910 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -165,7 +165,6 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { // Request the room summary and ensure the expected rooms are in the response. func RequestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, expected_rooms []interface{}) { - // The room appears for no one at first since hs2 doesn't know about who is in ss1. res := user.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ From 01141341acf23a02d73401627ecad8410c42c245 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 17 May 2021 09:58:10 -0400 Subject: [PATCH 6/9] Do not export functions. --- tests/msc3083_test.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 7dc74910..d3bf51d7 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -21,7 +21,7 @@ var ( msc1772SpaceChildEventType = "org.matrix.msc1772.space.child" ) -func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string) { +func failJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string) { // This is copied from Client.JoinRoom to test a join failure. query := make(url.Values, 1) query.Set("server_name", serverName) @@ -39,7 +39,7 @@ func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverNam // Create a space and put a room in it which is set to: // * The experimental room version. // * restricted join rules with allow set to the space. -func SetupRestrictedRoom(t *testing.T, deployment *docker.Deployment) (*client.CSAPI, string, string) { +func setupRestrictedRoom(t *testing.T, deployment *docker.Deployment) (*client.CSAPI, string, string) { alice := deployment.Client(t, "hs1", "@alice:hs1") space := alice.CreateRoom(t, map[string]interface{}{ "preset": "public_chat", @@ -77,8 +77,8 @@ func SetupRestrictedRoom(t *testing.T, deployment *docker.Deployment) (*client.C return alice, space, room } -func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, space string, room string) { - FailJoinRoom(bob, t, room, "hs1") +func checkRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, space string, room string) { + failJoinRoom(bob, t, room, "hs1") // Join the space, attempt to join the room again, which now should succeed. bob.JoinRoom(t, space, []string{"hs1"}) @@ -87,7 +87,7 @@ func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, s // Leaving the room works and the user is unable to re-join. bob.LeaveRoom(t, room) bob.LeaveRoom(t, space) - FailJoinRoom(bob, t, room, "hs1") + failJoinRoom(bob, t, room, "hs1") // Invite the user and joining should work. alice.InviteRoom(t, room, bob.UserID) @@ -114,7 +114,7 @@ func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, s }, ) // Fails since invalid values get filtered out of allow. - FailJoinRoom(bob, t, room, "hs1") + failJoinRoom(bob, t, room, "hs1") alice.SendEventSynced( t, @@ -130,7 +130,7 @@ func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, s }, ) // Fails since a fully invalid allow key requires an invite. - FailJoinRoom(bob, t, room, "hs1") + failJoinRoom(bob, t, room, "hs1") } // Test joining a room with join rules restricted to membership in a space. @@ -139,13 +139,13 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { defer deployment.Destroy(t) // Setup the user, space, and restricted room. - alice, space, room := SetupRestrictedRoom(t, deployment) + alice, space, room := setupRestrictedRoom(t, deployment) // Create a second user on the same homeserver. bob := deployment.Client(t, "hs1", "@bob:hs1") // Execute the checks. - CheckRestrictedRoom(t, alice, bob, space, room) + checkRestrictedRoom(t, alice, bob, space, room) } // Test joining a room with join rules restricted to membership in a space. @@ -154,17 +154,17 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { defer deployment.Destroy(t) // Setup the user, space, and restricted room. - alice, space, room := SetupRestrictedRoom(t, deployment) + alice, space, room := setupRestrictedRoom(t, deployment) // Create a second user on a different homeserver. bob := deployment.Client(t, "hs2", "@bob:hs2") // Execute the checks. - CheckRestrictedRoom(t, alice, bob, space, room) + checkRestrictedRoom(t, alice, bob, space, room) } // Request the room summary and ensure the expected rooms are in the response. -func RequestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, expected_rooms []interface{}) { +func requestAndAssertSummary(t *testing.T, user *client.CSAPI, space string, expected_rooms []interface{}) { res := user.MustDo(t, "POST", []string{"_matrix", "client", "unstable", "org.matrix.msc2946", "rooms", space, "spaces"}, map[string]interface{}{}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ @@ -238,11 +238,11 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { bob := deployment.Client(t, "hs1", "@bob:hs1") // Querying the space returns only the space, as the room is restricted. - RequestAndAssertSummary(t, bob, space, []interface{}{space}) + requestAndAssertSummary(t, bob, space, []interface{}{space}) // Join the space, and now the restricted room should appear. bob.JoinRoom(t, space, []string{"hs1"}) - RequestAndAssertSummary(t, bob, space, []interface{}{space, room}) + requestAndAssertSummary(t, bob, space, []interface{}{space, room}) } // Tests that MSC2946 works over federation for a restricted room. @@ -307,13 +307,13 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { }) // The room appears for no one at first since hs2 doesn't know about who is in ss1. - RequestAndAssertSummary(t, alice, space, []interface{}{space}) - RequestAndAssertSummary(t, bob, space, []interface{}{space}) + requestAndAssertSummary(t, alice, space, []interface{}{space}) + requestAndAssertSummary(t, bob, space, []interface{}{space}) // Charlie joins ss1 and now hs2 knows that alice is in it. charlie.JoinRoom(t, space, []string{"hs1"}) // The restricted room should appear for alice (who is in the space). - RequestAndAssertSummary(t, alice, space, []interface{}{space, room}) - RequestAndAssertSummary(t, bob, space, []interface{}{space}) + requestAndAssertSummary(t, alice, space, []interface{}{space, room}) + requestAndAssertSummary(t, bob, space, []interface{}{space}) } From e731fe2a7414541ba3fe8aeb1a0a690c0ef3338a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 2 Jun 2021 11:54:27 -0400 Subject: [PATCH 7/9] Review comments. --- tests/msc3083_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index ec6c22de..3b60cb19 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -256,6 +256,10 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { // // The user should be unable to see the room in the spaces summary unless they // are a member of the space. +// +// This tests the interactions over federation where the space and room are on +// difference homeservers, which might not have the proper information needed to +// decide if a user is in a room. func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { deployment := Deploy(t, b.BlueprintFederationTwoLocalOneRemote) defer deployment.Destroy(t) @@ -265,6 +269,7 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { bob := deployment.Client(t, "hs1", "@bob:hs1") space := alice.CreateRoom(t, map[string]interface{}{ "preset": "public_chat", + "name": "Space", "initial_state": []map[string]interface{}{ { "type": "m.room.history_visibility", @@ -310,11 +315,13 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { }, }) - // The room appears for no one at first since hs2 doesn't know about who is in ss1. + // The room appears for neither alice or bob initially. Although alice is in + // the space and should be able to access the room, hs2 doesn't know this! requestAndAssertSummary(t, alice, space, []interface{}{space}) requestAndAssertSummary(t, bob, space, []interface{}{space}) - // Charlie joins ss1 and now hs2 knows that alice is in it. + // charlie joins the space and now hs2 knows that alice is in the space (and + // can join room). charlie.JoinRoom(t, space, []string{"hs1"}) // The restricted room should appear for alice (who is in the space). From d1855cb626548e46ce6f0c2482a7fab4ec034ddf Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 2 Jun 2021 12:37:39 -0400 Subject: [PATCH 8/9] Fix typos. Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- tests/msc3083_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 3b60cb19..7c7e87e9 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -258,7 +258,7 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { // are a member of the space. // // This tests the interactions over federation where the space and room are on -// difference homeservers, which might not have the proper information needed to +// different homeservers, and one might not have the proper information needed to // decide if a user is in a room. func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { deployment := Deploy(t, b.BlueprintFederationTwoLocalOneRemote) @@ -321,7 +321,7 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) { requestAndAssertSummary(t, bob, space, []interface{}{space}) // charlie joins the space and now hs2 knows that alice is in the space (and - // can join room). + // can join the room). charlie.JoinRoom(t, space, []string{"hs1"}) // The restricted room should appear for alice (who is in the space). From 469891e06575860750bfc3943dbff46ee2aa3a6f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 2 Jun 2021 12:38:52 -0400 Subject: [PATCH 9/9] More clarifying comments. --- tests/msc3083_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 7c7e87e9..706b20ec 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -252,7 +252,9 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) { // Tests that MSC2946 works over federation for a restricted room. // // Create a space with a room in it that has join rules restricted to membership -// in that space. The space and room are on different homeservers. +// in that space. The space and room are on different homeservers. While generating +// the summary of space hs1 needs to ask hs2 to generate the summary for room since +// it is not participating in the room. // // The user should be unable to see the room in the spaces summary unless they // are a member of the space.