From eb70f05e8eefbf664669cee0becb10096a4de369 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 21 Jul 2021 16:28:34 +0100 Subject: [PATCH] Fix some more bugs --- clientapi/routing/directory.go | 68 ++++++---------------------- roomserver/api/alias.go | 7 ++- roomserver/internal/alias.go | 29 +++++++++++- roomserver/storage/shared/storage.go | 3 ++ 4 files changed, 50 insertions(+), 57 deletions(-) diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go index 10b56d152..ae4660656 100644 --- a/clientapi/routing/directory.go +++ b/clientapi/routing/directory.go @@ -196,60 +196,6 @@ func RemoveLocalAlias( alias string, rsAPI roomserverAPI.RoomserverInternalAPI, ) util.JSONResponse { - - creatorQueryReq := roomserverAPI.GetCreatorIDForAliasRequest{ - Alias: alias, - } - var creatorQueryRes roomserverAPI.GetCreatorIDForAliasResponse - if err := rsAPI.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 { - // Query the roomserver API to check if the alias exists locally. - roomReq := &roomserverAPI.GetRoomIDForAliasRequest{ - Alias: alias, - } - roomRes := &roomserverAPI.GetRoomIDForAliasResponse{} - if err := rsAPI.GetRoomIDForAlias(req.Context(), roomReq, roomRes); err != nil { - util.GetLogger(req.Context()).WithError(err).Error("rsAPI.GetRoomIDForAlias failed") - return jsonerror.InternalServerError() - } - - // Now request the power levels so that we can see if we have enough power to remove it. - queryEventsReq := roomserverAPI.QueryLatestEventsAndStateRequest{ - RoomID: roomRes.RoomID, - StateToFetch: []gomatrixserverlib.StateKeyTuple{{ - EventType: gomatrixserverlib.MRoomPowerLevels, - StateKey: "", - }}, - } - var queryEventsRes roomserverAPI.QueryLatestEventsAndStateResponse - err := rsAPI.QueryLatestEventsAndState(req.Context(), &queryEventsReq, &queryEventsRes) - if err != nil || len(queryEventsRes.StateEvents) == 0 { - util.GetLogger(req.Context()).WithError(err).Error("could not query events from room") - return jsonerror.InternalServerError() - } - - // NOTSPEC: Check if the user's power is greater than power required to change m.room.canonical_alias event. - // Despite being NOTSPEC, there seems to be a sytest for this? - power, _ := gomatrixserverlib.NewPowerLevelContentFromEvent(queryEventsRes.StateEvents[0].Event) - if power.UserLevel(device.UserID) < power.EventLevel(gomatrixserverlib.MRoomCanonicalAlias, true) { - 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, @@ -260,6 +206,20 @@ func RemoveLocalAlias( 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{}{}, 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 42d742a9b..7995279d2 100644 --- a/roomserver/internal/alias.go +++ b/roomserver/internal/alias.go @@ -150,13 +150,37 @@ func (r *RoomserverInternalAPI) RemoveRoomAlias( request *api.RemoveRoomAliasRequest, response *api.RemoveRoomAliasResponse, ) error { + roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias) + if err != nil { + return fmt.Errorf("r.DB.GetRoomIDForAlias: %w", err) + } + if roomID == "" { + response.Found = false + response.Removed = false + return nil + } + + response.Found = true creatorID, err := r.DB.GetCreatorIDForAlias(ctx, request.Alias) if err != nil { - return err + return fmt.Errorf("r.DB.GetCreatorIDForAlias: %w", err) } if creatorID != request.UserID { - return fmt.Errorf("not allowed to delete this alias") + 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 @@ -164,5 +188,6 @@ func (r *RoomserverInternalAPI) RemoveRoomAlias( return err } + response.Removed = true return nil } diff --git a/roomserver/storage/shared/storage.go b/roomserver/storage/shared/storage.go index 4c1aae42d..36642600a 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 { + 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