Fix some more bugs

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{}{},

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

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

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