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

@@ -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,
+	}
+}

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)
 	}
 

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"),

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 {

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

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-20210721155151-4575fad563b0 h1:W1oqIcus66YGUb2HkawPTrU3OKqT3PXhXpBKoxEwiiA=
-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/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=

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

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"`
+}

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
+	return nil
-	// 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)
 }
 
 // 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
+	response.Removed = true
-	// At this point we've already committed the alias to the database so we
+	return nil
-	// 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()
 }

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,
+		gomatrixserverlib.MRoomJoinRules, gomatrixserverlib.MRoomAvatar,
-		"m.room.avatar", "m.room.encryption", gomatrixserverlib.MRoomCreate,
+		gomatrixserverlib.MRoomEncryption, gomatrixserverlib.MRoomCreate,
 	} {
 		stateWanted = append(stateWanted, gomatrixserverlib.StateKeyTuple{
 			EventType: t,

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

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