Merge branch 'main' into neilalexander/v064

clientapi/routing/state.go

@@ -235,7 +235,7 @@ func OnIncomingStateTypeRequest(
 		}
 		// If the user has never been in the room then stop at this point.
 		// We won't tell the user about a room they have never joined.
-		if !membershipRes.HasBeenInRoom {
+		if !membershipRes.HasBeenInRoom || membershipRes.Membership == gomatrixserverlib.Ban {
 			return util.JSONResponse{
 				Code: http.StatusForbidden,
 				JSON: jsonerror.Forbidden(fmt.Sprintf("Unknown room %q or user %q has never joined this room", roomID, device.UserID)),

federationapi/internal/perform.go

@@ -554,7 +554,6 @@ func (r *FederationInternalAPI) PerformInvite(
 	if err != nil {
 		return fmt.Errorf("r.federation.SendInviteV2: failed to send invite: %w", err)
 	}
-	logrus.Infof("GOT INVITE RESPONSE %s", string(inviteRes.Event))
 
 	inviteEvent, err := inviteRes.Event.UntrustedEvent(request.RoomVersion)
 	if err != nil {

keyserver/api/api.go

@@ -15,6 +15,7 @@
 package api
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"strings"
@@ -73,6 +74,26 @@ type DeviceMessage struct {
 	DeviceChangeID int64
 }
 
+// DeviceKeysEqual returns true if the device keys updates contain the
+// same display name and key JSON. This will return false if either of
+// the updates is not a device keys update, or if the user ID/device ID
+// differ between the two.
+func (m1 *DeviceMessage) DeviceKeysEqual(m2 *DeviceMessage) bool {
+	if m1.DeviceKeys == nil || m2.DeviceKeys == nil {
+		return false
+	}
+	if m1.UserID != m2.UserID || m1.DeviceID != m2.DeviceID {
+		return false
+	}
+	if m1.DisplayName != m2.DisplayName {
+		return false // different display names
+	}
+	if len(m1.KeyJSON) == 0 || len(m2.KeyJSON) == 0 {
+		return false // either is empty
+	}
+	return bytes.Equal(m1.KeyJSON, m2.KeyJSON)
+}
+
 // DeviceKeys represents a set of device keys for a single device
 // https://matrix.org/docs/spec/client_server/r0.6.1#post-matrix-client-r0-keys-upload
 type DeviceKeys struct {

keyserver/internal/device_list_update.go

@@ -241,14 +241,33 @@ func (u *DeviceListUpdater) update(ctx context.Context, event gomatrixserverlib.
 				StreamID: event.StreamID,
 			},
 		}
+
+		// DeviceKeysJSON will side-effect modify this, so it needs
+		// to be a copy, not sharing any pointers with the above.
+		deviceKeysCopy := *keys[0].DeviceKeys
+		deviceKeysCopy.KeyJSON = nil
+		existingKeys := []api.DeviceMessage{
+			{
+				Type:       keys[0].Type,
+				DeviceKeys: &deviceKeysCopy,
+				StreamID:   keys[0].StreamID,
+			},
+		}
+
+		// fetch what keys we had already and only emit changes
+		if err = u.db.DeviceKeysJSON(ctx, existingKeys); err != nil {
+			// non-fatal, log and continue
+			util.GetLogger(ctx).WithError(err).WithField("user_id", event.UserID).Errorf(
+				"failed to query device keys json for calculating diffs",
+			)
+		}
+
 		err = u.db.StoreRemoteDeviceKeys(ctx, keys, nil)
 		if err != nil {
 			return false, fmt.Errorf("failed to store remote device keys for %s (%s): %w", event.UserID, event.DeviceID, err)
 		}
-		// ALWAYS emit key changes when we've been poked over federation even if there's no change
-		// just in case this poke is important for something.
-		err = u.producer.ProduceKeyChanges(keys)
-		if err != nil {
+
+		if err = emitDeviceKeyChanges(u.producer, existingKeys, keys); err != nil {
 			return false, fmt.Errorf("failed to produce device key changes for %s (%s): %w", event.UserID, event.DeviceID, err)
 		}
 		return false, nil

keyserver/internal/internal.go

@@ -718,7 +718,7 @@ func emitDeviceKeyChanges(producer KeyChangeProducer, existing, new []api.Device
 		for _, existingKey := range existing {
 			// Do not treat the absence of keys as equal, or else we will not emit key changes
 			// when users delete devices which never had a key to begin with as both KeyJSONs are nil.
-			if bytes.Equal(existingKey.KeyJSON, newKey.KeyJSON) && len(existingKey.KeyJSON) > 0 {
+			if existingKey.DeviceKeysEqual(&newKey) {
 				exists = true
 				break
 			}

roomserver/storage/shared/storage.go

@@ -13,6 +13,7 @@ import (
 	"github.com/matrix-org/dendrite/roomserver/types"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
+	"github.com/sirupsen/logrus"
 	"github.com/tidwall/gjson"
 )
 
@@ -1101,7 +1102,7 @@ func (d *Database) JoinedUsersSetInRooms(ctx context.Context, roomIDs []string)
 		return nil, err
 	}
 	if len(nidToUserID) != len(userNIDToCount) {
-		return nil, fmt.Errorf("found %d users but only have state key nids for %d of them", len(userNIDToCount), len(nidToUserID))
+		logrus.Warnf("SelectJoinedUsersSetForRooms found %d users but BulkSelectEventStateKey only returned state key NIDs for %d of them", len(userNIDToCount), len(nidToUserID))
 	}
 	result := make(map[string]int, len(userNIDToCount))
 	for nid, count := range userNIDToCount {

syncapi/routing/context.go

@@ -64,7 +64,7 @@ func Context(
 	ctx := req.Context()
 	membershipRes := roomserver.QueryMembershipForUserResponse{}
 	membershipReq := roomserver.QueryMembershipForUserRequest{UserID: device.UserID, RoomID: roomID}
-	if err := rsAPI.QueryMembershipForUser(ctx, &membershipReq, &membershipRes); err != nil {
+	if err = rsAPI.QueryMembershipForUser(ctx, &membershipReq, &membershipRes); err != nil {
 		logrus.WithError(err).Error("unable to fo membership")
 		return jsonerror.InternalServerError()
 	}
@@ -86,11 +86,12 @@ func Context(
 	state, _ := syncDB.CurrentState(ctx, roomID, &stateFilter, nil)
 	// verify the user is allowed to see the context for this room/event
 	for _, x := range state {
-		hisVis, err := x.HistoryVisibility()
+		var hisVis string
+		hisVis, err = x.HistoryVisibility()
 		if err != nil {
 			continue
 		}
-		allowed := hisVis != "world_readable" && membershipRes.Membership == "join"
+		allowed := hisVis == gomatrixserverlib.WorldReadable || membershipRes.Membership == gomatrixserverlib.Join
 		if !allowed {
 			return util.JSONResponse{
 				Code: http.StatusForbidden,

sytest-whitelist

@@ -597,3 +597,4 @@ Device list doesn't change if remote server is down
 /context/ on non world readable room does not work
 /context/ returns correct number of events
 /context/ with lazy_load_members filter works
+Remote banned user is kicked and may not rejoin until unbanned