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.

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 

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

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 {
+					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(

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

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

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

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
@@ -743,3 +743,4 @@ 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
+Inbound federation can return missing events for joined visibility