fix: validate identifier hash on widget contact update (#14884)
## Description The widget contact update endpoint (`PATCH /api/v1/widget/contact`) applied updates that set an `identifier` without validating the `identifier_hash`, unlike `set_user`, which already does. This change runs the same validation on the update path whenever an `identifier` is supplied, keeping identity validation consistent across the widget contact endpoints. Anonymous updates that don't pass an identifier (prechat name/email/phone and custom attributes) keep working unchanged, including on inboxes with mandatory identity validation. Fixes https://linear.app/chatwoot/issue/CW-7118 --------- Co-authored-by: Shivam Mishra <scm.mymail@gmail.com> Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
This commit is contained in:
co-authored by
Shivam Mishra
Sivin Varghese
parent
83dda621c5
commit
c8bfbadae2
@@ -2,6 +2,7 @@ class Api::V1::Widget::ContactsController < Api::V1::Widget::BaseController
|
||||
include WidgetHelper
|
||||
|
||||
before_action :validate_hmac, only: [:set_user]
|
||||
before_action :validate_hmac_for_identified_update, only: [:update]
|
||||
|
||||
def show; end
|
||||
|
||||
@@ -46,6 +47,16 @@ class Api::V1::Widget::ContactsController < Api::V1::Widget::BaseController
|
||||
@contact.identifier.present? && @contact.identifier != permitted_params[:identifier]
|
||||
end
|
||||
|
||||
# The plain update endpoint is also used for anonymous prechat updates
|
||||
# (name/email/phone/custom_attributes with no identifier), which must keep
|
||||
# working on hmac_mandatory inboxes. Only the identity-binding path, where an
|
||||
# identifier is supplied and the contact can be rebound, requires HMAC.
|
||||
def validate_hmac_for_identified_update
|
||||
return if params[:identifier].blank?
|
||||
|
||||
validate_hmac
|
||||
end
|
||||
|
||||
def validate_hmac
|
||||
return unless should_verify_hmac?
|
||||
|
||||
|
||||
@@ -116,6 +116,78 @@ RSpec.describe '/api/v1/widget/contacts', type: :request do
|
||||
end
|
||||
end
|
||||
|
||||
describe 'PATCH /api/v1/widget/contact with HMAC enforcement' do
|
||||
let(:web_widget) { create(:channel_widget, account: account, hmac_mandatory: true) }
|
||||
let!(:victim) { create(:contact, account: account, identifier: 'victim-identifier', name: 'Victim') }
|
||||
let(:correct_identifier_hash) { OpenSSL::HMAC.hexdigest('sha256', web_widget.hmac_token, 'victim-identifier') }
|
||||
|
||||
context 'when an identifier is supplied on a mandatory-hmac inbox' do
|
||||
it 'rejects when identifier_hash is omitted' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, identifier: 'victim-identifier', name: 'Attacker' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(victim.reload.name).to eq('Victim')
|
||||
end
|
||||
|
||||
it 'rejects when identifier_hash is blank' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, identifier: 'victim-identifier', identifier_hash: '', name: 'Attacker' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(victim.reload.name).to eq('Victim')
|
||||
end
|
||||
|
||||
it 'rejects when identifier_hash is null' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, identifier: 'victim-identifier', identifier_hash: nil, name: 'Attacker' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(victim.reload.name).to eq('Victim')
|
||||
end
|
||||
|
||||
it 'rejects when identifier_hash is invalid' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, identifier: 'victim-identifier',
|
||||
identifier_hash: 'DEFINITELY_INVALID_AAAAA_NOT_A_REAL_HMAC', name: 'Attacker' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:unauthorized)
|
||||
expect(victim.reload.name).to eq('Victim')
|
||||
end
|
||||
|
||||
it 'succeeds when a valid identifier_hash is provided' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, identifier: 'victim-identifier',
|
||||
identifier_hash: correct_identifier_hash, name: 'Legit' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when no identifier is supplied (anonymous prechat update)' do
|
||||
it 'allows updating name/email without an identifier_hash' do
|
||||
patch '/api/v1/widget/contact',
|
||||
params: { website_token: web_widget.website_token, email: 'prechat@test.com', name: 'Prechat User' },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(victim.reload.email).to be_nil
|
||||
expect(Contact.from_email('prechat@test.com')).to be_present
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'PATCH /api/v1/widget/contact/set_user' do
|
||||
let(:params) { { website_token: web_widget.website_token, identifier: 'test' } }
|
||||
let(:web_widget) { create(:channel_widget, account: account, hmac_mandatory: true) }
|
||||
|
||||
Reference in New Issue
Block a user