Yes, if there is anything else in the public key table that you don't want to be updated via form fields, then you should be protecting yourself against it. However, every developer - or at minimum, every experienced developer - should know the basic maxim "never trust the data from your users".
But this method does protect resources from being arbitrarily assigned to other users.
In any controller where the user should be authenticated, I would suggest the following two guidelines (let's assume the model is PublicKey, as before):
1. In the index action, where you get a list of resources, use:
@keys = current_user.public_keys
2. In all of the actions (such as show, edit, update, etc.) where the method starts with:
@key = PublicKey.find(params[:id])
Remove that method and instead create a private method in your controller:
def get_key
@key = current_user.public_keys.find(params[:id]) if params[:id]
end
And at the top of your controller:
before_filter :get_key
This should just be a habit. With these two modifications, you've just scoped the resource down to the user everywhere that it is used. Additionally, you've DRYed out your single resource-specific actions (show, edit, etc.) because the code to find the resource only exists in one place.
Of course, you still need to think about attr_accessible. But even this is not a panacea. Consider the case where a user has a role_id field that specifies whether they are an admin, manager, or regular user. In this three role scenario, managers are allowed to create managers or regular users, but not admins. Admins can create users of all three roles.
This means that you may want to allow the role_id to be updated by mass assignment. You just have to ensure that users cannot update the role_id if the role they have picked is more privileged than their current role. You could just add a validator to the user model that does exactly that.
Alternatively, you could keep role_id as a blacklisted attribute, but in your controller you could check for the new role in the params, and then only assign it if the user should be able to assign it. Both approaches have merit. The bottom-line is that you still have to THINK.
and then define the scoping rules in the declarative auth file:
authorization do
role :user do
has_permission_on :public_keys do
to [:write, :read]
# user refers to the current_user when evaluating
if_attribute :user_id => is {user.id}
end
end
end
This is a bit more DRY, because you are abstract out the conditions of access. This is especially useful in situations where you have readonly access or other types of acl.
But this method does protect resources from being arbitrarily assigned to other users.
In any controller where the user should be authenticated, I would suggest the following two guidelines (let's assume the model is PublicKey, as before):
1. In the index action, where you get a list of resources, use:
2. In all of the actions (such as show, edit, update, etc.) where the method starts with: Remove that method and instead create a private method in your controller: And at the top of your controller: This should just be a habit. With these two modifications, you've just scoped the resource down to the user everywhere that it is used. Additionally, you've DRYed out your single resource-specific actions (show, edit, etc.) because the code to find the resource only exists in one place.Of course, you still need to think about attr_accessible. But even this is not a panacea. Consider the case where a user has a role_id field that specifies whether they are an admin, manager, or regular user. In this three role scenario, managers are allowed to create managers or regular users, but not admins. Admins can create users of all three roles.
This means that you may want to allow the role_id to be updated by mass assignment. You just have to ensure that users cannot update the role_id if the role they have picked is more privileged than their current role. You could just add a validator to the user model that does exactly that.
Alternatively, you could keep role_id as a blacklisted attribute, but in your controller you could check for the new role in the params, and then only assign it if the user should be able to assign it. Both approaches have merit. The bottom-line is that you still have to THINK.