Skip to content

Add tests for /make_knock and /send_knock (same as the join counterparts) for when the federation server is in the middle of a partial join #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
reivilibre merged 7 commits into from
Sep 5, 2022
+176 −7
Merged

Add tests for /make_knock and /send_knock (same as the join counterparts) for when the federation server is in the middle of a partial join #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2f3a7f6
Drive-by correction
reivilibre Aug 8, 2022
bf76071
Add test for /make_knock and /send_knock
reivilibre Aug 8, 2022
f23be67
Add needed MakeRespMakeKnock
reivilibre Aug 12, 2022
f766d51
Fixes
reivilibre Aug 18, 2022
b196ba4
Update gomatrixserverlib
reivilibre Aug 18, 2022
127095f
Merge branch 'main' into rei/frj_send_knock_during_partial_join
reivilibre Aug 30, 2022
6ef95e0
Update gomatrixserverlib
reivilibre Aug 31, 2022
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
12 changes: 6 additions & 6 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ require (
github.com/gogo/protobuf v1.3.2 // indirect
github.com/gorilla/mux v1.8.0
github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16
github.com/matrix-org/gomatrixserverlib v0.0.0-20220815094957-74b7ff4ae09c
github.com/matrix-org/gomatrixserverlib v0.0.0-20220830164018-c71e518537a2
github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4
github.com/moby/term v0.0.0-20210610120745-9d4ed1856297 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.0.3-0.20211202183452-c5a74bcca799 // indirect
github.com/sirupsen/logrus v1.8.1
github.com/tidwall/gjson v1.14.1
github.com/tidwall/sjson v1.2.4
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect
github.com/sirupsen/logrus v1.9.0
github.com/tidwall/gjson v1.14.3
github.com/tidwall/sjson v1.2.5
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90 // indirect
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6 // indirect
golang.org/x/image v0.0.0-20220413100746-70e8d0d3baa9 // indirect
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2 // indirect
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 // indirect
golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
gonum.org/v1/plot v0.11.0
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
Expand Down
19 changes: 19 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5d
github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220815094957-74b7ff4ae09c h1:GhKmb8s9iXA9qsFD1SbiRo6Ee7cnbfcgJQ/iy43wczM=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220815094957-74b7ff4ae09c/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220818102127-cb703928745b h1:AjPdx/kCWB3uuABDITUEP3+nolku/6zgwqsmL2JEQ8Y=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220818102127-cb703928745b/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220818155954-baceb7ed8c5f h1:dVCrkyX3cJCF6pc0WPI4YjTQu5qpfcSxbFSqQJ4kK/0=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220818155954-baceb7ed8c5f/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220830164018-c71e518537a2 h1:esbNn9hg//tAStA6TogatAJAursw23A+yfVRQsdiv70=
github.com/matrix-org/gomatrixserverlib v0.0.0-20220830164018-c71e518537a2/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk=
github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U=
github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 h1:eCEHXWDv9Rm335MSuB49mFUK44bwZPFSDde3ORE3syk=
github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U=
Expand Down Expand Up @@ -107,22 +113,30 @@ github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6Mwd
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0=
github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/tidwall/gjson v1.12.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.14.1 h1:iymTbGkQBhveq21bEvAQ81I0LEBork8BFe1CUZXdyuo=
github.com/tidwall/gjson v1.14.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.14.3 h1:9jvXn7olKEHU1S9vwoMGliaT8jq1vJ7IH/n9zD9Dnlw=
github.com/tidwall/gjson v1.14.3/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tidwall/sjson v1.2.4 h1:cuiLzLnaMeBhRmEv00Lpk3tkYrcxpmbU81tAY4Dw0tc=
github.com/tidwall/sjson v1.2.4/go.mod h1:098SZ494YoMWPmMO6ct4dcFnqxwj9r/gF0Etp19pSNM=
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
Expand All @@ -137,6 +151,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
golang.org/x/crypto v0.0.0-20220513210258-46612604a0f9/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e h1:T8NU3HyQ8ClP4SEE+KbFlg6n0NhuTsN4MyznaarGsZM=
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90 h1:Y/gsMcFOcR+6S6f3YeMKl5g+dZMEWqcz5Czj/GWYbkM=
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -203,6 +219,9 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY=
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
31 changes: 31 additions & 0 deletions internal/federation/handle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,37 @@ func MakeRespMakeJoin(s *Server, room *ServerRoom, userID string) (resp gomatrix
return
}

// MakeRespMakeKnock makes the response for a /make_knock request, without verifying any signatures
// or dealing with HTTP responses itself.
func MakeRespMakeKnock(s *Server, room *ServerRoom, userID string) (resp gomatrixserverlib.RespMakeKnock, err error) {
// Generate a knock event
builder := gomatrixserverlib.EventBuilder{
Sender: userID,
RoomID: room.RoomID,
Type: "m.room.member",
StateKey: &userID,
PrevEvents: []string{room.Timeline[len(room.Timeline)-1].EventID()},
Depth: room.Timeline[len(room.Timeline)-1].Depth() + 1,
}
err = builder.SetContent(map[string]interface{}{"membership": gomatrixserverlib.Join})
if err != nil {
err = fmt.Errorf("make_knock cannot set membership content: %w", err)
return
}
stateNeeded, err := gomatrixserverlib.StateNeededForEventBuilder(&builder)
if err != nil {
err = fmt.Errorf("make_knock cannot calculate auth_events: %w", err)
return
}
builder.AuthEvents = room.AuthEvents(stateNeeded)

resp = gomatrixserverlib.RespMakeKnock{
RoomVersion: room.Version,
KnockEvent: builder,
}
return
Copy link
Copy Markdown
Contributor

@DMRobertson DMRobertson Sep 5, 2022

Choose a reason for hiding this comment

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

Are we implicitly returning resp, err here?

Copy link
Copy Markdown
Contributor Author

@reivilibre reivilibre Sep 5, 2022

Choose a reason for hiding this comment

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

yeah, I find this weird but Go supports this old-school Pascal-ish style of assigning to a result variable rather than returning, except the result variable's names are declared in the function declaration.
I nicked this from what was already there; it's 'not my choice' but it seems like this may be idiomatic

}

// SendJoinRequestsHandler is the http.Handler implementation for the send_join part of
// HandleMakeSendJoinRequests.
//
Expand Down
121 changes: 120 additions & 1 deletion tests/federation_room_join_partial_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ func TestPartialStateJoin(t *testing.T) {
t.Fatalf("MakeRespMakeJoin failed : %s", err)
}

// charlie then tries to /send_join via the homeserver under test
// daniel then tries to /send_join via the homeserver under test
joinEvent, err := makeJoinResp.JoinEvent.Build(time.Now(), gomatrixserverlib.ServerName(testServer2.ServerName()), testServer2.KeyID, testServer2.Priv, makeJoinResp.RoomVersion)
must.NotError(t, "JoinEvent.Build", err)

Expand Down Expand Up @@ -1229,6 +1229,125 @@ func TestPartialStateJoin(t *testing.T) {
},
})
})

// when the server is in the middle of a partial state join, it should not accept
// /make_knock because it can't give a full answer.
t.Run("Rejects make_knock during partial join", func(t *testing.T) {
// In this test, we have 3 homeservers:
// hs1 (the server under test) with @alice:hs1
// This is the server that will be in the middle of a partial join.
// testServer1 (a Complement test server) with @bob:<server name>
// This is the server that created the room originally.
// testServer2 (another Complement test server) with @charlie:<server name>
// This is the server that will try to make a knock via testServer1.
deployment := Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)
alice := deployment.Client(t, "hs1", "@alice:hs1")

testServer1 := createTestServer(t, deployment)
cancel := testServer1.Listen()
defer cancel()
serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t))
roomID := serverRoom.RoomID
psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice)
defer psjResult.Destroy()

// The partial join is now in progress.
// Let's have a new test server rock up and ask to join the room by making a
// /make_knock request.

testServer2 := createTestServer(t, deployment)
cancel2 := testServer2.Listen()
defer cancel2()

fedClient2 := testServer2.FederationClient(deployment)

// charlie sends a make_knock
_, err := fedClient2.MakeKnock(context.Background(), "hs1", roomID, testServer2.UserID("charlie"), federation.SupportedRoomVersions())

if err == nil {
t.Errorf("MakeKnock returned 200, want 404")
} else if httpError, ok := err.(gomatrix.HTTPError); ok {
t.Logf("MakeKnock => %d/%s", httpError.Code, string(httpError.Contents))
if httpError.Code != 404 {
t.Errorf("expected 404, got %d", httpError.Code)
}
errcode := must.GetJSONFieldStr(t, httpError.Contents, "errcode")
if errcode != "M_NOT_FOUND" {
t.Errorf("errcode: got %s, want M_NOT_FOUND", errcode)
}
} else {
t.Errorf("MakeKnock: non-HTTPError: %v", err)
}
})

// when the server is in the middle of a partial state join, it should not accept
// /send_knock because it can't give a full answer.
t.Run("Rejects send_knock during partial join", func(t *testing.T) {
// In this test, we have 3 homeservers:
// hs1 (the server under test) with @alice:hs1
// This is the server that will be in the middle of a partial join.
// testServer1 (a Complement test server) with @charlie:<server name>
// This is the server that will create the room originally.
// testServer2 (another Complement test server) with @daniel:<server name>
// This is the server that will try to knock on the room via hs2,
// but only after using hs1 to /make_knock (as otherwise we have no way
// of being able to build a request to /send_knock)
//
deployment := Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)
alice := deployment.Client(t, "hs1", "@alice:hs1")

testServer1 := createTestServer(t, deployment)
cancel := testServer1.Listen()
defer cancel()
serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t))
psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice)
defer psjResult.Destroy()

// hs1's partial join is now in progress.
// Let's have a test server rock up and ask to /send_knock in the room via hs1.
// To do that, we need to /make_knock first.
// Asking hs1 to /make_knock won't work, because it should reject that request.
// To work around that, we /make_knock via hs2.

testServer2 := createTestServer(t, deployment)
cancel2 := testServer2.Listen()
defer cancel2()

fedClient2 := testServer2.FederationClient(deployment)

// Manually /make_knock via testServer1.
// This is permissible because testServer1 is fully joined to the room.
// We can't actually use /make_knock because host.docker.internal doesn't resolve,
// so compute it without making any requests:
makeKnockResp, err := federation.MakeRespMakeKnock(testServer1, serverRoom, testServer2.UserID("daniel"))
if err != nil {
t.Fatalf("MakeRespMakeKnock failed : %s", err)
}

// daniel then tries to /send_knock via the homeserver under test
knockEvent, err := makeKnockResp.KnockEvent.Build(time.Now(), gomatrixserverlib.ServerName(testServer2.ServerName()), testServer2.KeyID, testServer2.Priv, makeKnockResp.RoomVersion)
must.NotError(t, "KnockEvent.Build", err)
Copy link
Copy Markdown
Contributor

@DMRobertson DMRobertson Sep 5, 2022

Choose a reason for hiding this comment

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

is this just shorthand for if err != nil { /* fail the test with given message */ } ?

Copy link
Copy Markdown
Contributor Author

@reivilibre reivilibre Sep 5, 2022

Choose a reason for hiding this comment

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

yeah, seems to be

// NotError will ensure `err` is nil else terminate the test with `msg`.
func NotError(t *testing.T, msg string, err error) {
	t.Helper()
	if err != nil {
		t.Fatalf("MustNotError: %s -> %s", msg, err)
	}
}


// SendKnock should return a 404 because the homeserver under test has not
// finished its partial join.
_, err = fedClient2.SendKnock(context.Background(), "hs1", knockEvent)
if err == nil {
t.Errorf("SendKnock returned 200, want 404")
} else if httpError, ok := err.(gomatrix.HTTPError); ok {
t.Logf("SendKnock => %d/%s", httpError.Code, string(httpError.Contents))
if httpError.Code != 404 {
t.Errorf("expected 404, got %d", httpError.Code)
}
errcode := must.GetJSONFieldStr(t, httpError.Contents, "errcode")
if errcode != "M_NOT_FOUND" {
t.Errorf("errcode: got %s, want M_NOT_FOUND", errcode)
}
} else {
t.Errorf("SendKnock: non-HTTPError: %v", err)
}
})
}

// test reception of an event over federation during a resync
Expand Down