FIX: Don't invite new users via group with SSO on or local logins off (#11950)
Issue originally reported in https://meta.discourse.org/t/bypass-sso-by-adding-unkown-email-to-group/177339 Inviting people via email address to a group when SSO is enabled (or local logins are disabled) led to a situation where user records were being created bypassing single sign-on. We already prevent that in most places. This adds required checks to `GroupsController`.
This commit is contained in:
parent
45931f86be
commit
704778f448
|
@ -333,15 +333,23 @@ class GroupsController < ApplicationController
|
||||||
end
|
end
|
||||||
|
|
||||||
if users.empty? && emails.empty?
|
if users.empty? && emails.empty?
|
||||||
raise Discourse::InvalidParameters.new(
|
raise Discourse::InvalidParameters.new(I18n.t("usernames_or_emails_required"))
|
||||||
'usernames or emails must be present'
|
|
||||||
)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if emails.any?
|
||||||
|
if SiteSetting.enable_sso?
|
||||||
|
raise Discourse::InvalidParameters.new(I18n.t("no_invites_with_sso"))
|
||||||
|
elsif !SiteSetting.enable_local_logins?
|
||||||
|
raise Discourse::InvalidParameters.new(I18n.t("no_invites_without_local_logins"))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
if users.length > ADD_MEMBERS_LIMIT
|
if users.length > ADD_MEMBERS_LIMIT
|
||||||
return render_json_error(
|
return render_json_error(
|
||||||
I18n.t("groups.errors.adding_too_many_users", count: ADD_MEMBERS_LIMIT)
|
I18n.t("groups.errors.adding_too_many_users", count: ADD_MEMBERS_LIMIT)
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
usernames_already_in_group = group.users.where(id: users.map(&:id)).pluck(:username)
|
usernames_already_in_group = group.users.where(id: users.map(&:id)).pluck(:username)
|
||||||
if usernames_already_in_group.present? && usernames_already_in_group.length == users.length
|
if usernames_already_in_group.present? && usernames_already_in_group.length == users.length
|
||||||
render_json_error(I18n.t(
|
render_json_error(I18n.t(
|
||||||
|
|
|
@ -447,6 +447,9 @@ en:
|
||||||
adding_too_many_users:
|
adding_too_many_users:
|
||||||
one: "Maximum %{count} user can be added at once"
|
one: "Maximum %{count} user can be added at once"
|
||||||
other: "Maximum %{count} users can be added at once"
|
other: "Maximum %{count} users can be added at once"
|
||||||
|
usernames_or_emails_required: "Usernames or emails must be present"
|
||||||
|
no_invites_with_sso: "You can invite only registered users when SSO is enabled"
|
||||||
|
no_invites_without_local_logins: "You can invite only registered users when local logins are disabled"
|
||||||
default_names:
|
default_names:
|
||||||
everyone: "everyone"
|
everyone: "everyone"
|
||||||
admins: "admins"
|
admins: "admins"
|
||||||
|
|
|
@ -1346,6 +1346,34 @@ describe GroupsController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "adds known users by email when SSO is enabled" do
|
||||||
|
SiteSetting.sso_url = "https://www.example.com/sso"
|
||||||
|
SiteSetting.enable_sso = true
|
||||||
|
|
||||||
|
expect do
|
||||||
|
put "/groups/#{group.id}/members.json", params: { emails: other_user.email }
|
||||||
|
end.to change { group.users.count }.by(1)
|
||||||
|
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "rejects unknown emails when SSO is enabled" do
|
||||||
|
SiteSetting.sso_url = "https://www.example.com/sso"
|
||||||
|
SiteSetting.enable_sso = true
|
||||||
|
put "/groups/#{group.id}/members.json", params: { emails: "newuser@example.com" }
|
||||||
|
|
||||||
|
expect(response.status).to eq(400)
|
||||||
|
expect(response.parsed_body["error_type"]).to eq("invalid_parameters")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "rejects unknown emails when local logins are disabled" do
|
||||||
|
SiteSetting.enable_local_logins = false
|
||||||
|
put "/groups/#{group.id}/members.json", params: { emails: "newuser@example.com" }
|
||||||
|
|
||||||
|
expect(response.status).to eq(400)
|
||||||
|
expect(response.parsed_body["error_type"]).to eq("invalid_parameters")
|
||||||
|
end
|
||||||
|
|
||||||
it "will find users by email, and invite the correct user" do
|
it "will find users by email, and invite the correct user" do
|
||||||
new_user = Fabricate(:user)
|
new_user = Fabricate(:user)
|
||||||
expect(new_user.group_ids.include?(group.id)).to eq(false)
|
expect(new_user.group_ids.include?(group.id)).to eq(false)
|
||||||
|
|
Loading…
Reference in New Issue