FIX: respect automatic group membership when sso changes email

Previously we were only updating group membership when a user record was
first created in an SSO setting.

This corrects it so we also update it if SSO changes the email

app/models/discourse_single_sign_on.rb

@@ -100,6 +100,10 @@ class DiscourseSingleSignOn < SingleSignOn
     user.user_avatar.save! if user.user_avatar
     user.save!
 
+    if @email_changed && user.active
+      user.set_automatic_groups
+    end
+
     # The user might require approval
     user.create_reviewable
 
@@ -251,9 +255,12 @@ class DiscourseSingleSignOn < SingleSignOn
   end
 
   def change_external_attributes_and_override(sso_record, user)
+    @email_changed = false
+
     if SiteSetting.sso_overrides_email && user.email != Email.downcase(email)
       user.email = email
       user.active = false if require_activation
+      @email_changed = true
     end
 
     if SiteSetting.sso_overrides_username? && username.present?

spec/requests/session_controller_spec.rb

@@ -584,8 +584,9 @@ RSpec.describe SessionController do
     end
 
     it 'can take over an account' do
+      user = Fabricate(:user, email: 'bill@bill.com')
+
       sso = get_sso("/")
-      user = Fabricate(:user)
       sso.email = user.email
       sso.external_id = 'abc'
       sso.username = 'sam'
@@ -597,6 +598,25 @@ RSpec.describe SessionController do
       expect(logged_on_user.email).to eq(user.email)
       expect(logged_on_user.single_sign_on_record.external_id).to eq("abc")
       expect(logged_on_user.single_sign_on_record.external_username).to eq('sam')
+
+      # we are updating the email ... ensure auto group membership works
+
+      sign_out
+
+      SiteSetting.email_editable = false
+      SiteSetting.sso_overrides_email = true
+
+      group = Fabricate(:group, name: :bob, automatic_membership_email_domains: 'jane.com')
+      sso = get_sso("/")
+      sso.email = "hello@jane.com"
+      sso.external_id = 'abc'
+
+      get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers
+
+      logged_on_user = Discourse.current_user_provider.new(request.env).current_user
+
+      expect(logged_on_user.email).to eq('hello@jane.com')
+      expect(group.users.count).to eq(1)
     end
 
     def sso_for_ip_specs
@@ -722,6 +742,8 @@ RSpec.describe SessionController do
     end
 
     it 'allows you to create an account' do
+      group = Fabricate(:group, name: :bob, automatic_membership_email_domains: 'bob.com')
+
       sso = get_sso('/a/')
       sso.external_id = '666' # the number of the beast
       sso.email = 'bob@bob.com'
@@ -742,6 +764,8 @@ RSpec.describe SessionController do
 
       logged_on_user = Discourse.current_user_provider.new(request.env).current_user
 
+      expect(group.users.where(id: logged_on_user.id).count).to eq(1)
+
       # ensure nothing is transient
       logged_on_user = User.find(logged_on_user.id)
 

spec/support/integration_helpers.rb

@@ -30,6 +30,10 @@ module IntegrationHelpers
     user
   end
 
+  def sign_out
+    delete "/session"
+  end
+
   def read_secure_session
     SecureSession.new(session[:secure_session_id])
   end