From c1447a58e5de5408d80e0de84c0424342121b06c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 21 Jul 2021 16:53:50 +0100 Subject: [PATCH] Various alias fixes (#1934) * Generate m.room.canonical_alias instead of legacy m.room.aliases * Add omitempty tags * Add aliases endpoint to client API * Check power levels when setting aliases * Don't return null on /aliases * Don't return error if the state event fails * Update sytest-whitelist * Don't send updated m.room.canonical_alias events * Don't check PLs after all because for local aliases they are apparently irrelevant * Fix some bugs * Allow deleting a local alias with enough PL * Fix some more bugs * Update sytest-whitelist * Fix copyright notices * Review comments --- clientapi/routing/aliases.go | 96 +++++++++++ clientapi/routing/createroom.go | 2 - clientapi/routing/directory.go | 52 +++--- clientapi/routing/routing.go | 8 + go.mod | 2 +- go.sum | 4 +- internal/eventutil/eventcontent.go | 1 - roomserver/api/alias.go | 7 +- roomserver/internal/alias.go | 151 ++++-------------- roomserver/internal/perform/perform_invite.go | 4 +- roomserver/storage/shared/storage.go | 3 + sytest-whitelist | 3 + 12 files changed, 174 insertions(+), 159 deletions(-) create mode 100644 clientapi/routing/aliases.go diff --git a/clientapi/routing/aliases.go b/clientapi/routing/aliases.go new file mode 100644 index 000000000..8c4830532 --- /dev/null +++ b/clientapi/routing/aliases.go @@ -0,0 +1,96 @@ +// Copyright 2021 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package routing + +import ( + "fmt" + "net/http" + + "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/dendrite/roomserver/api" + userapi "github.com/matrix-org/dendrite/userapi/api" + "github.com/matrix-org/gomatrixserverlib" + + "github.com/matrix-org/util" +) + +// GetAliases implements GET /_matrix/client/r0/rooms/{roomId}/aliases +func GetAliases( + req *http.Request, rsAPI api.RoomserverInternalAPI, device *userapi.Device, roomID string, +) util.JSONResponse { + stateTuple := gomatrixserverlib.StateKeyTuple{ + EventType: gomatrixserverlib.MRoomHistoryVisibility, + StateKey: "", + } + stateReq := &api.QueryCurrentStateRequest{ + RoomID: roomID, + StateTuples: []gomatrixserverlib.StateKeyTuple{stateTuple}, + } + stateRes := &api.QueryCurrentStateResponse{} + if err := rsAPI.QueryCurrentState(req.Context(), stateReq, stateRes); err != nil { + util.GetLogger(req.Context()).WithError(err).Error("rsAPI.QueryCurrentState failed") + return util.ErrorResponse(fmt.Errorf("rsAPI.QueryCurrentState: %w", err)) + } + + visibility := "invite" + if historyVisEvent, ok := stateRes.StateEvents[stateTuple]; ok { + var err error + visibility, err = historyVisEvent.HistoryVisibility() + if err != nil { + util.GetLogger(req.Context()).WithError(err).Error("historyVisEvent.HistoryVisibility failed") + return util.ErrorResponse(fmt.Errorf("historyVisEvent.HistoryVisibility: %w", err)) + } + } + if visibility != gomatrixserverlib.WorldReadable { + queryReq := api.QueryMembershipForUserRequest{ + RoomID: roomID, + UserID: device.UserID, + } + var queryRes api.QueryMembershipForUserResponse + if err := rsAPI.QueryMembershipForUser(req.Context(), &queryReq, &queryRes); err != nil { + util.GetLogger(req.Context()).WithError(err).Error("rsAPI.QueryMembershipsForRoom failed") + return jsonerror.InternalServerError() + } + if !queryRes.IsInRoom { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.Forbidden("You aren't a member of this room."), + } + } + } + + aliasesReq := api.GetAliasesForRoomIDRequest{ + RoomID: roomID, + } + aliasesRes := api.GetAliasesForRoomIDResponse{} + if err := rsAPI.GetAliasesForRoomID(req.Context(), &aliasesReq, &aliasesRes); err != nil { + util.GetLogger(req.Context()).WithError(err).Error("rsAPI.GetAliasesForRoomID failed") + return util.ErrorResponse(fmt.Errorf("rsAPI.GetAliasesForRoomID: %w", err)) + } + + response := struct { + Aliases []string `json:"aliases"` + }{ + Aliases: aliasesRes.Aliases, + } + if response.Aliases == nil { + response.Aliases = []string{} // pleases sytest + } + + return util.JSONResponse{ + Code: 200, + JSON: response, + } +} diff --git a/clientapi/routing/createroom.go b/clientapi/routing/createroom.go index 4219bb37c..b3b996ecb 100644 --- a/clientapi/routing/createroom.go +++ b/clientapi/routing/createroom.go @@ -383,7 +383,6 @@ func createRoom( // 10- m.room.topic (opt) // 11- invite events (opt) - with is_direct flag if applicable TODO // 12- 3pid invite events (opt) TODO - // 13- m.room.aliases event for HS (if alias specified) TODO // This differs from Synapse slightly. Synapse would vary the ordering of 3-7 // depending on if those events were in "initial_state" or not. This made it // harder to reason about, hence sticking to a strict static ordering. @@ -404,7 +403,6 @@ func createRoom( if aliasEvent != nil { // TODO: bit of a chicken and egg problem here as the alias doesn't exist and cannot until we have made the room. // This means we might fail creating the alias but say the canonical alias is something that doesn't exist. - // m.room.aliases is handled when we call roomserver.SetRoomAlias eventsToMake = append(eventsToMake, *aliasEvent) } diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go index 0e994b645..ae4660656 100644 --- a/clientapi/routing/directory.go +++ b/clientapi/routing/directory.go @@ -113,13 +113,12 @@ func DirectoryRoom( } // SetLocalAlias implements PUT /directory/room/{roomAlias} -// TODO: Check if the user has the power level to set an alias func SetLocalAlias( req *http.Request, device *api.Device, alias string, cfg *config.ClientAPI, - aliasAPI roomserverAPI.RoomserverInternalAPI, + rsAPI roomserverAPI.RoomserverInternalAPI, ) util.JSONResponse { _, domain, err := gomatrixserverlib.SplitID('#', alias) if err != nil { @@ -172,7 +171,7 @@ func SetLocalAlias( Alias: alias, } var queryRes roomserverAPI.SetRoomAliasResponse - if err := aliasAPI.SetRoomAlias(req.Context(), &queryReq, &queryRes); err != nil { + if err := rsAPI.SetRoomAlias(req.Context(), &queryReq, &queryRes); err != nil { util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.SetRoomAlias failed") return jsonerror.InternalServerError() } @@ -195,43 +194,32 @@ func RemoveLocalAlias( req *http.Request, device *api.Device, alias string, - aliasAPI roomserverAPI.RoomserverInternalAPI, + rsAPI roomserverAPI.RoomserverInternalAPI, ) util.JSONResponse { - - creatorQueryReq := roomserverAPI.GetCreatorIDForAliasRequest{ - Alias: alias, - } - var creatorQueryRes roomserverAPI.GetCreatorIDForAliasResponse - if err := aliasAPI.GetCreatorIDForAlias(req.Context(), &creatorQueryReq, &creatorQueryRes); err != nil { - util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.GetCreatorIDForAlias failed") - return jsonerror.InternalServerError() - } - - if creatorQueryRes.UserID == "" { - return util.JSONResponse{ - Code: http.StatusNotFound, - JSON: jsonerror.NotFound("Alias does not exist"), - } - } - - if creatorQueryRes.UserID != device.UserID { - // TODO: Still allow deletion if user is admin - return util.JSONResponse{ - Code: http.StatusForbidden, - JSON: jsonerror.Forbidden("You do not have permission to delete this alias"), - } - } - queryReq := roomserverAPI.RemoveRoomAliasRequest{ Alias: alias, UserID: device.UserID, } var queryRes roomserverAPI.RemoveRoomAliasResponse - if err := aliasAPI.RemoveRoomAlias(req.Context(), &queryReq, &queryRes); err != nil { + if err := rsAPI.RemoveRoomAlias(req.Context(), &queryReq, &queryRes); err != nil { util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.RemoveRoomAlias failed") return jsonerror.InternalServerError() } + if !queryRes.Found { + return util.JSONResponse{ + Code: http.StatusNotFound, + JSON: jsonerror.NotFound("The alias does not exist."), + } + } + + if !queryRes.Removed { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.Forbidden("You do not have permission to remove this alias."), + } + } + return util.JSONResponse{ Code: http.StatusOK, JSON: struct{}{}, @@ -294,9 +282,9 @@ func SetVisibility( return jsonerror.InternalServerError() } - // NOTSPEC: Check if the user's power is greater than power required to change m.room.aliases event + // NOTSPEC: Check if the user's power is greater than power required to change m.room.canonical_alias event power, _ := gomatrixserverlib.NewPowerLevelContentFromEvent(queryEventsRes.StateEvents[0].Event) - if power.UserLevel(dev.UserID) < power.EventLevel(gomatrixserverlib.MRoomAliases, true) { + if power.UserLevel(dev.UserID) < power.EventLevel(gomatrixserverlib.MRoomCanonicalAlias, true) { return util.JSONResponse{ Code: http.StatusForbidden, JSON: jsonerror.Forbidden("userID doesn't have power level to change visibility"), diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 37279e8ed..d768247a4 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -275,6 +275,14 @@ func Setup( return OnIncomingStateRequest(req.Context(), device, rsAPI, vars["roomID"]) })).Methods(http.MethodGet, http.MethodOptions) + r0mux.Handle("/rooms/{roomID}/aliases", httputil.MakeAuthAPI("aliases", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return GetAliases(req, rsAPI, device, 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 { vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) if err != nil { diff --git a/go.mod b/go.mod index ec17e6444..e6d55f208 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 - github.com/matrix-org/gomatrixserverlib v0.0.0-20210721094149-75792185bf42 + github.com/matrix-org/gomatrixserverlib v0.0.0-20210721155151-4575fad563b0 github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0 github.com/matrix-org/pinecone v0.0.0-20210623102758-74f885644c1b github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 diff --git a/go.sum b/go.sum index 86149343a..064c03cdd 100644 --- a/go.sum +++ b/go.sum @@ -1027,8 +1027,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d/go.mod h1 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= -github.com/matrix-org/gomatrixserverlib v0.0.0-20210721094149-75792185bf42 h1:UsCdEX9G3svG07bBV8RKAWIyGzCgJpbX4BCP1n4ezH8= -github.com/matrix-org/gomatrixserverlib v0.0.0-20210721094149-75792185bf42/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20210721155151-4575fad563b0 h1:W1oqIcus66YGUb2HkawPTrU3OKqT3PXhXpBKoxEwiiA= +github.com/matrix-org/gomatrixserverlib v0.0.0-20210721155151-4575fad563b0/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0 h1:HZCzy4oVzz55e+cOMiX/JtSF2UOY1evBl2raaE7ACcU= github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0/go.mod h1:sjyPyRxKM5uw1nD2cJ6O2OxI6GOqyVBfNXqKjBZTBZE= github.com/matrix-org/pinecone v0.0.0-20210623102758-74f885644c1b h1:5X5vdWQ13xrNkJVqaJHPsrt7rKkMJH5iac0EtfOuxSg= diff --git a/internal/eventutil/eventcontent.go b/internal/eventutil/eventcontent.go index 873e20a8e..4ecb5fb56 100644 --- a/internal/eventutil/eventcontent.go +++ b/internal/eventutil/eventcontent.go @@ -53,7 +53,6 @@ func InitialPowerLevelsContent(roomCreator string) (c gomatrixserverlib.PowerLev "m.room.history_visibility": 100, "m.room.canonical_alias": 50, "m.room.avatar": 50, - "m.room.aliases": 0, // anyone can publish aliases by default. Has to be 0 else state_default is used. } c.Users = map[string]int64{roomCreator: 100} return c diff --git a/roomserver/api/alias.go b/roomserver/api/alias.go index 2eb911293..df69e5b4d 100644 --- a/roomserver/api/alias.go +++ b/roomserver/api/alias.go @@ -78,4 +78,9 @@ type RemoveRoomAliasRequest struct { } // RemoveRoomAliasResponse is a response to RemoveRoomAlias -type RemoveRoomAliasResponse struct{} +type RemoveRoomAliasResponse struct { + // Did the alias exist before? + Found bool `json:"found"` + // Did we remove it? + Removed bool `json:"removed"` +} diff --git a/roomserver/internal/alias.go b/roomserver/internal/alias.go index f15881d75..7995279d2 100644 --- a/roomserver/internal/alias.go +++ b/roomserver/internal/alias.go @@ -16,10 +16,7 @@ package internal import ( "context" - "encoding/json" - "errors" "fmt" - "time" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/gomatrixserverlib" @@ -73,11 +70,7 @@ func (r *RoomserverInternalAPI) SetRoomAlias( return err } - // Send a m.room.aliases event with the updated list of aliases for this room - // At this point we've already committed the alias to the database so we - // shouldn't cancel this request. - // TODO: Ensure that we send unsent events when if server restarts. - return r.sendUpdatedAliasesEvent(context.TODO(), request.UserID, request.RoomID) + return nil } // GetRoomIDForAlias implements alias.RoomserverInternalAPI @@ -157,122 +150,44 @@ func (r *RoomserverInternalAPI) RemoveRoomAlias( request *api.RemoveRoomAliasRequest, response *api.RemoveRoomAliasResponse, ) error { - // Look up the room ID in the database roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias) if err != nil { - return err + return fmt.Errorf("r.DB.GetRoomIDForAlias: %w", err) + } + if roomID == "" { + response.Found = false + response.Removed = false + return nil } - // Remove the dalias from the database + response.Found = true + creatorID, err := r.DB.GetCreatorIDForAlias(ctx, request.Alias) + if err != nil { + return fmt.Errorf("r.DB.GetCreatorIDForAlias: %w", err) + } + + if creatorID != request.UserID { + plEvent, err := r.DB.GetStateEvent(ctx, roomID, gomatrixserverlib.MRoomPowerLevels, "") + if err != nil { + return fmt.Errorf("r.DB.GetStateEvent: %w", err) + } + + pls, err := plEvent.PowerLevels() + if err != nil { + return fmt.Errorf("plEvent.PowerLevels: %w", err) + } + + if pls.UserLevel(request.UserID) < pls.EventLevel(gomatrixserverlib.MRoomCanonicalAlias, true) { + response.Removed = false + return nil + } + } + + // Remove the alias from the database if err := r.DB.RemoveRoomAlias(ctx, request.Alias); err != nil { return err } - // Send an updated m.room.aliases event - // At this point we've already committed the alias to the database so we - // shouldn't cancel this request. - // TODO: Ensure that we send unsent events when if server restarts. - return r.sendUpdatedAliasesEvent(context.TODO(), request.UserID, roomID) -} - -type roomAliasesContent struct { - Aliases []string `json:"aliases"` -} - -// Build the updated m.room.aliases event to send to the room after addition or -// removal of an alias -func (r *RoomserverInternalAPI) sendUpdatedAliasesEvent( - ctx context.Context, userID string, roomID string, -) error { - serverName := string(r.Cfg.Matrix.ServerName) - - builder := gomatrixserverlib.EventBuilder{ - Sender: userID, - RoomID: roomID, - Type: "m.room.aliases", - StateKey: &serverName, - } - - // Retrieve the updated list of aliases, marhal it and set it as the - // event's content - aliases, err := r.DB.GetAliasesForRoomID(ctx, roomID) - if err != nil { - return err - } - content := roomAliasesContent{Aliases: aliases} - rawContent, err := json.Marshal(content) - if err != nil { - return err - } - err = builder.SetContent(json.RawMessage(rawContent)) - if err != nil { - return err - } - - // Get needed state events and depth - eventsNeeded, err := gomatrixserverlib.StateNeededForEventBuilder(&builder) - if err != nil { - return err - } - if len(eventsNeeded.Tuples()) == 0 { - return errors.New("expecting state tuples for event builder, got none") - } - req := api.QueryLatestEventsAndStateRequest{ - RoomID: roomID, - StateToFetch: eventsNeeded.Tuples(), - } - var res api.QueryLatestEventsAndStateResponse - if err = r.QueryLatestEventsAndState(ctx, &req, &res); err != nil { - return err - } - builder.Depth = res.Depth - builder.PrevEvents = res.LatestEvents - - // Add auth events - authEvents := gomatrixserverlib.NewAuthEvents(nil) - for i := range res.StateEvents { - err = authEvents.AddEvent(res.StateEvents[i].Event) - if err != nil { - return err - } - } - refs, err := eventsNeeded.AuthEventReferences(&authEvents) - if err != nil { - return err - } - builder.AuthEvents = refs - - roomInfo, err := r.DB.RoomInfo(ctx, roomID) - if err != nil { - return err - } - if roomInfo == nil { - return fmt.Errorf("room %s does not exist", roomID) - } - - // Build the event - now := time.Now() - event, err := builder.Build( - now, r.Cfg.Matrix.ServerName, r.Cfg.Matrix.KeyID, - r.Cfg.Matrix.PrivateKey, roomInfo.RoomVersion, - ) - if err != nil { - return err - } - - // Create the request - ire := api.InputRoomEvent{ - Kind: api.KindNew, - Event: event.Headered(roomInfo.RoomVersion), - AuthEventIDs: event.AuthEventIDs(), - SendAsServer: serverName, - } - inputReq := api.InputRoomEventsRequest{ - InputRoomEvents: []api.InputRoomEvent{ire}, - } - var inputRes api.InputRoomEventsResponse - - // Send the request - r.InputRoomEvents(ctx, &inputReq, &inputRes) - return inputRes.Err() + response.Removed = true + return nil } diff --git a/roomserver/internal/perform/perform_invite.go b/roomserver/internal/perform/perform_invite.go index fa65ce9b5..c6ad79d9f 100644 --- a/roomserver/internal/perform/perform_invite.go +++ b/roomserver/internal/perform/perform_invite.go @@ -223,8 +223,8 @@ func buildInviteStrippedState( // https://matrix.org/docs/spec/client_server/r0.6.0#m-room-member for _, t := range []string{ gomatrixserverlib.MRoomName, gomatrixserverlib.MRoomCanonicalAlias, - gomatrixserverlib.MRoomAliases, gomatrixserverlib.MRoomJoinRules, - "m.room.avatar", "m.room.encryption", gomatrixserverlib.MRoomCreate, + gomatrixserverlib.MRoomJoinRules, gomatrixserverlib.MRoomAvatar, + gomatrixserverlib.MRoomEncryption, gomatrixserverlib.MRoomCreate, } { stateWanted = append(stateWanted, gomatrixserverlib.StateKeyTuple{ EventType: t, diff --git a/roomserver/storage/shared/storage.go b/roomserver/storage/shared/storage.go index 4c1aae42d..e18516791 100644 --- a/roomserver/storage/shared/storage.go +++ b/roomserver/storage/shared/storage.go @@ -857,6 +857,9 @@ func (d *Database) GetStateEvent(ctx context.Context, roomID, evType, stateKey s if err != nil { return nil, err } + if roomInfo == nil || roomInfo.IsStub { + return nil, fmt.Errorf("room %s doesn't exist", roomID) + } eventTypeNID, err := d.EventTypesTable.SelectEventTypeNID(ctx, nil, evType) if err == sql.ErrNoRows { // No rooms have an event of this type, otherwise we'd have an event type NID diff --git a/sytest-whitelist b/sytest-whitelist index 4d0b9fcf5..d90ba4fb9 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -537,3 +537,6 @@ Remote servers should reject attempts by non-creators to set the power levels Federation handles empty auth_events in state_ids sanely Key notary server should return an expired key if it can't find any others Key notary server must not overwrite a valid key with a spurious result from the origin server +GET /rooms/:room_id/aliases lists aliases +Only room members can list aliases of a room +Users with sufficient power-level can delete other's aliases