github-mirror/discourse
github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-04-08 18:50:59 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Don't attempt to add user again to a group when syncing groups via SSO (#18772)

Browse Source 
This commit fixes a regression introduced in 8979adc where under certain conditions the groups syncing logic in Discourse Connect would try to add users to groups they're already members of and cause errors when users try to sign in using Discourse Connect.
This commit is contained in:
Osama Sayegh 2022-10-28 13:27:12 +03:00 committed by GitHub
parent fa5f43e7c0
commit e120c94236
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
app/models/discourse_connect.rb
View File

@ -187,15 +187,17 @@ class DiscourseConnect < DiscourseConnectBase
def synchronize_groups(user) def synchronize_groups(user)
names = (groups || "").split(",").map(&:downcase) names = (groups || "").split(",").map(&:downcase)
to_be_added = Group.where('LOWER(NAME) in (?) AND NOT automatic', names) current_groups = user.groups.where(automatic: false)
to_be_removed = Group.joins(:group_users).where(automatic: false, group_users: { user_id: user.id }) desired_groups = Group.where('LOWER(NAME) in (?) AND NOT automatic', names)
if to_be_added.present? to_be_added = desired_groups
to_be_removed = to_be_removed.where("groups.id NOT IN (?)", to_be_added.map(&:id)).to_a if current_groups.present?
to_be_added = to_be_added.where("groups.id NOT IN (?)", current_groups.map(&:id))
end end
if to_be_removed.present? to_be_removed = current_groups
to_be_added = to_be_added.where("groups.id NOT IN (?)", to_be_removed.map(&:id)).to_a if desired_groups.present?
to_be_removed = to_be_removed.where("groups.id NOT IN (?)", desired_groups.map(&:id))
end end
if to_be_added.present? || to_be_removed.present? if to_be_added.present? || to_be_removed.present?
@ -406,12 +408,8 @@ class DiscourseConnect < DiscourseConnectBase
def add_user_to_groups(user, groups) def add_user_to_groups(user, groups)
groups.each do |group| groups.each do |group|
begin GroupUser.create!(user_id: user.id, group_id: group.id)
GroupUser.create!(user_id: user.id, group_id: group.id) GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user)
GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user)
rescue Exception => e
Discourse.warn_exception(e, message: "User already in group")
end
end end
end end

12
spec/models/discourse_connect_spec.rb
View File

@ -206,7 +206,7 @@ RSpec.describe DiscourseConnect do
expect(group2.usernames).to eq("") expect(group2.usernames).to eq("")
group1.add(user) group1.add(user)
group1.save group1.save!
sso.lookup_or_create_user(ip_address) sso.lookup_or_create_user(ip_address)
expect(group1.usernames).to eq("") expect(group1.usernames).to eq("")
@ -216,6 +216,16 @@ RSpec.describe DiscourseConnect do
sso.lookup_or_create_user(ip_address) sso.lookup_or_create_user(ip_address)
expect(group1.usernames).to eq("") expect(group1.usernames).to eq("")
expect(group2.usernames).to eq("") expect(group2.usernames).to eq("")
group1.add(user)
group1.save!
group2.add(user)
group2.save!
sso.groups = "#{group1.name},badname,trust_level_4"
sso.lookup_or_create_user(ip_address)
expect(group1.usernames).to eq(user.username)
expect(group2.usernames).to eq("")
end end
it "can specify groups" do it "can specify groups" do