From 1a954ab6f4b946bc6efd6c7615729e7f2b5b0cb9 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 19 Nov 2021 13:46:19 +0000 Subject: [PATCH 1/2] Add ability to skip individual tests - Added `runtime.SkipIf` - Re-combined federation test as it can now be individually skipped on Dendrite. From a CI perspective, no changes are required as this uses the existing build tags setup to detect HS impl. --- runtime/hs.go | 34 +++++++ runtime/hs_dendrite.go | 7 ++ runtime/hs_synapse.go | 7 ++ tests/federation_room_join_test.go | 55 +++++++++++ ...join_with_unverifiable_auth_events_test.go | 91 ------------------- 5 files changed, 103 insertions(+), 91 deletions(-) create mode 100644 runtime/hs.go create mode 100644 runtime/hs_dendrite.go create mode 100644 runtime/hs_synapse.go delete mode 100644 tests/federation_room_join_with_unverifiable_auth_events_test.go diff --git a/runtime/hs.go b/runtime/hs.go new file mode 100644 index 00000000..2c616347 --- /dev/null +++ b/runtime/hs.go @@ -0,0 +1,34 @@ +package runtime + +import "testing" + +const ( + Dendrite = "dendrite" + Synapse = "synapse" +) + +var Homeserver string + +// Skip the test (via t.Skipf) if the homeserver being tested matches `hs`, else return. +// +// The homeserver being tested is detected via the presence of a `*_blacklist` tag e.g: +// go test -tags="dendrite_blacklist" +// This means it is important to always specify this tag when running tests. Failure to do +// so will result in a warning being printed to stdout, and the test will be run. When a new server +// implementation is added, a respective `hs_$name.go` needs to be created in this directory. This +// file pairs together the tag name with a string constant declared in this package +// e.g. dendrite_blacklist == runtime.Dendrite +func SkipIf(t *testing.T, hs string) { + t.Helper() + if Homeserver == hs { + t.Skipf("skipped on %s", hs) + } + if Homeserver == "" { + // they ran Complement without a blacklist so it's impossible to know what HS they are + // running, warn them. + t.Logf( + "WARNING: %s called runtime.SkipIf(%s) but Complement doesn't know which HS is running as it was run without a *_blacklist tag: executing test.", + t.Name(), hs, + ) + } +} diff --git a/runtime/hs_dendrite.go b/runtime/hs_dendrite.go new file mode 100644 index 00000000..f908c3e4 --- /dev/null +++ b/runtime/hs_dendrite.go @@ -0,0 +1,7 @@ +// +build dendrite_blacklist + +package runtime + +func init() { + Homeserver = Dendrite +} diff --git a/runtime/hs_synapse.go b/runtime/hs_synapse.go new file mode 100644 index 00000000..326c6dc5 --- /dev/null +++ b/runtime/hs_synapse.go @@ -0,0 +1,7 @@ +// +build synapse_blacklist + +package runtime + +func init() { + Homeserver = Synapse +} diff --git a/tests/federation_room_join_test.go b/tests/federation_room_join_test.go index 0e32bee5..79051050 100644 --- a/tests/federation_room_join_test.go +++ b/tests/federation_room_join_test.go @@ -20,6 +20,7 @@ import ( "github.com/matrix-org/complement/internal/federation" "github.com/matrix-org/complement/internal/match" "github.com/matrix-org/complement/internal/must" + "github.com/matrix-org/complement/runtime" ) // This tests that joining a room with ?server_name= works correctly. @@ -196,6 +197,60 @@ func TestJoinFederatedRoomWithUnverifiableEvents(t *testing.T) { alice := deployment.Client(t, "hs1", "@alice:hs1") alice.JoinRoom(t, roomAlias, nil) }) + t.Run("/send_join response with state with unverifiable auth events shouldn't block room join", func(t *testing.T) { + runtime.SkipIf(t, runtime.Dendrite) // https://github.com/matrix-org/dendrite/issues/2028 + room := srv.MustMakeRoom(t, ver, federation.InitialRoomEvents(ver, charlie)) + roomAlias := srv.MakeAliasMapping("UnverifiableAuthEvents", room.RoomID) + + // create a normal event then modify the signatures + rawEvent := srv.MustCreateEvent(t, room, b.Event{ + Sender: charlie, + StateKey: &charlie, + Type: "m.room.member", + Content: map[string]interface{}{ + "membership": "join", + "name": "This event has a bad signature", + }, + }).JSON() + rawSig, err := json.Marshal(map[string]interface{}{ + docker.HostnameRunningComplement: map[string]string{ + string(srv.KeyID): "/3z+pJjiJXWhwfqIEzmNksvBHCoXTktK/y0rRuWJXw6i1+ygRG/suDCKhFuuz6gPapRmEMPVILi2mJqHHXPKAg", + }, + }) + must.NotError(t, "failed to marshal bad signature block", err) + rawEvent, err = sjson.SetRawBytes(rawEvent, "signatures", rawSig) + must.NotError(t, "failed to modify signatures key from event", err) + badlySignedEvent, err := gomatrixserverlib.NewEventFromTrustedJSON(rawEvent, false, ver) + must.NotError(t, "failed to make Event from badly signed event JSON", err) + room.AddEvent(badlySignedEvent) + t.Logf("Created badly signed auth event %s", badlySignedEvent.EventID()) + + // and now add another event which will use it as an auth event. + goodEvent := srv.MustCreateEvent(t, room, b.Event{ + Sender: charlie, + StateKey: &charlie, + Type: "m.room.member", + Content: map[string]interface{}{ + "membership": "leave", + }, + }) + // double-check that the bad event is in its auth events + containsEvent := false + for _, authEventID := range goodEvent.AuthEventIDs() { + if authEventID == badlySignedEvent.EventID() { + containsEvent = true + break + } + } + if !containsEvent { + t.Fatalf("Bad event didn't appear in auth events of state event") + } + room.AddEvent(goodEvent) + t.Logf("Created state event %s", goodEvent.EventID()) + + alice := deployment.Client(t, "hs1", "@alice:hs1") + alice.JoinRoom(t, roomAlias, nil) + }) } // This test checks that users cannot circumvent the auth checks via send_join. diff --git a/tests/federation_room_join_with_unverifiable_auth_events_test.go b/tests/federation_room_join_with_unverifiable_auth_events_test.go deleted file mode 100644 index 6868b548..00000000 --- a/tests/federation_room_join_with_unverifiable_auth_events_test.go +++ /dev/null @@ -1,91 +0,0 @@ -// +build !dendrite_blacklist - -package tests - -import ( - "encoding/json" - "testing" - - "github.com/matrix-org/gomatrixserverlib" - "github.com/tidwall/sjson" - - "github.com/matrix-org/complement/internal/b" - "github.com/matrix-org/complement/internal/docker" - "github.com/matrix-org/complement/internal/federation" - "github.com/matrix-org/complement/internal/must" -) - -// This test is an extension of TestJoinFederatedRoomWithUnverifiableEvents. -// It's currently split out, because it fails on Dendrite -// - see https://github.com/matrix-org/dendrite/issues/2028 -func TestJoinFederatedRoomWithUnverifiableAuthEvents(t *testing.T) { - deployment := Deploy(t, b.BlueprintAlice) - defer deployment.Destroy(t) - - srv := federation.NewServer(t, deployment, - federation.HandleKeyRequests(), - federation.HandleMakeSendJoinRequests(), - federation.HandleTransactionRequests(nil, nil), - ) - srv.UnexpectedRequestsAreErrors = false - cancel := srv.Listen() - defer cancel() - - ver := gomatrixserverlib.RoomVersionV6 - charlie := srv.UserID("charlie") - - t.Run("/send_join response with state with unverifiable auth events shouldn't block room join", func(t *testing.T) { - //t.Parallel() - room := srv.MustMakeRoom(t, ver, federation.InitialRoomEvents(ver, charlie)) - roomAlias := srv.MakeAliasMapping("UnverifiableAuthEvents", room.RoomID) - - // create a normal event then modify the signatures - rawEvent := srv.MustCreateEvent(t, room, b.Event{ - Sender: charlie, - StateKey: &charlie, - Type: "m.room.member", - Content: map[string]interface{}{ - "membership": "join", - "name": "This event has a bad signature", - }, - }).JSON() - rawSig, err := json.Marshal(map[string]interface{}{ - docker.HostnameRunningComplement: map[string]string{ - string(srv.KeyID): "/3z+pJjiJXWhwfqIEzmNksvBHCoXTktK/y0rRuWJXw6i1+ygRG/suDCKhFuuz6gPapRmEMPVILi2mJqHHXPKAg", - }, - }) - must.NotError(t, "failed to marshal bad signature block", err) - rawEvent, err = sjson.SetRawBytes(rawEvent, "signatures", rawSig) - must.NotError(t, "failed to modify signatures key from event", err) - badlySignedEvent, err := gomatrixserverlib.NewEventFromTrustedJSON(rawEvent, false, ver) - must.NotError(t, "failed to make Event from badly signed event JSON", err) - room.AddEvent(badlySignedEvent) - t.Logf("Created badly signed auth event %s", badlySignedEvent.EventID()) - - // and now add another event which will use it as an auth event. - goodEvent := srv.MustCreateEvent(t, room, b.Event{ - Sender: charlie, - StateKey: &charlie, - Type: "m.room.member", - Content: map[string]interface{}{ - "membership": "leave", - }, - }) - // double-check that the bad event is in its auth events - containsEvent := false - for _, authEventID := range goodEvent.AuthEventIDs() { - if authEventID == badlySignedEvent.EventID() { - containsEvent = true - break - } - } - if !containsEvent { - t.Fatalf("Bad event didn't appear in auth events of state event") - } - room.AddEvent(goodEvent) - t.Logf("Created state event %s", goodEvent.EventID()) - - alice := deployment.Client(t, "hs1", "@alice:hs1") - alice.JoinRoom(t, roomAlias, nil) - }) -} From 8c60fbe10e5f623d981ea2a82515c9a39c5432e3 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 19 Nov 2021 13:57:15 +0000 Subject: [PATCH 2/2] Update onboarding --- ONBOARDING.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ONBOARDING.md b/ONBOARDING.md index ab25c934..71c4999e 100644 --- a/ONBOARDING.md +++ b/ONBOARDING.md @@ -189,7 +189,19 @@ Normally, server logs are only printed when one of the tests fail. To override t ### How do I skip a test? -Use one of `t.Skipf(...)` or `t.SkipNow()`. +To conditionally skip a *single* test based on the homeserver being run, add a single line at the start of the test: +```go +runtime.SkipIf(t, runtime.Dendrite) +``` +To conditionally skip an entire *file* based on the homeserver being run, add a [build tag](https://pkg.go.dev/cmd/go#hdr-Build_constraints) at the top of the file which will skip execution of all the tests in this file if Complement is run with this flag: +```go +// +build !dendrite_blacklist +``` +You can also do this based on features for MSC tests (which means you must run Complement *with* this tag for these tests *to run*): +```go +// +build msc_2836 +``` +See [GH Actions](https://github.com/matrix-org/complement/blob/master/.github/workflows/ci.yaml) for an example of how this is used for different homeservers in practice. ### Why do we use `t.Errorf` sometimes and `t.Fatalf` other times?