FIX: automatic group membership when using SSO

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
 

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
+    name.present? && name != username ? "#{name} (#{username})" : 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")

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

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,