Fix permission and 404 response for alias deletion - #654 (#706)

clientapi/routing/directory.go

@@ -164,13 +164,36 @@ func SetLocalAlias(
 }
 
 // RemoveLocalAlias implements DELETE /directory/room/{roomAlias}
-// TODO: Check if the user has the power level to remove an alias
 func RemoveLocalAlias(
 	req *http.Request,
 	device *authtypes.Device,
 	alias string,
 	aliasAPI roomserverAPI.RoomserverAliasAPI,
 ) util.JSONResponse {
+
+	creatorQueryReq := roomserverAPI.GetCreatorIDForAliasRequest{
+		Alias: alias,
+	}
+	var creatorQueryRes roomserverAPI.GetCreatorIDForAliasResponse
+	if err := aliasAPI.GetCreatorIDForAlias(req.Context(), &creatorQueryReq, &creatorQueryRes); err != nil {
+		return httputil.LogThenError(req, err)
+	}
+
+	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,

roomserver/alias/alias.go

@@ -33,13 +33,16 @@ import (
 type RoomserverAliasAPIDatabase interface {
 	// Save a given room alias with the room ID it refers to.
 	// Returns an error if there was a problem talking to the database.
-	SetRoomAlias(ctx context.Context, alias string, roomID string) error
+	SetRoomAlias(ctx context.Context, alias string, roomID string, creatorUserID string) error
 	// Look up the room ID a given alias refers to.
 	// Returns an error if there was a problem talking to the database.
 	GetRoomIDForAlias(ctx context.Context, alias string) (string, error)
 	// Look up all aliases referring to a given room ID.
 	// Returns an error if there was a problem talking to the database.
 	GetAliasesForRoomID(ctx context.Context, roomID string) ([]string, error)
+	// Get the user ID of the creator of an alias.
+	// Returns an error if there was a problem talking to the database.
+	GetCreatorIDForAlias(ctx context.Context, alias string) (string, error)
 	// Remove a given room alias.
 	// Returns an error if there was a problem talking to the database.
 	RemoveRoomAlias(ctx context.Context, alias string) error
@@ -73,7 +76,7 @@ func (r *RoomserverAliasAPI) SetRoomAlias(
 	response.AliasExists = false
 
 	// Save the new alias
-	if err := r.DB.SetRoomAlias(ctx, request.Alias, request.RoomID); err != nil {
+	if err := r.DB.SetRoomAlias(ctx, request.Alias, request.RoomID, request.UserID); err != nil {
 		return err
 	}
 
@@ -133,6 +136,22 @@ func (r *RoomserverAliasAPI) GetAliasesForRoomID(
 	return nil
 }
 
+// GetCreatorIDForAlias implements alias.RoomserverAliasAPI
+func (r *RoomserverAliasAPI) GetCreatorIDForAlias(
+	ctx context.Context,
+	request *roomserverAPI.GetCreatorIDForAliasRequest,
+	response *roomserverAPI.GetCreatorIDForAliasResponse,
+) error {
+	// Look up the aliases in the database for the given RoomID
+	creatorID, err := r.DB.GetCreatorIDForAlias(ctx, request.Alias)
+	if err != nil {
+		return err
+	}
+
+	response.UserID = creatorID
+	return nil
+}
+
 // RemoveRoomAlias implements alias.RoomserverAliasAPI
 func (r *RoomserverAliasAPI) RemoveRoomAlias(
 	ctx context.Context,
@@ -277,6 +296,20 @@ func (r *RoomserverAliasAPI) SetupHTTP(servMux *http.ServeMux) {
 			return util.JSONResponse{Code: http.StatusOK, JSON: &response}
 		}),
 	)
+	servMux.Handle(
+		roomserverAPI.RoomserverGetCreatorIDForAliasPath,
+		common.MakeInternalAPI("GetCreatorIDForAlias", func(req *http.Request) util.JSONResponse {
+			var request roomserverAPI.GetCreatorIDForAliasRequest
+			var response roomserverAPI.GetCreatorIDForAliasResponse
+			if err := json.NewDecoder(req.Body).Decode(&request); err != nil {
+				return util.ErrorResponse(err)
+			}
+			if err := r.GetCreatorIDForAlias(req.Context(), &request, &response); err != nil {
+				return util.ErrorResponse(err)
+			}
+			return util.JSONResponse{Code: http.StatusOK, JSON: &response}
+		}),
+	)
 	servMux.Handle(
 		roomserverAPI.RoomserverGetAliasesForRoomIDPath,
 		common.MakeInternalAPI("getAliasesForRoomID", func(req *http.Request) util.JSONResponse {

roomserver/alias/alias_test.go

@@ -30,7 +30,7 @@ type MockRoomserverAliasAPIDatabase struct {
 }
 
 // These methods can be essentially noop
-func (db MockRoomserverAliasAPIDatabase) SetRoomAlias(ctx context.Context, alias string, roomID string) error {
+func (db MockRoomserverAliasAPIDatabase) SetRoomAlias(ctx context.Context, alias string, roomID string, creatorUserID string) error {
 	return nil
 }
 
@@ -43,6 +43,12 @@ func (db MockRoomserverAliasAPIDatabase) RemoveRoomAlias(ctx context.Context, al
 	return nil
 }
 
+func (db *MockRoomserverAliasAPIDatabase) GetCreatorIDForAlias(
+	ctx context.Context, alias string,
+) (string, error) {
+	return "", nil
+}
+
 // This method needs to change depending on test case
 func (db *MockRoomserverAliasAPIDatabase) GetRoomIDForAlias(
 	ctx context.Context,

roomserver/api/alias.go

@@ -62,6 +62,18 @@ type GetAliasesForRoomIDResponse struct {
 	Aliases []string `json:"aliases"`
 }
 
+// GetCreatorIDForAliasRequest is a request to GetCreatorIDForAlias
+type GetCreatorIDForAliasRequest struct {
+	// The alias we want to find the creator of
+	Alias string `json:"alias"`
+}
+
+// GetCreatorIDForAliasResponse is a response to GetCreatorIDForAlias
+type GetCreatorIDForAliasResponse struct {
+	// The user ID of the alias creator
+	UserID string `json:"user_id"`
+}
+
 // RemoveRoomAliasRequest is a request to RemoveRoomAlias
 type RemoveRoomAliasRequest struct {
 	// ID of the user removing the alias
@@ -96,6 +108,13 @@ type RoomserverAliasAPI interface {
 		response *GetAliasesForRoomIDResponse,
 	) error
 
+	// Get the user ID of the creator of an alias
+	GetCreatorIDForAlias(
+		ctx context.Context,
+		req *GetCreatorIDForAliasRequest,
+		response *GetCreatorIDForAliasResponse,
+	) error
+
 	// Remove a room alias
 	RemoveRoomAlias(
 		ctx context.Context,
@@ -113,6 +132,9 @@ const RoomserverGetRoomIDForAliasPath = "/api/roomserver/GetRoomIDForAlias"
 // RoomserverGetAliasesForRoomIDPath is the HTTP path for the GetAliasesForRoomID API.
 const RoomserverGetAliasesForRoomIDPath = "/api/roomserver/GetAliasesForRoomID"
 
+// RoomserverGetCreatorIDForAliasPath is the HTTP path for the GetCreatorIDForAlias API.
+const RoomserverGetCreatorIDForAliasPath = "/api/roomserver/GetCreatorIDForAlias"
+
 // RoomserverRemoveRoomAliasPath is the HTTP path for the RemoveRoomAlias API.
 const RoomserverRemoveRoomAliasPath = "/api/roomserver/removeRoomAlias"
 
@@ -169,6 +191,19 @@ func (h *httpRoomserverAliasAPI) GetAliasesForRoomID(
 	return commonHTTP.PostJSON(ctx, span, h.httpClient, apiURL, request, response)
 }
 
+// GetCreatorIDForAlias implements RoomserverAliasAPI
+func (h *httpRoomserverAliasAPI) GetCreatorIDForAlias(
+	ctx context.Context,
+	request *GetCreatorIDForAliasRequest,
+	response *GetCreatorIDForAliasResponse,
+) error {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "GetCreatorIDForAlias")
+	defer span.Finish()
+
+	apiURL := h.roomserverURL + RoomserverGetCreatorIDForAliasPath
+	return commonHTTP.PostJSON(ctx, span, h.httpClient, apiURL, request, response)
+}
+
 // RemoveRoomAlias implements RoomserverAliasAPI
 func (h *httpRoomserverAliasAPI) RemoveRoomAlias(
 	ctx context.Context,

roomserver/storage/room_aliases_table.go

@@ -25,14 +25,16 @@ CREATE TABLE IF NOT EXISTS roomserver_room_aliases (
     -- Alias of the room
     alias TEXT NOT NULL PRIMARY KEY,
     -- Room ID the alias refers to
-    room_id TEXT NOT NULL
+    room_id TEXT NOT NULL,
+    -- User ID of the creator of this alias
+    creator_id TEXT NOT NULL
 );
 
 CREATE INDEX IF NOT EXISTS roomserver_room_id_idx ON roomserver_room_aliases(room_id);
 `
 
 const insertRoomAliasSQL = "" +
-	"INSERT INTO roomserver_room_aliases (alias, room_id) VALUES ($1, $2)"
+	"INSERT INTO roomserver_room_aliases (alias, room_id, creator_id) VALUES ($1, $2, $3)"
 
 const selectRoomIDFromAliasSQL = "" +
 	"SELECT room_id FROM roomserver_room_aliases WHERE alias = $1"
@@ -40,14 +42,18 @@ const selectRoomIDFromAliasSQL = "" +
 const selectAliasesFromRoomIDSQL = "" +
 	"SELECT alias FROM roomserver_room_aliases WHERE room_id = $1"
 
+const selectCreatorIDFromAliasSQL = "" +
+	"SELECT creator_id FROM roomserver_room_aliases WHERE alias = $1"
+
 const deleteRoomAliasSQL = "" +
 	"DELETE FROM roomserver_room_aliases WHERE alias = $1"
 
 type roomAliasesStatements struct {
-	insertRoomAliasStmt         *sql.Stmt
+	insertRoomAliasStmt          *sql.Stmt
-	selectRoomIDFromAliasStmt   *sql.Stmt
+	selectRoomIDFromAliasStmt    *sql.Stmt
-	selectAliasesFromRoomIDStmt *sql.Stmt
+	selectAliasesFromRoomIDStmt  *sql.Stmt
-	deleteRoomAliasStmt         *sql.Stmt
+	selectCreatorIDFromAliasStmt *sql.Stmt
+	deleteRoomAliasStmt          *sql.Stmt
 }
 
 func (s *roomAliasesStatements) prepare(db *sql.DB) (err error) {
@@ -59,14 +65,15 @@ func (s *roomAliasesStatements) prepare(db *sql.DB) (err error) {
 		{&s.insertRoomAliasStmt, insertRoomAliasSQL},
 		{&s.selectRoomIDFromAliasStmt, selectRoomIDFromAliasSQL},
 		{&s.selectAliasesFromRoomIDStmt, selectAliasesFromRoomIDSQL},
+		{&s.selectCreatorIDFromAliasStmt, selectCreatorIDFromAliasSQL},
 		{&s.deleteRoomAliasStmt, deleteRoomAliasSQL},
 	}.prepare(db)
 }
 
 func (s *roomAliasesStatements) insertRoomAlias(
-	ctx context.Context, alias string, roomID string,
+	ctx context.Context, alias string, roomID string, creatorUserID string,
 ) (err error) {
-	_, err = s.insertRoomAliasStmt.ExecContext(ctx, alias, roomID)
+	_, err = s.insertRoomAliasStmt.ExecContext(ctx, alias, roomID, creatorUserID)
 	return
 }
 
@@ -101,6 +108,16 @@ func (s *roomAliasesStatements) selectAliasesFromRoomID(
 	return
 }
 
+func (s *roomAliasesStatements) selectCreatorIDFromAlias(
+	ctx context.Context, alias string,
+) (creatorID string, err error) {
+	err = s.selectCreatorIDFromAliasStmt.QueryRowContext(ctx, alias).Scan(&creatorID)
+	if err == sql.ErrNoRows {
+		return "", nil
+	}
+	return
+}
+
 func (s *roomAliasesStatements) deleteRoomAlias(
 	ctx context.Context, alias string,
 ) (err error) {

roomserver/storage/storage.go

@@ -441,8 +441,8 @@ func (d *Database) GetInvitesForUser(
 }
 
 // SetRoomAlias implements alias.RoomserverAliasAPIDB
-func (d *Database) SetRoomAlias(ctx context.Context, alias string, roomID string) error {
+func (d *Database) SetRoomAlias(ctx context.Context, alias string, roomID string, creatorUserID string) error {
-	return d.statements.insertRoomAlias(ctx, alias, roomID)
+	return d.statements.insertRoomAlias(ctx, alias, roomID, creatorUserID)
 }
 
 // GetRoomIDForAlias implements alias.RoomserverAliasAPIDB
@@ -455,6 +455,13 @@ func (d *Database) GetAliasesForRoomID(ctx context.Context, roomID string) ([]st
 	return d.statements.selectAliasesFromRoomID(ctx, roomID)
 }
 
+// GetCreatorIDForAlias implements alias.RoomserverAliasAPIDB
+func (d *Database) GetCreatorIDForAlias(
+	ctx context.Context, alias string,
+) (string, error) {
+	return d.statements.selectCreatorIDFromAlias(ctx, alias)
+}
+
 // RemoveRoomAlias implements alias.RoomserverAliasAPIDB
 func (d *Database) RemoveRoomAlias(ctx context.Context, alias string) error {
 	return d.statements.deleteRoomAlias(ctx, alias)