From aea126c311b5dd45d6c89fc8b0bdde16a47f6ea5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 6 Apr 2021 10:25:22 -0400 Subject: [PATCH 1/9] Add tests for joining over federation. --- tests/msc3083_test.go | 64 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 86b51327..099f689a 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -18,7 +18,7 @@ var ( spaceParentEventType = "org.matrix.msc1772.space.parent" ) -func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string) { +func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string, expectedHttpCode int) { // This is copied from Client.JoinRoom to test a join failure. query := make(url.Values, 1) query.Set("server_name", serverName) @@ -29,13 +29,13 @@ func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverNam nil, "application/json", query, - 403, + expectedHttpCode, ) } // Test joining a room with join rules restricted to membership in a space. func TestRestrictedRoomsLocalJoin(t *testing.T) { - deployment := Deploy(t, "msc3083", b.BlueprintOneToOneRoom) + deployment := Deploy(t, "msc3083_local", b.BlueprintOneToOneRoom) defer deployment.Destroy(t) // Create the space and put a room in it. @@ -75,7 +75,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { // Create a second user and attempt to join the room, it should fail. bob := deployment.Client(t, "hs1", "@bob:hs1") - FailJoinRoom(bob, t, room, "hs1") + FailJoinRoom(bob, t, room, "hs1", 403) // Join the space, attempt to join the room again, which now should succeed. bob.JoinRoom(t, space, []string{"hs1"}) @@ -84,7 +84,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { // 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", 403) // Invite the user and joining should work. alice.InviteRoom(t, room, "@bob:hs1") @@ -111,7 +111,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { }, ) // Fails since invalid values get filtered out of allow. - FailJoinRoom(bob, t, room, "hs1") + FailJoinRoom(bob, t, room, "hs1", 403) alice.SendEventSynced( t, @@ -127,5 +127,55 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { }, ) // Fails since a fully invalid allow key rquires an invite. - FailJoinRoom(bob, t, room, "hs1") + FailJoinRoom(bob, t, room, "hs1", 403) +} + +// Test joining a room with join rules restricted to membership in a space. +func TestRestrictedRoomsRemoteJoin(t *testing.T) { + deployment := Deploy(t, "msc3083_remote", b.BlueprintFederationOneToOneRoom) + defer deployment.Destroy(t) + + // Create the space and put a room in it. + alice := deployment.Client(t, "hs1", "@alice:hs1") + space := alice.CreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "name": "Space", + }) + // 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: spaceChildEventType, + StateKey: &room, + Content: map[string]interface{}{ + "via": []string{"hs1"}, + }, + }) + + // Create a second user and attempt to join the room, it should fail. + bob := deployment.Client(t, "hs2", "@bob:hs2") + // Note that this is a 400 error because it comes back over federation. + FailJoinRoom(bob, t, room, "hs1", 400) + + // Join the space, attempt to join the room again, which now should succeed. + bob.JoinRoom(t, space, []string{"hs1"}) + bob.JoinRoom(t, room, []string{"hs1"}) } From b237b3c3aeac28ffc87da1ef3c8b26ac86ac1b12 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 6 Apr 2021 10:27:13 -0400 Subject: [PATCH 2/9] Copy error checking over federation. --- tests/msc3083_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 099f689a..fde7a815 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -178,4 +178,52 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { // Join the space, attempt to join the room again, which now should succeed. bob.JoinRoom(t, space, []string{"hs1"}) bob.JoinRoom(t, room, []string{"hs1"}) + + // 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", 400) + + // Invite the user and joining shuold work. + alice.InviteRoom(t, room, "@bob:hs2") + bob.JoinRoom(t, room, []string{"hs1"}) + + // Leave the room again, and join the space. + bob.LeaveRoom(t, room) + bob.JoinRoom(t, space, []string{"hs1"}) + + // Update the room to have bad values in the "allow" field, which should stop + // joining from working properly. + emptyStateKey := "" + alice.SendEventSynced( + t, + room, + b.Event{ + Type: "m.room.join_rules", + Sender: alice.UserID, + StateKey: &emptyStateKey, + Content: map[string]interface{}{ + "join_rule": "restricted", + "allow": []string{"invalid"}, + }, + }, + ) + // Fails since invalid values get filtered out of allow. + FailJoinRoom(bob, t, room, "hs1", 400) + + alice.SendEventSynced( + t, + room, + b.Event{ + Type: "m.room.join_rules", + Sender: alice.UserID, + StateKey: &emptyStateKey, + Content: map[string]interface{}{ + "join_rule": "restricted", + "allow": "invalid", + }, + }, + ) + // Fails since a fully invalid allow key rquires an invite. + FailJoinRoom(bob, t, room, "hs1", 400) } From 5425d5d2b51224744d16d2c974b0716689835ca4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 8 Apr 2021 10:00:37 -0400 Subject: [PATCH 3/9] Rename variable to avoid conflicting with msc2946_test. --- tests/msc3083_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index fde7a815..b7382361 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -14,8 +14,7 @@ import ( ) var ( - spaceChildEventType = "org.matrix.msc1772.space.child" - spaceParentEventType = "org.matrix.msc1772.space.parent" + msc1772SpaceChildEventType = "org.matrix.msc1772.space.child" ) func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string, expectedHttpCode int) { @@ -66,7 +65,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { }, }) alice.SendEventSynced(t, space, b.Event{ - Type: spaceChildEventType, + Type: msc1772SpaceChildEventType, StateKey: &room, Content: map[string]interface{}{ "via": []string{"hs1"}, @@ -163,7 +162,7 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { }, }) alice.SendEventSynced(t, space, b.Event{ - Type: spaceChildEventType, + Type: msc1772SpaceChildEventType, StateKey: &room, Content: map[string]interface{}{ "via": []string{"hs1"}, From dbefc6c358b6215ab8359df2ecb62e65ddfb6a70 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 8 Apr 2021 12:46:45 -0400 Subject: [PATCH 4/9] gofmt --- tests/msc3083_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index b7382361..df024228 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -14,7 +14,7 @@ import ( ) var ( - msc1772SpaceChildEventType = "org.matrix.msc1772.space.child" + msc1772SpaceChildEventType = "org.matrix.msc1772.space.child" ) func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string, expectedHttpCode int) { From d76431297becf09aff6fe940db08f61d674ea67c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 9 Apr 2021 08:20:17 -0400 Subject: [PATCH 5/9] Fix typos. --- tests/msc3083_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index df024228..3cbd3a67 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -125,7 +125,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { }, }, ) - // Fails since a fully invalid allow key rquires an invite. + // Fails since a fully invalid allow key requires an invite. FailJoinRoom(bob, t, room, "hs1", 403) } @@ -183,7 +183,7 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { bob.LeaveRoom(t, space) FailJoinRoom(bob, t, room, "hs1", 400) - // Invite the user and joining shuold work. + // Invite the user and joining should work. alice.InviteRoom(t, room, "@bob:hs2") bob.JoinRoom(t, room, []string{"hs1"}) @@ -223,6 +223,6 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { }, }, ) - // Fails since a fully invalid allow key rquires an invite. + // Fails since a fully invalid allow key requires an invite. FailJoinRoom(bob, t, room, "hs1", 400) } From 10842efe5730dc1c21c72bd1a4f04ac394778b4d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 9 Apr 2021 08:54:42 -0400 Subject: [PATCH 6/9] Abstract some code. --- tests/msc3083_test.go | 58 ++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 3cbd3a67..1af05939 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -11,6 +11,7 @@ import ( "github.com/matrix-org/complement/internal/b" "github.com/matrix-org/complement/internal/client" + "github.com/matrix-org/complement/internal/docker" ) var ( @@ -32,12 +33,10 @@ func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverNam ) } -// Test joining a room with join rules restricted to membership in a space. -func TestRestrictedRoomsLocalJoin(t *testing.T) { - deployment := Deploy(t, "msc3083_local", b.BlueprintOneToOneRoom) - defer deployment.Destroy(t) - - // Create the space and put a room in it. +// 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) { alice := deployment.Client(t, "hs1", "@alice:hs1") space := alice.CreateRoom(t, map[string]interface{}{ "preset": "public_chat", @@ -72,6 +71,17 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { }, }) + return alice, space, room +} + +// Test joining a room with join rules restricted to membership in a space. +func TestRestrictedRoomsLocalJoin(t *testing.T) { + deployment := Deploy(t, "msc3083_local", b.BlueprintOneToOneRoom) + defer deployment.Destroy(t) + + // Setup the user, space, and restricted room. + alice, space, room := SetupRestrictedRoom(t, deployment) + // Create a second user and attempt to join the room, it should fail. bob := deployment.Client(t, "hs1", "@bob:hs1") FailJoinRoom(bob, t, room, "hs1", 403) @@ -134,40 +144,8 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { deployment := Deploy(t, "msc3083_remote", b.BlueprintFederationOneToOneRoom) defer deployment.Destroy(t) - // Create the space and put a room in it. - alice := deployment.Client(t, "hs1", "@alice:hs1") - space := alice.CreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "name": "Space", - }) - // 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: msc1772SpaceChildEventType, - StateKey: &room, - Content: map[string]interface{}{ - "via": []string{"hs1"}, - }, - }) + // Setup the user, space, and restricted room. + alice, space, room := SetupRestrictedRoom(t, deployment) // Create a second user and attempt to join the room, it should fail. bob := deployment.Client(t, "hs2", "@bob:hs2") From 12e388ec61a5a170f75f99bbd027afe09a3dc625 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 15 Apr 2021 08:53:20 -0400 Subject: [PATCH 7/9] Update errors over federation to 403. --- tests/msc3083_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 1af05939..b91794cf 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -149,8 +149,7 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { // Create a second user and attempt to join the room, it should fail. bob := deployment.Client(t, "hs2", "@bob:hs2") - // Note that this is a 400 error because it comes back over federation. - FailJoinRoom(bob, t, room, "hs1", 400) + FailJoinRoom(bob, t, room, "hs1", 403) // Join the space, attempt to join the room again, which now should succeed. bob.JoinRoom(t, space, []string{"hs1"}) @@ -159,7 +158,7 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { // 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", 400) + FailJoinRoom(bob, t, room, "hs1", 403) // Invite the user and joining should work. alice.InviteRoom(t, room, "@bob:hs2") @@ -186,7 +185,7 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { }, ) // Fails since invalid values get filtered out of allow. - FailJoinRoom(bob, t, room, "hs1", 400) + FailJoinRoom(bob, t, room, "hs1", 403) alice.SendEventSynced( t, @@ -202,5 +201,5 @@ func TestRestrictedRoomsRemoteJoin(t *testing.T) { }, ) // Fails since a fully invalid allow key requires an invite. - FailJoinRoom(bob, t, room, "hs1", 400) + FailJoinRoom(bob, t, room, "hs1", 403) } From 2a262a688a4d43693c565ebacca9323b8db35f90 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 15 Apr 2021 10:12:33 -0400 Subject: [PATCH 8/9] Share code between local and remote tests. --- tests/msc3083_test.go | 82 ++++++++++--------------------------------- 1 file changed, 19 insertions(+), 63 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index b91794cf..0944dbd5 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -74,16 +74,7 @@ func SetupRestrictedRoom(t *testing.T, deployment *docker.Deployment) (*client.C return alice, space, room } -// Test joining a room with join rules restricted to membership in a space. -func TestRestrictedRoomsLocalJoin(t *testing.T) { - deployment := Deploy(t, "msc3083_local", b.BlueprintOneToOneRoom) - defer deployment.Destroy(t) - - // Setup the user, space, and restricted room. - alice, space, room := SetupRestrictedRoom(t, deployment) - - // Create a second user and attempt to join the room, it should fail. - bob := deployment.Client(t, "hs1", "@bob:hs1") +func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, space string, room string) { FailJoinRoom(bob, t, room, "hs1", 403) // Join the space, attempt to join the room again, which now should succeed. @@ -96,7 +87,7 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { FailJoinRoom(bob, t, room, "hs1", 403) // Invite the user and joining should work. - alice.InviteRoom(t, room, "@bob:hs1") + alice.InviteRoom(t, room, bob.UserID) bob.JoinRoom(t, room, []string{"hs1"}) // Leave the room again, and join the space. @@ -140,66 +131,31 @@ func TestRestrictedRoomsLocalJoin(t *testing.T) { } // Test joining a room with join rules restricted to membership in a space. -func TestRestrictedRoomsRemoteJoin(t *testing.T) { - deployment := Deploy(t, "msc3083_remote", b.BlueprintFederationOneToOneRoom) +func TestRestrictedRoomsLocalJoin(t *testing.T) { + deployment := Deploy(t, "msc3083_local", b.BlueprintOneToOneRoom) defer deployment.Destroy(t) // Setup the user, space, and restricted room. alice, space, room := SetupRestrictedRoom(t, deployment) - // Create a second user and attempt to join the room, it should fail. - bob := deployment.Client(t, "hs2", "@bob:hs2") - FailJoinRoom(bob, t, room, "hs1", 403) - - // Join the space, attempt to join the room again, which now should succeed. - bob.JoinRoom(t, space, []string{"hs1"}) - bob.JoinRoom(t, room, []string{"hs1"}) + // Create a second user on the same homeserver. + bob := deployment.Client(t, "hs1", "@bob:hs1") - // 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", 403) + // Execute the checks. + CheckRestrictedRoom(t, alice, bob, space, room) +} - // Invite the user and joining should work. - alice.InviteRoom(t, room, "@bob:hs2") - bob.JoinRoom(t, room, []string{"hs1"}) +// Test joining a room with join rules restricted to membership in a space. +func TestRestrictedRoomsRemoteJoin(t *testing.T) { + deployment := Deploy(t, "msc3083_remote", b.BlueprintFederationOneToOneRoom) + defer deployment.Destroy(t) - // Leave the room again, and join the space. - bob.LeaveRoom(t, room) - bob.JoinRoom(t, space, []string{"hs1"}) + // Setup the user, space, and restricted room. + alice, space, room := SetupRestrictedRoom(t, deployment) - // Update the room to have bad values in the "allow" field, which should stop - // joining from working properly. - emptyStateKey := "" - alice.SendEventSynced( - t, - room, - b.Event{ - Type: "m.room.join_rules", - Sender: alice.UserID, - StateKey: &emptyStateKey, - Content: map[string]interface{}{ - "join_rule": "restricted", - "allow": []string{"invalid"}, - }, - }, - ) - // Fails since invalid values get filtered out of allow. - FailJoinRoom(bob, t, room, "hs1", 403) + // Create a second user on a different homeserver. + bob := deployment.Client(t, "hs2", "@bob:hs2") - alice.SendEventSynced( - t, - room, - b.Event{ - Type: "m.room.join_rules", - Sender: alice.UserID, - StateKey: &emptyStateKey, - Content: map[string]interface{}{ - "join_rule": "restricted", - "allow": "invalid", - }, - }, - ) - // Fails since a fully invalid allow key requires an invite. - FailJoinRoom(bob, t, room, "hs1", 403) + // Execute the checks. + CheckRestrictedRoom(t, alice, bob, space, room) } From 95247272e84ea9aefafc977e7736f00e5628c78a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 15 Apr 2021 10:21:58 -0400 Subject: [PATCH 9/9] Simplify code. --- tests/msc3083_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/msc3083_test.go b/tests/msc3083_test.go index 0944dbd5..3ddb0449 100644 --- a/tests/msc3083_test.go +++ b/tests/msc3083_test.go @@ -18,7 +18,7 @@ var ( msc1772SpaceChildEventType = "org.matrix.msc1772.space.child" ) -func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverName string, expectedHttpCode int) { +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) @@ -29,7 +29,7 @@ func FailJoinRoom(c *client.CSAPI, t *testing.T, roomIDOrAlias string, serverNam nil, "application/json", query, - expectedHttpCode, + 403, ) } @@ -75,7 +75,7 @@ func SetupRestrictedRoom(t *testing.T, deployment *docker.Deployment) (*client.C } func CheckRestrictedRoom(t *testing.T, alice *client.CSAPI, bob *client.CSAPI, space string, room string) { - FailJoinRoom(bob, t, room, "hs1", 403) + FailJoinRoom(bob, t, room, "hs1") // Join the space, attempt to join the room again, which now should succeed. bob.JoinRoom(t, space, []string{"hs1"}) @@ -84,7 +84,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", 403) + FailJoinRoom(bob, t, room, "hs1") // Invite the user and joining should work. alice.InviteRoom(t, room, bob.UserID) @@ -111,7 +111,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", 403) + FailJoinRoom(bob, t, room, "hs1") alice.SendEventSynced( t, @@ -127,7 +127,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", 403) + FailJoinRoom(bob, t, room, "hs1") } // Test joining a room with join rules restricted to membership in a space.