fix: do not merge into an identified contact without a matching identifier
ContactIdentifyAction merges an incoming contact into an existing one that matches by identifier, email, or phone_number. merge_contacts? returned true whenever the request supplied no identifier, short-circuiting before the guard that prevents merging contacts with mismatched identifiers. As a result a request with no identifier could merge into (and overwrite) a contact that is already identified, purely by matching its email or phone_number. Drop the blank-identifier short-circuit so the identifier-mismatch guard runs in that case too: a contact that owns an identifier is only merged into when the request supplies the same identifier. Merges into non-identified contacts (anonymous dedup) are unchanged.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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: '')
|
||||
|
||||
Reference in New Issue
Block a user