FEATURE: Fetch email from auth provider if current user email is invalid (#7163)

approved

#1

FEATURE: Fetch email from auth provider if current user email is invalid (#7163)

If the existing email address for a user ends in .invalid, we should take the email address from an authentication payload, and replace the invalid address. This typically happens when we import users from a system without email addresses.

This commit also adds some extensibility so that plugin authenticators can define always_update_user_email?

diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb
index 1d21453..12c9b8d 100644
--- a/lib/auth/managed_authenticator.rb
+++ b/lib/auth/managed_authenticator.rb
@@ -24,6 +24,10 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     true
   end
 
+  def always_update_user_email?
+    false
+  end
+
   def revoke(user, skip_remote: false)
     association = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)
     raise Discourse::NotFound if association.nil?
@@ -60,6 +64,17 @@ 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)
diff --git a/spec/components/auth/managed_authenticator_spec.rb b/spec/components/auth/managed_authenticator_spec.rb
index f088f4f..2af82a2 100644
--- a/spec/components/auth/managed_authenticator_spec.rb
+++ b/spec/components/auth/managed_authenticator_spec.rb
@@ -180,6 +180,35 @@ describe Auth::ManagedAuthenticator do
       end
     end
 
+    describe "email update" do
+      let(: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
       let(:user) { Fabricate(:user) }
       let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") }

GitHub sha: fc0cf3ec


Approved #2