From 86f8734bc0b245f23b4268aae0e485d060fccb9d Mon Sep 17 00:00:00 2001 From: David Taylor 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. --- lib/auth/managed_authenticator.rb | 2 +- spec/components/auth/managed_authenticator_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb index 95746b6ead7..666c5a1cd8b 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 5153debeb93..e02a45d3b20 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