From 2dee706f9ef2de70516dbc993dcfc8ec6f7fdd52 Mon Sep 17 00:00:00 2001 From: kegsay Date: Wed, 2 Feb 2022 13:30:48 +0000 Subject: [PATCH] PerformInvite: bugfix and rejig control flow (#2137) * PerformInvite: bugfix and rejig control flow Local clients would not be notified of invites to rooms Dendrite had already joined in all cases due to not returning an `api.OutputNewInviteEvent` for local invites. We now do this. This was an easy mistake to make due to the control flow of the function which doesn't handle the happy case at the end of the function and instead forks the function depending on if the invite was via federation or not. This has now been changed to handle the federated invite as if it were an error (in that we check it, do it and bail out) rather than outstay our welcome. This ends up with the local invite being the happy case, which now both sends an `InputRoomEvent` to the roomserver _and_ a `api.OutputNewInviteEvent` is returned. * Don't send invite pokes in PerformInvite * Move event ID into logger --- roomserver/internal/perform/perform_invite.go | 161 ++++++++++-------- 1 file changed, 88 insertions(+), 73 deletions(-) diff --git a/roomserver/internal/perform/perform_invite.go b/roomserver/internal/perform/perform_invite.go index e23ed47b..6559cd08 100644 --- a/roomserver/internal/perform/perform_invite.go +++ b/roomserver/internal/perform/perform_invite.go @@ -27,6 +27,7 @@ import ( "github.com/matrix-org/dendrite/roomserver/types" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" log "github.com/sirupsen/logrus" ) @@ -54,18 +55,23 @@ func (r *Inviter) PerformInvite( return nil, fmt.Errorf("failed to load RoomInfo: %w", err) } - log.WithFields(log.Fields{ - "event_id": event.EventID(), - "room_id": roomID, - "room_version": req.RoomVersion, - "target_user_id": targetUserID, - "room_info_exists": info != nil, - }).Debug("processing invite event") - _, domain, _ := gomatrixserverlib.SplitID('@', targetUserID) isTargetLocal := domain == r.Cfg.Matrix.ServerName isOriginLocal := event.Origin() == r.Cfg.Matrix.ServerName + logger := util.GetLogger(ctx).WithFields(map[string]interface{}{ + "inviter": event.Sender(), + "invitee": *event.StateKey(), + "room_id": roomID, + "event_id": event.EventID(), + }) + logger.WithFields(log.Fields{ + "room_version": req.RoomVersion, + "room_info_exists": info != nil, + "target_local": isTargetLocal, + "origin_local": isOriginLocal, + }).Debug("processing invite event") + inviteState := req.InviteRoomState if len(inviteState) == 0 && info != nil { var is []gomatrixserverlib.InviteV2StrippedState @@ -122,75 +128,17 @@ func (r *Inviter) PerformInvite( Code: api.PerformErrorNotAllowed, Msg: "User is already joined to room", } + logger.Debugf("user already joined") return nil, nil } - if isOriginLocal { - // The invite originated locally. Therefore we have a responsibility to - // try and see if the user is allowed to make this invite. We can't do - // this for invites coming in over federation - we have to take those on - // trust. - _, err = helpers.CheckAuthEvents(ctx, r.DB, event, event.AuthEventIDs()) - if err != nil { - log.WithError(err).WithField("event_id", event.EventID()).WithField("auth_event_ids", event.AuthEventIDs()).Error( - "processInviteEvent.checkAuthEvents failed for event", - ) - res.Error = &api.PerformError{ - Msg: err.Error(), - Code: api.PerformErrorNotAllowed, - } - } - - // If the invite originated from us and the target isn't local then we - // should try and send the invite over federation first. It might be - // that the remote user doesn't exist, in which case we can give up - // processing here. - if req.SendAsServer != api.DoNotSendToOtherServers && !isTargetLocal { - fsReq := &federationAPI.PerformInviteRequest{ - RoomVersion: req.RoomVersion, - Event: event, - InviteRoomState: inviteState, - } - fsRes := &federationAPI.PerformInviteResponse{} - if err = r.FSAPI.PerformInvite(ctx, fsReq, fsRes); err != nil { - res.Error = &api.PerformError{ - Msg: err.Error(), - Code: api.PerformErrorNotAllowed, - } - log.WithError(err).WithField("event_id", event.EventID()).Error("r.FSAPI.PerformInvite failed") - return nil, nil - } - event = fsRes.Event - } - - // Send the invite event to the roomserver input stream. This will - // notify existing users in the room about the invite, update the - // membership table and ensure that the event is ready and available - // to use as an auth event when accepting the invite. - inputReq := &api.InputRoomEventsRequest{ - InputRoomEvents: []api.InputRoomEvent{ - { - Kind: api.KindNew, - Event: event, - Origin: event.Origin(), - SendAsServer: req.SendAsServer, - }, - }, - } - inputRes := &api.InputRoomEventsResponse{} - r.Inputer.InputRoomEvents(context.Background(), inputReq, inputRes) - if err = inputRes.Err(); err != nil { - res.Error = &api.PerformError{ - Msg: fmt.Sprintf("r.InputRoomEvents: %s", err.Error()), - Code: api.PerformErrorNotAllowed, - } - log.WithError(err).WithField("event_id", event.EventID()).Error("r.InputRoomEvents failed") - return nil, nil - } - } else { + if !isOriginLocal { // The invite originated over federation. Process the membership // update, which will notify the sync API etc about the incoming - // invite. + // invite. We do NOT send an InputRoomEvent for the invite as it + // will never pass auth checks due to lacking room state, but we + // still need to tell the client about the invite so we can accept + // it, hence we return an output event to send to the sync api. updater, err := r.DB.MembershipUpdater(ctx, roomID, targetUserID, isTargetLocal, req.RoomVersion) if err != nil { return nil, fmt.Errorf("r.DB.MembershipUpdater: %w", err) @@ -205,10 +153,77 @@ func (r *Inviter) PerformInvite( if err = updater.Commit(); err != nil { return nil, fmt.Errorf("updater.Commit: %w", err) } - + logger.Debugf("updated membership to invite and sending invite OutputEvent") return outputUpdates, nil } + // The invite originated locally. Therefore we have a responsibility to + // try and see if the user is allowed to make this invite. We can't do + // this for invites coming in over federation - we have to take those on + // trust. + _, err = helpers.CheckAuthEvents(ctx, r.DB, event, event.AuthEventIDs()) + if err != nil { + logger.WithError(err).WithField("event_id", event.EventID()).WithField("auth_event_ids", event.AuthEventIDs()).Error( + "processInviteEvent.checkAuthEvents failed for event", + ) + res.Error = &api.PerformError{ + Msg: err.Error(), + Code: api.PerformErrorNotAllowed, + } + return nil, nil + } + + // If the invite originated from us and the target isn't local then we + // should try and send the invite over federation first. It might be + // that the remote user doesn't exist, in which case we can give up + // processing here. + if req.SendAsServer != api.DoNotSendToOtherServers && !isTargetLocal { + fsReq := &federationAPI.PerformInviteRequest{ + RoomVersion: req.RoomVersion, + Event: event, + InviteRoomState: inviteState, + } + fsRes := &federationAPI.PerformInviteResponse{} + if err = r.FSAPI.PerformInvite(ctx, fsReq, fsRes); err != nil { + res.Error = &api.PerformError{ + Msg: err.Error(), + Code: api.PerformErrorNotAllowed, + } + logger.WithError(err).WithField("event_id", event.EventID()).Error("r.FSAPI.PerformInvite failed") + return nil, nil + } + event = fsRes.Event + logger.Debugf("Federated PerformInvite success with event ID %s", event.EventID()) + } + + // Send the invite event to the roomserver input stream. This will + // notify existing users in the room about the invite, update the + // membership table and ensure that the event is ready and available + // to use as an auth event when accepting the invite. + // It will NOT notify the invitee of this invite. + inputReq := &api.InputRoomEventsRequest{ + InputRoomEvents: []api.InputRoomEvent{ + { + Kind: api.KindNew, + Event: event, + Origin: event.Origin(), + SendAsServer: req.SendAsServer, + }, + }, + } + inputRes := &api.InputRoomEventsResponse{} + r.Inputer.InputRoomEvents(context.Background(), inputReq, inputRes) + if err = inputRes.Err(); err != nil { + res.Error = &api.PerformError{ + Msg: fmt.Sprintf("r.InputRoomEvents: %s", err.Error()), + Code: api.PerformErrorNotAllowed, + } + logger.WithError(err).WithField("event_id", event.EventID()).Error("r.InputRoomEvents failed") + return nil, nil + } + + // Don't notify the sync api of this event in the same way as a federated invite so the invitee + // gets the invite, as the roomserver will do this when it processes the m.room.member invite. return nil, nil }