VRFS-3451 musician_search verifying instrument and genres inputs

ruby/lib/jam_ruby/models/base_search.rb

@@ -102,26 +102,38 @@ module JamRuby
     def self.search_target_class
     end
 
-    # FIXME:  SQL INJECTION
     def _genres(rel, query_data=json)
       gids = query_data[KEY_GENRES]
       unless gids.blank?
-        gidsql = gids.join("','")
+        allgids = Genre.order(:id).pluck(:id)
-        gpsql = "SELECT player_id FROM genre_players WHERE (player_type = '#{self.class.search_target_class.name}' AND genre_id IN ('#{gidsql}'))"
+        gids = gids.select { |gg| allgids.index(gg).present? }
-        rel = rel.where("#{self.class.search_target_class.table_name}.id IN (#{gpsql})")
+        
+        unless gids.blank?
+          gidsql = gids.join("','")
+          gpsql = "SELECT player_id FROM genre_players WHERE (player_type = '#{self.class.search_target_class.name}' AND genre_id IN ('#{gidsql}'))"
+          rel = rel.where("#{self.class.search_target_class.table_name}.id IN (#{gpsql})")
+        end
       end
       rel
     end
 
-    # FIXME:  SQL INJECTION
     def _instruments(rel, query_data=json)
       unless (instruments = query_data[KEY_INSTRUMENTS]).blank?
-        instsql = "SELECT player_id FROM musicians_instruments WHERE (("
+        instrids = Instrument.order(:id).pluck(:id)
-        instsql += instruments.collect do |inst|
+        instruments = instruments.select { |ii| instrids.index(ii['instrument_id']).present? }
-          "instrument_id = '#{inst['instrument_id']}' AND proficiency_level = #{inst['proficiency_level']}"
+
-        end.join(") OR (")
+        unless instruments.blank?
-        instsql += "))"
+          instsql = "SELECT player_id FROM musicians_instruments WHERE (("
-        rel = rel.where("#{self.class.search_target_class.table_name}.id IN (#{instsql})")
+          instsql += instruments.collect do |inst|
+            unless MusicianInstrument::PROFICIENCY_RANGE === (proflvl=inst['proficiency_level'].to_i) 
+              proflvl = MusicianInstrument::LEVEL_INTERMEDIATE 
+            end
+            "instrument_id = '#{inst['instrument_id']}' AND proficiency_level = #{proflvl}"
+          end.join(") OR (")
+          instsql += "))"
+
+          rel = rel.where("#{self.class.search_target_class.table_name}.id IN (#{instsql})")
+        end
       end
       rel
     end

ruby/lib/jam_ruby/models/musician_instrument.rb

@@ -13,8 +13,13 @@ module JamRuby
     belongs_to :player, polymorphic: true
     belongs_to :instrument, :class_name => "JamRuby::Instrument"
 
+    LEVEL_BEGIN = 1
+    LEVEL_INTERMEDIATE = 2
+    LEVEL_EXPERT = 3
+    PROFICIENCY_RANGE = (LEVEL_BEGIN..LEVEL_EXPERT)
+
     def description
       @description = self.instrument.description
     end
   end
-end
+end

ruby/spec/jam_ruby/models/musician_search_spec.rb

@@ -173,7 +173,8 @@ describe 'Musician Search Model' do
 
       it "gets expected number of users" do
         instjson = [{ instrument_id: Instrument.first.id, proficiency_level: 2 },
-                    { instrument_id: Instrument.first(2)[1].id, proficiency_level: 2 }
+                    { instrument_id: Instrument.first(2)[1].id, proficiency_level: 2 },
+                    { instrument_id: 'foo', proficiency_level: 2 },
                    ]
         search.update_json_value(MusicianSearch::KEY_INSTRUMENTS, instjson)
         expect(search.do_search.count).to eq(3)