From a5ea928d0fc52f0efb6607791ac59e18103b57de Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Wed, 28 Jun 2023 10:05:00 +0200 Subject: [PATCH] Fix syncAPI redactions (#3118) Previously we were setting `redacted_because` to the PDU event, but as per the spec it should really be a client event. This fixes it. --- internal/eventutil/events.go | 14 +++++- syncapi/consumers/roomserver.go | 2 +- syncapi/storage/interface.go | 2 +- syncapi/storage/shared/storage_consumer.go | 4 +- syncapi/storage/storage_test.go | 51 ++++++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/internal/eventutil/events.go b/internal/eventutil/events.go index 0f73db2d..56ee576a 100644 --- a/internal/eventutil/events.go +++ b/internal/eventutil/events.go @@ -22,6 +22,7 @@ import ( "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/roomserver/types" + "github.com/matrix-org/dendrite/syncapi/synctypes" "github.com/matrix-org/gomatrixserverlib/fclient" "github.com/matrix-org/gomatrixserverlib/spec" @@ -169,13 +170,22 @@ func truncateAuthAndPrevEvents(auth, prev []string) ( // RedactEvent redacts the given event and sets the unsigned field appropriately. This should be used by // downstream components to the roomserver when an OutputTypeRedactedEvent occurs. -func RedactEvent(redactionEvent, redactedEvent gomatrixserverlib.PDU) error { +func RedactEvent(ctx context.Context, redactionEvent, redactedEvent gomatrixserverlib.PDU, querier api.QuerySenderIDAPI) error { // sanity check if redactionEvent.Type() != spec.MRoomRedaction { return fmt.Errorf("RedactEvent: redactionEvent isn't a redaction event, is '%s'", redactionEvent.Type()) } redactedEvent.Redact() - if err := redactedEvent.SetUnsignedField("redacted_because", redactionEvent); err != nil { + validRoomID, err := spec.NewRoomID(redactionEvent.RoomID()) + if err != nil { + return err + } + senderID, err := querier.QueryUserIDForSender(ctx, *validRoomID, redactionEvent.SenderID()) + if err != nil { + return err + } + redactedBecause := synctypes.ToClientEvent(redactionEvent, synctypes.FormatSync, *senderID, redactionEvent.StateKey()) + if err := redactedEvent.SetUnsignedField("redacted_because", redactedBecause); err != nil { return err } // NOTSPEC: sytest relies on this unspecced field existing :( diff --git a/syncapi/consumers/roomserver.go b/syncapi/consumers/roomserver.go index d468dfc9..90f9ff67 100644 --- a/syncapi/consumers/roomserver.go +++ b/syncapi/consumers/roomserver.go @@ -151,7 +151,7 @@ func (s *OutputRoomEventConsumer) onMessage(ctx context.Context, msgs []*nats.Ms func (s *OutputRoomEventConsumer) onRedactEvent( ctx context.Context, msg api.OutputRedactedEvent, ) error { - err := s.db.RedactEvent(ctx, msg.RedactedEventID, msg.RedactedBecause) + err := s.db.RedactEvent(ctx, msg.RedactedEventID, msg.RedactedBecause, s.rsAPI) if err != nil { log.WithError(err).Error("RedactEvent error'd") return err diff --git a/syncapi/storage/interface.go b/syncapi/storage/interface.go index 8798b62e..243b2592 100644 --- a/syncapi/storage/interface.go +++ b/syncapi/storage/interface.go @@ -174,7 +174,7 @@ type Database interface { // goes wrong. PutFilter(ctx context.Context, localpart string, filter *synctypes.Filter) (string, error) // RedactEvent wipes an event in the database and sets the unsigned.redacted_because key to the redaction event - RedactEvent(ctx context.Context, redactedEventID string, redactedBecause *rstypes.HeaderedEvent) error + RedactEvent(ctx context.Context, redactedEventID string, redactedBecause *rstypes.HeaderedEvent, querier api.QuerySenderIDAPI) error // StoreReceipt stores new receipt events StoreReceipt(ctx context.Context, roomId, receiptType, userId, eventId string, timestamp spec.Timestamp) (pos types.StreamPosition, err error) UpdateIgnoresForUser(ctx context.Context, userID string, ignores *types.IgnoredUsers) error diff --git a/syncapi/storage/shared/storage_consumer.go b/syncapi/storage/shared/storage_consumer.go index 1827218b..746a324f 100644 --- a/syncapi/storage/shared/storage_consumer.go +++ b/syncapi/storage/shared/storage_consumer.go @@ -364,7 +364,7 @@ func (d *Database) PutFilter( return filterID, err } -func (d *Database) RedactEvent(ctx context.Context, redactedEventID string, redactedBecause *rstypes.HeaderedEvent) error { +func (d *Database) RedactEvent(ctx context.Context, redactedEventID string, redactedBecause *rstypes.HeaderedEvent, querier api.QuerySenderIDAPI) error { redactedEvents, err := d.Events(ctx, []string{redactedEventID}) if err != nil { return err @@ -375,7 +375,7 @@ func (d *Database) RedactEvent(ctx context.Context, redactedEventID string, reda } eventToRedact := redactedEvents[0].PDU redactionEvent := redactedBecause.PDU - if err = eventutil.RedactEvent(redactionEvent, eventToRedact); err != nil { + if err = eventutil.RedactEvent(ctx, redactionEvent, eventToRedact, querier); err != nil { return err } diff --git a/syncapi/storage/storage_test.go b/syncapi/storage/storage_test.go index bc64aa50..f56e44a3 100644 --- a/syncapi/storage/storage_test.go +++ b/syncapi/storage/storage_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/matrix-org/dendrite/internal/sqlutil" + "github.com/matrix-org/dendrite/roomserver/api" rstypes "github.com/matrix-org/dendrite/roomserver/types" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/syncapi/storage" @@ -19,6 +20,7 @@ import ( "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/stretchr/testify/assert" + "github.com/tidwall/gjson" ) var ctx = context.Background() @@ -978,3 +980,52 @@ func TestRecentEvents(t *testing.T) { } }) } + +type FakeQuerier struct { + api.QuerySenderIDAPI +} + +func (f *FakeQuerier) QueryUserIDForSender(ctx context.Context, roomID spec.RoomID, senderID spec.SenderID) (*spec.UserID, error) { + return spec.NewUserID(string(senderID), true) +} + +func TestRedaction(t *testing.T) { + alice := test.NewUser(t) + room := test.NewRoom(t, alice) + + redactedEvent := room.CreateAndInsert(t, alice, "m.room.message", map[string]interface{}{"body": "hi"}) + redactionEvent := room.CreateEvent(t, alice, spec.MRoomRedaction, map[string]string{"redacts": redactedEvent.EventID()}) + test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) { + db, close := MustCreateDatabase(t, dbType) + t.Cleanup(close) + MustWriteEvents(t, db, room.Events()) + + err := db.RedactEvent(context.Background(), redactedEvent.EventID(), redactionEvent, &FakeQuerier{}) + if err != nil { + t.Fatal(err) + } + + evs, err := db.Events(context.Background(), []string{redactedEvent.EventID()}) + if err != nil { + t.Fatal(err) + } + + if len(evs) != 1 { + t.Fatalf("expected 1 event, got %d", len(evs)) + } + + // check a few fields which shouldn't be there in unsigned + authEvs := gjson.GetBytes(evs[0].Unsigned(), "redacted_because.auth_events") + if authEvs.Exists() { + t.Error("unexpected auth_events in redacted event") + } + prevEvs := gjson.GetBytes(evs[0].Unsigned(), "redacted_because.prev_events") + if prevEvs.Exists() { + t.Error("unexpected auth_events in redacted event") + } + depth := gjson.GetBytes(evs[0].Unsigned(), "redacted_because.depth") + if depth.Exists() { + t.Error("unexpected auth_events in redacted event") + } + }) +}