From 9aa0982cfd787d5f7bdddee2332552e1fc28dbe2 Mon Sep 17 00:00:00 2001 From: Sam Wedgwood Date: Thu, 27 Jul 2023 15:24:15 +0100 Subject: [PATCH] remove request/response structs from RemoveRoomAlias --- clientapi/routing/directory.go | 14 ++-- roomserver/api/alias.go | 18 ----- roomserver/api/api.go | 9 ++- roomserver/internal/alias.go | 78 ++++++++----------- .../internal/perform/perform_upgrade.go | 9 ++- 5 files changed, 52 insertions(+), 76 deletions(-) diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go index 42655b7e6..3ec959b4b 100644 --- a/clientapi/routing/directory.go +++ b/clientapi/routing/directory.go @@ -300,27 +300,23 @@ func RemoveLocalAlias( } } - queryReq := roomserverAPI.RemoveRoomAliasRequest{ - Alias: alias, - SenderID: deviceSenderID, - } - var queryRes roomserverAPI.RemoveRoomAliasResponse - if err := rsAPI.RemoveRoomAlias(req.Context(), &queryReq, &queryRes); err != nil { + aliasFound, aliasRemoved, err := rsAPI.RemoveRoomAlias(req.Context(), deviceSenderID, alias) + if err != nil { util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.RemoveRoomAlias failed") return util.JSONResponse{ Code: http.StatusInternalServerError, - JSON: spec.InternalServerError{}, + JSON: spec.Unknown("internal server error"), } } - if !queryRes.Found { + if !aliasFound { return util.JSONResponse{ Code: http.StatusNotFound, JSON: spec.NotFound("The alias does not exist."), } } - if !queryRes.Removed { + if !aliasRemoved { return util.JSONResponse{ Code: http.StatusForbidden, JSON: spec.Forbidden("You do not have permission to remove this alias."), diff --git a/roomserver/api/alias.go b/roomserver/api/alias.go index 0bb1386c6..6269d0b04 100644 --- a/roomserver/api/alias.go +++ b/roomserver/api/alias.go @@ -16,8 +16,6 @@ package api import ( "regexp" - - "github.com/matrix-org/gomatrixserverlib/spec" ) // GetRoomIDForAliasRequest is a request to GetRoomIDForAlias @@ -47,22 +45,6 @@ type GetAliasesForRoomIDResponse struct { Aliases []string `json:"aliases"` } -// RemoveRoomAliasRequest is a request to RemoveRoomAlias -type RemoveRoomAliasRequest struct { - // ID of the user removing the alias - SenderID spec.SenderID `json:"user_id"` - // The room alias to remove - Alias string `json:"alias"` -} - -// RemoveRoomAliasResponse is a response to RemoveRoomAlias -type RemoveRoomAliasResponse struct { - // Did the alias exist before? - Found bool `json:"found"` - // Did we remove it? - Removed bool `json:"removed"` -} - type AliasEvent struct { Alias string `json:"alias"` AltAliases []string `json:"alt_aliases"` diff --git a/roomserver/api/api.go b/roomserver/api/api.go index 2d1d363cd..ed87ce93a 100644 --- a/roomserver/api/api.go +++ b/roomserver/api/api.go @@ -237,12 +237,19 @@ type ClientRoomserverAPI interface { PerformPublish(ctx context.Context, req *PerformPublishRequest) error // PerformForget forgets a rooms history for a specific user PerformForget(ctx context.Context, req *PerformForgetRequest, resp *PerformForgetResponse) error + // Sets a room alias, as provided sender, pointing to the provided room ID. // // If err is nil, then the returned boolean indicates if the alias is already in use. // If true, then the alias has not been set to the provided room, as it already in use. SetRoomAlias(ctx context.Context, senderID spec.SenderID, roomID spec.RoomID, alias string) (aliasAlreadyExists bool, err error) - RemoveRoomAlias(ctx context.Context, req *RemoveRoomAliasRequest, res *RemoveRoomAliasResponse) error + + //RemoveRoomAlias(ctx context.Context, req *RemoveRoomAliasRequest, res *RemoveRoomAliasResponse) error + // Removes a room alias, as provided sender. + // + // Returns whether the alias was found, whether it was removed, and an error (if any occurred) + RemoveRoomAlias(ctx context.Context, senderID spec.SenderID, alias string) (aliasFound bool, aliasRemoved bool, err error) + SigningIdentityFor(ctx context.Context, roomID spec.RoomID, senderID spec.UserID) (fclient.SigningIdentity, error) } diff --git a/roomserver/internal/alias.go b/roomserver/internal/alias.go index 2f93c6e45..a7f0aab9c 100644 --- a/roomserver/internal/alias.go +++ b/roomserver/internal/alias.go @@ -116,90 +116,79 @@ func (r *RoomserverInternalAPI) GetAliasesForRoomID( // nolint:gocyclo // RemoveRoomAlias implements alias.RoomserverInternalAPI // nolint: gocyclo -func (r *RoomserverInternalAPI) RemoveRoomAlias( - ctx context.Context, - request *api.RemoveRoomAliasRequest, - response *api.RemoveRoomAliasResponse, -) error { - roomID, err := r.DB.GetRoomIDForAlias(ctx, request.Alias) +func (r *RoomserverInternalAPI) RemoveRoomAlias(ctx context.Context, senderID spec.SenderID, alias string) (aliasFound bool, aliasRemoved bool, err error) { + roomID, err := r.DB.GetRoomIDForAlias(ctx, alias) if err != nil { - return fmt.Errorf("r.DB.GetRoomIDForAlias: %w", err) + return false, false, fmt.Errorf("r.DB.GetRoomIDForAlias: %w", err) } if roomID == "" { - response.Found = false - response.Removed = false - return nil + return false, false, nil } validRoomID, err := spec.NewRoomID(roomID) if err != nil { - return err + return true, false, err } - sender, err := r.QueryUserIDForSender(ctx, *validRoomID, request.SenderID) + sender, err := r.QueryUserIDForSender(ctx, *validRoomID, senderID) if err != nil || sender == nil { - return fmt.Errorf("r.QueryUserIDForSender: %w", err) + return true, false, fmt.Errorf("r.QueryUserIDForSender: %w", err) } virtualHost := sender.Domain() - response.Found = true - creatorID, err := r.DB.GetCreatorIDForAlias(ctx, request.Alias) + creatorID, err := r.DB.GetCreatorIDForAlias(ctx, alias) if err != nil { - return fmt.Errorf("r.DB.GetCreatorIDForAlias: %w", err) + return true, false, fmt.Errorf("r.DB.GetCreatorIDForAlias: %w", err) } - if spec.SenderID(creatorID) != request.SenderID { + if spec.SenderID(creatorID) != senderID { var plEvent *types.HeaderedEvent var pls *gomatrixserverlib.PowerLevelContent plEvent, err = r.DB.GetStateEvent(ctx, roomID, spec.MRoomPowerLevels, "") if err != nil { - return fmt.Errorf("r.DB.GetStateEvent: %w", err) + return true, false, fmt.Errorf("r.DB.GetStateEvent: %w", err) } pls, err = plEvent.PowerLevels() if err != nil { - return fmt.Errorf("plEvent.PowerLevels: %w", err) + return true, false, fmt.Errorf("plEvent.PowerLevels: %w", err) } - if pls.UserLevel(request.SenderID) < pls.EventLevel(spec.MRoomCanonicalAlias, true) { - response.Removed = false - return nil + if pls.UserLevel(senderID) < pls.EventLevel(spec.MRoomCanonicalAlias, true) { + return true, false, nil } } ev, err := r.DB.GetStateEvent(ctx, roomID, spec.MRoomCanonicalAlias, "") if err != nil && err != sql.ErrNoRows { - return err + return true, false, err } else if ev != nil { stateAlias := gjson.GetBytes(ev.Content(), "alias").Str // the alias to remove is currently set as the canonical alias, remove it - if stateAlias == request.Alias { + if stateAlias == alias { res, err := sjson.DeleteBytes(ev.Content(), "alias") if err != nil { - return err + return true, false, err } - senderID := request.SenderID - if request.SenderID != ev.SenderID() { - senderID = ev.SenderID() - } - sender, err := r.QueryUserIDForSender(ctx, *validRoomID, senderID) - if err != nil || sender == nil { - return err + canonicalSenderID := ev.SenderID() + canonicalSender, err := r.QueryUserIDForSender(ctx, *validRoomID, canonicalSenderID) + if err != nil || canonicalSender == nil { + return true, false, err } validRoomID, err := spec.NewRoomID(roomID) if err != nil { - return err + return true, false, err } - identity, err := r.SigningIdentityFor(ctx, *validRoomID, *sender) + identity, err := r.SigningIdentityFor(ctx, *validRoomID, *canonicalSender) if err != nil { - return err + return true, false, err } proto := &gomatrixserverlib.ProtoEvent{ - SenderID: string(senderID), + SenderID: string(canonicalSenderID), RoomID: ev.RoomID(), Type: ev.Type(), StateKey: ev.StateKey(), @@ -208,34 +197,33 @@ func (r *RoomserverInternalAPI) RemoveRoomAlias( eventsNeeded, err := gomatrixserverlib.StateNeededForProtoEvent(proto) if err != nil { - return fmt.Errorf("gomatrixserverlib.StateNeededForEventBuilder: %w", err) + return true, false, fmt.Errorf("gomatrixserverlib.StateNeededForEventBuilder: %w", err) } if len(eventsNeeded.Tuples()) == 0 { - return errors.New("expecting state tuples for event builder, got none") + return true, false, errors.New("expecting state tuples for event builder, got none") } stateRes := &api.QueryLatestEventsAndStateResponse{} if err = helpers.QueryLatestEventsAndState(ctx, r.DB, r, &api.QueryLatestEventsAndStateRequest{RoomID: roomID, StateToFetch: eventsNeeded.Tuples()}, stateRes); err != nil { - return err + return true, false, err } newEvent, err := eventutil.BuildEvent(ctx, proto, &identity, time.Now(), &eventsNeeded, stateRes) if err != nil { - return err + return true, false, err } err = api.SendEvents(ctx, r, api.KindNew, []*types.HeaderedEvent{newEvent}, virtualHost, r.ServerName, r.ServerName, nil, false) if err != nil { - return err + return true, false, err } } } // Remove the alias from the database - if err := r.DB.RemoveRoomAlias(ctx, request.Alias); err != nil { - return err + if err := r.DB.RemoveRoomAlias(ctx, alias); err != nil { + return true, false, err } - response.Removed = true - return nil + return true, true, nil } diff --git a/roomserver/internal/perform/perform_upgrade.go b/roomserver/internal/perform/perform_upgrade.go index ad51128e1..74c62cd9e 100644 --- a/roomserver/internal/perform/perform_upgrade.go +++ b/roomserver/internal/perform/perform_upgrade.go @@ -188,10 +188,13 @@ func moveLocalAliases(ctx context.Context, } for _, alias := range aliasRes.Aliases { - removeAliasReq := api.RemoveRoomAliasRequest{SenderID: senderID, Alias: alias} - removeAliasRes := api.RemoveRoomAliasResponse{} - if err = URSAPI.RemoveRoomAlias(ctx, &removeAliasReq, &removeAliasRes); err != nil { + aliasFound, aliasRemoved, err := URSAPI.RemoveRoomAlias(ctx, senderID, alias) + if err != nil { return fmt.Errorf("Failed to remove old room alias: %w", err) + } else if !aliasFound { + return fmt.Errorf("Failed to remove old room alias: alias not found, possible race") + } else if !aliasRemoved { + return fmt.Errorf("Failed to remove old alias") } aliasAlreadyExists, err := URSAPI.SetRoomAlias(ctx, senderID, *parsedNewRoomID, alias)