From b672915ba0ab5e1212fc627c774a742f840cf4f4 Mon Sep 17 00:00:00 2001
From: Kegan Dougal <kegan@matrix.org>
Date: Fri, 31 Jul 2020 10:28:07 +0100
Subject: [PATCH] Fix New users appear in /keys/changes

---
 syncapi/internal/keychange.go | 33 +++++++++++++++++++++++++++++++++
 syncapi/sync/requestpool.go   |  2 +-
 sytest-whitelist              |  1 +
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/syncapi/internal/keychange.go b/syncapi/internal/keychange.go
index cb4fca7d8..0272ffc06 100644
--- a/syncapi/internal/keychange.go
+++ b/syncapi/internal/keychange.go
@@ -16,6 +16,7 @@ package internal
 
 import (
 	"context"
+	"strings"
 
 	"github.com/Shopify/sarama"
 	currentstateAPI "github.com/matrix-org/dendrite/currentstateserver/api"
@@ -88,6 +89,16 @@ func DeviceListCatchup(
 		if !userSet[userID] {
 			res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID)
 			hasNew = true
+			userSet[userID] = true
+		}
+	}
+	// if the response has any join/leave events, add them now.
+	// TODO: This is sub-optimal because we will add users to `changed` even if we already shared a room with them.
+	for _, userID := range membershipEvents(res) {
+		if !userSet[userID] {
+			res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID)
+			hasNew = true
+			userSet[userID] = true
 		}
 	}
 	return hasNew, nil
@@ -219,3 +230,25 @@ func membershipEventPresent(events []gomatrixserverlib.ClientEvent, userID strin
 	}
 	return false
 }
+
+// returns the user IDs of anyone joining or leaving a room in this response. These users will be added to
+// the 'changed' property because of https://matrix.org/docs/spec/client_server/r0.6.1#id84
+// "For optimal performance, Alice should be added to changed in Bob's sync only when she adds a new device,
+// or when Alice and Bob now share a room but didn't share any room previously. However, for the sake of simpler
+// logic, a server may add Alice to changed when Alice and Bob share a new room, even if they previously already shared a room."
+func membershipEvents(res *types.Response) (userIDs []string) {
+	for _, room := range res.Rooms.Join {
+		for _, ev := range room.Timeline.Events {
+			if ev.Type == gomatrixserverlib.MRoomMember && ev.StateKey != nil {
+				if strings.Contains(string(ev.Content), `"join"`) {
+					userIDs = append(userIDs, *ev.StateKey)
+				} else if strings.Contains(string(ev.Content), `"leave"`) {
+					userIDs = append(userIDs, *ev.StateKey)
+				} else if strings.Contains(string(ev.Content), `"ban"`) {
+					userIDs = append(userIDs, *ev.StateKey)
+				}
+			}
+		}
+	}
+	return
+}
diff --git a/syncapi/sync/requestpool.go b/syncapi/sync/requestpool.go
index f817f0981..b530b34d1 100644
--- a/syncapi/sync/requestpool.go
+++ b/syncapi/sync/requestpool.go
@@ -168,7 +168,7 @@ func (rp *RequestPool) OnIncomingKeyChangeRequest(req *http.Request, device *use
 	}
 	// work out room joins/leaves
 	res, err := rp.db.IncrementalSync(
-		req.Context(), types.NewResponse(), *device, fromToken, toToken, 0, false,
+		req.Context(), types.NewResponse(), *device, fromToken, toToken, 10, false,
 	)
 	if err != nil {
 		util.GetLogger(req.Context()).WithError(err).Error("Failed to IncrementalSync")
diff --git a/sytest-whitelist b/sytest-whitelist
index 03baf4d44..c90bcc3ad 100644
--- a/sytest-whitelist
+++ b/sytest-whitelist
@@ -129,6 +129,7 @@ Can claim one time key using POST
 Can claim remote one time key using POST
 Local device key changes appear in v2 /sync
 Local device key changes appear in /keys/changes
+New users appear in /keys/changes
 Local delete device changes appear in v2 /sync
 Get left notifs for other users in sync and /keys/changes when user leaves
 Can add account data