Charles Miller examines the getter/setter syndrome in Java1 from a usage (rather than the normal construction) perspective and devices a shortcut. Instead of using individual access methods, he proposes the unified set_properties that takes a hash of attributes and automatically assigns them on the object instance. Unfortunately, he fails to see its uses and discards the idea with a clever Simpsons reference (we like those).
That's a shame because he had the right idea.
In constructing Rails, I reached exactly the same shortcut—albeit with slightly more natural Ruby-like semantics by using a signature of Object#attributes=(attributes). More importantly, I found an extremely handy use for it. Passing data from the view to the model in a MVC-style application usually means doing it twice. In the view with input tags as part of a form and in the controller by picking attributes from the request and calling access methods on the model. That's inefficient at best and error prone at worst.
Instead Rails adopts a combination of PHP-like input names that turns form data into hashes and the unified attribute assignment to do as follows:
<!-- The view -->
<form>
Name: <input type="text" name="user[name]">
Email: <input type="text" name="user[email]">
Website: <input type="text" name="user[website]">
</form>
# The controller action
def create_user
user = User.new
user.attributes = @params["user"]
user.save
end
As the form is submitted by the user, the Rails controller turns user[name], user[email], and user[website] into a hash that contains the name, email, and website as keys and the user supplied input as values. This hash can be accessed through @params["user"] from the action method.
Now the clever part is that the name, email, and website keys from the form maps to attributes with accessors on the model. This relieves the controller of a per-attribute knowledge responsibility, which allows us to add new fields on the view and model without changing the controller.
Actually, we only lavishly spent three lines in the controller to make it slightly easier to follow each step separately. Normally, it would likely look as follows (if there's to be no validation of the attributes):
def create_user
User.new(@params["user"]).save
end
Relief from per-attribute knowledge responsibility
In fact, the Rails framework goes even further in its attempt to relieve as many layers of per-attribute knowledge responsibility as possible. In your average web-application layer stack, it's not uncommon to have per-attribute knowledge spread across five concerns:
The form: One input field per attribute.
The action: The request is picked for attributes and they're assigned to the model.
The model: Each attribute is specified on the model with a type and boiler-plate serving of accessor methods.
The ORM: Column names are matched against model attributes and a type-conversion specified.
The database table: All persistable attributes in the model are given a column with a name and a type.
Everytime the model needs to evolve to either add, remove, or change attributes, you have to change each concern in a similar but distinct manner to move forward. That's an extremely cumbersome, time-consuming, and—again—error-prone dance of repetition.
Rails allows you to cut all that clutter in between the form and the database table and only work at the ends when the model needs to evolve. The action, model, and mapping steps are all handled by building on the knowledge already present in the form and in the database table.
So let's imagine that our lovely user from the example above needs to also be identified by a instant message handle. In Rails, this would require the following changes:
Addition to view: IM: <input type="text" name="user[im_handle]" />
Addition to database table: im_handle VARCHAR(100)
That's it. Now you're free to enjoy the rest of the Sunny day on the lawn with a drink and a hat while your lesser fortunate friends is caught in the toils of per-attribute knowledge spread across the entire layer stack. So much for low coupling, eh?
1. That onerous expenditure of 8-12 lines boiler-plate access code per attribute in stark contrast to Ruby's use of just 1 line for all attributes. An inefficiency that it shares with C#.
Challenge by Florian on April 04, 22:51
As handy as it might seemt to directly transfer input data to business objects, when you do it like in your example above you allow users to edit ALL properties of the user object. just something to consider..
I've found that to be what I want in 9 out of 10 cases. When it's not what I want, I'll either strip it out in the controller or as a validation on the model. This goes to the core of Rails. Assume the common case, make the odd case possible.
But I hear you. Using Rails, or any framework, isn't an invitation to leave the head on the counter. You still have to think.
What if a user property one would like to view/set isn't in the user table, but is, perhaps, in another table (say, for instance, in the company table (because only the id, not the name, of the company is stored in the user table)), or is a read-only figure such as user.articles_posted, or something completely different which isn't a single read/write attribute stored in the user table?
How would one handle these kind of fields with Rails?
Yes, I'd have the same worry about parameter-injection attacks. There are some properties on your business-layer objects that you really don't want the user to change, and it's always better from a security perspective to explicitly state what is permitted, rather than rely on remembering what is not allowed.
It's the same sort of problem that PHP runs into (or at least was prone to when I used to code PHP) because HTTP parameters are assigned directly to local variables in the script: you have to be very careful to initialise local variables explicitly, or an attacker can hijack them.
Conceptually, I'd also prefer type-coercion to happen at a higher level so my business objects only deal with their natural types, but that's a matter preference.
I'm not sure I understood.
does attribute=(hsh) work like this:
def attribute=(hsh)
hsh.each do |key,val|
self.send(key+"=",val)
end
end
?
Cause I believe that if it works like this there is little chance to hijack things.
If User has more attributes it sounds reasonable to strip them in initialize() and even if you don't the worst thing you can do is call an accessor methods that must exist, and should check it's argument anyway, ain't it ?
OTOH if there is direct raw access a-la eval I agree that it seem little hackish & insecure
Rails, in this case the model part of Rails called Active Record, has a wide array choices for representing relationships. There's "has_one", "has_many", "belongs_to", and just lately, we've added a many-to-many link tentatively called "has_and_belongs_to_many" (better names that still match the style of the previous three are much appriciated).
So let's say the user belongs to a company. We'd represent that in an Active Record with the following:
class User < ActiveRecord::Base
belongs_to :company
end
This gives you a number of new methods on the user class, such as User#company (which returns the company object that this user belongs to) and User#company?(some_company) (which is a comparison method for whether the argument is the same company as that associated with the user).
That way, you can traverse the object graph in a natural way, such as user.company.name, which would get the name of the company that the user belongs to.
But no, you can't automatically assign the name of the users company from the view to the model. This would require some direct involvement from the controller.
The point isn't to put the controller out of work. It's to make the laborous default case as easy as possible and other cases doable.
gabriele: it's a matter of access-control. Generally, a User object will only have a certain subset of its fields that are user-changeable, others are maintained by the system, or its administrators.
Say you add a new admin-only attribute. You need to make no changes whatsoever to the admin-accessible actions, but you _do_ need some mechanism to block this attribute from the user. If you miss anywhere, you end up with a security hole.
Security is one of those things where you'd rather not rely on someone remembering stuff like that. It works 99% of the time in a small application, but once the app grows beyond your ability to hold the whole thing in your head at once, you'll inevitably miss something, and end up that day's featured attraction on Bugtraq.
Alternatively, you can rearrange your object model to separate fields by user role, but that's a bit ugly.
David: Sounds reasonable. So, if one wanted the value of user.company.name in a form, how would that look?
Sorry if I'm pre-empting your revealing of Rails, I can't help myself. Rails seems to swing it in favor of Ruby, instead of Python, for me (regarding web development at least).
No, all instance variables aren't exposed to this access structure. Only those fetched from the database. So if you like Charles wants to keep properties on your business objects that's not accessible like this, you can either keep them outside of the table (just as regular instance variables initialized by force or configuration files) or as an associated object, such as PersonStatistics with a articles_posted field.
Miller: I guess it depends on the way you structure your application. I've build the domain model for Basecamp to keep user-editable and admin-stuff separate to allow for this type of development without security concerns. But I'm glad you brought it up. This should certainly be part of documentation and guiding in how to build Rails applications.
And no, I don't subscribe to the model of forcing tedious work on the developer in order to make sure he thinks about security. Fowler has a good essay on directing vs enabling that goes to the heart of this issue. You seem to want some directing, Rails wants to be enabling.
Thomas: Your template would look something like this
User name: <%= @user.name %>
Company name: <%= @user.company.name %>
That's another cool thing with Rails. You have full access to the object graphs through the template, so you don't have to specifically fetch and assign the user's company to a variable that's accessible to the template. You just find the user, through something like User.find(user_id) (or one of the more security sensitive finders that the Active Record base class provides if user_id is tainted), and make it available to the template.
Miller: PHP hasn't worked like that for quite a long time. I can't remember when they turned of automatic assignment, but I think it was around 4.1 or something. I agree that this was probably too far into the land of convenience over security. I remember Rasmus Lerdof disagreed, though.
David: As far as I know, the @ in Ruby signifies that the object is an instance variable, what implied class is the "user" an instance variable of? Why did you choose this approach, as opposed to the user object being unattached to another object?
The template runs off an instance of the template class. This instance is handed a set of variables from the controller which it provides access to via instance readers. The Rails frameworks make it seems like you're just setting instance variables on the controller in order to make them accessible to the template, but that's just for convenience (which goes to the philosophy of doing only enough work to make the framework able to infer the complete meaning).
A sample action method on the controller might look like this:
class UserController < ActionController::Base
def display
@user = User.find(@params["id"])
end
end
Since we haven't specified either a render or redirect, which is what all controllers do, the default case is assumed: render "user/display". In the user/display template, we can now access the user object as in the example I provided above.
David: Thanks for the explanation, really appreciate it. Keep working on Rails, it seems to be a wonderful framework, right up my alley. I'm looking forward to learning more about it.
PHP was like that in 1999, which was the last time I programmed in it. :)
As to the other point, I consider security a special case. Enabling security is fundamentally more dangerous than directing it, because when it comes to security, there is a vital and painful difference between a system that fails open, and one that fails closed.
No need for apologies, Charles. I'm glad you brought these issues up. As a direct consequence, I think I'm going to add a "mass_assign_protection" macro or something like it that'll make it really, really easy to guard parts of the business object from being accessible for through mass assignment. So you'd do something like:
class User < ActiveRecord::Base
mass_assign_protection :rating, :balance
end
That would prevent "rating" and "balance" from being assigned with the attributes= writer.
Also, your concerns make it easier to know exactly how to address the security issues when explaining Rails. Thanks!
David: I would much rather see the reverse of mass_assign_protection
ex:
class User < ActiveRecord::Base
attributes :name, :passwd, :fullname
end
or what have you.
Mind you, I understand that how you have things now is to cut out the middle layer... and allow changes to the database to only effect changes to the layout... and that aspect is a great one. But I think allowing arbitrary changes by default is a bad idea.
Think of your ActiveRecords as a firewall.
Take OpenBSD packet filters for example. in pf... you generally set the tightest possible filter, and then open things up as needed.
ex:
block in all
pass in from any to 192.168.1.5/32
Maybe this could be applied to ActiveRecord... By default you 'block in all'... but you can widen things as needed. ex:
secure business object: (Mind you, this code as is might not be valid)
class BusinessObject < ActiveRecord::Base
attributes_block_all
attributes_allow :name, :street
attributes_allow_prefix :option_
end
This would by default, block assignment to all variables.
It would then allow assignment to name and street...
Then finally allow assignment to any variable with the option_ prefix... which would allow you to add arbitrary options to the table without having to change anything else but the view.
If you wanted your object to allow assignment in a totally 'transparent' behavior, this could also be achived.
unsecure object:
UserPrefs < ActiveRecord::Base
attributes_allow_all
end
I think it's a good idea, but also that it's placing too much of a burden on the Active Record. Acting as the firewall should be the role of the controller. So "mass_assign_protection" would just be really simple shortcut. Yearh, it's bleeding responsibility a tad, but enough to be really useful and not enough to offset the greater picture.
And then if you really want OpenBSD-level security then you probably shouldn't use the mass-assignment method. Or, if you do, implement a before_filter on the controller that makes sure bad variables doesn't make it to the mass-assignment (you can just take them out of the request).
Remember, though, that you're not putting every single model object up for general editing like this. If you want to use this convenience either:
Make sure that the model objects you present to the user consists of all safe-to-change attributes (this has been the case for Basecamp more often than not)
Use the upcoming shortcut to protect a few variables
Keep the sensitive attributes in a separate model-object associated with a link that's not available for mass-assignment
Implement an OpenBSD-like firewall on the controller as a before_filter
I think that's a reasonable amount of options for ensuring security without detracting from ease of use by default. But I'm more than open to additional suggestions. Through this discussion I think there should be a good understanding of my position on ease of use vs security.
Thanks again to all that have contributed to this discussion. Keep it coming!
I understand your position, David, but I must say I agree with Luke's as well. In my opinion, the optimal solution would be if one could select which road to take, by either explicitly stating which attributes are exempt from mass-assignment (or rather easy-assignment as I like to think of it), or the exact opposite; explicitly stating which attributes _are_ available.
Instead of putting the filter logic in the ActiveRecord, how about a filter proxy which implements this model? You could then move ALL of the access control methods, and even the attributes = method to this new class.
So you would have something like: (again, more code that probably syntactically wrong... but should get the idea accross)
user = User.new
# Would return 'undefined method'
user.attributes = @params["user"]
proxuser = RecordProxy.attach(user) do
attributes_block_all
attributes_allow :name, :passwd
attributes_allow_prefix :option_
end
# Only the attributes allowed by the filter are set
proxuser.attributes = @params["user"]
proxuser.save
And of course, you could do a one time proxy...
user = RecordProxy.assign(User.new, @params["user"]) do
attributes_block_all
attributes_allow :name, :passwd
attributes_allow_prefix :option_
end
or if you needed the filter more than once but for different objects... you could do something like:
userproxy = RecordProxy.new
userproxy.filter do
attributes_block_all
attributes_allow :name, :passwd
attributes_allow_prefix :option_
end
I think this debate is coming partly from the fact that I didn't make it clear how easy it is to do per-attribute assignment on the controller level. If you're uneasy with mass-assignment and like to keep sensitive data mixed with non-sensitive data (that's often a good idea), you'd just do like this:
def create_user
user = User.new
user.name = @params["name"]
user.email = @params["email"]
user.website = @params["website"]
user.save
end
Now you can add membership_level as an attribute on the user object without worrying that it's accessible for parameter-injection.
But of course this goes much against the entire idea of the post, which was to use mass-assignment for something useful and to decrease the per-attribute knowledge in the layers.
It is, however, much better to keep this information on the controller than on the model from my conceptual viewpoint. And it's also a much lighter system than building firewalls or wrapping proxies around the models.
So that would be my recommendation. Use mass-assignment when you work with all-accessible objects (you'll discover that many objects are indeed safe to make accessible in their entirety). Or use the upcoming shortcut if you only have a handful of sensitive fields and still want the mass-assignment advantages. Or use per-attribute assignment on the controller if mass-assignment makes you feel uneasy or if you have more sensitive fields than not.
The id is the one attribute that you can't mass assign :)
Challenge by Xen on May 11, 14:19
Someone said something about only having user changable attributes in the model, which made me think something along these lines:
class User
mass_assignable :name, :passwd, :fullname
end
class UserAdmin
all_mass_assignable
end
Then one uses the User class in the frontend, and UserAdmin in the admin interface. I'd prefer one User class, but then it gets messy when you have different 'views'. Leveraging (gah, marketingspeak) the object hierarchy makes code reuse simple, while still allowing for seperation.
'Acting as the firewall should be the role of the controller.' That pretty much kills off the usefulness of attributes= and neatness of not having to change the controller, doesn't it? Then the controller has to know about all the models and whether they have protected attributes, or not, and what those limitations is. I'd say that it's the model that knows whats assignable, and what limitations there is on attributes, not the controller, and putting the validation into the controller only gets us back to where we started. Going out on a limb here, but assuming that attributes= uses normal accessor fuctions, we'd do something like:
class User
def email=(email)
raise InvalidInput, "Bad email address" if email !~ /^[a-z0-9_.-]@[a-z0-9.-]$/
super(email)
end
end
and then in the controller:
def update
begin
@user = User.find(id)
@user.attributes = @params["user"]
@user.save
rescue InvalidInput => e
@error = e.message
render "user/edit"
end
redirect "user/display/" + @user.id.to_s
end
or, with a convenience method:
def update
update_or_reedit("user/edit") do
@user = User.find(id)
@user.attributes=(@params["user"]).save
end
redirect "user/display/" + @user.id
end
That way, when I decide that VARCHAR(255) was too much for User.name, I cut it down to 80 in the DB, modify the model and the view, and I don't have to hop around and fix the input validation on user/create, user/update, admin/newuser admin/edituser, so on and so forth (though I'm assuming that in such a simple case, that ActiveRecord is doing much the same from the data it gleaned from the DB in the first place). Extend the InvalidInput exception to carry information about which field failed, and why, a bit of magic to attributes= to collect exceptions from all the attribues, and an easy way for the view to use this information, and things could get very nice indeed.
Active Record implements a sophisticated validations and callback system that does much as you describe in a very clean way. Have a look at the latest slides and look forward to the release in just a few days.
Challenge by Xen on May 12, 1:23
Oho, I just spotted a 'if person.save' thing in the slides... Which I must have missed in Roskilde.. Very, very nice indeed...
And if I understand it correctly, it's doing exactly what I was thinking, keeping the 'firewall' in the model, and the controller only have to deal with whether there was an error or not.
I need to wrap my head around Ruby better. I keep going 'Damn.. Now *that's* elegance...'
I'm looking forward.. Man, am I looking forward...
