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.
MODIFY
clientapi/routing/membership.go
clientapi/routing/membership.go
@@ -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?

MODIFY
clientapi/routing/routing.go
clientapi/routing/routing.go
@@ -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",

MODIFY
sytest-whitelist
sytest-whitelist
@@ -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

Return HTTP errors when trying to kick invalid users (#1221)

Room integrity was never compromised as GMSL does auth checks,
but we would incorrectly 200 OK the request instead of 403ing.