diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb index 4331f90c2..88e2731f5 100644 --- a/app/services/filter_service.rb +++ b/app/services/filter_service.rb @@ -161,7 +161,10 @@ class FilterService when 'date' Date.iso8601(raw_value.to_s) when 'numeric' - BigDecimal(raw_value.to_s) + decimal = BigDecimal(raw_value.to_s) + raise CustomExceptions::CustomFilter::InvalidValue.new(attribute_name: attribute_key) unless decimal.finite? + + decimal else raise CustomExceptions::CustomFilter::InvalidValue.new(attribute_name: attribute_key) end @@ -195,8 +198,11 @@ class FilterService end def validate_query_operator - @params[:payload].each do |query_hash| + @params[:payload].each_with_index do |query_hash, index| validate_single_condition(query_hash) + next unless index == @params[:payload].length - 1 + + raise CustomExceptions::CustomFilter::InvalidQueryOperator.new({}) if query_hash['query_operator'].present? end end end diff --git a/app/services/filters/custom_attribute_filter_helper.rb b/app/services/filters/custom_attribute_filter_helper.rb index f0715c611..aac9cea6c 100644 --- a/app/services/filters/custom_attribute_filter_helper.rb +++ b/app/services/filters/custom_attribute_filter_helper.rb @@ -20,6 +20,7 @@ module Filters::CustomAttributeFilterHelper end def build_custom_attr_query(query_hash, current_index) + validate_custom_attribute_values!(query_hash) filter_operator_value = filter_operation(query_hash, current_index) query_operator = query_hash[:query_operator] table_name = attribute_model == 'conversation_attribute' ? 'conversations' : 'contacts' @@ -37,6 +38,16 @@ module Filters::CustomAttributeFilterHelper query + not_in_custom_attr_query(table_name, query_hash, attribute_data_type) end + def validate_custom_attribute_values!(query_hash) + return unless @attribute_data_type.in?(%w[date numeric]) + return if query_hash[:filter_operator].in?(%w[is_present is_not_present]) + return if @attribute_data_type == 'date' && query_hash[:filter_operator] == 'days_before' + + Array(query_hash[:values]).each do |value| + coerce_lt_gt_value(value, @attribute_data_type, @attribute_key) + end + end + def custom_attribute(attribute_key, account, custom_attribute_type) current_account = account || Current.account attribute_model = custom_attribute_type.presence || self.class::ATTRIBUTE_MODEL diff --git a/spec/services/contacts/filter_service_spec.rb b/spec/services/contacts/filter_service_spec.rb index 77a543011..1a25f862d 100644 --- a/spec/services/contacts/filter_service_spec.rb +++ b/spec/services/contacts/filter_service_spec.rb @@ -460,6 +460,52 @@ describe Contacts::FilterService do expect(result[:contacts].pluck(:id)).to eq([cs_contact.id]) end + it 'filters custom date attributes by days before' do + cs_contact.update!(custom_attributes: cs_contact.custom_attributes.merge('signed_in_at' => (Time.zone.today - 4.days).to_s)) + el_contact.update!(custom_attributes: el_contact.custom_attributes.merge('signed_in_at' => (Time.zone.today - 2.days).to_s)) + params[:payload] = [ + { + attribute_key: 'signed_in_at', + filter_operator: 'days_before', + values: [3], + query_operator: nil + }.with_indifferent_access + ] + + result = filter_service.new(account, first_user, params).perform + + expect(result[:contacts].pluck(:id)).to include(cs_contact.id) + expect(result[:contacts].pluck(:id)).not_to include(el_contact.id) + end + + it 'rejects blank custom numeric values' do + params[:payload] = [ + { + attribute_key: 'lifetime_value', + values: [''], + query_operator: nil + }.with_indifferent_access + ] + + expect { filter_service.new(account, first_user, params).perform }.to raise_error(CustomExceptions::CustomFilter::InvalidValue) + end + + it 'rejects non-finite custom numeric values' do + %w[Infinity NaN].each do |value| + params[:payload] = [ + { + attribute_key: 'lifetime_value', + filter_operator: 'is_greater_than', + values: [value], + query_operator: nil + }.with_indifferent_access + ] + + expect { filter_service.new(account, first_user, params).perform } + .to raise_error(CustomExceptions::CustomFilter::InvalidValue) + end + end + it 'rejects invalid custom date comparison values' do malicious_value = "2024-01-01'::date OR (SELECT pg_sleep(5)) IS NOT NULL --" params[:payload] = [ diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index 5dc1a35f4..60c4f661c 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -420,6 +420,20 @@ describe Conversations::FilterService do expect { filter_service.new(params, user_1, account).perform }.to raise_error(CustomExceptions::CustomFilter::InvalidQueryOperator) end + + it 'rejects a query operator on the final condition' do + params[:payload] = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'], + query_operator: 'AND', + custom_attribute_type: '' + }.with_indifferent_access + ] + + expect { filter_service.new(params, user_1, account).perform }.to raise_error(CustomExceptions::CustomFilter::InvalidQueryOperator) + end end end @@ -427,6 +441,28 @@ describe Conversations::FilterService do context 'with query present' do let!(:params) { { payload: [], page: 1 } } + it 'filters custom date attributes by days before' do + en_conversation_1.update!( + custom_attributes: en_conversation_1.custom_attributes.merge('conversation_created' => (Time.zone.today - 4.days).to_s) + ) + en_conversation_2.update!( + custom_attributes: en_conversation_2.custom_attributes.merge('conversation_created' => (Time.zone.today - 2.days).to_s) + ) + params[:payload] = [ + { + attribute_key: 'conversation_created', + filter_operator: 'days_before', + values: [3], + query_operator: nil + }.with_indifferent_access + ] + + result = filter_service.new(params, user_1, account).perform + + expect(result[:conversations].pluck(:id)).to include(en_conversation_1.id) + expect(result[:conversations].pluck(:id)).not_to include(en_conversation_2.id) + end + it 'filter by custom_attributes and labels' do user_2_assigned_conversation.update_labels('support') params[:payload] = [