Add test case reproducing matrix-org/synapse#5677 for local users (#199)

* Mark MustDo as Deprecated
* Introduce SyncUntilInvitedTo
* match: use rs.Exists() in JSONKeyEqual, for consistency with other matchers
* match: matcher that seeks an array of a fixed size
* Introduce `AnyOf` matcher
* Fix PUT call to set displayname

Author: David Robertson

.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:

internal/client/client.go

@@ -127,15 +127,34 @@ 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) {
 	t.Helper()
 	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))

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",

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())
+	}
+}

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)
+}