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

.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

@@ -108,6 +108,21 @@ func (c *CSAPI) InviteRoom(t *testing.T, roomID string, userID string) {
 	c.MustDo(t, "POST", []string{"_matrix", "client", "r0", "rooms", roomID, "invite"}, body)
 }
 
+// SearchUserDirectory makes a request to search the user directory with a given query.
+// We don't include an explicit limit on the number of results returned, relying on
+// the spec's default of 10.
+func (c *CSAPI) SearchUserDirectory(t *testing.T, query string) *http.Response {
+	t.Helper()
+	return c.MustDoFunc(
+		t,
+		"POST",
+		[]string{"_matrix", "client", "r0", "user_directory", "search"},
+		WithJSONBody(t, map[string]interface{}{
+			"search_term": query,
+		}),
+	)
+}
+
 // SendEventSynced sends `e` into the room and waits for its event ID to come down /sync.
 // Returns the event ID of the sent event.
 func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string {
@@ -127,15 +142,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 +247,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/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,124 @@
+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"
+)
+
+func TestRoomSpecificUsernameHandling(t *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:
+	// - Even knows about Alice,
+	// - Alice reveals a private name to another friend Bob
+	// - Eve shouldn't be able to see that private name.
+	deployment := Deploy(t, b.BlueprintAlice)
+	defer deployment.Destroy(t)
+
+	alice := deployment.Client(t, "hs1", "@alice:hs1")
+	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 creates a public room (so Eve can see that Alice exists)
+	alice.CreateRoom(t, map[string]interface{}{"visibility": "public"})
+
+	// 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:hs1"},
+		client.WithJSONBody(t, map[string]interface{}{
+			"displayname": "Alice Cooper",
+			"membership":  "join",
+		}),
+	)
+
+	justAliceByPublicName := []match.JSON{
+		match.JSONKeyArrayOfSize("results", 1),
+		match.JSONKeyEqual("results.0.display_name", "alice"),
+		match.JSONKeyEqual("results.0.user_id", alice.UserID),
+	}
+
+	t.Run("Searcher can find target by display name "+
+		"when searcher and target have no room in common, share a homeserver, and "+
+		"target is in a public room on that homeserver",
+		func(t *testing.T) {
+			res := eve.SearchUserDirectory(t, "alice")
+			must.MatchResponse(t, res, match.HTTPResponse{JSON: justAliceByPublicName})
+		})
+
+	t.Run("Searcher can find target by mxid "+
+		"when searcher and target have no room in common, share a homeserver, and "+
+		"target is in a public room on that homeserver",
+		func(t *testing.T) {
+			res := eve.SearchUserDirectory(t, alice.UserID)
+			must.MatchResponse(t, res, match.HTTPResponse{JSON: justAliceByPublicName})
+		})
+
+	noResults := []match.JSON{
+		match.JSONKeyArrayOfSize("results", 0),
+	}
+
+	t.Run("Searcher cannot find target by room-specific name they are not privy to "+
+		"when searcher and target have no room in common, share a homeserver, and "+
+		"target is in a public room on that homeserver",
+		func(t *testing.T) {
+			res := eve.SearchUserDirectory(t, "Alice Cooper")
+			must.MatchResponse(t, res, match.HTTPResponse{JSON: noResults})
+		})
+
+	justAliceByPublicOrPrivateName := []match.JSON{
+		match.JSONKeyArrayOfSize("results", 1),
+		// TODO should bob find alice by her public or private name?
+		match.AnyOf(
+			match.JSONKeyEqual("results.0.display_name", "Alice Cooper"),
+			match.JSONKeyEqual("results.0.display_name", "alice"),
+		),
+		match.JSONKeyEqual("results.0.user_id", alice.UserID),
+	}
+
+	t.Run("Searcher can find target by public display name "+
+		"when searcher and target share a private room with a specific display_name for the target",
+		func(t *testing.T) {
+			res := bob.SearchUserDirectory(t, "alice")
+			must.MatchResponse(t, res, match.HTTPResponse{
+				JSON: justAliceByPublicOrPrivateName,
+			})
+		})
+
+	t.Run("Searcher can find target by mxid "+
+		"when searcher and target share a private room with a specific display_name for the target",
+		func(t *testing.T) {
+			res := bob.SearchUserDirectory(t, alice.UserID)
+			must.MatchResponse(t, res, match.HTTPResponse{
+				JSON: justAliceByPublicOrPrivateName,
+			})
+		})
+
+	t.Run("Searcher can find target by room-specific name"+
+		"when searcher and target share a private room with a specific display_name for the target",
+		func(t *testing.T) {
+			res := bob.SearchUserDirectory(t, "Alice Cooper")
+			must.MatchResponse(t, res, match.HTTPResponse{
+				JSON: justAliceByPublicOrPrivateName,
+			})
+		})
+}