Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions tests/knock_restricted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,13 @@ func TestRestrictedRoomsRemoteJoinLocalUserInMSC3787Room(t *testing.T) {
func TestRestrictedRoomsRemoteJoinFailOverInMSC3787Room(t *testing.T) {
doTestRestrictedRoomsRemoteJoinFailOver(t, roomVersion, joinRule)
}

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`
func TestKnockRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12(t *testing.T) {
doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "knock_restricted")
}

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`
func TestKnockRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV11(t *testing.T) {
doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "11", "knock_restricted")
}
102 changes: 102 additions & 0 deletions tests/restricted_rooms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,105 @@ func doTestRestrictedRoomsRemoteJoinFailOver(t *testing.T, roomVersion string, j
return true
}))
}

// A homeserver should be able to do a local join (when someone else from the same
// homeserver is already joined to the room) to a room using any local user who has
// invite power levels.
//
// In the test case, there are no local room creators on hs2, so hs2 will need to use
// one of the local people listed in the power levels who can invite. While hs2, could
// do another remote join to get charlie in the room, we assert it was a local join by
// checking the `join_authorised_via_users_server` (homeservers should prefer a local
// join).
//
// This is a regression test for Synapse as it previously only looked for local room
// creators in v12 rooms, https://github.com/element-hq/synapse/issues/19120
func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12(t *testing.T) {
doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted")
}

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`
func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV11(t *testing.T) {
doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "11", "restricted")
}

func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the reason other tests are written this way is so that it can be shared with tests/knock_restricted_test.go. I'm guessing we should also have knock_restricted test there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(needs change)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added knock variants in 3cb69dd. Do you want them all to have docstrings or is TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12's just being within view fine?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can say this:

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`

deployment := complement.Deploy(t, 2)
defer deployment.Destroy(t)
// Create the room
alice, allowed_room, room := setupRestrictedRoom(t, deployment, roomVersion, joinRule)
// Create two users on the other homeserver.
bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{})
charlie := deployment.Register(t, "hs2", helpers.RegistrationOpts{})

// Bob joins the allowed room.
bob.JoinRoom(t, allowed_room, []spec.ServerName{
deployment.GetFullyQualifiedHomeserverName(t, "hs1"),
})
// Bob joins the restricted room. This join should go remotely
// and consequently be authorised by Alice (on hs1) as she is the only
// member.
bob.JoinRoom(t, room, []spec.ServerName{
deployment.GetFullyQualifiedHomeserverName(t, "hs1"),
})
// Ensure the join was authorised by alice
bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, room, func(ev gjson.Result) bool {
must.MatchGJSON(t, ev,
match.JSONKeyEqual("content.join_authorised_via_users_server", alice.UserID),
)
return true
}))

// Alice restricts the invite power level to moderators and promotes Bob to
// moderator.
state_key := ""
if roomVersion == "12" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this works but doesn't feel the best given future room versions will come along. I guess we will leave the refactor until then. And most of these tests should be running across most of the room versions anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a more proper way to do it let me know, I'm used to something like mautrix's RoomVersion type which has feature flags, but I'm not familiar with complement's codebase all that much and I'm equally short on time so I haven't looked to see if there's a more appropriate solution

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a good strategy to address this kind of thing in Complement yet.

We can leave it to future refactors of the the tests that make them run across all room versions.

// Alice is a creator and cannot appear in the power levels
alice.SendEventSynced(t, room, b.Event{
Type: "m.room.power_levels",
StateKey: &state_key,
Content: map[string]interface{}{
"invite": 50,
"users": map[string]interface{}{
bob.UserID: 50,
},
},
})
} else {
// rooms <v12 need alice to be in the power levels to retain power
alice.SendEventSynced(t, room, b.Event{
Type: "m.room.power_levels",
StateKey: &state_key,
Content: map[string]interface{}{
"invite": 50,
"users": map[string]interface{}{
alice.UserID: 100,
bob.UserID: 50,
},
},
})
}

// Charlie joins the allowed room.
charlie.JoinRoom(t, allowed_room, []spec.ServerName{
deployment.GetFullyQualifiedHomeserverName(t, "hs1"),
})
// Charlie attempts to join the restricted room.
// hs2 should use bob to authorise the join as he is a local user with
// invite power levels.
// If the server did not correctly detect that bob could issue an invite,
// this join would instead be a remote join authorised via @alice:hs1.
charlie.JoinRoom(t, room, []spec.ServerName{
deployment.GetFullyQualifiedHomeserverName(t, "hs2"),
})

// Ensure the join was authorised by bob. The join should not be
// authorised by alice as hs2 should not have attempted a remote
// join given bob is a local user that can authorise the join.
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", bob.UserID),
)
return true
}))
}
Loading