DEV: Introduce Auth::Result API for overrides_* (#15378)

This allows authenticators to instruct the Auth::Result to override attributes without using the general site settings. This provides an easy migration path for auth plugins which offer their own "overrides email", "overrides username" or "overrides name" settings. With this new api, they can set `overrides_*` on the result object, and the attribute will be overriden regardless of the general site setting.

ManagedAuthenticator is updated to use this new API. Plugins which consume ManagedAuthenticator will instantly take advantage of this change.

lib/auth/managed_authenticator.rb

@@ -77,17 +77,6 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     # Save to the DB. Do this even if we don't have a user - it might be linked up later in after_create_account
     association.save!
 
-    # Update the user's email address from the auth payload
-    if association.user &&
-        (always_update_user_email? || association.user.email.end_with?(".invalid")) &&
-        primary_email_verified?(auth_token) &&
-        (email = auth_token.dig(:info, :email)) &&
-        (email != association.user.email) &&
-        !User.find_by_email(email)
-
-      association.user.update!(email: email)
-    end
-
     # Update avatar/profile
     retrieve_avatar(association.user, association.info["image"])
     retrieve_profile(association.user, association.info)
@@ -104,6 +93,7 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     end
     result.username = info[:nickname]
     result.email_valid = primary_email_verified?(auth_token) if result.email.present?
+    result.overrides_email = always_update_user_email?
     result.extra_data = {
       provider: auth_token[:provider],
       uid: auth_token[:uid]

lib/auth/result.rb

@@ -15,14 +15,16 @@ class Auth::Result
     :requires_invite,
     :not_allowed_from_ip_address,
     :admin_not_allowed_from_ip_address,
-    :omit_username, # Used by plugins to prevent username edits
     :skip_email_validation,
     :destination_url,
     :omniauth_disallow_totp,
     :failed,
     :failed_reason,
     :failed_code,
-    :associated_groups
+    :associated_groups,
+    :overrides_email,
+    :overrides_username,
+    :overrides_name,
   ]
 
   attr_accessor *ATTRIBUTES
@@ -33,12 +35,14 @@ class Auth::Result
     :email,
     :username,
     :email_valid,
-    :omit_username,
     :name,
     :authenticator_name,
     :extra_data,
     :skip_email_validation,
-    :associated_groups
+    :associated_groups,
+    :overrides_email,
+    :overrides_username,
+    :overrides_name,
   ]
 
   def [](key)
@@ -79,16 +83,19 @@ class Auth::Result
 
   def apply_user_attributes!
     change_made = false
-    if SiteSetting.auth_overrides_username? && username.present?
+    if (SiteSetting.auth_overrides_username? || overrides_username) && username.present?
       change_made = UsernameChanger.override(user, username)
     end
 
-    if SiteSetting.auth_overrides_email && email_valid && email.present? && user.email != Email.downcase(email)
+    if (SiteSetting.auth_overrides_email || overrides_email || user&.email&.ends_with?(".invalid")) &&
+        email_valid &&
+        email.present? &&
+        user.email != Email.downcase(email)
       user.email = email
       change_made = true
     end
 
-    if SiteSetting.auth_overrides_name && name.present? && user.name != name
+    if (SiteSetting.auth_overrides_name || overrides_name) && name.present? && user.name != name
       user.name = name
       change_made = true
     end
@@ -120,11 +127,11 @@ class Auth::Result
   end
 
   def can_edit_name
-    !SiteSetting.auth_overrides_name
+    !(SiteSetting.auth_overrides_name || overrides_name)
   end
 
   def can_edit_username
-    !(SiteSetting.auth_overrides_username || omit_username)
+    !(SiteSetting.auth_overrides_username || overrides_username)
   end
 
   def to_client_hash

spec/components/auth/managed_authenticator_spec.rb

@@ -222,35 +222,6 @@ describe Auth::ManagedAuthenticator do
       end
     end
 
-    describe "email update" do
-      fab!(:user) { Fabricate(:user) }
-      let!(:associated) { UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234") }
-
-      it "updates the user's email if currently invalid" do
-        user.update!(email: "someemail@discourse.org")
-        # Existing email is valid, do not change
-        expect { result = authenticator.after_authenticate(hash) }
-          .not_to change { user.reload.email }
-
-        user.update!(email: "someemail@discourse.invalid")
-        # Existing email is invalid, expect change
-        expect { result = authenticator.after_authenticate(hash) }
-          .to change { user.reload.email }
-
-        expect(user.email).to eq("awesome@example.com")
-      end
-
-      it "doesn't raise error if email is taken" do
-        other_user = Fabricate(:user, email: "awesome@example.com")
-        user.update!(email: "someemail@discourse.invalid")
-
-        expect { result = authenticator.after_authenticate(hash) }
-          .not_to change { user.reload.email }
-
-        expect(user.email).to eq("someemail@discourse.invalid")
-      end
-    end
-
     describe "avatar on create" do
       fab!(:user) { Fabricate(:user) }
       let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") }

spec/lib/auth/result_spec.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+describe Auth::Result do
+  fab!(:initial_email) { "initialemail@example.org" }
+  fab!(:initial_username) { "initialusername" }
+  fab!(:initial_name) { "Initial Name" }
+  fab!(:user) { Fabricate(:user, email: initial_email, username: initial_username, name: initial_name) }
+
+  let(:new_email) { "newemail@example.org" }
+  let(:new_username) { "newusername" }
+  let(:new_name) { "New Name" }
+
+  let(:result) do
+    result = Auth::Result.new
+    result.email = new_email
+    result.username = new_username
+    result.name = new_name
+    result.user = user
+    result.email_valid = true
+    result
+  end
+
+  it "doesn't override user attributes by default" do
+    result.apply_user_attributes!
+    expect(user.email).to eq(initial_email)
+    expect(user.username).to eq(initial_username)
+    expect(user.name).to eq(initial_name)
+  end
+
+  it "overrides user attributes when site settings enabled" do
+    SiteSetting.email_editable = false
+    SiteSetting.auth_overrides_email = true
+    SiteSetting.auth_overrides_name = true
+    SiteSetting.auth_overrides_username = true
+
+    result.apply_user_attributes!
+
+    expect(user.email).to eq(new_email)
+    expect(user.username).to eq(new_username)
+    expect(user.name).to eq(new_name)
+  end
+
+  it "overrides user attributes when result attributes set" do
+    result.overrides_email = true
+    result.overrides_name = true
+    result.overrides_username = true
+
+    result.apply_user_attributes!
+
+    expect(user.email).to eq(new_email)
+    expect(user.username).to eq(new_username)
+    expect(user.name).to eq(new_name)
+  end
+
+  it "updates the user's email if currently invalid" do
+    user.update!(email: "someemail@discourse.org")
+    expect { result.apply_user_attributes! }.not_to change { user.email }
+
+    user.update!(email: "someemail@discourse.invalid")
+    expect { result.apply_user_attributes! }.to change { user.email }
+
+    expect(user.email).to eq(new_email)
+  end
+end

spec/requests/omniauth_callbacks_controller_spec.rb

@@ -411,7 +411,7 @@ RSpec.describe Users::OmniauthCallbacksController do
         expect(user.confirm_password?("securepassword")).to eq(false)
       end
 
-      it "should update name/username/email when sso_overrides is enabled" do
+      it "should update name/username/email when SiteSetting.auth_overrides_* are enabled" do
         SiteSetting.email_editable = false
         SiteSetting.auth_overrides_email = true
         SiteSetting.auth_overrides_name = true