Auth adapter does not override record attributes but still prevents illegal creation/modification

[bartonmcguire]

This fixes an active bug in which the authorization adapter interacts with permissions gem CanCan in such a way that records are erroneously assigned attributes, which has been reported in countless issues dating back years such as this one: #1880

These issues all get pseudo-resolved with the suggestion to use the work-around to specify CanCan conditional permissions using a block instead of an attribute hash.

This is an unsatisfactory solution for three reasons:

1. A workaround is still a workaround, this is a bug, it should be fixed.
2. When someone employs the workaround, they fail to take advantage of the important case that RailsAdmin is trying to protect against (explained below).
3. When using the block workaround, RailsAdmin can't build list views or complete certain batch-actions with the records, as these rely on CanCan's accessible_by, which is incompatible with ability block conditions.

It's really time for this to get fixed.

Whats happening is this, if you have a permission rule like this one, stipulating that a user can only create edit a comment that they own:

can [:create, :update], Comment, :user_id => user.id

And let's say the user above does have access to the admin dashboard, but is not a full admin (perhaps, a user with an "intern" role).

Rails Admin sees this permissions rule, and so they don't show you or let you edit any comments by other users. BUT they do let you create new comments, and also let you edit comments that you currently own. Which theoretically lets you create or modify a comment that would be owned by another user. Sure, you'd then no longer have access to it. But you could theoretically create a comment for someone else saying something nasty!

So what rails admin was doing was deducing what attributes a comment you create should always have. It sees that rule, and so after it has applied all the attributes you manually assign to your new/modified record, it then creates a hash of attributes prescribed by the permission scheme, and applies them to the record before it is saved, potentially overriding attributes you would have set (but perhaps shouldn't have).

So in the case of the apocryphal comment, this works great:

goofball intern tries to create a comment someone else as the user, with body "im a jerk!"
rails admin applies the user_id and the body specified
rails admin then pulls @authorization_adapter.attributes_for(:create, RailsAdmin::AbstractModel.new(Comment) to get the attributes that should be present given the current users abilities, which in this case, are {:user_id => goofball_user.id}
It then overrides that attribute, so instead of assigning the comment to the innocent other use, goofball users comment is assigned to himself. take that goofball user - ya jerk!

So far so good. BUT, problems arise when we have a legit admin user, and we've assigned rules like this:

can [:create, :update], Comment, :user_id => user.id

if user.admin?
  can :manage, :all
end

This says that all users can update a comment if they own it, but also, if you're an admin, you can just do everything. And this is a common way of assigning rules in can can.

So now, say you're an admin who can do everything and you go to edit a comment made by another user. Say, edit that goofball comment from before because it was flagged. You're allowed to do that, you're an admin. but @authorization_adapter.attributes_for(:create, RailsAdmin::AbstractModel.new(Comment) still pulls that hash {:user_id => admin.id}. Even though that can :manage, :all specification should wipe that out, it doesn't. So when the admin edits the goofball comment, rails admin force applies the admins own ID as the user_id. Re-saving the comment as belonging to the admin user. We don't want this.

So how do we keep the best of both worlds: preventing users with limited abilities from creating/modifying records such that they break permissions, but without introducing this occasionally disastrous case where users with extended permissions accidentally rob other users records (or users themselves) or unwittingly change other attributes?

I'm removing the hard-assignment of @authorization_adapter.attributes_for when records are updated/created, no more overriding. But after the record is instantiated or updated in memory, but before it's persisted to the database, I'm re-checking to see if the current user still has permissions to take the current action for this record, and then refusing to save if they don't. For the admin this means they update someone else's comment, no problem. For the goofball, instead of them creating a comment for someone else, or instead of them creating a comment that gets reassigned to themselves, the comment just refuses to save if they try to attribute it to someone else.

The only place I'm keeping use of @authorization_adapter.attributes_for is in the new action (but not create). This means that when an admin or anyone goes to create a new comment, it will pre-populate the form with them as the comment's owner. That's all!

I see that there are specs in this gem, but couldn't find any documentation for contributors on how they are to be run. Happy to update them if some documentation could be provided. Thanks!

[mshibuya]

Thanks for the proposal, applied this change as c84d170.