diff options
| author | Kegsay <kegan@matrix.org> | 2020-07-27 09:20:09 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-07-27 09:20:09 +0100 |
| commit | c8d476a3cca2fe0850373c0276144eea65d0a219 (patch) | |
| tree | b1756f28c14481a90c7e03039a0cff079e4ddaff | |
| parent | 61963a74ae84df94238cf384419acf4d59c311c1 (diff) | |
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.
| -rw-r--r-- | clientapi/routing/membership.go | 12 | ||||
| -rw-r--r-- | clientapi/routing/routing.go | 2 | ||||
| -rw-r--r-- | sytest-whitelist | 2 |
3 files changed, 12 insertions, 4 deletions
diff --git a/clientapi/routing/membership.go b/clientapi/routing/membership.go index a9a8fa00..90ddb699 100644 --- a/clientapi/routing/membership.go +++ b/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? diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index c9ed5ea5..311f64d1 100644 --- a/clientapi/routing/routing.go +++ b/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", diff --git a/sytest-whitelist b/sytest-whitelist index 5bf6d68b..234eae39 100644 --- a/sytest-whitelist +++ b/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 |