diff --git a/app/actions/contact_identify_action.rb b/app/actions/contact_identify_action.rb index 9afa5d019..0e8cf2f61 100644 --- a/app/actions/contact_identify_action.rb +++ b/app/actions/contact_identify_action.rb @@ -72,9 +72,11 @@ class ContactIdentifyAction def merge_contacts?(existing_contact, key) return if existing_contact.blank? - return true if params[:identifier].blank? - - # we want to prevent merging contacts with different identifiers + # Never merge into a contact that already owns an identifier unless the request + # supplies the matching one. This also covers the blank-identifier case: an + # unverified/anonymous request must not take over an identified contact just by + # matching its email or phone_number. Merges into non-identified contacts + # (anonymous dedup) are still allowed. if existing_contact.identifier.present? && existing_contact.identifier != params[:identifier] # we will remove attribute from update list @attributes_to_update.delete(key) diff --git a/spec/actions/contact_identify_action_spec.rb b/spec/actions/contact_identify_action_spec.rb index 76b370f35..70b977d69 100644 --- a/spec/actions/contact_identify_action_spec.rb +++ b/spec/actions/contact_identify_action_spec.rb @@ -107,6 +107,24 @@ describe ContactIdentifyAction do end end + context 'when a request without an identifier matches an already identified contact' do + it 'does not merge into the identified contact via a matching email' do + victim = create(:contact, account: account, identifier: 'victim_id', email: 'victim@test.com', name: 'Victim') + params = { email: 'victim@test.com', name: 'Attacker' } + result = described_class.new(contact: contact, params: params).perform + expect(result.id).not_to eq victim.id + expect(victim.reload.name).to eq 'Victim' + end + + it 'does not merge into the identified contact via a matching phone_number' do + victim = create(:contact, account: account, identifier: 'victim_id', phone_number: '+919999888877', name: 'Victim') + params = { phone_number: '+919999888877', name: 'Attacker' } + result = described_class.new(contact: contact, params: params).perform + expect(result.id).not_to eq victim.id + expect(victim.reload.name).to eq 'Victim' + end + end + context 'when contacts with blank identifiers exist and identify action is called with blank identifier' do it 'updates the attributes of contact passed in to identify action' do create(:contact, account: account, identifier: '')