From 94ea325c93d03362795ce85c49008797d37d36dd Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Wed, 7 Aug 2019 11:00:58 +0800 Subject: [PATCH] Fix permission and 404 response for alias deletion - #654 (#706) --- clientapi/routing/directory.go | 25 +++++++++++++++- roomserver/alias/alias.go | 37 ++++++++++++++++++++++-- roomserver/alias/alias_test.go | 8 ++++- roomserver/api/alias.go | 35 ++++++++++++++++++++++ roomserver/storage/room_aliases_table.go | 33 ++++++++++++++++----- roomserver/storage/storage.go | 11 +++++-- 6 files changed, 135 insertions(+), 14 deletions(-) diff --git a/clientapi/routing/directory.go b/clientapi/routing/directory.go index ab85e86a9..0d91d0426 100644 --- a/clientapi/routing/directory.go +++ b/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, diff --git a/roomserver/alias/alias.go b/roomserver/alias/alias.go index f699e3362..aeaf5ae94 100644 --- a/roomserver/alias/alias.go +++ b/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 { diff --git a/roomserver/alias/alias_test.go b/roomserver/alias/alias_test.go index 4b9ca022d..6ddb63a73 100644 --- a/roomserver/alias/alias_test.go +++ b/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, diff --git a/roomserver/api/alias.go b/roomserver/api/alias.go index 576710713..cb78f726a 100644 --- a/roomserver/api/alias.go +++ b/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, diff --git a/roomserver/storage/room_aliases_table.go b/roomserver/storage/room_aliases_table.go index f640c37fe..3ed20e8e3 100644 --- a/roomserver/storage/room_aliases_table.go +++ b/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 - selectRoomIDFromAliasStmt *sql.Stmt - selectAliasesFromRoomIDStmt *sql.Stmt - deleteRoomAliasStmt *sql.Stmt + insertRoomAliasStmt *sql.Stmt + selectRoomIDFromAliasStmt *sql.Stmt + selectAliasesFromRoomIDStmt *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) { diff --git a/roomserver/storage/storage.go b/roomserver/storage/storage.go index f6c2fccd4..71c13b7ca 100644 --- a/roomserver/storage/storage.go +++ b/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 { - return d.statements.insertRoomAlias(ctx, alias, roomID) +func (d *Database) SetRoomAlias(ctx context.Context, alias string, roomID string, creatorUserID string) error { + 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)