diff --git a/.golangci.yml b/.golangci.yml index 38038f32..0961ec9f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -438,6 +438,11 @@ issues: - G107 # gosec: Error on TLS InsecureSkipVerify set to true. - G402 + # staticcheck: Using a deprecated function, variable, constant or field + # Tried to make this a warning rather than error in the severity section. + # I failed. But it's nice to have goland know that certain things are deprecated + # so it can strike them through. + - SA1019 # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: diff --git a/internal/client/client.go b/internal/client/client.go index ed3b174f..04556088 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -127,7 +127,10 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { return eventID } -// SyncUntilTimelineHas blocks and continually calls /sync until the `check` function returns true. +// SyncUntilTimelineHas is a wrapper around `SyncUntil`. +// It blocks and continually calls `/sync` until +// - we have joined the given room +// - we see an event in the room for which the `check` function returns True // If the `check` function fails the test, the failing event will be automatically logged. // Will time out after CSAPI.SyncUntilTimeout. func (c *CSAPI) SyncUntilTimelineHas(t *testing.T, roomID string, check func(gjson.Result) bool) { @@ -135,7 +138,23 @@ func (c *CSAPI) SyncUntilTimelineHas(t *testing.T, roomID string, check func(gjs c.SyncUntil(t, "", "", "rooms.join."+GjsonEscape(roomID)+".timeline.events", check) } -// SyncUntil blocks and continually calls /sync until the `check` function returns true. +// SyncUntilInvitedTo is a wrapper around SyncUntil. +// It blocks and continually calls `/sync` until we've been invited to the given room. +// Will time out after CSAPI.SyncUntilTimeout. +func (c *CSAPI) SyncUntilInvitedTo(t *testing.T, roomID string) { + t.Helper() + check := func(event gjson.Result) bool { + return event.Get("type").Str == "m.room.member" && + event.Get("content.membership").Str == "invite" && + event.Get("state_key").Str == c.UserID + } + c.SyncUntil(t, "", "", "rooms.invite."+GjsonEscape(roomID)+".invite_state.events", check) +} + +// SyncUntil blocks and continually calls /sync until +// - the response contains a particular `key`, and +// - its corresponding value is an array +// - some element in that array makes the `check` function return true. // If the `check` function fails the test, the failing event will be automatically logged. // Will time out after CSAPI.SyncUntilTimeout. func (c *CSAPI) SyncUntil(t *testing.T, since, filter, key string, check func(gjson.Result) bool) { @@ -213,6 +232,9 @@ func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, } // MustDo will do the HTTP request and fail the test if the response is not 2xx +// +// Deprecated: Prefer MustDoFunc. MustDo is the older format which doesn't allow for vargs +// and will be removed in the future. MustDoFunc also logs HTTP response bodies on error. func (c *CSAPI) MustDo(t *testing.T, method string, paths []string, jsonBody interface{}) *http.Response { t.Helper() res := c.DoFunc(t, method, paths, WithJSONBody(t, jsonBody)) diff --git a/internal/instruction/runner.go b/internal/instruction/runner.go index 4635d5da..20ec4900 100644 --- a/internal/instruction/runner.go +++ b/internal/instruction/runner.go @@ -289,6 +289,9 @@ func calculateUserInstructionSets(r *Runner, hs b.Homeserver) [][]instruction { instrs = append(instrs, instructionLogin(hs, user)) } else { instrs = append(instrs, instructionRegister(hs, user)) + if user.DisplayName != "" { + instrs = append(instrs, instructionDisplayName(hs, user)) + } } createdUsers[user.Localpart] = true @@ -440,6 +443,21 @@ func instructionRegister(hs b.Homeserver, user b.User) instruction { } } +func instructionDisplayName(hs b.Homeserver, user b.User) instruction { + body := map[string]interface{}{ + "displayname": user.DisplayName, + } + return instruction{ + method: "PUT", + path: fmt.Sprintf( + "/_matrix/client/r0/profile/@%s:%s/displayname", + user.Localpart, hs.Name, + ), + accessToken: fmt.Sprintf("user_@%s:%s", user.Localpart, hs.Name), + body: body, + } +} + func instructionLogin(hs b.Homeserver, user b.User) instruction { body := map[string]interface{}{ "type": "m.login.password", diff --git a/internal/match/json.go b/internal/match/json.go index 84432ce5..b61190fa 100644 --- a/internal/match/json.go +++ b/internal/match/json.go @@ -3,6 +3,7 @@ package match import ( "fmt" "reflect" + "strings" "github.com/tidwall/gjson" ) @@ -17,7 +18,7 @@ type JSON func(body []byte) error func JSONKeyEqual(wantKey string, wantValue interface{}) JSON { return func(body []byte) error { res := gjson.GetBytes(body, wantKey) - if res.Index == 0 { + if !res.Exists() { return fmt.Errorf("key '%s' missing", wantKey) } gotValue := res.Value() @@ -55,6 +56,26 @@ func JSONKeyTypeEqual(wantKey string, wantType gjson.Type) JSON { } } +// JSONKeyArrayOfSize returns a matcher which will check that `wantKey` is present and +// its value is an array with the given size. +// `wantKey` can be nested, see https://godoc.org/github.com/tidwall/gjson#Get for details. +func JSONKeyArrayOfSize(wantKey string, wantSize int) JSON { + return func(body []byte) error { + res := gjson.GetBytes(body, wantKey) + if !res.Exists() { + return fmt.Errorf("key '%s' missing", wantKey) + } + if !res.IsArray() { + return fmt.Errorf("key '%s' is not an array", wantKey) + } + entries := res.Array() + if len(entries) != wantSize { + return fmt.Errorf("key '%s' is an array of the wrong size, got %v want %v", wantKey, len(entries), wantSize) + } + return nil + } +} + func jsonCheckOffInternal(wantKey string, wantItems []interface{}, allowUnwantedItems bool, mapper func(gjson.Result) interface{}, fn func(interface{}, gjson.Result) error) JSON { return func(body []byte) error { res := gjson.GetBytes(body, wantKey) @@ -204,3 +225,29 @@ func JSONMapEach(wantKey string, fn func(k, v gjson.Result) error) JSON { return err } } + +// AnyOf takes 1 or more `checkers`, and builds a new checker which accepts a given +// json body iff it's accepted by at least one of the original `checkers`. +func AnyOf(checkers ...JSON) JSON { + return func(body []byte) error { + if len(checkers) == 0 { + return fmt.Errorf("must provide at least one checker to AnyOf") + } + + errors := make([]error, len(checkers)) + for i, check := range checkers { + errors[i] = check(body) + if errors[i] == nil { + return nil + } + } + + builder := strings.Builder{} + builder.WriteString("all checks failed:") + for _, err := range errors { + builder.WriteString("\n ") + builder.WriteString(err.Error()) + } + return fmt.Errorf(builder.String()) + } +} diff --git a/tests/csapi/user_directory_display_names_test.go b/tests/csapi/user_directory_display_names_test.go new file mode 100644 index 00000000..fee4fdc6 --- /dev/null +++ b/tests/csapi/user_directory_display_names_test.go @@ -0,0 +1,187 @@ +// +build !dendrite_blacklist + +// Rationale for being included in Dendrite's blacklist: https://github.com/matrix-org/complement/pull/199#issuecomment-904852233 +package csapi_tests + +import ( + "testing" + + "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/client" + "github.com/matrix-org/complement/internal/match" + "github.com/matrix-org/complement/internal/must" +) + +const aliceUserID = "@alice:hs1" +const alicePublicName = "Alice Cooper" +const alicePrivateName = "Freddy" + +var justAliceByPublicName = []match.JSON{ + match.JSONKeyArrayOfSize("results", 1), + match.JSONKeyEqual("results.0.display_name", alicePublicName), + match.JSONKeyEqual("results.0.user_id", aliceUserID), +} + +var noResults = []match.JSON{ + match.JSONKeyArrayOfSize("results", 0), +} + +func setupUsers(t *testing.T) (*client.CSAPI, *client.CSAPI, *client.CSAPI, func(*testing.T)) { + // Originally written to reproduce https://github.com/matrix-org/synapse/issues/5677 + // In that bug report, + // - Bob knows about Alice, and + // - Alice has revealed a private name to another friend X, + // - Bob can see that private name when he shouldn't be able to. + // + // I've tweaked the names to be more traditional: + // - Eve knows about Alice, + // - Alice reveals a private name to another friend Bob + // - Eve shouldn't be able to see that private name via the directory. + deployment := Deploy(t, b.BlueprintAlice) + cleanup := func(t *testing.T) { + deployment.Destroy(t) + } + + alice := deployment.Client(t, "hs1", aliceUserID) + bob := deployment.RegisterUser(t, "hs1", "bob", "bob-has-a-very-secret-pw") + eve := deployment.RegisterUser(t, "hs1", "eve", "eve-has-a-very-secret-pw") + + // Alice sets her profile displayname. This ensures that her + // public name, private name and userid localpart are all + // distinguishable, even case-insensitively. + alice.MustDoFunc( + t, + "PUT", + []string{"_matrix", "client", "r0", "profile", alice.UserID, "displayname"}, + client.WithJSONBody(t, map[string]interface{}{ + "displayname": alicePublicName, + }), + ) + + // Alice creates a public room (so when Eve searches, she can see that Alice exists) + alice.CreateRoom(t, map[string]interface{}{"visibility": "public"}) + return alice, bob, eve, cleanup +} + +func checkExpectations(t *testing.T, bob, eve *client.CSAPI) { + t.Run("Eve can find Alice by profile display name", func(t *testing.T) { + res := eve.MustDoFunc( + t, + "POST", + []string{"_matrix", "client", "r0", "user_directory", "search"}, + client.WithJSONBody(t, map[string]interface{}{ + "search_term": alicePublicName, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{JSON: justAliceByPublicName}) + }) + + t.Run("Eve can find Alice by mxid", func(t *testing.T) { + res := eve.MustDoFunc( + t, + "POST", + []string{"_matrix", "client", "r0", "user_directory", "search"}, + client.WithJSONBody(t, map[string]interface{}{ + "search_term": aliceUserID, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{JSON: justAliceByPublicName}) + }) + + t.Run("Eve cannot find Alice by room-specific name that Eve is not privy to", func(t *testing.T) { + res := eve.MustDoFunc( + t, + "POST", + []string{"_matrix", "client", "r0", "user_directory", "search"}, + client.WithJSONBody(t, map[string]interface{}{ + "search_term": alicePrivateName, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{JSON: noResults}) + }) + + t.Run("Bob can find Alice by profile display name", func(t *testing.T) { + res := bob.MustDoFunc( + t, + "POST", + []string{"_matrix", "client", "r0", "user_directory", "search"}, + client.WithJSONBody(t, map[string]interface{}{ + "search_term": alicePublicName, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: justAliceByPublicName, + }) + }) + + t.Run("Bob can find Alice by mxid", func(t *testing.T) { + res := bob.MustDoFunc( + t, + "POST", + []string{"_matrix", "client", "r0", "user_directory", "search"}, + client.WithJSONBody(t, map[string]interface{}{ + "search_term": aliceUserID, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: justAliceByPublicName, + }) + }) +} + +func TestRoomSpecificUsernameChange(t *testing.T) { + alice, bob, eve, cleanup := setupUsers(t) + defer cleanup(t) + + // Bob creates a new room and invites Alice. + privateRoom := bob.CreateRoom(t, map[string]interface{}{ + "visibility": "private", + "invite": []string{alice.UserID}, + }) + + // Alice waits until she sees the invite, then accepts. + alice.SyncUntilInvitedTo(t, privateRoom) + alice.JoinRoom(t, privateRoom, nil) + + // Alice reveals her private name to Bob + alice.MustDoFunc( + t, + "PUT", + []string{"_matrix", "client", "r0", "rooms", privateRoom, "state", "m.room.member", alice.UserID}, + client.WithJSONBody(t, map[string]interface{}{ + "displayname": alicePrivateName, + "membership": "join", + }), + ) + + checkExpectations(t, bob, eve) +} + +func TestRoomSpecificUsernameAtJoin(t *testing.T) { + alice, bob, eve, cleanup := setupUsers(t) + defer cleanup(t) + + // Bob creates a new room and invites Alice. + privateRoom := bob.CreateRoom(t, map[string]interface{}{ + "visibility": "private", + "invite": []string{alice.UserID}, + }) + + // Alice waits until she sees the invite, then accepts. + // When she accepts, she does so with a specific displayname. + alice.SyncUntilInvitedTo(t, privateRoom) + alice.JoinRoom(t, privateRoom, nil) + + // Alice reveals her private name to Bob + alice.MustDoFunc( + t, + "PUT", + []string{"_matrix", "client", "r0", "rooms", privateRoom, "state", "m.room.member", alice.UserID}, + client.WithJSONBody(t, map[string]interface{}{ + "displayname": alicePrivateName, + "membership": "join", + }), + ) + + checkExpectations(t, bob, eve) +}