[pseudoID] More pseudo ID fixes (#3167)

Signed-off-by: `Sam Wedgwood <sam@wedgwood.dev>`

4 files changed, 65 insertions, 30 deletions

diff --git a/syncapi/routing/context.go b/syncapi/routing/context.go
index 649d77b4..b0c91c40 100644
--- a/syncapi/routing/context.go
+++ b/syncapi/routing/context.go
@@ -138,7 +138,7 @@ func Context(
 
 	// verify the user is allowed to see the context for this room/event
 	startTime := time.Now()
-	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(ctx, snapshot, rsAPI, []*rstypes.HeaderedEvent{&requestedEvent}, nil, device.UserID, "context")
+	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(ctx, snapshot, rsAPI, []*rstypes.HeaderedEvent{&requestedEvent}, nil, *userID, "context")
 	if err != nil {
 		logrus.WithError(err).Error("unable to apply history visibility filter")
 		return util.JSONResponse{
@@ -176,7 +176,7 @@ func Context(
 	}
 
 	startTime = time.Now()
-	eventsBeforeFiltered, eventsAfterFiltered, err := applyHistoryVisibilityOnContextEvents(ctx, snapshot, rsAPI, eventsBefore, eventsAfter, device.UserID)
+	eventsBeforeFiltered, eventsAfterFiltered, err := applyHistoryVisibilityOnContextEvents(ctx, snapshot, rsAPI, eventsBefore, eventsAfter, *userID)
 	if err != nil {
 		logrus.WithError(err).Error("unable to apply history visibility filter")
 		return util.JSONResponse{
@@ -257,7 +257,7 @@ func Context(
 func applyHistoryVisibilityOnContextEvents(
 	ctx context.Context, snapshot storage.DatabaseTransaction, rsAPI roomserver.SyncRoomserverAPI,
 	eventsBefore, eventsAfter []*rstypes.HeaderedEvent,
-	userID string,
+	userID spec.UserID,
 ) (filteredBefore, filteredAfter []*rstypes.HeaderedEvent, err error) {
 	eventIDsBefore := make(map[string]struct{}, len(eventsBefore))
 	eventIDsAfter := make(map[string]struct{}, len(eventsAfter))
diff --git a/syncapi/routing/getevent.go b/syncapi/routing/getevent.go
index 09c2aef0..4fa282f3 100644
--- a/syncapi/routing/getevent.go
+++ b/syncapi/routing/getevent.go
@@ -37,7 +37,7 @@ import (
 func GetEvent(
 	req *http.Request,
 	device *userapi.Device,
-	roomID string,
+	rawRoomID string,
 	eventID string,
 	cfg *config.SyncAPI,
 	syncDB storage.Database,
@@ -47,7 +47,7 @@ func GetEvent(
 	db, err := syncDB.NewDatabaseTransaction(ctx)
 	logger := util.GetLogger(ctx).WithFields(logrus.Fields{
 		"event_id": eventID,
-		"room_id":  roomID,
+		"room_id":  rawRoomID,
 	})
 	if err != nil {
 		logger.WithError(err).Error("GetEvent: syncDB.NewDatabaseTransaction failed")
@@ -57,6 +57,14 @@ func GetEvent(
 		}
 	}
 
+	roomID, err := spec.NewRoomID(rawRoomID)
+	if err != nil {
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: spec.InvalidParam("invalid room ID"),
+		}
+	}
+
 	events, err := db.Events(ctx, []string{eventID})
 	if err != nil {
 		logger.WithError(err).Error("GetEvent: syncDB.Events failed")
@@ -76,13 +84,22 @@ func GetEvent(
 	}
 
 	// If the request is coming from an appservice, get the user from the request
-	userID := device.UserID
+	rawUserID := device.UserID
 	if asUserID := req.FormValue("user_id"); device.AppserviceID != "" && asUserID != "" {
-		userID = asUserID
+		rawUserID = asUserID
+	}
+
+	userID, err := spec.NewUserID(rawUserID, true)
+	if err != nil {
+		util.GetLogger(req.Context()).WithError(err).Error("invalid device.UserID")
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown("internal server error"),
+		}
 	}
 
 	// Apply history visibility to determine if the user is allowed to view the event
-	events, err = internal.ApplyHistoryVisibilityFilter(ctx, db, rsAPI, events, nil, userID, "event")
+	events, err = internal.ApplyHistoryVisibilityFilter(ctx, db, rsAPI, events, nil, *userID, "event")
 	if err != nil {
 		logger.WithError(err).Error("GetEvent: internal.ApplyHistoryVisibilityFilter failed")
 		return util.JSONResponse{
@@ -101,18 +118,14 @@ func GetEvent(
 		}
 	}
 
-	sender := spec.UserID{}
-	validRoomID, err := spec.NewRoomID(roomID)
-	if err != nil {
+	senderUserID, err := rsAPI.QueryUserIDForSender(req.Context(), *roomID, events[0].SenderID())
+	if err != nil || senderUserID == nil {
+		util.GetLogger(req.Context()).WithError(err).WithField("senderID", events[0].SenderID()).WithField("roomID", *roomID).Error("QueryUserIDForSender errored or returned nil-user ID when user should be part of a room")
 		return util.JSONResponse{
-			Code: http.StatusBadRequest,
-			JSON: spec.BadJSON("roomID is invalid"),
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown("internal server error"),
 		}
 	}
-	senderUserID, err := rsAPI.QueryUserIDForSender(req.Context(), *validRoomID, events[0].SenderID())
-	if err == nil && senderUserID != nil {
-		sender = *senderUserID
-	}
 
 	sk := events[0].StateKey()
 	if sk != nil && *sk != "" {
@@ -131,6 +144,6 @@ func GetEvent(
 	}
 	return util.JSONResponse{
 		Code: http.StatusOK,
-		JSON: synctypes.ToClientEvent(events[0], synctypes.FormatAll, sender, sk),
+		JSON: synctypes.ToClientEvent(events[0], synctypes.FormatAll, *senderUserID, sk),
 	}
 }
diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go
index 23a09544..3333cb54 100644
--- a/syncapi/routing/messages.go
+++ b/syncapi/routing/messages.go
@@ -50,6 +50,7 @@ type messagesReq struct {
 	from             *types.TopologyToken
 	to               *types.TopologyToken
 	device           *userapi.Device
+	deviceUserID     spec.UserID
 	wasToProvided    bool
 	backwardOrdering bool
 	filter           *synctypes.RoomEventFilter
@@ -77,6 +78,15 @@ func OnIncomingMessagesRequest(
 ) util.JSONResponse {
 	var err error
 
+	deviceUserID, err := spec.NewUserID(device.UserID, true)
+	if err != nil {
+		util.GetLogger(req.Context()).WithError(err).Error("device.UserID invalid")
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown("internal server error"),
+		}
+	}
+
 	// NewDatabaseTransaction is used here instead of NewDatabaseSnapshot as we
 	// expect to be able to write to the database in response to a /messages
 	// request that requires backfilling from the roomserver or federation.
@@ -240,6 +250,7 @@ func OnIncomingMessagesRequest(
 		filter:           filter,
 		backwardOrdering: backwardOrdering,
 		device:           device,
+		deviceUserID:     *deviceUserID,
 	}
 
 	clientEvents, start, end, err := mReq.retrieveEvents(req.Context(), rsAPI)
@@ -359,7 +370,7 @@ func (r *messagesReq) retrieveEvents(ctx context.Context, rsAPI api.SyncRoomserv
 
 	// Apply room history visibility filter
 	startTime := time.Now()
-	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(r.ctx, r.snapshot, r.rsAPI, events, nil, r.device.UserID, "messages")
+	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(r.ctx, r.snapshot, r.rsAPI, events, nil, r.deviceUserID, "messages")
 	if err != nil {
 		return []synctypes.ClientEvent{}, *r.from, *r.to, nil
 	}
diff --git a/syncapi/routing/relations.go b/syncapi/routing/relations.go
index 17933b2f..e3d1069a 100644
--- a/syncapi/routing/relations.go
+++ b/syncapi/routing/relations.go
@@ -43,9 +43,25 @@ func Relations(
 	req *http.Request, device *userapi.Device,
 	syncDB storage.Database,
 	rsAPI api.SyncRoomserverAPI,
-	roomID, eventID, relType, eventType string,
+	rawRoomID, eventID, relType, eventType string,
 ) util.JSONResponse {
-	var err error
+	roomID, err := spec.NewRoomID(rawRoomID)
+	if err != nil {
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: spec.InvalidParam("invalid room ID"),
+		}
+	}
+
+	userID, err := spec.NewUserID(device.UserID, true)
+	if err != nil {
+		util.GetLogger(req.Context()).WithError(err).Error("device.UserID invalid")
+		return util.JSONResponse{
+			Code: http.StatusInternalServerError,
+			JSON: spec.Unknown("internal server error"),
+		}
+	}
+
 	var from, to types.StreamPosition
 	var limit int
 	dir := req.URL.Query().Get("dir")
@@ -93,7 +109,7 @@ func Relations(
 	}
 	var events []types.StreamEvent
 	events, res.PrevBatch, res.NextBatch, err = snapshot.RelationsFor(
-		req.Context(), roomID, eventID, relType, eventType, from, to, dir == "b", limit,
+		req.Context(), roomID.String(), eventID, relType, eventType, from, to, dir == "b", limit,
 	)
 	if err != nil {
 		return util.ErrorResponse(err)
@@ -105,12 +121,7 @@ func Relations(
 	}
 
 	// Apply history visibility to the result events.
-	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(req.Context(), snapshot, rsAPI, headeredEvents, nil, device.UserID, "relations")
-	if err != nil {
-		return util.ErrorResponse(err)
-	}
-
-	validRoomID, err := spec.NewRoomID(roomID)
+	filteredEvents, err := internal.ApplyHistoryVisibilityFilter(req.Context(), snapshot, rsAPI, headeredEvents, nil, *userID, "relations")
 	if err != nil {
 		return util.ErrorResponse(err)
 	}
@@ -120,14 +131,14 @@ func Relations(
 	res.Chunk = make([]synctypes.ClientEvent, 0, len(filteredEvents))
 	for _, event := range filteredEvents {
 		sender := spec.UserID{}
-		userID, err := rsAPI.QueryUserIDForSender(req.Context(), *validRoomID, event.SenderID())
+		userID, err := rsAPI.QueryUserIDForSender(req.Context(), *roomID, event.SenderID())
 		if err == nil && userID != nil {
 			sender = *userID
 		}
 
 		sk := event.StateKey()
 		if sk != nil && *sk != "" {
-			skUserID, err := rsAPI.QueryUserIDForSender(req.Context(), *validRoomID, spec.SenderID(*event.StateKey()))
+			skUserID, err := rsAPI.QueryUserIDForSender(req.Context(), *roomID, spec.SenderID(*event.StateKey()))
 			if err == nil && skUserID != nil {
 				skString := skUserID.String()
 				sk = &skString