Fix `/get_missing_events` for rooms with `joined`/`invited` history_visibility (#2787)

Sytest was using a wrong `history_visibility` for `invited`
(https://github.com/matrix-org/sytest/pull/1303), so `invited` was
passing for the wrong reason (-> defaulted to `shared`, as `invite`
wasn't understood).
This change now handles missing events like Synapse, if a server isn't
allowed to see the event, it gets a redacted version of it, making the
`get_missing_events` tests pass.

7 files changed, 33 insertions, 34 deletions

diff --git a/are-we-synapse-yet.list b/are-we-synapse-yet.list
index c776a740..81c0f804 100644
--- a/are-we-synapse-yet.list
+++ b/are-we-synapse-yet.list
@@ -643,7 +643,7 @@ fed Inbound federation redacts events from erased users
 fme Outbound federation can request missing events 
 fme Inbound federation can return missing events for world_readable visibility 
 fme Inbound federation can return missing events for shared visibility 
-fme Inbound federation can return missing events for invite visibility 
+fme Inbound federation can return missing events for invited visibility
 fme Inbound federation can return missing events for joined visibility 
 fme outliers whose auth_events are in a different room are correctly rejected 
 fbk Outbound federation can backfill events 
diff --git a/roomserver/auth/auth.go b/roomserver/auth/auth.go
index aa1d5bc2..31a856e8 100644
--- a/roomserver/auth/auth.go
+++ b/roomserver/auth/auth.go
@@ -13,8 +13,6 @@
 package auth
 
 import (
-	"encoding/json"
-
 	"github.com/matrix-org/gomatrixserverlib"
 )
 
@@ -30,7 +28,7 @@ func IsServerAllowed(
 	historyVisibility := HistoryVisibilityForRoom(authEvents)
 
 	// 1. If the history_visibility was set to world_readable, allow.
-	if historyVisibility == "world_readable" {
+	if historyVisibility == gomatrixserverlib.HistoryVisibilityWorldReadable {
 		return true
 	}
 	// 2. If the user's membership was join, allow.
@@ -39,12 +37,12 @@ func IsServerAllowed(
 		return true
 	}
 	// 3. If history_visibility was set to shared, and the user joined the room at any point after the event was sent, allow.
-	if historyVisibility == "shared" && serverCurrentlyInRoom {
+	if historyVisibility == gomatrixserverlib.HistoryVisibilityShared && serverCurrentlyInRoom {
 		return true
 	}
 	// 4. If the user's membership was invite, and the history_visibility was set to invited, allow.
 	invitedUserExists := IsAnyUserOnServerWithMembership(serverName, authEvents, gomatrixserverlib.Invite)
-	if invitedUserExists && historyVisibility == "invited" {
+	if invitedUserExists && historyVisibility == gomatrixserverlib.HistoryVisibilityInvited {
 		return true
 	}
 
@@ -52,27 +50,16 @@ func IsServerAllowed(
 	return false
 }
 
-func HistoryVisibilityForRoom(authEvents []*gomatrixserverlib.Event) string {
+func HistoryVisibilityForRoom(authEvents []*gomatrixserverlib.Event) gomatrixserverlib.HistoryVisibility {
 	// https://matrix.org/docs/spec/client_server/r0.6.0#id87
 	// By default if no history_visibility is set, or if the value is not understood, the visibility is assumed to be shared.
-	visibility := "shared"
-	knownStates := []string{"invited", "joined", "shared", "world_readable"}
+	visibility := gomatrixserverlib.HistoryVisibilityShared
 	for _, ev := range authEvents {
 		if ev.Type() != gomatrixserverlib.MRoomHistoryVisibility {
 			continue
 		}
-		// TODO: This should be HistoryVisibilityContent to match things like 'MemberContent'. Do this when moving to GMSL
-		content := struct {
-			HistoryVisibility string `json:"history_visibility"`
-		}{}
-		if err := json.Unmarshal(ev.Content(), &content); err != nil {
-			break // value is not understood
-		}
-		for _, s := range knownStates {
-			if s == content.HistoryVisibility {
-				visibility = s
-				break
-			}
+		if vis, err := ev.HistoryVisibility(); err == nil {
+			visibility = vis
 		}
 	}
 	return visibility
@@ -80,6 +67,9 @@ func HistoryVisibilityForRoom(authEvents []*gomatrixserverlib.Event) string {
 
 func IsAnyUserOnServerWithMembership(serverName gomatrixserverlib.ServerName, authEvents []*gomatrixserverlib.Event, wantMembership string) bool {
 	for _, ev := range authEvents {
+		if ev.Type() != gomatrixserverlib.MRoomMember {
+			continue
+		}
 		membership, err := ev.Membership()
 		if err != nil || membership != wantMembership {
 			continue
diff --git a/roomserver/internal/helpers/helpers.go b/roomserver/internal/helpers/helpers.go
index 3b83a0a6..a6de8ac8 100644
--- a/roomserver/internal/helpers/helpers.go
+++ b/roomserver/internal/helpers/helpers.go
@@ -324,7 +324,7 @@ func slowGetHistoryVisibilityState(
 func ScanEventTree(
 	ctx context.Context, db storage.Database, info *types.RoomInfo, front []string, visited map[string]bool, limit int,
 	serverName gomatrixserverlib.ServerName,
-) ([]types.EventNID, error) {
+) ([]types.EventNID, map[string]struct{}, error) {
 	var resultNIDs []types.EventNID
 	var err error
 	var allowed bool
@@ -345,6 +345,7 @@ func ScanEventTree(
 
 	var checkedServerInRoom bool
 	var isServerInRoom bool
+	redactEventIDs := make(map[string]struct{})
 
 	// Loop through the event IDs to retrieve the requested events and go
 	// through the whole tree (up to the provided limit) using the events'
@@ -358,7 +359,7 @@ BFSLoop:
 		// Retrieve the events to process from the database.
 		events, err = db.EventsFromIDs(ctx, front)
 		if err != nil {
-			return resultNIDs, err
+			return resultNIDs, redactEventIDs, err
 		}
 
 		if !checkedServerInRoom && len(events) > 0 {
@@ -395,16 +396,16 @@ BFSLoop:
 						)
 						// drop the error, as we will often error at the DB level if we don't have the prev_event itself. Let's
 						// just return what we have.
-						return resultNIDs, nil
+						return resultNIDs, redactEventIDs, nil
 					}
 
 					// If the event hasn't been seen before and the HS
 					// requesting to retrieve it is allowed to do so, add it to
 					// the list of events to retrieve.
-					if allowed {
-						next = append(next, pre)
-					} else {
+					next = append(next, pre)
+					if !allowed {
 						util.GetLogger(ctx).WithField("server", serverName).WithField("event_id", pre).Info("Not allowed to see event")
+						redactEventIDs[pre] = struct{}{}
 					}
 				}
 			}
@@ -413,7 +414,7 @@ BFSLoop:
 		front = next
 	}
 
-	return resultNIDs, err
+	return resultNIDs, redactEventIDs, err
 }
 
 func QueryLatestEventsAndState(
diff --git a/roomserver/internal/perform/perform_backfill.go b/roomserver/internal/perform/perform_backfill.go
index 69a07573..57e121ea 100644
--- a/roomserver/internal/perform/perform_backfill.go
+++ b/roomserver/internal/perform/perform_backfill.go
@@ -78,7 +78,7 @@ func (r *Backfiller) PerformBackfill(
 	}
 
 	// Scan the event tree for events to send back.
-	resultNIDs, err := helpers.ScanEventTree(ctx, r.DB, info, front, visited, request.Limit, request.ServerName)
+	resultNIDs, redactEventIDs, err := helpers.ScanEventTree(ctx, r.DB, info, front, visited, request.Limit, request.ServerName)
 	if err != nil {
 		return err
 	}
@@ -95,6 +95,9 @@ func (r *Backfiller) PerformBackfill(
 	}
 
 	for _, event := range loadedEvents {
+		if _, ok := redactEventIDs[event.EventID()]; ok {
+			event.Redact()
+		}
 		response.Events = append(response.Events, event.Headered(info.RoomVersion))
 	}
 
diff --git a/roomserver/internal/query/query.go b/roomserver/internal/query/query.go
index 7a424a33..f4113240 100644
--- a/roomserver/internal/query/query.go
+++ b/roomserver/internal/query/query.go
@@ -453,7 +453,7 @@ func (r *Queryer) QueryMissingEvents(
 		return fmt.Errorf("missing RoomInfo for room %s", events[0].RoomID())
 	}
 
-	resultNIDs, err := helpers.ScanEventTree(ctx, r.DB, info, front, visited, request.Limit, request.ServerName)
+	resultNIDs, redactEventIDs, err := helpers.ScanEventTree(ctx, r.DB, info, front, visited, request.Limit, request.ServerName)
 	if err != nil {
 		return err
 	}
@@ -470,7 +470,9 @@ func (r *Queryer) QueryMissingEvents(
 			if verr != nil {
 				return verr
 			}
-
+			if _, ok := redactEventIDs[event.EventID()]; ok {
+				event.Redact()
+			}
 			response.Events = append(response.Events, event.Headered(roomVersion))
 		}
 	}
diff --git a/roomserver/storage/postgres/state_snapshot_table.go b/roomserver/storage/postgres/state_snapshot_table.go
index 99c76bef..a00c026f 100644
--- a/roomserver/storage/postgres/state_snapshot_table.go
+++ b/roomserver/storage/postgres/state_snapshot_table.go
@@ -21,10 +21,11 @@ import (
 	"fmt"
 
 	"github.com/lib/pq"
+	"github.com/matrix-org/util"
+
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/roomserver/storage/tables"
 	"github.com/matrix-org/dendrite/roomserver/types"
-	"github.com/matrix-org/util"
 )
 
 const stateSnapshotSchema = `
@@ -91,6 +92,7 @@ const bulkSelectStateForHistoryVisibilitySQL = `
 	      WHERE state_snapshot_nid = $1
 	    )
 	  )
+	  ORDER BY depth ASC
 	) AS roomserver_events
 	INNER JOIN roomserver_event_state_keys
 	  ON roomserver_events.event_state_key_nid = roomserver_event_state_keys.event_state_key_nid
diff --git a/sytest-whitelist b/sytest-whitelist
index 9ba9df75..a3218ed7 100644
--- a/sytest-whitelist
+++ b/sytest-whitelist
@@ -306,7 +306,7 @@ Alternative server names do not cause a routing loop
 Events whose auth_events are in the wrong room do not mess up the room state
 Inbound federation can return events
 Inbound federation can return missing events for world_readable visibility
-Inbound federation can return missing events for invite visibility
+Inbound federation can return missing events for invited visibility
 Inbound federation can get public room list
 PUT /rooms/:room_id/redact/:event_id/:txn_id as power user redacts message
 PUT /rooms/:room_id/redact/:event_id/:txn_id as original message sender redacts message
@@ -742,4 +742,5 @@ User in private room doesn't appear in user directory
 User joining then leaving public room appears and dissappears from directory
 User in remote room doesn't appear in user directory after server left room
 User in shared private room does appear in user directory until leave
-Existing members see new member's presence
\ No newline at end of file
+Existing members see new member's presence
+Inbound federation can return missing events for joined visibility
\ No newline at end of file