From c60270eea31f062f176b59e69e9d9e9e43b4b004 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 25 Aug 2020 18:43:56 +0100 Subject: [PATCH 1/2] Enforce history visibility etc for /rooms/{roomID}/state (#1340) * Enforce history visibility etc for /rooms/{roomID}/state * Deduplicate OnIncomingStateRequest and OnIncomingStateTypeRequest * Revert "Deduplicate OnIncomingStateRequest and OnIncomingStateTypeRequest" This reverts commit 335035d66e629022232abc682d6631e3cf669e23. --- clientapi/routing/routing.go | 2 +- clientapi/routing/state.go | 138 +++++++++++++++++++++++++---------- 2 files changed, 100 insertions(+), 40 deletions(-) diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index e15625f7d..c259e5293 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -201,7 +201,7 @@ func Setup( if err != nil { return util.ErrorResponse(err) } - return OnIncomingStateRequest(req.Context(), rsAPI, vars["roomID"]) + return OnIncomingStateRequest(req.Context(), device, rsAPI, vars["roomID"]) })).Methods(http.MethodGet, http.MethodOptions) r0mux.Handle("/rooms/{roomID}/state/{type:[^/]+/?}", httputil.MakeAuthAPI("room_state", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { diff --git a/clientapi/routing/state.go b/clientapi/routing/state.go index c4f7c4f2c..2a424cbe8 100644 --- a/clientapi/routing/state.go +++ b/clientapi/routing/state.go @@ -22,7 +22,6 @@ import ( "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/roomserver/api" - "github.com/matrix-org/dendrite/syncapi/types" userapi "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" @@ -42,57 +41,118 @@ type stateEventInStateResp struct { // TODO: Check if the user is in the room. If not, check if the room's history // is publicly visible. Current behaviour is returning an empty array if the // user cannot see the room's history. -func OnIncomingStateRequest(ctx context.Context, rsAPI api.RoomserverInternalAPI, roomID string) util.JSONResponse { - // TODO(#287): Auth request and handle the case where the user has left (where - // we should return the state at the poin they left) - stateReq := api.QueryLatestEventsAndStateRequest{ - RoomID: roomID, - } - stateRes := api.QueryLatestEventsAndStateResponse{} +func OnIncomingStateRequest(ctx context.Context, device *userapi.Device, rsAPI api.RoomserverInternalAPI, roomID string) util.JSONResponse { + var worldReadable bool + var wantLatestState bool - if err := rsAPI.QueryLatestEventsAndState(ctx, &stateReq, &stateRes); err != nil { + // First of all, get the latest state of the room. We need to do this + // so that we can look at the history visibility of the room. If the + // room is world-readable then we will always return the latest state. + stateRes := api.QueryLatestEventsAndStateResponse{} + if err := rsAPI.QueryLatestEventsAndState(ctx, &api.QueryLatestEventsAndStateRequest{ + RoomID: roomID, + StateToFetch: []gomatrixserverlib.StateKeyTuple{}, + }, &stateRes); err != nil { util.GetLogger(ctx).WithError(err).Error("queryAPI.QueryLatestEventsAndState failed") return jsonerror.InternalServerError() } - if len(stateRes.StateEvents) == 0 { - return util.JSONResponse{ - Code: http.StatusNotFound, - JSON: jsonerror.NotFound("cannot find state"), - } - } - - resp := []stateEventInStateResp{} - // Fill the prev_content and replaces_state keys if necessary - for _, event := range stateRes.StateEvents { - stateEvent := stateEventInStateResp{ - ClientEvent: gomatrixserverlib.HeaderedToClientEvents( - []gomatrixserverlib.HeaderedEvent{event}, gomatrixserverlib.FormatAll, - )[0], - } - var prevEventRef types.PrevEventRef - if len(event.Unsigned()) > 0 { - if err := json.Unmarshal(event.Unsigned(), &prevEventRef); err != nil { - util.GetLogger(ctx).WithError(err).Error("json.Unmarshal failed") + // Look at the room state and see if we have a history visibility event + // that marks the room as world-readable. If we don't then we assume that + // the room is not world-readable. + for _, ev := range stateRes.StateEvents { + if ev.Type() == gomatrixserverlib.MRoomHistoryVisibility { + content := map[string]string{} + if err := json.Unmarshal(ev.Content(), &content); err != nil { + util.GetLogger(ctx).WithError(err).Error("json.Unmarshal for history visibility failed") return jsonerror.InternalServerError() } - // Fills the previous state event ID if the state event replaces another - // state event - if len(prevEventRef.ReplacesState) > 0 { - stateEvent.ReplacesState = prevEventRef.ReplacesState - } - // Fill the previous event if the state event references a previous event - if prevEventRef.PrevContent != nil { - stateEvent.PrevContent = prevEventRef.PrevContent + if visibility, ok := content["history_visibility"]; ok { + worldReadable = visibility == "world_readable" + break } } - - resp = append(resp, stateEvent) } + // If the room isn't world-readable then we will instead try to find out + // the state of the room based on the user's membership. If the user is + // in the room then we'll want the latest state. If the user has never + // been in the room and the room isn't world-readable, then we won't + // return any state. If the user was in the room previously but is no + // longer then we will return the state at the time that the user left. + // membershipRes will only be populated if the room is not world-readable. + var membershipRes api.QueryMembershipForUserResponse + if !worldReadable { + // The room isn't world-readable so try to work out based on the + // user's membership if we want the latest state or not. + err := rsAPI.QueryMembershipForUser(ctx, &api.QueryMembershipForUserRequest{ + RoomID: roomID, + UserID: device.UserID, + }, &membershipRes) + if err != nil { + util.GetLogger(ctx).WithError(err).Error("Failed to QueryMembershipForUser") + return jsonerror.InternalServerError() + } + // If the user has never been in the room then stop at this point. + // We won't tell the user about a room they have never joined. + if !membershipRes.HasBeenInRoom { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.Forbidden(fmt.Sprintf("Unknown room %q or user %q has never joined this room", roomID, device.UserID)), + } + } + // Otherwise, if the user has been in the room, whether or not we + // give them the latest state will depend on if they are *still* in + // the room. + wantLatestState = membershipRes.IsInRoom + } else { + // The room is world-readable so the user join state is irrelevant, + // just get the latest room state instead. + wantLatestState = true + } + + util.GetLogger(ctx).WithFields(log.Fields{ + "roomID": roomID, + "state_at_event": !wantLatestState, + }).Info("Fetching all state") + + stateEvents := []gomatrixserverlib.ClientEvent{} + if wantLatestState { + // If we are happy to use the latest state, either because the user is + // still in the room, or because the room is world-readable, then just + // use the result of the previous QueryLatestEventsAndState response + // to find the state event, if provided. + for _, ev := range stateRes.StateEvents { + stateEvents = append( + stateEvents, + gomatrixserverlib.HeaderedToClientEvent(ev, gomatrixserverlib.FormatAll), + ) + } + } else { + // Otherwise, take the event ID of their leave event and work out what + // the state of the room was before that event. + var stateAfterRes api.QueryStateAfterEventsResponse + err := rsAPI.QueryStateAfterEvents(ctx, &api.QueryStateAfterEventsRequest{ + RoomID: roomID, + PrevEventIDs: []string{membershipRes.EventID}, + StateToFetch: []gomatrixserverlib.StateKeyTuple{}, + }, &stateAfterRes) + if err != nil { + util.GetLogger(ctx).WithError(err).Error("Failed to QueryMembershipForUser") + return jsonerror.InternalServerError() + } + for _, ev := range stateRes.StateEvents { + stateEvents = append( + stateEvents, + gomatrixserverlib.HeaderedToClientEvent(ev, gomatrixserverlib.FormatAll), + ) + } + } + + // Return the results to the requestor. return util.JSONResponse{ Code: http.StatusOK, - JSON: resp, + JSON: stateEvents, } } From 55498c8deb262e564e1b79bf4409eb5593f3034e Mon Sep 17 00:00:00 2001 From: Kegsay Date: Tue, 25 Aug 2020 18:59:00 +0100 Subject: [PATCH 2/2] Fix 'Invited user can reject invite over federation several times' (#1341) --- syncapi/storage/postgres/invites_table.go | 8 ++++++++ syncapi/storage/sqlite3/invites_table.go | 8 ++++++++ syncapi/storage/tables/interface.go | 3 ++- sytest-whitelist | 1 + 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/syncapi/storage/postgres/invites_table.go b/syncapi/storage/postgres/invites_table.go index 530dc6452..eed58c158 100644 --- a/syncapi/storage/postgres/invites_table.go +++ b/syncapi/storage/postgres/invites_table.go @@ -139,6 +139,14 @@ func (s *inviteEventsStatements) SelectInviteEventsInRange( return nil, nil, err } + // if we have seen this room before, it has a higher stream position and hence takes priority + // because the query is ORDER BY id DESC so drop them + _, isRetired := retired[roomID] + _, isInvited := result[roomID] + if isRetired || isInvited { + continue + } + var event gomatrixserverlib.HeaderedEvent if err := json.Unmarshal(eventJSON, &event); err != nil { return nil, nil, err diff --git a/syncapi/storage/sqlite3/invites_table.go b/syncapi/storage/sqlite3/invites_table.go index 45862efbb..7da866831 100644 --- a/syncapi/storage/sqlite3/invites_table.go +++ b/syncapi/storage/sqlite3/invites_table.go @@ -150,6 +150,14 @@ func (s *inviteEventsStatements) SelectInviteEventsInRange( return nil, nil, err } + // if we have seen this room before, it has a higher stream position and hence takes priority + // because the query is ORDER BY id DESC so drop them + _, isRetired := retired[roomID] + _, isInvited := result[roomID] + if isRetired || isInvited { + continue + } + var event gomatrixserverlib.HeaderedEvent if err := json.Unmarshal(eventJSON, &event); err != nil { return nil, nil, err diff --git a/syncapi/storage/tables/interface.go b/syncapi/storage/tables/interface.go index 9d239d233..2ff229cbc 100644 --- a/syncapi/storage/tables/interface.go +++ b/syncapi/storage/tables/interface.go @@ -33,7 +33,8 @@ type AccountData interface { type Invites interface { InsertInviteEvent(ctx context.Context, txn *sql.Tx, inviteEvent gomatrixserverlib.HeaderedEvent) (streamPos types.StreamPosition, err error) DeleteInviteEvent(ctx context.Context, inviteEventID string) (types.StreamPosition, error) - // SelectInviteEventsInRange returns a map of room ID to invite events. + // SelectInviteEventsInRange returns a map of room ID to invite events. If multiple invite/retired invites exist in the given range, return the latest value + // for the room. SelectInviteEventsInRange(ctx context.Context, txn *sql.Tx, targetUserID string, r types.Range) (invites map[string]gomatrixserverlib.HeaderedEvent, retired map[string]gomatrixserverlib.HeaderedEvent, err error) SelectMaxInviteID(ctx context.Context, txn *sql.Tx) (id int64, err error) } diff --git a/sytest-whitelist b/sytest-whitelist index 8084bbe39..a17ed8407 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -421,6 +421,7 @@ Remote users may not join unfederated rooms Non-numeric ports in server names are rejected Invited user can reject invite over federation Invited user can reject invite over federation for empty room +Invited user can reject invite over federation several times Can reject invites over federation for rooms with version 1 Can reject invites over federation for rooms with version 2 Can reject invites over federation for rooms with version 3