FIX: Prioritize explicit 'connect' over matching by email

FIX: Prioritize explicit 'connect' over matching by email

This is an edge case that was previously handled by TwitterAuthenticator, but not FacebookAuthenticator.

From 86f8734bc0b245f23b4268aae0e485d060fccb9d Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 7 Dec 2018 14:12:08 +0000
Subject: [PATCH] FIX: Prioritize explicit 'connect' over matching by email

This is an edge case that was previously handled by TwitterAuthenticator, but not FacebookAuthenticator.

diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb
index 95746b6..666c5a1 100644
--- a/lib/auth/managed_authenticator.rb
+++ b/lib/auth/managed_authenticator.rb
@@ -55,7 +55,7 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     end
 
     # Matching an account by email
-    if match_by_email && association.nil? && (user = User.find_by_email(email))
+    if match_by_email && association.nil? && result.user.nil? && (user = User.find_by_email(email))
       UserAssociatedAccount.where(user: user, provider_name: auth_token[:provider]).destroy_all # Destroy existing associations for the new user
       result.user = user
     end
diff --git a/spec/components/auth/managed_authenticator_spec.rb b/spec/components/auth/managed_authenticator_spec.rb
index 5153deb..e02a45d 100644
--- a/spec/components/auth/managed_authenticator_spec.rb
+++ b/spec/components/auth/managed_authenticator_spec.rb
@@ -55,6 +55,14 @@ describe Auth::ManagedAuthenticator do
         expect(UserAssociatedAccount.exists?(user_id: user2.id)).to eq(true)
       end
 
+      it 'still works if another user has a matching email' do
+        Fabricate(:user, email: hash.dig(:info, :email))
+        result = authenticator.after_authenticate(hash, existing_account: user2)
+        expect(result.user.id).to eq(user2.id)
+        expect(UserAssociatedAccount.exists?(user_id: user1.id)).to eq(false)
+        expect(UserAssociatedAccount.exists?(user_id: user2.id)).to eq(true)
+      end
+
       it 'does not work when disabled' do
         authenticator = Class.new(described_class) do
           def name

GitHub

2 Likes