diff --git a/app/models/columns/json_path_column.rb b/app/models/columns/json_path_column.rb index f78585688..523b065ff 100644 --- a/app/models/columns/json_path_column.rb +++ b/app/models/columns/json_path_column.rb @@ -1,19 +1,32 @@ # frozen_string_literal: true -class Columns::JSONPathColumn < Column - def column - "#{@column}->#{value_column}" # override column otherwise json path facets will have same id as other +class Columns::JSONPathColumn < Columns::ChampColumn + attr_reader :jsonpath + + def initialize(procedure_id:, label:, stable_id:, jsonpath:, displayable:, type: :text) + @jsonpath = quote_string(jsonpath) + + super( + procedure_id:, + label:, + stable_id:, + displayable:, + type: + ) end - def filtered_ids(dossiers, search_occurences) - queries = Array.new(search_occurences.count, "(#{json_path_query_part} ILIKE ?)").join(' OR ') + def filtered_ids(dossiers, search_terms) + value = quote_string(search_terms.join('|')) + + condition = %{champs.value_json @? '#{jsonpath} ? (@ like_regex "#{value}" flag "i")'} + dossiers.with_type_de_champ(stable_id) - .where(queries, *(search_occurences.map { |value| "%#{value}%" })) + .where(condition) .ids end def options_for_select - case value_column.last + case jsonpath.split('.').last when 'departement_code' APIGeoService.departements.map { ["#{_1[:code]} – #{_1[:name]}", _1[:code]] } when 'region_name' @@ -25,34 +38,11 @@ class Columns::JSONPathColumn < Column private + def column_id = "type_de_champ/#{stable_id}-#{jsonpath}" + def typed_value(champ) - champ.value_json&.dig(*value_column) + champ.value_json&.dig(*jsonpath.split('.')[1..]) end - def stable_id - @column - end - - # given a value_column as ['value_json', 'address', 'postal_code'] - # build SQL query as 'champs'.'value_json'->'address'->>'postal_code' - # see: https://www.postgresql.org/docs/9.5/functions-json.html - def json_path_query_part - *json_segments, key = value_column - - if json_segments.blank? # not nested, only access using ->> Get JSON array element as text - "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->>#{quote_json_segment(key)}" - else # nested, have to dig in json using -> Get JSON object field by key - field_accessor = json_segments.map(&method(:quote_json_segment)).join('->') - - "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->#{field_accessor}->>#{quote_json_segment(key)}" - end - end - - def quote_table_column(table_or_column) - ActiveRecord::Base.connection.quote_column_name(table_or_column) - end - - def quote_json_segment(path) - "'#{path}'" - end + def quote_string(string) = ActiveRecord::Base.connection.quote_string(string) end diff --git a/app/models/concerns/addressable_column_concern.rb b/app/models/concerns/addressable_column_concern.rb index b439d50bc..23c80acb7 100644 --- a/app/models/concerns/addressable_column_concern.rb +++ b/app/models/concerns/addressable_column_concern.rb @@ -6,19 +6,18 @@ module AddressableColumnConcern included do def columns(procedure_id:, displayable: true, prefix: nil) super.concat([ - ["code postal (5 chiffres)", ['postal_code'], :text], - ["commune", ['city_name'], :text], - ["département", ['departement_code'], :enum], - ["region", ['region_name'], :enum] - ].map do |(label, value_column, type)| + ["code postal (5 chiffres)", '$.postal_code', :text], + ["commune", '$.city_name', :text], + ["département", '$.departement_code', :enum], + ["region", '$.region_name', :enum] + ].map do |(label, jsonpath, type)| Columns::JSONPathColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id, + stable_id:, label: "#{libelle_with_prefix(prefix)} – #{label}", - displayable: false, - type:, - value_column: + jsonpath:, + displayable:, + type: ) end) end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 753bf47e5..78ea26d4c 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -67,29 +67,6 @@ ], "note": "filtered by rails query params where(something: ?, values)" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "5ba3f5d525b15c710215829e0db49f58e8cca06d68eff5931ebfd7d0ca0e35de", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/columns/json_path_column.rb", - "line": 11, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "dossiers.with_type_de_champ(stable_id).where(\"#{search_occurences.count} OR #{\"(#{json_path_query_part} ILIKE ?)\"}\", *search_occurences.map do\n \"%#{value}%\"\n end)", - "render_path": null, - "location": { - "type": "method", - "class": "Columns::JSONPathColumn", - "method": "filtered_ids" - }, - "user_input": "search_occurences.count", - "confidence": "Weak", - "cwe_id": [ - 89 - ], - "note": "already sanitized" - }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -136,6 +113,29 @@ ], "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "83b5a474065af330c47603d1f60fc31edaab55be162825385d53b77c1c98a6d7", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/columns/json_path_column.rb", + "line": 24, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "dossiers.with_type_de_champ(stable_id).where(\"champs.value_json @? '#{jsonpath} ? (@ like_regex \\\"#{quote_string(search_terms.join(\"|\"))}\\\" flag \\\"i\\\")'\")", + "render_path": null, + "location": { + "type": "method", + "class": "Columns::JSONPathColumn", + "method": "filtered_ids" + }, + "user_input": "jsonpath", + "confidence": "Weak", + "cwe_id": [ + 89 + ], + "note": "escaped by hand" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -295,6 +295,6 @@ "note": "Current is not a model" } ], - "updated": "2024-10-16 18:07:17 +0200", + "updated": "2024-11-04 09:56:55 +0100", "brakeman_version": "6.1.2" } diff --git a/spec/models/columns/json_path_column_spec.rb b/spec/models/columns/json_path_column_spec.rb new file mode 100644 index 000000000..0da283764 --- /dev/null +++ b/spec/models/columns/json_path_column_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +describe Columns::JSONPathColumn do + let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :address }]) } + let(:dossier) { create(:dossier, procedure:) } + let(:champ) { dossier.champs.first } + let(:stable_id) { champ.stable_id } + let(:column) { described_class.new(procedure_id: procedure.id, label: 'label', stable_id:, jsonpath:, displayable: true) } + + describe '#value' do + let(:jsonpath) { '$.city_name' } + + subject { column.value(champ) } + + context 'when champ has value_json' do + before { champ.update(value_json: { city_name: 'Grenoble' }) } + + it { is_expected.to eq('Grenoble') } + end + + context 'when champ has no value_json' do + it { is_expected.to be_nil } + end + end + + describe '#filtered_ids' do + let(:jsonpath) { '$.city_name' } + + subject { column.filtered_ids(Dossier.all, ['reno', 'Lyon']) } + + context 'when champ has value_json' do + before { champ.update(value_json: { city_name: 'Grenoble' }) } + + it { is_expected.to eq([dossier.id]) } + end + + context 'when champ has no value_json' do + it { is_expected.to eq([]) } + end + end + + describe '#initializer' do + let(:jsonpath) { %{$.'city_name} } + + it { expect(column.jsonpath).to eq(%{$.''city_name}) } + end +end diff --git a/spec/services/dossier_filter_service_spec.rb b/spec/services/dossier_filter_service_spec.rb index cb8e395bd..c1dbc539a 100644 --- a/spec/services/dossier_filter_service_spec.rb +++ b/spec/services/dossier_filter_service_spec.rb @@ -528,7 +528,7 @@ describe DossierFilterService do context "when searching by postal_code (text)" do let(:value) { "60580" } - let(:filter) { ["rna – code postal (5 chiffres)", value] } + let(:filter) { ["rna – code postal (5 chiffres)", value] } before do kept_dossier.project_champs_public.find { _1.stable_id == 1 }.update(value_json: { "postal_code" => value })