Source diving in Clearance: a question

Ruby on Rails
1

I was going through the Clearance source code and I found this expression to be curious:

 def user_from_params
    user_params = params[:user] || Hash.new
    email = user_params.delete(:email)
    password = user_params.delete(:password)

    #...
  end

Is there a reason for assigning the params to local variables by calling delete? I discussed this with @halogenandtoast in the Campfire room once, but I am still curious about it.

Part of what makes strange for me is using a destructive operation for assignment, I guess it makes reading a little awkward. Is there an advantage to using delete?

1 Like
2

The goal here is that we want to bank on the destructive nature of delete here. The end goal here was to avoid interacting with both Whitelisted attributes and strong parameters. By pulling these values out of the hash, they avoid being passed in during the call to:

Clearance.configuration.user_model.new(user_params).tap do |user|

Typically at that point, user_params will just be an empty hash. Afterwards, we can directly assign these attributes (avoiding mass assignment security requirements):

user.email = email
user.password = password

From Clearanceā€™s perspective this is preferable since we donā€™t need to know which type of mass assignment security the user is using.

1 Like
3

Thanks for the quick reply, @halogenandtoast.

So, delete is used in order to strip out the contents of the passed in params before it gets used to interact with the model, hence why you donā€™t just do a regular assignment (like email = params[:email], for example)?

I think I got it know. Would it make sense to name the method in a more revealing manner, like secure_user_params or sanitize_user_params? (not great names, but I think you understand what I mean)

4

@pedromoreira I think the method name makes sense because it returns a user and in the context weā€™re dealing with this (the clearance controller), nothing else is touching these parameters. That being said, my personal preference is not to use destructive methods. This method could be rewritten to:

def user_from_params
  user_params = params[:user] || Hash.new
  email = user_params[:email]
  password = user_params[:password]
  sanitized_user_params = user_params.except(:email, :password)

  Clearance.configuration.user_model.new(sanitized_user_params).tap do |user|
     user.email = email
     user.password = password
   end
 end

If I was being my usual nitpicky self, Iā€™d rewrite it even further into several methods

def user_from_params
  user_model.new(sanitized_user_params).tap do |user|
    user.email = user_params[:email]
    user.password = user_params[:password]
  end
end

def user_params
  params[:user] || Hash.new
end

def sanitized_user_params
  user_params.except(:email, :password)
end

def user_model
  Clearance.configuration.user_model
end
3 Likes
5

Also for reference purposes, here is where except comes from

http://api.rubyonrails.org/classes/Hash.html#method-i-except

6

Thanks for the further examples: Iā€™m all for nitpicking as well :slight_smile: