DEV: Re-use core email and username overriding logic (#60)

config/locales/server.en.yml

@@ -43,7 +43,7 @@ en:
 
     saml_user_field_statements: If provided, user fields will be set based on SAML attributes. Each entry should be in the format `saml_attribute_name:discourse_field_id`
 
-    saml_sync_email: Update the user email on every SAML login
+    saml_sync_email: On every login, override the user's email using the SAML value. Works the same as the `auth_overrides_email` setting, but is specific to SAML logins.
 
     saml_sync_moderator: Sync moderator status from SAML result?
     saml_moderator_attribute: The SAML attribute which contains the moderator boolean

lib/saml_authenticator.rb

@@ -156,19 +156,20 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator
 
     if result.user.blank?
       result.username = '' if setting(:clear_username)
-      result.omit_username = true if setting(:omit_username)
       result.user = auto_create_account(result, uid) if setting(:auto_create_account) && result.email_valid
     else
       @user = result.user
       sync_groups
       sync_custom_fields
-      sync_email(result.email, uid)
       sync_moderator
       sync_admin
       sync_trust_level
       sync_locale
     end
 
+    result.overrides_username = setting(:omit_username)
+    result.overrides_email = setting(:sync_email)
+
     result
   end
 
@@ -279,22 +280,6 @@ class SamlAuthenticator < ::Auth::OAuth2Authenticator
     end
   end
 
-  def sync_email(email, uid)
-    return unless setting(:sync_email)
-
-    email = Email.downcase(email)
-
-    return if user.email == email
-
-    existing_user = User.find_by_email(email)
-    if email =~ EmailValidator.email_regex && existing_user.nil?
-      user.email = email
-      user.save
-
-      user.oauth2_user_infos.where(provider: name, uid: uid).update_all(email: email)
-    end
-  end
-
   def sync_moderator
     return unless setting(:sync_moderator)
 

spec/integration/saml_sync_attributes_spec.rb

+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe "SAML Overrides Email", type: :request do
+  fab!(:initial_email) { "initial@example.com" }
+  fab!(:initial_username) { "initialusername" }
+  fab!(:new_email) { "new@example.com" }
+  fab!(:new_username) { "newusername" }
+  fab!(:user) { Fabricate(:user, email: initial_email, username: initial_username) }
+  fab!(:uac) { Oauth2UserInfo.create!(user: user, provider: "saml", uid: "12345") }
+
+  before do
+    SiteSetting.saml_enabled = true
+
+    OmniAuth.config.test_mode = true
+    OmniAuth.config.mock_auth[:saml] = OmniAuth::AuthHash.new(
+      provider: 'saml',
+      uid: '12345',
+      info: OmniAuth::AuthHash::InfoHash.new(
+        email: new_email,
+        nickname: new_username,
+      ),
+    )
+  end
+
+  it "doesn't sync attributes by default" do
+    get "/auth/saml/callback"
+    expect(response.status).to eq(302)
+    expect(session[:current_user_id]).to eq(user.id)
+
+    user.reload
+    expect(user.email).to eq(initial_email)
+    expect(user.username).to eq(initial_username)
+  end
+
+  it 'updates user email if enabled' do
+    SiteSetting.saml_sync_email = true
+
+    get "/auth/saml/callback"
+    expect(response.status).to eq(302)
+    expect(session[:current_user_id]).to eq(user.id)
+
+    user.reload
+    expect(user.username).to eq(initial_username)
+  end
+
+  it 'updates username if enabled' do
+    SiteSetting.saml_omit_username = true
+
+    get "/auth/saml/callback"
+    expect(response.status).to eq(302)
+    expect(session[:current_user_id]).to eq(user.id)
+
+    user.reload
+    expect(user.username).to eq(new_username)
+  end
+end

spec/saml_authenticator_spec.rb

@@ -243,25 +243,6 @@ describe SamlAuthenticator do
       end
     end
 
-    describe "sync_email" do
-      let(:new_email) { "johndoe@demo.com" }
-
-      before do
-        SiteSetting.saml_sync_email = true
-        @hash = auth_hash({})
-        @hash.info.email = new_email
-      end
-
-      it 'update email in user and oauth2_user_info models' do
-        oauth2_user_info = Fabricate(:saml_user_info, uid: @uid, user: @user)
-
-        result = @authenticator.after_authenticate(@hash)
-        expect(result.user.email).to eq(new_email)
-        oauth2_user_info.reload
-        expect(oauth2_user_info.email).to eq(new_email)
-      end
-    end
-
     describe "sync_groups" do
       let(:group_names) { ["group_1", "Group_2", "GROUP_3", "group_4"] }