[Fix #1961] Check that sorted_ids works for individual
This commit is contained in:
parent
abd066c6f4
commit
3dca3c7dee
3 changed files with 52 additions and 24 deletions
|
@ -95,7 +95,7 @@ class ProcedurePresentation < ApplicationRecord
|
|||
.where("champs.type_de_champ_id = #{sort['column'].to_i}")
|
||||
.order("champs.value #{order}")
|
||||
.pluck(:id)
|
||||
else
|
||||
when 'user', 'individual', 'etablissement'
|
||||
return dossiers
|
||||
.includes(table)
|
||||
.order("#{column} #{order}")
|
||||
|
|
|
@ -1,25 +1,5 @@
|
|||
{
|
||||
"ignored_warnings": [
|
||||
{
|
||||
"warning_type": "SQL Injection",
|
||||
"warning_code": 0,
|
||||
"fingerprint": "030551e51e29561702bcb9760bdeeed15e1936d4a9537f27e5d1d13a0ebb34ef",
|
||||
"check_name": "SQL",
|
||||
"message": "Possible SQL injection",
|
||||
"file": "app/models/procedure_presentation.rb",
|
||||
"line": 130,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
|
||||
"code": "dossiers.includes(sort[\"table\"]).order(\"#{sanitized_column(sort)} #{sort[\"order\"]}\")",
|
||||
"render_path": null,
|
||||
"location": {
|
||||
"type": "method",
|
||||
"class": "ProcedurePresentation",
|
||||
"method": "sorted_ids"
|
||||
},
|
||||
"user_input": "sanitized_column(sort)",
|
||||
"confidence": "Weak",
|
||||
"note": "Not an injection because of `sanitized_column`"
|
||||
},
|
||||
{
|
||||
"warning_type": "SQL Injection",
|
||||
"warning_code": 0,
|
||||
|
@ -27,7 +7,7 @@
|
|||
"check_name": "SQL",
|
||||
"message": "Possible SQL injection",
|
||||
"file": "app/models/procedure_presentation.rb",
|
||||
"line": 119,
|
||||
"line": 90,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
|
||||
"code": "dossiers.order(\"#{sanitized_column(sort)} #{sort[\"order\"]}\")",
|
||||
"render_path": null,
|
||||
|
@ -47,7 +27,7 @@
|
|||
"check_name": "SQL",
|
||||
"message": "Possible SQL injection",
|
||||
"file": "app/models/procedure_presentation.rb",
|
||||
"line": 125,
|
||||
"line": 96,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
|
||||
"code": "dossiers.includes(((\"type_de_champ\" == \"type_de_champ\") ? (:champs) : (:champs_private))).where(\"champs.type_de_champ_id = #{sort[\"column\"].to_i}\").order(\"champs.value #{sort[\"order\"]}\")",
|
||||
"render_path": null,
|
||||
|
@ -59,8 +39,28 @@
|
|||
"user_input": "sort[\"order\"]",
|
||||
"confidence": "Weak",
|
||||
"note": "Not an injection because `sort[\"order\"]` has passed `check_allowed_sort_order`"
|
||||
},
|
||||
{
|
||||
"warning_type": "SQL Injection",
|
||||
"warning_code": 0,
|
||||
"fingerprint": "e0e5b55126891df8fe144835ea99367ffd7a92ae6d7227a923fe79f4a79f67f4",
|
||||
"check_name": "SQL",
|
||||
"message": "Possible SQL injection",
|
||||
"file": "app/models/procedure_presentation.rb",
|
||||
"line": 101,
|
||||
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
|
||||
"code": "dossiers.includes(\"user\").order(\"#{sanitized_column(sort)} #{sort[\"order\"]}\")",
|
||||
"render_path": null,
|
||||
"location": {
|
||||
"type": "method",
|
||||
"class": "ProcedurePresentation",
|
||||
"method": "sorted_ids"
|
||||
},
|
||||
"user_input": "sanitized_column(sort)",
|
||||
"confidence": "Weak",
|
||||
"note": "Not an injection because of `sanitized_column`"
|
||||
}
|
||||
],
|
||||
"updated": "2018-10-05 16:12:32 +0200",
|
||||
"updated": "2018-10-11 12:09:03 +0200",
|
||||
"brakeman_version": "4.3.1"
|
||||
}
|
||||
|
|
|
@ -286,6 +286,34 @@ describe ProcedurePresentation do
|
|||
it { is_expected.to eq([biere_dossier, vin_dossier].map(&:id)) }
|
||||
end
|
||||
|
||||
context 'for individual table' do
|
||||
let(:table) { 'individual' }
|
||||
let(:order) { 'asc' } # Desc works the same, no extra test required
|
||||
|
||||
let(:procedure) { create(:procedure, :for_individual) }
|
||||
|
||||
let!(:first_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Alain', nom: 'Antonelli')) }
|
||||
let!(:last_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) }
|
||||
|
||||
context 'for gender column' do
|
||||
let(:column) { 'gender' }
|
||||
|
||||
it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
|
||||
end
|
||||
|
||||
context 'for prenom column' do
|
||||
let(:column) { 'prenom' }
|
||||
|
||||
it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
|
||||
end
|
||||
|
||||
context 'for nom column' do
|
||||
let(:column) { 'nom' }
|
||||
|
||||
it { is_expected.to eq([first_dossier, last_dossier].map(&:id)) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'for other tables' do
|
||||
# All other columns and tables work the same so it’s ok to test only one
|
||||
let(:table) { 'etablissement' }
|
||||
|
|
Loading…
Reference in a new issue