views:

1123

answers:

5

I need to implement fine-grained access control in a Ruby on Rails app. The permissions for individual users are saved in a database table and I thought that it would be best to let the respective resource (i.e. the instance of a model) decide whether a certain user is allowed to read from or write to it. Making this decision in the controller each time certainly wouldn’t be very DRY.
The problem is that in order to do this, the model needs access to the current user, to call something like may_read?(current_user, attribute_name). Models in general do not have access to session data, though.

There are quite some suggestions to save a reference to the current user in the current thread, e.g. in this blog post. This would certainly solve the problem.

Neighboring Google results advised me to save a reference to the current user in the User class though, which I guess was thought up by someone whose application does not have to accommodate a lot of users at once. ;)

Long story short, I get the feeling that my wish to access the current user (i.e. session data) from within a model comes from me doing it wrong.

Can you tell me how I’m wrong?

Thanks!

+2  A: 

Well my guess here is that current_user is finally a User instance, so, why don't u add these permissions to the User model or to the data model u want to have the permissions to be applied or queried?

My guess is that u need to restructure your model somehow and pass the current user as a param, like doing:

class Node < ActiveRecord
  belongs_to :user

  def authorized?(user)
    user.admin? or self.user_id == user.id
  end
end

# inside controllers or helpers
Node.authorized? current_user
khelll
+1  A: 

I'm using the Declarative Authorization plugin, and it does something similar to what you are mentioning with current_user It uses a before_filter to pull current_user out and store it where the model layer can get to it. Looks like this:

# set_current_user sets the global current user for this request.  This
# is used by model security that does not have access to the
# controller#current_user method.  It is called as a before_filter.
def set_current_user
  Authorization.current_user = current_user
end

I'm not using the model features of Declarative Authorization though. I'm all for the "Skinny Controller - Fat Model" approach, but my feeling is that authorization (as well as authentication) is something that belongs in the controller layer.

Daniel Kristensen
+1  A: 

I have this in an application of mine. It simply looks for the current controllers session[:user] and sets it to a User.current_user class variable. This code works in production and is pretty simple. I wish I could say I came up with it, but I believe I borrowed it from an internet genius elsewhere.

class ApplicationController < ActionController::Base
   before_filter do |c|
     User.current_user = User.find(c.session[:user]) unless c.session[:user].nil?  
   end
end

class User < ActiveRecord::Base
  cattr_accessor :current_user
end
jimfish
Thanks!If this really works, that would mean that a separate instance of the User class (by which I really mean a class, not an object) is being generated for each request/session. Is that so?You say that you use it in production, so I guess it has to be that way, but it sure appears strange to me.
knuton
The above code is actually used in conjunction with automatically setting a created_by and updated_by values of a record on save, based of course on session data. We aren't actually creating multiple instances, we are updating a class variable.
jimfish
Why I am confused: You are setting a class variable, User.current_user. I though that that would be a bad idea, as I assumed that the same instance of the User class (as in “Classes in Ruby are first-class objects—each is an instance of class Class.” http://www.ruby-doc.org/core/classes/Class.html) would be used throughout a Rails application. If that were the case, this method would obviously lead to unwanted results if several users were active simultaneously. As you are using this in production I assume that it works, but that means that there are several instances of the User class.
knuton
It would normally be a bad idea if you were dealing with an application with state. However, this variable is being set on every request in a web stack. Hence, Request -> Set Class Var -> Do Something -> Return Result. The two requests don't bleed into each other, everything starts fresh at the beginning of each request. Out with old, in with the new.
jimfish
I just did a google search and found where I think I may have picked this up. In this post he actually goes into threading, and at the end realizes that it wasn't necessary.http://www.pluitsolutions.com/2006/08/15/rails-auto-assign-created-by-and-updated-by/#comments
jimfish
Actually this has a serious flaw in it in production mode that you would never find in development mode. In development the class is reloaded on every request, so the variable would always start out empty. However in production the class stays. jimfish's logic is almost correct, but because of the 'unless c.session[:user].nil?' clause, a non-logged-in user could potentially piggyback on the previous users permissions. If you really want to use this, you should remove that clause.
dasil003
Good to know, thanks to both of you! Not sure how I am going to implement it yet (in the model or not).
knuton
+3  A: 

I'd say your instincts to keep current_user out of the model are correct.

Like Daniel I'm all for skinny controllers and fat models, but there is also a clear division of responsibilities. The purpose of the controller is to manage the incoming request and session. The model should be able to answer the question "Can user x do y to this object?", but it's nonsensical for it to reference the current_user. What if you are in the console? What if it's a cron job running?

In many cases with the right permissions API in the model, this can be handled with one-line before_filters that apply to several actions. However if things are getting more complex you may want to implement a separate layer (possibly in lib/) that encapsulates the more complex authorization logic to prevent your controller from becoming bloated, and prevent your model from becoming too tightly coupled to the web request/response cycle.

dasil003
Thanks, your concerns are convincing. I will have to investigate deeper into how some of the access control plugins for Rails are ticking.
knuton
+1  A: 

Although this question has been answered by many I just wanted to add my two cents in quickly.

Using the #current_user approach on the User model should be implemented with caution due to Thread Safety.

It is fine to use a class/singleton method on User if you remember to use Thread.current as a way or storing and retrieving your values. But it is not as easy as that because you also have to reset Thread.current so the next request does not inherit permissions it shouldn't.

The point I am trying to make is, if you store state in class or singleton variables, remember that you are throwing thread safety out the window.

Josh K
+10 if I could for this remark. Saving request state to any singleton class methods/variables is a Very Bad Idea.
molf