Return HTTP errors when trying to kick invalid users

Room integrity was never compromised as GMSL does auth checks,
but we would incorrectly 200 OK the request instead of 403ing.
This commit is contained in:
Kegan Dougal 2020-07-24 17:06:53 +01:00
parent b63fa7b880
commit be55ad4702
3 changed files with 12 additions and 4 deletions

View file

@ -96,6 +96,7 @@ func SendKick(
req *http.Request, accountDB accounts.Database, device *userapi.Device,
roomID string, cfg *config.Dendrite,
rsAPI roomserverAPI.RoomserverInternalAPI, asAPI appserviceAPI.AppServiceQueryAPI,
stateAPI currentstateAPI.CurrentStateInternalAPI,
) util.JSONResponse {
body, evTime, roomVer, reqErr := extractRequestData(req, roomID, rsAPI)
if reqErr != nil {
@ -108,6 +109,11 @@ func SendKick(
}
}
errRes := checkMemberInRoom(req.Context(), stateAPI, device.UserID, roomID)
if errRes != nil {
return *errRes
}
var queryRes roomserverAPI.QueryMembershipForUserResponse
err := rsAPI.QueryMembershipForUser(req.Context(), &roomserverAPI.QueryMembershipForUserRequest{
RoomID: roomID,
@ -116,11 +122,11 @@ func SendKick(
if err != nil {
return util.ErrorResponse(err)
}
// kick is only valid if the user is not currently banned
if queryRes.Membership == "ban" {
// kick is only valid if the user is not currently banned or left (that is, they are joined or invited)
if queryRes.Membership != "join" && queryRes.Membership != "invite" {
return util.JSONResponse{
Code: 403,
JSON: jsonerror.Unknown("cannot /kick banned users"),
JSON: jsonerror.Unknown("cannot /kick banned or left users"),
}
}
// TODO: should we be using SendLeave instead?

View file

@ -155,7 +155,7 @@ func Setup(
if err != nil {
return util.ErrorResponse(err)
}
return SendKick(req, accountDB, device, vars["roomID"], cfg, rsAPI, asAPI)
return SendKick(req, accountDB, device, vars["roomID"], cfg, rsAPI, asAPI, stateAPI)
}),
).Methods(http.MethodPost, http.MethodOptions)
r0mux.Handle("/rooms/{roomID}/unban",

View file

@ -413,3 +413,5 @@ A full_state incremental update returns only recent timeline
A prev_batch token can be used in the v1 messages API
We don't send redundant membership state across incremental syncs by default
Typing notifications don't leak
Users cannot kick users from a room they are not in
Users cannot kick users who have already left a room