diff --git a/app/models/concerns/columns_concern.rb b/app/models/concerns/columns_concern.rb index 01125ab89..3f8e11a52 100644 --- a/app/models/concerns/columns_concern.rb +++ b/app/models/concerns/columns_concern.rb @@ -53,7 +53,7 @@ module ColumnsConcern Column.new(table: 'user', column: 'email'), Column.new(table: 'followers_instructeurs', column: 'email'), Column.new(table: 'groupe_instructeur', column: 'id', type: :enum), - Column.new(table: 'avis', column: 'question_answer', filterable: false) + Column.new(table: 'avis', column: 'question_answer', filterable: false) # not filterable ? ] end diff --git a/app/models/concerns/dossier_filtering_concern.rb b/app/models/concerns/dossier_filtering_concern.rb index 5e2d8e104..e102c1239 100644 --- a/app/models/concerns/dossier_filtering_concern.rb +++ b/app/models/concerns/dossier_filtering_concern.rb @@ -33,5 +33,11 @@ module DossierFilteringConcern q = Array.new(values.count, "(#{table_column} ILIKE ?)").join(' OR ') where(q, *(values.map { |value| "%#{value}%" })) } + + scope :filter_enum, lambda { |table, column, values| + table_column = ProcedurePresentation.sanitized_column(table, column) + q = Array.new(values.count, "(#{table_column} = ?)").join(' OR ') + where(q, *(values)) + } end end diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index 8994db9de..7ac5e26a1 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -220,8 +220,13 @@ class ProcedurePresentation < ApplicationRecord dossiers.where("dossiers.#{column} IN (?)", values) end when TYPE_DE_CHAMP - dossiers.with_type_de_champ(column) - .filter_ilike(:champs, value_column, values) + if dossier_column.type == :enum + dossiers.with_type_de_champ(column) + .filter_enum(:champs, value_column, values) + else + dossiers.with_type_de_champ(column) + .filter_ilike(:champs, value_column, values) + end when 'etablissement' if column == 'entreprise_date_creation' dates = values @@ -240,22 +245,17 @@ class ProcedurePresentation < ApplicationRecord dossiers .includes(:followers_instructeurs) .joins('INNER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id') - .filter_ilike('instructeurs_users', :email, values) - when 'user', 'individual', 'avis' + .filter_ilike('instructeurs_users', :email, values) # ilike OK, user may want to search by *@domain + when 'user', 'individual' # user_columns: [email], individual_columns: ['nom', 'prenom', 'gender'] dossiers .includes(table) - .filter_ilike(table, column, values) + .filter_ilike(table, column, values) # ilike or where column == 'value' are both valid, we opted for ilike when 'groupe_instructeur' assert_supported_column(table, column) - if column == 'label' - dossiers - .joins(:groupe_instructeur) - .filter_ilike(table, column, values) - else - dossiers - .joins(:groupe_instructeur) - .where(groupe_instructeur_id: values) - end + + dossiers + .joins(:groupe_instructeur) + .where(groupe_instructeur_id: values) end.pluck(:id) end end.reduce(:&) diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 2890961bb..9185336fb 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -15,7 +15,7 @@ "type": "controller", "class": "Users::DossiersController", "method": "merci", - "line": 329, + "line": 323, "file": "app/controllers/users/dossiers_controller.rb", "rendered": { "name": "users/dossiers/merci", @@ -44,6 +44,29 @@ ], "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "31693060072e27c02ca4f884f2a07f4f1c1247b7a6f5cc5c724e88e6ca9b4873", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/concerns/dossier_filtering_concern.rb", + "line": 40, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "where(\"#{values.count} OR #{\"(#{ProcedurePresentation.sanitized_column(table, column)} = ?)\"}\", *values)", + "render_path": null, + "location": { + "type": "method", + "class": "DossierFilteringConcern", + "method": null + }, + "user_input": "values.count", + "confidence": "Medium", + "cwe_id": [ + 89 + ], + "note": "filtered by rails query params where(something: ?, values)" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -51,7 +74,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/columns/json_path_column.rb", - "line": 10, + "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, @@ -74,7 +97,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/graphql/connections/cursor_connection.rb", - "line": 150, + "line": 152, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "items.order(order_column => ((:desc or :asc)), :id => ((:desc or :asc))).limit(limit).where(\"(#{order_table}.#{order_column}, #{order_table}.id) < (?, ?)\", timestamp, id)", "render_path": null, @@ -131,7 +154,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/graphql/connections/cursor_connection.rb", - "line": 153, + "line": 155, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "items.order(order_column => ((:desc or :asc)), :id => ((:desc or :asc))).limit(limit).where(\"(#{order_table}.#{order_column}, #{order_table}.id) > (?, ?)\", timestamp, id)", "render_path": null, @@ -154,7 +177,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/concerns/dossier_filtering_concern.rb", - "line": 32, + "line": 34, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "where(\"#{values.count} OR #{\"(#{ProcedurePresentation.sanitized_column(table, column)} ILIKE ?)\"}\", *values.map do\n \"%#{value}%\"\n end)", "render_path": null, @@ -226,6 +249,6 @@ "note": "Current is not a model" } ], - "updated": "2024-08-20 14:34:27 +0200", + "updated": "2024-09-24 20:56:24 +0200", "brakeman_version": "6.1.2" } diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index cf889a92c..1759d8525 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -530,6 +530,19 @@ describe ProcedurePresentation do it { is_expected.to contain_exactly(kept_dossier.id) } end + + context 'with enum type_de_champ' do + let(:filter_value) { 'Favorable' } + let(:filter) { [{ 'table' => 'type_de_champ', 'column' => type_de_champ.stable_id.to_s, 'value_column' => :value, 'value' => filter_value }] } + let(:types_de_champ_public) { [{ type: :drop_down_list, options: ['Favorable', 'Defavorable'] }] } + + before do + kept_dossier.champs.find_by(stable_id: type_de_champ.stable_id).update(value: 'Favorable') + discarded_dossier.champs.find_by(stable_id: type_de_champ.stable_id).update(external_id: 'Defavorable') + end + + it { is_expected.to contain_exactly(kept_dossier.id) } + end end context 'for type_de_champ_private table' do