FIX: user_id handling on remove user from group

Under some conditions it was possible to pass in a user_id as an
integer, but we would try and parse it as a comma delimited string
resulting in an error. This has been fixed so that we are no longer
mapping the user_id param to user_ids.
This commit is contained in:
Blake Erickson 2019-01-24 17:28:48 -07:00
parent 0947fa2bad
commit de47b35b2d
2 changed files with 22 additions and 2 deletions

View File

@ -327,7 +327,6 @@ class GroupsController < ApplicationController
# Maintain backwards compatibility
params[:usernames] = params[:username] if params[:username].present?
params[:user_ids] = params[:user_id] if params[:user_id].present?
params[:user_emails] = params[:user_email] if params[:user_email].present?
users = users_from_params
@ -492,8 +491,11 @@ class GroupsController < ApplicationController
if params[:usernames].present?
users = User.where(username_lower: params[:usernames].split(",").map(&:downcase))
raise Discourse::InvalidParameters.new(:usernames) if users.blank?
elsif params[:user_id].present?
users = User.where(id: params[:user_id].to_i)
raise Discourse::InvalidParameters.new(:user_id) if users.blank?
elsif params[:user_ids].present?
users = User.where(id: params[:user_ids].split(","))
users = User.where(id: params[:user_ids].to_s.split(","))
raise Discourse::InvalidParameters.new(:user_ids) if users.blank?
elsif params[:user_emails].present?
users = User.with_email(params[:user_emails].split(","))

View File

@ -1017,6 +1017,15 @@ describe GroupsController do
expect(response.status).to eq(200)
end
it "removes by id with integer in json" do
expect do
headers = { "CONTENT_TYPE": "application/json" }
delete "/groups/#{group.id}/members.json", params: "{\"user_id\":#{user.id}}", headers: headers
end.to change { group.users.count }.by(-1)
expect(response.status).to eq(200)
end
it "removes by username" do
expect do
delete "/groups/#{group.id}/members.json", params: { username: user.username }
@ -1102,6 +1111,15 @@ describe GroupsController do
expect(response.status).to eq(200)
end
it "removes by id with integer in json" do
expect do
headers = { "CONTENT_TYPE": "application/json" }
delete "/groups/#{group1.id}/members.json", params: "{\"user_ids\":#{user1.id}}", headers: headers
end.to change { group1.users.count }.by(-1)
expect(response.status).to eq(200)
end
it "removes by email" do
expect do
delete "/groups/#{group1.id}/members.json",