diff --git a/app/jobs/regular/automatic_group_membership.rb b/app/jobs/regular/automatic_group_membership.rb index 4e688d84000..2b53ffb49cc 100644 --- a/app/jobs/regular/automatic_group_membership.rb +++ b/app/jobs/regular/automatic_group_membership.rb @@ -4,10 +4,10 @@ module Jobs def execute(args) group_id = args[:group_id] - raise Discourse::InvalidParameters.new(:group_id) if group_id.blank? - group = Group.find(group_id) + group = Group.find_by(id: group_id) + raise Discourse::InvalidParameters.new(:group_id) if group.nil? return unless group.automatic_membership_retroactive diff --git a/app/models/user.rb b/app/models/user.rb index 2b8685f6a2c..e17796c45b5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -782,7 +782,9 @@ class User < ActiveRecord::Base end def email_confirmed? - email_tokens.where(email: email, confirmed: true).present? || email_tokens.empty? + email_tokens.where(email: email, confirmed: true).present? || + email_tokens.empty? || + single_sign_on_record&.external_email == email end def activate @@ -803,8 +805,7 @@ class User < ActiveRecord::Base end def readable_name - return "#{name} (#{username})" if name.present? && name != username - username + name.present? && name != username ? "#{name} (#{username})" : username end def badge_count @@ -1033,7 +1034,7 @@ class User < ActiveRecord::Base end def set_automatic_groups - return unless active && email_confirmed? && !staged + return if !active || staged || !email_confirmed? Group.where(automatic: false) .where("LENGTH(COALESCE(automatic_membership_email_domains, '')) > 0") diff --git a/spec/jobs/automatic_group_membership_spec.rb b/spec/jobs/automatic_group_membership_spec.rb index 55776b99e42..93758a56953 100644 --- a/spec/jobs/automatic_group_membership_spec.rb +++ b/spec/jobs/automatic_group_membership_spec.rb @@ -11,8 +11,13 @@ describe Jobs::AutomaticGroupMembership do user1 = Fabricate(:user, email: "no@bar.com") user2 = Fabricate(:user, email: "no@wat.com") user3 = Fabricate(:user, email: "noo@wat.com", staged: true) + EmailToken.confirm(user3.email_tokens.last.token) user4 = Fabricate(:user, email: "yes@wat.com") EmailToken.confirm(user4.email_tokens.last.token) + user5 = Fabricate(:user, email: "sso@wat.com") + user5.create_single_sign_on_record(external_id: 123, external_email: "hacker@wat.com", last_payload: "") + user6 = Fabricate(:user, email: "sso2@wat.com") + user6.create_single_sign_on_record(external_id: 456, external_email: "sso2@wat.com", last_payload: "") group = Fabricate(:group, automatic_membership_email_domains: "wat.com", automatic_membership_retroactive: true) @@ -23,6 +28,8 @@ describe Jobs::AutomaticGroupMembership do expect(group.users.include?(user2)).to eq(false) expect(group.users.include?(user3)).to eq(false) expect(group.users.include?(user4)).to eq(true) + expect(group.users.include?(user5)).to eq(false) + expect(group.users.include?(user6)).to eq(true) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e1471394e48..f725472bc00 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1314,6 +1314,14 @@ describe User do expect(group_history.target_user).to eq(user) end + it "is automatically added to a group when the email matches the SSO record" do + user = Fabricate(:user, active: true, email: "sso@bar.com") + user.create_single_sign_on_record(external_id: 123, external_email: "sso@bar.com", last_payload: "") + user.set_automatic_groups + group.reload + expect(group.users.include?(user)).to eq(true) + end + it "get attributes from the group" do user = Fabricate.build(:user, active: true,