Permalink
Browse files

Refactor activity_user to remove possible SQL injection points.

  • Loading branch information...
steveyken committed Dec 27, 2013
1 parent 078035f commit d4b2de81a4d8c1b201482edcb2488ed9280a65fd
Showing with 10 additions and 6 deletions.
  1. +6 −4 app/controllers/home_controller.rb
  2. +4 −2 spec/controllers/home_controller_spec.rb
@@ -122,6 +122,9 @@ def activity_event
end
#----------------------------------------------------------------------------
+ # TODO: this is ugly, ugly code. It's being security patched now but urgently
+ # needs refactoring to use user id instead. Permuations based on name or email
+ # yield incorrect results.
def activity_user
user = current_user.pref[:activity_user]
if user && user != "all_users"
@@ -130,12 +133,11 @@ def activity_user
else # first_name middle_name last_name any_name
name_query = if user.include?(" ")
user.name_permutations.map{ |first, last|
- "(upper(first_name) LIKE upper('%#{first}%') AND upper(last_name) LIKE upper('%#{last}%'))"
- }.join(" OR ")
+ User.where(:first_name => first, :last_name => last)
+ }.map(&:to_a).flatten.first
else
- "upper(first_name) LIKE upper('%#{user}%') OR upper(last_name) LIKE upper('%#{user}%')"
+ [User.where(:first_name => user), User.where(:last_name => user)].map(&:to_a).flatten.first
end
- User.where(name_query).first
end
end
user.is_a?(User) ? user.id : nil
@@ -171,14 +171,16 @@
it "should find a user by first name or last name" do
@cur_user.stub(:pref).and_return(:activity_user => 'Billy')
controller.instance_variable_set(:@current_user, @cur_user)
- User.should_receive(:where).with("upper(first_name) LIKE upper('%Billy%') OR upper(last_name) LIKE upper('%Billy%')").and_return([@user])
+ User.should_receive(:where).with(:first_name => 'Billy').and_return([@user])
+ User.should_receive(:where).with(:last_name => 'Billy').and_return([@user])
controller.send(:activity_user).should == 1
end
it "should find a user by first name and last name" do
@cur_user.stub(:pref).and_return(:activity_user => 'Billy Elliot')
controller.instance_variable_set(:@current_user, @cur_user)
- User.should_receive(:where).with("(upper(first_name) LIKE upper('%Billy%') AND upper(last_name) LIKE upper('%Elliot%')) OR (upper(first_name) LIKE upper('%Elliot%') AND upper(last_name) LIKE upper('%Billy%'))").and_return([@user])
+ User.should_receive(:where).with(:first_name => 'Billy', :last_name => "Elliot").and_return([@user])
+ User.should_receive(:where).with(:first_name => 'Elliot', :last_name => "Billy").and_return([@user])
controller.send(:activity_user).should == 1
end

0 comments on commit d4b2de8

Please sign in to comment.